The output stream to write the downloaded data is created in the didReceiveResponse callback of the download client. When a download is created for an existing ResourceHandle (this happens for example when policy decision is download), the response has already been received. In this case we should make sure that the download client is notified about the response.
Created attachment 145045 [details] Patch
Comment on attachment 145045 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145045&action=review > Source/WebKit2/WebProcess/Downloads/soup/DownloadSoup.cpp:154 > + // If the handle already got a response, make sure the download client is notified. > + ResourceHandleInternal* handleInternal = m_resourceHandle->getInternal(); > + if (!handleInternal->m_response.isNull()) > + m_downloadClient->didReceiveResponse(m_resourceHandle.get(), handleInternal->m_response); Is there a chance that data has already been received or the request already failed as well? If that was the case then we'd have to forward those events. Or is the resource handle paused during policy decisions?
(In reply to comment #2) > (From update of attachment 145045 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=145045&action=review > > > Source/WebKit2/WebProcess/Downloads/soup/DownloadSoup.cpp:154 > > + // If the handle already got a response, make sure the download client is notified. > > + ResourceHandleInternal* handleInternal = m_resourceHandle->getInternal(); > > + if (!handleInternal->m_response.isNull()) > > + m_downloadClient->didReceiveResponse(m_resourceHandle.get(), handleInternal->m_response); > > Is there a chance that data has already been received or the request already failed as well? If that was the case then we'd have to forward those events. Or is the resource handle paused during policy decisions? Policy decisions happen before data is sent, when the response has been received. If it fails, it's not converted to a download.
Committed r119107: <http://trac.webkit.org/changeset/119107>
Comment on attachment 145045 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145045&action=review >>> Source/WebKit2/WebProcess/Downloads/soup/DownloadSoup.cpp:154 >>> + m_downloadClient->didReceiveResponse(m_resourceHandle.get(), handleInternal->m_response); >> >> Is there a chance that data has already been received or the request already failed as well? If that was the case then we'd have to forward those events. Or is the resource handle paused during policy decisions? > > Policy decisions happen before data is sent, when the response has been received. If it fails, it's not converted to a download. The download client still handles didReceiveData and didFail though...
(In reply to comment #5) > (From update of attachment 145045 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=145045&action=review > > >>> Source/WebKit2/WebProcess/Downloads/soup/DownloadSoup.cpp:154 > >>> + m_downloadClient->didReceiveResponse(m_resourceHandle.get(), handleInternal->m_response); > >> > >> Is there a chance that data has already been received or the request already failed as well? If that was the case then we'd have to forward those events. Or is the resource handle paused during policy decisions? > > > > Policy decisions happen before data is sent, when the response has been received. If it fails, it's not converted to a download. > > The download client still handles didReceiveData and didFail though... once the download client has been set as the handle client, if it fails, it will be handled by the download, and of course didReceiveData is handled to write the downloaded data.
Could you please clarify what kind of crashes there used to be? Layout test names, crash logs.. The landed patch caused crash of fast/loader/reload-zero-byte-plugin.html on WK2 EFL (and most probably on WK2 GTK as well). Please take a look at https://bugs.webkit.org/show_bug.cgi?id=97565
(In reply to comment #7) > Could you please clarify what kind of crashes there used to be? Layout test names, crash logs.. > > The landed patch caused crash of fast/loader/reload-zero-byte-plugin.html on WK2 EFL (and most probably on WK2 GTK as well). Please take a look at > https://bugs.webkit.org/show_bug.cgi?id=97565 The problem was that the output stream is created when handling the response, so if we receive data and and the response hasn't been handled, there's no output stream to write to. When the ResourceHandle is converted into a download, typically by the policy client, the response has already been received, but the download client hasn't been notified because the download is created after the response is received. So it always crashed when a download was started by the policy client for example.