PSON: http/tests/resourceLoadStatistics/classify-as-prevalent-based-on-top-frame...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Jun 2018 22:16:10 +0000 (22:16 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Jun 2018 22:16:10 +0000 (22:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186545

Reviewed by Brady Eidson.

Source/WebCore:

Move frame navigation logging for ITP purposes from the WebProcess to the UIProcess.
This information was previously logged in DocumentLoader::willSendRequest() and was getting
sync'd to the UIProcess at regular intervals or when the layout tests call testRunner's
statisticsNotifyObserver(). We now do the logging directly in the UIProcess, in
WebPageProxy::decidePolicyForNavigationAction (which was getting called via IPC from
DocumentLoader::willSendRequest()).

This is more efficient and will also be needed soon due to the way process swap on navigation
deals with cross-origin redirects. On cross-origin redirect of the main frame, PSON cancels
the load and started a new load to the redirected to URL in the new WebProcess. As a result,
the new WebProcess is not aware that the load is a redirect, which is information that ITP
requires. By moving the ITP logging to the UIProcess, we still have access to this
information.

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::willSendRequest):
Stop logging the navigation now that it is logged in the UIProcess.

* loader/EmptyClients.cpp:
(WebCore::EmptyFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
* loader/EmptyFrameLoaderClient.h:
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadURL):
(WebCore::FrameLoader::loadWithDocumentLoader):
* loader/FrameLoaderClient.h:
* loader/PolicyChecker.cpp:
(WebCore::PolicyChecker::checkNavigationPolicy):
* loader/PolicyChecker.h:
We now pass the full redirect request to the decidePolicyForNavigationAction
delegate instead of a simple isRedirect boolean, so that we have the redirect
response URL in the UIProcess for ITP logging.

* loader/ResourceLoadObserver.cpp:
(WebCore::areDomainsAssociated):
(WebCore::ResourceLoadObserver::logSubresourceLoading):
(WebCore::ResourceLoadObserver::logWebSocketLoading):
(WebCore::ResourceLoadObserver::logUserInteractionWithReducedTimeResolution):
* loader/ResourceLoadObserver.h:
* loader/ResourceLoadStatistics.cpp:
(WebCore::ResourceLoadStatistics::areDomainsAssociated):
(WebCore::ResourceLoadStatistics::reduceTimeResolution):
* loader/ResourceLoadStatistics.h:
- Remove ResourceLoadObserver::logFrameNavigation() now that it is on the WebResourceLoadStatisticsStore.
- Move some code from ResourceLoadObserver to ResourceLoadStatistics so that it can
  be called from the UIProcess and to avoid code duplication.

Source/WebKit:

Move frame navigation logging for ITP purposes from the WebProcess to the UIProcess.
This information was previously logged in DocumentLoader::willSendRequest() and was getting
sync'd to the UIProcess at regular intervals or when the layout tests call testRunner's
statisticsNotifyObserver(). We now do the logging directly in the UIProcess, in
WebPageProxy::decidePolicyForNavigationAction (which was getting called via IPC from
DocumentLoader::willSendRequest()).

This is more efficient and will also be needed soon due to the way process swap on navigation
deals with cross-origin redirects. On cross-origin redirect of the main frame, PSON cancels
the load and started a new load to the redirected to URL in the new WebProcess. As a result,
the new WebProcess is not aware that the load is a redirect, which is information that ITP
requires. By moving the ITP logging to the UIProcess, we still have access to this
information.

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::decidePolicyForNavigationAction):
(WebKit::WebPageProxy::decidePolicyForNavigationActionSync):
* UIProcess/WebPageProxy.h:
* UIProcess/WebPageProxy.messages.in:
- We now pass the full redirect response the the delegate method instead of a simple
isRedirect boolean.
- Log the navigation in the WebResourceLoadStatisticsStore for ITP purposes.

* UIProcess/WebResourceLoadStatisticsStore.cpp:
(WebKit::areDomainsAssociated):
Equivalent of ResourceLoadObserver's areDomainsAssociated(). Most of the logic was moved
to ResourceLoadStatistics::areDomainsAssociated() to avoid code duplication.

(WebKit::WebResourceLoadStatisticsStore::resourceLoadStatisticsUpdated):
This is called whenever a WebProcess sends new resource load statistics to the UIProcess.
Whenever this happens, we call processStatisticsAndDataRecords() right away, which is
sometimes the tests currently rely on. As a result, we can cancels any pending statistics
processing request that was scheduled by logFrameNavigation().

(WebKit::WebResourceLoadStatisticsStore::scheduleStatisticsProcessingRequestIfNecessary):
(WebKit::WebResourceLoadStatisticsStore::cancelPendingStatisticsProcessingRequest):
Whenever a navigation is logged and statistics have been updated, we need to make sure we
schedule a "timer" to process the new data. We do this at most every 5 seconds for performance
reasons. This 5 second interval matches what the ResourceLoadObserver is using in the WebProcess
to notify the UIProcess of new data.

(WebKit::WebResourceLoadStatisticsStore::logFrameNavigation):
This code was moved from ResourceLoadObserver to WebResourceLoadStatisticsStore now that we
do this logging in the UIProcess instead of the WebProcess. One difference with WebCore is
that we use WebPageProxy::pageLoadState().url() as mainFrameURL instead of
WebPageProxy::mainFrame().url(). The reason for that is that WebPageProxy::mainFrame().url()
becomes empty in case of process swap but ITP still needs the actual main frame URL when the
navigation was triggered. WebPageProxy::pageLoadState().url() gives us this information.

* UIProcess/WebResourceLoadStatisticsStore.h:

* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
* WebProcess/WebCoreSupport/WebFrameLoaderClient.h:
We now pass the full redirect response the the delegate method instead of a simple
isRedirect boolean.

Source/WebKitLegacy/mac:

Update client delegate now that parameter type has changed.

* WebCoreSupport/WebFrameLoaderClient.h:
* WebCoreSupport/WebFrameLoaderClient.mm:
(WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):

Source/WebKitLegacy/win:

Update client delegate now that parameter type has changed.

* WebCoreSupport/WebFrameLoaderClient.cpp:
(WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
* WebCoreSupport/WebFrameLoaderClient.h:

LayoutTests:

Attempt to mark the test as non-flaky now that it no longer relies on sync'ing from the WebProcess
to the UIProcess.

* platform/wk2/TestExpectations:

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

29 files changed:
LayoutTests/ChangeLog
LayoutTests/platform/wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/loader/DocumentLoader.cpp
Source/WebCore/loader/EmptyClients.cpp
Source/WebCore/loader/EmptyFrameLoaderClient.h
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/FrameLoaderClient.h
Source/WebCore/loader/PolicyChecker.cpp
Source/WebCore/loader/PolicyChecker.h
Source/WebCore/loader/ResourceLoadObserver.cpp
Source/WebCore/loader/ResourceLoadObserver.h
Source/WebCore/loader/ResourceLoadStatistics.cpp
Source/WebCore/loader/ResourceLoadStatistics.h
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/WebPageProxy.messages.in
Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp
Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h
Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h
Source/WebKit/WebProcess/WebProcess.cpp
Source/WebKitLegacy/mac/ChangeLog
Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.h
Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm
Source/WebKitLegacy/win/ChangeLog
Source/WebKitLegacy/win/WebCoreSupport/WebFrameLoaderClient.cpp
Source/WebKitLegacy/win/WebCoreSupport/WebFrameLoaderClient.h

index 7cd11f4..91948a5 100644 (file)
@@ -1,3 +1,15 @@
+2018-06-13  Chris Dumez  <cdumez@apple.com>
+
+        PSON: http/tests/resourceLoadStatistics/classify-as-prevalent-based-on-top-frame-redirect-collusion.html ASSERTS with process swap enabled
+        https://bugs.webkit.org/show_bug.cgi?id=186545
+
+        Reviewed by Brady Eidson.
+
+        Attempt to mark the test as non-flaky now that it no longer relies on sync'ing from the WebProcess
+        to the UIProcess.
+
+        * platform/wk2/TestExpectations:
+
 2018-06-13  David Fenton  <david_fenton@apple.com>
 
         [macOS Debug WK1] LayoutTest fast/parser/xml-error-adopted.xml is a flaky timeout.
index 895b2c1..48a7876 100644 (file)
@@ -705,7 +705,7 @@ http/tests/resourceLoadStatistics/classify-as-prevalent-based-on-sub-frame-under
 http/tests/resourceLoadStatistics/classify-as-prevalent-based-on-subresource-under-top-frame-origins.html [ Pass ]
 http/tests/resourceLoadStatistics/classify-as-prevalent-based-on-subresource-unique-redirects-to.html [ Pass ]
 http/tests/resourceLoadStatistics/classify-as-prevalent-based-on-subresource-redirect-collusion.html [ Pass ]
-http/tests/resourceLoadStatistics/classify-as-prevalent-based-on-top-frame-redirect-collusion.html [ Pass Failure ]
+http/tests/resourceLoadStatistics/classify-as-prevalent-based-on-top-frame-redirect-collusion.html [ Pass ]
 http/tests/resourceLoadStatistics/classify-as-prevalent-based-on-top-frame-unique-redirects-to.html [ Pass ]
 http/tests/resourceLoadStatistics/clear-in-memory-and-persistent-store.html [ Pass ]
 http/tests/resourceLoadStatistics/grandfathering.html [ Pass ]
index 0c4c1a2..99e2840 100644 (file)
@@ -1,3 +1,56 @@
+2018-06-13  Chris Dumez  <cdumez@apple.com>
+
+        PSON: http/tests/resourceLoadStatistics/classify-as-prevalent-based-on-top-frame-redirect-collusion.html ASSERTS with process swap enabled
+        https://bugs.webkit.org/show_bug.cgi?id=186545
+
+        Reviewed by Brady Eidson.
+
+        Move frame navigation logging for ITP purposes from the WebProcess to the UIProcess.
+        This information was previously logged in DocumentLoader::willSendRequest() and was getting
+        sync'd to the UIProcess at regular intervals or when the layout tests call testRunner's
+        statisticsNotifyObserver(). We now do the logging directly in the UIProcess, in
+        WebPageProxy::decidePolicyForNavigationAction (which was getting called via IPC from
+        DocumentLoader::willSendRequest()).
+
+        This is more efficient and will also be needed soon due to the way process swap on navigation
+        deals with cross-origin redirects. On cross-origin redirect of the main frame, PSON cancels
+        the load and started a new load to the redirected to URL in the new WebProcess. As a result,
+        the new WebProcess is not aware that the load is a redirect, which is information that ITP
+        requires. By moving the ITP logging to the UIProcess, we still have access to this
+        information. 
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::willSendRequest):
+        Stop logging the navigation now that it is logged in the UIProcess.
+
+        * loader/EmptyClients.cpp:
+        (WebCore::EmptyFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
+        * loader/EmptyFrameLoaderClient.h:
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::loadURL):
+        (WebCore::FrameLoader::loadWithDocumentLoader):
+        * loader/FrameLoaderClient.h:
+        * loader/PolicyChecker.cpp:
+        (WebCore::PolicyChecker::checkNavigationPolicy):
+        * loader/PolicyChecker.h:
+        We now pass the full redirect request to the decidePolicyForNavigationAction
+        delegate instead of a simple isRedirect boolean, so that we have the redirect
+        response URL in the UIProcess for ITP logging.
+
+        * loader/ResourceLoadObserver.cpp:
+        (WebCore::areDomainsAssociated):
+        (WebCore::ResourceLoadObserver::logSubresourceLoading):
+        (WebCore::ResourceLoadObserver::logWebSocketLoading):
+        (WebCore::ResourceLoadObserver::logUserInteractionWithReducedTimeResolution):
+        * loader/ResourceLoadObserver.h:
+        * loader/ResourceLoadStatistics.cpp:
+        (WebCore::ResourceLoadStatistics::areDomainsAssociated):
+        (WebCore::ResourceLoadStatistics::reduceTimeResolution):
+        * loader/ResourceLoadStatistics.h:
+        - Remove ResourceLoadObserver::logFrameNavigation() now that it is on the WebResourceLoadStatisticsStore.
+        - Move some code from ResourceLoadObserver to ResourceLoadStatistics so that it can
+          be called from the UIProcess and to avoid code duplication.
+
 2018-06-13  Mark Lam  <mark.lam@apple.com>
 
         FloatingPointEnvironment is only needed for ARM CPUs.
index fb07ee5..47a369e 100644 (file)
@@ -591,8 +591,6 @@ void DocumentLoader::willSendRequest(ResourceRequest&& newRequest, const Resourc
 
     ASSERT(m_frame->document());
     ASSERT(topFrame.document());
-
-    ResourceLoadObserver::shared().logFrameNavigation(*m_frame, topFrame, newRequest, redirectResponse.url());
     
     // Update cookie policy base URL as URL changes, except for subframes, which use the
     // URL of the main frame which doesn't change when we redirect.
@@ -657,7 +655,7 @@ void DocumentLoader::willSendRequest(ResourceRequest&& newRequest, const Resourc
         return;
     }
 
-    frameLoader()->policyChecker().checkNavigationPolicy(WTFMove(newRequest), didReceiveRedirectResponse, WTFMove(navigationPolicyCompletionHandler));
+    frameLoader()->policyChecker().checkNavigationPolicy(WTFMove(newRequest), redirectResponse, WTFMove(navigationPolicyCompletionHandler));
 }
 
 bool DocumentLoader::tryLoadingRequestFromApplicationCache()
index 6053b87..c33bc03 100644 (file)
@@ -446,7 +446,7 @@ void EmptyFrameLoaderClient::dispatchDecidePolicyForNewWindowAction(const Naviga
 {
 }
 
-void EmptyFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, bool, FormState*, PolicyDecisionMode, FramePolicyFunction&&)
+void EmptyFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, const ResourceResponse&, FormState*, PolicyDecisionMode, FramePolicyFunction&&)
 {
 }
 
index 34a508a..dcfaf90 100644 (file)
@@ -95,7 +95,7 @@ class WEBCORE_EXPORT EmptyFrameLoaderClient : public FrameLoaderClient {
 
     void dispatchDecidePolicyForResponse(const ResourceResponse&, const ResourceRequest&, FramePolicyFunction&&) final { }
     void dispatchDecidePolicyForNewWindowAction(const NavigationAction&, const ResourceRequest&, FormState*, const String&, FramePolicyFunction&&) final;
-    void dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, bool didReceiveRedirectResponse, FormState*, PolicyDecisionMode, FramePolicyFunction&&) final;
+    void dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, const ResourceResponse& redirectResponse, FormState*, PolicyDecisionMode, FramePolicyFunction&&) final;
     void cancelPolicyCheck() final { }
 
     void dispatchUnableToImplementPolicy(const ResourceError&) final { }
index 04d436e..9d726ab 100644 (file)
@@ -1380,7 +1380,7 @@ void FrameLoader::loadURL(FrameLoadRequest&& frameLoadRequest, const String& ref
         oldDocumentLoader->setLastCheckedRequest(ResourceRequest());
         policyChecker().stopCheck();
         policyChecker().setLoadType(newLoadType);
-        policyChecker().checkNavigationPolicy(WTFMove(request), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), WTFMove(formState), [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, WeakPtr<FormState>&&, ShouldContinue shouldContinue) {
+        policyChecker().checkNavigationPolicy(WTFMove(request), ResourceResponse { } /* redirectResponse */, oldDocumentLoader.get(), WTFMove(formState), [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, WeakPtr<FormState>&&, ShouldContinue shouldContinue) {
             continueFragmentScrollAfterNavigationPolicy(request, shouldContinue == ShouldContinue::Yes);
         }, PolicyDecisionMode::Synchronous);
         return;
@@ -1555,7 +1555,7 @@ void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType t
         oldDocumentLoader->setTriggeringAction(action);
         oldDocumentLoader->setLastCheckedRequest(ResourceRequest());
         policyChecker().stopCheck();
-        policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), WTFMove(formState), [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, WeakPtr<FormState>&&, ShouldContinue shouldContinue) {
+        policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), ResourceResponse { }  /* redirectResponse */, oldDocumentLoader.get(), WTFMove(formState), [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, WeakPtr<FormState>&&, ShouldContinue shouldContinue) {
             continueFragmentScrollAfterNavigationPolicy(request, shouldContinue == ShouldContinue::Yes);
         }, PolicyDecisionMode::Synchronous);
         return;
@@ -1589,7 +1589,7 @@ void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType t
         return;
     }
 
-    policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, loader, WTFMove(formState), [this, protectedFrame = makeRef(m_frame), allowNavigationToInvalidURL, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, WeakPtr<FormState>&& formState, ShouldContinue shouldContinue) {
+    policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), ResourceResponse { } /* redirectResponse */, loader, WTFMove(formState), [this, protectedFrame = makeRef(m_frame), allowNavigationToInvalidURL, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, WeakPtr<FormState>&& formState, ShouldContinue shouldContinue) {
         continueLoadAfterNavigationPolicy(request, formState.get(), shouldContinue, allowNavigationToInvalidURL);
         completionHandler();
     });
index c6f67de..f722618 100644 (file)
@@ -191,7 +191,7 @@ public:
 
     virtual void dispatchDecidePolicyForResponse(const ResourceResponse&, const ResourceRequest&, FramePolicyFunction&&) = 0;
     virtual void dispatchDecidePolicyForNewWindowAction(const NavigationAction&, const ResourceRequest&, FormState*, const String& frameName, FramePolicyFunction&&) = 0;
-    virtual void dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, bool didReceiveRedirectResponse, FormState*, PolicyDecisionMode, FramePolicyFunction&&) = 0;
+    virtual void dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, const ResourceResponse& redirectResponse, FormState*, PolicyDecisionMode, FramePolicyFunction&&) = 0;
     virtual void cancelPolicyCheck() = 0;
 
     virtual void dispatchUnableToImplementPolicy(const ResourceError&) = 0;
index 5f9751c..4e0eedc 100644 (file)
@@ -79,9 +79,9 @@ PolicyChecker::PolicyChecker(Frame& frame)
 {
 }
 
-void PolicyChecker::checkNavigationPolicy(ResourceRequest&& newRequest, bool didReceiveRedirectResponse, NavigationPolicyDecisionFunction&& function)
+void PolicyChecker::checkNavigationPolicy(ResourceRequest&& newRequest, const ResourceResponse& redirectResponse, NavigationPolicyDecisionFunction&& function)
 {
-    checkNavigationPolicy(WTFMove(newRequest), didReceiveRedirectResponse, m_frame.loader().activeDocumentLoader(), { }, WTFMove(function));
+    checkNavigationPolicy(WTFMove(newRequest), redirectResponse, m_frame.loader().activeDocumentLoader(), { }, WTFMove(function));
 }
 
 CompletionHandlerCallingScope PolicyChecker::extendBlobURLLifetimeIfNecessary(ResourceRequest& request) const
@@ -98,7 +98,7 @@ CompletionHandlerCallingScope PolicyChecker::extendBlobURLLifetimeIfNecessary(Re
     });
 }
 
-void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didReceiveRedirectResponse, DocumentLoader* loader, RefPtr<FormState>&& formState, NavigationPolicyDecisionFunction&& function, PolicyDecisionMode policyDecisionMode)
+void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, const ResourceResponse& redirectResponse, DocumentLoader* loader, RefPtr<FormState>&& formState, NavigationPolicyDecisionFunction&& function, PolicyDecisionMode policyDecisionMode)
 {
     NavigationAction action = loader->triggeringAction();
     if (action.isEmpty()) {
@@ -128,7 +128,7 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec
         return;
     }
 
-    if (!isAllowedByContentSecurityPolicy(request.url(), m_frame.ownerElement(), didReceiveRedirectResponse)) {
+    if (!isAllowedByContentSecurityPolicy(request.url(), m_frame.ownerElement(), !redirectResponse.isNull())) {
         if (m_frame.ownerElement()) {
             // Fire a load event (even though we were blocked by CSP) as timing attacks would otherwise
             // reveal that the frame was blocked. This way, it looks like any other cross-origin page load.
@@ -168,7 +168,7 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec
 
     m_delegateIsDecidingNavigationPolicy = true;
     String suggestedFilename = action.downloadAttribute().isEmpty() ? nullAtom() : action.downloadAttribute();
-    m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, didReceiveRedirectResponse, formState.get(), policyDecisionMode, [this, function = WTFMove(function), request = ResourceRequest(request), formState = WTFMove(formState), suggestedFilename = WTFMove(suggestedFilename), blobURLLifetimeExtension = WTFMove(blobURLLifetimeExtension)](PolicyAction policyAction) mutable {
+    m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, redirectResponse, formState.get(), policyDecisionMode, [this, function = WTFMove(function), request = ResourceRequest(request), formState = WTFMove(formState), suggestedFilename = WTFMove(suggestedFilename), blobURLLifetimeExtension = WTFMove(blobURLLifetimeExtension)](PolicyAction policyAction) mutable {
         m_delegateIsDecidingNavigationPolicy = false;
 
         switch (policyAction) {
index 9f04897..2071ffa 100644 (file)
@@ -69,8 +69,8 @@ class PolicyChecker {
 public:
     explicit PolicyChecker(Frame&);
 
-    void checkNavigationPolicy(ResourceRequest&&, bool didReceiveRedirectResponse, DocumentLoader*, RefPtr<FormState>&&, NavigationPolicyDecisionFunction&&, PolicyDecisionMode = PolicyDecisionMode::Asynchronous);
-    void checkNavigationPolicy(ResourceRequest&&, bool didReceiveRedirectResponse, NavigationPolicyDecisionFunction&&);
+    void checkNavigationPolicy(ResourceRequest&&, const ResourceResponse& redirectResponse, DocumentLoader*, RefPtr<FormState>&&, NavigationPolicyDecisionFunction&&, PolicyDecisionMode = PolicyDecisionMode::Asynchronous);
+    void checkNavigationPolicy(ResourceRequest&&, const ResourceResponse& redirectResponse, NavigationPolicyDecisionFunction&&);
     void checkNewWindowPolicy(NavigationAction&&, ResourceRequest&&, RefPtr<FormState>&&, const String& frameName, NewWindowPolicyDecisionFunction&&);
 
     void stopCheck();
index f5f5ef8..8b9836e 100644 (file)
@@ -47,7 +47,6 @@ template<typename T> static inline String primaryDomain(const T& value)
     return ResourceLoadStatistics::primaryDomain(value);
 }
 
-static Seconds timestampResolution { 5_s };
 static const Seconds minimumNotificationInterval { 5_s };
 
 ResourceLoadObserver& ResourceLoadObserver::shared()
@@ -69,36 +68,9 @@ static bool shouldEnableSiteSpecificQuirks(Page* page)
 #endif
 }
 
-// FIXME: Temporary fix for <rdar://problem/32343256> until content can be updated.
 static bool areDomainsAssociated(Page* page, const String& firstDomain, const String& secondDomain)
 {
-    static NeverDestroyed<HashMap<String, unsigned>> metaDomainIdentifiers = [] {
-        HashMap<String, unsigned> map;
-
-        // Domains owned by Dow Jones & Company, Inc.
-        const unsigned dowJonesIdentifier = 1;
-        map.add(ASCIILiteral("dowjones.com"), dowJonesIdentifier);
-        map.add(ASCIILiteral("wsj.com"), dowJonesIdentifier);
-        map.add(ASCIILiteral("barrons.com"), dowJonesIdentifier);
-        map.add(ASCIILiteral("marketwatch.com"), dowJonesIdentifier);
-        map.add(ASCIILiteral("wsjplus.com"), dowJonesIdentifier);
-
-        return map;
-    }();
-
-    if (firstDomain == secondDomain)
-        return true;
-
-    ASSERT(!equalIgnoringASCIICase(firstDomain, secondDomain));
-
-    if (!shouldEnableSiteSpecificQuirks(page))
-        return false;
-
-    unsigned firstMetaDomainIdentifier = metaDomainIdentifiers.get().get(firstDomain);
-    if (!firstMetaDomainIdentifier)
-        return false;
-
-    return firstMetaDomainIdentifier == metaDomainIdentifiers.get().get(secondDomain);
+    return ResourceLoadStatistics::areDomainsAssociated(shouldEnableSiteSpecificQuirks(page), firstDomain, secondDomain);
 }
 
 void ResourceLoadObserver::setNotificationCallback(WTF::Function<void (Vector<ResourceLoadStatistics>&&)>&& notificationCallback)
@@ -132,75 +104,6 @@ bool ResourceLoadObserver::shouldLog(Page* page) const
     return DeprecatedGlobalSettings::resourceLoadStatisticsEnabled() && !page->usesEphemeralSession() && m_notificationCallback;
 }
 
-static WallTime reduceTimeResolution(WallTime time)
-{
-    return WallTime::fromRawSeconds(std::floor(time.secondsSinceEpoch() / timestampResolution) * timestampResolution.seconds());
-}
-
-void ResourceLoadObserver::logFrameNavigation(const Frame& frame, const Frame& topFrame, const ResourceRequest& newRequest, const URL& redirectUrl)
-{
-    ASSERT(frame.document());
-    ASSERT(topFrame.document());
-    ASSERT(topFrame.page());
-
-    auto* page = topFrame.page();
-    if (!shouldLog(page))
-        return;
-
-    auto sourceURL = redirectUrl;
-    bool isRedirect = !redirectUrl.isNull();
-    if (!isRedirect)
-        sourceURL = nonNullOwnerURL(*frame.document());
-
-    auto& targetURL = newRequest.url();
-    auto& mainFrameURL = topFrame.document()->url();
-    
-    if (!targetURL.isValid() || !mainFrameURL.isValid())
-        return;
-
-    auto targetHost = targetURL.host();
-    auto mainFrameHost = mainFrameURL.host();
-
-    if (targetHost.isEmpty() || mainFrameHost.isEmpty() || targetHost == sourceURL.host())
-        return;
-
-    auto targetPrimaryDomain = primaryDomain(targetURL);
-    auto mainFramePrimaryDomain = primaryDomain(mainFrameURL);
-    auto sourcePrimaryDomain = primaryDomain(sourceURL);
-    bool shouldCallNotificationCallback = false;
-
-    if (targetHost != mainFrameHost
-        && !(areDomainsAssociated(page, targetPrimaryDomain, mainFramePrimaryDomain) || areDomainsAssociated(page, targetPrimaryDomain, sourcePrimaryDomain))) {
-        auto& targetStatistics = ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
-        targetStatistics.lastSeen = reduceTimeResolution(WallTime::now());
-        if (targetStatistics.subframeUnderTopFrameOrigins.add(mainFramePrimaryDomain).isNewEntry)
-            shouldCallNotificationCallback = true;
-    }
-
-    if (isRedirect
-        && !areDomainsAssociated(page, sourcePrimaryDomain, targetPrimaryDomain)) {
-        bool isNewRedirectToEntry = false;
-        bool isNewRedirectFromEntry = false;
-        if (frame.isMainFrame()) {
-            auto& redirectingOriginStatistics = ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
-            isNewRedirectToEntry = redirectingOriginStatistics.topFrameUniqueRedirectsTo.add(targetPrimaryDomain).isNewEntry;
-            auto& targetStatistics = ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
-            isNewRedirectFromEntry = targetStatistics.topFrameUniqueRedirectsFrom.add(sourcePrimaryDomain).isNewEntry;
-        } else {
-            auto& redirectingOriginStatistics = ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
-            isNewRedirectToEntry = redirectingOriginStatistics.subresourceUniqueRedirectsTo.add(targetPrimaryDomain).isNewEntry;
-            auto& targetStatistics = ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
-            isNewRedirectFromEntry = targetStatistics.subresourceUniqueRedirectsFrom.add(sourcePrimaryDomain).isNewEntry;
-        }
-
-        if (isNewRedirectToEntry || isNewRedirectFromEntry)
-            shouldCallNotificationCallback = true;
-    }
-
-    if (shouldCallNotificationCallback)
-        scheduleNotificationIfNeeded();
-}
-
 // FIXME: This quirk was added to address <rdar://problem/33325881> and should be removed once content is fixed.
 static bool resourceNeedsSSOQuirk(Page* page, const URL& url)
 {
@@ -242,7 +145,7 @@ void ResourceLoadObserver::logSubresourceLoading(const Frame* frame, const Resou
     bool shouldCallNotificationCallback = false;
     {
         auto& targetStatistics = ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
-        targetStatistics.lastSeen = reduceTimeResolution(WallTime::now());
+        targetStatistics.lastSeen = ResourceLoadStatistics::reduceTimeResolution(WallTime::now());
         if (targetStatistics.subresourceUnderTopFrameOrigins.add(mainFramePrimaryDomain).isNewEntry)
             shouldCallNotificationCallback = true;
     }
@@ -287,7 +190,7 @@ void ResourceLoadObserver::logWebSocketLoading(const Frame* frame, const URL& ta
         return;
 
     auto& targetStatistics = ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
-    targetStatistics.lastSeen = reduceTimeResolution(WallTime::now());
+    targetStatistics.lastSeen = ResourceLoadStatistics::reduceTimeResolution(WallTime::now());
     if (targetStatistics.subresourceUnderTopFrameOrigins.add(mainFramePrimaryDomain).isNewEntry)
         scheduleNotificationIfNeeded();
 }
@@ -304,7 +207,7 @@ void ResourceLoadObserver::logUserInteractionWithReducedTimeResolution(const Doc
         return;
 
     auto domain = primaryDomain(url);
-    auto newTime = reduceTimeResolution(WallTime::now());
+    auto newTime = ResourceLoadStatistics::reduceTimeResolution(WallTime::now());
     auto lastReportedUserInteraction = m_lastReportedUserInteractionMap.get(domain);
     if (newTime == lastReportedUserInteraction)
         return;
index 1880f67..52570a7 100644 (file)
@@ -53,7 +53,6 @@ class ResourceLoadObserver {
 public:
     WEBCORE_EXPORT static ResourceLoadObserver& shared();
 
-    void logFrameNavigation(const Frame&, const Frame& topFrame, const ResourceRequest& newRequest, const URL& redirectUrl);
     void logSubresourceLoading(const Frame*, const ResourceRequest& newRequest, const ResourceResponse& redirectResponse);
     void logWebSocketLoading(const Frame*, const URL&);
     void logUserInteractionWithReducedTimeResolution(const Document&);
index adb136e..40599d7 100644 (file)
 
 #include "KeyedCoding.h"
 #include "PublicSuffix.h"
+#include <wtf/MainThread.h>
 #include <wtf/text/StringBuilder.h>
 #include <wtf/text/StringHash.h>
 
 namespace WebCore {
 
+static Seconds timestampResolution { 5_s };
+
 typedef WTF::HashMap<String, unsigned, StringHash, HashTraits<String>, HashTraits<unsigned>>::KeyValuePairType ResourceLoadStatisticsValue;
 
 static void encodeHashCountedSet(KeyedEncoder& encoder, const String& label, const HashCountedSet<String>& hashCountedSet)
@@ -352,4 +355,43 @@ String ResourceLoadStatistics::primaryDomain(StringView host)
     return hostString;
 }
 
+// FIXME: Temporary fix for <rdar://problem/32343256> until content can be updated.
+bool ResourceLoadStatistics::areDomainsAssociated(bool needsSiteSpecificQuirks, const String& firstDomain, const String& secondDomain)
+{
+    ASSERT(isMainThread());
+
+    static NeverDestroyed<HashMap<String, unsigned>> metaDomainIdentifiers = [] {
+        HashMap<String, unsigned> map;
+
+        // Domains owned by Dow Jones & Company, Inc.
+        const unsigned dowJonesIdentifier = 1;
+        map.add(ASCIILiteral("dowjones.com"), dowJonesIdentifier);
+        map.add(ASCIILiteral("wsj.com"), dowJonesIdentifier);
+        map.add(ASCIILiteral("barrons.com"), dowJonesIdentifier);
+        map.add(ASCIILiteral("marketwatch.com"), dowJonesIdentifier);
+        map.add(ASCIILiteral("wsjplus.com"), dowJonesIdentifier);
+
+        return map;
+    }();
+
+    if (firstDomain == secondDomain)
+        return true;
+
+    ASSERT(!equalIgnoringASCIICase(firstDomain, secondDomain));
+
+    if (!needsSiteSpecificQuirks)
+        return false;
+
+    unsigned firstMetaDomainIdentifier = metaDomainIdentifiers.get().get(firstDomain);
+    if (!firstMetaDomainIdentifier)
+        return false;
+
+    return firstMetaDomainIdentifier == metaDomainIdentifiers.get().get(secondDomain);
+}
+
+WallTime ResourceLoadStatistics::reduceTimeResolution(WallTime time)
+{
+    return WallTime::fromRawSeconds(std::floor(time.secondsSinceEpoch() / timestampResolution) * timestampResolution.seconds());
+}
+
 }
index 500d2d8..f047c77 100644 (file)
@@ -53,6 +53,9 @@ struct ResourceLoadStatistics {
     WEBCORE_EXPORT static String primaryDomain(const URL&);
     WEBCORE_EXPORT static String primaryDomain(StringView host);
 
+    WEBCORE_EXPORT static bool areDomainsAssociated(bool needsSiteSpecificQuirks, const String&, const String&);
+    WEBCORE_EXPORT static WallTime reduceTimeResolution(WallTime);
+
     WEBCORE_EXPORT void encode(KeyedEncoder&) const;
     WEBCORE_EXPORT bool decode(KeyedDecoder&, unsigned modelVersion);
 
index 2478621..0cc0322 100644 (file)
@@ -1,3 +1,67 @@
+2018-06-13  Chris Dumez  <cdumez@apple.com>
+
+        PSON: http/tests/resourceLoadStatistics/classify-as-prevalent-based-on-top-frame-redirect-collusion.html ASSERTS with process swap enabled
+        https://bugs.webkit.org/show_bug.cgi?id=186545
+
+        Reviewed by Brady Eidson.
+
+        Move frame navigation logging for ITP purposes from the WebProcess to the UIProcess.
+        This information was previously logged in DocumentLoader::willSendRequest() and was getting
+        sync'd to the UIProcess at regular intervals or when the layout tests call testRunner's
+        statisticsNotifyObserver(). We now do the logging directly in the UIProcess, in
+        WebPageProxy::decidePolicyForNavigationAction (which was getting called via IPC from
+        DocumentLoader::willSendRequest()).
+
+        This is more efficient and will also be needed soon due to the way process swap on navigation 
+        deals with cross-origin redirects. On cross-origin redirect of the main frame, PSON cancels
+        the load and started a new load to the redirected to URL in the new WebProcess. As a result,
+        the new WebProcess is not aware that the load is a redirect, which is information that ITP
+        requires. By moving the ITP logging to the UIProcess, we still have access to this
+        information.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::decidePolicyForNavigationAction):
+        (WebKit::WebPageProxy::decidePolicyForNavigationActionSync):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/WebPageProxy.messages.in:
+        - We now pass the full redirect response the the delegate method instead of a simple
+        isRedirect boolean.
+        - Log the navigation in the WebResourceLoadStatisticsStore for ITP purposes.
+
+        * UIProcess/WebResourceLoadStatisticsStore.cpp:
+        (WebKit::areDomainsAssociated):
+        Equivalent of ResourceLoadObserver's areDomainsAssociated(). Most of the logic was moved
+        to ResourceLoadStatistics::areDomainsAssociated() to avoid code duplication.
+
+        (WebKit::WebResourceLoadStatisticsStore::resourceLoadStatisticsUpdated):
+        This is called whenever a WebProcess sends new resource load statistics to the UIProcess.
+        Whenever this happens, we call processStatisticsAndDataRecords() right away, which is
+        sometimes the tests currently rely on. As a result, we can cancels any pending statistics
+        processing request that was scheduled by logFrameNavigation().
+
+        (WebKit::WebResourceLoadStatisticsStore::scheduleStatisticsProcessingRequestIfNecessary):
+        (WebKit::WebResourceLoadStatisticsStore::cancelPendingStatisticsProcessingRequest):
+        Whenever a navigation is logged and statistics have been updated, we need to make sure we
+        schedule a "timer" to process the new data. We do this at most every 5 seconds for performance
+        reasons. This 5 second interval matches what the ResourceLoadObserver is using in the WebProcess
+        to notify the UIProcess of new data.
+
+        (WebKit::WebResourceLoadStatisticsStore::logFrameNavigation):
+        This code was moved from ResourceLoadObserver to WebResourceLoadStatisticsStore now that we
+        do this logging in the UIProcess instead of the WebProcess. One difference with WebCore is
+        that we use WebPageProxy::pageLoadState().url() as mainFrameURL instead of
+        WebPageProxy::mainFrame().url(). The reason for that is that WebPageProxy::mainFrame().url()
+        becomes empty in case of process swap but ITP still needs the actual main frame URL when the
+        navigation was triggered. WebPageProxy::pageLoadState().url() gives us this information.
+
+        * UIProcess/WebResourceLoadStatisticsStore.h:
+
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.h:
+        We now pass the full redirect response the the delegate method instead of a simple
+        isRedirect boolean.
+
 2018-06-13  Brent Fulgham  <bfulgham@apple.com>
 
         Crash during interrupted process termination
index 8c1d35f..49e6d6c 100644 (file)
 #include "WebProcessPool.h"
 #include "WebProcessProxy.h"
 #include "WebProtectionSpace.h"
+#include "WebResourceLoadStatisticsStore.h"
 #include "WebURLSchemeHandler.h"
 #include "WebUserContentControllerProxy.h"
 #include "WebsiteDataStore.h"
@@ -3981,7 +3982,7 @@ void WebPageProxy::frameDidBecomeFrameSet(uint64_t frameID, bool value)
         m_frameSetLargestFrame = value ? m_mainFrame : 0;
 }
 
-void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& originatingFrameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, ResourceRequest&& request, uint64_t listenerID, const UserData& userData)
+void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& originatingFrameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, ResourceRequest&& request, ResourceResponse&& redirectResponse, uint64_t listenerID, const UserData& userData)
 {
     LOG(Loading, "WebPageProxy::decidePolicyForNavigationAction - Original URL %s, current target URL %s", originalRequest.url().string().utf8().data(), request.url().string().utf8().data());
 
@@ -4040,6 +4041,9 @@ void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const Secur
 
     WebFrameProxy* originatingFrame = m_process->webFrame(originatingFrameInfoData.frameID);
 
+    if (auto* resourceLoadStatisticsStore = websiteDataStore().resourceLoadStatistics())
+        resourceLoadStatisticsStore->logFrameNavigation(*frame, URL(URL(), m_pageLoadState.url()), request, redirectResponse.url());
+
     if (m_navigationClient) {
         auto destinationFrameInfo = API::FrameInfo::create(*frame, frameSecurityOrigin.securityOrigin());
         RefPtr<API::FrameInfo> sourceFrameInfo;
@@ -4060,12 +4064,12 @@ void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const Secur
     m_shouldSuppressAppLinksInNextNavigationPolicyDecision = false;
 }
 
-void WebPageProxy::decidePolicyForNavigationActionSync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& originatingFrameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, uint64_t listenerID, const UserData& userData, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&& reply)
+void WebPageProxy::decidePolicyForNavigationActionSync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& originatingFrameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, ResourceResponse&& redirectResponse, uint64_t listenerID, const UserData& userData, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&& reply)
 {
     ASSERT(!m_syncNavigationActionPolicyReply);
     m_syncNavigationActionPolicyReply = WTFMove(reply);
 
-    decidePolicyForNavigationAction(frameID, frameSecurityOrigin, navigationID, WTFMove(navigationActionData), originatingFrameInfoData, originatingPageID, originalRequest, WTFMove(request), listenerID, userData);
+    decidePolicyForNavigationAction(frameID, frameSecurityOrigin, navigationID, WTFMove(navigationActionData), originatingFrameInfoData, originatingPageID, originalRequest, WTFMove(request), WTFMove(redirectResponse), listenerID, userData);
 
     // If the client did not respond synchronously, proceed with the load.
     if (auto syncNavigationActionPolicyReply = WTFMove(m_syncNavigationActionPolicyReply))
index 039250f..b1050c9 100644 (file)
@@ -1399,8 +1399,8 @@ private:
 
     void didDestroyNavigation(uint64_t navigationID);
 
-    void decidePolicyForNavigationAction(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, uint64_t listenerID, const UserData&);
-    void decidePolicyForNavigationActionSync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, uint64_t listenerID, const UserData&, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&&);
+    void decidePolicyForNavigationAction(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, WebCore::ResourceResponse&& redirectResponse, uint64_t listenerID, const UserData&);
+    void decidePolicyForNavigationActionSync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, WebCore::ResourceResponse&& redirectResponse, uint64_t listenerID, const UserData&, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&&);
     void decidePolicyForNewWindowAction(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, NavigationActionData&&, WebCore::ResourceRequest&&, const String& frameName, uint64_t listenerID, const UserData&);
     void decidePolicyForResponse(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, uint64_t listenerID, const UserData&);
     void unableToImplementPolicy(uint64_t frameID, const WebCore::ResourceError&, const UserData&);
index 10500ee..7f3c668 100644 (file)
@@ -99,8 +99,8 @@ messages -> WebPageProxy {
 
     # Policy messages
     DecidePolicyForResponse(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, WebCore::ResourceResponse response, WebCore::ResourceRequest request, bool canShowMIMEType, uint64_t listenerID, WebKit::UserData userData)
-    DecidePolicyForNavigationAction(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, uint64_t originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, uint64_t listenerID, WebKit::UserData userData)
-    DecidePolicyForNavigationActionSync(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, uint64_t originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, uint64_t listenerID, WebKit::UserData userData) -> (uint64_t newNavigationID, enum WebCore::PolicyAction policyAction, WebKit::DownloadID downloadID, std::optional<WebKit::WebsitePoliciesData> websitePolicies) Delayed
+    DecidePolicyForNavigationAction(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, uint64_t originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, WebCore::ResourceResponse redirectResponse, uint64_t listenerID, WebKit::UserData userData)
+    DecidePolicyForNavigationActionSync(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, uint64_t originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, WebCore::ResourceResponse redirectResponse, uint64_t listenerID, WebKit::UserData userData) -> (uint64_t newNavigationID, enum WebCore::PolicyAction policyAction, WebKit::DownloadID downloadID, std::optional<WebKit::WebsitePoliciesData> websitePolicies) Delayed
     DecidePolicyForNewWindowAction(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, struct WebKit::NavigationActionData navigationActionData, WebCore::ResourceRequest request, String frameName, uint64_t listenerID, WebKit::UserData userData)
     UnableToImplementPolicy(uint64_t frameID, WebCore::ResourceError error, WebKit::UserData userData)
 
index 5382a55..06ef9ff 100644 (file)
@@ -29,6 +29,8 @@
 #include "Logging.h"
 #include "PluginProcessManager.h"
 #include "PluginProcessProxy.h"
+#include "WebFrameProxy.h"
+#include "WebPageProxy.h"
 #include "WebProcessMessages.h"
 #include "WebProcessProxy.h"
 #include "WebResourceLoadStatisticsStoreMessages.h"
@@ -52,12 +54,19 @@ constexpr unsigned operatingDatesWindow { 30 };
 constexpr unsigned statisticsModelVersion { 12 };
 constexpr unsigned maxImportance { 3 };
 constexpr unsigned maxNumberOfRecursiveCallsInRedirectTraceBack { 50 };
+constexpr Seconds minimumStatisticsProcessingInterval { 5_s };
 
 template<typename T> static inline String isolatedPrimaryDomain(const T& value)
 {
     return ResourceLoadStatistics::primaryDomain(value).isolatedCopy();
 }
 
+static bool areDomainsAssociated(WebPageProxy* page, const String& firstDomain, const String& secondDomain)
+{
+    bool needsSiteSpecificQuirks = page && page->preferences().needsSiteSpecificQuirks();
+    return ResourceLoadStatistics::areDomainsAssociated(needsSiteSpecificQuirks, firstDomain, secondDomain);
+}
+
 const OptionSet<WebsiteDataType>& WebResourceLoadStatisticsStore::monitoredDataTypes()
 {
     static NeverDestroyed<OptionSet<WebsiteDataType>> dataTypes(std::initializer_list<WebsiteDataType>({
@@ -328,6 +337,10 @@ void WebResourceLoadStatisticsStore::resourceLoadStatisticsUpdated(Vector<WebCor
     ASSERT(!RunLoop::isMain());
 
     mergeStatistics(WTFMove(origins));
+
+    // We can cancel any pending request to process statistics since we're doing it synchronously below.
+    cancelPendingStatisticsProcessingRequest();
+
     // Fire before processing statistics to propagate user interaction as fast as possible to the network process.
     updateCookiePartitioning([]() { });
     processStatisticsAndDataRecords();
@@ -546,6 +559,92 @@ void WebResourceLoadStatisticsStore::setResourceLoadStatisticsDebugMode(bool ena
 #endif
 }
 
+void WebResourceLoadStatisticsStore::scheduleStatisticsProcessingRequestIfNecessary()
+{
+    ASSERT(!RunLoop::isMain());
+
+    m_pendingStatisticsProcessingRequestIdentifier = ++m_lastStatisticsProcessingRequestIdentifier;
+    m_statisticsQueue->dispatchAfter(minimumStatisticsProcessingInterval, [this, protectedThis = makeRef(*this), statisticsProcessingRequestIdentifier = *m_pendingStatisticsProcessingRequestIdentifier] {
+        if (!m_pendingStatisticsProcessingRequestIdentifier || *m_pendingStatisticsProcessingRequestIdentifier != statisticsProcessingRequestIdentifier) {
+            // This request has been canceled.
+            return;
+        }
+
+        updateCookiePartitioning([]() { });
+        processStatisticsAndDataRecords();
+    });
+}
+
+void WebResourceLoadStatisticsStore::cancelPendingStatisticsProcessingRequest()
+{
+    ASSERT(!RunLoop::isMain());
+
+    m_pendingStatisticsProcessingRequestIdentifier = std::nullopt;
+}
+
+void WebResourceLoadStatisticsStore::logFrameNavigation(const WebFrameProxy& frame, const URL& pageURL, const WebCore::ResourceRequest& request, const WebCore::URL& redirectURL)
+{
+    ASSERT(RunLoop::isMain());
+
+    auto sourceURL = redirectURL;
+    bool isRedirect = !redirectURL.isNull();
+    if (!isRedirect) {
+        sourceURL = frame.url();
+        if (sourceURL.isNull())
+            sourceURL = pageURL;
+    }
+
+    auto& targetURL = request.url();
+
+    if (!targetURL.isValid() || !pageURL.isValid())
+        return;
+
+    auto targetHost = targetURL.host();
+    auto mainFrameHost = pageURL.host();
+
+    if (targetHost.isEmpty() || mainFrameHost.isEmpty() || targetHost == sourceURL.host())
+        return;
+
+    auto* page = frame.page();
+    auto targetPrimaryDomain = ResourceLoadStatistics::primaryDomain(targetURL);
+    auto mainFramePrimaryDomain = ResourceLoadStatistics::primaryDomain(pageURL);
+    auto sourcePrimaryDomain = ResourceLoadStatistics::primaryDomain(sourceURL);
+
+    bool areTargetAndMainFrameDomainsAssociated = areDomainsAssociated(page, targetPrimaryDomain, mainFramePrimaryDomain);
+    bool areTargetAndSourceDomainsAssociated = areDomainsAssociated(page, targetPrimaryDomain, sourcePrimaryDomain);
+
+    m_statisticsQueue->dispatch([this, protectedThis = makeRef(*this), targetPrimaryDomain = targetPrimaryDomain.isolatedCopy(), mainFramePrimaryDomain = mainFramePrimaryDomain.isolatedCopy(), sourcePrimaryDomain = sourcePrimaryDomain.isolatedCopy(), targetHost = targetHost.toString().isolatedCopy(), mainFrameHost = mainFrameHost.toString().isolatedCopy(), areTargetAndMainFrameDomainsAssociated, areTargetAndSourceDomainsAssociated, isRedirect, isMainFrame = frame.isMainFrame()] {
+        bool statisticsWereUpdated = false;
+        if (targetHost != mainFrameHost && !(areTargetAndMainFrameDomainsAssociated || areTargetAndSourceDomainsAssociated)) {
+            auto& targetStatistics = ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
+            targetStatistics.lastSeen = ResourceLoadStatistics::reduceTimeResolution(WallTime::now());
+            if (targetStatistics.subframeUnderTopFrameOrigins.add(mainFramePrimaryDomain).isNewEntry)
+                statisticsWereUpdated = true;
+        }
+
+        if (isRedirect && !areTargetAndSourceDomainsAssociated) {
+            if (isMainFrame) {
+                auto& redirectingOriginStatistics = ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
+                if (redirectingOriginStatistics.topFrameUniqueRedirectsTo.add(targetPrimaryDomain).isNewEntry)
+                    statisticsWereUpdated = true;
+                auto& targetStatistics = ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
+                if (targetStatistics.topFrameUniqueRedirectsFrom.add(sourcePrimaryDomain).isNewEntry)
+                    statisticsWereUpdated = true;
+            } else {
+                auto& redirectingOriginStatistics = ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
+                if (redirectingOriginStatistics.subresourceUniqueRedirectsTo.add(targetPrimaryDomain).isNewEntry)
+                    statisticsWereUpdated = true;
+                auto& targetStatistics = ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
+                if (targetStatistics.subresourceUniqueRedirectsFrom.add(sourcePrimaryDomain).isNewEntry)
+                    statisticsWereUpdated = true;
+            }
+        }
+
+        if (statisticsWereUpdated)
+            scheduleStatisticsProcessingRequestIfNecessary();
+    });
+}
+
 void WebResourceLoadStatisticsStore::logUserInteraction(const URL& url)
 {
     if (url.isBlankURL() || url.isEmpty())
index 468d594..1fa8646 100644 (file)
@@ -48,6 +48,7 @@ class WorkQueue;
 namespace WebCore {
 class KeyedDecoder;
 class KeyedEncoder;
+class ResourceRequest;
 class URL;
 struct ResourceLoadStatistics;
 }
@@ -55,6 +56,7 @@ struct ResourceLoadStatistics;
 namespace WebKit {
 
 class OperatingDate;
+class WebFrameProxy;
 class WebProcessProxy;
 
 enum class ShouldClearFirst;
@@ -99,6 +101,7 @@ public:
     void processDidCloseConnection(WebProcessProxy&, IPC::Connection&);
     void applicationWillTerminate();
 
+    void logFrameNavigation(const WebFrameProxy&, const WebCore::URL& pageURL, const WebCore::ResourceRequest&, const WebCore::URL& redirectURL);
     void logUserInteraction(const WebCore::URL&);
     void logNonRecentUserInteraction(const WebCore::URL&);
     void clearUserInteraction(const WebCore::URL&);
@@ -185,6 +188,9 @@ private:
     void setPrevalentResource(WebCore::ResourceLoadStatistics&, ResourceLoadPrevalence);
     void processStatisticsAndDataRecords();
 
+    void scheduleStatisticsProcessingRequestIfNecessary();
+    void cancelPendingStatisticsProcessingRequest();
+
     void resetCookiePartitioningState();
     StorageAccessStatus storageAccessStatus(const String& subFramePrimaryDomain, const String& topFramePrimaryDomain);
     void grantStorageAccessInternal(String&& subFrameHost, String&& topFrameHost, std::optional<uint64_t> frameID, uint64_t pageID, bool userWasPromptedNowOrEarlier, CompletionHandler<void(bool)>&&);
@@ -241,8 +247,12 @@ private:
     bool m_debugModeEnabled { false };
     bool m_debugLoggingEnabled { false };
     bool m_storageAccessPromptsEnabled { false };
+    bool m_hasScheduledProcessStats { false };
 
     Function<void (const String&)> m_statisticsTestingCallback;
+
+    uint64_t m_lastStatisticsProcessingRequestIdentifier { 0 };
+    std::optional<uint64_t> m_pendingStatisticsProcessingRequestIdentifier;
 };
 
 } // namespace WebKit
index 019d8be..8430fb6 100644 (file)
@@ -813,7 +813,7 @@ void WebFrameLoaderClient::applyToDocumentLoader(WebsitePoliciesData&& websitePo
     WebsitePoliciesData::applyToDocumentLoader(WTFMove(websitePolicies), *documentLoader);
 }
 
-void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& navigationAction, const ResourceRequest& request, bool didReceiveRedirectResponse, FormState* formState, PolicyDecisionMode policyDecisionMode, FramePolicyFunction&& function)
+void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& navigationAction, const ResourceRequest& request, const ResourceResponse& redirectResponse, FormState* formState, PolicyDecisionMode policyDecisionMode, FramePolicyFunction&& function)
 {
     WebPage* webPage = m_frame ? m_frame->page() : nullptr;
     if (!webPage) {
@@ -864,7 +864,7 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const Navigat
     navigationActionData.canHandleRequest = webPage->canHandleRequest(request);
     navigationActionData.shouldOpenExternalURLsPolicy = navigationAction.shouldOpenExternalURLsPolicy();
     navigationActionData.downloadAttribute = navigationAction.downloadAttribute();
-    navigationActionData.isRedirect = didReceiveRedirectResponse;
+    navigationActionData.isRedirect = !redirectResponse.isNull();
     navigationActionData.treatAsSameOriginNavigation = navigationAction.treatAsSameOriginNavigation();
     navigationActionData.isCrossOriginWindowOpenNavigation = navigationAction.isCrossOriginWindowOpenNavigation();
     navigationActionData.hasOpenedFrames = navigationAction.hasOpenedFrames();
@@ -892,7 +892,7 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const Navigat
         DownloadID downloadID;
         std::optional<WebsitePoliciesData> websitePolicies;
 
-        if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForNavigationActionSync(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())), Messages::WebPageProxy::DecidePolicyForNavigationActionSync::Reply(newNavigationID, policyAction, downloadID, websitePolicies))) {
+        if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForNavigationActionSync(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, redirectResponse, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())), Messages::WebPageProxy::DecidePolicyForNavigationActionSync::Reply(newNavigationID, policyAction, downloadID, websitePolicies))) {
             m_frame->didReceivePolicyDecision(listenerID, PolicyAction::Ignore, 0, { }, { });
             return;
         }
@@ -902,7 +902,7 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const Navigat
     }
 
     ASSERT(policyDecisionMode == PolicyDecisionMode::Asynchronous);
-    if (!webPage->send(Messages::WebPageProxy::DecidePolicyForNavigationAction(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get()))))
+    if (!webPage->send(Messages::WebPageProxy::DecidePolicyForNavigationAction(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, redirectResponse, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get()))))
         m_frame->didReceivePolicyDecision(listenerID, PolicyAction::Ignore, 0, { }, { });
 }
 
index 1f92701..63cbb12 100644 (file)
@@ -125,7 +125,7 @@ private:
     
     void dispatchDecidePolicyForResponse(const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, WebCore::FramePolicyFunction&&) final;
     void dispatchDecidePolicyForNewWindowAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, WebCore::FormState*, const String& frameName, WebCore::FramePolicyFunction&&) final;
-    void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, bool didReceiveRedirectResponse, WebCore::FormState*, WebCore::PolicyDecisionMode, WebCore::FramePolicyFunction&&) final;
+    void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, const WebCore::ResourceResponse& redirectResponse, WebCore::FormState*, WebCore::PolicyDecisionMode, WebCore::FramePolicyFunction&&) final;
     void cancelPolicyCheck() final;
     
     void dispatchUnableToImplementPolicy(const WebCore::ResourceError&) final;
index 69b7687..6492beb 100644 (file)
@@ -200,7 +200,6 @@ WebProcess::WebProcess()
     m_plugInAutoStartOriginHashes.add(PAL::SessionID::defaultSessionID(), HashMap<unsigned, WallTime>());
 
     ResourceLoadObserver::shared().setNotificationCallback([this] (Vector<ResourceLoadStatistics>&& statistics) {
-        ASSERT(!statistics.isEmpty());
         parentProcessConnection()->send(Messages::WebResourceLoadStatisticsStore::ResourceLoadStatisticsUpdated(WTFMove(statistics)), 0);
     });
 
index 9ca6668..8a12041 100644 (file)
@@ -1,3 +1,16 @@
+2018-06-13  Chris Dumez  <cdumez@apple.com>
+
+        PSON: http/tests/resourceLoadStatistics/classify-as-prevalent-based-on-top-frame-redirect-collusion.html ASSERTS with process swap enabled
+        https://bugs.webkit.org/show_bug.cgi?id=186545
+
+        Reviewed by Brady Eidson.
+
+        Update client delegate now that parameter type has changed.
+
+        * WebCoreSupport/WebFrameLoaderClient.h:
+        * WebCoreSupport/WebFrameLoaderClient.mm:
+        (WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
+
 2018-06-11  Tim Horton  <timothy_horton@apple.com>
 
         Link drag image is inconsistently unreadable in dark mode
index 0d6aaf8..388d14a 100644 (file)
@@ -128,7 +128,7 @@ private:
 
     void dispatchDecidePolicyForResponse(const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, WebCore::FramePolicyFunction&&) final;
     void dispatchDecidePolicyForNewWindowAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, WebCore::FormState*, const WTF::String& frameName, WebCore::FramePolicyFunction&&) final;
-    void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, bool didReceiveRedirectResponse, WebCore::FormState*, WebCore::PolicyDecisionMode, WebCore::FramePolicyFunction&&) final;
+    void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, const WebCore::ResourceResponse& redirectResponse, WebCore::FormState*, WebCore::PolicyDecisionMode, WebCore::FramePolicyFunction&&) final;
     void cancelPolicyCheck() final;
 
     void dispatchUnableToImplementPolicy(const WebCore::ResourceError&) final;
index e59c189..49870de 100644 (file)
@@ -901,7 +901,7 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction(const Navigati
                           decisionListener:setUpPolicyListener(WTFMove(function), PolicyAction::Ignore, tryAppLink ? (NSURL *)request.url() : nil).get()];
 }
 
-void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& action, const ResourceRequest& request, bool, FormState* formState, PolicyDecisionMode, FramePolicyFunction&& function)
+void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& action, const ResourceRequest& request, const ResourceResponse&, FormState* formState, PolicyDecisionMode, FramePolicyFunction&& function)
 {
     WebView *webView = getWebView(m_webFrame.get());
     BOOL tryAppLink = shouldTryAppLink(webView, action, core(m_webFrame.get()));
index 988187a..19587e7 100644 (file)
@@ -1,3 +1,16 @@
+2018-06-13  Chris Dumez  <cdumez@apple.com>
+
+        PSON: http/tests/resourceLoadStatistics/classify-as-prevalent-based-on-top-frame-redirect-collusion.html ASSERTS with process swap enabled
+        https://bugs.webkit.org/show_bug.cgi?id=186545
+
+        Reviewed by Brady Eidson.
+
+        Update client delegate now that parameter type has changed.
+
+        * WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
+        * WebCoreSupport/WebFrameLoaderClient.h:
+
 2018-06-11  Chris Dumez  <cdumez@apple.com>
 
         http/tests/security/cors-post-redirect-307.html fails with PSON enabled
index d45f3c3..2058be5 100644 (file)
@@ -565,7 +565,7 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction(const Navigati
     m_policyListenerPrivate->m_policyFunction(PolicyAction::Use);
 }
 
-void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& action, const ResourceRequest& request, bool, FormState* formState, WebCore::PolicyDecisionMode, FramePolicyFunction&& function)
+void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& action, const ResourceRequest& request, const ResourceResponse&, FormState* formState, WebCore::PolicyDecisionMode, FramePolicyFunction&& function)
 {
     WebView* webView = m_webFrame->webView();
     Frame* coreFrame = core(m_webFrame);
index 1a03106..35b665a 100644 (file)
@@ -102,7 +102,7 @@ public:
 
     void dispatchDecidePolicyForResponse(const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, WebCore::FramePolicyFunction&&) override;
     void dispatchDecidePolicyForNewWindowAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, WebCore::FormState*, const WTF::String& frameName, WebCore::FramePolicyFunction&&) override;
-    void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, bool didReceiveRedirectResponse, WebCore::FormState*, WebCore::PolicyDecisionMode, WebCore::FramePolicyFunction&&) override;
+    void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, const WebCore::ResourceResponse& redirectResponse, WebCore::FormState*, WebCore::PolicyDecisionMode, WebCore::FramePolicyFunction&&) override;
     void cancelPolicyCheck() override;
 
     void dispatchUnableToImplementPolicy(const WebCore::ResourceError&) override;