Changeset 239974 in webkit


Ignore:
Timestamp:
Jan 14, 2019, 9:15:57 PM (7 years ago)
Author:
achristensen@apple.com
Message:

Split headerValueForVary into specialized functions for NetworkProcess and WebProcess/WebKitLegacy
​https://bugs.webkit.org/show_bug.cgi?id=193429

Reviewed by Joseph Pecoraro.

Source/WebCore:

headerValueForVary is a strange function that is causing trouble with my NetworkProcess global state removal project.
It currently accesses the cookie storage to see if there's a match in two different ways currently written as fallbacks.
In the WebProcess or in WebKitLegacy, it uses cookiesStrategy to access cookies via IPC or directly, respectively,
depending on the PlatformStrategies implementation of cookiesStrategy for that process.
In the NetworkProcess, it uses WebCore::NetworkStorageSession to access cookies directly.
Both of these cookie accessing methods use global state in the process, and I must split them to refactor them separately.
This patch does the split by passing in the method of cookie access: a CookiesStrategy& or a NetworkStorageSession&.
Further refactoring will be done in bug 193368 and bug 161106 to build on this and replace the global state with
member variables of the correct containing objects.

  • loader/cache/CachedResource.cpp:

(WebCore::CachedResource::setResponse):
(WebCore::CachedResource::varyHeaderValuesMatch):

  • platform/network/CacheValidation.cpp:

(WebCore::cookieRequestHeaderFieldValue):
(WebCore::headerValueForVary):
(WebCore::collectVaryingRequestHeaders):
(WebCore::verifyVaryingRequestHeaders):

  • platform/network/CacheValidation.h:

Source/WebKit:

  • NetworkProcess/cache/NetworkCache.cpp:

(WebKit::NetworkCache::makeUseDecision):
(WebKit::NetworkCache::Cache::retrieve):
(WebKit::NetworkCache::Cache::makeEntry):
(WebKit::NetworkCache::Cache::makeRedirectEntry):
(WebKit::NetworkCache::Cache::update):

Location:
trunk/Source
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r239971 r239974  
     12019-01-14  Alex Christensen  <achristensen@webkit.org>
     2
     3        Split headerValueForVary into specialized functions for NetworkProcess and WebProcess/WebKitLegacy
     4        https://bugs.webkit.org/show_bug.cgi?id=193429
     5
     6        Reviewed by Joseph Pecoraro.
     7
     8        headerValueForVary is a strange function that is causing trouble with my NetworkProcess global state removal project.
     9        It currently accesses the cookie storage to see if there's a match in two different ways currently written as fallbacks.
     10        In the WebProcess or in WebKitLegacy, it uses cookiesStrategy to access cookies via IPC or directly, respectively,
     11        depending on the PlatformStrategies implementation of cookiesStrategy for that process.
     12        In the NetworkProcess, it uses WebCore::NetworkStorageSession to access cookies directly.
     13        Both of these cookie accessing methods use global state in the process, and I must split them to refactor them separately.
     14        This patch does the split by passing in the method of cookie access: a CookiesStrategy& or a NetworkStorageSession&.
     15        Further refactoring will be done in bug 193368 and bug 161106 to build on this and replace the global state with
     16        member variables of the correct containing objects.
     17
     18        * loader/cache/CachedResource.cpp:
     19        (WebCore::CachedResource::setResponse):
     20        (WebCore::CachedResource::varyHeaderValuesMatch):
     21        * platform/network/CacheValidation.cpp:
     22        (WebCore::cookieRequestHeaderFieldValue):
     23        (WebCore::headerValueForVary):
     24        (WebCore::collectVaryingRequestHeaders):
     25        (WebCore::verifyVaryingRequestHeaders):
     26        * platform/network/CacheValidation.h:
     27
    1282019-01-14  Simon Fraser  <simon.fraser@apple.com>
    229

  • trunk/Source/WebCore/loader/cache/CachedResource.cpp

    r239634 r239974  
    477477    ASSERT(m_response.type() == ResourceResponse::Type::Default);
    478478    m_response = response;
    479     m_varyingHeaderValues = collectVaryingRequestHeaders(m_resourceRequest, m_response, m_sessionID);
     479    m_varyingHeaderValues = collectVaryingRequestHeaders(*platformStrategies()->cookiesStrategy(), m_resourceRequest, m_response, m_sessionID);
    480480
    481481#if ENABLE(SERVICE_WORKER)
    … …  
    859859        return true;
    860860
    861     return verifyVaryingRequestHeaders(m_varyingHeaderValues, request, m_sessionID);
     861    return verifyVaryingRequestHeaders(*platformStrategies()->cookiesStrategy(), m_varyingHeaderValues, request, m_sessionID);
    862862}
    863863

  • trunk/Source/WebCore/platform/network/CacheValidation.cpp

    r239461 r239974  
    327327}
    328328
    329 static String headerValueForVary(const ResourceRequest& request, const String& headerName, PAL::SessionID sessionID)
     329static String cookieRequestHeaderFieldValue(const NetworkStorageSession& session, const ResourceRequest& request)
     330{
     331    return session.cookieRequestHeaderFieldValue(request.firstPartyForCookies(), SameSiteInfo::create(request), request.url(), WTF::nullopt, WTF::nullopt, request.url().protocolIs("https") ? IncludeSecureCookies::Yes : IncludeSecureCookies::No).first;
     332}
     333
     334static String cookieRequestHeaderFieldValue(CookiesStrategy& cookiesStrategy, const PAL::SessionID& sessionID, const ResourceRequest& request)
     335{
     336    return cookiesStrategy.cookieRequestHeaderFieldValue(sessionID, request.firstPartyForCookies(), SameSiteInfo::create(request), request.url(), WTF::nullopt, WTF::nullopt, request.url().protocolIs("https") ? IncludeSecureCookies::Yes : IncludeSecureCookies::No).first;
     337}
     338
     339static String headerValueForVary(const ResourceRequest& request, const String& headerName, Function<String()>&& cookieRequestHeaderFieldValueFunction)
    330340{
    331341    // Explicit handling for cookies is needed because they are added magically by the networking layer.
    … …  
    333343    // We could fetch the cookie when making the request but that seems overkill as the case is very rare and it
    334344    // is a blocking operation. This should be sufficient to cover reasonable cases.
    335     if (headerName == httpHeaderNameString(HTTPHeaderName::Cookie)) {
    336         auto includeSecureCookies = request.url().protocolIs("https") ? IncludeSecureCookies::Yes : IncludeSecureCookies::No;
    337         auto* cookieStrategy = platformStrategies() ? platformStrategies()->cookiesStrategy() : nullptr;
    338         if (!cookieStrategy) {
    339             ASSERT(sessionID == PAL::SessionID::defaultSessionID());
    340             return NetworkStorageSession::defaultStorageSession().cookieRequestHeaderFieldValue(request.firstPartyForCookies(), SameSiteInfo::create(request), request.url(), WTF::nullopt, WTF::nullopt, includeSecureCookies).first;
    341         }
    342         return cookieStrategy->cookieRequestHeaderFieldValue(sessionID, request.firstPartyForCookies(), SameSiteInfo::create(request), request.url(), WTF::nullopt, WTF::nullopt, includeSecureCookies).first;
    343     }
     345    if (headerName == httpHeaderNameString(HTTPHeaderName::Cookie))
     346        return cookieRequestHeaderFieldValueFunction();
    344347    return request.httpHeaderField(headerName);
    345348}
    346349
    347 Vector<std::pair<String, String>> collectVaryingRequestHeaders(const WebCore::ResourceRequest& request, const WebCore::ResourceResponse& response, PAL::SessionID sessionID)
    348 {
    349     String varyValue = response.httpHeaderField(WebCore::HTTPHeaderName::Vary);
     350static Vector<std::pair<String, String>> collectVaryingRequestHeadersInternal(const ResourceResponse& response, Function<String(const String& headerName)>&& headerValueForVaryFunction)
     351{
     352    String varyValue = response.httpHeaderField(HTTPHeaderName::Vary);
    350353    if (varyValue.isEmpty())
    351354        return { };
    … …  
    355358    for (auto& varyHeaderName : varyingHeaderNames) {
    356359        String headerName = varyHeaderName.stripWhiteSpace();
    357         String headerValue = headerValueForVary(request, headerName, sessionID);
     360        String headerValue = headerValueForVaryFunction(headerName);
    358361        varyingRequestHeaders.append(std::make_pair(headerName, headerValue));
    359362    }
    … …  
    361364}
    362365
    363 bool verifyVaryingRequestHeaders(const Vector<std::pair<String, String>>& varyingRequestHeaders, const WebCore::ResourceRequest& request, PAL::SessionID sessionID)
     366Vector<std::pair<String, String>> collectVaryingRequestHeaders(NetworkStorageSession& storageSession, const ResourceRequest& request, const ResourceResponse& response)
     367{
     368    return collectVaryingRequestHeadersInternal(response, [&] (const String& headerName) {
     369        return headerValueForVary(request, headerName, [&] {
     370            return cookieRequestHeaderFieldValue(storageSession, request);
     371        });
     372    });
     373}
     374
     375Vector<std::pair<String, String>> collectVaryingRequestHeaders(CookiesStrategy& cookiesStrategy, const ResourceRequest& request, const ResourceResponse& response, const PAL::SessionID& sessionID)
     376{
     377    return collectVaryingRequestHeadersInternal(response, [&] (const String& headerName) {
     378        return headerValueForVary(request, headerName, [&] {
     379            return cookieRequestHeaderFieldValue(cookiesStrategy, sessionID, request);
     380        });
     381    });
     382}
     383
     384static bool verifyVaryingRequestHeadersInternal(const Vector<std::pair<String, String>>& varyingRequestHeaders, Function<String(const String&)>&& headerValueForVary)
    364385{
    365386    for (auto& varyingRequestHeader : varyingRequestHeaders) {
    … …  
    367388        if (varyingRequestHeader.first == "*")
    368389            return false;
    369         String headerValue = headerValueForVary(request, varyingRequestHeader.first, sessionID);
    370         if (headerValue != varyingRequestHeader.second)
     390        if (headerValueForVary(varyingRequestHeader.first) != varyingRequestHeader.second)
    371391            return false;
    372392    }
    373393    return true;
     394}
     395
     396bool verifyVaryingRequestHeaders(NetworkStorageSession& storageSession, const Vector<std::pair<String, String>>& varyingRequestHeaders, const ResourceRequest& request)
     397{
     398    return verifyVaryingRequestHeadersInternal(varyingRequestHeaders, [&] (const String& headerName) {
     399        return headerValueForVary(request, headerName, [&] {
     400            return cookieRequestHeaderFieldValue(storageSession, request);
     401        });
     402    });
     403}
     404
     405bool verifyVaryingRequestHeaders(CookiesStrategy& cookiesStrategy, const Vector<std::pair<String, String>>& varyingRequestHeaders, const ResourceRequest& request, const PAL::SessionID& sessionID)
     406{
     407    return verifyVaryingRequestHeadersInternal(varyingRequestHeaders, [&] (const String& headerName) {
     408        return headerValueForVary(request, headerName, [&] {
     409            return cookieRequestHeaderFieldValue(cookiesStrategy, sessionID, request);
     410        });
     411    });
    374412}
    375413

  • trunk/Source/WebCore/platform/network/CacheValidation.h

    r235911 r239974  
    3535namespace WebCore {
    3636
     37class CookiesStrategy;
    3738class HTTPHeaderMap;
     39class NetworkStorageSession;
    3840class ResourceRequest;
    3941class ResourceResponse;
    … …  
    7880WEBCORE_EXPORT CacheControlDirectives parseCacheControlDirectives(const HTTPHeaderMap&);
    7981
    80 WEBCORE_EXPORT Vector<std::pair<String, String>> collectVaryingRequestHeaders(const ResourceRequest&, const ResourceResponse&, PAL::SessionID = PAL::SessionID::defaultSessionID());
    81 WEBCORE_EXPORT bool verifyVaryingRequestHeaders(const Vector<std::pair<String, String>>& varyingRequestHeaders, const ResourceRequest&, PAL::SessionID = PAL::SessionID::defaultSessionID());
     82WEBCORE_EXPORT Vector<std::pair<String, String>> collectVaryingRequestHeaders(NetworkStorageSession&, const ResourceRequest&, const ResourceResponse&);
     83WEBCORE_EXPORT Vector<std::pair<String, String>> collectVaryingRequestHeaders(CookiesStrategy&, const ResourceRequest&, const ResourceResponse&, const PAL::SessionID&);
     84WEBCORE_EXPORT bool verifyVaryingRequestHeaders(NetworkStorageSession&, const Vector<std::pair<String, String>>& varyingRequestHeaders, const ResourceRequest&);
     85WEBCORE_EXPORT bool verifyVaryingRequestHeaders(CookiesStrategy&, const Vector<std::pair<String, String>>& varyingRequestHeaders, const ResourceRequest&, const PAL::SessionID&);
    8286
    8387WEBCORE_EXPORT bool isStatusCodeCacheableByDefault(int statusCode);

  • trunk/Source/WebKit/ChangeLog

    r239972 r239974  
     12019-01-14  Alex Christensen  <achristensen@webkit.org>
     2
     3        Split headerValueForVary into specialized functions for NetworkProcess and WebProcess/WebKitLegacy
     4        https://bugs.webkit.org/show_bug.cgi?id=193429
     5
     6        Reviewed by Joseph Pecoraro.
     7
     8        * NetworkProcess/cache/NetworkCache.cpp:
     9        (WebKit::NetworkCache::makeUseDecision):
     10        (WebKit::NetworkCache::Cache::retrieve):
     11        (WebKit::NetworkCache::Cache::makeEntry):
     12        (WebKit::NetworkCache::Cache::makeRedirectEntry):
     13        (WebKit::NetworkCache::Cache::update):
     14
    1152019-01-14  Tim Horton  <timothy_horton@apple.com>
    216

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

    r239759 r239974  
    192192        return UseDecision::Validate;
    193193
    194     if (!WebCore::verifyVaryingRequestHeaders(entry.varyingRequestHeaders(), request))
     194    if (!WebCore::verifyVaryingRequestHeaders(WebCore::NetworkStorageSession::defaultStorageSession(), entry.varyingRequestHeaders(), request))
    195195        return UseDecision::NoDueToVaryingHeaderMismatch;
    196196
    … …  
    308308        m_speculativeLoadManager->retrieve(storageKey, [request, completionHandler = WTFMove(completionHandler), info = WTFMove(info)](std::unique_ptr<Entry> entry) mutable {
    309309            info.wasSpeculativeLoad = true;
    310             if (entry && WebCore::verifyVaryingRequestHeaders(entry->varyingRequestHeaders(), request))
     310            if (entry && WebCore::verifyVaryingRequestHeaders(WebCore::NetworkStorageSession::defaultStorageSession(), entry->varyingRequestHeaders(), request))
    311311                completeRetrieve(WTFMove(completionHandler), WTFMove(entry), info);
    312312            else
    … …  
    365365std::unique_ptr<Entry> Cache::makeEntry(const WebCore::ResourceRequest& request, const WebCore::ResourceResponse& response, RefPtr<WebCore::SharedBuffer>&& responseData)
    366366{
    367     return std::make_unique<Entry>(makeCacheKey(request), response, WTFMove(responseData), WebCore::collectVaryingRequestHeaders(request, response));
     367    return std::make_unique<Entry>(makeCacheKey(request), response, WTFMove(responseData), WebCore::collectVaryingRequestHeaders(WebCore::NetworkStorageSession::defaultStorageSession(), request, response));
    368368}
    369369
    370370std::unique_ptr<Entry> Cache::makeRedirectEntry(const WebCore::ResourceRequest& request, const WebCore::ResourceResponse& response, const WebCore::ResourceRequest& redirectRequest)
    371371{
    372     return std::make_unique<Entry>(makeCacheKey(request), response, redirectRequest, WebCore::collectVaryingRequestHeaders(request, response));
     372    return std::make_unique<Entry>(makeCacheKey(request), response, redirectRequest, WebCore::collectVaryingRequestHeaders(WebCore::NetworkStorageSession::defaultStorageSession(), request, response));
    373373}
    374374
    … …  
    454454    WebCore::updateResponseHeadersAfterRevalidation(response, validatingResponse);
    455455
    456     auto updateEntry = std::make_unique<Entry>(existingEntry.key(), response, existingEntry.buffer(), WebCore::collectVaryingRequestHeaders(originalRequest, response));
     456    auto updateEntry = std::make_unique<Entry>(existingEntry.key(), response, existingEntry.buffer(), WebCore::collectVaryingRequestHeaders(WebCore::NetworkStorageSession::defaultStorageSession(), originalRequest, response));
    457457    auto updateRecord = updateEntry->encodeAsStorageRecord();
    458458
Note: See TracChangeset for help on using the changeset viewer.