Title: [207454] trunk/Source/WebKit2
Revision
207454
Author
carlo...@webkit.org
Date
2016-10-17 23:03:55 -0700 (Mon, 17 Oct 2016)

Log Message

NetworkSession: PendingDownload is leaked if canceled before willDecidePendingDownloadDestination
https://bugs.webkit.org/show_bug.cgi?id=163545

Reviewed by Alex Christensen.

If a download started by DownloadManager::startDownload() is cancelled before
DownloadManager::willDecidePendingDownloadDestination() is called, DownloadManager::cancelDownload() does
nothing, because the Download hasn't been created yet and m_downloadsWaitingForDestination map doesn't contain
the download ID, and the PendingDownload is never removed from the m_pendingDownloads map.

* NetworkProcess/Downloads/DownloadManager.cpp:
(WebKit::DownloadManager::cancelDownload): Always take the PendingDownload from m_pendingDownloads map. Then, if
the download was already in m_downloadsWaitingForDestination map, get the network data task to properly cancel
it and then call the completion handler to ignore the request. Otherwise cancel the pending download if exists.
* NetworkProcess/Downloads/PendingDownload.cpp:
(WebKit::PendingDownload::cancel): Cancel the network load and notify the UI process that the download was canceled.
* NetworkProcess/Downloads/PendingDownload.h:

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (207453 => 207454)


--- trunk/Source/WebKit2/ChangeLog	2016-10-18 02:02:48 UTC (rev 207453)
+++ trunk/Source/WebKit2/ChangeLog	2016-10-18 06:03:55 UTC (rev 207454)
@@ -1,3 +1,23 @@
+2016-10-17  Carlos Garcia Campos  <cgar...@igalia.com>
+
+        NetworkSession: PendingDownload is leaked if canceled before willDecidePendingDownloadDestination
+        https://bugs.webkit.org/show_bug.cgi?id=163545
+
+        Reviewed by Alex Christensen.
+
+        If a download started by DownloadManager::startDownload() is cancelled before
+        DownloadManager::willDecidePendingDownloadDestination() is called, DownloadManager::cancelDownload() does
+        nothing, because the Download hasn't been created yet and m_downloadsWaitingForDestination map doesn't contain
+        the download ID, and the PendingDownload is never removed from the m_pendingDownloads map.
+
+        * NetworkProcess/Downloads/DownloadManager.cpp:
+        (WebKit::DownloadManager::cancelDownload): Always take the PendingDownload from m_pendingDownloads map. Then, if
+        the download was already in m_downloadsWaitingForDestination map, get the network data task to properly cancel
+        it and then call the completion handler to ignore the request. Otherwise cancel the pending download if exists.
+        * NetworkProcess/Downloads/PendingDownload.cpp:
+        (WebKit::PendingDownload::cancel): Cancel the network load and notify the UI process that the download was canceled.
+        * NetworkProcess/Downloads/PendingDownload.h:
+
 2016-10-17  Megan Gardner  <megan_gard...@apple.com>
 
         Add test and infrastructure for link popover

Modified: trunk/Source/WebKit2/NetworkProcess/Downloads/DownloadManager.cpp (207453 => 207454)


--- trunk/Source/WebKit2/NetworkProcess/Downloads/DownloadManager.cpp	2016-10-18 02:02:48 UTC (rev 207453)
+++ trunk/Source/WebKit2/NetworkProcess/Downloads/DownloadManager.cpp	2016-10-18 06:03:55 UTC (rev 207454)
@@ -158,15 +158,30 @@
 void DownloadManager::cancelDownload(DownloadID downloadID)
 {
     if (Download* download = m_downloads.get(downloadID)) {
+#if USE(NETWORK_SESSION)
+        ASSERT(!m_downloadsWaitingForDestination.contains(downloadID));
+        ASSERT(!m_pendingDownloads.contains(downloadID));
+#endif
         download->cancel();
         return;
     }
 #if USE(NETWORK_SESSION)
-    if (auto completionHandler = m_downloadsWaitingForDestination.take(downloadID).second) {
+    auto pendingDownload = m_pendingDownloads.take(downloadID);
+    if (m_downloadsWaitingForDestination.contains(downloadID)) {
+        auto pair = m_downloadsWaitingForDestination.take(downloadID);
+        auto networkDataTask = WTFMove(pair.first);
+        auto completionHandler = WTFMove(pair.second);
+        ASSERT(networkDataTask);
+        ASSERT(completionHandler);
+
+        networkDataTask->cancel();
+        completionHandler(PolicyIgnore);
         m_client.pendingDownloadCanceled(downloadID);
-        completionHandler(PolicyIgnore);
         return;
     }
+
+    if (pendingDownload)
+        pendingDownload->cancel();
 #endif
 }
 

Modified: trunk/Source/WebKit2/NetworkProcess/Downloads/PendingDownload.cpp (207453 => 207454)


--- trunk/Source/WebKit2/NetworkProcess/Downloads/PendingDownload.cpp	2016-10-18 02:02:48 UTC (rev 207453)
+++ trunk/Source/WebKit2/NetworkProcess/Downloads/PendingDownload.cpp	2016-10-18 06:03:55 UTC (rev 207454)
@@ -58,6 +58,13 @@
     m_networkLoad->continueWillSendRequest(WTFMove(newRequest));
 }
 
+void PendingDownload::cancel()
+{
+    ASSERT(m_networkLoad);
+    m_networkLoad->cancel();
+    send(Messages::DownloadProxy::DidCancel({ }));
+}
+
 #if USE(PROTECTION_SPACE_AUTH_CALLBACK)
 void PendingDownload::canAuthenticateAgainstProtectionSpaceAsync(const WebCore::ProtectionSpace& protectionSpace)
 {

Modified: trunk/Source/WebKit2/NetworkProcess/Downloads/PendingDownload.h (207453 => 207454)


--- trunk/Source/WebKit2/NetworkProcess/Downloads/PendingDownload.h	2016-10-18 02:02:48 UTC (rev 207453)
+++ trunk/Source/WebKit2/NetworkProcess/Downloads/PendingDownload.h	2016-10-18 06:03:55 UTC (rev 207454)
@@ -51,6 +51,7 @@
 #if USE(PROTECTION_SPACE_AUTH_CALLBACK)
     void continueCanAuthenticateAgainstProtectionSpace(bool canAuthenticate);
 #endif
+    void cancel();
 
 private:    
     // NetworkLoadClient.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to