Pages frequently fail to enter the back/forward cache due to frames with a quick...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Oct 2019 20:26:15 +0000 (20:26 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Oct 2019 20:26:15 +0000 (20:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=202768
<rdar://problem/56132022>

Reviewed by Alex Christensen.

Source/WebCore:

When a quick redirect is scheduled with the navigation scheduler, we set the m_quickRedirectComing flag.
This flag is supposed to get unset if the navigation gets cancelled and when the navigation actually
happens. However, for a navigation to a javascript: URL, we would return early after executing the JS
and fail to reset the m_quickRedirectComing flag. Later on, we would fail to enter the page cache because
we would think that the iframe still has a quick redirect scheduled.

Test: fast/history/page-cache-iframe-js-url.html

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::urlSelected):
Reset the m_quickRedirectComing flag if we return early after executing
the javascript URL.

(WebCore::FrameLoader::stopForPageCache):
Stop policy checks & cancel scheduled navigations after stopping loads. Stopping loads may
run script which may in theory schedule new navigations. This is hardening.

LayoutTests:

Add lauout test coverage.

* fast/history/page-cache-iframe-js-url-expected.txt: Added.
* fast/history/page-cache-iframe-js-url.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/history/page-cache-iframe-js-url-expected.txt [new file with mode: 0644]
LayoutTests/fast/history/page-cache-iframe-js-url.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/FrameLoader.cpp

index ba47bef..bb0f06b 100644 (file)
@@ -1,3 +1,16 @@
+2019-10-11  Chris Dumez  <cdumez@apple.com>
+
+        Pages frequently fail to enter the back/forward cache due to frames with a quick redirect coming
+        https://bugs.webkit.org/show_bug.cgi?id=202768
+        <rdar://problem/56132022>
+
+        Reviewed by Alex Christensen.
+
+        Add lauout test coverage.
+
+        * fast/history/page-cache-iframe-js-url-expected.txt: Added.
+        * fast/history/page-cache-iframe-js-url.html: Added.
+
 2019-10-11  Kate Cheney  <katherine_cheney@apple.com>
 
         Get StorageAccess API features working on SQLite database implementation (195422)
diff --git a/LayoutTests/fast/history/page-cache-iframe-js-url-expected.txt b/LayoutTests/fast/history/page-cache-iframe-js-url-expected.txt
new file mode 100644 (file)
index 0000000..b3d4b80
--- /dev/null
@@ -0,0 +1,13 @@
+Tests that a page with an iframe that ran a JavaScript URL is able to enter the page cache.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+pageshow - not from cache
+pagehide - entering cache
+pageshow - from cache
+PASS - Page did enter and was restored from the page cache
+PASS successfullyParsed is true
+
+TEST COMPLETE
+  
diff --git a/LayoutTests/fast/history/page-cache-iframe-js-url.html b/LayoutTests/fast/history/page-cache-iframe-js-url.html
new file mode 100644 (file)
index 0000000..ccc6a53
--- /dev/null
@@ -0,0 +1,41 @@
+<!-- webkit-test-runner [ enablePageCache=true ] -->
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test.js"></script>
+</head>
+<body>
+<img src="../dom/resources/abe.png">
+<a id="testLink" style="display: none" href="resources/page-cache-helper.html">Link</a>
+<iframe id="testFrame" src="about:blank"></iframe>
+<script>
+description('Tests that a page with an iframe that ran a JavaScript URL is able to enter the page cache.');
+jsTestIsAsync = true;
+
+window.addEventListener("pageshow", function(event) {
+    debug("pageshow - " + (event.persisted ? "" : "not ") + "from cache");
+    if (event.persisted) {
+        debug("PASS - Page did enter and was restored from the page cache");
+        finishJSTest();
+    }
+});
+
+window.addEventListener("pagehide", function(event) {
+    debug("pagehide - " + (event.persisted ? "" : "not ") + "entering cache");
+    if (!event.persisted) {
+        debug("FAIL - Page did not enter the page cache.");
+        finishJSTest();
+    }
+});
+
+window.addEventListener('load', function() {
+    setTimeout(() => {
+        document.getElementById("testFrame").contentWindow.location.replace("javascript:'foo';");
+        setTimeout(() => {
+            testLink.click();
+        }, 0);
+    }, 0);
+});
+</script>
+</body>
+</html>
index 20378d4..d8060e9 100644 (file)
@@ -1,3 +1,28 @@
+2019-10-11  Chris Dumez  <cdumez@apple.com>
+
+        Pages frequently fail to enter the back/forward cache due to frames with a quick redirect coming
+        https://bugs.webkit.org/show_bug.cgi?id=202768
+        <rdar://problem/56132022>
+
+        Reviewed by Alex Christensen.
+
+        When a quick redirect is scheduled with the navigation scheduler, we set the m_quickRedirectComing flag.
+        This flag is supposed to get unset if the navigation gets cancelled and when the navigation actually
+        happens. However, for a navigation to a javascript: URL, we would return early after executing the JS
+        and fail to reset the m_quickRedirectComing flag. Later on, we would fail to enter the page cache because
+        we would think that the iframe still has a quick redirect scheduled.
+
+        Test: fast/history/page-cache-iframe-js-url.html
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::urlSelected):
+        Reset the m_quickRedirectComing flag if we return early after executing
+        the javascript URL.
+
+        (WebCore::FrameLoader::stopForPageCache):
+        Stop policy checks & cancel scheduled navigations after stopping loads. Stopping loads may
+        run script which may in theory schedule new navigations. This is hardening.
+
 2019-10-11  Antti Koivisto  <antti@apple.com>
 
         Position::upstream/downstream should not need to call ensureLineBoxes
index 8b869f7..ed7409c 100644 (file)
@@ -395,8 +395,10 @@ void FrameLoader::urlSelected(FrameLoadRequest&& frameRequest, Event* triggering
 
     Ref<Frame> protect(m_frame);
 
-    if (m_frame.script().executeIfJavaScriptURL(frameRequest.resourceRequest().url(), frameRequest.shouldReplaceDocumentIfJavaScriptURL()))
+    if (m_frame.script().executeIfJavaScriptURL(frameRequest.resourceRequest().url(), frameRequest.shouldReplaceDocumentIfJavaScriptURL())) {
+        m_quickRedirectComing = false;
         return;
+    }
 
     if (frameRequest.frameName().isEmpty())
         frameRequest.setFrameName(m_frame.document()->baseTarget());
@@ -1847,10 +1849,6 @@ void FrameLoader::stopAllLoaders(ClearProvisionalItemPolicy clearProvisionalItem
 
 void FrameLoader::stopForPageCache()
 {
-    // Make sure there are no scheduled loads or policy checks.
-    policyChecker().stopCheck();
-    m_frame.navigationScheduler().cancel();
-
     // Stop provisional loads in subframes (The one in the main frame is about to be committed).
     if (!m_frame.isMainFrame()) {
         if (m_provisionalDocumentLoader)
@@ -1864,6 +1862,10 @@ void FrameLoader::stopForPageCache()
 
     for (RefPtr<Frame> child = m_frame.tree().firstChild(); child; child = child->tree().nextSibling())
         child->loader().stopForPageCache();
+
+    // Make sure there are no scheduled loads or policy checks.
+    policyChecker().stopCheck();
+    m_frame.navigationScheduler().cancel();
 }
 
 void FrameLoader::stopAllLoadersAndCheckCompleteness()