Changeset 251019 in webkit

Timestamp:

Oct 11, 2019 1:26:15 PM (5 years ago)

Author:

Chris Dumez

Message:

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.

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/history/page-cache-iframe-js-url-expected.txt (added)
• LayoutTests/fast/history/page-cache-iframe-js-url.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/FrameLoader.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/loader/FrameLoader.cpp

@@ -396 +396 @@
     Ref<Frame> protect(m_frame);
 
-    if (m_frame.script().executeIfJavaScriptURL(frameRequest.resourceRequest().url(), frameRequest.shouldReplaceDocumentIfJavaScriptURL()))
-        return;
+    if (m_frame.script().executeIfJavaScriptURL(frameRequest.resourceRequest().url(), frameRequest.shouldReplaceDocumentIfJavaScriptURL())) {
+        m_quickRedirectComing = false;
+        return;
+    }
 
     if (frameRequest.frameName().isEmpty())
@@ -1848 +1850 @@
 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()) {
@@ -1865 +1863 @@
     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();
 }