[Webkit-unassigned] [Bug 163006] New: Network Session: PendingDownload is always nullptr in DownloadManager::dataTaskBecameDownloadTask

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=163006

            Bug ID: 163006
           Summary: Network Session: PendingDownload is always nullptr in
                    DownloadManager::dataTaskBecameDownloadTask
    Classification: Unclassified
           Product: WebKit
           Version: WebKit Local Build
          Hardware: Unspecified
                OS: Unspecified
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: WebKit2
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: cgarcia at igalia.com
                CC: achristensen at apple.com, beidson at apple.com

I'm not sure when dataTaskBecameDownloadTask() is called exactly in Mac, but it seems to me that PendingDownload is always nullptr at that point. Here are the steps for downloads started with startDownload and convertTaskToDownload:

1- DownloadManager::startDownload()
  m_pendingDownloads.add(downloadID, PendingDownload())

2- PendingDownload()
  m_networkLoad(NetworkLoad())
  set pending download ID, pending download and suggested filename on new NetworkLoad

3- NetworkLoad::resume()
  Starts the load normally until receiving the response

4- NetworkLoad::didReceiveResponse()
  The task has pending download ID

5- NetworkProcess::findPendingDownloadLocation(task, completionHandler)
  Notifies the download manager that is waiting for the location

6- DownloadManager::willDecidePendingDownloadDestination()
  m_pendingDownloads.take(downloadID)
  m_downloadsWaitingForDestination.set(<task, completionHandler>)
  The pending download is now deleted and its NetworkLoad too.
  Once it has the loation it either cancels the download or continues

7- DownloadManager::continueDecidePendingDownloadDestination
  <task, completionHandler> = m_downloadsWaitingForDestination.take(downloadID)
  completionHandler(PolicyDownload)
  m_downloadsAfterDestinationDecided.set(downloadID, task)

8- DownloadManager::dataTaskBecameDownloadTask(downloadID, download)
  The completionHandler() ended up converting the task to a download, so at some point
  dataTaskBecameDownloadTask is called right after creating the download.
  // This is needed for downloads started with startDownload, otherwise it will return nullptr.
  m_pendingDownloads.take(downloadID)
  // This is needed for downloads started with convertTaskToDownload, otherwise it will return nullptr.
  m_downloadsAfterDestinationDecided.take(downloadID)

At this point m_pendingDownloads is supposed to contain the download, because it was started by DownloadManager::startDownload(), but the download was removed from m_pendingDownloads at step 6.
If the completionHandler calls dataTaskBecameDownloadTask() then m_downloadsAfterDestinationDecided doesn't contain the download yet either, so both are indeed nullptr. If it's supposed to happen asynchronously then we indeed have a task even when this was not started by convertTaskToDownload.

1- NetworkLoad()::resume()
  Starts the load normally until policy checker converts the load into a download

2- NetworkLoad::convertTaskToDownload(downloadID, request, response)
  Sets pending download ID to the the task

3- NetworkProcess::findPendingDownloadLocation(task, completionHandler)
  Notifies the download manager that is waiting for the location

The next steps are exactly the same, with the difference that in this case m_pendingDownloads will not contain a PendingDownload for the downloadID, but in both cases we have a NetworkDataTask.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20161006/516b631f/attachment.html>