Make ResourceLoader::willSendRequestInternal asynchronous
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Nov 2017 00:55:47 +0000 (00:55 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Nov 2017 00:55:47 +0000 (00:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179251

Reviewed by Andy Estes.

Source/WebCore:

ResourceLoader::willSendRequestInternal is used for redirects, which need to be asynchronous, and for the initial request.
Making it asynchronous requires making load initialization asynchronous, too.

No change in behavior.  This will allow us to make more things asynchronous.

* loader/LoaderStrategy.h:
* loader/NetscapePlugInStreamLoader.cpp:
(WebCore::NetscapePlugInStreamLoader::create):
(WebCore::NetscapePlugInStreamLoader::init):
(WebCore::NetscapePlugInStreamLoader::willSendRequest):
* loader/NetscapePlugInStreamLoader.h:
* loader/ResourceLoader.cpp:
(WebCore::ResourceLoader::init):
(WebCore::ResourceLoader::willSendRequestInternal):
(WebCore::ResourceLoader::willSendRequest):
(WebCore::ResourceLoader::willSendRequestAsync):
* loader/ResourceLoader.h:
(WebCore::ResourceLoader::startLoading):
* loader/SubresourceLoader.cpp:
(WebCore::SubresourceLoader::create):
(WebCore::SubresourceLoader::startLoading):
(WebCore::SubresourceLoader::init):
(WebCore::SubresourceLoader::willSendRequestInternal):
* loader/SubresourceLoader.h:
* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::load):
* platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm:
(-[WebCoreResourceHandleAsOperationQueueDelegate connection:willSendRequest:redirectResponse:]):
(-[WebCoreResourceHandleAsOperationQueueDelegate connection:willCacheResponse:]):
Calling autorelease from a non-main thread was causing crashes.  This is because we need to set up an autorelease pool on that thread, which we have not done.  See:
https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/mmAutoreleasePools.html#//apple_ref/doc/uid/20000047-1041876
I replaced the calls to RetainPtr::autorelease with RetainPtr::get.  This causes us to keep the NSURLRequest and NSURLResponse alive as long as the request is being
responded to in WebKitLegacy and in El Capitan.  Given the number of ResourceRequest and ResourceResponse copies we store, this shouldn't be a problem memory-wise.
This will all go away once NSURLSession is used for loading in WebKitLegacy, and this is a large step towards that.

Source/WebKit:

* WebProcess/Network/WebLoaderStrategy.cpp:
(WebKit::WebLoaderStrategy::loadResource):
(WebKit::WebLoaderStrategy::schedulePluginStreamLoad):
* WebProcess/Network/WebLoaderStrategy.h:
* WebProcess/Plugins/PluginView.cpp:
(WebKit::PluginView::Stream::start):

Source/WebKitLegacy:

* WebCoreSupport/WebResourceLoadScheduler.cpp:
(WebResourceLoadScheduler::loadResource):
(WebResourceLoadScheduler::schedulePluginStreamLoad):
* WebCoreSupport/WebResourceLoadScheduler.h:

Source/WebKitLegacy/mac:

* Plugins/Hosted/HostedNetscapePluginStream.mm:
(WebKit::HostedNetscapePluginStream::start):
* Plugins/WebNetscapePluginStream.mm:
(WebNetscapePluginStream::start):

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

21 files changed:
Source/WebCore/ChangeLog
Source/WebCore/loader/LoaderStrategy.h
Source/WebCore/loader/NetscapePlugInStreamLoader.cpp
Source/WebCore/loader/NetscapePlugInStreamLoader.h
Source/WebCore/loader/ResourceLoader.cpp
Source/WebCore/loader/ResourceLoader.h
Source/WebCore/loader/SubresourceLoader.cpp
Source/WebCore/loader/SubresourceLoader.h
Source/WebCore/loader/cache/CachedResource.cpp
Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp
Source/WebKit/WebProcess/Network/WebLoaderStrategy.h
Source/WebKit/WebProcess/Plugins/PluginView.cpp
Source/WebKitLegacy/ChangeLog
Source/WebKitLegacy/WebCoreSupport/WebResourceLoadScheduler.cpp
Source/WebKitLegacy/WebCoreSupport/WebResourceLoadScheduler.h
Source/WebKitLegacy/mac/ChangeLog
Source/WebKitLegacy/mac/Plugins/Hosted/HostedNetscapePluginStream.mm
Source/WebKitLegacy/mac/Plugins/WebNetscapePluginStream.mm
Source/WebKitLegacy/win/Plugins/PluginStream.cpp

index e743fa1..2412bdd 100644 (file)
@@ -1,3 +1,45 @@
+2017-11-06  Alex Christensen  <achristensen@webkit.org>
+
+        Make ResourceLoader::willSendRequestInternal asynchronous
+        https://bugs.webkit.org/show_bug.cgi?id=179251
+
+        Reviewed by Andy Estes.
+
+        ResourceLoader::willSendRequestInternal is used for redirects, which need to be asynchronous, and for the initial request.
+        Making it asynchronous requires making load initialization asynchronous, too.
+
+        No change in behavior.  This will allow us to make more things asynchronous.
+
+        * loader/LoaderStrategy.h:
+        * loader/NetscapePlugInStreamLoader.cpp:
+        (WebCore::NetscapePlugInStreamLoader::create):
+        (WebCore::NetscapePlugInStreamLoader::init):
+        (WebCore::NetscapePlugInStreamLoader::willSendRequest):
+        * loader/NetscapePlugInStreamLoader.h:
+        * loader/ResourceLoader.cpp:
+        (WebCore::ResourceLoader::init):
+        (WebCore::ResourceLoader::willSendRequestInternal):
+        (WebCore::ResourceLoader::willSendRequest):
+        (WebCore::ResourceLoader::willSendRequestAsync):
+        * loader/ResourceLoader.h:
+        (WebCore::ResourceLoader::startLoading):
+        * loader/SubresourceLoader.cpp:
+        (WebCore::SubresourceLoader::create):
+        (WebCore::SubresourceLoader::startLoading):
+        (WebCore::SubresourceLoader::init):
+        (WebCore::SubresourceLoader::willSendRequestInternal):
+        * loader/SubresourceLoader.h:
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::load):
+        * platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm:
+        (-[WebCoreResourceHandleAsOperationQueueDelegate connection:willSendRequest:redirectResponse:]):
+        (-[WebCoreResourceHandleAsOperationQueueDelegate connection:willCacheResponse:]):
+        Calling autorelease from a non-main thread was causing crashes.  This is because we need to set up an autorelease pool on that thread, which we have not done.  See:
+        https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/mmAutoreleasePools.html#//apple_ref/doc/uid/20000047-1041876
+        I replaced the calls to RetainPtr::autorelease with RetainPtr::get.  This causes us to keep the NSURLRequest and NSURLResponse alive as long as the request is being
+        responded to in WebKitLegacy and in El Capitan.  Given the number of ResourceRequest and ResourceResponse copies we store, this shouldn't be a problem memory-wise.
+        This will all go away once NSURLSession is used for loading in WebKitLegacy, and this is a large step towards that.
+
 2017-11-06  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         [Attachment Support] Implement delegate hooks for attachment element insertion and removal
index 1e06469..a8f37d0 100644 (file)
@@ -54,7 +54,7 @@ struct FetchOptions;
 
 class WEBCORE_EXPORT LoaderStrategy {
 public:
-    virtual RefPtr<SubresourceLoader> loadResource(Frame&, CachedResource&, const ResourceRequest&, const ResourceLoaderOptions&) = 0;
+    virtual void loadResource(Frame&, CachedResource&, ResourceRequest&&, const ResourceLoaderOptions&, CompletionHandler<void(RefPtr<SubresourceLoader>&&)>&&) = 0;
     virtual void loadResourceSynchronously(NetworkingContext*, unsigned long identifier, const ResourceRequest&, StoredCredentialsPolicy, ClientCredentialPolicy, ResourceError&, ResourceResponse&, Vector<char>& data) = 0;
 
     virtual void remove(ResourceLoader*) = 0;
index a006b4f..edf099d 100644 (file)
@@ -54,13 +54,14 @@ NetscapePlugInStreamLoader::NetscapePlugInStreamLoader(Frame& frame, NetscapePlu
 
 NetscapePlugInStreamLoader::~NetscapePlugInStreamLoader() = default;
 
-RefPtr<NetscapePlugInStreamLoader> NetscapePlugInStreamLoader::create(Frame& frame, NetscapePlugInStreamLoaderClient& client, const ResourceRequest& request)
+void NetscapePlugInStreamLoader::create(Frame& frame, NetscapePlugInStreamLoaderClient& client, ResourceRequest&& request, CompletionHandler<void(RefPtr<NetscapePlugInStreamLoader>&&)>&& completionHandler)
 {
-    auto loader(adoptRef(new NetscapePlugInStreamLoader(frame, client)));
-    if (!loader->init(request))
-        return nullptr;
-
-    return loader;
+    auto loader(adoptRef(*new NetscapePlugInStreamLoader(frame, client)));
+    loader->init(WTFMove(request), [loader = loader.copyRef(), completionHandler = WTFMove(completionHandler)] (bool initialized) mutable {
+        if (!initialized)
+            return completionHandler(nullptr);
+        completionHandler(WTFMove(loader));
+    });
 }
 
 bool NetscapePlugInStreamLoader::isDone() const
@@ -74,26 +75,25 @@ void NetscapePlugInStreamLoader::releaseResources()
     ResourceLoader::releaseResources();
 }
 
-bool NetscapePlugInStreamLoader::init(const ResourceRequest& request)
+void NetscapePlugInStreamLoader::init(ResourceRequest&& request, CompletionHandler<void(bool)>&& completionHandler)
 {
-    if (!ResourceLoader::init(request))
-        return false;
-
-    ASSERT(!reachedTerminalState());
-
-    m_documentLoader->addPlugInStreamLoader(*this);
-    m_isInitialized = true;
-
-    return true;
+    ResourceLoader::init(WTFMove(request), [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (bool success) mutable {
+        if (!success)
+            return completionHandler(false);
+        ASSERT(!reachedTerminalState());
+        m_documentLoader->addPlugInStreamLoader(*this);
+        m_isInitialized = true;
+        completionHandler(true);
+    });
 }
 
 void NetscapePlugInStreamLoader::willSendRequest(ResourceRequest&& request, const ResourceResponse& redirectResponse, CompletionHandler<void(ResourceRequest&&)>&& callback)
 {
-    m_client->willSendRequest(this, WTFMove(request), redirectResponse, [protectedThis = makeRef(*this), redirectResponse, callback = WTFMove(callback)](ResourceRequest request) mutable {
+    m_client->willSendRequest(this, WTFMove(request), redirectResponse, [protectedThis = makeRef(*this), redirectResponse, callback = WTFMove(callback)] (ResourceRequest&& request) mutable {
         if (!request.isNull())
-            protectedThis->willSendRequestInternal(request, redirectResponse);
-
-        callback(WTFMove(request));
+            protectedThis->willSendRequestInternal(WTFMove(request), redirectResponse, WTFMove(callback));
+        else
+            callback({ });
     });
 }
 
index f409b10..600792e 100644 (file)
@@ -51,13 +51,13 @@ protected:
 
 class NetscapePlugInStreamLoader final : public ResourceLoader {
 public:
-    WEBCORE_EXPORT static RefPtr<NetscapePlugInStreamLoader> create(Frame&, NetscapePlugInStreamLoaderClient&, const ResourceRequest&);
+    WEBCORE_EXPORT static void create(Frame&, NetscapePlugInStreamLoaderClient&, ResourceRequest&&, CompletionHandler<void(RefPtr<NetscapePlugInStreamLoader>&&)>&&);
     virtual ~NetscapePlugInStreamLoader();
 
     WEBCORE_EXPORT bool isDone() const;
 
 private:
-    bool init(const ResourceRequest&) override;
+    void init(ResourceRequest&&, CompletionHandler<void(bool)>&&) override;
 
     void willSendRequest(ResourceRequest&&, const ResourceResponse& redirectResponse, CompletionHandler<void(ResourceRequest&&)>&& callback) override;
     void didReceiveResponse(const ResourceResponse&) override;
index b8220f5..844bc87 100644 (file)
@@ -114,15 +114,13 @@ void ResourceLoader::releaseResources()
     m_deferredRequest = ResourceRequest();
 }
 
-bool ResourceLoader::init(const ResourceRequest& r)
+void ResourceLoader::init(ResourceRequest&& clientRequest, CompletionHandler<void(bool)>&& completionHandler)
 {
     ASSERT(!m_handle);
     ASSERT(m_request.isNull());
     ASSERT(m_deferredRequest.isNull());
     ASSERT(!m_documentLoader->isSubstituteLoadPending(this));
     
-    ResourceRequest clientRequest(r);
-
     m_loadTiming.markStartTimeAndFetchStart();
 
 #if PLATFORM(IOS)
@@ -130,17 +128,17 @@ bool ResourceLoader::init(const ResourceRequest& r)
     // in ResourceLoadScheduler queue, don't continue.
     if (!m_documentLoader->frame()) {
         cancel();
-        return false;
+        return completionHandler(false);
     }
 #endif
     
     m_defersLoading = m_options.defersLoadingPolicy == DefersLoadingPolicy::AllowDefersLoading && m_frame->page()->defersLoading();
-    m_canAskClientForCredentials = m_options.clientCredentialPolicy == ClientCredentialPolicy::MayAskClientForCredentials && !isMixedContent(r.url());
+    m_canAskClientForCredentials = m_options.clientCredentialPolicy == ClientCredentialPolicy::MayAskClientForCredentials && !isMixedContent(clientRequest.url());
 
     if (m_options.securityCheck == DoSecurityCheck && !m_frame->document()->securityOrigin().canDisplay(clientRequest.url())) {
         FrameLoader::reportLocalLoadFailed(m_frame.get(), clientRequest.url().string());
         releaseResources();
-        return false;
+        return completionHandler(false);
     }
     
     // https://bugs.webkit.org/show_bug.cgi?id=26391
@@ -153,21 +151,23 @@ bool ResourceLoader::init(const ResourceRequest& r)
             clientRequest.setFirstPartyForCookies(document->firstPartyForCookies());
     }
 
-    willSendRequestInternal(clientRequest, ResourceResponse());
+    willSendRequestInternal(WTFMove(clientRequest), ResourceResponse(), [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)](ResourceRequest&& request) mutable {
 
 #if PLATFORM(IOS)
-    // If this ResourceLoader was stopped as a result of willSendRequest, bail out.
-    if (m_reachedTerminalState)
-        return false;
+        // If this ResourceLoader was stopped as a result of willSendRequest, bail out.
+        if (m_reachedTerminalState)
+            return completionHandler(false);
 #endif
 
-    if (clientRequest.isNull()) {
-        cancel();
-        return false;
-    }
+        if (request.isNull()) {
+            cancel();
+            return completionHandler(false);
+        }
 
-    m_originalRequest = m_request = clientRequest;
-    return true;
+        m_request = WTFMove(request);
+        m_originalRequest = m_request;
+        completionHandler(true);
+    });
 }
 
 void ResourceLoader::deliverResponseAndData(const ResourceResponse& response, RefPtr<SharedBuffer>&& buffer)
@@ -340,7 +340,7 @@ bool ResourceLoader::isMixedContent(const URL& url) const
     return false;
 }
 
-void ResourceLoader::willSendRequestInternal(ResourceRequest& request, const ResourceResponse& redirectResponse)
+void ResourceLoader::willSendRequestInternal(ResourceRequest&& request, const ResourceResponse& redirectResponse, CompletionHandler<void(ResourceRequest&&)>&& completionHandler)
 {
     // Protect this in this delegate method since the additional processing can do
     // anything including possibly derefing this; one example of this is Radar 3266216.
@@ -365,8 +365,8 @@ void ResourceLoader::willSendRequestInternal(ResourceRequest& request, const Res
             auto blockedStatus = page->userContentProvider().processContentExtensionRulesForLoad(request.url(), m_resourceType, *m_documentLoader);
             applyBlockedStatusToRequest(blockedStatus, page, request);
             if (blockedStatus.blockedLoad) {
-                request = { };
                 didFail(blockedByContentBlockerError());
+                completionHandler({ });
                 return;
             }
         }
@@ -375,6 +375,7 @@ void ResourceLoader::willSendRequestInternal(ResourceRequest& request, const Res
 
     if (request.isNull()) {
         didFail(cannotShowURLError());
+        completionHandler({ });
         return;
     }
 
@@ -384,8 +385,10 @@ void ResourceLoader::willSendRequestInternal(ResourceRequest& request, const Res
 
 #if PLATFORM(IOS)
         // If this ResourceLoader was stopped as a result of assignIdentifierToInitialRequest, bail out
-        if (m_reachedTerminalState)
+        if (m_reachedTerminalState) {
+            completionHandler(WTFMove(request));
             return;
+        }
 #endif
 
         frameLoader()->notifier().willSendRequest(this, request, redirectResponse);
@@ -419,12 +422,12 @@ void ResourceLoader::willSendRequestInternal(ResourceRequest& request, const Res
             loadDataURL();
         }
     }
+    completionHandler(WTFMove(request));
 }
 
-void ResourceLoader::willSendRequest(ResourceRequest&& request, const ResourceResponse& redirectResponse, CompletionHandler<void(ResourceRequest&&)>&& callback)
+void ResourceLoader::willSendRequest(ResourceRequest&& request, const ResourceResponse& redirectResponse, CompletionHandler<void(ResourceRequest&&)>&& completionHandler)
 {
-    willSendRequestInternal(request, redirectResponse);
-    callback(WTFMove(request));
+    willSendRequestInternal(WTFMove(request), redirectResponse, WTFMove(completionHandler));
 }
 
 void ResourceLoader::didSendData(unsigned long long, unsigned long long)
@@ -646,8 +649,7 @@ void ResourceLoader::willSendRequestAsync(ResourceHandle* handle, ResourceReques
         completionHandler(WTFMove(request));
         return;
     }
-    willSendRequestInternal(request, redirectResponse);
-    completionHandler(WTFMove(request));
+    willSendRequestInternal(WTFMove(request), redirectResponse, WTFMove(completionHandler));
 }
 
 void ResourceLoader::didSendData(ResourceHandle*, unsigned long long bytesSent, unsigned long long totalBytesToBeSent)
index 377e7fa..099d4e5 100644 (file)
@@ -60,15 +60,14 @@ public:
 
     WEBCORE_EXPORT void cancel();
 
-    virtual bool init(const ResourceRequest&);
+    virtual void init(ResourceRequest&&, CompletionHandler<void(bool)>&&);
 
     void deliverResponseAndData(const ResourceResponse&, RefPtr<SharedBuffer>&&);
 
 #if PLATFORM(IOS)
-    virtual bool startLoading()
+    virtual void startLoading()
     {
         start();
-        return true;
     }
 
     virtual const ResourceRequest& iOSOriginalRequest() const { return request(); }
@@ -165,7 +164,7 @@ protected:
     CFCachedURLResponseRef willCacheResponse(ResourceHandle*, CFCachedURLResponseRef) override;
 #endif
 
-    virtual void willSendRequestInternal(ResourceRequest&, const ResourceResponse& redirectResponse);
+    virtual void willSendRequestInternal(ResourceRequest&&, const ResourceResponse& redirectResponse, CompletionHandler<void(ResourceRequest&&)>&&);
 
     RefPtr<ResourceHandle> m_handle;
     RefPtr<Frame> m_frame;
index 6c885dd..2b63ca3 100644 (file)
@@ -45,6 +45,7 @@
 #include "ResourceLoadObserver.h"
 #include "ResourceTiming.h"
 #include "RuntimeEnabledFeatures.h"
+#include <wtf/CompletionHandler.h>
 #include <wtf/Ref.h>
 #include <wtf/RefCountedLeakCounter.h>
 #include <wtf/StdLibExtras.h>
@@ -102,32 +103,36 @@ SubresourceLoader::~SubresourceLoader()
 #endif
 }
 
-RefPtr<SubresourceLoader> SubresourceLoader::create(Frame& frame, CachedResource& resource, const ResourceRequest& request, const ResourceLoaderOptions& options)
+void SubresourceLoader::create(Frame& frame, CachedResource& resource, ResourceRequest&& request, const ResourceLoaderOptions& options, CompletionHandler<void(RefPtr<SubresourceLoader>&&)>&& completionHandler)
 {
-    RefPtr<SubresourceLoader> subloader(adoptRef(new SubresourceLoader(frame, resource, options)));
+    auto subloader(adoptRef(*new SubresourceLoader(frame, resource, options)));
 #if PLATFORM(IOS)
     if (!IOSApplication::isWebProcess()) {
         // On iOS, do not invoke synchronous resource load delegates while resource load scheduling
         // is disabled to avoid re-entering style selection from a different thread (see <rdar://problem/9121719>).
         // FIXME: This should be fixed for all ports in <https://bugs.webkit.org/show_bug.cgi?id=56647>.
         subloader->m_iOSOriginalRequest = request;
-        return subloader;
+        return completionHandler(WTFMove(subloader));
     }
 #endif
-    if (!subloader->init(request))
-        return nullptr;
-    return subloader;
+    subloader->init(WTFMove(request), [subloader = subloader.copyRef(), completionHandler = WTFMove(completionHandler)] (bool initialized) mutable {
+        if (!initialized)
+            return completionHandler(nullptr);
+        completionHandler(WTFMove(subloader));
+    });
 }
     
 #if PLATFORM(IOS)
-bool SubresourceLoader::startLoading()
+void SubresourceLoader::startLoading()
 {
+    // FIXME: this should probably be removed.
     ASSERT(!IOSApplication::isWebProcess());
-    if (!init(m_iOSOriginalRequest))
-        return false;
-    m_iOSOriginalRequest = ResourceRequest();
-    start();
-    return true;
+    init(ResourceRequest(m_iOSOriginalRequest), [this, protectedThis = makeRef(*this)] (bool success) {
+        if (!success)
+            return;
+        m_iOSOriginalRequest = ResourceRequest();
+        start();
+    });
 }
 #endif
 
@@ -144,18 +149,17 @@ void SubresourceLoader::cancelIfNotFinishing()
     ResourceLoader::cancel();
 }
 
-bool SubresourceLoader::init(const ResourceRequest& request)
+void SubresourceLoader::init(ResourceRequest&& request, CompletionHandler<void(bool)>&& completionHandler)
 {
-    if (!ResourceLoader::init(request))
-        return false;
-
-    ASSERT(!reachedTerminalState());
-    m_state = Initialized;
-    m_documentLoader->addSubresourceLoader(this);
-
-    m_origin = m_resource->origin();
-
-    return true;
+    ResourceLoader::init(WTFMove(request), [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (bool initialized) mutable {
+        if (!initialized)
+            return completionHandler(false);
+        ASSERT(!reachedTerminalState());
+        m_state = Initialized;
+        m_documentLoader->addSubresourceLoader(this);
+        m_origin = m_resource->origin();
+        completionHandler(true);
+    });
 }
 
 bool SubresourceLoader::isSubresourceLoader()
@@ -163,7 +167,7 @@ bool SubresourceLoader::isSubresourceLoader()
     return true;
 }
 
-void SubresourceLoader::willSendRequestInternal(ResourceRequest& newRequest, const ResourceResponse& redirectResponse)
+void SubresourceLoader::willSendRequestInternal(ResourceRequest&& newRequest, const ResourceResponse& redirectResponse, CompletionHandler<void(ResourceRequest&&)>&& completionHandler)
 {
     // Store the previous URL because the call to ResourceLoader::willSendRequest will modify it.
     URL previousURL = request().url();
@@ -171,7 +175,7 @@ void SubresourceLoader::willSendRequestInternal(ResourceRequest& newRequest, con
 
     if (!newRequest.url().isValid()) {
         cancel(cannotShowURLError());
-        return;
+        return completionHandler(WTFMove(newRequest));
     }
 
     if (newRequest.requester() != ResourceRequestBase::Requester::Main) {
@@ -184,7 +188,7 @@ void SubresourceLoader::willSendRequestInternal(ResourceRequest& newRequest, con
         if (options().redirect != FetchOptions::Redirect::Follow) {
             if (options().redirect == FetchOptions::Redirect::Error) {
                 cancel();
-                return;
+                return completionHandler(WTFMove(newRequest));
             }
 
             ResourceResponse opaqueRedirectedResponse = redirectResponse;
@@ -194,10 +198,10 @@ void SubresourceLoader::willSendRequestInternal(ResourceRequest& newRequest, con
 
             NetworkLoadMetrics emptyMetrics;
             didFinishLoading(emptyMetrics);
-            return;
+            return completionHandler(WTFMove(newRequest));
         } else if (m_redirectCount++ >= options().maxRedirectCount) {
             cancel(ResourceError(String(), 0, request().url(), ASCIILiteral("Too many redirections"), ResourceError::Type::General));
-            return;
+            return completionHandler(WTFMove(newRequest));
         }
 
         // CachedResources are keyed off their original request URL.
@@ -213,7 +217,7 @@ void SubresourceLoader::willSendRequestInternal(ResourceRequest& newRequest, con
 
         if (!m_documentLoader->cachedResourceLoader().updateRequestAfterRedirection(m_resource->type(), newRequest, options())) {
             cancel();
-            return;
+            return completionHandler(WTFMove(newRequest));
         }
 
         String errorDescription;
@@ -222,32 +226,33 @@ void SubresourceLoader::willSendRequestInternal(ResourceRequest& newRequest, con
             if (m_frame && m_frame->document())
                 m_frame->document()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, errorMessage);
             cancel(ResourceError(String(), 0, request().url(), errorMessage, ResourceError::Type::AccessControl));
-            return;
+            return completionHandler(WTFMove(newRequest));
         }
 
         if (m_resource->isImage() && m_documentLoader->cachedResourceLoader().shouldDeferImageLoad(newRequest.url())) {
             cancel();
-            return;
+            return completionHandler(WTFMove(newRequest));
         }
         m_loadTiming.addRedirect(redirectResponse.url(), newRequest.url());
         m_resource->redirectReceived(newRequest, redirectResponse);
     }
 
     if (newRequest.isNull() || reachedTerminalState())
-        return;
-
-    ResourceLoader::willSendRequestInternal(newRequest, redirectResponse);
-
-    if (reachedTerminalState())
-        return;
-
-    if (newRequest.isNull()) {
-        cancel();
-        return;
-    }
+        return completionHandler(WTFMove(newRequest));
 
-    if (m_resource->type() == CachedResource::MainResource && !redirectResponse.isNull())
-        m_documentLoader->willContinueMainResourceLoadAfterRedirect(newRequest);
+    ResourceLoader::willSendRequestInternal(WTFMove(newRequest), redirectResponse, [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler), redirectResponse] (ResourceRequest&& request) mutable {
+        if (reachedTerminalState())
+            return completionHandler(WTFMove(request));
+        
+        if (request.isNull()) {
+            cancel();
+            return completionHandler(WTFMove(request));
+        }
+        
+        if (m_resource->type() == CachedResource::MainResource && !redirectResponse.isNull())
+            m_documentLoader->willContinueMainResourceLoadAfterRedirect(request);
+        completionHandler(WTFMove(request));
+    });
 }
 
 void SubresourceLoader::didSendData(unsigned long long bytesSent, unsigned long long totalBytesToBeSent)
index 9af9872..79bb3d5 100644 (file)
@@ -42,7 +42,7 @@ class SecurityOrigin;
 
 class SubresourceLoader final : public ResourceLoader {
 public:
-    WEBCORE_EXPORT static RefPtr<SubresourceLoader> create(Frame&, CachedResource&, const ResourceRequest&, const ResourceLoaderOptions&);
+    WEBCORE_EXPORT static void create(Frame&, CachedResource&, ResourceRequest&&, const ResourceLoaderOptions&, CompletionHandler<void(RefPtr<SubresourceLoader>&&)>&&);
 
     virtual ~SubresourceLoader();
 
@@ -52,7 +52,7 @@ public:
 
     SecurityOrigin* origin() { return m_origin.get(); }
 #if PLATFORM(IOS)
-    bool startLoading() override;
+    void startLoading() override;
 
     // FIXME: What is an "iOS" original request? Why is it necessary?
     const ResourceRequest& iOSOriginalRequest() const override { return m_iOSOriginalRequest; }
@@ -63,9 +63,9 @@ public:
 private:
     SubresourceLoader(Frame&, CachedResource&, const ResourceLoaderOptions&);
 
-    bool init(const ResourceRequest&) override;
+    void init(ResourceRequest&&, CompletionHandler<void(bool)>&&) override;
 
-    void willSendRequestInternal(ResourceRequest&, const ResourceResponse& redirectResponse) override;
+    void willSendRequestInternal(ResourceRequest&&, const ResourceResponse& redirectResponse, CompletionHandler<void(ResourceRequest&&)>&&) override;
     void didSendData(unsigned long long bytesSent, unsigned long long totalBytesToBeSent) override;
     void didReceiveResponse(const ResourceResponse&) override;
     void didReceiveData(const char*, unsigned, long long encodedDataLength, DataPayloadType) override;
index d89dcc9..971a6e3 100644 (file)
@@ -48,6 +48,7 @@
 #include "SecurityOrigin.h"
 #include "SubresourceLoader.h"
 #include "URL.h"
+#include <wtf/CompletionHandler.h>
 #include <wtf/CurrentTime.h>
 #include <wtf/MathExtras.h>
 #include <wtf/RefCountedLeakCounter.h>
@@ -293,14 +294,15 @@ void CachedResource::load(CachedResourceLoader& cachedResourceLoader)
         }
     }
 
-    m_loader = platformStrategies()->loaderStrategy()->loadResource(frame, *this, request, m_options);
-    if (!m_loader) {
-        RELEASE_LOG_IF_ALLOWED("load: Unable to create SubresourceLoader (frame = %p)", &frame);
-        failBeforeStarting();
-        return;
-    }
-
-    m_status = Pending;
+    platformStrategies()->loaderStrategy()->loadResource(frame, *this, WTFMove(request), m_options, [this, protectedThis = CachedResourceHandle<CachedResource>(this), frame = makeRef(frame), loggingAllowed = cachedResourceLoader.isAlwaysOnLoggingAllowed()] (RefPtr<SubresourceLoader>&& loader) {
+        m_loader = WTFMove(loader);
+        if (!m_loader) {
+            RELEASE_LOG_IF(loggingAllowed, Network, "%p - CachedResource::load: Unable to create SubresourceLoader (frame = %p)", this, frame.ptr());
+            failBeforeStarting();
+            return;
+        }
+        m_status = Pending;
+    });
 }
 
 void CachedResource::loadFrom(const CachedResource& resource)
index b343788..7867a7e 100644 (file)
@@ -128,7 +128,7 @@ using namespace WebCore;
         callOnMainThread(WTFMove(work));
 
     dispatch_semaphore_wait(m_semaphore, DISPATCH_TIME_FOREVER);
-    return m_requestResult.autorelease();
+    return m_requestResult.get();
 }
 
 - (void)connection:(NSURLConnection *)connection didReceiveAuthenticationChallenge:(NSURLAuthenticationChallenge *)challenge
@@ -334,7 +334,7 @@ using namespace WebCore;
         callOnMainThread(WTFMove(work));
 
     dispatch_semaphore_wait(m_semaphore, DISPATCH_TIME_FOREVER);
-    return m_cachedResponseResult.autorelease();
+    return m_cachedResponseResult.get();
 }
 
 @end
index 06e4451..5562344 100644 (file)
@@ -1,3 +1,17 @@
+2017-11-06  Alex Christensen  <achristensen@webkit.org>
+
+        Make ResourceLoader::willSendRequestInternal asynchronous
+        https://bugs.webkit.org/show_bug.cgi?id=179251
+
+        Reviewed by Andy Estes.
+
+        * WebProcess/Network/WebLoaderStrategy.cpp:
+        (WebKit::WebLoaderStrategy::loadResource):
+        (WebKit::WebLoaderStrategy::schedulePluginStreamLoad):
+        * WebProcess/Network/WebLoaderStrategy.h:
+        * WebProcess/Plugins/PluginView.cpp:
+        (WebKit::PluginView::Stream::start):
+
 2017-11-06  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         [Attachment Support] Implement delegate hooks for attachment element insertion and removal
index c75ba58..1df3fc9 100644 (file)
@@ -65,6 +65,7 @@
 #include <WebCore/SubresourceLoader.h>
 #include <WebCore/UserContentProvider.h>
 #include <pal/SessionID.h>
+#include <wtf/CompletionHandler.h>
 #include <wtf/text/CString.h>
 
 #if USE(QUICK_LOOK)
@@ -87,22 +88,24 @@ WebLoaderStrategy::~WebLoaderStrategy()
 {
 }
 
-RefPtr<SubresourceLoader> WebLoaderStrategy::loadResource(Frame& frame, CachedResource& resource, const ResourceRequest& request, const ResourceLoaderOptions& options)
+void WebLoaderStrategy::loadResource(Frame& frame, CachedResource& resource, ResourceRequest&& request, const ResourceLoaderOptions& options, CompletionHandler<void(RefPtr<SubresourceLoader>&&)>&& completionHandler)
 {
-    RefPtr<SubresourceLoader> loader = SubresourceLoader::create(frame, resource, request, options);
-    if (loader)
-        scheduleLoad(*loader, &resource, frame.document()->referrerPolicy() == ReferrerPolicy::NoReferrerWhenDowngrade);
-    else
-        RELEASE_LOG_IF_ALLOWED(frame, "loadResource: Unable to create SubresourceLoader (frame = %p", &frame);
-    return loader;
+    SubresourceLoader::create(frame, resource, WTFMove(request), options, [this, completionHandler = WTFMove(completionHandler), resource = CachedResourceHandle<CachedResource>(&resource), frame = makeRef(frame)] (RefPtr<SubresourceLoader>&& loader) mutable {
+        if (loader)
+            scheduleLoad(*loader, resource.get(), frame->document()->referrerPolicy() == ReferrerPolicy::NoReferrerWhenDowngrade);
+        else
+            RELEASE_LOG_IF_ALLOWED(frame.get(), "loadResource: Unable to create SubresourceLoader (frame = %p", &frame);
+        completionHandler(WTFMove(loader));
+    });
 }
 
-RefPtr<NetscapePlugInStreamLoader> WebLoaderStrategy::schedulePluginStreamLoad(Frame& frame, NetscapePlugInStreamLoaderClient& client, const ResourceRequest& request)
+void WebLoaderStrategy::schedulePluginStreamLoad(Frame& frame, NetscapePlugInStreamLoaderClient& client, ResourceRequest&& request, CompletionHandler<void(RefPtr<NetscapePlugInStreamLoader>&&)>&& completionHandler)
 {
-    RefPtr<NetscapePlugInStreamLoader> loader = NetscapePlugInStreamLoader::create(frame, client, request);
-    if (loader)
-        scheduleLoad(*loader, 0, frame.document()->referrerPolicy() == ReferrerPolicy::NoReferrerWhenDowngrade);
-    return loader;
+    NetscapePlugInStreamLoader::create(frame, client, WTFMove(request), [this, completionHandler = WTFMove(completionHandler), frame = makeRef(frame)] (RefPtr<NetscapePlugInStreamLoader>&& loader) mutable {
+        if (loader)
+            scheduleLoad(*loader, 0, frame->document()->referrerPolicy() == ReferrerPolicy::NoReferrerWhenDowngrade);
+        completionHandler(WTFMove(loader));
+    });
 }
 
 static Seconds maximumBufferingTime(CachedResource* resource)
index e2c5561..5c7f6da 100644 (file)
@@ -47,7 +47,7 @@ public:
     WebLoaderStrategy();
     ~WebLoaderStrategy() final;
     
-    RefPtr<WebCore::SubresourceLoader> loadResource(WebCore::Frame&, WebCore::CachedResource&, const WebCore::ResourceRequest&, const WebCore::ResourceLoaderOptions&) final;
+    void loadResource(WebCore::Frame&, WebCore::CachedResource&, WebCore::ResourceRequest&&, const WebCore::ResourceLoaderOptions&, CompletionHandler<void(RefPtr<WebCore::SubresourceLoader>&&)>&&) final;
     void loadResourceSynchronously(WebCore::NetworkingContext*, unsigned long resourceLoadIdentifier, const WebCore::ResourceRequest&, WebCore::StoredCredentialsPolicy, WebCore::ClientCredentialPolicy, WebCore::ResourceError&, WebCore::ResourceResponse&, Vector<char>& data) final;
 
     void remove(WebCore::ResourceLoader*) final;
@@ -70,7 +70,7 @@ public:
     void setCaptureExtraNetworkLoadMetricsEnabled(bool) final;
 
     WebResourceLoader* webResourceLoaderForIdentifier(ResourceLoadIdentifier identifier) const { return m_webResourceLoaders.get(identifier); }
-    RefPtr<WebCore::NetscapePlugInStreamLoader> schedulePluginStreamLoad(WebCore::Frame&, WebCore::NetscapePlugInStreamLoaderClient&, const WebCore::ResourceRequest&);
+    void schedulePluginStreamLoad(WebCore::Frame&, WebCore::NetscapePlugInStreamLoaderClient&, WebCore::ResourceRequest&&, CompletionHandler<void(RefPtr<WebCore::NetscapePlugInStreamLoader>&&)>&&);
 
     void networkProcessCrashed();
 
index 956984b..1ae3205 100644 (file)
@@ -167,7 +167,9 @@ void PluginView::Stream::start()
     Frame* frame = m_pluginView->m_pluginElement->document().frame();
     ASSERT(frame);
 
-    m_loader = WebProcess::singleton().webLoaderStrategy().schedulePluginStreamLoad(*frame, *this, m_request);
+    WebProcess::singleton().webLoaderStrategy().schedulePluginStreamLoad(*frame, *this, ResourceRequest {m_request}, [this, protectedThis = makeRef(*this)](RefPtr<NetscapePlugInStreamLoader>&& loader) {
+        m_loader = WTFMove(loader);
+    });
 }
 
 void PluginView::Stream::cancel()
index 9d819cf..e3e412d 100644 (file)
@@ -1,3 +1,15 @@
+2017-11-06  Alex Christensen  <achristensen@webkit.org>
+
+        Make ResourceLoader::willSendRequestInternal asynchronous
+        https://bugs.webkit.org/show_bug.cgi?id=179251
+
+        Reviewed by Andy Estes.
+
+        * WebCoreSupport/WebResourceLoadScheduler.cpp:
+        (WebResourceLoadScheduler::loadResource):
+        (WebResourceLoadScheduler::schedulePluginStreamLoad):
+        * WebCoreSupport/WebResourceLoadScheduler.h:
+
 2017-11-02  Christopher Reid  <chris.reid@sony.com>
 
         Add a FileSystem namespace to FileSystem.cpp
index 3b39264..f0a9439 100644 (file)
@@ -88,21 +88,22 @@ WebResourceLoadScheduler::~WebResourceLoadScheduler()
 {
 }
 
-RefPtr<SubresourceLoader> WebResourceLoadScheduler::loadResource(Frame& frame, CachedResource& resource, const ResourceRequest& request, const ResourceLoaderOptions& options)
+void WebResourceLoadScheduler::loadResource(Frame& frame, CachedResource& resource, ResourceRequest&& request, const ResourceLoaderOptions& options, CompletionHandler<void(RefPtr<WebCore::SubresourceLoader>&&)>&& completionHandler)
 {
-    RefPtr<SubresourceLoader> loader = SubresourceLoader::create(frame, resource, request, options);
-    if (loader)
-        scheduleLoad(loader.get());
+    SubresourceLoader::create(frame, resource, WTFMove(request), options, [this, completionHandler = WTFMove(completionHandler)] (RefPtr<WebCore::SubresourceLoader>&& loader) mutable {
+        if (loader)
+            scheduleLoad(loader.get());
 #if PLATFORM(IOS)
-    // Since we defer loader initialization until scheduling on iOS, the frame
-    // load delegate that would be called in SubresourceLoader::create() on
-    // other ports might be called in scheduleLoad() instead. Our contract to
-    // callers of this method is that a null loader is returned if the load was
-    // cancelled by a frame load delegate.
-    if (!loader || loader->reachedTerminalState())
-        return nullptr;
+        // Since we defer loader initialization until scheduling on iOS, the frame
+        // load delegate that would be called in SubresourceLoader::create() on
+        // other ports might be called in scheduleLoad() instead. Our contract to
+        // callers of this method is that a null loader is returned if the load was
+        // cancelled by a frame load delegate.
+        if (!loader || loader->reachedTerminalState())
+            return completionHandler(nullptr);
 #endif
-    return loader;
+        completionHandler(WTFMove(loader));
+    });
 }
 
 void WebResourceLoadScheduler::loadResourceSynchronously(NetworkingContext* context, unsigned long, const ResourceRequest& request, StoredCredentialsPolicy storedCredentialsPolicy, ClientCredentialPolicy, ResourceError& error, ResourceResponse& response, Vector<char>& data)
@@ -110,12 +111,13 @@ void WebResourceLoadScheduler::loadResourceSynchronously(NetworkingContext* cont
     ResourceHandle::loadResourceSynchronously(context, request, storedCredentialsPolicy, error, response, data);
 }
 
-RefPtr<NetscapePlugInStreamLoader> WebResourceLoadScheduler::schedulePluginStreamLoad(Frame& frame, NetscapePlugInStreamLoaderClient& client, const ResourceRequest& request)
+void WebResourceLoadScheduler::schedulePluginStreamLoad(Frame& frame, NetscapePlugInStreamLoaderClient& client, ResourceRequest&& request, CompletionHandler<void(RefPtr<WebCore::NetscapePlugInStreamLoader>&&)>&& completionHandler)
 {
-    RefPtr<NetscapePlugInStreamLoader> loader = NetscapePlugInStreamLoader::create(frame, client, request);
-    if (loader)
-        scheduleLoad(loader.get());
-    return loader;
+    NetscapePlugInStreamLoader::create(frame, client, WTFMove(request), [this, completionHandler = WTFMove(completionHandler)] (RefPtr<WebCore::NetscapePlugInStreamLoader>&& loader) mutable {
+        if (loader)
+            scheduleLoad(loader.get());
+        completionHandler(WTFMove(loader));
+    });
 }
 
 void WebResourceLoadScheduler::scheduleLoad(ResourceLoader* resourceLoader)
index a0bc167..40265bc 100644 (file)
@@ -49,7 +49,7 @@ class WebResourceLoadScheduler final : public WebCore::LoaderStrategy {
 public:
     WebResourceLoadScheduler();
 
-    RefPtr<WebCore::SubresourceLoader> loadResource(WebCore::Frame&, WebCore::CachedResource&, const WebCore::ResourceRequest&, const WebCore::ResourceLoaderOptions&) final;
+    void loadResource(WebCore::Frame&, WebCore::CachedResource&, WebCore::ResourceRequest&&, const WebCore::ResourceLoaderOptions&, CompletionHandler<void(RefPtr<WebCore::SubresourceLoader>&&)>&&) final;
     void loadResourceSynchronously(WebCore::NetworkingContext*, unsigned long, const WebCore::ResourceRequest&, WebCore::StoredCredentialsPolicy, WebCore::ClientCredentialPolicy, WebCore::ResourceError&, WebCore::ResourceResponse&, Vector<char>&) final;
     void remove(WebCore::ResourceLoader*) final;
     void setDefersLoading(WebCore::ResourceLoader*, bool) final;
@@ -70,7 +70,7 @@ public:
     bool isSerialLoadingEnabled() const { return m_isSerialLoadingEnabled; }
     void setSerialLoadingEnabled(bool b) { m_isSerialLoadingEnabled = b; }
 
-    RefPtr<WebCore::NetscapePlugInStreamLoader> schedulePluginStreamLoad(WebCore::Frame&, WebCore::NetscapePlugInStreamLoaderClient&, const WebCore::ResourceRequest&);
+    void schedulePluginStreamLoad(WebCore::Frame&, WebCore::NetscapePlugInStreamLoaderClient&, WebCore::ResourceRequest&&, CompletionHandler<void(RefPtr<WebCore::NetscapePlugInStreamLoader>&&)>&&);
 
 protected:
     virtual ~WebResourceLoadScheduler();
index cfd52af..603ef2d 100644 (file)
@@ -1,3 +1,15 @@
+2017-11-06  Alex Christensen  <achristensen@webkit.org>
+
+        Make ResourceLoader::willSendRequestInternal asynchronous
+        https://bugs.webkit.org/show_bug.cgi?id=179251
+
+        Reviewed by Andy Estes.
+
+        * Plugins/Hosted/HostedNetscapePluginStream.mm:
+        (WebKit::HostedNetscapePluginStream::start):
+        * Plugins/WebNetscapePluginStream.mm:
+        (WebNetscapePluginStream::start):
+
 2017-11-01  Darin Adler  <darin@apple.com>
 
         Simplify event dispatch code and make it a bit more consistent
index 6e64bf0..5e22328 100644 (file)
@@ -223,7 +223,9 @@ void HostedNetscapePluginStream::start()
     ASSERT(!m_frameLoader);
     ASSERT(!m_loader);
 
-    m_loader = webResourceLoadScheduler().schedulePluginStreamLoad(*core([m_instance->pluginView() webFrame]), *this, m_request.get());
+    webResourceLoadScheduler().schedulePluginStreamLoad(*core([m_instance->pluginView() webFrame]), *this, m_request.get(), [this, protectedThis = makeRef(*this)] (RefPtr<WebCore::NetscapePlugInStreamLoader>&& loader) {
+        m_loader = WTFMove(loader);
+    });
 }
 
 void HostedNetscapePluginStream::stop()
index ab75d3c..930df3d 100644 (file)
@@ -291,7 +291,9 @@ void WebNetscapePluginStream::start()
     ASSERT(!m_frameLoader);
     ASSERT(!m_loader);
 
-    m_loader = webResourceLoadScheduler().schedulePluginStreamLoad(*core([m_pluginView.get() webFrame]), *this, m_request.get());
+    webResourceLoadScheduler().schedulePluginStreamLoad(*core([m_pluginView.get() webFrame]), *this, m_request.get(), [this, protectedThis = makeRef(*this)] (RefPtr<WebCore::NetscapePlugInStreamLoader>&& loader) {
+        m_loader = WTFMove(loader);
+    });
 }
 
 void WebNetscapePluginStream::stop()
index 896bb85..59fc3bf 100644 (file)
@@ -98,7 +98,9 @@ void PluginStream::start()
 {
     ASSERT(!m_loadManually);
     ASSERT(m_frame);
-    m_loader = webResourceLoadScheduler().schedulePluginStreamLoad(*m_frame, *this, m_resourceRequest);
+    webResourceLoadScheduler().schedulePluginStreamLoad(*m_frame, *this, ResourceRequest(m_resourceRequest), [this, protectedThis = makeRef(*this)] (RefPtr<WebCore::NetscapePlugInStreamLoader>&& loader) {
+        m_loader = WTFMove(loader);
+    });
 }
 
 void PluginStream::stop()