[WK2] 304 revalidation on the network process does not update the validated response
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 Oct 2016 08:30:01 +0000 (08:30 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 Oct 2016 08:30:01 +0000 (08:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=162973

Patch by Youenn Fablet <youenn@apple.com> on 2016-10-06
Reviewed by Darin Adler.

LayoutTests/imported/w3c:

* web-platform-tests/fetch/api/basic/conditional-get-expected.txt: Added.
* web-platform-tests/fetch/api/basic/conditional-get.html: Added.
* web-platform-tests/fetch/api/resources/cache.py: Added.

Source/WebKit2:

* NetworkProcess/NetworkResourceLoader.cpp: Updating cache entry with the revalidated one.

LayoutTests:

* http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header-expected.txt:
Rebasing expectation as memory cache revalidation is no longer needed now that the disk cache is updating the response passed to the memory cache.
The disk cache is doing revalidation on the second load. It receives the updated response with longer validity.
As the extended validity response is now passed to the memory cache, the memory cache revalidation no longer happens.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header-expected.txt
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/conditional-get-expected.txt [new file with mode: 0644]
LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/conditional-get.html [new file with mode: 0644]
LayoutTests/imported/w3c/web-platform-tests/fetch/api/resources/cache.py [new file with mode: 0644]
Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp

index 301d4e0..03cbc7f 100644 (file)
@@ -1,3 +1,15 @@
+2016-10-06  Youenn Fablet  <youenn@apple.com>
+
+        [WK2] 304 revalidation on the network process does not update the validated response
+        https://bugs.webkit.org/show_bug.cgi?id=162973
+
+        Reviewed by Darin Adler.
+
+        * http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header-expected.txt:
+        Rebasing expectation as memory cache revalidation is no longer needed now that the disk cache is updating the response passed to the memory cache.
+        The disk cache is doing revalidation on the second load. It receives the updated response with longer validity.
+        As the extended validity response is now passed to the memory cache, the memory cache revalidation no longer happens.
+
 2016-10-06  Nan Wang  <n_wang@apple.com>
 
         AX:[Mac] Unable to edit text input, textarea fields in iframe using VO naivgation
index b629a56..9ec5fd0 100644 (file)
@@ -12,12 +12,12 @@ response source: Disk cache after validation
 --------Testing loads through memory cache (XHR behavior)--------
 response headers: {"Expires":"now(0)","ETag":"match"}
 response's 'Expires' header is overriden by future date in 304 response
-response source: Memory cache after validation
+response source: Memory cache
 
 --------Testing loads through memory cache (subresource behavior)--------
 response headers: {"Expires":"now(0)","ETag":"match"}
 response's 'Expires' header is overriden by future date in 304 response
-response source: Memory cache after validation
+response source: Memory cache
 
 304 response included an 'Expires' header in the future, so we should not need to revalidate this time.
 --------Testing loads from disk cache--------
index 7ca98f0..ccbccd2 100644 (file)
@@ -1,3 +1,14 @@
+2016-10-06  Youenn Fablet  <youenn@apple.com>
+
+        [WK2] 304 revalidation on the network process does not update the validated response
+        https://bugs.webkit.org/show_bug.cgi?id=162973
+
+        Reviewed by Darin Adler.
+
+        * web-platform-tests/fetch/api/basic/conditional-get-expected.txt: Added.
+        * web-platform-tests/fetch/api/basic/conditional-get.html: Added.
+        * web-platform-tests/fetch/api/resources/cache.py: Added.
+
 2016-10-05  Ryosuke Niwa  <rniwa@webkit.org>
 
         Import v1 custom elements tests from W3C
diff --git a/LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/conditional-get-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/conditional-get-expected.txt
new file mode 100644 (file)
index 0000000..f1ec6f2
--- /dev/null
@@ -0,0 +1,3 @@
+
+PASS Testing conditional GET with ETags 
+
diff --git a/LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/conditional-get.html b/LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/conditional-get.html
new file mode 100644 (file)
index 0000000..b80e929
--- /dev/null
@@ -0,0 +1,51 @@
+<!doctype html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>Request ETag</title>
+    <meta name="help" href="https://fetch.spec.whatwg.org/#request">
+    <script src="/resources/testharness.js"></script>
+    <script src="/resources/testharnessreport.js"></script>
+    <script src="/common/utils.js"></script>
+  </head>
+  <body>
+    <script>
+    promise_test(function() {
+      var cacheBuster = token(); // ensures first request is uncached
+      var url = "../resources/cache.py?v=" + cacheBuster;
+      var etag;
+
+      // make the first request
+      return fetch(url).then(function(response) {
+        // ensure we're getting the regular, uncached response
+        assert_equals(response.status, 200);
+        assert_equals(response.headers.get("X-HTTP-STATUS"), null)
+
+        return response.text(); // consuming the body, just to be safe
+      }).then(function(body) {
+        // make a second request
+        return fetch(url);
+      }).then(function(response) {
+        // while the server responds with 304 if our browser sent the correct
+        // If-None-Match request header, at the JavaScript level this surfaces
+        // as 200
+        assert_equals(response.status, 200);
+        assert_equals(response.headers.get("X-HTTP-STATUS"), "304")
+
+        etag = response.headers.get("ETag")
+
+        return response.text(); // consuming the body, just to be safe
+      }).then(function(body) {
+        // make a third request, explicitly setting If-None-Match request header
+        var headers = { "If-None-Match": etag }
+        return fetch(url, { headers: headers })
+      }).then(function(response) {
+        // 304 now surfaces thanks to the explicit If-None-Match request header
+        assert_equals(response.status, 304);
+      });
+    }, "Testing conditional GET with ETags");
+
+    done();
+    </script>
+  </body>
+</html>
diff --git a/LayoutTests/imported/w3c/web-platform-tests/fetch/api/resources/cache.py b/LayoutTests/imported/w3c/web-platform-tests/fetch/api/resources/cache.py
new file mode 100644 (file)
index 0000000..8996387
--- /dev/null
@@ -0,0 +1,18 @@
+ETAG = '"123abc"'
+CONTENT_TYPE = "text/plain"
+CONTENT = "lorem ipsum dolor sit amet"
+
+
+def main(request, response):
+    # let caching kick in if possible (conditional GET)
+    etag = request.headers.get("If-None-Match", None)
+    if etag == ETAG:
+        response.headers.set("X-HTTP-STATUS", 304)
+        response.status = (304, "Not Modified")
+        return ""
+
+    # cache miss, so respond with the actual content
+    response.status = (200, "OK")
+    response.headers.set("ETag", ETAG)
+    response.headers.set("Content-Type", CONTENT_TYPE)
+    return CONTENT
index d1a0e90..fb708ac 100644 (file)
@@ -1,3 +1,12 @@
+2016-10-06  Youenn Fablet  <youenn@apple.com>
+
+        [WK2] 304 revalidation on the network process does not update the validated response
+        https://bugs.webkit.org/show_bug.cgi?id=162973
+
+        Reviewed by Darin Adler.
+
+        * NetworkProcess/NetworkResourceLoader.cpp: Updating cache entry with the revalidated one.
+
 2016-10-05  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Introduce InputEvent bindings in preparation for the input events spec
index ad280a3..2cbef9e 100644 (file)
@@ -325,9 +325,8 @@ auto NetworkResourceLoader::didReceiveResponse(ResourceResponse&& receivedRespon
     if (m_cacheEntryForValidation) {
         bool validationSucceeded = m_response.httpStatusCode() == 304; // 304 Not Modified
         if (validationSucceeded) {
-            NetworkCache::singleton().update(originalRequest(), { m_parameters.webPageID, m_parameters.webFrameID }, *m_cacheEntryForValidation, m_response);
-            // If the request was conditional then this revalidation was not triggered by the network cache and we pass the
-            // 304 response to WebCore.
+            m_cacheEntryForValidation = NetworkCache::singleton().update(originalRequest(), { m_parameters.webPageID, m_parameters.webFrameID }, *m_cacheEntryForValidation, m_response);
+            // If the request was conditional then this revalidation was not triggered by the network cache and we pass the 304 response to WebCore.
             if (originalRequest().isConditional())
                 m_cacheEntryForValidation = nullptr;
         } else