Diff
Modified: trunk/LayoutTests/ChangeLog (181479 => 181480)
--- trunk/LayoutTests/ChangeLog 2015-03-13 16:00:03 UTC (rev 181479)
+++ trunk/LayoutTests/ChangeLog 2015-03-13 17:58:50 UTC (rev 181480)
@@ -1,3 +1,18 @@
+2015-03-13 Chris Dumez <[email protected]>
+
+ XMLHttpRequests should not prevent a page from entering PageCache
+ https://bugs.webkit.org/show_bug.cgi?id=142612
+ <rdar://problem/19923085>
+
+ Reviewed by Alexey Proskuryakov.
+
+ Add a tests to make sure that loading XMLHttpRequests do not prevent a
+ page from entering PageCache.
+
+ * http/tests/navigation/page-cache-xhr-expected.txt: Added.
+ * http/tests/navigation/page-cache-xhr.html: Added.
+ * http/tests/navigation/resources/page-cache-helper.html: Added.
+
2015-03-13 Marcos ChavarrÃa Teijeiro <[email protected]>
Unreviewed Gardening 13th March.
Added: trunk/LayoutTests/http/tests/navigation/page-cache-xhr-expected.txt (0 => 181480)
--- trunk/LayoutTests/http/tests/navigation/page-cache-xhr-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/navigation/page-cache-xhr-expected.txt 2015-03-13 17:58:50 UTC (rev 181480)
@@ -0,0 +1,15 @@
+Tests that a page with a loading 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 Executed the XHR error handler after restoring from page cache
+PASS xhr.status is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/http/tests/navigation/page-cache-xhr.html (0 => 181480)
--- trunk/LayoutTests/http/tests/navigation/page-cache-xhr.html (rev 0)
+++ trunk/LayoutTests/http/tests/navigation/page-cache-xhr.html 2015-03-13 17:58:50 UTC (rev 181480)
@@ -0,0 +1,66 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description('Tests that a page with a loading XMLHttpRequest goes into the page cache.');
+window.jsTestIsAsync = true;
+
+var restoredFromPageCache = false;
+
+if (window.testRunner)
+ testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+
+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");
+ restoredFromPageCache = true;
+ }
+}, 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);
+
+function xhrLoaded()
+{
+ testFailed("The XMLHttpRequest should not haved loaded");
+ finishJSTest();
+}
+
+function xhrError() {
+ if (restoredFromPageCache)
+ testPassed("Executed the XHR error handler after restoring from page cache");
+ else
+ testFailed("Executed the XHR error handler before restoring from page cache");
+
+ shouldBe("xhr.status", "0");
+ finishJSTest();
+}
+
+window.addEventListener('load', function() {
+ xhr = new XMLHttpRequest();
+ xhr._onload_ = xhrLoaded;
+ xhr._onerror_ = xhrError;
+ // Slow loading XHR (3-second stall).
+ xhr.open("GET", "/resources/load-and-stall.cgi?name=../../../http/tests/xmlhttprequest/timeout/xmlhttprequest-timeout.js&stallFor=3&stallAt=0&mimeType=text/plain", true);
+ xhr.send();
+
+ // This needs to happen in a setTimeout because a navigation inside the onload handler would
+ // not create a history entry.
+ setTimeout(function() {
+ // Force a back navigation back to this page.
+ window.location.href = ""
+ }, 0);
+}, false);
+
+</script>
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/navigation/resources/page-cache-helper.html (0 => 181480)
--- trunk/LayoutTests/http/tests/navigation/resources/page-cache-helper.html (rev 0)
+++ trunk/LayoutTests/http/tests/navigation/resources/page-cache-helper.html 2015-03-13 17:58:50 UTC (rev 181480)
@@ -0,0 +1,9 @@
+This page should go back. If a test outputs the contents of this
+page, then the test page failed to enter the page cache.
+<script>
+ window.addEventListener("load", function() {
+ setTimeout(function() {
+ history.back();
+ }, 0);
+ }, false);
+</script>
Modified: trunk/Source/WebCore/ChangeLog (181479 => 181480)
--- trunk/Source/WebCore/ChangeLog 2015-03-13 16:00:03 UTC (rev 181479)
+++ trunk/Source/WebCore/ChangeLog 2015-03-13 17:58:50 UTC (rev 181480)
@@ -1,3 +1,73 @@
+2015-03-13 Chris Dumez <[email protected]>
+
+ XMLHttpRequests should not prevent a page from entering PageCache
+ https://bugs.webkit.org/show_bug.cgi?id=142612
+ <rdar://problem/19923085>
+
+ Reviewed by Alexey Proskuryakov.
+
+ Make XMLHttpRequest ActiveDOMObjects suspendable in most cases to
+ drastically improve the likelihood of pages using them to enter
+ PageCache. XMLHttpRequest used to be only suspendable when not
+ loading. After this patch, if the XMLHttpRequest is loading when
+ navigating away from the page, it will be aborted and the page
+ will enter the PageCache. Upon restoring the page from PageCache,
+ the XMLHttpRequests' error handlers will be executed to give them
+ a chance to reload if they want to.
+
+ Test: http/tests/navigation/page-cache-xhr.html
+
+ * history/PageCache.cpp:
+ (WebCore::logCanCacheFrameDecision):
+ (WebCore::PageCache::canCachePageContainingThisFrame):
+ Do not prevent a page to enter the page cache ff the main document has
+ an error that is a cancellation and all remaining subresource loaders
+ are for XHR. We extend the pre-existing mechanism used on iOS, which
+ allowed PageCaching if the remaining resource loads are for images.
+
+ * loader/DocumentLoader.cpp:
+ (WebCore::areAllLoadersPageCacheAcceptable):
+ Mark XHR loaders as PageCache acceptable.
+
+ * loader/DocumentThreadableLoader.cpp:
+ (WebCore::DocumentThreadableLoader::isXMLHttpRequest):
+ * loader/DocumentThreadableLoader.h:
+ * loader/ThreadableLoader.h:
+ * loader/cache/CachedResource.cpp:
+ (WebCore::CachedResource::areAllClientsXMLHttpRequests):
+ * loader/cache/CachedResource.h:
+ * loader/cache/CachedResourceClient.h:
+ (WebCore::CachedResourceClient::isXMLHttpRequest):
+ * xml/XMLHttpRequest.cpp:
+ (WebCore::XMLHttpRequest::XMLHttpRequest):
+ (WebCore::XMLHttpRequest::createRequest):
+ (WebCore::XMLHttpRequest::canSuspend):
+ Report that we can suspend XMLHttpRequests as long as the window load
+ event has already fired. If the window load event has not fired yet,
+ it would be unsafe to cancel the load in suspend() as it would
+ potentially cause arbitrary JS execution while suspending.
+
+ (WebCore::XMLHttpRequest::suspend):
+ If suspending for PageCache and the request is currently loading, abort
+ the load and mark that we should fire the error event upon restoring
+ from PageCache.
+
+ (WebCore::XMLHttpRequest::resume):
+ (WebCore::XMLHttpRequest::resumeTimerFired):
+ Upon resuming, fire the error event in a timer if the load was aborted
+ for suspending. We need to do this in a timer because we are not allowed
+ to execute arbitrary JS inside resume().
+
+ (WebCore::XMLHttpRequest::stop):
+ Add a assertion to make sure we are not firing event inside stop() as
+ this would potentially cause arbitrary JS execution and it would be
+ unsafe. It seems to me that our code is currently unsafe but the
+ assertion does not seem to be hit by our current layout tests. I am
+ adding the assertion as it would make it clear we have a bug and we
+ need to fix it.
+
+ * xml/XMLHttpRequest.h:
+
2015-03-13 Joonghun Park <[email protected]>
Fix Debug build error 'comparison is always true due to limited range of data type [-Werror=type-limits]'
Modified: trunk/Source/WebCore/history/PageCache.cpp (181479 => 181480)
--- trunk/Source/WebCore/history/PageCache.cpp 2015-03-13 16:00:03 UTC (rev 181479)
+++ trunk/Source/WebCore/history/PageCache.cpp 2015-03-13 17:58:50 UTC (rev 181480)
@@ -128,14 +128,11 @@
if (!frame.loader().documentLoader()->mainDocumentError().isNull()) {
PCLOG(" -Main document has an error");
logPageCacheFailureDiagnosticMessage(diagnosticLoggingClient, DiagnosticLoggingKeys::mainDocumentErrorKey());
-#if !PLATFORM(IOS)
- rejectReasons |= 1 << MainDocumentError;
-#else
+
if (frame.loader().documentLoader()->mainDocumentError().isCancellation() && frame.loader().documentLoader()->subresourceLoadersArePageCacheAcceptable())
- PCLOG(" -But, it was a cancellation and all loaders during the cancel were loading images.");
+ PCLOG(" -But, it was a cancellation and all loaders during the cancelation were loading images or XHR.");
else
rejectReasons |= 1 << MainDocumentError;
-#endif
}
if (frame.loader().documentLoader()->substituteData().isValid() && frame.loader().documentLoader()->substituteData().failingURL().isEmpty()) {
PCLOG(" -Frame is an error page");
@@ -304,11 +301,7 @@
Document* document = frame.document();
return documentLoader
-#if !PLATFORM(IOS)
- && documentLoader->mainDocumentError().isNull()
-#else
&& (documentLoader->mainDocumentError().isNull() || (documentLoader->mainDocumentError().isCancellation() && documentLoader->subresourceLoadersArePageCacheAcceptable()))
-#endif
// Do not cache error pages (these can be recognized as pages with substitute data or unreachable URLs).
&& !(documentLoader->substituteData().isValid() && !documentLoader->substituteData().failingURL().isEmpty())
&& (!frameLoader.subframeLoader().containsPlugins() || frame.page()->settings().pageCacheSupportsPlugins())
Modified: trunk/Source/WebCore/loader/DocumentLoader.cpp (181479 => 181480)
--- trunk/Source/WebCore/loader/DocumentLoader.cpp 2015-03-13 16:00:03 UTC (rev 181479)
+++ trunk/Source/WebCore/loader/DocumentLoader.cpp 2015-03-13 17:58:50 UTC (rev 181480)
@@ -99,19 +99,16 @@
Vector<RefPtr<ResourceLoader>> loadersCopy;
copyValuesToVector(loaders, loadersCopy);
for (auto& loader : loadersCopy) {
- ResourceHandle* handle = loader->handle();
- if (!handle)
- return false;
-
if (!loader->frameLoader())
return false;
- CachedResource* cachedResource = MemoryCache::singleton().resourceForURL(handle->firstRequest().url(), loader->frameLoader()->frame().page()->sessionID());
+ CachedResource* cachedResource = MemoryCache::singleton().resourceForURL(loader->request().url(), loader->frameLoader()->frame().page()->sessionID());
if (!cachedResource)
return false;
+ // Only image and XHR loads do prevent the page from entering the PageCache.
// All non-image loads will prevent the page from entering the PageCache.
- if (!cachedResource->isImage())
+ if (!cachedResource->isImage() && !cachedResource->areAllClientsXMLHttpRequests())
return false;
}
return true;
Modified: trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp (181479 => 181480)
--- trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp 2015-03-13 16:00:03 UTC (rev 181479)
+++ trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp 2015-03-13 17:58:50 UTC (rev 181480)
@@ -34,6 +34,7 @@
#include "CachedRawResource.h"
#include "CachedResourceLoader.h"
#include "CachedResourceRequest.h"
+#include "CachedResourceRequestInitiators.h"
#include "CrossOriginAccessControl.h"
#include "CrossOriginPreflightResultCache.h"
#include "Document.h"
@@ -418,6 +419,11 @@
return m_sameOriginRequest && securityOrigin()->canRequest(url);
}
+bool DocumentThreadableLoader::isXMLHttpRequest() const
+{
+ return m_options.initiator == cachedResourceRequestInitiators().xmlhttprequest;
+}
+
SecurityOrigin* DocumentThreadableLoader::securityOrigin() const
{
return m_options.securityOrigin ? m_options.securityOrigin.get() : m_document.securityOrigin();
Modified: trunk/Source/WebCore/loader/DocumentThreadableLoader.h (181479 => 181480)
--- trunk/Source/WebCore/loader/DocumentThreadableLoader.h 2015-03-13 16:00:03 UTC (rev 181479)
+++ trunk/Source/WebCore/loader/DocumentThreadableLoader.h 2015-03-13 17:58:50 UTC (rev 181480)
@@ -96,6 +96,8 @@
void loadRequest(const ResourceRequest&, SecurityCheckPolicy);
bool isAllowedRedirect(const URL&);
+ bool isXMLHttpRequest() const override final;
+
SecurityOrigin* securityOrigin() const;
CachedResourceHandle<CachedRawResource> m_resource;
Modified: trunk/Source/WebCore/loader/ThreadableLoader.h (181479 => 181480)
--- trunk/Source/WebCore/loader/ThreadableLoader.h 2015-03-13 16:00:03 UTC (rev 181479)
+++ trunk/Source/WebCore/loader/ThreadableLoader.h 2015-03-13 17:58:50 UTC (rev 181480)
@@ -36,10 +36,7 @@
#include <wtf/PassRefPtr.h>
#include <wtf/RefPtr.h>
#include <wtf/Vector.h>
-
-#if ENABLE(RESOURCE_TIMING)
#include <wtf/text/AtomicString.h>
-#endif
namespace WebCore {
@@ -69,9 +66,7 @@
PreflightPolicy preflightPolicy; // If AccessControl is used, how to determine if a preflight is needed.
CrossOriginRequestPolicy crossOriginRequestPolicy;
RefPtr<SecurityOrigin> securityOrigin;
-#if ENABLE(RESOURCE_TIMING)
AtomicString initiator;
-#endif
};
// Useful for doing loader operations from any thread (not threadsafe,
Modified: trunk/Source/WebCore/loader/cache/CachedResource.cpp (181479 => 181480)
--- trunk/Source/WebCore/loader/cache/CachedResource.cpp 2015-03-13 16:00:03 UTC (rev 181479)
+++ trunk/Source/WebCore/loader/cache/CachedResource.cpp 2015-03-13 17:58:50 UTC (rev 181480)
@@ -735,6 +735,18 @@
return sizeof(CachedResource) + m_response.memoryUsage() + kAverageClientsHashMapSize + m_resourceRequest.url().string().length() * 2;
}
+bool CachedResource::areAllClientsXMLHttpRequests() const
+{
+ if (type() != RawResource)
+ return false;
+
+ for (auto& client : m_clients) {
+ if (!client.key->isXMLHttpRequest())
+ return false;
+ }
+ return true;
+}
+
void CachedResource::setLoadPriority(const Optional<ResourceLoadPriority>& loadPriority)
{
if (loadPriority)
Modified: trunk/Source/WebCore/loader/cache/CachedResource.h (181479 => 181480)
--- trunk/Source/WebCore/loader/cache/CachedResource.h 2015-03-13 16:00:03 UTC (rev 181479)
+++ trunk/Source/WebCore/loader/cache/CachedResource.h 2015-03-13 17:58:50 UTC (rev 181480)
@@ -158,6 +158,8 @@
SubresourceLoader* loader() { return m_loader.get(); }
+ bool areAllClientsXMLHttpRequests() const;
+
bool isImage() const { return type() == ImageResource; }
// FIXME: CachedRawResource could be either a main resource or a raw XHR resource.
bool isMainOrRawResource() const { return type() == MainResource || type() == RawResource; }
Modified: trunk/Source/WebCore/loader/cache/CachedResourceClient.h (181479 => 181480)
--- trunk/Source/WebCore/loader/cache/CachedResourceClient.h 2015-03-13 16:00:03 UTC (rev 181479)
+++ trunk/Source/WebCore/loader/cache/CachedResourceClient.h 2015-03-13 17:58:50 UTC (rev 181480)
@@ -43,6 +43,7 @@
virtual ~CachedResourceClient() { }
virtual void notifyFinished(CachedResource*) { }
virtual void deprecatedDidReceiveCachedResource(CachedResource*) { }
+ virtual bool isXMLHttpRequest() const { return false; }
static CachedResourceClientType expectedType() { return BaseResourceType; }
virtual CachedResourceClientType resourceClientType() const { return expectedType(); }
Modified: trunk/Source/WebCore/xml/XMLHttpRequest.cpp (181479 => 181480)
--- trunk/Source/WebCore/xml/XMLHttpRequest.cpp 2015-03-13 16:00:03 UTC (rev 181479)
+++ trunk/Source/WebCore/xml/XMLHttpRequest.cpp 2015-03-13 17:58:50 UTC (rev 181480)
@@ -24,6 +24,7 @@
#include "XMLHttpRequest.h"
#include "Blob.h"
+#include "CachedResourceRequestInitiators.h"
#include "ContentSecurityPolicy.h"
#include "CrossOriginAccessControl.h"
#include "DOMFormData.h"
@@ -64,10 +65,6 @@
#include <wtf/StdLibExtras.h>
#include <wtf/text/CString.h>
-#if ENABLE(RESOURCE_TIMING)
-#include "CachedResourceRequestInitiators.h"
-#endif
-
namespace WebCore {
DEFINE_DEBUG_ONLY_GLOBAL(WTF::RefCountedLeakCounter, xmlHttpRequestCounter, ("XMLHttpRequest"));
@@ -139,6 +136,8 @@
, m_progressEventThrottle(this)
, m_responseTypeCode(ResponseTypeDefault)
, m_responseCacheIsValid(false)
+ , m_resumeTimer(*this, &XMLHttpRequest::resumeTimerFired)
+ , m_dispatchErrorOnResuming(false)
{
#ifndef NDEBUG
xmlHttpRequestCounter.increment();
@@ -764,9 +763,7 @@
options.setAllowCredentials((m_sameOriginRequest || m_includeCredentials) ? AllowStoredCredentials : DoNotAllowStoredCredentials);
options.crossOriginRequestPolicy = UseAccessControl;
options.securityOrigin = securityOrigin();
-#if ENABLE(RESOURCE_TIMING)
options.initiator = cachedResourceRequestInitiators().xmlhttprequest;
-#endif
#if ENABLE(XHR_TIMEOUT)
if (m_timeoutMilliseconds)
@@ -1255,7 +1252,10 @@
bool XMLHttpRequest::canSuspend() const
{
- return !m_loader;
+ // 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();
}
const char* XMLHttpRequest::activeDOMObjectName() const
@@ -1263,18 +1263,50 @@
return "XMLHttpRequest";
}
-void XMLHttpRequest::suspend(ReasonForSuspension)
+void XMLHttpRequest::suspend(ReasonForSuspension reason)
{
+ NoEventDispatchAssertion assertNoEventDispatch;
+
m_progressEventThrottle.suspend();
+
+ if (m_resumeTimer.isActive()) {
+ m_resumeTimer.stop();
+ m_dispatchErrorOnResuming = true;
+ }
+
+ if (reason == ActiveDOMObject::DocumentWillBecomeInactive && m_loader) {
+ // Going into PageCache, abort the request and dispatch a network error on resuming.
+ genericError();
+ m_dispatchErrorOnResuming = true;
+ bool aborted = internalAbort();
+ // It should not be possible to restart the load when aborting in suspend() because
+ // we are not allowed to execute in JS in suspend().
+ ASSERT_UNUSED(aborted, aborted);
+ }
}
void XMLHttpRequest::resume()
{
+ NoEventDispatchAssertion assertNoEventDispatch;
+
m_progressEventThrottle.resume();
+
+ // We are not allowed to execute arbitrary JS in resume() so dispatch
+ // the error event in a timer.
+ if (m_dispatchErrorOnResuming && !m_resumeTimer.isActive())
+ m_resumeTimer.startOneShot(0);
}
+void XMLHttpRequest::resumeTimerFired()
+{
+ ASSERT(m_dispatchErrorOnResuming);
+ m_dispatchErrorOnResuming = false;
+ dispatchErrorEvents(eventNames().errorEvent);
+}
+
void XMLHttpRequest::stop()
{
+ NoEventDispatchAssertion assertNoEventDispatch;
internalAbort();
}
Modified: trunk/Source/WebCore/xml/XMLHttpRequest.h (181479 => 181480)
--- trunk/Source/WebCore/xml/XMLHttpRequest.h 2015-03-13 16:00:03 UTC (rev 181479)
+++ trunk/Source/WebCore/xml/XMLHttpRequest.h 2015-03-13 17:58:50 UTC (rev 181480)
@@ -205,6 +205,8 @@
void dispatchErrorEvents(const AtomicString&);
+ void resumeTimerFired();
+
std::unique_ptr<XMLHttpRequestUpload> m_upload;
URL m_url;
@@ -254,6 +256,9 @@
// An enum corresponding to the allowed string values for the responseType attribute.
ResponseTypeCode m_responseTypeCode;
bool m_responseCacheIsValid;
+
+ Timer m_resumeTimer;
+ bool m_dispatchErrorOnResuming;
};
} // namespace WebCore
