Unreviewed. Fix Soup downloads after r201943.
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Jun 2016 07:43:24 +0000 (07:43 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Jun 2016 07:43:24 +0000 (07:43 +0000)
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):

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

Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/Downloads/soup/DownloadSoup.cpp

index 1ab9310..f0c7f98 100644 (file)
@@ -1,3 +1,27 @@
+2016-06-14  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        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):
+
 2016-06-13  Dan Bernstein  <mitz@apple.com>
 
         [Mac] Web Content service with a restricted entitlement may load arbitrary dylibs
index b4198b1..56bbd00 100644 (file)
@@ -45,10 +45,10 @@ using namespace WebCore;
 
 namespace WebKit {
 
-class DownloadClient : public ResourceHandleClient {
+class DownloadClient final : public ResourceHandleClient {
     WTF_MAKE_NONCOPYABLE(DownloadClient);
 public:
-    DownloadClient(Download* download)
+    DownloadClient(Download& download)
         : m_download(download)
         , m_handleResponseLater(RunLoop::main(), this, &DownloadClient::handleResponse)
         , m_allowOverwrite(false)
@@ -73,28 +73,28 @@ public:
     void downloadFailed(const ResourceError& error)
     {
         deleteFilesIfNeeded();
-        m_download->didFail(error, IPC::DataReference());
+        m_download.didFail(error, IPC::DataReference());
     }
 
-    void didReceiveResponse(ResourceHandle*, const ResourceResponse& response)
+    void didReceiveResponse(ResourceHandle*, ResourceResponse&& response) override
     {
-        m_response = response;
-        m_download->didReceiveResponse(response);
+        m_response = WTFMove(response);
+        m_download.didReceiveResponse(m_response);
 
-        if (response.httpStatusCode() >= 400) {
-            downloadFailed(platformDownloadNetworkError(response.httpStatusCode(), response.url(), response.httpStatusText()));
+        if (m_response.httpStatusCode() >= 400) {
+            downloadFailed(platformDownloadNetworkError(m_response.httpStatusCode(), m_response.url(), m_response.httpStatusText()));
             return;
         }
 
-        String suggestedFilename = response.suggestedFilename();
+        String suggestedFilename = m_response.suggestedFilename();
         if (suggestedFilename.isEmpty()) {
-            URL url = response.url();
+            URL url = m_response.url();
             url.setQuery(String());
             url.removeFragmentIdentifier();
             suggestedFilename = decodeURLEscapeSequences(url.lastPathComponent());
         }
 
-        String destinationURI = m_download->decideDestinationWithSuggestedFilename(suggestedFilename, m_allowOverwrite);
+        String destinationURI = m_download.decideDestinationWithSuggestedFilename(suggestedFilename, m_allowOverwrite);
         if (destinationURI.isEmpty()) {
 #if PLATFORM(GTK)
             GUniquePtr<char> buffer(g_strdup_printf(_("Cannot determine destination URI for download with suggested filename %s"), suggestedFilename.utf8().data()));
@@ -102,7 +102,7 @@ public:
 #else
             String errorMessage = makeString("Cannot determine destination URI for download with suggested filename ", suggestedFilename);
 #endif
-            downloadFailed(platformDownloadDestinationError(response, errorMessage));
+            downloadFailed(platformDownloadDestinationError(m_response, errorMessage));
             return;
         }
 
@@ -115,7 +115,7 @@ public:
             outputStream = adoptGRef(g_file_create(m_destinationFile.get(), G_FILE_CREATE_NONE, nullptr, &error.outPtr()));
         if (!outputStream) {
             m_destinationFile.clear();
-            downloadFailed(platformDownloadDestinationError(response, error->message));
+            downloadFailed(platformDownloadDestinationError(m_response, error->message));
             return;
         }
 
@@ -123,14 +123,14 @@ public:
         m_intermediateFile = adoptGRef(g_file_new_for_uri(intermediateURI.utf8().data()));
         m_outputStream = adoptGRef(g_file_replace(m_intermediateFile.get(), 0, TRUE, G_FILE_CREATE_NONE, 0, &error.outPtr()));
         if (!m_outputStream) {
-            downloadFailed(platformDownloadDestinationError(response, error->message));
+            downloadFailed(platformDownloadDestinationError(m_response, error->message));
             return;
         }
 
-        m_download->didCreateDestination(destinationURI);
+        m_download.didCreateDestination(destinationURI);
     }
 
-    void didReceiveData(ResourceHandle*, const char* data, unsigned length, int /*encodedDataLength*/)
+    void didReceiveData(ResourceHandle*, const char* data, unsigned length, int /*encodedDataLength*/) override
     {
         if (m_handleResponseLater.isActive()) {
             m_handleResponseLater.stop();
@@ -144,12 +144,12 @@ public:
             downloadFailed(platformDownloadDestinationError(m_response, error->message));
             return;
         }
-        m_download->didReceiveData(bytesWritten);
+        m_download.didReceiveData(bytesWritten);
     }
 
-    void didFinishLoading(ResourceHandle*, double)
+    void didFinishLoading(ResourceHandle*, double) override
     {
-        m_outputStream = 0;
+        m_outputStream = nullptr;
 
         ASSERT(m_destinationFile);
         ASSERT(m_intermediateFile);
@@ -165,34 +165,24 @@ public:
         g_file_info_set_attribute_string(info.get(), "xattr::xdg.origin.url", uri.data());
         g_file_set_attributes_async(m_destinationFile.get(), info.get(), G_FILE_QUERY_INFO_NONE, G_PRIORITY_DEFAULT, nullptr, nullptr, nullptr);
 
-        m_download->didFinish();
+        m_download.didFinish();
     }
 
-    void didFail(ResourceHandle*, const ResourceError& error)
+    void didFail(ResourceHandle*, const ResourceError& error) override
     {
         downloadFailed(platformDownloadNetworkError(error.errorCode(), error.failingURL(), error.localizedDescription()));
     }
 
-    void wasBlocked(ResourceHandle*)
-    {
-        notImplemented();
-    }
-
-    void cannotShowURL(ResourceHandle*)
-    {
-        notImplemented();
-    }
-
     void cancel(ResourceHandle* handle)
     {
         handle->cancel();
         deleteFilesIfNeeded();
-        m_download->didCancel(IPC::DataReference());
+        m_download.didCancel(IPC::DataReference());
     }
 
     void handleResponse()
     {
-        didReceiveResponse(nullptr, m_delayedResponse);
+        didReceiveResponse(nullptr, WTFMove(m_delayedResponse));
     }
 
     void handleResponseLater(const ResourceResponse& response)
@@ -207,7 +197,7 @@ public:
         m_handleResponseLater.startOneShot(0);
     }
 
-    Download* m_download;
+    Download& m_download;
     GRefPtr<GFileOutputStream> m_outputStream;
     ResourceResponse m_response;
     GRefPtr<GFile> m_destinationFile;
@@ -221,7 +211,7 @@ void Download::start()
 {
     ASSERT(!m_downloadClient);
     ASSERT(!m_resourceHandle);
-    m_downloadClient = std::make_unique<DownloadClient>(this);
+    m_downloadClient = std::make_unique<DownloadClient>(*this);
     m_resourceHandle = ResourceHandle::create(0, m_request, m_downloadClient.get(), false, false);
     didStart();
 }
@@ -230,7 +220,7 @@ void Download::startWithHandle(ResourceHandle* resourceHandle, const ResourceRes
 {
     ASSERT(!m_downloadClient);
     ASSERT(!m_resourceHandle);
-    m_downloadClient = std::make_unique<DownloadClient>(this);
+    m_downloadClient = std::make_unique<DownloadClient>(*this);
     m_resourceHandle = resourceHandle->releaseForDownload(m_downloadClient.get());
     didStart();
     static_cast<DownloadClient*>(m_downloadClient.get())->handleResponseLater(response);