Changeset 229181 in webkit


Ignore:
Timestamp:
Mar 2, 2018 10:29:42 AM (6 years ago)
Author:
commit-queue@webkit.org
Message:

Loads for a Document controlled by a Service Worker should not use AppCache
​https://bugs.webkit.org/show_bug.cgi?id=183148

Patch by Youenn Fablet <​youenn@apple.com> on 2018-03-02
Reviewed by Chris Dumez.

LayoutTests/imported/w3c:

  • web-platform-tests/service-workers/service-worker/appcache-ordering-main.https-expected.txt:

Source/WebCore:

Covered by updated test.

Postponing document loading through app cache after matching service worker registration.
Trying to load through app cache only if there is no service worker registration.

Disabling app cache for any load that has a service worker registration identifier.

  • loader/DocumentLoader.cpp:

(WebCore::DocumentLoader::redirectReceived):
(WebCore::DocumentLoader::willSendRequest):
(WebCore::DocumentLoader::tryLoadingRequestFromApplicationCache):
(WebCore::DocumentLoader::tryLoadingRedirectRequestFromApplicationCache):
(WebCore::DocumentLoader::restartLoadingDueToServiceWorkerRegistrationChange):
(WebCore::DocumentLoader::scheduleSubstituteResourceLoad):
(WebCore::DocumentLoader::startLoadingMainResource):

  • loader/DocumentLoader.h:
  • loader/appcache/ApplicationCacheHost.cpp:

(WebCore::ApplicationCacheHost::maybeLoadMainResource):
(WebCore::ApplicationCacheHost::maybeLoadMainResourceForRedirect):
(WebCore::ApplicationCacheHost::maybeLoadResource):
(WebCore::ApplicationCacheHost::scheduleLoadFallbackResourceFromApplicationCache):

  • loader/appcache/ApplicationCacheHost.h:
Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r229119 r229181  
     12018-03-02  Youenn Fablet  <youenn@apple.com>
     2
     3        Loads for a Document controlled by a Service Worker should not use AppCache
     4        https://bugs.webkit.org/show_bug.cgi?id=183148
     5
     6        Reviewed by Chris Dumez.
     7
     8        * web-platform-tests/service-workers/service-worker/appcache-ordering-main.https-expected.txt:
     9
    1102018-02-28  Youenn Fablet  <youenn@apple.com>
    211

  • trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/appcache-ordering-main.https-expected.txt

    r227225 r229181  
    11CONSOLE MESSAGE: line 1: ApplicationCache is deprecated. Please use ServiceWorkers instead.
    22
    3 FAIL serviceworkers take priority over appcaches assert_unreached: unexpected rejection: assert_false: but serviceworkers should take priority expected false got true Reached unreachable code
     3PASS serviceworkers take priority over appcaches
    44

  • trunk/Source/WebCore/ChangeLog

    r229179 r229181  
     12018-03-02  Youenn Fablet  <youenn@apple.com>
     2
     3        Loads for a Document controlled by a Service Worker should not use AppCache
     4        https://bugs.webkit.org/show_bug.cgi?id=183148
     5
     6        Reviewed by Chris Dumez.
     7
     8        Covered by updated test.
     9
     10        Postponing document loading through app cache after matching service worker registration.
     11        Trying to load through app cache only if there is no service worker registration.
     12
     13        Disabling app cache for any load that has a service worker registration identifier.
     14
     15        * loader/DocumentLoader.cpp:
     16        (WebCore::DocumentLoader::redirectReceived):
     17        (WebCore::DocumentLoader::willSendRequest):
     18        (WebCore::DocumentLoader::tryLoadingRequestFromApplicationCache):
     19        (WebCore::DocumentLoader::tryLoadingRedirectRequestFromApplicationCache):
     20        (WebCore::DocumentLoader::restartLoadingDueToServiceWorkerRegistrationChange):
     21        (WebCore::DocumentLoader::scheduleSubstituteResourceLoad):
     22        (WebCore::DocumentLoader::startLoadingMainResource):
     23        * loader/DocumentLoader.h:
     24        * loader/appcache/ApplicationCacheHost.cpp:
     25        (WebCore::ApplicationCacheHost::maybeLoadMainResource):
     26        (WebCore::ApplicationCacheHost::maybeLoadMainResourceForRedirect):
     27        (WebCore::ApplicationCacheHost::maybeLoadResource):
     28        (WebCore::ApplicationCacheHost::scheduleLoadFallbackResourceFromApplicationCache):
     29        * loader/appcache/ApplicationCacheHost.h:
     30
    1312018-03-02  Chris Dumez  <cdumez@apple.com>
    232

  • trunk/Source/WebCore/loader/DocumentLoader.cpp

    r229177 r229181  
    520520    bool isRedirectionFromServiceWorker = redirectResponse.source() == ResourceResponse::Source::ServiceWorker;
    521521    willSendRequest(WTFMove(request), redirectResponse, [isRedirectionFromServiceWorker, completionHandler = WTFMove(completionHandler), protectedThis = makeRef(*this), this] (auto&& request) mutable {
    522         if (request.isNull() || !m_mainDocumentError.isNull() || !m_frame || m_substituteData.isValid()) {
     522        ASSERT(!m_substituteData.isValid());
     523        if (request.isNull() || !m_mainDocumentError.isNull() || !m_frame) {
    523524            completionHandler({ });
    524525            return;
    525526        }
     527
    526528        auto url = request.url();
    527529        this->matchRegistration(url, [request = WTFMove(request), isRedirectionFromServiceWorker, completionHandler = WTFMove(completionHandler), protectedThis = WTFMove(protectedThis), this] (auto&& registrationData) mutable {
    … …  
    530532                return;
    531533            }
     534
     535            if (!registrationData && this->tryLoadingRedirectRequestFromApplicationCache(request)) {
     536                completionHandler({ });
     537                return;
     538            }
     539
    532540            bool shouldContinueLoad = areRegistrationsEqual(m_serviceWorkerRegistrationData, registrationData)
    533541                && isRedirectionFromServiceWorker == !!registrationData;
    … …  
    538546            }
    539547
    540             // Service worker registration changed, we need to cancel the current load to restart a new one.
    541             this->clearMainResource();
     548            this->restartLoadingDueToServiceWorkerRegistrationChange(WTFMove(request), WTFMove(registrationData));
    542549            completionHandler({ });
    543 
    544             m_serviceWorkerRegistrationData = WTFMove(registrationData);
    545             this->loadMainResource(WTFMove(request));
    546550            return;
    547551        });
    … …  
    626630    setRequest(newRequest);
    627631
    628     if (didReceiveRedirectResponse) {
    629         // We checked application cache for initial URL, now we need to check it for redirected one.
    630         ASSERT(!m_substituteData.isValid());
    631         m_applicationCacheHost->maybeLoadMainResourceForRedirect(newRequest, m_substituteData);
    632         if (m_substituteData.isValid()) {
    633             RELEASE_ASSERT(m_mainResource);
    634             ResourceLoader* loader = m_mainResource->loader();
    635             m_identifierForLoadWithoutResourceLoader = loader ? loader->identifier() : m_mainResource->identifierForLoadWithoutResourceLoader();
    636         }
    637     }
    638 
    639632    // FIXME: Ideally we'd stop the I/O until we hear back from the navigation policy delegate
    640633    // listener. But there's no way to do that in practice. So instead we cancel later if the
    … …  
    647640    m_waitingForNavigationPolicy = true;
    648641    frameLoader()->policyChecker().checkNavigationPolicy(ResourceRequest(newRequest), didReceiveRedirectResponse, [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (ResourceRequest&& request, FormState*, bool shouldContinue) mutable {
    649         continueAfterNavigationPolicy(request, shouldContinue);
     642        m_waitingForNavigationPolicy = false;
     643        if (!shouldContinue)
     644            stopLoadingForPolicyChange();
    650645        completionHandler(WTFMove(request));
    651646    });
    652647}
    653648
    654 void DocumentLoader::continueAfterNavigationPolicy(const ResourceRequest&, bool shouldContinue)
    655 {
    656     ASSERT(m_waitingForNavigationPolicy);
    657     m_waitingForNavigationPolicy = false;
    658     if (!shouldContinue)
    659         stopLoadingForPolicyChange();
    660     else if (m_substituteData.isValid()) {
    661         // A redirect resulted in loading substitute data.
    662         ASSERT(timing().redirectCount());
    663 
    664         // We need to remove our reference to the CachedResource in favor of a SubstituteData load.
    665         // This will probably trigger the cancellation of the CachedResource's underlying ResourceLoader, though there is a
    666         // small chance that the resource is being loaded by a different Frame, preventing the ResourceLoader from being cancelled.
    667         // If the ResourceLoader is indeed cancelled, it would normally send resource load callbacks.
    668         // However, from an API perspective, this isn't a cancellation. Therefore, sever our relationship with the network load,
    669         // but prevent the ResourceLoader from sending ResourceLoadNotifier callbacks.
    670         RefPtr<ResourceLoader> resourceLoader = mainResourceLoader();
    671         if (resourceLoader) {
    672             ASSERT(resourceLoader->shouldSendResourceLoadCallbacks());
    673             resourceLoader->setSendCallbackPolicy(DoNotSendCallbacks);
    674         }
    675 
    676         clearMainResource();
    677 
    678         if (resourceLoader)
    679             resourceLoader->setSendCallbackPolicy(SendCallbacks);
    680         handleSubstituteDataLoadSoon();
    681     }
    682 }
     649bool DocumentLoader::tryLoadingRequestFromApplicationCache()
     650{
     651    m_applicationCacheHost->maybeLoadMainResource(m_request, m_substituteData);
     652
     653    if (!m_substituteData.isValid() || !m_frame->page())
     654        return false;
     655
     656    RELEASE_LOG_IF_ALLOWED("startLoadingMainResource: Returning cached main resource (frame = %p, main = %d)", m_frame, m_frame->isMainFrame());
     657    m_identifierForLoadWithoutResourceLoader = m_frame->page()->progress().createUniqueIdentifier();
     658    frameLoader()->notifier().assignIdentifierToInitialRequest(m_identifierForLoadWithoutResourceLoader, this, m_request);
     659    frameLoader()->notifier().dispatchWillSendRequest(this, m_identifierForLoadWithoutResourceLoader, m_request, ResourceResponse());
     660    handleSubstituteDataLoadSoon();
     661    return true;
     662}
     663
     664bool DocumentLoader::tryLoadingRedirectRequestFromApplicationCache(const ResourceRequest& request)
     665{
     666    m_applicationCacheHost->maybeLoadMainResourceForRedirect(request, m_substituteData);
     667    if (!m_substituteData.isValid())
     668        return false;
     669
     670    RELEASE_ASSERT(m_mainResource);
     671    auto* loader = m_mainResource->loader();
     672    m_identifierForLoadWithoutResourceLoader = loader ? loader->identifier() : m_mainResource->identifierForLoadWithoutResourceLoader();
     673
     674    // We need to remove our reference to the CachedResource in favor of a SubstituteData load, which can triger the cancellation of the underyling ResourceLoader.
     675    // If the ResourceLoader is indeed cancelled, it would normally send resource load callbacks.
     676    // Therefore, sever our relationship with the network load but prevent the ResourceLoader from sending ResourceLoadNotifier callbacks.
     677
     678    auto resourceLoader = makeRefPtr(mainResourceLoader());
     679    if (resourceLoader) {
     680        ASSERT(resourceLoader->shouldSendResourceLoadCallbacks());
     681        resourceLoader->setSendCallbackPolicy(DoNotSendCallbacks);
     682    }
     683
     684    clearMainResource();
     685
     686    if (resourceLoader)
     687        resourceLoader->setSendCallbackPolicy(SendCallbacks);
     688
     689    handleSubstituteDataLoadNow();
     690    return true;
     691}
     692
     693#if ENABLE(SERVICE_WORKER)
     694void DocumentLoader::restartLoadingDueToServiceWorkerRegistrationChange(ResourceRequest&& request, std::optional<ServiceWorkerRegistrationData>&& registrationData)
     695{
     696    clearMainResource();
     697
     698    m_serviceWorkerRegistrationData = WTFMove(registrationData);
     699    loadMainResource(WTFMove(request));
     700}
     701#endif
    683702
    684703void DocumentLoader::stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied(unsigned long identifier, const ResourceResponse& response)
    … …  
    14361455void DocumentLoader::scheduleSubstituteResourceLoad(ResourceLoader& loader, SubstituteResource& resource)
    14371456{
     1457#if ENABLE(SERVICE_WORKER)
     1458    ASSERT(!loader.options().serviceWorkerRegistrationIdentifier);
     1459#endif
    14381460    m_pendingSubstituteResources.set(&loader, &resource);
    14391461    deliverSubstituteResourcesAfterDelay();
    … …  
    16481670        }
    16491671
    1650         m_applicationCacheHost->maybeLoadMainResource(m_request, m_substituteData);
    1651 
    1652         if (m_substituteData.isValid() && m_frame->page()) {
    1653             RELEASE_LOG_IF_ALLOWED("startLoadingMainResource: Returning cached main resource (frame = %p, main = %d)", m_frame, m_frame->isMainFrame());
    1654             m_identifierForLoadWithoutResourceLoader = m_frame->page()->progress().createUniqueIdentifier();
    1655             frameLoader()->notifier().assignIdentifierToInitialRequest(m_identifierForLoadWithoutResourceLoader, this, m_request);
    1656             frameLoader()->notifier().dispatchWillSendRequest(this, m_identifierForLoadWithoutResourceLoader, m_request, ResourceResponse());
    1657             handleSubstituteDataLoadSoon();
    1658             return;
    1659         }
    1660 
    16611672        request.setRequester(ResourceRequest::Requester::Main);
    16621673        // If this is a reload the cache layer might have made the previous request conditional. DocumentLoader can't handle 304 responses itself.
    … …  
    16731684
    16741685            m_serviceWorkerRegistrationData = WTFMove(registrationData);
     1686            if (!m_serviceWorkerRegistrationData && tryLoadingRequestFromApplicationCache())
     1687                return;
    16751688            this->loadMainResource(WTFMove(request));
    16761689        });
    16771690#else
     1691        if (tryLoadingRequestFromApplicationCache())
     1692            return;
    16781693        loadMainResource(WTFMove(request));
    16791694#endif

  • trunk/Source/WebCore/loader/DocumentLoader.h

    r228975 r229181  
    368368    bool isPostOrRedirectAfterPost(const ResourceRequest&, const ResourceResponse&);
    369369
    370     void continueAfterNavigationPolicy(const ResourceRequest&, bool shouldContinue);
     370    bool tryLoadingRequestFromApplicationCache();
     371    bool tryLoadingRedirectRequestFromApplicationCache(const ResourceRequest&);
     372#if ENABLE(SERVICE_WORKER)
     373    void restartLoadingDueToServiceWorkerRegistrationChange(ResourceRequest&&, std::optional<ServiceWorkerRegistrationData>&&);
     374#endif
    371375    void continueAfterContentPolicy(PolicyAction);
    372376

  • trunk/Source/WebCore/loader/appcache/ApplicationCacheHost.cpp

    r228975 r229181  
    7676}
    7777
    78 void ApplicationCacheHost::maybeLoadMainResource(ResourceRequest& request, SubstituteData& substituteData)
     78void ApplicationCacheHost::maybeLoadMainResource(const ResourceRequest& request, SubstituteData& substituteData)
    7979{
    8080    // Check if this request should be loaded from the application cache
    … …  
    105105}
    106106
    107 void ApplicationCacheHost::maybeLoadMainResourceForRedirect(ResourceRequest& request, SubstituteData& substituteData)
     107void ApplicationCacheHost::maybeLoadMainResourceForRedirect(const ResourceRequest& request, SubstituteData& substituteData)
    108108{
    109109    ASSERT(status() == UNCACHED);
    … …  
    178178    if (request.url() != originalURL)
    179179        return false;
     180
     181#if ENABLE(SERVICE_WORKER)
     182    if (loader.options().serviceWorkerRegistrationIdentifier)
     183        return false;
     184#endif
    180185
    181186    ApplicationCacheResource* resource;
    … …  
    454459bool ApplicationCacheHost::scheduleLoadFallbackResourceFromApplicationCache(ResourceLoader* loader, ApplicationCache* cache)
    455460{
     461    if (!loader)
     462        return false;
     463
    456464    if (!isApplicationCacheEnabled() && !isApplicationCacheBlockedForRequest(loader->request()))
    457465        return false;
     466
     467#if ENABLE(SERVICE_WORKER)
     468    if (loader->options().serviceWorkerRegistrationIdentifier)
     469        return false;
     470#endif
    458471
    459472    ApplicationCacheResource* resource;

  • trunk/Source/WebCore/loader/appcache/ApplicationCacheHost.h

    r210845 r229181  
    9090    void selectCacheWithManifest(const URL& manifestURL);
    9191
    92     void maybeLoadMainResource(ResourceRequest&, SubstituteData&);
    93     void maybeLoadMainResourceForRedirect(ResourceRequest&, SubstituteData&);
     92    void maybeLoadMainResource(const ResourceRequest&, SubstituteData&);
     93    void maybeLoadMainResourceForRedirect(const ResourceRequest&, SubstituteData&);
    9494    bool maybeLoadFallbackForMainResponse(const ResourceRequest&, const ResourceResponse&);
    9595    void mainResourceDataReceived(const char* data, int length, long long encodedDataLength, bool allAtOnce);
Note: See TracChangeset for help on using the changeset viewer.