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 feb91dafbe885aa2461f5e206289f96ffc3b5ea7..856bbf888b9937dc2f0063ce1a748424b4661227 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 3f002aa4024a21046dd9b63ea3785c274d524684..be2fe46be00261b98ba635b5e11b62b2e4005d50 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 8827567c444c19b245315e2d29514b9b02abb25a..d6590a2accf6d753e2cbf012d3653efb676cfa99 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 a2eb98de3cef1e6c267dc1d6dbc12e2beb034dcf..53aa2578c5c633abc2cb5c5351797953dcabda92 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 05c5497528a4979f974cdfeaa4f9c085ad2326ec..175a470a5904a5feb70804a5d6a0ced0342b7ec1 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 485fc39172351d5450e56d8bedf20d97b4f4f61f..adf755844338d069f558d35e70e33ee4e64c3687 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 1f8614d623107531fa623b9de833282f6a64c00e..de2289fcd3d616cec8c58555c3f9ad23b7694032 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 e97fcbd7f920cf941e9066d879d1dee0e11ea9f6..6fbe306e4d75760a0c26cb6325c882f0c4e42649 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 2f44f8db80adc3c440893ebd7446cfb739609a73..f4f15772a7344e7f08c609557b4febcfca09cacf 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 8ab717257e1bcfcd1c3480a0d862b7f8a07f3d15..643d510b103c2f478e815777114ef816f7725b7c 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 dbe9d3d677caba633b4f804bbfaba5353843e8fd..0be002b3175f6db5ffb5f2ef3557c684a38e0cf9 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 f816627ea44bffefd82eaf531e9b0d6208396755..c18c32e9dfe2426e017fb73b0f176b833a3199b4 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 { }