Resource Load Statistics: Immediately forward cookie access for domains with previous...
authorwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Mar 2018 05:17:57 +0000 (05:17 +0000)
committerwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Mar 2018 05:17:57 +0000 (05:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=183620
<rdar://problem/38431469>

Reviewed by Brent Fulgham.

Source/WebCore:

Tests: http/tests/storageAccess/deny-storage-access-under-opener.html
       http/tests/storageAccess/grant-storage-access-under-opener.html

It turns out the fix in https://bugs.webkit.org/show_bug.cgi?id=183577
wasn't enough to address the compatibility issues with popups. Some of
them just detect their unpartitioned cookies, auto-dismiss themselves,
and expect their unpartitioned cookies to be available under the opener
afterwards. We should grant them access if the popup's domain has had
user interaction _previously_.

Note that we still need https://bugs.webkit.org/show_bug.cgi?id=183577
because if the popup's domain has not received user interaction
previously, we will not grant it storage access on just the window open.

* dom/Document.cpp:
(WebCore::Document::hasRequestedPageSpecificStorageAccessWithUserInteraction):
(WebCore::Document::setHasRequestedPageSpecificStorageAccessWithUserInteraction):
(WebCore::Document::hasGrantedPageSpecificStorageAccess): Deleted.
(WebCore::Document::setHasGrantedPageSpecificStorageAccess): Deleted.
    Renamed from *Granted* to *Requested* since there is now a case
    where access will not be granted, i.e. when the popup domain has
    not had user interaction previously.
* dom/Document.h:
* loader/ResourceLoadObserver.cpp:
(WebCore::ResourceLoadObserver::setRequestStorageAccessUnderOpenerCallback):
    Renamed *Grant* to *Request*.
(WebCore::ResourceLoadObserver::logUserInteractionWithReducedTimeResolution):
(WebCore::ResourceLoadObserver::logWindowCreation):
    New function called from DOMWindow::createWindow().
(WebCore::ResourceLoadObserver::requestStorageAccessUnderOpener):
    New convenience function.
(WebCore::ResourceLoadObserver::setGrantStorageAccessUnderOpenerCallback): Deleted.
    Renamed *Grant* to *Request*.
* loader/ResourceLoadObserver.h:
* page/DOMWindow.cpp:
(WebCore::DOMWindow::createWindow):
    Now calls ResourceLoadObserver::logWindowCreation() if a window
    was created and the opener has a document and a page ID.

Source/WebKit:

It turns out the fix in https://bugs.webkit.org/show_bug.cgi?id=183577
wasn't enough to address the compatibility issues with popups. Some of
them just detect their unpartitioned cookies, auto-dismiss themselves,
and expect their unpartitioned cookies to be available under the opener
afterwards. We should grant them access if the popup's domain has had
user interaction _previously_.

Note that we still need https://bugs.webkit.org/show_bug.cgi?id=183577
because if the popup's domain has not received user interaction
previously, we will not grant it storage access on just the window open.

* UIProcess/WebResourceLoadStatisticsStore.cpp:
(WebKit::WebResourceLoadStatisticsStore::requestStorageAccessUnderOpener):
(WebKit::WebResourceLoadStatisticsStore::grantStorageAccessUnderOpener): Deleted.
    Renamed WebResourceLoadStatisticsStore::grantStorageAccessUnderOpener()
    to WebResourceLoadStatisticsStore::requestStorageAccessUnderOpener()
    since there is now a case where access will not be granted, i.e. when
    the popup domain has not had user interaction previously.
* UIProcess/WebResourceLoadStatisticsStore.h:
* UIProcess/WebResourceLoadStatisticsStore.messages.in:
     Similar renaming.
* WebProcess/WebProcess.cpp:
(WebProcess::WebProcess):
     Similar renaming.

LayoutTests:

* http/tests/storageAccess/deny-storage-access-under-opener-expected.txt: Added.
* http/tests/storageAccess/deny-storage-access-under-opener.html: Added.
* http/tests/storageAccess/grant-storage-access-under-opener-expected.txt: Added.
* http/tests/storageAccess/grant-storage-access-under-opener.html: Added.
* http/tests/storageAccess/resources/set-cookie-and-report-back.html: Added.
* platform/ios/TestExpectations:
    New tests marked as [ Pass ].
* platform/mac-wk2/TestExpectations:
    New tests marked as [ Pass ].

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

19 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/storageAccess/deny-storage-access-under-opener-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/storageAccess/deny-storage-access-under-opener.html [new file with mode: 0644]
LayoutTests/http/tests/storageAccess/grant-storage-access-under-opener-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/storageAccess/grant-storage-access-under-opener.html [new file with mode: 0644]
LayoutTests/http/tests/storageAccess/resources/set-cookie-and-report-back.html [new file with mode: 0644]
LayoutTests/platform/ios/TestExpectations
LayoutTests/platform/mac-wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/loader/ResourceLoadObserver.cpp
Source/WebCore/loader/ResourceLoadObserver.h
Source/WebCore/page/DOMWindow.cpp
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp
Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h
Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.messages.in
Source/WebKit/WebProcess/WebProcess.cpp

index 3ee85ef..c301810 100644 (file)
@@ -1,3 +1,21 @@
+2018-03-13  John Wilander  <wilander@apple.com>
+
+        Resource Load Statistics: Immediately forward cookie access for domains with previous user interaction when there's an opener document
+        https://bugs.webkit.org/show_bug.cgi?id=183620
+        <rdar://problem/38431469>
+
+        Reviewed by Brent Fulgham.
+
+        * http/tests/storageAccess/deny-storage-access-under-opener-expected.txt: Added.
+        * http/tests/storageAccess/deny-storage-access-under-opener.html: Added.
+        * http/tests/storageAccess/grant-storage-access-under-opener-expected.txt: Added.
+        * http/tests/storageAccess/grant-storage-access-under-opener.html: Added.
+        * http/tests/storageAccess/resources/set-cookie-and-report-back.html: Added.
+        * platform/ios/TestExpectations:
+            New tests marked as [ Pass ].
+        * platform/mac-wk2/TestExpectations:
+            New tests marked as [ Pass ].
+
 2018-03-13  Youenn Fablet  <youenn@apple.com>
 
         Layout Test imported/w3c/web-platform-tests/service-workers/service-worker/register-closed-window.https.html is flaky
diff --git a/LayoutTests/http/tests/storageAccess/deny-storage-access-under-opener-expected.txt b/LayoutTests/http/tests/storageAccess/deny-storage-access-under-opener-expected.txt
new file mode 100644 (file)
index 0000000..5d644c6
--- /dev/null
@@ -0,0 +1,18 @@
+Tests that a cross-origin window from a prevalent domain without user interaction is denied storage access under its opener.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Cookie created.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+
+--------
+Frame: '<!--framePath //<!--frame0-->-->'
+--------
+Should not receive first-party cookie.
+Did not receive cookie named 'firstPartyCookie'.
+Did not receive cookie named ''.
+Client-side document.cookie:
diff --git a/LayoutTests/http/tests/storageAccess/deny-storage-access-under-opener.html b/LayoutTests/http/tests/storageAccess/deny-storage-access-under-opener.html
new file mode 100644 (file)
index 0000000..c1ad838
--- /dev/null
@@ -0,0 +1,71 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script src="/js-test-resources/js-test.js"></script>
+    <script>
+        description("Tests that a cross-origin window from a prevalent domain without user interaction is denied storage access under its opener.");
+        jsTestIsAsync = true;
+
+        function setEnableFeature(enable) {
+            if (!enable)
+                testRunner.statisticsResetToConsistentState();
+            internals.setResourceLoadStatisticsEnabled(enable);
+            testRunner.setCookieStoragePartitioningEnabled(enable);
+            testRunner.setStorageAccessAPIEnabled(enable);
+        }
+
+        window.addEventListener("message", receiveMessage, false);
+
+        function finishTest() {
+            setEnableFeature(false);
+            finishJSTest();
+        }
+
+        function openIframe(url, onLoadHandler) {
+            const element = document.createElement("iframe");
+            element.src = url;
+            if (onLoadHandler) {
+                element.onload = onLoadHandler;
+            }
+            document.body.appendChild(element);
+        }
+
+        function receiveMessage(event) {
+            if (event.origin === "http://localhost:8000") {
+                if (event.data.indexOf("PASS") !== -1)
+                    testPassed(event.data.replace("PASS ", ""));
+                else
+                    testFailed(event.data);
+            } else
+                testFailed("Received a message from an unexpected origin: " + event.origin);
+
+            newWin.close();
+            openIframe(thirdPartyBaseUrl + subPathToGetCookies + "&message=Should not receive first-party cookie.", finishTest);
+        }
+
+        setEnableFeature(true);
+        testRunner.setCanOpenWindows();
+
+        const thirdPartyOrigin = "http://localhost:8000";
+        const resourcePath = "/storageAccess/resources";
+        const thirdPartyBaseUrl = thirdPartyOrigin + resourcePath;
+        const firstPartyCookieName = "firstPartyCookie";
+        const subPathToGetCookies = "/get-cookies.php?name1=" + firstPartyCookieName;
+
+        testRunner.setStatisticsPrevalentResource(thirdPartyOrigin, true);
+        if (!testRunner.isStatisticsPrevalentResource(thirdPartyOrigin))
+            testFailed("Host did not get set as prevalent resource.");
+        if (testRunner.isStatisticsHasHadUserInteraction(thirdPartyOrigin))
+            testFailed("Host logged for user interaction.");
+        testRunner.dumpChildFramesAsText();
+        testRunner.setCloseRemainingWindowsWhenComplete(true);
+
+        var newWin;
+        testRunner.statisticsUpdateCookiePartitioning(function () {
+            newWin = window.open(thirdPartyOrigin + "/storageAccess/resources/set-cookie-and-report-back.html", "testWindow");
+        });
+    </script>
+</head>
+<body>
+</body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/http/tests/storageAccess/grant-storage-access-under-opener-expected.txt b/LayoutTests/http/tests/storageAccess/grant-storage-access-under-opener-expected.txt
new file mode 100644 (file)
index 0000000..8ec4d34
--- /dev/null
@@ -0,0 +1,18 @@
+Tests that a cross-origin window from a prevalent domain with non-recent user interaction gets immediate storage access under its opener.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Cookie created.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+
+--------
+Frame: '<!--framePath //<!--frame0-->-->'
+--------
+Should receive first-party cookie.
+Received cookie named 'firstPartyCookie'.
+Did not receive cookie named ''.
+Client-side document.cookie: firstPartyCookie=value
diff --git a/LayoutTests/http/tests/storageAccess/grant-storage-access-under-opener.html b/LayoutTests/http/tests/storageAccess/grant-storage-access-under-opener.html
new file mode 100644 (file)
index 0000000..aa514e3
--- /dev/null
@@ -0,0 +1,72 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script src="/js-test-resources/js-test.js"></script>
+    <script>
+        description("Tests that a cross-origin window from a prevalent domain with non-recent user interaction gets immediate storage access under its opener.");
+        jsTestIsAsync = true;
+
+        function setEnableFeature(enable) {
+            if (!enable)
+                testRunner.statisticsResetToConsistentState();
+            internals.setResourceLoadStatisticsEnabled(enable);
+            testRunner.setCookieStoragePartitioningEnabled(enable);
+            testRunner.setStorageAccessAPIEnabled(enable);
+        }
+
+        window.addEventListener("message", receiveMessage, false);
+
+        function finishTest() {
+            setEnableFeature(false);
+            finishJSTest();
+        }
+
+        function openIframe(url, onLoadHandler) {
+            const element = document.createElement("iframe");
+            element.src = url;
+            if (onLoadHandler) {
+                element.onload = onLoadHandler;
+            }
+            document.body.appendChild(element);
+        }
+
+        function receiveMessage(event) {
+            if (event.origin === "http://localhost:8000") {
+                if (event.data.indexOf("PASS") !== -1)
+                    testPassed(event.data.replace("PASS ", ""));
+                else
+                    testFailed(event.data);
+            } else
+                testFailed("Received a message from an unexpected origin: " + event.origin);
+
+            newWin.close();
+            openIframe(thirdPartyBaseUrl + subPathToGetCookies + "&message=Should receive first-party cookie.", finishTest);
+        }
+
+        setEnableFeature(true);
+        testRunner.setCanOpenWindows();
+
+        const thirdPartyOrigin = "http://localhost:8000";
+        const resourcePath = "/storageAccess/resources";
+        const thirdPartyBaseUrl = thirdPartyOrigin + resourcePath;
+        const firstPartyCookieName = "firstPartyCookie";
+        const subPathToGetCookies = "/get-cookies.php?name1=" + firstPartyCookieName;
+
+        testRunner.setStatisticsPrevalentResource(thirdPartyOrigin, true);
+        if (!testRunner.isStatisticsPrevalentResource(thirdPartyOrigin))
+            testFailed("Host did not get set as prevalent resource.");
+        testRunner.setStatisticsHasHadNonRecentUserInteraction(thirdPartyOrigin, true);
+        if (!testRunner.isStatisticsHasHadUserInteraction(thirdPartyOrigin))
+            testFailed("Host did not get logged for user interaction.");
+        testRunner.dumpChildFramesAsText();
+        testRunner.setCloseRemainingWindowsWhenComplete(true);
+
+        var newWin;
+        testRunner.statisticsUpdateCookiePartitioning(function () {
+            newWin = window.open(thirdPartyOrigin + "/storageAccess/resources/set-cookie-and-report-back.html", "testWindow");
+        });
+    </script>
+</head>
+<body>
+</body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/http/tests/storageAccess/resources/set-cookie-and-report-back.html b/LayoutTests/http/tests/storageAccess/resources/set-cookie-and-report-back.html
new file mode 100644 (file)
index 0000000..1be02d1
--- /dev/null
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script src="/js-test-resources/ui-helper.js"></script>
+    <script>
+        function setCookieAndPostMessage() {
+            document.cookie = "firstPartyCookie=value";
+            window.opener.postMessage("PASS Cookie created.", "http://127.0.0.1:8000");
+        }
+    </script>
+</head>
+<body onload="setCookieAndPostMessage()">
+</body>
+</html>
\ No newline at end of file
index c1f9830..129df93 100644 (file)
@@ -3007,6 +3007,8 @@ http/tests/resourceLoadStatistics/clear-in-memory-and-persistent-store.html [ Pa
 http/tests/resourceLoadStatistics/clear-in-memory-and-persistent-store-one-hour.html [ Pass ]
 http/tests/resourceLoadStatistics/strip-referrer-to-origin-for-prevalent-subresource-redirects.html [ Pass ]
 http/tests/resourceLoadStatistics/strip-referrer-to-origin-for-prevalent-subresource-requests.html [ Pass ]
+http/tests/storageAccess/deny-storage-access-under-opener.html [ Pass ]
+http/tests/storageAccess/grant-storage-access-under-opener.html [ Pass ]
 
 # Skipped in general expectations since they only work on iOS and Mac, WK2.
 http/tests/security/strip-referrer-to-origin-for-third-party-redirects-in-private-mode.html [ Pass ]
index 6ac8ae5..c502b0e 100644 (file)
@@ -760,6 +760,8 @@ http/tests/resourceLoadStatistics/user-interaction-reported-after-website-data-r
 [ HighSierra+ ] http/tests/storageAccess/has-storage-access-from-prevalent-domain-with-recent-user-interaction.html [ Pass ]
 [ HighSierra+ ] http/tests/storageAccess/request-and-grant-access-then-detach-should-not-have-access.html [ Pass ]
 [ HighSierra+ ] http/tests/storageAccess/request-and-grant-access-then-navigate-should-not-have-access.html [ Pass ]
+[ HighSierra+ ] http/tests/storageAccess/deny-storage-access-under-opener.html [ Pass ]
+[ HighSierra+ ] http/tests/storageAccess/grant-storage-access-under-opener.html [ Pass ]
 
 # As of https://trac.webkit.org/changeset/227762 the timestampResolution is just 5 seconds which makes this test flaky
 http/tests/resourceLoadStatistics/user-interaction-only-reported-once-within-short-period-of-time.html [ Skip ]
index 765f772..5984932 100644 (file)
@@ -1,3 +1,50 @@
+2018-03-13  John Wilander  <wilander@apple.com>
+
+        Resource Load Statistics: Immediately forward cookie access for domains with previous user interaction when there's an opener document
+        https://bugs.webkit.org/show_bug.cgi?id=183620
+        <rdar://problem/38431469>
+
+        Reviewed by Brent Fulgham.
+
+        Tests: http/tests/storageAccess/deny-storage-access-under-opener.html
+               http/tests/storageAccess/grant-storage-access-under-opener.html
+
+        It turns out the fix in https://bugs.webkit.org/show_bug.cgi?id=183577
+        wasn't enough to address the compatibility issues with popups. Some of
+        them just detect their unpartitioned cookies, auto-dismiss themselves,
+        and expect their unpartitioned cookies to be available under the opener
+        afterwards. We should grant them access if the popup's domain has had
+        user interaction _previously_.
+
+        Note that we still need https://bugs.webkit.org/show_bug.cgi?id=183577
+        because if the popup's domain has not received user interaction
+        previously, we will not grant it storage access on just the window open.
+
+        * dom/Document.cpp:
+        (WebCore::Document::hasRequestedPageSpecificStorageAccessWithUserInteraction):
+        (WebCore::Document::setHasRequestedPageSpecificStorageAccessWithUserInteraction):
+        (WebCore::Document::hasGrantedPageSpecificStorageAccess): Deleted.
+        (WebCore::Document::setHasGrantedPageSpecificStorageAccess): Deleted.
+            Renamed from *Granted* to *Requested* since there is now a case
+            where access will not be granted, i.e. when the popup domain has
+            not had user interaction previously.
+        * dom/Document.h:
+        * loader/ResourceLoadObserver.cpp:
+        (WebCore::ResourceLoadObserver::setRequestStorageAccessUnderOpenerCallback):
+            Renamed *Grant* to *Request*.
+        (WebCore::ResourceLoadObserver::logUserInteractionWithReducedTimeResolution):
+        (WebCore::ResourceLoadObserver::logWindowCreation):
+            New function called from DOMWindow::createWindow().
+        (WebCore::ResourceLoadObserver::requestStorageAccessUnderOpener):
+            New convenience function.
+        (WebCore::ResourceLoadObserver::setGrantStorageAccessUnderOpenerCallback): Deleted.
+            Renamed *Grant* to *Request*.
+        * loader/ResourceLoadObserver.h:
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::createWindow):
+            Now calls ResourceLoadObserver::logWindowCreation() if a window
+            was created and the opener has a document and a page ID.
+
 2018-03-13  Chris Dumez  <cdumez@apple.com>
 
         fast/loader/javascript-url-iframe-remove-on-navigate.html is a flaky crash on iOS with async delegates
index 40e93f2..ab3e634 100644 (file)
@@ -7623,14 +7623,14 @@ void Document::setHasFrameSpecificStorageAccess(bool value)
     m_frame->loader().client().setHasFrameSpecificStorageAccess(value);
 }
 
-bool Document::hasGrantedPageSpecificStorageAccess(String& primaryDomain)
+bool Document::hasRequestedPageSpecificStorageAccessWithUserInteraction(const String& primaryDomain)
 {
-    return m_primaryDomainGrantedPageSpecificStorageAccess == primaryDomain;
+    return m_primaryDomainRequestedPageSpecificStorageAccessWithUserInteraction == primaryDomain;
 }
 
-void Document::setHasGrantedPageSpecificStorageAccess(String& primaryDomain)
+void Document::setHasRequestedPageSpecificStorageAccessWithUserInteraction(const String& primaryDomain)
 {
-    m_primaryDomainGrantedPageSpecificStorageAccess = primaryDomain;
+    m_primaryDomainRequestedPageSpecificStorageAccessWithUserInteraction = primaryDomain;
 }
 
 #endif
index f78f0c7..4824793 100644 (file)
@@ -1416,8 +1416,8 @@ public:
     JSC::ThreadLocalCache& threadLocalCache();
 
 #if HAVE(CFNETWORK_STORAGE_PARTITIONING)
-    bool hasGrantedPageSpecificStorageAccess(String& primaryDomain);
-    void setHasGrantedPageSpecificStorageAccess(String& primaryDomain);
+    bool hasRequestedPageSpecificStorageAccessWithUserInteraction(const String& primaryDomain);
+    void setHasRequestedPageSpecificStorageAccessWithUserInteraction(const String& primaryDomain);
 #endif
 
 protected:
@@ -1908,7 +1908,7 @@ private:
     RefPtr<JSC::ThreadLocalCache> m_threadLocalCache;
 
 #if HAVE(CFNETWORK_STORAGE_PARTITIONING)
-    String m_primaryDomainGrantedPageSpecificStorageAccess { };
+    String m_primaryDomainRequestedPageSpecificStorageAccessWithUserInteraction { };
 #endif
 };
 
index 7443efb..3c8fb53 100644 (file)
@@ -108,10 +108,10 @@ void ResourceLoadObserver::setNotificationCallback(WTF::Function<void (Vector<Re
     m_notificationCallback = WTFMove(notificationCallback);
 }
 
-void ResourceLoadObserver::setGrantStorageAccessUnderOpenerCallback(WTF::Function<void(const String& domainReceivingUserInteraction, uint64_t openerPageID, const String& openerDomain)>&& callback)
+void ResourceLoadObserver::setRequestStorageAccessUnderOpenerCallback(WTF::Function<void(const String& domainInNeedOfStorageAccess, uint64_t openerPageID, const String& openerDomain, bool isTriggeredByUserGesture)>&& callback)
 {
-    ASSERT(!m_grantStorageAccessUnderOpenerCallback);
-    m_grantStorageAccessUnderOpenerCallback = WTFMove(callback);
+    ASSERT(!m_requestStorageAccessUnderOpenerCallback);
+    m_requestStorageAccessUnderOpenerCallback = WTFMove(callback);
 }
 
 ResourceLoadObserver::ResourceLoadObserver()
@@ -322,14 +322,7 @@ void ResourceLoadObserver::logUserInteractionWithReducedTimeResolution(const Doc
         if (auto* openerDocument = opener->document()) {
             if (auto* openerFrame = openerDocument->frame()) {
                 if (auto openerPageID = openerFrame->loader().client().pageID()) {
-                    auto openerUrl = openerDocument->url();
-                    auto openerPrimaryDomain = primaryDomain(openerUrl);
-                    if (domain != openerPrimaryDomain
-                        && !openerDocument->hasGrantedPageSpecificStorageAccess(domain)
-                        && !equalIgnoringASCIICase(openerUrl.string(), blankURL())) {
-                        openerDocument->setHasGrantedPageSpecificStorageAccess(domain);
-                        m_grantStorageAccessUnderOpenerCallback(domain, openerPageID.value(), openerPrimaryDomain);
-                    }
+                    requestStorageAccessUnderOpener(domain, openerPageID.value(), *openerDocument, true);
                 }
             }
         }
@@ -361,6 +354,33 @@ void ResourceLoadObserver::logUserInteractionWithReducedTimeResolution(const Doc
 #endif
 }
 
+void ResourceLoadObserver::logWindowCreation(const URL& popupUrl, uint64_t openerPageID, Document& openerDocument)
+{
+#if HAVE(CFNETWORK_STORAGE_PARTITIONING)
+    requestStorageAccessUnderOpener(primaryDomain(popupUrl), openerPageID, openerDocument, false);
+#else
+    UNUSED_PARAM(popupUrl);
+    UNUSED_PARAM(openerPageID);
+    UNUSED_PARAM(openerDocument);
+#endif
+}
+
+#if HAVE(CFNETWORK_STORAGE_PARTITIONING)
+void ResourceLoadObserver::requestStorageAccessUnderOpener(const String& domainInNeedOfStorageAccess, uint64_t openerPageID, Document& openerDocument, bool isTriggeredByUserGesture)
+{
+    auto openerUrl = openerDocument.url();
+    auto openerPrimaryDomain = primaryDomain(openerUrl);
+    if (domainInNeedOfStorageAccess != openerPrimaryDomain
+        && !openerDocument.hasRequestedPageSpecificStorageAccessWithUserInteraction(domainInNeedOfStorageAccess)
+        && !equalIgnoringASCIICase(openerUrl.string(), blankURL())) {
+        m_requestStorageAccessUnderOpenerCallback(domainInNeedOfStorageAccess, openerPageID, openerPrimaryDomain, isTriggeredByUserGesture);
+        // Remember user interaction-based requests since they don't need to be repeated.
+        if (isTriggeredByUserGesture)
+            openerDocument.setHasRequestedPageSpecificStorageAccessWithUserInteraction(domainInNeedOfStorageAccess);
+    }
+}
+#endif
+
 ResourceLoadStatistics& ResourceLoadObserver::ensureResourceStatisticsForPrimaryDomain(const String& primaryDomain)
 {
     auto addResult = m_resourceStatisticsMap.ensure(primaryDomain, [&primaryDomain] {
index b92dd5e..1880f67 100644 (file)
@@ -57,11 +57,12 @@ public:
     void logSubresourceLoading(const Frame*, const ResourceRequest& newRequest, const ResourceResponse& redirectResponse);
     void logWebSocketLoading(const Frame*, const URL&);
     void logUserInteractionWithReducedTimeResolution(const Document&);
+    void logWindowCreation(const URL& popupUrl, uint64_t openerPageID, Document& openerDocument);
 
     WEBCORE_EXPORT String statisticsForOrigin(const String&);
 
     WEBCORE_EXPORT void setNotificationCallback(WTF::Function<void (Vector<ResourceLoadStatistics>&&)>&&);
-    WEBCORE_EXPORT void setGrantStorageAccessUnderOpenerCallback(WTF::Function<void(const String&, uint64_t, const String&)>&&);
+    WEBCORE_EXPORT void setRequestStorageAccessUnderOpenerCallback(WTF::Function<void(const String&, uint64_t, const String&, bool)>&&);
 
     WEBCORE_EXPORT void notifyObserver();
     WEBCORE_EXPORT void clearState();
@@ -80,10 +81,14 @@ private:
     void scheduleNotificationIfNeeded();
     Vector<ResourceLoadStatistics> takeStatistics();
 
+#if HAVE(CFNETWORK_STORAGE_PARTITIONING)
+    void requestStorageAccessUnderOpener(const String& domainInNeedOfStorageAccess, uint64_t openerPageID, Document& openerDocument, bool isTriggeredByUserGesture);
+#endif
+
     HashMap<String, ResourceLoadStatistics> m_resourceStatisticsMap;
     HashMap<String, WTF::WallTime> m_lastReportedUserInteractionMap;
     WTF::Function<void (Vector<ResourceLoadStatistics>&&)> m_notificationCallback;
-    WTF::Function<void(String, uint64_t, String)> m_grantStorageAccessUnderOpenerCallback;
+    WTF::Function<void(const String&, uint64_t, const String&, bool)> m_requestStorageAccessUnderOpenerCallback;
     Timer m_notificationTimer;
 #if HAVE(CFNETWORK_STORAGE_PARTITIONING) && !RELEASE_LOG_DISABLED
     uint64_t m_loggingCounter { 0 };
index a62e200..31fb95c 100644 (file)
@@ -80,6 +80,7 @@
 #include "Performance.h"
 #include "RequestAnimationFrameCallback.h"
 #include "ResourceLoadInfo.h"
+#include "ResourceLoadObserver.h"
 #include "RuntimeApplicationChecks.h"
 #include "RuntimeEnabledFeatures.h"
 #include "ScheduledAction.h"
@@ -2279,6 +2280,13 @@ RefPtr<Frame> DOMWindow::createWindow(const String& urlString, const AtomicStrin
         ResourceRequest resourceRequest { completedURL, referrer, UseProtocolCachePolicy };
         FrameLoadRequest frameLoadRequest { *activeWindow.document(), activeWindow.document()->securityOrigin(), resourceRequest, ASCIILiteral("_self"), LockHistory::No, LockBackForwardList::No, MaybeSendReferrer, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Allow, activeDocument->shouldOpenExternalURLsPolicyToPropagate(), initiatedByMainFrame };
         newFrame->loader().changeLocation(WTFMove(frameLoadRequest));
+
+#if HAVE(CFNETWORK_STORAGE_PARTITIONING)
+        if (auto openerDocument = openerFrame.document()) {
+            if (auto openerPageID = openerFrame.loader().client().pageID())
+                ResourceLoadObserver::shared().logWindowCreation(completedURL, openerPageID.value(), *openerDocument);
+        }
+#endif
     } else if (!urlString.isEmpty()) {
         LockHistory lockHistory = UserGestureIndicator::processingUserGesture() ? LockHistory::No : LockHistory::Yes;
         newFrame->navigationScheduler().scheduleLocationChange(*activeDocument, activeDocument->securityOrigin(), completedURL, referrer, lockHistory, LockBackForwardList::No);
index ae29651..6acf389 100644 (file)
@@ -1,3 +1,36 @@
+2018-03-13  John Wilander  <wilander@apple.com>
+
+        Resource Load Statistics: Immediately forward cookie access for domains with previous user interaction when there's an opener document
+        https://bugs.webkit.org/show_bug.cgi?id=183620
+        <rdar://problem/38431469>
+
+        Reviewed by Brent Fulgham.
+
+        It turns out the fix in https://bugs.webkit.org/show_bug.cgi?id=183577
+        wasn't enough to address the compatibility issues with popups. Some of
+        them just detect their unpartitioned cookies, auto-dismiss themselves,
+        and expect their unpartitioned cookies to be available under the opener
+        afterwards. We should grant them access if the popup's domain has had
+        user interaction _previously_.
+
+        Note that we still need https://bugs.webkit.org/show_bug.cgi?id=183577
+        because if the popup's domain has not received user interaction
+        previously, we will not grant it storage access on just the window open.
+
+        * UIProcess/WebResourceLoadStatisticsStore.cpp:
+        (WebKit::WebResourceLoadStatisticsStore::requestStorageAccessUnderOpener):
+        (WebKit::WebResourceLoadStatisticsStore::grantStorageAccessUnderOpener): Deleted.
+            Renamed WebResourceLoadStatisticsStore::grantStorageAccessUnderOpener()
+            to WebResourceLoadStatisticsStore::requestStorageAccessUnderOpener()
+            since there is now a case where access will not be granted, i.e. when
+            the popup domain has not had user interaction previously.
+        * UIProcess/WebResourceLoadStatisticsStore.h:
+        * UIProcess/WebResourceLoadStatisticsStore.messages.in:
+             Similar renaming.
+        * WebProcess/WebProcess.cpp:
+        (WebProcess::WebProcess):
+             Similar renaming.
+
 2018-03-13  Jer Noble  <jer.noble@apple.com>
 
         Add missing artwork for fullscreen mode.
index 3ba7842..5240677 100644 (file)
@@ -385,21 +385,28 @@ void WebResourceLoadStatisticsStore::requestStorageAccess(String&& subFrameHost,
     });
 }
 
-void WebResourceLoadStatisticsStore::grantStorageAccessUnderOpener(String&& domainReceivingUserInteraction, uint64_t openerPageID, String&& openerDomain)
+void WebResourceLoadStatisticsStore::requestStorageAccessUnderOpener(String&& domainInNeedOfStorageAccess, uint64_t openerPageID, String&& openerDomain, bool isTriggeredByUserGesture)
 {
-    ASSERT(domainReceivingUserInteraction != openerDomain);
+    ASSERT(domainInNeedOfStorageAccess != openerDomain);
     ASSERT(!RunLoop::isMain());
 
-    if (domainReceivingUserInteraction == openerDomain)
+    if (domainInNeedOfStorageAccess == openerDomain)
         return;
 
-    auto& domainReceivingUserInteractionStatistic = ensureResourceStatisticsForPrimaryDomain(domainReceivingUserInteraction);
-    if (!shouldPartitionCookies(domainReceivingUserInteractionStatistic) && !shouldBlockCookies(domainReceivingUserInteractionStatistic))
+    auto& domainInNeedOfStorageAccessStatistic = ensureResourceStatisticsForPrimaryDomain(domainInNeedOfStorageAccess);
+    auto cookiesBlocked = shouldBlockCookies(domainInNeedOfStorageAccessStatistic);
+
+    // There are no cookies to get access to if the domain has its cookies blocked and did not get user interaction now.
+    if (cookiesBlocked && !isTriggeredByUserGesture)
+        return;
+
+    // The domain already has access if its cookies are neither blocked nor partitioned.
+    if (!cookiesBlocked && !shouldPartitionCookies(domainInNeedOfStorageAccessStatistic))
         return;
 
-    m_grantStorageAccessHandler(WTFMove(domainReceivingUserInteraction), WTFMove(openerDomain), std::nullopt, openerPageID, [](bool) { });
+    m_grantStorageAccessHandler(WTFMove(domainInNeedOfStorageAccess), WTFMove(openerDomain), std::nullopt, openerPageID, [](bool) { });
 #if !RELEASE_LOG_DISABLED
-    RELEASE_LOG_INFO_IF(m_debugLoggingEnabled, ResourceLoadStatisticsDebug, "Grant storage access for %s under opener %s.", domainReceivingUserInteraction.utf8().data(), openerDomain.utf8().data());
+    RELEASE_LOG_INFO_IF(m_debugLoggingEnabled, ResourceLoadStatisticsDebug, "Grant storage access for %{public}s under opener %{public}s, %{public}s user interaction.", domainInNeedOfStorageAccess.utf8().data(), openerDomain.utf8().data(), (isTriggeredByUserGesture ? "with" : "without"));
 #endif
 }
 
index cfc91fa..91c225e 100644 (file)
@@ -85,7 +85,7 @@ public:
 
     void hasStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, WTF::CompletionHandler<void (bool)>&& callback);
     void requestStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, WTF::CompletionHandler<void (bool)>&& callback);
-    void grantStorageAccessUnderOpener(String&& domainReceivingUserInteraction, uint64_t openerPageID, String&& openerDomain);
+    void requestStorageAccessUnderOpener(String&& domainInNeedOfStorageAccess, uint64_t openerPageID, String&& openerDomain, bool isTriggeredByUserGesture);
     void requestStorageAccessCallback(bool wasGranted, uint64_t contextId);
 
     void processWillOpenConnection(WebProcessProxy&, IPC::Connection&);
index 0f5f0b8..57156c7 100644 (file)
@@ -21,6 +21,6 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 messages -> WebResourceLoadStatisticsStore {
-    GrantStorageAccessUnderOpener(String domainReceivingUserInteraction, uint64_t openerPageID, String openerDomain)
+    RequestStorageAccessUnderOpener(String domainReceivingUserInteraction, uint64_t openerPageID, String openerDomain, bool isTriggeredByUserGesture)
     ResourceLoadStatisticsUpdated(Vector<WebCore::ResourceLoadStatistics> origins)
 }
index 628f6ed..cc484cc 100644 (file)
@@ -206,8 +206,8 @@ WebProcess::WebProcess()
         parentProcessConnection()->send(Messages::WebResourceLoadStatisticsStore::ResourceLoadStatisticsUpdated(WTFMove(statistics)), 0);
     });
 
-    ResourceLoadObserver::shared().setGrantStorageAccessUnderOpenerCallback([this] (const String& domainReceivingUserInteraction, uint64_t openerPageID, const String& openerDomain) {
-        parentProcessConnection()->send(Messages::WebResourceLoadStatisticsStore::GrantStorageAccessUnderOpener(domainReceivingUserInteraction, openerPageID, openerDomain), 0);
+    ResourceLoadObserver::shared().setRequestStorageAccessUnderOpenerCallback([this] (const String& domainInNeedOfStorageAccess, uint64_t openerPageID, const String& openerDomain, bool isTriggeredByUserGesture) {
+        parentProcessConnection()->send(Messages::WebResourceLoadStatisticsStore::RequestStorageAccessUnderOpener(domainInNeedOfStorageAccess, openerPageID, openerDomain, isTriggeredByUserGesture), 0);
     });
     
     Gigacage::disableDisablingPrimitiveGigacageIfShouldBeEnabled();