Use per-tab QNAM for QtNetwork downloads again

When starting a download due to unsupportedContent being emitted, we
need to use (and later adopt) the page's QNetworkAccessManager.

Since we need the whole adopting logic for that case anyways, let's keep
things as they were and always run downloads in per-tab QNAMs.

This reverts 53e360ec4b and fixes #2134.

qutebrowser/browser/commands.py

@@ -1332,11 +1332,22 @@ class CommandDispatcher:
         elif mhtml_:
             self._download_mhtml(dest)
         else:
+            tab = self._current_widget()
+
+            # FIXME:qtwebengine have a proper API for this
+            # pylint: disable=protected-access
+            try:
+                qnam = tab._widget.page().networkAccessManager()
+            except AttributeError:
+                # QtWebEngine
+                qnam = None
+            # pylint: enable=protected-access
+
             if dest is None:
                 target = None
             else:
                 target = downloads.FileDownloadTarget(dest)
-            download_manager.get(self._current_url(), target=target)
+            download_manager.get(self._current_url(), qnam=qnam, target=target)
 
     def _download_mhtml(self, dest=None):
         """Download the current page as an MHTML file, including all assets.

qutebrowser/browser/hints.py

@@ -284,10 +284,19 @@ class HintActions:
         else:
             prompt = None
 
+        # FIXME:qtwebengine get a proper API for this
+        # pylint: disable=protected-access
+        try:
+            qnam = elem._elem.webFrame().page().networkAccessManager()
+        except AttributeError:
+            # QtWebEngine
+            qnam = None
+        # pylint: enable=protected-access
+
         # FIXME:qtwebengine do this with QtWebEngine downloads?
         download_manager = objreg.get('qtnetwork-download-manager',
                                       scope='window', window=self._win_id)
-        download_manager.get(url, prompt_download_directory=prompt)
+        download_manager.get(url, qnam=qnam, prompt_download_directory=prompt)
 
     def call_userscript(self, elem, context):
         """Call a userscript from a hint.

qutebrowser/browser/qtnetworkdownloads.py

@@ -24,7 +24,7 @@ import shutil
 import functools
 import collections
 
-from PyQt5.QtCore import pyqtSlot, QTimer
+from PyQt5.QtCore import pyqtSlot, pyqtSignal, QTimer
 from PyQt5.QtNetwork import QNetworkRequest, QNetworkReply
 
 from qutebrowser.utils import message, usertypes, log, urlutils
@@ -67,9 +67,15 @@ class DownloadItem(downloads.AbstractDownloadItem):
         _reply: The QNetworkReply associated with this download.
         _autoclose: Whether to close the associated file when the download is
                     done.
+
+    Signals:
+        adopt_download: Emitted when a download is retried and should be
+                        adopted by the QNAM if needed.
+                        arg 0: The new DownloadItem
     """
 
     _MAX_REDIRECTS = 10
+    adopt_download = pyqtSignal(object)  # DownloadItem
 
     def __init__(self, reply, manager):
         """Constructor.
@@ -162,7 +168,9 @@ class DownloadItem(downloads.AbstractDownloadItem):
         assert self.done
         assert not self.successful
         new_reply = self._retry_info.manager.get(self._retry_info.request)
-        self._manager.fetch(new_reply, suggested_filename=self.basename)
+        new_download = self._manager.fetch(new_reply,
+                                           suggested_filename=self.basename)
+        self.adopt_download.emit(new_download)
         self.cancel()
 
     def _get_open_filename(self):
@@ -335,6 +343,14 @@ class DownloadItem(downloads.AbstractDownloadItem):
         old_reply.deleteLater()
         return True
 
+    def _uses_nam(self, nam):
+        """Check if this download uses the given QNetworkAccessManager."""
+        running_nam = self._reply is not None and self._reply.manager() is nam
+        # user could request retry after tab is closed.
+        retry_nam = (self.done and (not self.successful) and
+                     self._retry_info.manager is nam)
+        return running_nam or retry_nam
+
 
 class DownloadManager(downloads.AbstractDownloadManager):
 
@@ -409,17 +425,20 @@ class DownloadManager(downloads.AbstractDownloadManager):
                                    suggested_filename=suggested_fn,
                                    **kwargs)
 
-    def _fetch_request(self, request, **kwargs):
+    def _fetch_request(self, request, *, qnam=None, **kwargs):
         """Download a QNetworkRequest to disk.
 
         Args:
             request: The QNetworkRequest to download.
+            qnam: The QNetworkAccessManager to use.
             **kwargs: passed to fetch().
 
         Return:
             The created DownloadItem.
         """
-        reply = self._networkmanager.get(request)
+        if qnam is None:
+            qnam = self._networkmanager
+        reply = qnam.get(request)
         return self.fetch(reply, **kwargs)
 
     @pyqtSlot('QNetworkReply')
@@ -467,3 +486,18 @@ class DownloadManager(downloads.AbstractDownloadManager):
         message.global_bridge.ask(question, blocking=False)
 
         return download
+
+    def has_downloads_with_nam(self, nam):
+        """Check if the DownloadManager has any downloads with the given QNAM.
+
+        Args:
+            nam: The QNetworkAccessManager to check.
+
+        Return:
+            A boolean.
+        """
+        assert nam.adopted_downloads == 0
+        for download in self.downloads:
+            if download._uses_nam(nam):  # pylint: disable=protected-access
+                nam.adopt_download(download)
+        return nam.adopted_downloads

qutebrowser/browser/webkit/network/networkmanager.py

@@ -116,6 +116,11 @@ class NetworkManager(QNetworkAccessManager):
     """Our own QNetworkAccessManager.
 
     Attributes:
+        adopted_downloads: If downloads are running with this QNAM but the
+                           associated tab gets closed already, the NAM gets
+                           reparented to the DownloadManager. This counts the
+                           still running downloads, so the QNAM can clean
+                           itself up when this reaches zero again.
         _requests: Pending requests.
         _scheme_handlers: A dictionary (scheme -> handler) of supported custom
                           schemes.
@@ -137,6 +142,7 @@ class NetworkManager(QNetworkAccessManager):
             # http://www.riverbankcomputing.com/pipermail/pyqt/2014-November/035045.html
             super().__init__(parent)
         log.init.debug("NetworkManager init done")
+        self.adopted_downloads = 0
         self._win_id = win_id
         self._tab_id = tab_id
         self._requests = []
@@ -328,6 +334,28 @@ class NetworkManager(QNetworkAccessManager):
             # switched from private mode to normal mode
             self._set_cookiejar()
 
+    @pyqtSlot()
+    def on_adopted_download_destroyed(self):
+        """Check if we can clean up if an adopted download was destroyed.
+
+        See the description for adopted_downloads for details.
+        """
+        self.adopted_downloads -= 1
+        log.downloads.debug("Adopted download destroyed, {} left.".format(
+            self.adopted_downloads))
+        assert self.adopted_downloads >= 0
+        if self.adopted_downloads == 0:
+            self.deleteLater()
+
+    @pyqtSlot(object)  # DownloadItem
+    def adopt_download(self, download):
+        """Adopt a new DownloadItem."""
+        self.adopted_downloads += 1
+        log.downloads.debug("Adopted download, {} adopted.".format(
+            self.adopted_downloads))
+        download.destroyed.connect(self.on_adopted_download_destroyed)
+        download.adopt_download.connect(self.adopt_download)
+
     def set_referer(self, req, current_url):
         """Set the referer header."""
         referer_header_conf = config.get('network', 'referer-header')

qutebrowser/browser/webkit/webpage.py

@@ -210,7 +210,13 @@ class BrowserPage(QWebPage):
         """Prepare the web page for being deleted."""
         self._is_shutting_down = True
         self.shutting_down.emit()
-        self.networkAccessManager().shutdown()
+        download_manager = objreg.get('qtnetwork-download-manager',
+                                      scope='window', window=self._win_id)
+        nam = self.networkAccessManager()
+        if download_manager.has_downloads_with_nam(nam):
+            nam.setParent(download_manager)
+        else:
+            nam.shutdown()
 
     def display_content(self, reply, mimetype):
         """Display a QNetworkReply with an explicitly set mimetype."""
@@ -239,7 +245,7 @@ class BrowserPage(QWebPage):
         req = QNetworkRequest(request)
         download_manager = objreg.get('qtnetwork-download-manager',
                                       scope='window', window=self._win_id)
-        download_manager.get_request(req)
+        download_manager.get_request(req, qnam=self.networkAccessManager())
 
     @pyqtSlot('QNetworkReply*')
     def on_unsupported_content(self, reply):

tests/end2end/features/downloads.feature

@@ -100,6 +100,19 @@ Feature: Downloading things from a website.
         And I run :close
         Then qutebrowser should quit
 
+    # https://github.com/The-Compiler/qutebrowser/issues/2134
+    @qtwebengine_skip
+    Scenario: Downloading, then closing a tab
+        When I set storage -> prompt-download-directory to false
+        And I open about:blank
+        And I open data/downloads/issue2134.html in a new tab
+        # This needs to be a download connected to the tabs QNAM
+        And I hint with args "links normal" and follow a
+        And I wait for "fetch: * -> drip" in the log
+        And I run :tab-close
+        And I wait for "Download drip finished" in the log
+        Then the downloaded file drip should contain 128 bytes
+
     ## :download-retry
 
     Scenario: Retrying a failed download