Network Cache: Stale content after back navigation
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 31 Aug 2015 18:31:04 +0000 (18:31 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 31 Aug 2015 18:31:04 +0000 (18:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=148634

Reviewed by Chris Dumez.

Source/WebKit2:

It is possible to get an older version of the previous page when navigating back. This can happen
if the main resource load has not completed before navigating away from the page.

Network cache entry is normally updated when the load completes. In case of cancellation we would leave
any existing entry as-is. However we render incrementally and user might have seen some content from
the partial load already. Navigating back to the cached page could show older version of the content.

* NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::abort):

    If a network load is canceled by the client after receiving response but before the load has completed
    remove any existing cache entry for it.

LayoutTests:

* http/tests/cache/disk-cache/disk-cache-302-status-code.html:
* http/tests/cache/disk-cache/disk-cache-cancel-expected.txt: Added.
* http/tests/cache/disk-cache/disk-cache-cancel.html: Added.
* http/tests/cache/disk-cache/resources/cache-test.js:

    Support delayed responses so we can test canceling the load.
    Some minor improvements.

(makeHeaderValue):
(generateTestURL):
(loadResource):
(loadResourcesWithOptions):
(generateTests):
* http/tests/cache/disk-cache/resources/generate-response.cgi:

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

LayoutTests/ChangeLog
LayoutTests/http/tests/cache/disk-cache/disk-cache-302-status-code.html
LayoutTests/http/tests/cache/disk-cache/disk-cache-cancel-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/cache/disk-cache/disk-cache-cancel.html [new file with mode: 0644]
LayoutTests/http/tests/cache/disk-cache/resources/cache-test.js
LayoutTests/http/tests/cache/disk-cache/resources/generate-response.cgi
Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp

index e573433..b785312 100644 (file)
@@ -1,3 +1,25 @@
+2015-08-31  Antti Koivisto  <antti@apple.com>
+
+        Network Cache: Stale content after back navigation
+        https://bugs.webkit.org/show_bug.cgi?id=148634
+
+        Reviewed by Chris Dumez.
+
+        * http/tests/cache/disk-cache/disk-cache-302-status-code.html:
+        * http/tests/cache/disk-cache/disk-cache-cancel-expected.txt: Added.
+        * http/tests/cache/disk-cache/disk-cache-cancel.html: Added.
+        * http/tests/cache/disk-cache/resources/cache-test.js:
+
+            Support delayed responses so we can test canceling the load.
+            Some minor improvements.
+
+        (makeHeaderValue):
+        (generateTestURL):
+        (loadResource):
+        (loadResourcesWithOptions):
+        (generateTests):
+        * http/tests/cache/disk-cache/resources/generate-response.cgi:
+
 2015-08-31  Zalan Bujtas  <zalan@apple.com>
 
         Repaint cleanup: 4776765.html. Use repaint rect tracking.
index 0128125..1f461d3 100644 (file)
@@ -5,8 +5,8 @@
 
 var tests =
 [
- { responseHeaders: {'Status': '302', 'Location': '/', 'Cache-control': 'max-age=0' }, includeBody: false },
- { responseHeaders: {'Status': '302', 'Location': '/', 'Cache-control': 'max-age=100' }, includeBody: false },
+ { responseHeaders: {'Status': '302', 'Location': '/', 'Cache-control': 'max-age=0' }, body: "" },
+ { responseHeaders: {'Status': '302', 'Location': '/', 'Cache-control': 'max-age=100' }, body: "" },
 ];
 
 description("Test that responses with HTTP status code 302 are not cached");
diff --git a/LayoutTests/http/tests/cache/disk-cache/disk-cache-cancel-expected.txt b/LayoutTests/http/tests/cache/disk-cache/disk-cache-cancel-expected.txt
new file mode 100644 (file)
index 0000000..463e91f
--- /dev/null
@@ -0,0 +1,23 @@
+Test that canceled network loads don't leave behind stale cache entries
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+running 3 tests
+
+Warming cache
+Starting loads, then canceling
+Loading again with back navigation cache policy
+response headers: {"Cache-control":"max-age=0"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=100"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=0","ETag":"match"}
+response source: Network
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/cache/disk-cache/disk-cache-cancel.html b/LayoutTests/http/tests/cache/disk-cache/disk-cache-cancel.html
new file mode 100644 (file)
index 0000000..9bb2380
--- /dev/null
@@ -0,0 +1,45 @@
+<script src="/js-test-resources/js-test-pre.js"></script>
+<script src="resources/cache-test.js"></script>
+<body>
+<script>
+
+var tests =
+[
+ { responseHeaders: {'Cache-control': 'max-age=0' }, body: "unique", delay: 1 },
+ { responseHeaders: {'Cache-control': 'max-age=100' }, body: "unique", delay: 1 },
+ { responseHeaders: {'Cache-control': 'max-age=0', 'ETag': 'match' }, body: "unique", delay: 1 },
+];
+
+description("Test that canceled network loads don't leave behind stale cache entries");
+
+debug("running " + tests.length + " tests");
+debug("");
+
+function runTests(tests)
+{
+    debug("Warming cache");
+    internals.setOverrideCachePolicy("ReloadIgnoringCacheData");
+    loadResources(tests, function () {
+        debug("Starting loads, then canceling");
+        loadResources(tests, function (ev) {
+            debug("Loading again with back navigation cache policy");
+            internals.setOverrideCachePolicy("ReturnCacheDataElseLoad");
+            loadResources(tests, function (ev) {
+                printResults(tests);
+                finishJSTest();
+            });
+        });
+
+        setTimeout(function () {
+            for (var i = 0; i < tests.length; ++i) {
+                var test = tests[i];
+                test.xhr.abort();
+            }
+        }, 200);
+    });
+}
+
+runTests(tests);
+
+</script>
+<script src="/js-test-resources/js-test-post.js"></script>
index cbde17b..8d87fff 100644 (file)
@@ -40,14 +40,16 @@ function makeHeaderValue(value)
     return value;
 }
 
-function generateTestURL(test, includeBody, expiresInFutureIn304)
+function generateTestURL(test)
 {
-    includeBody = typeof includeBody !== 'undefined' ? includeBody : true;
-    expiresInFutureIn304 = typeof expiresInFutureIn304 !== 'undefined' ? expiresInFutureIn304 : false;
+    var body = typeof test.body !== 'undefined' ? test.body : "";
+    var expiresInFutureIn304 = typeof test.expiresInFutureIn304 !== 'undefined' ? test.expiresInFutureIn304 : false;
     var uniqueTestId = Math.floor((Math.random() * 1000000000000));
-    var testURL = "resources/generate-response.cgi?include-body=" + (includeBody ? "1" : "0");
+    var testURL = "resources/generate-response.cgi?body=" + body;
     if (expiresInFutureIn304)
         testURL += "&expires-in-future-in-304=1";
+    if (test.delay)
+        testURL += "&delay=" + test.delay;
     testURL += "&uniqueId=" + uniqueTestId++;
     if (!test.responseHeaders || !test.responseHeaders["Content-Type"])
         testURL += "&Content-Type=text/plain";
@@ -59,10 +61,12 @@ function generateTestURL(test, includeBody, expiresInFutureIn304)
 function loadResource(test, onload)
 {
     if (!test.url)
-        test.url = generateTestURL(test, test.includeBody, test.expiresInFutureIn304);
+        test.url = generateTestURL(test);
 
     test.xhr = new XMLHttpRequest();
     test.xhr.onload = onload;
+    test.xhr.onerror = onload;
+    test.xhr.onabort = onload;
     test.xhr.open("get", test.url, true);
 
     for (var header in test.requestHeaders)
@@ -82,7 +86,7 @@ function loadResourcesWithOptions(tests, options, completetion)
         loadResource(tests[i], function (ev) {
             --pendingCount;
             if (!pendingCount)
-                completetion();
+                completetion(ev);
          });
     }
 }
@@ -166,7 +170,8 @@ function generateTests(testMatrix, includeBody)
                 mergeFields(test[field], component[field]);
             }
         }
-        test.includeBody = includeBody;
+        if (includeBody && !test.body)
+            test.body = "test body";
         tests.push(test);
     }
     return tests;
index fc994b2..3d484b6 100755 (executable)
@@ -5,8 +5,13 @@ use HTTP::Date;
 
 my $query = new CGI;
 @names = $query->param;
-my $includeBody = $query->param('include-body') || 0;
 my $expiresInFutureIn304 = $query->param('expires-in-future-in-304') || 0;
+my $delay = $query->param('delay') || 0;
+my $body = $query->param('body') || 0;
+
+if ($body eq "unique") {
+    $body = sprintf "%08X\n", rand(0xffffffff)
+}
 
 my $hasStatusCode = 0;
 my $hasExpiresHeader = 0;
@@ -32,10 +37,18 @@ if ($query->http && $query->param("Range") =~ /bytes=(\d+)-(\d+)/) {
 
 foreach (@names) {
     next if ($_ eq "uniqueId");
-    next if ($_ eq "include-body");
+    next if ($_ eq "delay");
+    next if ($_ eq "body");
     next if ($_ eq "Status" and $hasStatusCode);
     next if ($_ eq "Expires" and $hasExpiresHeader);
     print $_ . ": " . $query->param($_) . "\n";
 }
 print "\n";
-print "test" if $includeBody;
+if ($delay) {
+    # Include some padding so headers and full body are sent separately.
+    for (my $i=0; $i < 1024; $i++) {
+        print "                                                                                ";
+    }
+    sleep $delay;
+}
+print $body if $body;
index 4ee5cf0..b89bc5e 100644 (file)
@@ -1,3 +1,23 @@
+2015-08-31  Antti Koivisto  <antti@apple.com>
+
+        Network Cache: Stale content after back navigation
+        https://bugs.webkit.org/show_bug.cgi?id=148634
+
+        Reviewed by Chris Dumez.
+
+        It is possible to get an older version of the previous page when navigating back. This can happen
+        if the main resource load has not completed before navigating away from the page.
+
+        Network cache entry is normally updated when the load completes. In case of cancellation we would leave
+        any existing entry as-is. However we render incrementally and user might have seen some content from
+        the partial load already. Navigating back to the cached page could show older version of the content.
+
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::abort):
+
+            If a network load is canceled by the client after receiving response but before the load has completed
+            remove any existing cache entry for it.
+
 2015-08-28  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Migrate GraphicsContexts from pointers to references
index f99d316..8a51102 100644 (file)
@@ -223,9 +223,18 @@ void NetworkResourceLoader::abort()
 {
     ASSERT(RunLoop::isMain());
 
-    if (m_handle && !m_didConvertHandleToDownload)
+    if (m_handle && !m_didConvertHandleToDownload) {
         m_handle->cancel();
 
+#if ENABLE(NETWORK_CACHE)
+        if (NetworkCache::singleton().isEnabled()) {
+            // We might already have used data from this incomplete load. Ensure older versions don't remain in the cache after cancel.
+            if (!m_response.isNull())
+                NetworkCache::singleton().remove(originalRequest());
+        }
+#endif
+    }
+
     cleanup();
 }