From c1122906648f0e39220930f028dbc254fa521a10 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sun, 11 Feb 2018 16:15:29 +0100 Subject: [PATCH] Make QtNetwork download manager great^H^H^H^Hlobal again We originally made it per-window in b502280c062acbc15a80966a9cabac7babecf0c1 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. --- qutebrowser/app.py | 6 +++++- qutebrowser/browser/adblock.py | 3 +-- qutebrowser/browser/commands.py | 3 +-- qutebrowser/browser/hints.py | 3 +-- qutebrowser/browser/qtnetworkdownloads.py | 4 ++-- qutebrowser/browser/webkit/mhtml.py | 5 +---- qutebrowser/browser/webkit/network/networkmanager.py | 8 ++++++-- qutebrowser/browser/webkit/network/schemehandler.py | 10 +--------- qutebrowser/browser/webkit/webpage.py | 9 +++------ qutebrowser/mainwindow/mainwindow.py | 9 ++------- tests/unit/browser/test_adblock.py | 6 ++---- tests/unit/browser/webkit/network/test_filescheme.py | 6 +++--- .../unit/browser/webkit/network/test_schemehandler.py | 5 ----- tests/unit/browser/webkit/test_downloads.py | 2 +- 14 files changed, 29 insertions(+), 50 deletions(-) diff --git a/qutebrowser/app.py b/qutebrowser/app.py index c87201931..ec477ce8f 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -64,7 +64,7 @@ from qutebrowser.completion.models import miscmodels from qutebrowser.commands import cmdutils, runners, cmdexc from qutebrowser.config import config, websettings, configfiles, configinit from qutebrowser.browser import (urlmarks, adblock, history, browsertab, - downloads, greasemonkey) + qtnetworkdownloads, downloads, greasemonkey) from qutebrowser.browser.network import proxy from qutebrowser.browser.webkit import cookies, cache from qutebrowser.browser.webkit.network import networkmanager @@ -491,6 +491,10 @@ def _init_modules(args, crash_handler): diskcache = cache.DiskCache(standarddir.cache(), parent=qApp) 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...") greasemonkey.init() diff --git a/qutebrowser/browser/adblock.py b/qutebrowser/browser/adblock.py index fa35684b9..f0462a778 100644 --- a/qutebrowser/browser/adblock.py +++ b/qutebrowser/browser/adblock.py @@ -176,8 +176,7 @@ class HostBlocker: self._config_blocked_hosts) self._blocked_hosts = set() self._done_count = 0 - download_manager = objreg.get('qtnetwork-download-manager', - scope='window', window='last-focused') + download_manager = objreg.get('qtnetwork-download-manager') for url in config.val.content.host_blocking.lists: if url.scheme() == 'file': filename = url.toLocalFile() diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 85f59bb00..b56c3d6ae 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1466,8 +1466,7 @@ class CommandDispatcher: mhtml_: Download the current page and all assets as mhtml file. """ # FIXME:qtwebengine do this with the QtWebEngine download manager? - download_manager = objreg.get('qtnetwork-download-manager', - scope='window', window=self._win_id) + download_manager = objreg.get('qtnetwork-download-manager') target = None if dest is not None: dest = downloads.transform_path(dest) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 3e0eddbeb..0390d5d1f 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -291,8 +291,7 @@ class HintActions: user_agent = context.tab.user_agent() # FIXME:qtwebengine do this with QtWebEngine downloads? - download_manager = objreg.get('qtnetwork-download-manager', - scope='window', window=self._win_id) + download_manager = objreg.get('qtnetwork-download-manager') download_manager.get(url, qnam=qnam, user_agent=user_agent, prompt_download_directory=prompt) diff --git a/qutebrowser/browser/qtnetworkdownloads.py b/qutebrowser/browser/qtnetworkdownloads.py index 746fd8522..82996a803 100644 --- a/qutebrowser/browser/qtnetworkdownloads.py +++ b/qutebrowser/browser/qtnetworkdownloads.py @@ -381,10 +381,10 @@ class DownloadManager(downloads.AbstractDownloadManager): _networkmanager: A NetworkManager for generic downloads. """ - def __init__(self, win_id, parent=None): + def __init__(self, parent=None): super().__init__(parent) 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) @pyqtSlot('QUrl') diff --git a/qutebrowser/browser/webkit/mhtml.py b/qutebrowser/browser/webkit/mhtml.py index ca807f705..5f495274a 100644 --- a/qutebrowser/browser/webkit/mhtml.py +++ b/qutebrowser/browser/webkit/mhtml.py @@ -254,7 +254,6 @@ class _Downloader: _finished_file: A flag indicating if the file has already been written. _used: A flag indicating if the downloader has already been used. - _win_id: The window this downloader belongs to. """ def __init__(self, tab, target): @@ -265,7 +264,6 @@ class _Downloader: self.pending_downloads = set() self._finished_file = False self._used = False - self._win_id = tab.win_id def run(self): """Download and save the page. @@ -365,8 +363,7 @@ class _Downloader: self.writer.add_file(urlutils.encoded_url(url), b'') return - download_manager = objreg.get('qtnetwork-download-manager', - scope='window', window=self._win_id) + download_manager = objreg.get('qtnetwork-download-manager') target = downloads.FileObjDownloadTarget(_NoCloseBytesIO()) item = download_manager.get(url, target=target, auto_remove=True) diff --git a/qutebrowser/browser/webkit/network/networkmanager.py b/qutebrowser/browser/webkit/network/networkmanager.py index 8ad486d29..49bf6ebab 100644 --- a/qutebrowser/browser/webkit/network/networkmanager.py +++ b/qutebrowser/browser/webkit/network/networkmanager.py @@ -125,7 +125,9 @@ class NetworkManager(QNetworkAccessManager): _scheme_handlers: A dictionary (scheme -> handler) of supported custom schemes. _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. + (or None for generic network managers) _rejected_ssl_errors: A {QUrl: [SslError]} dict of rejected errors. _accepted_ssl_errors: A {QUrl: [SslError]} dict of accepted errors. _private: Whether we're in private browsing mode. @@ -149,8 +151,8 @@ class NetworkManager(QNetworkAccessManager): self._tab_id = tab_id self._private = private self._scheme_handlers = { - 'qute': webkitqutescheme.QuteSchemeHandler(win_id), - 'file': filescheme.FileSchemeHandler(win_id), + 'qute': webkitqutescheme.QuteSchemeHandler(), + 'file': filescheme.FileSchemeHandler(), } self._set_cookiejar() self._set_cache() @@ -194,6 +196,7 @@ class NetworkManager(QNetworkAccessManager): # This might be a generic network manager, e.g. one belonging to a # DownloadManager. In this case, just skip the webview thing. 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=self._tab_id) abort_on.append(tab.load_started) @@ -392,6 +395,7 @@ class NetworkManager(QNetworkAccessManager): current_url = QUrl() if self._tab_id is not None: + assert self._win_id is not None try: tab = objreg.get('tab', scope='tab', window=self._win_id, tab=self._tab_id) diff --git a/qutebrowser/browser/webkit/network/schemehandler.py b/qutebrowser/browser/webkit/network/schemehandler.py index c50194172..278d7e3a3 100644 --- a/qutebrowser/browser/webkit/network/schemehandler.py +++ b/qutebrowser/browser/webkit/network/schemehandler.py @@ -27,15 +27,7 @@ from PyQt5.QtCore import QObject class SchemeHandler(QObject): - """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 + """Abstract base class for custom scheme handlers.""" def createRequest(self, op, request, outgoing_data): """Create a new request. diff --git a/qutebrowser/browser/webkit/webpage.py b/qutebrowser/browser/webkit/webpage.py index 52107a17b..679ec2d88 100644 --- a/qutebrowser/browser/webkit/webpage.py +++ b/qutebrowser/browser/webkit/webpage.py @@ -220,8 +220,7 @@ class BrowserPage(QWebPage): """Prepare the web page for being deleted.""" self._is_shutting_down = True self.shutting_down.emit() - download_manager = objreg.get('qtnetwork-download-manager', - scope='window', window=self._win_id) + download_manager = objreg.get('qtnetwork-download-manager') nam = self.networkAccessManager() if download_manager.has_downloads_with_nam(nam): nam.setParent(download_manager) @@ -249,8 +248,7 @@ class BrowserPage(QWebPage): after this slot returns. """ req = QNetworkRequest(request) - download_manager = objreg.get('qtnetwork-download-manager', - scope='window', window=self._win_id) + download_manager = objreg.get('qtnetwork-download-manager') download_manager.get_request(req, qnam=self.networkAccessManager()) @pyqtSlot('QNetworkReply*') @@ -264,8 +262,7 @@ class BrowserPage(QWebPage): here: http://mimesniff.spec.whatwg.org/ """ inline, suggested_filename = http.parse_content_disposition(reply) - download_manager = objreg.get('qtnetwork-download-manager', - scope='window', window=self._win_id) + download_manager = objreg.get('qtnetwork-download-manager') if not inline: # Content-Disposition: attachment -> force download download_manager.fetch(reply, diff --git a/qutebrowser/mainwindow/mainwindow.py b/qutebrowser/mainwindow/mainwindow.py index 235c15295..05482a1d5 100644 --- a/qutebrowser/mainwindow/mainwindow.py +++ b/qutebrowser/mainwindow/mainwindow.py @@ -34,8 +34,7 @@ from qutebrowser.utils import (message, log, usertypes, qtutils, objreg, utils, from qutebrowser.mainwindow import messageview, prompt from qutebrowser.completion import completionwidget, completer from qutebrowser.keyinput import modeman -from qutebrowser.browser import (commands, downloadview, hints, - qtnetworkdownloads, downloads) +from qutebrowser.browser import commands, downloadview, hints, downloads from qutebrowser.misc import crashsignal, keyhintwidget @@ -299,11 +298,7 @@ class MainWindow(QWidget): def _init_downloadmanager(self): log.init.debug("Initializing downloads...") - qtnetwork_download_manager = qtnetworkdownloads.DownloadManager( - self.win_id, self) - objreg.register('qtnetwork-download-manager', - qtnetwork_download_manager, - scope='window', window=self.win_id) + qtnetwork_download_manager = objreg.get('qtnetwork-download-manager') try: webengine_download_manager = objreg.get( diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index cbb2a2a6d..09161e806 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -104,11 +104,9 @@ class FakeDownloadManager: def download_stub(win_registry, tmpdir): """Register a FakeDownloadManager.""" stub = FakeDownloadManager(tmpdir) - objreg.register('qtnetwork-download-manager', stub, - scope='window', window='last-focused') + objreg.register('qtnetwork-download-manager', stub) yield - objreg.delete('qtnetwork-download-manager', scope='window', - window='last-focused') + objreg.delete('qtnetwork-download-manager') def create_zipfile(directory, files, zipname='test'): diff --git a/tests/unit/browser/webkit/network/test_filescheme.py b/tests/unit/browser/webkit/network/test_filescheme.py index 8b3287830..50b908dc0 100644 --- a/tests/unit/browser/webkit/network/test_filescheme.py +++ b/tests/unit/browser/webkit/network/test_filescheme.py @@ -248,7 +248,7 @@ class TestFileSchemeHandler: def test_dir(self, tmpdir): url = QUrl.fromLocalFile(str(tmpdir)) req = QNetworkRequest(url) - handler = filescheme.FileSchemeHandler(win_id=0) + handler = filescheme.FileSchemeHandler() reply = handler.createRequest(None, req, None) # The URL will always use /, even on Windows - so we force this here # too. @@ -260,14 +260,14 @@ class TestFileSchemeHandler: filename.ensure() url = QUrl.fromLocalFile(str(filename)) req = QNetworkRequest(url) - handler = filescheme.FileSchemeHandler(win_id=0) + handler = filescheme.FileSchemeHandler() reply = handler.createRequest(None, req, None) assert reply is None def test_unicode_encode_error(self, mocker): url = QUrl('file:///tmp/foo') req = QNetworkRequest(url) - handler = filescheme.FileSchemeHandler(win_id=0) + handler = filescheme.FileSchemeHandler() err = UnicodeEncodeError('ascii', '', 0, 2, 'foo') mocker.patch('os.path.isdir', side_effect=err) diff --git a/tests/unit/browser/webkit/network/test_schemehandler.py b/tests/unit/browser/webkit/network/test_schemehandler.py index 24e54e0d9..cb1ac3dec 100644 --- a/tests/unit/browser/webkit/network/test_schemehandler.py +++ b/tests/unit/browser/webkit/network/test_schemehandler.py @@ -24,11 +24,6 @@ import pytest from qutebrowser.browser.webkit.network import schemehandler -def test_init(): - handler = schemehandler.SchemeHandler(0) - assert handler._win_id == 0 - - def test_create_request(): handler = schemehandler.SchemeHandler(0) with pytest.raises(NotImplementedError): diff --git a/tests/unit/browser/webkit/test_downloads.py b/tests/unit/browser/webkit/test_downloads.py index da1fcb424..87fff7244 100644 --- a/tests/unit/browser/webkit/test_downloads.py +++ b/tests/unit/browser/webkit/test_downloads.py @@ -24,7 +24,7 @@ from qutebrowser.browser import downloads, qtnetworkdownloads def test_download_model(qapp, qtmodeltester, config_stub, cookiejar_and_cache): """Simple check for download model internals.""" - manager = qtnetworkdownloads.DownloadManager(win_id=0) + manager = qtnetworkdownloads.DownloadManager() model = downloads.DownloadModel(manager) qtmodeltester.check(model)