Changeset 252778 in webkit


Ignore:
Timestamp:
Nov 22, 2019 9:25:29 AM (4 years ago)
Author:
Chris Dumez
Message:

Speculative loading sometimes happens too early and is missing login cookies
​https://bugs.webkit.org/show_bug.cgi?id=204305
<rdar://problem/57063840>

Reviewed by Antti Koivisto.

Source/WebKit:

Speculative loads were issued before receiving the response from the main resource. However,
the main resource may set important cookies that are thus missing from the speculative requests.

To address the issue we now delay speculative loads for first-party subresources until we've
received the response from the main resource. To avoid regressing PLT, we still warm up the
first-party subresources from disk right away and preconnect to the server.

No new tests, extended existing test.

  • NetworkProcess/NetworkResourceLoader.cpp:

(WebKit::NetworkResourceLoader::didReceiveResponse):
(WebKit::NetworkResourceLoader::didReceiveMainResourceResponse):
(WebKit::NetworkResourceLoader::didRetrieveCacheEntry):

  • NetworkProcess/NetworkResourceLoader.h:
  • NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp:

(WebKit::NetworkCache::SpeculativeLoadManager::PendingFrameLoad::didReceiveMainResourceResponse const):
(WebKit::NetworkCache::SpeculativeLoadManager::PendingFrameLoad::markMainResourceResponseAsReceived):
(WebKit::NetworkCache::SpeculativeLoadManager::PendingFrameLoad::addPostMainResourceResponseTask):
(WebKit::NetworkCache::SpeculativeLoadManager::shouldRegisterLoad):
(WebKit::NetworkCache::SpeculativeLoadManager::registerLoad):
(WebKit::NetworkCache::SpeculativeLoadManager::registerMainResourceLoadResponse):
(WebKit::NetworkCache::SpeculativeLoadManager::preconnectForSubresource):
(WebKit::NetworkCache::SpeculativeLoadManager::revalidateSubresource):

  • NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.h:
  • NetworkProcess/cache/NetworkCacheSubresourcesEntry.cpp:

(WebKit::NetworkCache::SubresourceInfo::isFirstParty const):

  • NetworkProcess/cache/NetworkCacheSubresourcesEntry.h:

LayoutTests:

Extend layout test coverage to make sure that the validation request contains the latest cookies
set by the main resource.

  • http/tests/cache/disk-cache/speculative-validation/resources/validation-request-frame.php:
  • http/tests/cache/disk-cache/speculative-validation/validation-request-expected.txt:
  • http/tests/cache/disk-cache/speculative-validation/validation-request.html:
Location:
trunk
Files:
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r252774 r252778  
     12019-11-22  Chris Dumez  <cdumez@apple.com>
     2
     3        Speculative loading sometimes happens too early and is missing login cookies
     4        https://bugs.webkit.org/show_bug.cgi?id=204305
     5        <rdar://problem/57063840>
     6
     7        Reviewed by Antti Koivisto.
     8
     9        Extend layout test coverage to make sure that the validation request contains the latest cookies
     10        set by the main resource.
     11
     12        * http/tests/cache/disk-cache/speculative-validation/resources/validation-request-frame.php:
     13        * http/tests/cache/disk-cache/speculative-validation/validation-request-expected.txt:
     14        * http/tests/cache/disk-cache/speculative-validation/validation-request.html:
     15
    1162019-11-22  Per Arne Vollan  <pvollan@apple.com>
    217

  • trunk/LayoutTests/http/tests/cache/disk-cache/speculative-validation/resources/validation-request-frame.php

    r198450 r252778  
    33header('Cache-Control: max-age=0');
    44header('Etag: 123456789');
    5 
     5$cookie = "speculativeRequestValidation=" . uniqid();
     6header('Set-Cookie: ' . $cookie);
    67?>
    78<!DOCTYPE html>
    89<body>
    910<script src="request-headers-script.php"></script>
     11<script>
     12<?php
     13echo "sentSetCookieHeader = '" . $cookie . "';";
     14?>
     15</script>
    1016</body>

  • trunk/LayoutTests/http/tests/cache/disk-cache/speculative-validation/validation-request-expected.txt

    r198741 r252778  
    55
    66PASS validationRequestHeader('If-None-Match') is "123456789"
     7PASS isCookieHeaderCorrect is true
    78PASS validationRequestHeader('Accept') is initialHeaderValues['Accept']
    89PASS validationRequestHeader('Accept-Encoding') is initialHeaderValues['Accept-Encoding']

  • trunk/LayoutTests/http/tests/cache/disk-cache/speculative-validation/validation-request.html

    r198741 r252778  
    3838        // Validate the HTTP headers of the speculative validation request.
    3939        shouldBeEqualToString("validationRequestHeader('If-None-Match')", "123456789");
     40        isCookieHeaderCorrect = validationRequestHeader('Cookie') === document.getElementById("testFrame").contentWindow.sentSetCookieHeader;
     41        shouldBeTrue("isCookieHeaderCorrect");
    4042
    4143        for (var i = 0; i < headersToCheck.length; i++) {

  • trunk/Source/WebKit/ChangeLog

    r252770 r252778  
     12019-11-22  Chris Dumez  <cdumez@apple.com>
     2
     3        Speculative loading sometimes happens too early and is missing login cookies
     4        https://bugs.webkit.org/show_bug.cgi?id=204305
     5        <rdar://problem/57063840>
     6
     7        Reviewed by Antti Koivisto.
     8
     9        Speculative loads were issued before receiving the response from the main resource. However,
     10        the main resource may set important cookies that are thus missing from the speculative requests.
     11
     12        To address the issue we now delay speculative loads for first-party subresources until we've
     13        received the response from the main resource. To avoid regressing PLT, we still warm up the
     14        first-party subresources from disk right away and preconnect to the server.
     15
     16        No new tests, extended existing test.
     17
     18        * NetworkProcess/NetworkResourceLoader.cpp:
     19        (WebKit::NetworkResourceLoader::didReceiveResponse):
     20        (WebKit::NetworkResourceLoader::didReceiveMainResourceResponse):
     21        (WebKit::NetworkResourceLoader::didRetrieveCacheEntry):
     22        * NetworkProcess/NetworkResourceLoader.h:
     23        * NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp:
     24        (WebKit::NetworkCache::SpeculativeLoadManager::PendingFrameLoad::didReceiveMainResourceResponse const):
     25        (WebKit::NetworkCache::SpeculativeLoadManager::PendingFrameLoad::markMainResourceResponseAsReceived):
     26        (WebKit::NetworkCache::SpeculativeLoadManager::PendingFrameLoad::addPostMainResourceResponseTask):
     27        (WebKit::NetworkCache::SpeculativeLoadManager::shouldRegisterLoad):
     28        (WebKit::NetworkCache::SpeculativeLoadManager::registerLoad):
     29        (WebKit::NetworkCache::SpeculativeLoadManager::registerMainResourceLoadResponse):
     30        (WebKit::NetworkCache::SpeculativeLoadManager::preconnectForSubresource):
     31        (WebKit::NetworkCache::SpeculativeLoadManager::revalidateSubresource):
     32        * NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.h:
     33        * NetworkProcess/cache/NetworkCacheSubresourcesEntry.cpp:
     34        (WebKit::NetworkCache::SubresourceInfo::isFirstParty const):
     35        * NetworkProcess/cache/NetworkCacheSubresourcesEntry.h:
     36
    1372019-11-22  Carlos Garcia Campos  <cgarcia@igalia.com>
    238

  • trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp

    r252510 r252778  
    3131#include "Logging.h"
    3232#include "NetworkCache.h"
     33#include "NetworkCacheSpeculativeLoadManager.h"
    3334#include "NetworkConnectionToWebProcess.h"
    3435#include "NetworkConnectionToWebProcessMessages.h"
    … …  
    467468    RELEASE_LOG_IF_ALLOWED("didReceiveResponse: (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", httpStatusCode = %d, length = %" PRId64 ")", m_parameters.webPageID.toUInt64(), m_parameters.webFrameID.toUInt64(), m_parameters.identifier, receivedResponse.httpStatusCode(), receivedResponse.expectedContentLength());
    468469
     470    if (isMainResource())
     471        didReceiveMainResourceResponse(receivedResponse);
     472
    469473    m_response = WTFMove(receivedResponse);
    470474
    … …  
    901905}
    902906
     907void NetworkResourceLoader::didReceiveMainResourceResponse(const WebCore::ResourceResponse& response)
     908{
     909#if ENABLE(NETWORK_CACHE_SPECULATIVE_REVALIDATION)
     910    if (auto* speculativeLoadManager = m_cache ? m_cache->speculativeLoadManager() : nullptr)
     911        speculativeLoadManager->registerMainResourceLoadResponse(globalFrameID(), originalRequest(), response);
     912#endif
     913}
     914
    903915void NetworkResourceLoader::didRetrieveCacheEntry(std::unique_ptr<NetworkCache::Entry> entry)
    904916{
    905917    auto response = entry->response();
     918
     919    if (isMainResource())
     920        didReceiveMainResourceResponse(response);
    906921
    907922    if (isMainResource() && shouldInterruptLoadForCSPFrameAncestorsOrXFrameOptions(response)) {

  • trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.h

    r251445 r252778  
    156156    void restartNetworkLoad(WebCore::ResourceRequest&&);
    157157    void continueDidReceiveResponse();
     158    void didReceiveMainResourceResponse(const WebCore::ResourceResponse&);
    158159
    159160    enum class LoadResult {

  • trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp

    r252397 r252778  
    3434#include "NetworkCacheSubresourcesEntry.h"
    3535#include "NetworkProcess.h"
     36#include "PreconnectTask.h"
    3637#include <WebCore/DiagnosticLoggingKeys.h>
    3738#include <pal/HysteresisActivity.h>
    … …  
    204205    }
    205206
     207    bool didReceiveMainResourceResponse() const { return m_didReceiveMainResourceResponse; }
     208    void markMainResourceResponseAsReceived()
     209    {
     210        m_didReceiveMainResourceResponse = true;
     211        for (auto& task : m_postMainResourceResponseTasks)
     212            task();
     213    }
     214
     215    void addPostMainResourceResponseTask(Function<void()>&& task) { m_postMainResourceResponseTasks.append(WTFMove(task)); }
     216
    206217private:
    207218    PendingFrameLoad(Storage& storage, const Key& mainResourceKey, WTF::Function<void()>&& loadCompletionHandler)
    … …  
    243254    PAL::HysteresisActivity m_loadHysteresisActivity;
    244255    std::unique_ptr<SubresourcesEntry> m_existingEntry;
     256    Vector<Function<void()>> m_postMainResourceResponseTasks;
    245257    bool m_didFinishLoad { false };
    246258    bool m_didRetrieveExistingEntry { false };
     259    bool m_didReceiveMainResourceResponse { false };
    247260};
    248261
    … …  
    324337}
    325338
     339bool SpeculativeLoadManager::shouldRegisterLoad(const WebCore::ResourceRequest& request)
     340{
     341    if (request.httpMethod() != "GET")
     342        return false;
     343    if (!request.httpHeaderField(HTTPHeaderName::Range).isEmpty())
     344        return false;
     345    return true;
     346}
     347
    326348void SpeculativeLoadManager::registerLoad(const GlobalFrameID& frameID, const ResourceRequest& request, const Key& resourceKey)
    327349{
    … …  
    329351    ASSERT(request.url().protocolIsInHTTPFamily());
    330352
    331     if (request.httpMethod() != "GET")
    332         return;
    333     if (!request.httpHeaderField(HTTPHeaderName::Range).isEmpty())
     353    if (!shouldRegisterLoad(request))
    334354        return;
    335355
    … …  
    361381    if (auto* pendingFrameLoad = m_pendingFrameLoads.get(frameID))
    362382        pendingFrameLoad->registerSubresourceLoad(request, resourceKey);
     383}
     384
     385void SpeculativeLoadManager::registerMainResourceLoadResponse(const GlobalFrameID& frameID, const WebCore::ResourceRequest& request, const WebCore::ResourceResponse& response)
     386{
     387    if (!shouldRegisterLoad(request))
     388        return;
     389
     390    if (response.isRedirection())
     391        return;
     392
     393    if (auto* pendingFrameLoad = m_pendingFrameLoads.get(frameID))
     394        pendingFrameLoad->markMainResourceResponseAsReceived();
    363395}
    364396
    … …  
    418450}
    419451
     452void SpeculativeLoadManager::preconnectForSubresource(const SubresourceInfo& subresourceInfo, Entry* entry, const GlobalFrameID& frameID)
     453{
     454#if ENABLE(SERVER_PRECONNECT)
     455    NetworkLoadParameters parameters;
     456    parameters.webPageProxyID = frameID.webPageProxyID;
     457    parameters.webPageID = frameID.webPageID;
     458    parameters.webFrameID = frameID.frameID;
     459    parameters.storedCredentialsPolicy = StoredCredentialsPolicy::Use;
     460    parameters.contentSniffingPolicy = ContentSniffingPolicy::DoNotSniffContent;
     461    parameters.contentEncodingSniffingPolicy = ContentEncodingSniffingPolicy::Sniff;
     462    parameters.shouldPreconnectOnly = PreconnectOnly::Yes;
     463    parameters.request = constructRevalidationRequest(subresourceInfo.key(), subresourceInfo, entry);
     464    new PreconnectTask(m_cache.networkProcess(), m_cache.sessionID(), WTFMove(parameters), [](const WebCore::ResourceError&) { });
     465#else
     466    UNUSED_PARAM(subresourceInfo);
     467    UNUSED_PARAM(entry);
     468    UNUSED_PARAM(frameID);
     469#endif
     470}
     471
    420472void SpeculativeLoadManager::revalidateSubresource(const SubresourceInfo& subresourceInfo, std::unique_ptr<Entry> entry, const GlobalFrameID& frameID)
    421473{
    … …  
    427479    if (!key.range().isEmpty())
    428480        return;
     481
     482    auto* pendingLoad = m_pendingFrameLoads.get(frameID);
     483
     484    // Delay first-party speculative loads until we've received the response for the main resource, in case the main resource
     485    // response sets cookies that are needed for subsequent loads.
     486    if (pendingLoad && !pendingLoad->didReceiveMainResourceResponse() && subresourceInfo.isFirstParty()) {
     487        preconnectForSubresource(subresourceInfo, entry.get(), frameID);
     488        pendingLoad->addPostMainResourceResponseTask([this, subresourceInfo, entry = WTFMove(entry), frameID]() mutable {
     489            revalidateSubresource(subresourceInfo, WTFMove(entry), frameID);
     490        });
     491        return;
     492    }
    429493
    430494    ResourceRequest revalidationRequest = constructRevalidationRequest(key, subresourceInfo, entry.get());

  • trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.h

    r239427 r252778  
    5151
    5252    void registerLoad(const GlobalFrameID&, const WebCore::ResourceRequest&, const Key& resourceKey);
     53    void registerMainResourceLoadResponse(const GlobalFrameID&, const WebCore::ResourceRequest&, const WebCore::ResourceResponse&);
    5354
    5455    typedef Function<void (std::unique_ptr<Entry>)> RetrieveCompletionHandler;
    … …  
    6061    class PreloadedEntry;
    6162
     63    static bool shouldRegisterLoad(const WebCore::ResourceRequest&);
    6264    void addPreloadedEntry(std::unique_ptr<Entry>, const GlobalFrameID&, Optional<WebCore::ResourceRequest>&& revalidationRequest = WTF::nullopt);
    6365    void preloadEntry(const Key&, const SubresourceInfo&, const GlobalFrameID&);
    6466    void retrieveEntryFromStorage(const SubresourceInfo&, RetrieveCompletionHandler&&);
    6567    void revalidateSubresource(const SubresourceInfo&, std::unique_ptr<Entry>, const GlobalFrameID&);
     68    void preconnectForSubresource(const SubresourceInfo&, Entry*, const GlobalFrameID&);
    6669    bool satisfyPendingRequests(const Key&, Entry*);
    6770    void retrieveSubresourcesEntry(const Key& storageKey, WTF::Function<void (std::unique_ptr<SubresourcesEntry>)>&&);

  • trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSubresourcesEntry.cpp

    r248846 r252778  
    8080   
    8181    return true;
     82}
     83
     84bool SubresourceInfo::isFirstParty() const
     85{
     86    RegistrableDomain firstPartyDomain { m_firstPartyForCookies };
     87    return firstPartyDomain.matches(URL(URL(), key().identifier()));
    8288}
    8389

  • trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSubresourcesEntry.h

    r238771 r252778  
    5959    void setNonTransient() { m_isTransient = false; }
    6060
     61    bool isFirstParty() const;
     62
    6163private:
    6264    Key m_key;
Note: See TracChangeset for help on using the changeset viewer.