Skip to content

Commit

Permalink
Unreviewed. Fix Soup downloads after r201943.
Browse files Browse the repository at this point in the history 
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):

Canonical link: https://commits.webkit.org/176815@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@202036 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
@carlosgcampos
carlosgcampos committed Jun 14, 2016
1 parent 4759e3c commit e22b8b4
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 36 deletions.
24 changes: 24 additions & 0 deletions Source/WebKit2/ChangeLog
View 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
Expand Down
62 changes: 26 additions & 36 deletions Source/WebKit2/NetworkProcess/Downloads/soup/DownloadSoup.cpp
View file
Expand Up @@ -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)
Expand All @@ -73,36 +73,36 @@ class DownloadClient : public ResourceHandleClient {
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()));
String errorMessage = String::fromUTF8(buffer.get());
#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;
}

Expand All @@ -115,22 +115,22 @@ class DownloadClient : public ResourceHandleClient {
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;
}

String intermediateURI = destinationURI + ".wkdownload";
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();
Expand All @@ -144,12 +144,12 @@ class DownloadClient : public ResourceHandleClient {
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);
Expand All @@ -165,34 +165,24 @@ class DownloadClient : public ResourceHandleClient {
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)
Expand All @@ -207,7 +197,7 @@ class DownloadClient : public ResourceHandleClient {
m_handleResponseLater.startOneShot(0);
}

Download* m_download;
Download& m_download;
GRefPtr<GFileOutputStream> m_outputStream;
ResourceResponse m_response;
GRefPtr<GFile> m_destinationFile;
Expand All @@ -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();
}
Expand All @@ -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);
Expand Down

0 comments on commit e22b8b4

Please sign in to comment.