Diff
Modified: trunk/Source/WebCore/ChangeLog (239973 => 239974)
--- trunk/Source/WebCore/ChangeLog 2019-01-15 04:11:15 UTC (rev 239973)
+++ trunk/Source/WebCore/ChangeLog 2019-01-15 05:15:57 UTC (rev 239974)
@@ -1,3 +1,30 @@
+2019-01-14 Alex Christensen <achristen...@webkit.org>
+
+ Split headerValueForVary into specialized functions for NetworkProcess and WebProcess/WebKitLegacy
+ https://bugs.webkit.org/show_bug.cgi?id=193429
+
+ Reviewed by Joseph Pecoraro.
+
+ 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:
+
2019-01-14 Simon Fraser <simon.fra...@apple.com>
Only run the node comparison code in FrameSelection::respondToNodeModification() for range selections
Modified: trunk/Source/WebCore/loader/cache/CachedResource.cpp (239973 => 239974)
--- trunk/Source/WebCore/loader/cache/CachedResource.cpp 2019-01-15 04:11:15 UTC (rev 239973)
+++ trunk/Source/WebCore/loader/cache/CachedResource.cpp 2019-01-15 05:15:57 UTC (rev 239974)
@@ -476,7 +476,7 @@
{
ASSERT(m_response.type() == ResourceResponse::Type::Default);
m_response = response;
- m_varyingHeaderValues = collectVaryingRequestHeaders(m_resourceRequest, m_response, m_sessionID);
+ m_varyingHeaderValues = collectVaryingRequestHeaders(*platformStrategies()->cookiesStrategy(), m_resourceRequest, m_response, m_sessionID);
#if ENABLE(SERVICE_WORKER)
if (m_response.source() == ResourceResponse::Source::ServiceWorker) {
@@ -858,7 +858,7 @@
if (m_varyingHeaderValues.isEmpty())
return true;
- return verifyVaryingRequestHeaders(m_varyingHeaderValues, request, m_sessionID);
+ return verifyVaryingRequestHeaders(*platformStrategies()->cookiesStrategy(), m_varyingHeaderValues, request, m_sessionID);
}
unsigned CachedResource::overheadSize() const
Modified: trunk/Source/WebCore/platform/network/CacheValidation.cpp (239973 => 239974)
--- trunk/Source/WebCore/platform/network/CacheValidation.cpp 2019-01-15 04:11:15 UTC (rev 239973)
+++ trunk/Source/WebCore/platform/network/CacheValidation.cpp 2019-01-15 05:15:57 UTC (rev 239974)
@@ -326,27 +326,30 @@
return result;
}
-static String headerValueForVary(const ResourceRequest& request, const String& headerName, PAL::SessionID sessionID)
+static String cookieRequestHeaderFieldValue(const NetworkStorageSession& session, const ResourceRequest& request)
{
+ return session.cookieRequestHeaderFieldValue(request.firstPartyForCookies(), SameSiteInfo::create(request), request.url(), WTF::nullopt, WTF::nullopt, request.url().protocolIs("https") ? IncludeSecureCookies::Yes : IncludeSecureCookies::No).first;
+}
+
+static String cookieRequestHeaderFieldValue(CookiesStrategy& cookiesStrategy, const PAL::SessionID& sessionID, const ResourceRequest& request)
+{
+ return cookiesStrategy.cookieRequestHeaderFieldValue(sessionID, request.firstPartyForCookies(), SameSiteInfo::create(request), request.url(), WTF::nullopt, WTF::nullopt, request.url().protocolIs("https") ? IncludeSecureCookies::Yes : IncludeSecureCookies::No).first;
+}
+
+static String headerValueForVary(const ResourceRequest& request, const String& headerName, Function<String()>&& cookieRequestHeaderFieldValueFunction)
+{
// Explicit handling for cookies is needed because they are added magically by the networking layer.
// FIXME: The value might have changed between making the request and retrieving the cookie here.
// We could fetch the cookie when making the request but that seems overkill as the case is very rare and it
// is a blocking operation. This should be sufficient to cover reasonable cases.
- if (headerName == httpHeaderNameString(HTTPHeaderName::Cookie)) {
- auto includeSecureCookies = request.url().protocolIs("https") ? IncludeSecureCookies::Yes : IncludeSecureCookies::No;
- auto* cookieStrategy = platformStrategies() ? platformStrategies()->cookiesStrategy() : nullptr;
- if (!cookieStrategy) {
- ASSERT(sessionID == PAL::SessionID::defaultSessionID());
- return NetworkStorageSession::defaultStorageSession().cookieRequestHeaderFieldValue(request.firstPartyForCookies(), SameSiteInfo::create(request), request.url(), WTF::nullopt, WTF::nullopt, includeSecureCookies).first;
- }
- return cookieStrategy->cookieRequestHeaderFieldValue(sessionID, request.firstPartyForCookies(), SameSiteInfo::create(request), request.url(), WTF::nullopt, WTF::nullopt, includeSecureCookies).first;
- }
+ if (headerName == httpHeaderNameString(HTTPHeaderName::Cookie))
+ return cookieRequestHeaderFieldValueFunction();
return request.httpHeaderField(headerName);
}
-Vector<std::pair<String, String>> collectVaryingRequestHeaders(const WebCore::ResourceRequest& request, const WebCore::ResourceResponse& response, PAL::SessionID sessionID)
+static Vector<std::pair<String, String>> collectVaryingRequestHeadersInternal(const ResourceResponse& response, Function<String(const String& headerName)>&& headerValueForVaryFunction)
{
- String varyValue = response.httpHeaderField(WebCore::HTTPHeaderName::Vary);
+ String varyValue = response.httpHeaderField(HTTPHeaderName::Vary);
if (varyValue.isEmpty())
return { };
Vector<String> varyingHeaderNames = varyValue.split(',');
@@ -354,25 +357,60 @@
varyingRequestHeaders.reserveCapacity(varyingHeaderNames.size());
for (auto& varyHeaderName : varyingHeaderNames) {
String headerName = varyHeaderName.stripWhiteSpace();
- String headerValue = headerValueForVary(request, headerName, sessionID);
+ String headerValue = headerValueForVaryFunction(headerName);
varyingRequestHeaders.append(std::make_pair(headerName, headerValue));
}
return varyingRequestHeaders;
}
-bool verifyVaryingRequestHeaders(const Vector<std::pair<String, String>>& varyingRequestHeaders, const WebCore::ResourceRequest& request, PAL::SessionID sessionID)
+Vector<std::pair<String, String>> collectVaryingRequestHeaders(NetworkStorageSession& storageSession, const ResourceRequest& request, const ResourceResponse& response)
{
+ return collectVaryingRequestHeadersInternal(response, [&] (const String& headerName) {
+ return headerValueForVary(request, headerName, [&] {
+ return cookieRequestHeaderFieldValue(storageSession, request);
+ });
+ });
+}
+
+Vector<std::pair<String, String>> collectVaryingRequestHeaders(CookiesStrategy& cookiesStrategy, const ResourceRequest& request, const ResourceResponse& response, const PAL::SessionID& sessionID)
+{
+ return collectVaryingRequestHeadersInternal(response, [&] (const String& headerName) {
+ return headerValueForVary(request, headerName, [&] {
+ return cookieRequestHeaderFieldValue(cookiesStrategy, sessionID, request);
+ });
+ });
+}
+
+static bool verifyVaryingRequestHeadersInternal(const Vector<std::pair<String, String>>& varyingRequestHeaders, Function<String(const String&)>&& headerValueForVary)
+{
for (auto& varyingRequestHeader : varyingRequestHeaders) {
// FIXME: Vary: * in response would ideally trigger a cache delete instead of a store.
if (varyingRequestHeader.first == "*")
return false;
- String headerValue = headerValueForVary(request, varyingRequestHeader.first, sessionID);
- if (headerValue != varyingRequestHeader.second)
+ if (headerValueForVary(varyingRequestHeader.first) != varyingRequestHeader.second)
return false;
}
return true;
}
+bool verifyVaryingRequestHeaders(NetworkStorageSession& storageSession, const Vector<std::pair<String, String>>& varyingRequestHeaders, const ResourceRequest& request)
+{
+ return verifyVaryingRequestHeadersInternal(varyingRequestHeaders, [&] (const String& headerName) {
+ return headerValueForVary(request, headerName, [&] {
+ return cookieRequestHeaderFieldValue(storageSession, request);
+ });
+ });
+}
+
+bool verifyVaryingRequestHeaders(CookiesStrategy& cookiesStrategy, const Vector<std::pair<String, String>>& varyingRequestHeaders, const ResourceRequest& request, const PAL::SessionID& sessionID)
+{
+ return verifyVaryingRequestHeadersInternal(varyingRequestHeaders, [&] (const String& headerName) {
+ return headerValueForVary(request, headerName, [&] {
+ return cookieRequestHeaderFieldValue(cookiesStrategy, sessionID, request);
+ });
+ });
+}
+
// http://tools.ietf.org/html/rfc7231#page-48
bool isStatusCodeCacheableByDefault(int statusCode)
{
Modified: trunk/Source/WebCore/platform/network/CacheValidation.h (239973 => 239974)
--- trunk/Source/WebCore/platform/network/CacheValidation.h 2019-01-15 04:11:15 UTC (rev 239973)
+++ trunk/Source/WebCore/platform/network/CacheValidation.h 2019-01-15 05:15:57 UTC (rev 239974)
@@ -34,7 +34,9 @@
namespace WebCore {
+class CookiesStrategy;
class HTTPHeaderMap;
+class NetworkStorageSession;
class ResourceRequest;
class ResourceResponse;
@@ -77,8 +79,10 @@
};
WEBCORE_EXPORT CacheControlDirectives parseCacheControlDirectives(const HTTPHeaderMap&);
-WEBCORE_EXPORT Vector<std::pair<String, String>> collectVaryingRequestHeaders(const ResourceRequest&, const ResourceResponse&, PAL::SessionID = PAL::SessionID::defaultSessionID());
-WEBCORE_EXPORT bool verifyVaryingRequestHeaders(const Vector<std::pair<String, String>>& varyingRequestHeaders, const ResourceRequest&, PAL::SessionID = PAL::SessionID::defaultSessionID());
+WEBCORE_EXPORT Vector<std::pair<String, String>> collectVaryingRequestHeaders(NetworkStorageSession&, const ResourceRequest&, const ResourceResponse&);
+WEBCORE_EXPORT Vector<std::pair<String, String>> collectVaryingRequestHeaders(CookiesStrategy&, const ResourceRequest&, const ResourceResponse&, const PAL::SessionID&);
+WEBCORE_EXPORT bool verifyVaryingRequestHeaders(NetworkStorageSession&, const Vector<std::pair<String, String>>& varyingRequestHeaders, const ResourceRequest&);
+WEBCORE_EXPORT bool verifyVaryingRequestHeaders(CookiesStrategy&, const Vector<std::pair<String, String>>& varyingRequestHeaders, const ResourceRequest&, const PAL::SessionID&);
WEBCORE_EXPORT bool isStatusCodeCacheableByDefault(int statusCode);
WEBCORE_EXPORT bool isStatusCodePotentiallyCacheable(int statusCode);
Modified: trunk/Source/WebKit/ChangeLog (239973 => 239974)
--- trunk/Source/WebKit/ChangeLog 2019-01-15 04:11:15 UTC (rev 239973)
+++ trunk/Source/WebKit/ChangeLog 2019-01-15 05:15:57 UTC (rev 239974)
@@ -1,3 +1,17 @@
+2019-01-14 Alex Christensen <achristen...@webkit.org>
+
+ Split headerValueForVary into specialized functions for NetworkProcess and WebProcess/WebKitLegacy
+ https://bugs.webkit.org/show_bug.cgi?id=193429
+
+ Reviewed by Joseph Pecoraro.
+
+ * NetworkProcess/cache/NetworkCache.cpp:
+ (WebKit::NetworkCache::makeUseDecision):
+ (WebKit::NetworkCache::Cache::retrieve):
+ (WebKit::NetworkCache::Cache::makeEntry):
+ (WebKit::NetworkCache::Cache::makeRedirectEntry):
+ (WebKit::NetworkCache::Cache::update):
+
2019-01-14 Tim Horton <timothy_hor...@apple.com>
Fix a style mistake in PageClientImplMac
Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCache.cpp (239973 => 239974)
--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCache.cpp 2019-01-15 04:11:15 UTC (rev 239973)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCache.cpp 2019-01-15 05:15:57 UTC (rev 239974)
@@ -191,7 +191,7 @@
if (request.isConditional() && !entry.redirectRequest())
return UseDecision::Validate;
- if (!WebCore::verifyVaryingRequestHeaders(entry.varyingRequestHeaders(), request))
+ if (!WebCore::verifyVaryingRequestHeaders(WebCore::NetworkStorageSession::defaultStorageSession(), entry.varyingRequestHeaders(), request))
return UseDecision::NoDueToVaryingHeaderMismatch;
// We never revalidate in the case of a history navigation.
@@ -307,7 +307,7 @@
if (canUseSpeculativeRevalidation && m_speculativeLoadManager->canRetrieve(storageKey, request, frameID)) {
m_speculativeLoadManager->retrieve(storageKey, [request, completionHandler = WTFMove(completionHandler), info = WTFMove(info)](std::unique_ptr<Entry> entry) mutable {
info.wasSpeculativeLoad = true;
- if (entry && WebCore::verifyVaryingRequestHeaders(entry->varyingRequestHeaders(), request))
+ if (entry && WebCore::verifyVaryingRequestHeaders(WebCore::NetworkStorageSession::defaultStorageSession(), entry->varyingRequestHeaders(), request))
completeRetrieve(WTFMove(completionHandler), WTFMove(entry), info);
else
completeRetrieve(WTFMove(completionHandler), nullptr, info);
@@ -364,12 +364,12 @@
std::unique_ptr<Entry> Cache::makeEntry(const WebCore::ResourceRequest& request, const WebCore::ResourceResponse& response, RefPtr<WebCore::SharedBuffer>&& responseData)
{
- return std::make_unique<Entry>(makeCacheKey(request), response, WTFMove(responseData), WebCore::collectVaryingRequestHeaders(request, response));
+ return std::make_unique<Entry>(makeCacheKey(request), response, WTFMove(responseData), WebCore::collectVaryingRequestHeaders(WebCore::NetworkStorageSession::defaultStorageSession(), request, response));
}
std::unique_ptr<Entry> Cache::makeRedirectEntry(const WebCore::ResourceRequest& request, const WebCore::ResourceResponse& response, const WebCore::ResourceRequest& redirectRequest)
{
- return std::make_unique<Entry>(makeCacheKey(request), response, redirectRequest, WebCore::collectVaryingRequestHeaders(request, response));
+ return std::make_unique<Entry>(makeCacheKey(request), response, redirectRequest, WebCore::collectVaryingRequestHeaders(WebCore::NetworkStorageSession::defaultStorageSession(), request, response));
}
std::unique_ptr<Entry> Cache::store(const WebCore::ResourceRequest& request, const WebCore::ResourceResponse& response, RefPtr<WebCore::SharedBuffer>&& responseData, Function<void(MappedBody&)>&& completionHandler)
@@ -453,7 +453,7 @@
WebCore::ResourceResponse response = existingEntry.response();
WebCore::updateResponseHeadersAfterRevalidation(response, validatingResponse);
- auto updateEntry = std::make_unique<Entry>(existingEntry.key(), response, existingEntry.buffer(), WebCore::collectVaryingRequestHeaders(originalRequest, response));
+ auto updateEntry = std::make_unique<Entry>(existingEntry.key(), response, existingEntry.buffer(), WebCore::collectVaryingRequestHeaders(WebCore::NetworkStorageSession::defaultStorageSession(), originalRequest, response));
auto updateRecord = updateEntry->encodeAsStorageRecord();
m_storage->store(updateRecord, { });
