Make QtNetwork download manager great^H^H^H^Hlobal again

We originally made it per-window in b502280c06 for
issue #228, but that was back when we still needed window IDs for stuff like
message.info.

Nowadays, there's no reason for it to be per-window anymore. The rest of the
download code can deal with one global download manager (because QtWebEngine has
one), and apart from QNAM code which wasn't used here anyways (as tab_id=None)
there was nothing using the window ID anymore.

Also see #3456 which was the original motivation for this change.
This commit is contained in:
Florian Bruhin 2018-02-11 16:15:29 +01:00
parent e10940100d
commit c112290664
14 changed files with 29 additions and 50 deletions

View File

@ -64,7 +64,7 @@ from qutebrowser.completion.models import miscmodels
from qutebrowser.commands import cmdutils, runners, cmdexc from qutebrowser.commands import cmdutils, runners, cmdexc
from qutebrowser.config import config, websettings, configfiles, configinit from qutebrowser.config import config, websettings, configfiles, configinit
from qutebrowser.browser import (urlmarks, adblock, history, browsertab, from qutebrowser.browser import (urlmarks, adblock, history, browsertab,
downloads, greasemonkey) qtnetworkdownloads, downloads, greasemonkey)
from qutebrowser.browser.network import proxy from qutebrowser.browser.network import proxy
from qutebrowser.browser.webkit import cookies, cache from qutebrowser.browser.webkit import cookies, cache
from qutebrowser.browser.webkit.network import networkmanager from qutebrowser.browser.webkit.network import networkmanager
@ -491,6 +491,10 @@ def _init_modules(args, crash_handler):
diskcache = cache.DiskCache(standarddir.cache(), parent=qApp) diskcache = cache.DiskCache(standarddir.cache(), parent=qApp)
objreg.register('cache', diskcache) objreg.register('cache', diskcache)
log.init.debug("Initializing downloads...")
download_manager = qtnetworkdownloads.DownloadManager(parent=qApp)
objreg.register('qtnetwork-download-manager', download_manager)
log.init.debug("Initializing Greasemonkey...") log.init.debug("Initializing Greasemonkey...")
greasemonkey.init() greasemonkey.init()

View File

@ -176,8 +176,7 @@ class HostBlocker:
self._config_blocked_hosts) self._config_blocked_hosts)
self._blocked_hosts = set() self._blocked_hosts = set()
self._done_count = 0 self._done_count = 0
download_manager = objreg.get('qtnetwork-download-manager', download_manager = objreg.get('qtnetwork-download-manager')
scope='window', window='last-focused')
for url in config.val.content.host_blocking.lists: for url in config.val.content.host_blocking.lists:
if url.scheme() == 'file': if url.scheme() == 'file':
filename = url.toLocalFile() filename = url.toLocalFile()

View File

@ -1466,8 +1466,7 @@ class CommandDispatcher:
mhtml_: Download the current page and all assets as mhtml file. mhtml_: Download the current page and all assets as mhtml file.
""" """
# FIXME:qtwebengine do this with the QtWebEngine download manager? # FIXME:qtwebengine do this with the QtWebEngine download manager?
download_manager = objreg.get('qtnetwork-download-manager', download_manager = objreg.get('qtnetwork-download-manager')
scope='window', window=self._win_id)
target = None target = None
if dest is not None: if dest is not None:
dest = downloads.transform_path(dest) dest = downloads.transform_path(dest)

View File

@ -291,8 +291,7 @@ class HintActions:
user_agent = context.tab.user_agent() user_agent = context.tab.user_agent()
# FIXME:qtwebengine do this with QtWebEngine downloads? # FIXME:qtwebengine do this with QtWebEngine downloads?
download_manager = objreg.get('qtnetwork-download-manager', download_manager = objreg.get('qtnetwork-download-manager')
scope='window', window=self._win_id)
download_manager.get(url, qnam=qnam, user_agent=user_agent, download_manager.get(url, qnam=qnam, user_agent=user_agent,
prompt_download_directory=prompt) prompt_download_directory=prompt)

View File

@ -381,10 +381,10 @@ class DownloadManager(downloads.AbstractDownloadManager):
_networkmanager: A NetworkManager for generic downloads. _networkmanager: A NetworkManager for generic downloads.
""" """
def __init__(self, win_id, parent=None): def __init__(self, parent=None):
super().__init__(parent) super().__init__(parent)
self._networkmanager = networkmanager.NetworkManager( self._networkmanager = networkmanager.NetworkManager(
win_id=win_id, tab_id=None, win_id=None, tab_id=None,
private=config.val.content.private_browsing, parent=self) private=config.val.content.private_browsing, parent=self)
@pyqtSlot('QUrl') @pyqtSlot('QUrl')

View File

@ -254,7 +254,6 @@ class _Downloader:
_finished_file: A flag indicating if the file has already been _finished_file: A flag indicating if the file has already been
written. written.
_used: A flag indicating if the downloader has already been used. _used: A flag indicating if the downloader has already been used.
_win_id: The window this downloader belongs to.
""" """
def __init__(self, tab, target): def __init__(self, tab, target):
@ -265,7 +264,6 @@ class _Downloader:
self.pending_downloads = set() self.pending_downloads = set()
self._finished_file = False self._finished_file = False
self._used = False self._used = False
self._win_id = tab.win_id
def run(self): def run(self):
"""Download and save the page. """Download and save the page.
@ -365,8 +363,7 @@ class _Downloader:
self.writer.add_file(urlutils.encoded_url(url), b'') self.writer.add_file(urlutils.encoded_url(url), b'')
return return
download_manager = objreg.get('qtnetwork-download-manager', download_manager = objreg.get('qtnetwork-download-manager')
scope='window', window=self._win_id)
target = downloads.FileObjDownloadTarget(_NoCloseBytesIO()) target = downloads.FileObjDownloadTarget(_NoCloseBytesIO())
item = download_manager.get(url, target=target, item = download_manager.get(url, target=target,
auto_remove=True) auto_remove=True)

View File

@ -125,7 +125,9 @@ class NetworkManager(QNetworkAccessManager):
_scheme_handlers: A dictionary (scheme -> handler) of supported custom _scheme_handlers: A dictionary (scheme -> handler) of supported custom
schemes. schemes.
_win_id: The window ID this NetworkManager is associated with. _win_id: The window ID this NetworkManager is associated with.
(or None for generic network managers)
_tab_id: The tab ID this NetworkManager is associated with. _tab_id: The tab ID this NetworkManager is associated with.
(or None for generic network managers)
_rejected_ssl_errors: A {QUrl: [SslError]} dict of rejected errors. _rejected_ssl_errors: A {QUrl: [SslError]} dict of rejected errors.
_accepted_ssl_errors: A {QUrl: [SslError]} dict of accepted errors. _accepted_ssl_errors: A {QUrl: [SslError]} dict of accepted errors.
_private: Whether we're in private browsing mode. _private: Whether we're in private browsing mode.
@ -149,8 +151,8 @@ class NetworkManager(QNetworkAccessManager):
self._tab_id = tab_id self._tab_id = tab_id
self._private = private self._private = private
self._scheme_handlers = { self._scheme_handlers = {
'qute': webkitqutescheme.QuteSchemeHandler(win_id), 'qute': webkitqutescheme.QuteSchemeHandler(),
'file': filescheme.FileSchemeHandler(win_id), 'file': filescheme.FileSchemeHandler(),
} }
self._set_cookiejar() self._set_cookiejar()
self._set_cache() self._set_cache()
@ -194,6 +196,7 @@ class NetworkManager(QNetworkAccessManager):
# This might be a generic network manager, e.g. one belonging to a # This might be a generic network manager, e.g. one belonging to a
# DownloadManager. In this case, just skip the webview thing. # DownloadManager. In this case, just skip the webview thing.
if self._tab_id is not None: if self._tab_id is not None:
assert self._win_id is not None
tab = objreg.get('tab', scope='tab', window=self._win_id, tab = objreg.get('tab', scope='tab', window=self._win_id,
tab=self._tab_id) tab=self._tab_id)
abort_on.append(tab.load_started) abort_on.append(tab.load_started)
@ -392,6 +395,7 @@ class NetworkManager(QNetworkAccessManager):
current_url = QUrl() current_url = QUrl()
if self._tab_id is not None: if self._tab_id is not None:
assert self._win_id is not None
try: try:
tab = objreg.get('tab', scope='tab', window=self._win_id, tab = objreg.get('tab', scope='tab', window=self._win_id,
tab=self._tab_id) tab=self._tab_id)

View File

@ -27,15 +27,7 @@ from PyQt5.QtCore import QObject
class SchemeHandler(QObject): class SchemeHandler(QObject):
"""Abstract base class for custom scheme handlers. """Abstract base class for custom scheme handlers."""
Attributes:
_win_id: The window ID this scheme handler is associated with.
"""
def __init__(self, win_id, parent=None):
super().__init__(parent)
self._win_id = win_id
def createRequest(self, op, request, outgoing_data): def createRequest(self, op, request, outgoing_data):
"""Create a new request. """Create a new request.

View File

@ -220,8 +220,7 @@ class BrowserPage(QWebPage):
"""Prepare the web page for being deleted.""" """Prepare the web page for being deleted."""
self._is_shutting_down = True self._is_shutting_down = True
self.shutting_down.emit() self.shutting_down.emit()
download_manager = objreg.get('qtnetwork-download-manager', download_manager = objreg.get('qtnetwork-download-manager')
scope='window', window=self._win_id)
nam = self.networkAccessManager() nam = self.networkAccessManager()
if download_manager.has_downloads_with_nam(nam): if download_manager.has_downloads_with_nam(nam):
nam.setParent(download_manager) nam.setParent(download_manager)
@ -249,8 +248,7 @@ class BrowserPage(QWebPage):
after this slot returns. after this slot returns.
""" """
req = QNetworkRequest(request) req = QNetworkRequest(request)
download_manager = objreg.get('qtnetwork-download-manager', download_manager = objreg.get('qtnetwork-download-manager')
scope='window', window=self._win_id)
download_manager.get_request(req, qnam=self.networkAccessManager()) download_manager.get_request(req, qnam=self.networkAccessManager())
@pyqtSlot('QNetworkReply*') @pyqtSlot('QNetworkReply*')
@ -264,8 +262,7 @@ class BrowserPage(QWebPage):
here: http://mimesniff.spec.whatwg.org/ here: http://mimesniff.spec.whatwg.org/
""" """
inline, suggested_filename = http.parse_content_disposition(reply) inline, suggested_filename = http.parse_content_disposition(reply)
download_manager = objreg.get('qtnetwork-download-manager', download_manager = objreg.get('qtnetwork-download-manager')
scope='window', window=self._win_id)
if not inline: if not inline:
# Content-Disposition: attachment -> force download # Content-Disposition: attachment -> force download
download_manager.fetch(reply, download_manager.fetch(reply,

View File

@ -34,8 +34,7 @@ from qutebrowser.utils import (message, log, usertypes, qtutils, objreg, utils,
from qutebrowser.mainwindow import messageview, prompt from qutebrowser.mainwindow import messageview, prompt
from qutebrowser.completion import completionwidget, completer from qutebrowser.completion import completionwidget, completer
from qutebrowser.keyinput import modeman from qutebrowser.keyinput import modeman
from qutebrowser.browser import (commands, downloadview, hints, from qutebrowser.browser import commands, downloadview, hints, downloads
qtnetworkdownloads, downloads)
from qutebrowser.misc import crashsignal, keyhintwidget from qutebrowser.misc import crashsignal, keyhintwidget
@ -299,11 +298,7 @@ class MainWindow(QWidget):
def _init_downloadmanager(self): def _init_downloadmanager(self):
log.init.debug("Initializing downloads...") log.init.debug("Initializing downloads...")
qtnetwork_download_manager = qtnetworkdownloads.DownloadManager( qtnetwork_download_manager = objreg.get('qtnetwork-download-manager')
self.win_id, self)
objreg.register('qtnetwork-download-manager',
qtnetwork_download_manager,
scope='window', window=self.win_id)
try: try:
webengine_download_manager = objreg.get( webengine_download_manager = objreg.get(

View File

@ -104,11 +104,9 @@ class FakeDownloadManager:
def download_stub(win_registry, tmpdir): def download_stub(win_registry, tmpdir):
"""Register a FakeDownloadManager.""" """Register a FakeDownloadManager."""
stub = FakeDownloadManager(tmpdir) stub = FakeDownloadManager(tmpdir)
objreg.register('qtnetwork-download-manager', stub, objreg.register('qtnetwork-download-manager', stub)
scope='window', window='last-focused')
yield yield
objreg.delete('qtnetwork-download-manager', scope='window', objreg.delete('qtnetwork-download-manager')
window='last-focused')
def create_zipfile(directory, files, zipname='test'): def create_zipfile(directory, files, zipname='test'):

View File

@ -248,7 +248,7 @@ class TestFileSchemeHandler:
def test_dir(self, tmpdir): def test_dir(self, tmpdir):
url = QUrl.fromLocalFile(str(tmpdir)) url = QUrl.fromLocalFile(str(tmpdir))
req = QNetworkRequest(url) req = QNetworkRequest(url)
handler = filescheme.FileSchemeHandler(win_id=0) handler = filescheme.FileSchemeHandler()
reply = handler.createRequest(None, req, None) reply = handler.createRequest(None, req, None)
# The URL will always use /, even on Windows - so we force this here # The URL will always use /, even on Windows - so we force this here
# too. # too.
@ -260,14 +260,14 @@ class TestFileSchemeHandler:
filename.ensure() filename.ensure()
url = QUrl.fromLocalFile(str(filename)) url = QUrl.fromLocalFile(str(filename))
req = QNetworkRequest(url) req = QNetworkRequest(url)
handler = filescheme.FileSchemeHandler(win_id=0) handler = filescheme.FileSchemeHandler()
reply = handler.createRequest(None, req, None) reply = handler.createRequest(None, req, None)
assert reply is None assert reply is None
def test_unicode_encode_error(self, mocker): def test_unicode_encode_error(self, mocker):
url = QUrl('file:///tmp/foo') url = QUrl('file:///tmp/foo')
req = QNetworkRequest(url) req = QNetworkRequest(url)
handler = filescheme.FileSchemeHandler(win_id=0) handler = filescheme.FileSchemeHandler()
err = UnicodeEncodeError('ascii', '', 0, 2, 'foo') err = UnicodeEncodeError('ascii', '', 0, 2, 'foo')
mocker.patch('os.path.isdir', side_effect=err) mocker.patch('os.path.isdir', side_effect=err)

View File

@ -24,11 +24,6 @@ import pytest
from qutebrowser.browser.webkit.network import schemehandler from qutebrowser.browser.webkit.network import schemehandler
def test_init():
handler = schemehandler.SchemeHandler(0)
assert handler._win_id == 0
def test_create_request(): def test_create_request():
handler = schemehandler.SchemeHandler(0) handler = schemehandler.SchemeHandler(0)
with pytest.raises(NotImplementedError): with pytest.raises(NotImplementedError):

View File

@ -24,7 +24,7 @@ from qutebrowser.browser import downloads, qtnetworkdownloads
def test_download_model(qapp, qtmodeltester, config_stub, cookiejar_and_cache): def test_download_model(qapp, qtmodeltester, config_stub, cookiejar_and_cache):
"""Simple check for download model internals.""" """Simple check for download model internals."""
manager = qtnetworkdownloads.DownloadManager(win_id=0) manager = qtnetworkdownloads.DownloadManager()
model = downloads.DownloadModel(manager) model = downloads.DownloadModel(manager)
qtmodeltester.check(model) qtmodeltester.check(model)