Title: [207042] trunk/Source/WebKit2
Revision
207042
Author
carlo...@webkit.org
Date
2016-10-10 22:28:44 -0700 (Mon, 10 Oct 2016)

Log Message

NetworkSession: NetworkDataTask is leaked if download finishes in didReceiveResponse completion handler
https://bugs.webkit.org/show_bug.cgi?id=163204

Reviewed by Alex Christensen.

After the completion handler a reference of the NetworkDataTask is saved in m_downloadsAfterDestinationDecided.
If the download failed or was canceled DownloadManager::dataTaskBecameDownloadTask is never called and the data
task is kept in the download manager forever. This patch exposes NSURLSessionTask state property in
NetworkDataTask, so that the download manager can check the task state after the completion handler and return
early if the download finished or was cancelled.

* NetworkProcess/Downloads/DownloadManager.cpp:
(WebKit::DownloadManager::continueDecidePendingDownloadDestination):
* NetworkProcess/NetworkDataTask.h:
* NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
(WebKit::NetworkDataTask::state):

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (207041 => 207042)


--- trunk/Source/WebKit2/ChangeLog	2016-10-11 04:43:26 UTC (rev 207041)
+++ trunk/Source/WebKit2/ChangeLog	2016-10-11 05:28:44 UTC (rev 207042)
@@ -1,3 +1,22 @@
+2016-10-10  Carlos Garcia Campos  <cgar...@igalia.com>
+
+        NetworkSession: NetworkDataTask is leaked if download finishes in didReceiveResponse completion handler
+        https://bugs.webkit.org/show_bug.cgi?id=163204
+
+        Reviewed by Alex Christensen.
+
+        After the completion handler a reference of the NetworkDataTask is saved in m_downloadsAfterDestinationDecided.
+        If the download failed or was canceled DownloadManager::dataTaskBecameDownloadTask is never called and the data
+        task is kept in the download manager forever. This patch exposes NSURLSessionTask state property in
+        NetworkDataTask, so that the download manager can check the task state after the completion handler and return
+        early if the download finished or was cancelled.
+
+        * NetworkProcess/Downloads/DownloadManager.cpp:
+        (WebKit::DownloadManager::continueDecidePendingDownloadDestination):
+        * NetworkProcess/NetworkDataTask.h:
+        * NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
+        (WebKit::NetworkDataTask::state):
+
 2016-10-10  Konstantin Tokarev  <annu...@yandex.ru>
 
         Added final specifier to WebInspectorServer and to its overridden methods

Modified: trunk/Source/WebKit2/NetworkProcess/Downloads/DownloadManager.cpp (207041 => 207042)


--- trunk/Source/WebKit2/NetworkProcess/Downloads/DownloadManager.cpp	2016-10-11 04:43:26 UTC (rev 207041)
+++ trunk/Source/WebKit2/NetworkProcess/Downloads/DownloadManager.cpp	2016-10-11 05:28:44 UTC (rev 207042)
@@ -124,6 +124,8 @@
 
         networkDataTask->setPendingDownloadLocation(destination, sandboxExtensionHandle, allowOverwrite);
         completionHandler(PolicyDownload);
+        if (networkDataTask->state() == NetworkDataTask::State::Canceling || networkDataTask->state() == NetworkDataTask::State::Completed)
+            return;
 
         if (m_downloads.contains(downloadID)) {
             // The completion handler already called dataTaskBecameDownloadTask().

Modified: trunk/Source/WebKit2/NetworkProcess/NetworkDataTask.h (207041 => 207042)


--- trunk/Source/WebKit2/NetworkProcess/NetworkDataTask.h	2016-10-11 04:43:26 UTC (rev 207041)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkDataTask.h	2016-10-11 05:28:44 UTC (rev 207042)
@@ -93,7 +93,15 @@
     void suspend();
     void cancel();
     void resume();
-    
+
+    enum class State {
+        Running,
+        Suspended,
+        Canceling,
+        Completed
+    };
+    State state() const;
+
     typedef uint64_t TaskIdentifier;
     
     ~NetworkDataTask();

Modified: trunk/Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm (207041 => 207042)


--- trunk/Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm	2016-10-11 04:43:26 UTC (rev 207041)
+++ trunk/Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm	2016-10-11 05:28:44 UTC (rev 207042)
@@ -409,6 +409,23 @@
     [m_task suspend];
 }
 
+NetworkDataTask::State NetworkDataTask::state() const
+{
+    switch ([m_task state]) {
+    case NSURLSessionTaskStateRunning:
+        return State::Running;
+    case NSURLSessionTaskStateSuspended:
+        return State::Suspended;
+    case NSURLSessionTaskStateCanceling:
+        return State::Canceling;
+    case NSURLSessionTaskStateCompleted:
+        return State::Completed;
+    }
+
+    ASSERT_NOT_REACHED();
+    return State::Completed;
+}
+
 WebCore::Credential serverTrustCredential(const WebCore::AuthenticationChallenge& challenge)
 {
     return WebCore::Credential([NSURLCredential credentialForTrust:challenge.nsURLAuthenticationChallenge().protectionSpace.serverTrust]);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to