- Revision
- 248265
- Author
- [email protected]
- Date
- 2019-08-04 11:32:44 -0700 (Sun, 04 Aug 2019)
Log Message
Ping loads should not prevent page caching
https://bugs.webkit.org/show_bug.cgi?id=200418
<rdar://problem/53901632>
Reviewed by Darin Adler.
Source/WebCore:
We normally prevent page caching if there were any pending subresource loads when navigating,
to avoid caching partial / broken content. However, this should not apply to Ping / Beacon
loads since those do not impact page rendering and can outlive the page.
Tests: http/tests/navigation/page-cache-pending-ping-load-cross-origin.html
http/tests/navigation/page-cache-pending-ping-load-same-origin.html
* history/PageCache.cpp:
(WebCore::PageCache::addIfCacheable):
After we've fired the 'pagehide' event in each frame, stop all the loads again. This is needed
since pages are allowed to start ping / beacon loads in their 'pagehide' handlers. If we do not
stop those loads, then the next call to canCachePage() would fail because the DocumentLoader is
still loading. Note that we're not actually preventing these ping loads from hitting the server
since we never cancel page loads and those can outlive their page.
* loader/DocumentLoader.cpp:
(WebCore::shouldPendingCachedResourceLoadPreventPageCache):
(WebCore::areAllLoadersPageCacheAcceptable):
Make sure that Ping / Beacon / Prefetches / Icon loads do not prevent page caching.
(WebCore::DocumentLoader::addSubresourceLoader):
Tweak assertion that was incorrect since we actually allow ping / beacon loads when the
document is about to enter PageCache (while firing pagehide event).
Tools:
Add TestOption to enable PageCache at UIProcess-level so that we can test
page caching when navigating cross-origin with PSON enabled.
* WebKitTestRunner/TestController.cpp:
(WTR::TestController::resetPreferencesToConsistentValues):
(WTR::updateTestOptionsFromTestHeader):
* WebKitTestRunner/TestOptions.h:
(WTR::TestOptions::hasSameInitializationOptions const):
LayoutTests:
Add layout test coverage.
* http/tests/navigation/page-cache-pending-ping-load-cross-origin-expected.txt: Added.
* http/tests/navigation/page-cache-pending-ping-load-cross-origin.html: Added.
* http/tests/navigation/page-cache-pending-ping-load-same-origin-expected.txt: Added.
* http/tests/navigation/page-cache-pending-ping-load-same-origin.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (248264 => 248265)
--- trunk/LayoutTests/ChangeLog 2019-08-04 18:28:45 UTC (rev 248264)
+++ trunk/LayoutTests/ChangeLog 2019-08-04 18:32:44 UTC (rev 248265)
@@ -1,3 +1,18 @@
+2019-08-04 Chris Dumez <[email protected]>
+
+ Ping loads should not prevent page caching
+ https://bugs.webkit.org/show_bug.cgi?id=200418
+ <rdar://problem/53901632>
+
+ Reviewed by Darin Adler.
+
+ Add layout test coverage.
+
+ * http/tests/navigation/page-cache-pending-ping-load-cross-origin-expected.txt: Added.
+ * http/tests/navigation/page-cache-pending-ping-load-cross-origin.html: Added.
+ * http/tests/navigation/page-cache-pending-ping-load-same-origin-expected.txt: Added.
+ * http/tests/navigation/page-cache-pending-ping-load-same-origin.html: Added.
+
2019-08-03 Devin Rousso <[email protected]>
Web Inspector: Elements: Styles: add icons for various CSS rule types
Added: trunk/LayoutTests/http/tests/navigation/page-cache-pending-ping-load-cross-origin-expected.txt (0 => 248265)
--- trunk/LayoutTests/http/tests/navigation/page-cache-pending-ping-load-cross-origin-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/navigation/page-cache-pending-ping-load-cross-origin-expected.txt 2019-08-04 18:32:44 UTC (rev 248265)
@@ -0,0 +1,13 @@
+Tests that a page with pending ping loads can enter PageCache.
+
+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
+Click me
Added: trunk/LayoutTests/http/tests/navigation/page-cache-pending-ping-load-cross-origin.html (0 => 248265)
--- trunk/LayoutTests/http/tests/navigation/page-cache-pending-ping-load-cross-origin.html (rev 0)
+++ trunk/LayoutTests/http/tests/navigation/page-cache-pending-ping-load-cross-origin.html 2019-08-04 18:32:44 UTC (rev 248265)
@@ -0,0 +1,46 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enablePageCache=true ] -->
+<html>
+<body>
+<script src=""
+<script>
+description('Tests that a page with pending ping loads can enter PageCache.');
+window.jsTestIsAsync = true;
+
+if (window.testRunner)
+ testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+
+let afterPageCacheRestore = false;
+
+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");
+ finishJSTest();
+ }
+}, 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();
+ }
+
+ testPing2.click();
+}, false);
+
+window.addEventListener('load', function() {
+ testPing1.click();
+ setTimeout(function() {
+ testLink.click();
+ }, 0);
+}, false);
+
+</script>
+<a id="testPing1" href="" ping="/resources/load-and-stall.cgi?name=load-and-stall.cgi&stallFor=3&stallAt=0&mimeType=text/plain&bar" style="display: none;"></a>
+<a id="testPing2" href="" ping="/resources/load-and-stall.cgi?name=load-and-stall.cgi&stallFor=3&stallAt=0&mimeType=text/plain&baz" style="display: none;"></a>
+<a id="testLink" href="" ping="/resources/load-and-stall.cgi?name=load-and-stall.cgi&stallFor=3&stallAt=0&mimeType=text/plain&foo">Click me</a>
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/navigation/page-cache-pending-ping-load-same-origin-expected.txt (0 => 248265)
--- trunk/LayoutTests/http/tests/navigation/page-cache-pending-ping-load-same-origin-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/navigation/page-cache-pending-ping-load-same-origin-expected.txt 2019-08-04 18:32:44 UTC (rev 248265)
@@ -0,0 +1,13 @@
+Tests that a page with pending ping loads can enter PageCache.
+
+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
+Click me
Added: trunk/LayoutTests/http/tests/navigation/page-cache-pending-ping-load-same-origin.html (0 => 248265)
--- trunk/LayoutTests/http/tests/navigation/page-cache-pending-ping-load-same-origin.html (rev 0)
+++ trunk/LayoutTests/http/tests/navigation/page-cache-pending-ping-load-same-origin.html 2019-08-04 18:32:44 UTC (rev 248265)
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description('Tests that a page with pending ping loads can enter PageCache.');
+window.jsTestIsAsync = true;
+
+if (window.testRunner)
+ testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+
+let afterPageCacheRestore = false;
+
+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");
+ finishJSTest();
+ }
+}, 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();
+ }
+
+ testPing2.click();
+}, false);
+
+window.addEventListener('load', function() {
+ testPing1.click();
+ setTimeout(function() {
+ testLink.click();
+ }, 0);
+}, false);
+
+</script>
+<a id="testPing1" href="" ping="/resources/load-and-stall.cgi?name=load-and-stall.cgi&stallFor=3&stallAt=0&mimeType=text/plain&bar" style="display: none;"></a>
+<a id="testPing2" href="" ping="/resources/load-and-stall.cgi?name=load-and-stall.cgi&stallFor=3&stallAt=0&mimeType=text/plain&baz" style="display: none;"></a>
+<a id="testLink" href="" ping="/resources/load-and-stall.cgi?name=load-and-stall.cgi&stallFor=3&stallAt=0&mimeType=text/plain&foo">Click me</a>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (248264 => 248265)
--- trunk/Source/WebCore/ChangeLog 2019-08-04 18:28:45 UTC (rev 248264)
+++ trunk/Source/WebCore/ChangeLog 2019-08-04 18:32:44 UTC (rev 248265)
@@ -1,3 +1,35 @@
+2019-08-04 Chris Dumez <[email protected]>
+
+ Ping loads should not prevent page caching
+ https://bugs.webkit.org/show_bug.cgi?id=200418
+ <rdar://problem/53901632>
+
+ Reviewed by Darin Adler.
+
+ We normally prevent page caching if there were any pending subresource loads when navigating,
+ to avoid caching partial / broken content. However, this should not apply to Ping / Beacon
+ loads since those do not impact page rendering and can outlive the page.
+
+ Tests: http/tests/navigation/page-cache-pending-ping-load-cross-origin.html
+ http/tests/navigation/page-cache-pending-ping-load-same-origin.html
+
+ * history/PageCache.cpp:
+ (WebCore::PageCache::addIfCacheable):
+ After we've fired the 'pagehide' event in each frame, stop all the loads again. This is needed
+ since pages are allowed to start ping / beacon loads in their 'pagehide' handlers. If we do not
+ stop those loads, then the next call to canCachePage() would fail because the DocumentLoader is
+ still loading. Note that we're not actually preventing these ping loads from hitting the server
+ since we never cancel page loads and those can outlive their page.
+
+ * loader/DocumentLoader.cpp:
+ (WebCore::shouldPendingCachedResourceLoadPreventPageCache):
+ (WebCore::areAllLoadersPageCacheAcceptable):
+ Make sure that Ping / Beacon / Prefetches / Icon loads do not prevent page caching.
+
+ (WebCore::DocumentLoader::addSubresourceLoader):
+ Tweak assertion that was incorrect since we actually allow ping / beacon loads when the
+ document is about to enter PageCache (while firing pagehide event).
+
2019-08-04 Zalan Bujtas <[email protected]>
[LFC][TFC] Create formatting context/state.
Modified: trunk/Source/WebCore/history/PageCache.cpp (248264 => 248265)
--- trunk/Source/WebCore/history/PageCache.cpp 2019-08-04 18:28:45 UTC (rev 248264)
+++ trunk/Source/WebCore/history/PageCache.cpp 2019-08-04 18:32:44 UTC (rev 248265)
@@ -458,6 +458,13 @@
destroyRenderTree(page->mainFrame());
+ // Stop all loads again before checking if we can still cache the page after firing the pagehide
+ // event, since the page may have started ping loads in its pagehide event handler.
+ for (Frame* frame = &page->mainFrame(); frame; frame = frame->tree().traverseNext()) {
+ if (auto* documentLoader = frame->loader().documentLoader())
+ documentLoader->stopLoading();
+ }
+
// Check that the page is still page-cacheable after firing the pagehide event. The JS event handlers
// could have altered the page in a way that could prevent caching.
if (!canCache(*page)) {
Modified: trunk/Source/WebCore/loader/DocumentLoader.cpp (248264 => 248265)
--- trunk/Source/WebCore/loader/DocumentLoader.cpp 2019-08-04 18:28:45 UTC (rev 248264)
+++ trunk/Source/WebCore/loader/DocumentLoader.cpp 2019-08-04 18:32:44 UTC (rev 248265)
@@ -125,6 +125,42 @@
loader->setDefersLoading(defers);
}
+static bool shouldPendingCachedResourceLoadPreventPageCache(CachedResource& cachedResource)
+{
+ if (!cachedResource.isLoading())
+ return false;
+
+ switch (cachedResource.type()) {
+ case CachedResource::Type::ImageResource:
+ case CachedResource::Type::Icon:
+ case CachedResource::Type::Beacon:
+ case CachedResource::Type::Ping:
+ case CachedResource::Type::LinkPrefetch:
+ return false;
+ case CachedResource::Type::MainResource:
+ case CachedResource::Type::CSSStyleSheet:
+ case CachedResource::Type::Script:
+ case CachedResource::Type::FontResource:
+#if ENABLE(SVG_FONTS)
+ case CachedResource::Type::SVGFontResource:
+#endif
+ case CachedResource::Type::MediaResource:
+ case CachedResource::Type::RawResource:
+ case CachedResource::Type::SVGDocumentResource:
+#if ENABLE(XSLT)
+ case CachedResource::Type::XSLStyleSheet:
+#endif
+#if ENABLE(VIDEO_TRACK)
+ case CachedResource::Type::TextTrackResource:
+#endif
+#if ENABLE(APPLICATION_MANIFEST)
+ case CachedResource::Type::ApplicationManifest:
+#endif
+ break;
+ };
+ return !cachedResource.areAllClientsXMLHttpRequests();
+}
+
static bool areAllLoadersPageCacheAcceptable(const ResourceLoaderMap& loaders)
{
for (auto& loader : copyToVector(loaders.values())) {
@@ -137,7 +173,7 @@
// Only image and XHR loads do not prevent the page from entering the PageCache.
// All non-image loads will prevent the page from entering the PageCache.
- if (cachedResource->isLoading() && !cachedResource->isImage() && !cachedResource->areAllClientsXMLHttpRequests())
+ if (shouldPendingCachedResourceLoadPreventPageCache(*cachedResource))
return false;
}
return true;
@@ -1682,8 +1718,24 @@
if (loader->options().applicationCacheMode == ApplicationCacheMode::Bypass)
return;
- // A page in the PageCache or about to enter PageCache should not be able to start loads.
- ASSERT_WITH_SECURITY_IMPLICATION(!document() || document()->pageCacheState() == Document::NotInPageCache);
+#if !ASSERT_DISABLED
+ if (document()) {
+ switch (document()->pageCacheState()) {
+ case Document::NotInPageCache:
+ break;
+ case Document::AboutToEnterPageCache: {
+ // A page about to enter PageCache should only be able to start ping loads.
+ auto* cachedResource = MemoryCache::singleton().resourceForRequest(loader->request(), loader->frameLoader()->frame().page()->sessionID());
+ ASSERT(cachedResource && CachedResource::shouldUsePingLoad(cachedResource->type()));
+ break;
+ }
+ case Document::InPageCache:
+ // A page in the PageCache should not be able to start loads.
+ ASSERT_NOT_REACHED();
+ break;
+ }
+ }
+#endif
m_subresourceLoaders.add(loader->identifier(), loader);
}
Modified: trunk/Tools/ChangeLog (248264 => 248265)
--- trunk/Tools/ChangeLog 2019-08-04 18:28:45 UTC (rev 248264)
+++ trunk/Tools/ChangeLog 2019-08-04 18:32:44 UTC (rev 248265)
@@ -1,3 +1,20 @@
+2019-08-04 Chris Dumez <[email protected]>
+
+ Ping loads should not prevent page caching
+ https://bugs.webkit.org/show_bug.cgi?id=200418
+ <rdar://problem/53901632>
+
+ Reviewed by Darin Adler.
+
+ Add TestOption to enable PageCache at UIProcess-level so that we can test
+ page caching when navigating cross-origin with PSON enabled.
+
+ * WebKitTestRunner/TestController.cpp:
+ (WTR::TestController::resetPreferencesToConsistentValues):
+ (WTR::updateTestOptionsFromTestHeader):
+ * WebKitTestRunner/TestOptions.h:
+ (WTR::TestOptions::hasSameInitializationOptions const):
+
2019-08-02 Keith Rollin <[email protected]>
Consistently use Obj-C boolean literals
Modified: trunk/Tools/WebKitTestRunner/TestController.cpp (248264 => 248265)
--- trunk/Tools/WebKitTestRunner/TestController.cpp 2019-08-04 18:28:45 UTC (rev 248264)
+++ trunk/Tools/WebKitTestRunner/TestController.cpp 2019-08-04 18:32:44 UTC (rev 248265)
@@ -800,7 +800,6 @@
#if ENABLE(FULLSCREEN_API)
WKPreferencesSetFullScreenEnabled(preferences, true);
#endif
- WKPreferencesSetPageCacheEnabled(preferences, false);
WKPreferencesSetAsynchronousPluginInitializationEnabled(preferences, false);
WKPreferencesSetAsynchronousPluginInitializationEnabledForAllPlugins(preferences, false);
WKPreferencesSetArtificialPluginInitializationDelayEnabled(preferences, false);
@@ -821,6 +820,7 @@
WKPreferencesSetAllowCrossOriginSubresourcesToAskForCredentials(preferences, options.allowCrossOriginSubresourcesToAskForCredentials);
WKPreferencesSetColorFilterEnabled(preferences, options.enableColorFilter);
WKPreferencesSetPunchOutWhiteBackgroundsInDarkMode(preferences, options.punchOutWhiteBackgroundsInDarkMode);
+ WKPreferencesSetPageCacheEnabled(preferences, options.enablePageCache);
static WKStringRef defaultTextEncoding = WKStringCreateWithUTF8CString("ISO-8859-1");
WKPreferencesSetDefaultTextEncodingName(preferences, defaultTextEncoding);
@@ -1399,6 +1399,8 @@
testOptions.contentMode = { value.c_str() };
else if (key == "enableAppNap")
testOptions.enableAppNap = parseBooleanTestHeaderValue(value);
+ else if (key == "enablePageCache")
+ testOptions.enablePageCache = parseBooleanTestHeaderValue(value);
pairStart = pairEnd + 1;
}
}
Modified: trunk/Tools/WebKitTestRunner/TestOptions.h (248264 => 248265)
--- trunk/Tools/WebKitTestRunner/TestOptions.h 2019-08-04 18:28:45 UTC (rev 248264)
+++ trunk/Tools/WebKitTestRunner/TestOptions.h 2019-08-04 18:32:44 UTC (rev 248265)
@@ -92,6 +92,7 @@
bool shouldHandleRunOpenPanel { true };
bool shouldPresentPopovers { true };
bool enableAppNap { false };
+ bool enablePageCache { false };
double contentInsetTop { 0 };
@@ -146,7 +147,8 @@
|| shouldPresentPopovers != options.shouldPresentPopovers
|| contentInsetTop != options.contentInsetTop
|| contentMode != options.contentMode
- || enableAppNap != options.enableAppNap)
+ || enableAppNap != options.enableAppNap
+ || enablePageCache != options.enablePageCache)
return false;
if (!contextOptions.hasSameInitializationOptions(options.contextOptions))
