Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[Soup] Streamline cancellation and client checks 
https://bugs.webkit.org/show_bug.cgi?id=107808

Reviewed by Martin Robinson.

Covered by existing tests.

* platform/network/ResourceHandle.h:
(ResourceHandle):
* platform/network/soup/ResourceHandleSoup.cpp:
(WebCore::ResourceHandle::cancelledOrClientless): new method to check for cancellation and lack of client.
(WebCore):
(WebCore::gotHeadersCallback): use the new method.
(WebCore::restartedCallback): ditto.
(WebCore::redirectCloseCallback): ditto.
(WebCore::redirectSkipCallback): ditto.
(WebCore::wroteBodyDataCallback): ditto.
(WebCore::nextMultipartResponsePartCallback): ditto.
(WebCore::sendRequestCallback): ditto.
(WebCore::networkEventCallback): ditto.
(WebCore::ResourceHandle::platformSetDefersLoading): ditto.
(WebCore::readCallback): ditto.

Canonical link: https://commits.webkit.org/126151@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@140836 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
@kov
kov committed Jan 25, 2013
1 parent dcf292d commit 2052745
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 37 deletions.
25 changes: 25 additions & 0 deletions Source/WebCore/ChangeLog
View file
@@ -1,3 +1,28 @@
2013-01-25 Gustavo Noronha Silva <gns@gnome.org>

[Soup] Streamline cancellation and client checks
https://bugs.webkit.org/show_bug.cgi?id=107808

Reviewed by Martin Robinson.

Covered by existing tests.

* platform/network/ResourceHandle.h:
(ResourceHandle):
* platform/network/soup/ResourceHandleSoup.cpp:
(WebCore::ResourceHandle::cancelledOrClientless): new method to check for cancellation and lack of client.
(WebCore):
(WebCore::gotHeadersCallback): use the new method.
(WebCore::restartedCallback): ditto.
(WebCore::redirectCloseCallback): ditto.
(WebCore::redirectSkipCallback): ditto.
(WebCore::wroteBodyDataCallback): ditto.
(WebCore::nextMultipartResponsePartCallback): ditto.
(WebCore::sendRequestCallback): ditto.
(WebCore::networkEventCallback): ditto.
(WebCore::ResourceHandle::platformSetDefersLoading): ditto.
(WebCore::readCallback): ditto.

2013-01-25 Victor Carbune <vcarbune@chromium.org>

Heap-use-after-free in WebCore::TextTrackCue::isActive
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/platform/network/ResourceHandle.h
View file
Expand Up @@ -166,6 +166,7 @@ class ResourceHandle : public RefCounted<ResourceHandle>
void continueDidReceiveAuthenticationChallenge(const Credential& credentialFromPersistentStorage);
void sendPendingRequest();
bool shouldUseCredentialStorage();
bool cancelledOrClientless();
static SoupSession* defaultSession();
static uint64_t getSoupRequestInitiatingPageID(SoupRequest*);
static void setHostAllowsAnyHTTPSCertificate(const String&);
Expand Down
79 changes: 42 additions & 37 deletions Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp
View file
Expand Up @@ -304,6 +304,14 @@ SoupSession* ResourceHandleInternal::soupSession()
return session;
}

bool ResourceHandle::cancelledOrClientless()
{
if (!client())
return true;

return getInternal()->m_cancelled;
}

static bool isAuthenticationFailureStatusCode(int httpStatusCode)
{
return httpStatusCode == SOUP_STATUS_PROXY_AUTHENTICATION_REQUIRED || httpStatusCode == SOUP_STATUS_UNAUTHORIZED;
Expand All @@ -312,11 +320,10 @@ static bool isAuthenticationFailureStatusCode(int httpStatusCode)
static void gotHeadersCallback(SoupMessage* message, gpointer data)
{
ResourceHandle* handle = static_cast<ResourceHandle*>(data);
if (!handle)
if (!handle || handle->cancelledOrClientless())
return;

ResourceHandleInternal* d = handle->getInternal();
if (d->m_cancelled)
return;

#if ENABLE(WEB_TIMING)
if (d->m_response.resourceLoadTiming())
Expand Down Expand Up @@ -383,11 +390,10 @@ static void applyAuthenticationToRequest(ResourceHandle* handle, ResourceRequest
static void restartedCallback(SoupMessage*, gpointer data)
{
ResourceHandle* handle = static_cast<ResourceHandle*>(data);
if (!handle)
if (!handle || handle->cancelledOrClientless())
return;

ResourceHandleInternal* d = handle->getInternal();
if (d->m_cancelled)
return;

#if ENABLE(WEB_TIMING)
ResourceResponse& redirectResponse = d->m_response;
Expand Down Expand Up @@ -494,13 +500,13 @@ static void doRedirect(ResourceHandle* handle)
static void redirectCloseCallback(GObject*, GAsyncResult* result, gpointer data)
{
RefPtr<ResourceHandle> handle = static_cast<ResourceHandle*>(data);
ResourceHandleInternal* d = handle->getInternal();

if (d->m_cancelled || !handle->client()) {
if (handle->cancelledOrClientless()) {
cleanupSoupRequestOperation(handle.get());
return;
}

ResourceHandleInternal* d = handle->getInternal();
g_input_stream_close_finish(d->m_inputStream.get(), result, 0);
doRedirect(handle.get());
}
Expand All @@ -509,17 +515,16 @@ static void redirectSkipCallback(GObject*, GAsyncResult* asyncResult, gpointer d
{
RefPtr<ResourceHandle> handle = static_cast<ResourceHandle*>(data);

ResourceHandleInternal* d = handle->getInternal();
ResourceHandleClient* client = handle->client();

if (d->m_cancelled || !client) {
if (handle->cancelledOrClientless()) {
cleanupSoupRequestOperation(handle.get());
return;
}

GOwnPtr<GError> error;
ResourceHandleInternal* d = handle->getInternal();
gssize bytesSkipped = g_input_stream_read_finish(d->m_inputStream.get(), asyncResult, &error.outPtr());
if (error) {
ResourceHandleClient* client = handle->client();
client->didFail(handle.get(), ResourceError::genericIOError(error.get(), d->m_soupRequest.get()));
cleanupSoupRequestOperation(handle.get());
return;
Expand All @@ -541,16 +546,14 @@ static void wroteBodyDataCallback(SoupMessage*, SoupBuffer* buffer, gpointer dat
return;

ASSERT(buffer);
ResourceHandleInternal* internal = handle->getInternal();
internal->m_bodyDataSent += buffer->length;
ResourceHandleInternal* d = handle->getInternal();
d->m_bodyDataSent += buffer->length;

if (internal->m_cancelled)
return;
ResourceHandleClient* client = handle->client();
if (!client)
if (handle->cancelledOrClientless())
return;

client->didSendData(handle.get(), internal->m_bodyDataSent, internal->m_bodySize);
ResourceHandleClient* client = handle->client();
client->didSendData(handle.get(), d->m_bodyDataSent, d->m_bodySize);
}

static void cleanupSoupRequestOperation(ResourceHandle* handle, bool isDestroying)
Expand Down Expand Up @@ -608,18 +611,18 @@ static void nextMultipartResponsePartCallback(GObject* /*source*/, GAsyncResult*
{
RefPtr<ResourceHandle> handle = static_cast<ResourceHandle*>(data);

ResourceHandleInternal* d = handle->getInternal();
ResourceHandleClient* client = handle->client();

if (d->m_cancelled || !client) {
if (handle->cancelledOrClientless()) {
cleanupSoupRequestOperation(handle.get());
return;
}

ResourceHandleInternal* d = handle->getInternal();
ASSERT(!d->m_inputStream);

GOwnPtr<GError> error;
d->m_inputStream = adoptGRef(soup_multipart_input_stream_next_part_finish(d->m_multipartInputStream.get(), result, &error.outPtr()));

ResourceHandleClient* client = handle->client();
if (error) {
client->didFail(handle.get(), ResourceError::httpError(d->m_soupMessage.get(), error.get(), d->m_soupRequest.get()));
cleanupSoupRequestOperation(handle.get());
Expand All @@ -638,7 +641,7 @@ static void nextMultipartResponsePartCallback(GObject* /*source*/, GAsyncResult*

client->didReceiveResponse(handle.get(), d->m_response);

if (d->m_cancelled || !client) {
if (handle->cancelledOrClientless()) {
cleanupSoupRequestOperation(handle.get());
return;
}
Expand All @@ -651,14 +654,15 @@ static void sendRequestCallback(GObject*, GAsyncResult* result, gpointer data)
{
RefPtr<ResourceHandle> handle = static_cast<ResourceHandle*>(data);

if (handle->cancelledOrClientless()) {
cleanupSoupRequestOperation(handle.get());
return;
}

ResourceHandleInternal* d = handle->getInternal();
ResourceHandleClient* client = handle->client();
SoupMessage* soupMessage = d->m_soupMessage.get();

if (d->m_cancelled || !client) {
cleanupSoupRequestOperation(handle.get());
return;
}

if (d->m_defersLoading) {
d->m_deferredResult = result;
Expand Down Expand Up @@ -707,7 +711,7 @@ static void sendRequestCallback(GObject*, GAsyncResult* result, gpointer data)

client->didReceiveResponse(handle.get(), d->m_response);

if (d->m_cancelled) {
if (handle->cancelledOrClientless()) {
cleanupSoupRequestOperation(handle.get());
return;
}
Expand Down Expand Up @@ -869,10 +873,11 @@ static void networkEventCallback(SoupMessage*, GSocketClientEvent event, GIOStre
ResourceHandle* handle = static_cast<ResourceHandle*>(data);
if (!handle)
return;
ResourceHandleInternal* d = handle->getInternal();
if (d->m_cancelled)

if (handle->cancelledOrClientless())
return;

ResourceHandleInternal* d = handle->getInternal();
int deltaTime = milisecondsSinceRequest(d->m_response.resourceLoadTiming()->requestTime);
switch (event) {
case G_SOCKET_CLIENT_RESOLVING:
Expand Down Expand Up @@ -1249,7 +1254,7 @@ static bool waitingToSendRequest(ResourceHandle* handle)

void ResourceHandle::platformSetDefersLoading(bool defersLoading)
{
if (d->m_cancelled)
if (cancelledOrClientless())
return;

// Except when canceling a possible timeout timer, we only need to take action here to UN-defer loading.
Expand Down Expand Up @@ -1324,21 +1329,21 @@ static void readCallback(GObject*, GAsyncResult* asyncResult, gpointer data)
{
RefPtr<ResourceHandle> handle = static_cast<ResourceHandle*>(data);

ResourceHandleInternal* d = handle->getInternal();
ResourceHandleClient* client = handle->client();

if (d->m_cancelled || !client) {
if (handle->cancelledOrClientless()) {
cleanupSoupRequestOperation(handle.get());
return;
}

ResourceHandleInternal* d = handle->getInternal();
if (d->m_defersLoading) {
d->m_deferredResult = asyncResult;
return;
}

GOwnPtr<GError> error;
gssize bytesRead = g_input_stream_read_finish(d->m_inputStream.get(), asyncResult, &error.outPtr());

ResourceHandleClient* client = handle->client();
if (error) {
client->didFail(handle.get(), ResourceError::genericIOError(error.get(), d->m_soupRequest.get()));
cleanupSoupRequestOperation(handle.get());
Expand Down Expand Up @@ -1373,7 +1378,7 @@ static void readCallback(GObject*, GAsyncResult* asyncResult, gpointer data)
client->didReceiveData(handle.get(), d->m_buffer, bytesRead, bytesRead);

// didReceiveData may cancel the load, which may release the last reference.
if (d->m_cancelled || !client) {
if (handle->cancelledOrClientless()) {
cleanupSoupRequestOperation(handle.get());
return;
}
Expand Down

0 comments on commit 2052745

Please sign in to comment.