From 457f7c99e93855cc32ab42f0525dfa4903507978 Mon Sep 17 00:00:00 2001 From: "ap@apple.com" Date: Wed, 29 Jul 2015 01:26:49 +0000 Subject: [PATCH] Clean up usesAsyncCallbacks handling in ResourceHandle https://bugs.webkit.org/show_bug.cgi?id=147342 Reviewed by Darin Adler. Source/WebCore: Store "usesAsyncCallbacks" bit in ResourceHandle, because it's not accessible via client once the client is zeroed out. Changed ResourceHandle::setClient into ResourceHandle::clearClient, because it's only ever used to zero out the client pointer, and it doesn't support changing it. * loader/ResourceLoader.cpp: (WebCore::ResourceLoader::releaseResources): * loader/appcache/ApplicationCacheGroup.cpp: (WebCore::ApplicationCacheGroup::stopLoading): * platform/network/BlobResourceHandle.cpp: (WebCore::BlobResourceHandle::notifyResponseOnSuccess): (WebCore::BlobResourceHandle::notifyResponseOnError): * platform/network/ResourceHandle.cpp: (WebCore::ResourceHandle::client): (WebCore::ResourceHandle::clearClient): (WebCore::ResourceHandle::setDefersLoading): (WebCore::ResourceHandle::usesAsyncCallbacks): (WebCore::ResourceHandle::setClient): Deleted. * platform/network/ResourceHandle.h: * platform/network/ResourceHandleInternal.h: (WebCore::ResourceHandleInternal::ResourceHandleInternal): * platform/network/cf/ResourceHandleCFNet.cpp: (WebCore::ResourceHandle::createCFURLConnection): (WebCore::ResourceHandle::willSendRequest): (WebCore::ResourceHandle::shouldUseCredentialStorage): (WebCore::ResourceHandle::canAuthenticateAgainstProtectionSpace): * platform/network/mac/ResourceHandleMac.mm: (WebCore::ResourceHandle::start): (WebCore::ResourceHandle::makeDelegate): (WebCore::ResourceHandle::willSendRequest): (WebCore::ResourceHandle::continueWillSendRequest): (WebCore::ResourceHandle::continueDidReceiveResponse): (WebCore::ResourceHandle::shouldUseCredentialStorage): (WebCore::ResourceHandle::canAuthenticateAgainstProtectionSpace): (WebCore::ResourceHandle::continueCanAuthenticateAgainstProtectionSpace): (WebCore::ResourceHandle::continueWillCacheResponse): Source/WebKit2: Update for a renaming in WebCore. * NetworkProcess/NetworkResourceLoader.cpp: (WebKit::NetworkResourceLoader::cleanup): * Shared/Downloads/soup/DownloadSoup.cpp: (WebKit::Download::platformInvalidate): git-svn-id: https://svn.webkit.org/repository/webkit/trunk@187533 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/WebCore/ChangeLog | 45 ++++++++++++++++++++++ Source/WebCore/loader/ResourceLoader.cpp | 2 +- .../loader/appcache/ApplicationCacheGroup.cpp | 4 +- .../platform/network/BlobResourceHandle.cpp | 4 +- Source/WebCore/platform/network/ResourceHandle.cpp | 9 ++++- Source/WebCore/platform/network/ResourceHandle.h | 4 +- .../platform/network/ResourceHandleInternal.h | 5 ++- .../platform/network/cf/ResourceHandleCFNet.cpp | 17 ++++---- .../platform/network/mac/ResourceHandleMac.mm | 33 ++++++++-------- Source/WebKit2/ChangeLog | 14 +++++++ .../NetworkProcess/NetworkResourceLoader.cpp | 2 +- .../WebKit2/Shared/Downloads/soup/DownloadSoup.cpp | 2 +- 12 files changed, 106 insertions(+), 35 deletions(-) diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 61e28d5..a669ac2 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,48 @@ +2015-07-28 Alexey Proskuryakov + + Clean up usesAsyncCallbacks handling in ResourceHandle + https://bugs.webkit.org/show_bug.cgi?id=147342 + + Reviewed by Darin Adler. + + Store "usesAsyncCallbacks" bit in ResourceHandle, because it's not accessible + via client once the client is zeroed out. + + Changed ResourceHandle::setClient into ResourceHandle::clearClient, because it's + only ever used to zero out the client pointer, and it doesn't support changing it. + + * loader/ResourceLoader.cpp: + (WebCore::ResourceLoader::releaseResources): + * loader/appcache/ApplicationCacheGroup.cpp: + (WebCore::ApplicationCacheGroup::stopLoading): + * platform/network/BlobResourceHandle.cpp: + (WebCore::BlobResourceHandle::notifyResponseOnSuccess): + (WebCore::BlobResourceHandle::notifyResponseOnError): + * platform/network/ResourceHandle.cpp: + (WebCore::ResourceHandle::client): + (WebCore::ResourceHandle::clearClient): + (WebCore::ResourceHandle::setDefersLoading): + (WebCore::ResourceHandle::usesAsyncCallbacks): + (WebCore::ResourceHandle::setClient): Deleted. + * platform/network/ResourceHandle.h: + * platform/network/ResourceHandleInternal.h: + (WebCore::ResourceHandleInternal::ResourceHandleInternal): + * platform/network/cf/ResourceHandleCFNet.cpp: + (WebCore::ResourceHandle::createCFURLConnection): + (WebCore::ResourceHandle::willSendRequest): + (WebCore::ResourceHandle::shouldUseCredentialStorage): + (WebCore::ResourceHandle::canAuthenticateAgainstProtectionSpace): + * platform/network/mac/ResourceHandleMac.mm: + (WebCore::ResourceHandle::start): + (WebCore::ResourceHandle::makeDelegate): + (WebCore::ResourceHandle::willSendRequest): + (WebCore::ResourceHandle::continueWillSendRequest): + (WebCore::ResourceHandle::continueDidReceiveResponse): + (WebCore::ResourceHandle::shouldUseCredentialStorage): + (WebCore::ResourceHandle::canAuthenticateAgainstProtectionSpace): + (WebCore::ResourceHandle::continueCanAuthenticateAgainstProtectionSpace): + (WebCore::ResourceHandle::continueWillCacheResponse): + 2015-07-28 Michael Catanzaro Minor cleanups in FontCacheFreeType.cpp diff --git a/Source/WebCore/loader/ResourceLoader.cpp b/Source/WebCore/loader/ResourceLoader.cpp index b801386..e1c9716 100644 --- a/Source/WebCore/loader/ResourceLoader.cpp +++ b/Source/WebCore/loader/ResourceLoader.cpp @@ -104,7 +104,7 @@ void ResourceLoader::releaseResources() // Clear out the ResourceHandle's client so that it doesn't try to call // us back after we release it, unless it has been replaced by someone else. if (m_handle->client() == this) - m_handle->setClient(nullptr); + m_handle->clearClient(); m_handle = nullptr; } diff --git a/Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp b/Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp index dfed047..59d579d 100644 --- a/Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp +++ b/Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp @@ -324,7 +324,7 @@ void ApplicationCacheGroup::stopLoading() ASSERT(!m_currentHandle); ASSERT(m_manifestHandle->client() == this); - m_manifestHandle->setClient(0); + m_manifestHandle->clearClient(); m_manifestHandle->cancel(); m_manifestHandle = nullptr; @@ -335,7 +335,7 @@ void ApplicationCacheGroup::stopLoading() ASSERT(m_cacheBeingUpdated); ASSERT(m_currentHandle->client() == this); - m_currentHandle->setClient(0); + m_currentHandle->clearClient(); m_currentHandle->cancel(); m_currentHandle = nullptr; diff --git a/Source/WebCore/platform/network/BlobResourceHandle.cpp b/Source/WebCore/platform/network/BlobResourceHandle.cpp index 08d5f51..eb07027 100644 --- a/Source/WebCore/platform/network/BlobResourceHandle.cpp +++ b/Source/WebCore/platform/network/BlobResourceHandle.cpp @@ -594,7 +594,7 @@ void BlobResourceHandle::notifyResponseOnSuccess() // BlobResourceHandle cannot be used with downloading, and doesn't even wait for continueDidReceiveResponse. // It's currently client's responsibility to know that didReceiveResponseAsync cannot be used to convert a // load into a download or blobs. - if (client()->usesAsyncCallbacks()) + if (usesAsyncCallbacks()) client()->didReceiveResponseAsync(this, response); else client()->didReceiveResponse(this, response); @@ -626,7 +626,7 @@ void BlobResourceHandle::notifyResponseOnError() // Note that we don't wait for continueDidReceiveResponse when using didReceiveResponseAsync. // This is not formally correct, but the client has to be a no-op anyway, because blobs can't be downloaded. - if (client()->usesAsyncCallbacks()) + if (usesAsyncCallbacks()) client()->didReceiveResponseAsync(this, response); else client()->didReceiveResponse(this, response); diff --git a/Source/WebCore/platform/network/ResourceHandle.cpp b/Source/WebCore/platform/network/ResourceHandle.cpp index 8b6a6d9..5a3dd38 100644 --- a/Source/WebCore/platform/network/ResourceHandle.cpp +++ b/Source/WebCore/platform/network/ResourceHandle.cpp @@ -147,9 +147,9 @@ ResourceHandleClient* ResourceHandle::client() const return d->m_client; } -void ResourceHandle::setClient(ResourceHandleClient* client) +void ResourceHandle::clearClient() { - d->m_client = client; + d->m_client = nullptr; } #if !PLATFORM(COCOA) && !USE(CFNETWORK) && !USE(SOUP) @@ -239,4 +239,9 @@ void ResourceHandle::setDefersLoading(bool defers) platformSetDefersLoading(defers); } +bool ResourceHandle::usesAsyncCallbacks() const +{ + return d->m_usesAsyncCallbacks; +} + } // namespace WebCore diff --git a/Source/WebCore/platform/network/ResourceHandle.h b/Source/WebCore/platform/network/ResourceHandle.h index 41df86d..a1f591e 100644 --- a/Source/WebCore/platform/network/ResourceHandle.h +++ b/Source/WebCore/platform/network/ResourceHandle.h @@ -200,7 +200,7 @@ public: // The client may be 0, in which case no callbacks will be made. ResourceHandleClient* client() const; - WEBCORE_EXPORT void setClient(ResourceHandleClient*); + WEBCORE_EXPORT void clearClient(); // Called in response to ResourceHandleClient::willSendRequestAsync(). WEBCORE_EXPORT void continueWillSendRequest(const ResourceRequest&); @@ -250,6 +250,8 @@ public: protected: ResourceHandle(NetworkingContext*, const ResourceRequest&, ResourceHandleClient*, bool defersLoading, bool shouldContentSniff); + bool usesAsyncCallbacks() const; + private: enum FailureType { NoFailure, diff --git a/Source/WebCore/platform/network/ResourceHandleInternal.h b/Source/WebCore/platform/network/ResourceHandleInternal.h index 448ea76..cced8b2 100644 --- a/Source/WebCore/platform/network/ResourceHandleInternal.h +++ b/Source/WebCore/platform/network/ResourceHandleInternal.h @@ -28,6 +28,7 @@ #include "NetworkingContext.h" #include "ResourceHandle.h" +#include "ResourceHandleClient.h" #include "ResourceRequest.h" #include "AuthenticationChallenge.h" #include "Timer.h" @@ -70,8 +71,6 @@ typedef const struct __CFURLStorageSession* CFURLStorageSessionRef; namespace WebCore { - class ResourceHandleClient; - class ResourceHandleInternal { WTF_MAKE_NONCOPYABLE(ResourceHandleInternal); WTF_MAKE_FAST_ALLOCATED; public: @@ -83,6 +82,7 @@ namespace WebCore { , status(0) , m_defersLoading(defersLoading) , m_shouldContentSniff(shouldContentSniff) + , m_usesAsyncCallbacks(client && client->usesAsyncCallbacks()) #if USE(CFNETWORK) , m_currentRequest(request) #endif @@ -129,6 +129,7 @@ namespace WebCore { bool m_defersLoading; bool m_shouldContentSniff; + bool m_usesAsyncCallbacks; #if USE(CFNETWORK) RetainPtr m_connection; ResourceRequest m_currentRequest; diff --git a/Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp b/Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp index 2e5512d..3e61cd4 100644 --- a/Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp +++ b/Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp @@ -219,7 +219,7 @@ void ResourceHandle::createCFURLConnection(bool shouldUseCredentialStorage, bool CFRelease(streamProperties); #if PLATFORM(COCOA) - if (client() && client()->usesAsyncCallbacks()) + if (d->m_usesAsyncCallbacks) d->m_connectionDelegate = adoptRef(new ResourceHandleCFURLConnectionDelegateWithOperationQueue(this)); else d->m_connectionDelegate = adoptRef(new SynchronousResourceHandleCFURLConnectionDelegate(this)); @@ -304,7 +304,7 @@ void ResourceHandle::willSendRequest(ResourceRequest& request, const ResourceRes } Ref protect(*this); - if (client()->usesAsyncCallbacks()) + if (d->m_usesAsyncCallbacks) client()->willSendRequestAsync(this, request, redirectResponse); else { client()->willSendRequest(this, request, redirectResponse); @@ -322,7 +322,7 @@ bool ResourceHandle::shouldUseCredentialStorage() { LOG(Network, "CFNet - shouldUseCredentialStorage()"); if (ResourceHandleClient* client = this->client()) { - ASSERT(!client->usesAsyncCallbacks()); + ASSERT(!d->m_usesAsyncCallbacks); return client->shouldUseCredentialStorage(this); } return false; @@ -418,13 +418,16 @@ bool ResourceHandle::tryHandlePasswordBasedAuthentication(const AuthenticationCh #if USE(PROTECTION_SPACE_AUTH_CALLBACK) bool ResourceHandle::canAuthenticateAgainstProtectionSpace(const ProtectionSpace& protectionSpace) { - if (ResourceHandleClient* client = this->client()) { - if (client->usesAsyncCallbacks()) + ResourceHandleClient* client = this->client(); + if (d->m_usesAsyncCallbacks) { + if (client) client->canAuthenticateAgainstProtectionSpaceAsync(this, protectionSpace); else - return client->canAuthenticateAgainstProtectionSpace(this, protectionSpace); + continueCanAuthenticateAgainstProtectionSpace(false); + return false; // Ignored by caller. } - return false; + + return client && client->canAuthenticateAgainstProtectionSpace(this, protectionSpace); } #endif diff --git a/Source/WebCore/platform/network/mac/ResourceHandleMac.mm b/Source/WebCore/platform/network/mac/ResourceHandleMac.mm index d99915a..a3b0da1 100644 --- a/Source/WebCore/platform/network/mac/ResourceHandleMac.mm +++ b/Source/WebCore/platform/network/mac/ResourceHandleMac.mm @@ -274,7 +274,7 @@ bool ResourceHandle::start() } } - if (client() && client()->usesAsyncCallbacks()) { + if (d->m_usesAsyncCallbacks) { ASSERT(!scheduled); [connection() setDelegateQueue:operationQueueForAsyncClients()]; scheduled = true; @@ -351,7 +351,7 @@ id ResourceHandle::makeDelegate(bool shouldUseCredentialStorage) ASSERT(!d->m_delegate); id delegate; - if (client() && client()->usesAsyncCallbacks()) { + if (d->m_usesAsyncCallbacks) { if (shouldUseCredentialStorage) delegate = [[WebCoreResourceHandleAsOperationQueueDelegate alloc] initWithHandle:this]; else @@ -488,7 +488,7 @@ void ResourceHandle::willSendRequest(ResourceRequest& request, const ResourceRes } } - if (client()->usesAsyncCallbacks()) { + if (d->m_usesAsyncCallbacks) { client()->willSendRequestAsync(this, request, redirectResponse); } else { Ref protect(*this); @@ -502,8 +502,7 @@ void ResourceHandle::willSendRequest(ResourceRequest& request, const ResourceRes void ResourceHandle::continueWillSendRequest(const ResourceRequest& request) { - ASSERT(client()); - ASSERT(client()->usesAsyncCallbacks()); + ASSERT(d->m_usesAsyncCallbacks); // Client call may not preserve the session, especially if the request is sent over IPC. ResourceRequest newRequest = request; @@ -514,15 +513,14 @@ void ResourceHandle::continueWillSendRequest(const ResourceRequest& request) void ResourceHandle::continueDidReceiveResponse() { - ASSERT(client()); - ASSERT(client()->usesAsyncCallbacks()); + ASSERT(d->m_usesAsyncCallbacks); [delegate() continueDidReceiveResponse]; } bool ResourceHandle::shouldUseCredentialStorage() { - ASSERT(!client()->usesAsyncCallbacks()); + ASSERT(!d->m_usesAsyncCallbacks); return client() && client()->shouldUseCredentialStorage(this); } @@ -631,17 +629,21 @@ void ResourceHandle::didCancelAuthenticationChallenge(const AuthenticationChalle #if USE(PROTECTION_SPACE_AUTH_CALLBACK) bool ResourceHandle::canAuthenticateAgainstProtectionSpace(const ProtectionSpace& protectionSpace) { - if (client()->usesAsyncCallbacks()) { - client()->canAuthenticateAgainstProtectionSpaceAsync(this, protectionSpace); + ResourceHandleClient* client = this->client(); + if (d->m_usesAsyncCallbacks) { + if (client) + client->canAuthenticateAgainstProtectionSpaceAsync(this, protectionSpace); + else + continueCanAuthenticateAgainstProtectionSpace(false); return false; // Ignored by caller. - } else - return client() && client()->canAuthenticateAgainstProtectionSpace(this, protectionSpace); + } + + return client && client->canAuthenticateAgainstProtectionSpace(this, protectionSpace); } void ResourceHandle::continueCanAuthenticateAgainstProtectionSpace(bool result) { - ASSERT(client()); - ASSERT(client()->usesAsyncCallbacks()); + ASSERT(d->m_usesAsyncCallbacks); [(id)delegate() continueCanAuthenticateAgainstProtectionSpace:result]; } @@ -728,8 +730,7 @@ void ResourceHandle::receivedChallengeRejection(const AuthenticationChallenge& c void ResourceHandle::continueWillCacheResponse(NSCachedURLResponse *response) { - ASSERT(client()); - ASSERT(client()->usesAsyncCallbacks()); + ASSERT(d->m_usesAsyncCallbacks); [(id)delegate() continueWillCacheResponse:response]; } diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog index a58db60..796aa7c 100644 --- a/Source/WebKit2/ChangeLog +++ b/Source/WebKit2/ChangeLog @@ -1,3 +1,17 @@ +2015-07-28 Alexey Proskuryakov + + Clean up usesAsyncCallbacks handling in ResourceHandle + https://bugs.webkit.org/show_bug.cgi?id=147342 + + Reviewed by Darin Adler. + + Update for a renaming in WebCore. + + * NetworkProcess/NetworkResourceLoader.cpp: + (WebKit::NetworkResourceLoader::cleanup): + * Shared/Downloads/soup/DownloadSoup.cpp: + (WebKit::Download::platformInvalidate): + 2015-07-28 Chris Fleizach AX: iOS: VoiceOver hangs indefinitely when an JS alert appears diff --git a/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp b/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp index bb73bdb..3c7206a 100644 --- a/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp +++ b/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp @@ -205,7 +205,7 @@ void NetworkResourceLoader::cleanup() invalidateSandboxExtensions(); if (m_handle) { - m_handle->setClient(nullptr); + m_handle->clearClient(); m_handle = nullptr; } diff --git a/Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp b/Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp index a2249ba..74ff1a8 100644 --- a/Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp +++ b/Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp @@ -255,7 +255,7 @@ void Download::cancel() void Download::platformInvalidate() { if (m_resourceHandle) { - m_resourceHandle->setClient(0); + m_resourceHandle->clearClient(); m_resourceHandle->cancel(); m_resourceHandle = nullptr; } -- 1.8.3.1