Changeset 188755 in webkit

Timestamp:

Aug 21, 2015 9:18:52 AM (9 years ago)

Author:

Chris Dumez

Message:

Regression(r188698): http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html is very flaky
​https://bugs.webkit.org/show_bug.cgi?id=148205

Reviewed by Antti Koivisto.

Source/WebKit2:

After r188640, successful revalidation of resources in the memory cache
would cause us to drop the corresponding resource in the disk cache.
This patch addresses the issue by not removing the cache entry if the
response is a successful revalidation (i.e. status code == 304).

Longer term, we should probably update the entry in the disk cache (if
it exists) when it is revalidated by the memory cache. Currently,
revalidation by the memory cache bypasses the disk cache and goes
straight to the network. Then, when the response comes back as a 304,
we try and store the response in the cache. However, a 304 status code
is not cacheable so the cache rejects it.

• NetworkProcess/cache/NetworkCache.cpp:

(WebKit::NetworkCache::Cache::store):

LayoutTests:

• http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html:

Drop temporary fix landed in r188698 to make the test less flaky.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html (modified) (1 diff)
• Source/WebKit2/ChangeLog (modified) (1 diff)
• Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2015-08-21  Chris Dumez  <cdumez@apple.com>
+
+        Regression(r188698): http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html is very flaky
+        https://bugs.webkit.org/show_bug.cgi?id=148205
+
+        Reviewed by Antti Koivisto.
+
+        * http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html:
+        Drop temporary fix landed in r188698 to make the test less flaky.
+
 2015-08-20  Nan Wang  <n_wang@apple.com>
 

trunk/LayoutTests/http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html

@@ -14 +14 @@
 
 runTests(tests, function() {
-    // Wait for things to settle down in the cache.
-    setTimeout(function() {
-        debug("304 response included an 'Expires' header in the future, so we should not need to revalidate this time.");
-        runTests(tests);
-    }, 200);
+    debug("304 response included an 'Expires' header in the future, so we should not need to revalidate this time.");
+    runTests(tests);
 });
 

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2015-08-21  Chris Dumez  <cdumez@apple.com>
+
+        Regression(r188698): http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html is very flaky
+        https://bugs.webkit.org/show_bug.cgi?id=148205
+
+        Reviewed by Antti Koivisto.
+
+        After r188640, successful revalidation of resources in the memory cache
+        would cause us to drop the corresponding resource in the disk cache.
+        This patch addresses the issue by not removing the cache entry if the
+        response is a successful revalidation (i.e. status code == 304).
+
+        Longer term, we should probably update the entry in the disk cache (if
+        it exists) when it is revalidated by the memory cache. Currently,
+        revalidation by the memory cache bypasses the disk cache and goes
+        straight to the network. Then, when the response comes back as a 304,
+        we try and store the response in the cache. However, a 304 status code
+        is not cacheable so the cache rejects it.
+
+        * NetworkProcess/cache/NetworkCache.cpp:
+        (WebKit::NetworkCache::Cache::store):
+
 2015-08-20  Joonghun Park  <jh718.park@samsung.com>
 

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

@@ -408 +408 @@
         auto key = makeCacheKey(originalRequest);
 
-        // Make sure we don't keep a stale entry in the cache.
-        remove(key);
+        auto isSuccessfulRevalidation = response.httpStatusCode() == 304;
+        if (!isSuccessfulRevalidation) {
+            // Make sure we don't keep a stale entry in the cache.
+            remove(key);
+        }
 
         if (m_statistics)