<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[202036] trunk/Source/WebKit2</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/202036">202036</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2016-06-14 00:43:24 -0700 (Tue, 14 Jun 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>Unreviewed. Fix Soup downloads after <a href="http://trac.webkit.org/projects/webkit/changeset/201943">r201943</a>.

This is a follow up of <a href="http://trac.webkit.org/projects/webkit/changeset/201943">r201943</a>. The DownloadClient used in DownloadSoup was not updated to the new API of the
ResourceHandleClient because it was not using override on the virtual methods, so it was unnoticed. That broke
the downloads soup implementation, because didReceiveResponse is no longer used in the DownloadClient. This
patch updates the DownloadClient to the new ResourceHandleClient API adding also override to all the virtual
methods to prevent this from happening in the future.

* NetworkProcess/Downloads/soup/DownloadSoup.cpp:
(WebKit::Download::start):
(WebKit::Download::startWithHandle):
(WebKit::DownloadClient::DownloadClient):
(WebKit::DownloadClient::downloadFailed):
(WebKit::DownloadClient::didReceiveResponse):
(WebKit::DownloadClient::didReceiveData):
(WebKit::DownloadClient::didFinishLoading):
(WebKit::DownloadClient::didFail):
(WebKit::DownloadClient::wasBlocked): Deleted.
(WebKit::DownloadClient::cannotShowURL): Deleted.
(WebKit::DownloadClient::cancel):
(WebKit::DownloadClient::handleResponse):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2NetworkProcessDownloadssoupDownloadSoupcpp">trunk/Source/WebKit2/NetworkProcess/Downloads/soup/DownloadSoup.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (202035 => 202036)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog        2016-06-14 07:35:53 UTC (rev 202035)
+++ trunk/Source/WebKit2/ChangeLog        2016-06-14 07:43:24 UTC (rev 202036)
</span><span class="lines">@@ -1,3 +1,27 @@
</span><ins>+2016-06-14  Carlos Garcia Campos  &lt;cgarcia@igalia.com&gt;
+
+        Unreviewed. Fix Soup downloads after r201943.
+
+        This is a follow up of r201943. The DownloadClient used in DownloadSoup was not updated to the new API of the
+        ResourceHandleClient because it was not using override on the virtual methods, so it was unnoticed. That broke
+        the downloads soup implementation, because didReceiveResponse is no longer used in the DownloadClient. This
+        patch updates the DownloadClient to the new ResourceHandleClient API adding also override to all the virtual
+        methods to prevent this from happening in the future.
+
+        * NetworkProcess/Downloads/soup/DownloadSoup.cpp:
+        (WebKit::Download::start):
+        (WebKit::Download::startWithHandle):
+        (WebKit::DownloadClient::DownloadClient):
+        (WebKit::DownloadClient::downloadFailed):
+        (WebKit::DownloadClient::didReceiveResponse):
+        (WebKit::DownloadClient::didReceiveData):
+        (WebKit::DownloadClient::didFinishLoading):
+        (WebKit::DownloadClient::didFail):
+        (WebKit::DownloadClient::wasBlocked): Deleted.
+        (WebKit::DownloadClient::cannotShowURL): Deleted.
+        (WebKit::DownloadClient::cancel):
+        (WebKit::DownloadClient::handleResponse):
+
</ins><span class="cx"> 2016-06-13  Dan Bernstein  &lt;mitz@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         [Mac] Web Content service with a restricted entitlement may load arbitrary dylibs
</span></span></pre></div>
<a id="trunkSourceWebKit2NetworkProcessDownloadssoupDownloadSoupcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/NetworkProcess/Downloads/soup/DownloadSoup.cpp (202035 => 202036)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/NetworkProcess/Downloads/soup/DownloadSoup.cpp        2016-06-14 07:35:53 UTC (rev 202035)
+++ trunk/Source/WebKit2/NetworkProcess/Downloads/soup/DownloadSoup.cpp        2016-06-14 07:43:24 UTC (rev 202036)
</span><span class="lines">@@ -45,10 +45,10 @@
</span><span class="cx"> 
</span><span class="cx"> namespace WebKit {
</span><span class="cx"> 
</span><del>-class DownloadClient : public ResourceHandleClient {
</del><ins>+class DownloadClient final : public ResourceHandleClient {
</ins><span class="cx">     WTF_MAKE_NONCOPYABLE(DownloadClient);
</span><span class="cx"> public:
</span><del>-    DownloadClient(Download* download)
</del><ins>+    DownloadClient(Download&amp; download)
</ins><span class="cx">         : m_download(download)
</span><span class="cx">         , m_handleResponseLater(RunLoop::main(), this, &amp;DownloadClient::handleResponse)
</span><span class="cx">         , m_allowOverwrite(false)
</span><span class="lines">@@ -73,28 +73,28 @@
</span><span class="cx">     void downloadFailed(const ResourceError&amp; error)
</span><span class="cx">     {
</span><span class="cx">         deleteFilesIfNeeded();
</span><del>-        m_download-&gt;didFail(error, IPC::DataReference());
</del><ins>+        m_download.didFail(error, IPC::DataReference());
</ins><span class="cx">     }
</span><span class="cx"> 
</span><del>-    void didReceiveResponse(ResourceHandle*, const ResourceResponse&amp; response)
</del><ins>+    void didReceiveResponse(ResourceHandle*, ResourceResponse&amp;&amp; response) override
</ins><span class="cx">     {
</span><del>-        m_response = response;
-        m_download-&gt;didReceiveResponse(response);
</del><ins>+        m_response = WTFMove(response);
+        m_download.didReceiveResponse(m_response);
</ins><span class="cx"> 
</span><del>-        if (response.httpStatusCode() &gt;= 400) {
-            downloadFailed(platformDownloadNetworkError(response.httpStatusCode(), response.url(), response.httpStatusText()));
</del><ins>+        if (m_response.httpStatusCode() &gt;= 400) {
+            downloadFailed(platformDownloadNetworkError(m_response.httpStatusCode(), m_response.url(), m_response.httpStatusText()));
</ins><span class="cx">             return;
</span><span class="cx">         }
</span><span class="cx"> 
</span><del>-        String suggestedFilename = response.suggestedFilename();
</del><ins>+        String suggestedFilename = m_response.suggestedFilename();
</ins><span class="cx">         if (suggestedFilename.isEmpty()) {
</span><del>-            URL url = response.url();
</del><ins>+            URL url = m_response.url();
</ins><span class="cx">             url.setQuery(String());
</span><span class="cx">             url.removeFragmentIdentifier();
</span><span class="cx">             suggestedFilename = decodeURLEscapeSequences(url.lastPathComponent());
</span><span class="cx">         }
</span><span class="cx"> 
</span><del>-        String destinationURI = m_download-&gt;decideDestinationWithSuggestedFilename(suggestedFilename, m_allowOverwrite);
</del><ins>+        String destinationURI = m_download.decideDestinationWithSuggestedFilename(suggestedFilename, m_allowOverwrite);
</ins><span class="cx">         if (destinationURI.isEmpty()) {
</span><span class="cx"> #if PLATFORM(GTK)
</span><span class="cx">             GUniquePtr&lt;char&gt; buffer(g_strdup_printf(_(&quot;Cannot determine destination URI for download with suggested filename %s&quot;), suggestedFilename.utf8().data()));
</span><span class="lines">@@ -102,7 +102,7 @@
</span><span class="cx"> #else
</span><span class="cx">             String errorMessage = makeString(&quot;Cannot determine destination URI for download with suggested filename &quot;, suggestedFilename);
</span><span class="cx"> #endif
</span><del>-            downloadFailed(platformDownloadDestinationError(response, errorMessage));
</del><ins>+            downloadFailed(platformDownloadDestinationError(m_response, errorMessage));
</ins><span class="cx">             return;
</span><span class="cx">         }
</span><span class="cx"> 
</span><span class="lines">@@ -115,7 +115,7 @@
</span><span class="cx">             outputStream = adoptGRef(g_file_create(m_destinationFile.get(), G_FILE_CREATE_NONE, nullptr, &amp;error.outPtr()));
</span><span class="cx">         if (!outputStream) {
</span><span class="cx">             m_destinationFile.clear();
</span><del>-            downloadFailed(platformDownloadDestinationError(response, error-&gt;message));
</del><ins>+            downloadFailed(platformDownloadDestinationError(m_response, error-&gt;message));
</ins><span class="cx">             return;
</span><span class="cx">         }
</span><span class="cx"> 
</span><span class="lines">@@ -123,14 +123,14 @@
</span><span class="cx">         m_intermediateFile = adoptGRef(g_file_new_for_uri(intermediateURI.utf8().data()));
</span><span class="cx">         m_outputStream = adoptGRef(g_file_replace(m_intermediateFile.get(), 0, TRUE, G_FILE_CREATE_NONE, 0, &amp;error.outPtr()));
</span><span class="cx">         if (!m_outputStream) {
</span><del>-            downloadFailed(platformDownloadDestinationError(response, error-&gt;message));
</del><ins>+            downloadFailed(platformDownloadDestinationError(m_response, error-&gt;message));
</ins><span class="cx">             return;
</span><span class="cx">         }
</span><span class="cx"> 
</span><del>-        m_download-&gt;didCreateDestination(destinationURI);
</del><ins>+        m_download.didCreateDestination(destinationURI);
</ins><span class="cx">     }
</span><span class="cx"> 
</span><del>-    void didReceiveData(ResourceHandle*, const char* data, unsigned length, int /*encodedDataLength*/)
</del><ins>+    void didReceiveData(ResourceHandle*, const char* data, unsigned length, int /*encodedDataLength*/) override
</ins><span class="cx">     {
</span><span class="cx">         if (m_handleResponseLater.isActive()) {
</span><span class="cx">             m_handleResponseLater.stop();
</span><span class="lines">@@ -144,12 +144,12 @@
</span><span class="cx">             downloadFailed(platformDownloadDestinationError(m_response, error-&gt;message));
</span><span class="cx">             return;
</span><span class="cx">         }
</span><del>-        m_download-&gt;didReceiveData(bytesWritten);
</del><ins>+        m_download.didReceiveData(bytesWritten);
</ins><span class="cx">     }
</span><span class="cx"> 
</span><del>-    void didFinishLoading(ResourceHandle*, double)
</del><ins>+    void didFinishLoading(ResourceHandle*, double) override
</ins><span class="cx">     {
</span><del>-        m_outputStream = 0;
</del><ins>+        m_outputStream = nullptr;
</ins><span class="cx"> 
</span><span class="cx">         ASSERT(m_destinationFile);
</span><span class="cx">         ASSERT(m_intermediateFile);
</span><span class="lines">@@ -165,34 +165,24 @@
</span><span class="cx">         g_file_info_set_attribute_string(info.get(), &quot;xattr::xdg.origin.url&quot;, uri.data());
</span><span class="cx">         g_file_set_attributes_async(m_destinationFile.get(), info.get(), G_FILE_QUERY_INFO_NONE, G_PRIORITY_DEFAULT, nullptr, nullptr, nullptr);
</span><span class="cx"> 
</span><del>-        m_download-&gt;didFinish();
</del><ins>+        m_download.didFinish();
</ins><span class="cx">     }
</span><span class="cx"> 
</span><del>-    void didFail(ResourceHandle*, const ResourceError&amp; error)
</del><ins>+    void didFail(ResourceHandle*, const ResourceError&amp; error) override
</ins><span class="cx">     {
</span><span class="cx">         downloadFailed(platformDownloadNetworkError(error.errorCode(), error.failingURL(), error.localizedDescription()));
</span><span class="cx">     }
</span><span class="cx"> 
</span><del>-    void wasBlocked(ResourceHandle*)
-    {
-        notImplemented();
-    }
-
-    void cannotShowURL(ResourceHandle*)
-    {
-        notImplemented();
-    }
-
</del><span class="cx">     void cancel(ResourceHandle* handle)
</span><span class="cx">     {
</span><span class="cx">         handle-&gt;cancel();
</span><span class="cx">         deleteFilesIfNeeded();
</span><del>-        m_download-&gt;didCancel(IPC::DataReference());
</del><ins>+        m_download.didCancel(IPC::DataReference());
</ins><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     void handleResponse()
</span><span class="cx">     {
</span><del>-        didReceiveResponse(nullptr, m_delayedResponse);
</del><ins>+        didReceiveResponse(nullptr, WTFMove(m_delayedResponse));
</ins><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     void handleResponseLater(const ResourceResponse&amp; response)
</span><span class="lines">@@ -207,7 +197,7 @@
</span><span class="cx">         m_handleResponseLater.startOneShot(0);
</span><span class="cx">     }
</span><span class="cx"> 
</span><del>-    Download* m_download;
</del><ins>+    Download&amp; m_download;
</ins><span class="cx">     GRefPtr&lt;GFileOutputStream&gt; m_outputStream;
</span><span class="cx">     ResourceResponse m_response;
</span><span class="cx">     GRefPtr&lt;GFile&gt; m_destinationFile;
</span><span class="lines">@@ -221,7 +211,7 @@
</span><span class="cx"> {
</span><span class="cx">     ASSERT(!m_downloadClient);
</span><span class="cx">     ASSERT(!m_resourceHandle);
</span><del>-    m_downloadClient = std::make_unique&lt;DownloadClient&gt;(this);
</del><ins>+    m_downloadClient = std::make_unique&lt;DownloadClient&gt;(*this);
</ins><span class="cx">     m_resourceHandle = ResourceHandle::create(0, m_request, m_downloadClient.get(), false, false);
</span><span class="cx">     didStart();
</span><span class="cx"> }
</span><span class="lines">@@ -230,7 +220,7 @@
</span><span class="cx"> {
</span><span class="cx">     ASSERT(!m_downloadClient);
</span><span class="cx">     ASSERT(!m_resourceHandle);
</span><del>-    m_downloadClient = std::make_unique&lt;DownloadClient&gt;(this);
</del><ins>+    m_downloadClient = std::make_unique&lt;DownloadClient&gt;(*this);
</ins><span class="cx">     m_resourceHandle = resourceHandle-&gt;releaseForDownload(m_downloadClient.get());
</span><span class="cx">     didStart();
</span><span class="cx">     static_cast&lt;DownloadClient*&gt;(m_downloadClient.get())-&gt;handleResponseLater(response);
</span></span></pre>
</div>
</div>

</body>
</html>