Simplify calls to LoaderStrategy::startPingLoad()
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 20 Aug 2017 23:11:40 +0000 (23:11 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 20 Aug 2017 23:11:40 +0000 (23:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=175756

Reviewed by Sam Weinig.

Source/WebCore:

Simplify calls to LoaderStrategy::startPingLoad() by passing the Frame to it
and let its implementation gets what it needs from the frame. This reduces
the number of parameters to startPingLoad() and is more easily extensible.

* dom/Document.h:
* loader/LoaderStrategy.h:
* loader/PingLoader.cpp:
(WebCore::PingLoader::loadImage):
(WebCore::PingLoader::sendPing):
(WebCore::PingLoader::sendViolationReport):
(WebCore::PingLoader::startPingLoad):
* loader/PingLoader.h:
* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::load):

Source/WebKit:

* WebProcess/Network/WebLoaderStrategy.cpp:
(WebKit::WebLoaderStrategy::startPingLoad):
* WebProcess/Network/WebLoaderStrategy.h:

Source/WebKitLegacy:

* WebCoreSupport/WebResourceLoadScheduler.cpp:
(WebResourceLoadScheduler::startPingLoad):
* WebCoreSupport/WebResourceLoadScheduler.h:

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

12 files changed:
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.h
Source/WebCore/loader/LoaderStrategy.h
Source/WebCore/loader/PingLoader.cpp
Source/WebCore/loader/PingLoader.h
Source/WebCore/loader/cache/CachedResource.cpp
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp
Source/WebKit/WebProcess/Network/WebLoaderStrategy.h
Source/WebKitLegacy/ChangeLog
Source/WebKitLegacy/WebCoreSupport/WebResourceLoadScheduler.cpp
Source/WebKitLegacy/WebCoreSupport/WebResourceLoadScheduler.h

index feb91da..856bbf8 100644 (file)
@@ -1,3 +1,25 @@
+2017-08-20  Chris Dumez  <cdumez@apple.com>
+
+        Simplify calls to LoaderStrategy::startPingLoad()
+        https://bugs.webkit.org/show_bug.cgi?id=175756
+
+        Reviewed by Sam Weinig.
+
+        Simplify calls to LoaderStrategy::startPingLoad() by passing the Frame to it
+        and let its implementation gets what it needs from the frame. This reduces
+        the number of parameters to startPingLoad() and is more easily extensible.
+
+        * dom/Document.h:
+        * loader/LoaderStrategy.h:
+        * loader/PingLoader.cpp:
+        (WebCore::PingLoader::loadImage):
+        (WebCore::PingLoader::sendPing):
+        (WebCore::PingLoader::sendViolationReport):
+        (WebCore::PingLoader::startPingLoad):
+        * loader/PingLoader.h:
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::load):
+
 2017-08-20  Antti Koivisto  <antti@apple.com>
 
         Factor :before/:after render tree mutations into a RenderTreeUpdater helper class
index 3f002aa..be2fe46 100644 (file)
@@ -576,7 +576,7 @@ public:
     void prepareForDestruction();
 
     // Override ScriptExecutionContext methods to do additional work
-    bool shouldBypassMainWorldContentSecurityPolicy() const final;
+    WEBCORE_EXPORT bool shouldBypassMainWorldContentSecurityPolicy() const final;
     void suspendActiveDOMObjects(ActiveDOMObject::ReasonForSuspension) final;
     void resumeActiveDOMObjects(ActiveDOMObject::ReasonForSuspension) final;
     void stopActiveDOMObjects() final;
index 8827567..d6590a2 100644 (file)
@@ -65,7 +65,7 @@ public:
     virtual void resumePendingRequests() = 0;
 
     using PingLoadCompletionHandler = WTF::Function<void(const ResourceError&)>;
-    virtual void startPingLoad(NetworkingContext*, ResourceRequest&, const HTTPHeaderMap& originalRequestHeaders, Ref<SecurityOrigin>&& sourceOrigin, ContentSecurityPolicy*, const FetchOptions&, PingLoadCompletionHandler&& = { }) = 0;
+    virtual void startPingLoad(Frame&, ResourceRequest&, const HTTPHeaderMap& originalRequestHeaders, const FetchOptions&, PingLoadCompletionHandler&& = { }) = 0;
 
     virtual void storeDerivedDataToCache(const SHA1::Digest& bodyKey, const String& type, const String& partition, WebCore::SharedBuffer&) = 0;
 
index a2eb98d..53aa257 100644 (file)
@@ -106,7 +106,7 @@ void PingLoader::loadImage(Frame& frame, const URL& url)
         request.setHTTPReferrer(referrer);
     frame.loader().addExtraFieldsToSubresourceRequest(request);
 
-    startPingLoad(frame, request, WTFMove(originalRequestHeader), document, ShouldFollowRedirects::Yes);
+    startPingLoad(frame, request, WTFMove(originalRequestHeader), ShouldFollowRedirects::Yes);
 }
 
 // http://www.whatwg.org/specs/web-apps/current-work/multipage/links.html#hyperlink-auditing
@@ -145,7 +145,7 @@ void PingLoader::sendPing(Frame& frame, const URL& pingURL, const URL& destinati
         }
     }
 
-    startPingLoad(frame, request, WTFMove(originalRequestHeader), document, ShouldFollowRedirects::Yes);
+    startPingLoad(frame, request, WTFMove(originalRequestHeader), ShouldFollowRedirects::Yes);
 }
 
 void PingLoader::sendViolationReport(Frame& frame, const URL& reportURL, Ref<FormData>&& report, ViolationReportType reportType)
@@ -184,10 +184,10 @@ void PingLoader::sendViolationReport(Frame& frame, const URL& reportURL, Ref<For
     if (!referrer.isEmpty())
         request.setHTTPReferrer(referrer);
 
-    startPingLoad(frame, request, WTFMove(originalRequestHeader), document, ShouldFollowRedirects::No);
+    startPingLoad(frame, request, WTFMove(originalRequestHeader), ShouldFollowRedirects::No);
 }
 
-void PingLoader::startPingLoad(Frame& frame, ResourceRequest& request, HTTPHeaderMap&& originalRequestHeaders, Document& document, ShouldFollowRedirects shouldFollowRedirects)
+void PingLoader::startPingLoad(Frame& frame, ResourceRequest& request, HTTPHeaderMap&& originalRequestHeaders, ShouldFollowRedirects shouldFollowRedirects)
 {
     unsigned long identifier = frame.page()->progress().createUniqueIdentifier();
     // FIXME: Why activeDocumentLoader? I would have expected documentLoader().
@@ -202,8 +202,7 @@ void PingLoader::startPingLoad(Frame& frame, ResourceRequest& request, HTTPHeade
 
     InspectorInstrumentation::continueAfterPingLoader(frame, identifier, frame.loader().activeDocumentLoader(), request, ResourceResponse());
 
-    auto* contentSecurityPolicy = document.shouldBypassMainWorldContentSecurityPolicy() ? nullptr : document.contentSecurityPolicy();
-    platformStrategies()->loaderStrategy()->startPingLoad(frame.loader().networkingContext(), request, WTFMove(originalRequestHeaders), document.securityOrigin(), contentSecurityPolicy, options);
+    platformStrategies()->loaderStrategy()->startPingLoad(frame, request, WTFMove(originalRequestHeaders), options);
 }
 
 }
index 05c5497..175a470 100644 (file)
@@ -37,7 +37,6 @@
 
 namespace WebCore {
 
-class Document;
 class FormData;
 class Frame;
 class HTTPHeaderMap;
@@ -57,7 +56,7 @@ public:
 
 private:
     enum class ShouldFollowRedirects { No, Yes };
-    static void startPingLoad(Frame&, ResourceRequest&, HTTPHeaderMap&& originalRequestHeaders, Document&, ShouldFollowRedirects);
+    static void startPingLoad(Frame&, ResourceRequest&, HTTPHeaderMap&& originalRequestHeaders, ShouldFollowRedirects);
 };
 
 } // namespace WebCore
index 485fc39..adf7558 100644 (file)
@@ -268,13 +268,9 @@ void CachedResource::load(CachedResourceLoader& cachedResourceLoader)
         }
         // FIXME: We should not special-case Beacon here.
         if (shouldUsePingLoad(type())) {
-            ASSERT(m_origin);
-            // Beacon is not exposed to workers so it is safe to rely on the document here.
-            auto* document = cachedResourceLoader.document();
-            auto* contentSecurityPolicy = document && !document->shouldBypassMainWorldContentSecurityPolicy() ? document->contentSecurityPolicy() : nullptr;
             ASSERT(m_originalRequestHeaders);
             CachedResourceHandle<CachedResource> protectedThis(this);
-            platformStrategies()->loaderStrategy()->startPingLoad(frame.loader().networkingContext(), request, *m_originalRequestHeaders, *m_origin, contentSecurityPolicy, m_options, [this, protectedThis = WTFMove(protectedThis)] (const ResourceError& error) {
+            platformStrategies()->loaderStrategy()->startPingLoad(frame, request, *m_originalRequestHeaders, m_options, [this, protectedThis = WTFMove(protectedThis)] (const ResourceError& error) {
                 if (error.isNull())
                     finishLoading(nullptr);
                 else {
index 1f8614d..de2289f 100644 (file)
@@ -1,3 +1,14 @@
+2017-08-20  Chris Dumez  <cdumez@apple.com>
+
+        Simplify calls to LoaderStrategy::startPingLoad()
+        https://bugs.webkit.org/show_bug.cgi?id=175756
+
+        Reviewed by Sam Weinig.
+
+        * WebProcess/Network/WebLoaderStrategy.cpp:
+        (WebKit::WebLoaderStrategy::startPingLoad):
+        * WebProcess/Network/WebLoaderStrategy.h:
+
 2017-08-18  Chris Dumez  <cdumez@apple.com>
 
         [Beacon] Improve error reporting
index e97fcbd..6fbe306 100644 (file)
@@ -396,10 +396,11 @@ static uint64_t generatePingLoadIdentifier()
     return ++identifier;
 }
 
-void WebLoaderStrategy::startPingLoad(NetworkingContext* networkingContext, ResourceRequest& request, const HTTPHeaderMap& originalRequestHeaders, Ref<SecurityOrigin>&& sourceOrigin, ContentSecurityPolicy* contentSecurityPolicy, const FetchOptions& options, PingLoadCompletionHandler&& completionHandler)
+void WebLoaderStrategy::startPingLoad(Frame& frame, ResourceRequest& request, const HTTPHeaderMap& originalRequestHeaders, const FetchOptions& options, PingLoadCompletionHandler&& completionHandler)
 {
     // It's possible that call to createPingHandle might be made during initial empty Document creation before a NetworkingContext exists.
     // It is not clear that we should send ping loads during that process anyways.
+    auto* networkingContext = frame.loader().networkingContext();
     if (!networkingContext) {
         if (completionHandler)
             completionHandler(internalError(request.url()));
@@ -410,18 +411,27 @@ void WebLoaderStrategy::startPingLoad(NetworkingContext* networkingContext, Reso
     WebFrameLoaderClient* webFrameLoaderClient = webContext->webFrameLoaderClient();
     WebFrame* webFrame = webFrameLoaderClient ? webFrameLoaderClient->webFrame() : nullptr;
     WebPage* webPage = webFrame ? webFrame->page() : nullptr;
+
+    auto* document = frame.document();
+    if (!document) {
+        if (completionHandler)
+            completionHandler(internalError(request.url()));
+        return;
+    }
     
     NetworkResourceLoadParameters loadParameters;
     loadParameters.identifier = generatePingLoadIdentifier();
     loadParameters.request = request;
-    loadParameters.sourceOrigin = WTFMove(sourceOrigin);
+    loadParameters.sourceOrigin = &document->securityOrigin();
     loadParameters.sessionID = webPage ? webPage->sessionID() : PAL::SessionID::defaultSessionID();
     loadParameters.allowStoredCredentials = options.credentials == FetchOptions::Credentials::Omit ? DoNotAllowStoredCredentials : AllowStoredCredentials;
     loadParameters.mode = options.mode;
     loadParameters.shouldFollowRedirects = options.redirect == FetchOptions::Redirect::Follow;
     loadParameters.shouldClearReferrerOnHTTPSToHTTPRedirect = networkingContext->shouldClearReferrerOnHTTPSToHTTPRedirect();
-    if (contentSecurityPolicy)
-        loadParameters.cspResponseHeaders = contentSecurityPolicy->responseHeaders();
+    if (!document->shouldBypassMainWorldContentSecurityPolicy()) {
+        if (auto * contentSecurityPolicy = document->contentSecurityPolicy())
+            loadParameters.cspResponseHeaders = contentSecurityPolicy->responseHeaders();
+    }
 
     if (completionHandler)
         m_pingLoadCompletionHandlers.add(loadParameters.identifier, WTFMove(completionHandler));
index 2f44f8d..f4f1577 100644 (file)
@@ -59,7 +59,7 @@ public:
     void suspendPendingRequests() final;
     void resumePendingRequests() final;
 
-    void startPingLoad(WebCore::NetworkingContext*, WebCore::ResourceRequest&, const WebCore::HTTPHeaderMap& originalRequestHeaders, Ref<WebCore::SecurityOrigin>&& sourceOrigin, WebCore::ContentSecurityPolicy*, const WebCore::FetchOptions&, PingLoadCompletionHandler&&) final;
+    void startPingLoad(WebCore::Frame&, WebCore::ResourceRequest&, const WebCore::HTTPHeaderMap& originalRequestHeaders, const WebCore::FetchOptions&, PingLoadCompletionHandler&&) final;
     void didFinishPingLoad(uint64_t pingLoadIdentifier, WebCore::ResourceError&&);
 
     void storeDerivedDataToCache(const SHA1::Digest& bodyHash, const String& type, const String& partition, WebCore::SharedBuffer&) final;
index 8ab7172..643d510 100644 (file)
@@ -1,3 +1,14 @@
+2017-08-20  Chris Dumez  <cdumez@apple.com>
+
+        Simplify calls to LoaderStrategy::startPingLoad()
+        https://bugs.webkit.org/show_bug.cgi?id=175756
+
+        Reviewed by Sam Weinig.
+
+        * WebCoreSupport/WebResourceLoadScheduler.cpp:
+        (WebResourceLoadScheduler::startPingLoad):
+        * WebCoreSupport/WebResourceLoadScheduler.h:
+
 2017-08-18  Chris Dumez  <cdumez@apple.com>
 
         [Beacon] Improve error reporting
index dbe9d3d..0be002b 100644 (file)
@@ -363,9 +363,9 @@ bool WebResourceLoadScheduler::HostInformation::limitRequests(ResourceLoadPriori
     return m_requestsLoading.size() >= (webResourceLoadScheduler().isSerialLoadingEnabled() ? 1 : m_maxRequestsInFlight);
 }
 
-void WebResourceLoadScheduler::startPingLoad(NetworkingContext* networkingContext, ResourceRequest& request, const HTTPHeaderMap&, Ref<SecurityOrigin>&&, WebCore::ContentSecurityPolicy*, const FetchOptions& options, PingLoadCompletionHandler&& completionHandler)
+void WebResourceLoadScheduler::startPingLoad(Frame& frame, ResourceRequest& request, const HTTPHeaderMap&, const FetchOptions& options, PingLoadCompletionHandler&& completionHandler)
 {
     // PingHandle manages its own lifetime, deleting itself when its purpose has been fulfilled.
-    new PingHandle(networkingContext, request, options.credentials != FetchOptions::Credentials::Omit, PingHandle::UsesAsyncCallbacks::No, options.redirect == FetchOptions::Redirect::Follow, WTFMove(completionHandler));
+    new PingHandle(frame.loader().networkingContext(), request, options.credentials != FetchOptions::Credentials::Omit, PingHandle::UsesAsyncCallbacks::No, options.redirect == FetchOptions::Redirect::Follow, WTFMove(completionHandler));
 }
 
index f816627..c18c32e 100644 (file)
@@ -59,7 +59,7 @@ public:
     void suspendPendingRequests() final;
     void resumePendingRequests() final;
 
-    void startPingLoad(WebCore::NetworkingContext*, WebCore::ResourceRequest&, const WebCore::HTTPHeaderMap&, Ref<WebCore::SecurityOrigin>&& sourceOrigin, WebCore::ContentSecurityPolicy*, const WebCore::FetchOptions&, PingLoadCompletionHandler&&) final;
+    void startPingLoad(WebCore::Frame&, WebCore::ResourceRequest&, const WebCore::HTTPHeaderMap&, const WebCore::FetchOptions&, PingLoadCompletionHandler&&) final;
 
     void storeDerivedDataToCache(const SHA1::Digest&, const String&, const String&, WebCore::SharedBuffer&) final { }