Changeset 201600 in webkit

Timestamp:

Jun 2, 2016, 9:40:05 AM (10 years ago)

Author:

Chris Dumez

Message:

Speculative revalidated request returns 200 instead of 304
​https://bugs.webkit.org/show_bug.cgi?id=158256

Reviewed by Antti Koivisto and Carlos Garcia Campos.

Already conditional requests coming from WebCore should bypass the
SpeculativeLoadManager. This is because the validation is requested
by WebCore (usually the memory cache) and not our network cache. As
a result, we need to pipe the revalidation response (e.g. 304 status
code) back to WebCore instead of resolving it as disk cache level.

This should fix the /webkit2/WebKitWebResource/response unit test.

• NetworkProcess/cache/NetworkCache.cpp:

(WebKit::NetworkCache::Cache::retrieve):

• NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp:

(WebKit::NetworkCache::requestsHeadersMatch):

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/cache/NetworkCache.cpp (modified) (2 diffs)
• NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp (modified) (1 diff)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2016-06-02  Chris Dumez  <cdumez@apple.com>
+
+        Speculative revalidated request returns 200 instead of 304
+        https://bugs.webkit.org/show_bug.cgi?id=158256
+
+        Reviewed by Antti Koivisto and Carlos Garcia Campos.
+
+        Already conditional requests coming from WebCore should bypass the
+        SpeculativeLoadManager. This is because the validation is requested
+        by WebCore (usually the memory cache) and not our network cache. As
+        a result, we need to pipe the revalidation response (e.g. 304 status
+        code) back to WebCore instead of resolving it as disk cache level.
+
+        This should fix the /webkit2/WebKitWebResource/response unit test.
+
+        * NetworkProcess/cache/NetworkCache.cpp:
+        (WebKit::NetworkCache::Cache::retrieve):
+        * NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp:
+        (WebKit::NetworkCache::requestsHeadersMatch):
+
 2016-06-02  Youenn Fablet  <youenn.fablet@crf.canon.fr>
 

trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp

@@ -359 +359 @@
 
 #if ENABLE(NETWORK_CACHE_SPECULATIVE_REVALIDATION)
-    if (m_speculativeLoadManager)
+    bool canUseSpeculativeRevalidation = m_speculativeLoadManager && !request.isConditional();
+    if (canUseSpeculativeRevalidation)
         m_speculativeLoadManager->registerLoad(frameID, request, storageKey);
 #endif
@@ -373 +374 @@
 
 #if ENABLE(NETWORK_CACHE_SPECULATIVE_REVALIDATION)
-    if (m_speculativeLoadManager && m_speculativeLoadManager->retrieve(frameID, storageKey, request, [request, completionHandler](std::unique_ptr<Entry> entry) {
+    if (canUseSpeculativeRevalidation && m_speculativeLoadManager->retrieve(frameID, storageKey, request, [request, completionHandler](std::unique_ptr<Entry> entry) {
         if (entry && verifyVaryingRequestHeaders(entry->varyingRequestHeaders(), request))
             completionHandler(WTFMove(entry));

trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp

@@ -270 +270 @@
 #endif
 
-static bool requestsHeadersMatch(const ResourceRequest& a, const ResourceRequest& b)
-{
-    static const HTTPHeaderName headersAllowedToMismatch[] = {
-        HTTPHeaderName::IfMatch,
-        HTTPHeaderName::IfModifiedSince,
-        HTTPHeaderName::IfNoneMatch,
-        HTTPHeaderName::IfRange,
-        HTTPHeaderName::IfUnmodifiedSince,
-        HTTPHeaderName::CacheControl
-    };
-
-    HTTPHeaderMap headersA = a.httpHeaderFields();
-    HTTPHeaderMap headersB = b.httpHeaderFields();
-    for (auto headerName : headersAllowedToMismatch) {
-        headersA.remove(headerName);
-        headersB.remove(headerName);
-    }
-
-    if (headersA != headersB) {
+static bool requestsHeadersMatch(const ResourceRequest& speculativeValidationRequest, const ResourceRequest& actualRequest)
+{
+    ASSERT(!actualRequest.isConditional());
+    ResourceRequest speculativeRequest = speculativeValidationRequest;
+    speculativeRequest.makeUnconditional();
+
+    if (speculativeRequest.httpHeaderFields() != actualRequest.httpHeaderFields()) {
         LOG(NetworkCacheSpeculativePreloading, "Cannot reuse speculatively validated entry because HTTP headers used for validation do not match");
 #if !LOG_DISABLED
-        dumpHTTPHeadersDiff(headersA, headersB);
+        dumpHTTPHeadersDiff(speculativeRequest.httpHeaderFields(), actualRequest.httpHeaderFields());
 #endif
         return false;