Regression(r188698): http/tests/cache/disk-cache/disk-cache-revalidation-new-expire...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Aug 2015 16:18:52 +0000 (16:18 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Aug 2015 16:18:52 +0000 (16:18 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@188755 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html
Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp

index 160c001..68a7321 100644 (file)
@@ -1,3 +1,13 @@
+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>
 
         AX: fix accessibility/loading-iframe-updates-axtree.html test for mac
index 6b42041..033ca0f 100644 (file)
@@ -13,11 +13,8 @@ description("Test that the 'Expires' header is updated upon successful validatio
 debug("");
 
 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);
 });
 
 </script>
index 2656dc0..44d8631 100644 (file)
@@ -1,3 +1,25 @@
+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>
 
         [EFL] Revise PlatformWebView ctor according to r188718
index 163f6b0..e788115 100644 (file)
@@ -407,8 +407,11 @@ void Cache::store(const WebCore::ResourceRequest& originalRequest, const WebCore
         LOG(NetworkCache, "(NetworkProcess) didn't store, storeDecision=%d", storeDecision);
         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)
             m_statistics->recordNotCachingResponse(key, storeDecision);