NetworkSession: NetworkDataTask is leaked if download finishes in didReceiveResponse...
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Oct 2016 05:28:44 +0000 (05:28 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Oct 2016 05:28:44 +0000 (05:28 +0000)
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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@207042 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/Downloads/DownloadManager.cpp
Source/WebKit2/NetworkProcess/NetworkDataTask.h
Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm

index 8824a72..00a36bd 100644 (file)
@@ -1,3 +1,22 @@
+2016-10-10  Carlos Garcia Campos  <cgarcia@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  <annulen@yandex.ru>
 
         Added final specifier to WebInspectorServer and to its overridden methods
index 4f657fd..4c76ea5 100644 (file)
@@ -124,6 +124,8 @@ void DownloadManager::continueDecidePendingDownloadDestination(DownloadID downlo
 
         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().
index db94fac..11284b2 100644 (file)
@@ -93,7 +93,15 @@ public:
     void suspend();
     void cancel();
     void resume();
-    
+
+    enum class State {
+        Running,
+        Suspended,
+        Canceling,
+        Completed
+    };
+    State state() const;
+
     typedef uint64_t TaskIdentifier;
     
     ~NetworkDataTask();
index 0057266..a9d4adb 100644 (file)
@@ -409,6 +409,23 @@ void NetworkDataTask::suspend()
     [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]);