Changeset 248265 in webkit


Ignore:
Timestamp:
Aug 4, 2019 11:32:44 AM (5 years ago)
Author:
Chris Dumez
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.
Location:
trunk
Files:
4 added
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r248202 r248265  
     12019-08-04  Chris Dumez  <cdumez@apple.com>
     2
     3        Ping loads should not prevent page caching
     4        https://bugs.webkit.org/show_bug.cgi?id=200418
     5        <rdar://problem/53901632>
     6
     7        Reviewed by Darin Adler.
     8
     9        Add layout test coverage.
     10
     11        * http/tests/navigation/page-cache-pending-ping-load-cross-origin-expected.txt: Added.
     12        * http/tests/navigation/page-cache-pending-ping-load-cross-origin.html: Added.
     13        * http/tests/navigation/page-cache-pending-ping-load-same-origin-expected.txt: Added.
     14        * http/tests/navigation/page-cache-pending-ping-load-same-origin.html: Added.
     15
    1162019-08-03  Devin Rousso  <drousso@apple.com>
    217
  • trunk/Source/WebCore/ChangeLog

    r248263 r248265  
     12019-08-04  Chris Dumez  <cdumez@apple.com>
     2
     3        Ping loads should not prevent page caching
     4        https://bugs.webkit.org/show_bug.cgi?id=200418
     5        <rdar://problem/53901632>
     6
     7        Reviewed by Darin Adler.
     8
     9        We normally prevent page caching if there were any pending subresource loads when navigating,
     10        to avoid caching partial / broken content. However, this should not apply to Ping / Beacon
     11        loads since those do not impact page rendering and can outlive the page.
     12
     13        Tests: http/tests/navigation/page-cache-pending-ping-load-cross-origin.html
     14               http/tests/navigation/page-cache-pending-ping-load-same-origin.html
     15
     16        * history/PageCache.cpp:
     17        (WebCore::PageCache::addIfCacheable):
     18        After we've fired the 'pagehide' event in each frame, stop all the loads again. This is needed
     19        since pages are allowed to start ping / beacon loads in their 'pagehide' handlers. If we do not
     20        stop those loads, then the next call to canCachePage() would fail because the DocumentLoader is
     21        still loading. Note that we're not actually preventing these ping loads from hitting the server
     22        since we never cancel page loads and those can outlive their page.
     23
     24        * loader/DocumentLoader.cpp:
     25        (WebCore::shouldPendingCachedResourceLoadPreventPageCache):
     26        (WebCore::areAllLoadersPageCacheAcceptable):
     27        Make sure that Ping / Beacon / Prefetches / Icon loads do not prevent page caching.
     28
     29        (WebCore::DocumentLoader::addSubresourceLoader):
     30        Tweak assertion that was incorrect since we actually allow ping / beacon loads when the
     31        document is about to enter PageCache (while firing pagehide event).
     32
    1332019-08-04  Zalan Bujtas  <zalan@apple.com>
    234
  • trunk/Source/WebCore/history/PageCache.cpp

    r246187 r248265  
    459459    destroyRenderTree(page->mainFrame());
    460460
     461    // Stop all loads again before checking if we can still cache the page after firing the pagehide
     462    // event, since the page may have started ping loads in its pagehide event handler.
     463    for (Frame* frame = &page->mainFrame(); frame; frame = frame->tree().traverseNext()) {
     464        if (auto* documentLoader = frame->loader().documentLoader())
     465            documentLoader->stopLoading();
     466    }
     467
    461468    // Check that the page is still page-cacheable after firing the pagehide event. The JS event handlers
    462469    // could have altered the page in a way that could prevent caching.
  • trunk/Source/WebCore/loader/DocumentLoader.cpp

    r248148 r248265  
    126126}
    127127
     128static bool shouldPendingCachedResourceLoadPreventPageCache(CachedResource& cachedResource)
     129{
     130    if (!cachedResource.isLoading())
     131        return false;
     132
     133    switch (cachedResource.type()) {
     134    case CachedResource::Type::ImageResource:
     135    case CachedResource::Type::Icon:
     136    case CachedResource::Type::Beacon:
     137    case CachedResource::Type::Ping:
     138    case CachedResource::Type::LinkPrefetch:
     139        return false;
     140    case CachedResource::Type::MainResource:
     141    case CachedResource::Type::CSSStyleSheet:
     142    case CachedResource::Type::Script:
     143    case CachedResource::Type::FontResource:
     144#if ENABLE(SVG_FONTS)
     145    case CachedResource::Type::SVGFontResource:
     146#endif
     147    case CachedResource::Type::MediaResource:
     148    case CachedResource::Type::RawResource:
     149    case CachedResource::Type::SVGDocumentResource:
     150#if ENABLE(XSLT)
     151    case CachedResource::Type::XSLStyleSheet:
     152#endif
     153#if ENABLE(VIDEO_TRACK)
     154    case CachedResource::Type::TextTrackResource:
     155#endif
     156#if ENABLE(APPLICATION_MANIFEST)
     157    case CachedResource::Type::ApplicationManifest:
     158#endif
     159        break;
     160    };
     161    return !cachedResource.areAllClientsXMLHttpRequests();
     162}
     163
    128164static bool areAllLoadersPageCacheAcceptable(const ResourceLoaderMap& loaders)
    129165{
     
    138174        // Only image and XHR loads do not prevent the page from entering the PageCache.
    139175        // All non-image loads will prevent the page from entering the PageCache.
    140         if (cachedResource->isLoading() && !cachedResource->isImage() && !cachedResource->areAllClientsXMLHttpRequests())
     176        if (shouldPendingCachedResourceLoadPreventPageCache(*cachedResource))
    141177            return false;
    142178    }
     
    16831719        return;
    16841720
    1685     // A page in the PageCache or about to enter PageCache should not be able to start loads.
    1686     ASSERT_WITH_SECURITY_IMPLICATION(!document() || document()->pageCacheState() == Document::NotInPageCache);
     1721#if !ASSERT_DISABLED
     1722    if (document()) {
     1723        switch (document()->pageCacheState()) {
     1724        case Document::NotInPageCache:
     1725            break;
     1726        case Document::AboutToEnterPageCache: {
     1727            // A page about to enter PageCache should only be able to start ping loads.
     1728            auto* cachedResource = MemoryCache::singleton().resourceForRequest(loader->request(), loader->frameLoader()->frame().page()->sessionID());
     1729            ASSERT(cachedResource && CachedResource::shouldUsePingLoad(cachedResource->type()));
     1730            break;
     1731        }
     1732        case Document::InPageCache:
     1733            // A page in the PageCache should not be able to start loads.
     1734            ASSERT_NOT_REACHED();
     1735            break;
     1736        }
     1737    }
     1738#endif
    16871739
    16881740    m_subresourceLoaders.add(loader->identifier(), loader);
  • trunk/Tools/ChangeLog

    r248190 r248265  
     12019-08-04  Chris Dumez  <cdumez@apple.com>
     2
     3        Ping loads should not prevent page caching
     4        https://bugs.webkit.org/show_bug.cgi?id=200418
     5        <rdar://problem/53901632>
     6
     7        Reviewed by Darin Adler.
     8
     9        Add TestOption to enable PageCache at UIProcess-level so that we can test
     10        page caching when navigating cross-origin with PSON enabled.
     11
     12        * WebKitTestRunner/TestController.cpp:
     13        (WTR::TestController::resetPreferencesToConsistentValues):
     14        (WTR::updateTestOptionsFromTestHeader):
     15        * WebKitTestRunner/TestOptions.h:
     16        (WTR::TestOptions::hasSameInitializationOptions const):
     17
    1182019-08-02  Keith Rollin  <krollin@apple.com>
    219
  • trunk/Tools/WebKitTestRunner/TestController.cpp

    r247941 r248265  
    801801    WKPreferencesSetFullScreenEnabled(preferences, true);
    802802#endif
    803     WKPreferencesSetPageCacheEnabled(preferences, false);
    804803    WKPreferencesSetAsynchronousPluginInitializationEnabled(preferences, false);
    805804    WKPreferencesSetAsynchronousPluginInitializationEnabledForAllPlugins(preferences, false);
     
    822821    WKPreferencesSetColorFilterEnabled(preferences, options.enableColorFilter);
    823822    WKPreferencesSetPunchOutWhiteBackgroundsInDarkMode(preferences, options.punchOutWhiteBackgroundsInDarkMode);
     823    WKPreferencesSetPageCacheEnabled(preferences, options.enablePageCache);
    824824
    825825    static WKStringRef defaultTextEncoding = WKStringCreateWithUTF8CString("ISO-8859-1");
     
    14001400        else if (key == "enableAppNap")
    14011401            testOptions.enableAppNap = parseBooleanTestHeaderValue(value);
     1402        else if (key == "enablePageCache")
     1403            testOptions.enablePageCache = parseBooleanTestHeaderValue(value);
    14021404        pairStart = pairEnd + 1;
    14031405    }
  • trunk/Tools/WebKitTestRunner/TestOptions.h

    r246118 r248265  
    9393    bool shouldPresentPopovers { true };
    9494    bool enableAppNap { false };
     95    bool enablePageCache { false };
    9596
    9697    double contentInsetTop { 0 };
     
    147148            || contentInsetTop != options.contentInsetTop
    148149            || contentMode != options.contentMode
    149             || enableAppNap != options.enableAppNap)
     150            || enableAppNap != options.enableAppNap
     151            || enablePageCache != options.enablePageCache)
    150152            return false;
    151153
Note: See TracChangeset for help on using the changeset viewer.