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.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@251019 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index ba47bef..bb0f06b 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -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
index 0000000..b3d4b80
--- /dev/null
+++ b/LayoutTests/fast/history/page-cache-iframe-js-url-expected.txt
@@ -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
index 0000000..ccc6a53
--- /dev/null
+++ b/LayoutTests/fast/history/page-cache-iframe-js-url.html
@@ -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>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 20378d4..d8060e9 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -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
diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
index 8b869f7..ed7409c 100644
--- a/Source/WebCore/loader/FrameLoader.cpp
+++ b/Source/WebCore/loader/FrameLoader.cpp
@@ -395,8 +395,10 @@
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::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 @@
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()
