XMLHttpRequest sometimes prevents pages from entering the back/forward cache
https://bugs.webkit.org/show_bug.cgi?id=202434
<rdar://problem/55890340>
Reviewed by Geoffrey Garen.
Source/WebCore:
XMLHttpRequest::canSuspendForDocumentSuspension() was returning false if the document has not fired
the window's load event, with a comment explaining that cancelling the XHR in the upon suspension
may cause the load event to get fired and thus run script when forbidden. However, we should only
return false if the XMLHttpRequest is actually loading (m_loader is not null). XHRs that are not
loading should never prevent page caching.
I saw failures to enter the back/forward cache on yandex.ru and taobao.com because of this.
Test: http/tests/navigation/page-cache-failed-xhr.html
* xml/XMLHttpRequest.cpp:
(WebCore::XMLHttpRequest::canSuspendForDocumentSuspension const):
LayoutTests:
Add layout test coverage.
* http/tests/navigation/page-cache-failed-xhr-expected.txt: Added.
* http/tests/navigation/page-cache-failed-xhr.html: Added.
* http/tests/navigation/resources/page-cache-failed-xhr-frame.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@250678 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 88b8426..fd6277f 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2019-10-03 Chris Dumez <cdumez@apple.com>
+
+ XMLHttpRequest sometimes prevents pages from entering the back/forward cache
+ https://bugs.webkit.org/show_bug.cgi?id=202434
+ <rdar://problem/55890340>
+
+ Reviewed by Geoffrey Garen.
+
+ Add layout test coverage.
+
+ * http/tests/navigation/page-cache-failed-xhr-expected.txt: Added.
+ * http/tests/navigation/page-cache-failed-xhr.html: Added.
+ * http/tests/navigation/resources/page-cache-failed-xhr-frame.html: Added.
+
2019-10-03 John Wilander <wilander@apple.com>
Resource Load Statistics: Downgrade document.referrer for all third-party iframes
diff --git a/LayoutTests/http/tests/navigation/page-cache-failed-xhr-expected.txt b/LayoutTests/http/tests/navigation/page-cache-failed-xhr-expected.txt
new file mode 100644
index 0000000..fbe5933
--- /dev/null
+++ b/LayoutTests/http/tests/navigation/page-cache-failed-xhr-expected.txt
@@ -0,0 +1,13 @@
+Tests that a page with a failed XMLHttpRequest goes into 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/http/tests/navigation/page-cache-failed-xhr.html b/LayoutTests/http/tests/navigation/page-cache-failed-xhr.html
new file mode 100644
index 0000000..72d0bc9
--- /dev/null
+++ b/LayoutTests/http/tests/navigation/page-cache-failed-xhr.html
@@ -0,0 +1,47 @@
+<!-- webkit-test-runner [ enablePageCache=true ] -->
+<!DOCTYPE html>
+<html>
+<body>
+<script src="/js-test-resources/js-test.js"></script>
+<script>
+description('Tests that a page with a failed XMLHttpRequest goes into the page cache.');
+window.jsTestIsAsync = true;
+
+function iframeIsLoading()
+{
+ frames[0].stop();
+
+ setTimeout(function() {
+ // Force a back navigation back to this page.
+ window.location.href = "resources/page-cache-helper.html";
+ }, 0);
+}
+
+window.addEventListener("pageshow", function(event) {
+ debug("pageshow - " + (event.persisted ? "" : "not ") + "from cache");
+
+ if (event.persisted) {
+ testPassed("Page did enter and was restored from the page cache");
+ setTimeout(finishJSTest, 0);
+ }
+}, false);
+
+window.addEventListener("pagehide", function(event) {
+ debug("pagehide - " + (event.persisted ? "" : "not ") + "entering cache");
+ if (!event.persisted) {
+ testFailed("Page did not enter the page cache.");
+ finishJSTest();
+ }
+}, false);
+
+window.addEventListener('load', function() {
+ setTimeout(function() {
+ let iframe = document.createElement("iframe");
+ iframe.src = "resources/page-cache-failed-xhr-frame.html";
+ document.body.appendChild(iframe);
+ }, 0);
+}, false);
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/navigation/resources/page-cache-failed-xhr-frame.html b/LayoutTests/http/tests/navigation/resources/page-cache-failed-xhr-frame.html
new file mode 100644
index 0000000..7361861
--- /dev/null
+++ b/LayoutTests/http/tests/navigation/resources/page-cache-failed-xhr-frame.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script>
+xhr = new XMLHttpRequest();
+xhr.open("GET", "/resources/load-and-stall.cgi?name=load-and-stall.cgi&stallFor=3&stallAt=0&mimeType=text/plain", true);
+xhr.send();
+parent.iframeIsLoading();
+</script>
+<html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 9630122..9c0e831 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,24 @@
+2019-10-03 Chris Dumez <cdumez@apple.com>
+
+ XMLHttpRequest sometimes prevents pages from entering the back/forward cache
+ https://bugs.webkit.org/show_bug.cgi?id=202434
+ <rdar://problem/55890340>
+
+ Reviewed by Geoffrey Garen.
+
+ XMLHttpRequest::canSuspendForDocumentSuspension() was returning false if the document has not fired
+ the window's load event, with a comment explaining that cancelling the XHR in the upon suspension
+ may cause the load event to get fired and thus run script when forbidden. However, we should only
+ return false if the XMLHttpRequest is actually loading (m_loader is not null). XHRs that are not
+ loading should never prevent page caching.
+
+ I saw failures to enter the back/forward cache on yandex.ru and taobao.com because of this.
+
+ Test: http/tests/navigation/page-cache-failed-xhr.html
+
+ * xml/XMLHttpRequest.cpp:
+ (WebCore::XMLHttpRequest::canSuspendForDocumentSuspension const):
+
2019-10-03 John Wilander <wilander@apple.com>
Resource Load Statistics: Downgrade document.referrer for all third-party iframes
diff --git a/Source/WebCore/xml/XMLHttpRequest.cpp b/Source/WebCore/xml/XMLHttpRequest.cpp
index 1117bcd..7f0e035 100644
--- a/Source/WebCore/xml/XMLHttpRequest.cpp
+++ b/Source/WebCore/xml/XMLHttpRequest.cpp
@@ -1146,7 +1146,7 @@
// If the load event has not fired yet, cancelling the load in suspend() may cause
// the load event to be fired and arbitrary JS execution, which would be unsafe.
// Therefore, we prevent suspending in this case.
- return document()->loadEventFinished();
+ return !m_loader || document()->loadEventFinished();
}
const char* XMLHttpRequest::activeDOMObjectName() const
