[Cache API] Add support for overwriting responses with put on an existing record

https://bugs.webkit.org/show_bug.cgi?id=175825

Patch by Youenn Fablet <youenn@apple.com> on 2017-08-22
Reviewed by Geoffrey Garen.

LayoutTests/imported/w3c:

* web-platform-tests/service-workers/cache-storage/window/cache-put.https-expected.txt:
* web-platform-tests/service-workers/cache-storage/worker/cache-put.https-expected.txt:

Source/WebCore:

Tests: http/wpt/cache-storage/cache-put-keys.https.any.html
       http/wpt/cache-storage/cache-put-keys.https.any.worker.html

Adding support for the new response update counter.
Overwriting local cached response with new retrieved response when the counter is different.
Adding support for passing this value from/to workers.

* Modules/cache/Cache.cpp:
(WebCore::Cache::queryCacheWithTargetStorage):
(WebCore::toConnectionRecord):
(WebCore::Cache::updateRecords):
* Modules/cache/CacheStorageConnection.cpp:
(WebCore::CacheStorageConnection::Record::copy const):
* Modules/cache/CacheStorageConnection.h:
* Modules/cache/CacheStorageRecord.h:
* Modules/cache/WorkerCacheStorageConnection.cpp:
(WebCore::toCrossThreadRecordData):
(WebCore::fromCrossThreadRecordData):

Source/WebKit:

Add support for encoding/decoding the update counter.
Incrementing it when overwriting an existing response.
Storing the new body in addition to the new response.

* NetworkProcess/cache/CacheStorageEngine.cpp:
(WebKit::CacheStorageEngine::putRecords):
* Shared/WebCoreArgumentCoders.cpp:
(IPC::ArgumentCoder<CacheStorageConnection::Record>::encode):
(IPC::ArgumentCoder<CacheStorageConnection::Record>::decode):

LayoutTests:

Adding update counter for response
Skipping new test on WK1.

* platform/ios-wk1/TestExpectations:
* platform/mac-wk1/TestExpectations:
* http/wpt/cache-storage/cache-put-keys.https.any-expected.txt: Added.
* http/wpt/cache-storage/cache-put-keys.https.any.html: Added.
* http/wpt/cache-storage/cache-put-keys.https.any.js: Added.
(cache_test):
* http/wpt/cache-storage/cache-put-keys.https.any.worker-expected.txt: Added.
* http/wpt/cache-storage/cache-put-keys.https.any.worker.html: Added.

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 6aba61b..58f6fc8 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,22 @@
+2017-08-22  Youenn Fablet  <youenn@apple.com>
+
+        [Cache API] Add support for overwriting responses with put on an existing record
+        https://bugs.webkit.org/show_bug.cgi?id=175825
+
+        Reviewed by Geoffrey Garen.
+
+        Adding update counter for response
+        Skipping new test on WK1.
+
+        * platform/ios-wk1/TestExpectations:
+        * platform/mac-wk1/TestExpectations:
+        * http/wpt/cache-storage/cache-put-keys.https.any-expected.txt: Added.
+        * http/wpt/cache-storage/cache-put-keys.https.any.html: Added.
+        * http/wpt/cache-storage/cache-put-keys.https.any.js: Added.
+        (cache_test):
+        * http/wpt/cache-storage/cache-put-keys.https.any.worker-expected.txt: Added.
+        * http/wpt/cache-storage/cache-put-keys.https.any.worker.html: Added.
+
 2017-08-22  Matt Lewis  <jlewis3@apple.com>
 
         Marked imported/w3c/web-platform-tests/fetch/http-cache/invalidate.html as flaky on macOS WK2.

diff --git a/LayoutTests/http/wpt/cache-storage/cache-put-keys.https.any-expected.txt b/LayoutTests/http/wpt/cache-storage/cache-put-keys.https.any-expected.txt
new file mode 100644 (file)
index 0000000..07b5d68
--- /dev/null
+++ b/LayoutTests/http/wpt/cache-storage/cache-put-keys.https.any-expected.txt
@@ -0,0 +1,3 @@
+
+PASS Cache.put called twice with matching Requests - keys should remain the same 
+

diff --git a/LayoutTests/http/wpt/cache-storage/cache-put-keys.https.any.html b/LayoutTests/http/wpt/cache-storage/cache-put-keys.https.any.html
new file mode 100644 (file)
index 0000000..2382913
--- /dev/null
+++ b/LayoutTests/http/wpt/cache-storage/cache-put-keys.https.any.html
@@ -0,0 +1 @@
+<!-- This file is required for WebKit test infrastructure to run the templated test -->
\ No newline at end of file

diff --git a/LayoutTests/http/wpt/cache-storage/cache-put-keys.https.any.js b/LayoutTests/http/wpt/cache-storage/cache-put-keys.https.any.js
new file mode 100644 (file)
index 0000000..bf34e19
--- /dev/null
+++ b/LayoutTests/http/wpt/cache-storage/cache-put-keys.https.any.js
@@ -0,0 +1,30 @@
+// META: script=/service-workers/cache-storage/resources/test-helpers.js
+
+var test_url = 'https://example.com/foo';
+var test_body = 'Hello world!';
+
+cache_test(function(cache) {
+    var cache_keys;
+    var alternate_response_body = 'New body';
+    var alternate_response = new Response(alternate_response_body, { statusText: 'New status' });
+    return cache.put(new Request(test_url), new Response('Old body', { statusText: 'Old status' })).then(function() {
+        return cache.keys();
+    }).then(function(keys) {
+        cache_keys = keys;
+    }).then(function() {
+        return cache.put(new Request(test_url), alternate_response);
+    }).then(function() {
+        return cache.keys();
+    }).then(function(keys) {
+        assert_request_array_equals(keys, cache_keys);
+    }).then(function() {
+        return cache.match(test_url);
+    }).then(function(result) {
+        assert_response_equals(result, alternate_response, 'Cache.put should replace existing ' + 'response with new response.');
+        return result.text();
+    }).then(function(body) {
+        assert_equals(body, alternate_response_body, 'Cache put should store new response body.');
+    });
+}, 'Cache.put called twice with matching Requests - keys should remain the same');
+
+done();

diff --git a/LayoutTests/http/wpt/cache-storage/cache-put-keys.https.any.worker-expected.txt b/LayoutTests/http/wpt/cache-storage/cache-put-keys.https.any.worker-expected.txt
new file mode 100644 (file)
index 0000000..07b5d68
--- /dev/null
+++ b/LayoutTests/http/wpt/cache-storage/cache-put-keys.https.any.worker-expected.txt
@@ -0,0 +1,3 @@
+
+PASS Cache.put called twice with matching Requests - keys should remain the same 
+

diff --git a/LayoutTests/http/wpt/cache-storage/cache-put-keys.https.any.worker.html b/LayoutTests/http/wpt/cache-storage/cache-put-keys.https.any.worker.html
new file mode 100644 (file)
index 0000000..2382913
--- /dev/null
+++ b/LayoutTests/http/wpt/cache-storage/cache-put-keys.https.any.worker.html
@@ -0,0 +1 @@
+<!-- This file is required for WebKit test infrastructure to run the templated test -->
\ No newline at end of file

diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
index e19c817..a8531e7 100644 (file)
--- a/LayoutTests/imported/w3c/ChangeLog
+++ b/LayoutTests/imported/w3c/ChangeLog
@@ -1,3 +1,13 @@
+2017-08-22  Youenn Fablet  <youenn@apple.com>
+
+        [Cache API] Add support for overwriting responses with put on an existing record
+        https://bugs.webkit.org/show_bug.cgi?id=175825
+
+        Reviewed by Geoffrey Garen.
+
+        * web-platform-tests/service-workers/cache-storage/window/cache-put.https-expected.txt:
+        * web-platform-tests/service-workers/cache-storage/worker/cache-put.https-expected.txt:
+
 2017-08-22  Andy Estes  <aestes@apple.com>
 
         [Payment Request] Implement error checking for show(), abort(), and canMakePayment()

diff --git a/LayoutTests/imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-put.https-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-put.https-expected.txt
index 09cd12e..6105d67 100644 (file)
--- a/LayoutTests/imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-put.https-expected.txt
+++ b/LayoutTests/imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-put.https-expected.txt
@@ -8,8 +8,8 @@ PASS Cache.put with an empty response body
 PASS Cache.put with synthetic 206 response 
 PASS Cache.put with HTTP 206 response 
 PASS Cache.put with HTTP 500 response 
-FAIL Cache.put called twice with matching Requests and different Responses assert_equals: Cache put should store new response body. expected "New body" but got "Old body"
-FAIL Cache.put called twice with request URLs that differ only by a fragment assert_equals: Cache put should store new response body. expected "New body" but got "Old body"
+PASS Cache.put called twice with matching Requests and different Responses 
+PASS Cache.put called twice with request URLs that differ only by a fragment 
 PASS Cache.put with a string request 
 PASS Cache.put with an invalid response 
 PASS Cache.put with a non-HTTP/HTTPS request 

diff --git a/LayoutTests/imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-put.https-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-put.https-expected.txt
index 9449262..1c82dd9 100644 (file)
--- a/LayoutTests/imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-put.https-expected.txt
+++ b/LayoutTests/imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-put.https-expected.txt
@@ -8,8 +8,8 @@ PASS Cache.put with an empty response body
 PASS Cache.put with synthetic 206 response 
 PASS Cache.put with HTTP 206 response 
 PASS Cache.put with HTTP 500 response 
-FAIL Cache.put called twice with matching Requests and different Responses assert_equals: Cache put should store new response body. expected "New body" but got "Old body"
-FAIL Cache.put called twice with request URLs that differ only by a fragment assert_equals: Cache put should store new response body. expected "New body" but got "Old body"
+PASS Cache.put called twice with matching Requests and different Responses 
+PASS Cache.put called twice with request URLs that differ only by a fragment 
 PASS Cache.put with a string request 
 PASS Cache.put with an invalid response 
 PASS Cache.put with a non-HTTP/HTTPS request 

diff --git a/LayoutTests/platform/ios-wk1/TestExpectations b/LayoutTests/platform/ios-wk1/TestExpectations
index 9f7e6e2..4181049 100644 (file)
--- a/LayoutTests/platform/ios-wk1/TestExpectations
+++ b/LayoutTests/platform/ios-wk1/TestExpectations
@@ -9,6 +9,7 @@ editing/input/focus-change-with-marked-text.html [ Pass ]
 
 # No service worker implementation for WK1
 imported/w3c/web-platform-tests/service-workers [ Skip ]
+http/wpt/cache-storage [ Skip ]
 
 # Skip WebRTC for now in WK1
 imported/w3c/web-platform-tests/webrtc [ Skip ]

diff --git a/LayoutTests/platform/mac-wk1/TestExpectations b/LayoutTests/platform/mac-wk1/TestExpectations
index ecf659f..8f9fb4c 100644 (file)
--- a/LayoutTests/platform/mac-wk1/TestExpectations
+++ b/LayoutTests/platform/mac-wk1/TestExpectations
@@ -96,6 +96,7 @@ http/tests/ssl/media-stream
 
 # No service worker implementation for WK1
 imported/w3c/web-platform-tests/service-workers [ Skip ]
+http/wpt/cache-storage [ Skip ]
 
 # Skip WebRTC for now in WK1
 imported/w3c/web-platform-tests/webrtc [ Skip ]

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 5de1c35..0fa5318 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,29 @@
+2017-08-22  Youenn Fablet  <youenn@apple.com>
+
+        [Cache API] Add support for overwriting responses with put on an existing record
+        https://bugs.webkit.org/show_bug.cgi?id=175825
+
+        Reviewed by Geoffrey Garen.
+
+        Tests: http/wpt/cache-storage/cache-put-keys.https.any.html
+               http/wpt/cache-storage/cache-put-keys.https.any.worker.html
+
+        Adding support for the new response update counter.
+        Overwriting local cached response with new retrieved response when the counter is different.
+        Adding support for passing this value from/to workers.
+
+        * Modules/cache/Cache.cpp:
+        (WebCore::Cache::queryCacheWithTargetStorage):
+        (WebCore::toConnectionRecord):
+        (WebCore::Cache::updateRecords):
+        * Modules/cache/CacheStorageConnection.cpp:
+        (WebCore::CacheStorageConnection::Record::copy const):
+        * Modules/cache/CacheStorageConnection.h:
+        * Modules/cache/CacheStorageRecord.h:
+        * Modules/cache/WorkerCacheStorageConnection.cpp:
+        (WebCore::toCrossThreadRecordData):
+        (WebCore::fromCrossThreadRecordData):
+
 2017-08-22  Alex Christensen  <achristensen@webkit.org>
 
         Remove ChromeClient::hasOpenedPopup

diff --git a/Source/WebCore/Modules/cache/Cache.cpp b/Source/WebCore/Modules/cache/Cache.cpp
index 4a1334b..3781d43 100644 (file)
--- a/Source/WebCore/Modules/cache/Cache.cpp
+++ b/Source/WebCore/Modules/cache/Cache.cpp
@@ -411,7 +411,7 @@ Vector<CacheStorageRecord> Cache::queryCacheWithTargetStorage(const FetchRequest
     Vector<CacheStorageRecord> records;
     for (auto& record : targetStorage) {
         if (queryCacheMatch(request, record.request.get(), record.response->resourceResponse(), options))
-            records.append({ record.identifier, record.request.copyRef(), record.response.copyRef() });
+            records.append({ record.identifier, record.updateResponseCounter, record.request.copyRef(), record.response.copyRef() });
     }
     return records;
 }
@@ -440,7 +440,7 @@ CacheStorageConnection::Record toConnectionRecord(const FetchRequest& request, F
     ASSERT(!cachedRequest.isNull());
     ASSERT(!cachedResponse.isNull());
 
-    return { 0,
+    return { 0, 0,
         request.headers().guard(), WTFMove(cachedRequest), request.fetchOptions(), request.internalRequestReferrer(),
         response.headers().guard(), WTFMove(cachedResponse), WTFMove(responseBody)
     };
@@ -472,9 +472,18 @@ void Cache::updateRecords(Vector<CacheStorageConnection::Record>&& records)
 
     for (auto& record : records) {
         size_t index = m_records.findMatching([&](const auto& item) { return item.identifier == record.identifier; });
-        if (index != notFound)
-            newRecords.append(WTFMove(m_records[index]));
-        else {
+        if (index != notFound) {
+            auto& current = m_records[index];
+            if (current.updateResponseCounter != record.updateResponseCounter) {
+                auto responseHeaders = FetchHeaders::create(record.responseHeadersGuard, HTTPHeaderMap { record.response.httpHeaderFields() });
+                auto response = FetchResponse::create(*scriptExecutionContext(), std::nullopt, WTFMove(responseHeaders), WTFMove(record.response));
+                response->setBodyData(WTFMove(record.responseBody));
+
+                current.response = WTFMove(response);
+                current.updateResponseCounter = record.updateResponseCounter;
+            }
+            newRecords.append(WTFMove(current));
+        } else {
             auto requestHeaders = FetchHeaders::create(record.requestHeadersGuard, HTTPHeaderMap { record.request.httpHeaderFields() });
             FetchRequest::InternalRequest internalRequest = { WTFMove(record.request), WTFMove(record.options), WTFMove(record.referrer) };
             auto request = FetchRequest::create(*scriptExecutionContext(), std::nullopt, WTFMove(requestHeaders), WTFMove(internalRequest));
@@ -483,7 +492,7 @@ void Cache::updateRecords(Vector<CacheStorageConnection::Record>&& records)
             auto response = FetchResponse::create(*scriptExecutionContext(), std::nullopt, WTFMove(responseHeaders), WTFMove(record.response));
             response->setBodyData(WTFMove(record.responseBody));
 
-            newRecords.append(CacheStorageRecord { record.identifier, WTFMove(request), WTFMove(response) });
+            newRecords.append(CacheStorageRecord { record.identifier, record.updateResponseCounter, WTFMove(request), WTFMove(response) });
         }
     }
     m_records = WTFMove(newRecords);

diff --git a/Source/WebCore/Modules/cache/CacheStorageConnection.cpp b/Source/WebCore/Modules/cache/CacheStorageConnection.cpp
index 67f2f6d..3157fc3 100644 (file)
--- a/Source/WebCore/Modules/cache/CacheStorageConnection.cpp
+++ b/Source/WebCore/Modules/cache/CacheStorageConnection.cpp
@@ -180,7 +180,7 @@ static inline CacheStorageConnection::ResponseBody copyResponseBody(const CacheS
 
 CacheStorageConnection::Record CacheStorageConnection::Record::copy() const
 {
-    return Record { identifier, requestHeadersGuard, request, options, referrer, responseHeadersGuard, response, copyResponseBody(responseBody) };
+    return Record { identifier, updateResponseCounter, requestHeadersGuard, request, options, referrer, responseHeadersGuard, response, copyResponseBody(responseBody) };
 }
 
 } // namespace WebCore

diff --git a/Source/WebCore/Modules/cache/CacheStorageConnection.h b/Source/WebCore/Modules/cache/CacheStorageConnection.h
index 24fa18b..723616c 100644 (file)
--- a/Source/WebCore/Modules/cache/CacheStorageConnection.h
+++ b/Source/WebCore/Modules/cache/CacheStorageConnection.h
@@ -67,6 +67,7 @@ public:
         WEBCORE_EXPORT Record copy() const;
 
         uint64_t identifier;
+        uint64_t updateResponseCounter;
 
         FetchHeaders::Guard requestHeadersGuard;
         ResourceRequest request;

diff --git a/Source/WebCore/Modules/cache/CacheStorageRecord.h b/Source/WebCore/Modules/cache/CacheStorageRecord.h
index 00a6ace..5c3f981 100644 (file)
--- a/Source/WebCore/Modules/cache/CacheStorageRecord.h
+++ b/Source/WebCore/Modules/cache/CacheStorageRecord.h
@@ -32,6 +32,7 @@ namespace WebCore {
 
 struct CacheStorageRecord {
     uint64_t identifier;
+    uint64_t updateResponseCounter;
 
     Ref<FetchRequest> request;
     Ref<FetchResponse> response;

diff --git a/Source/WebCore/Modules/cache/WorkerCacheStorageConnection.cpp b/Source/WebCore/Modules/cache/WorkerCacheStorageConnection.cpp
index e5593a7..10aa654 100644 (file)
--- a/Source/WebCore/Modules/cache/WorkerCacheStorageConnection.cpp
+++ b/Source/WebCore/Modules/cache/WorkerCacheStorageConnection.cpp
@@ -40,6 +40,7 @@ namespace WebCore {
 
 struct CrossThreadRecordData {
     uint64_t identifier;
+    uint64_t updateResponseCounter;
 
     FetchHeaders::Guard requestHeadersGuard;
     ResourceRequest request;
@@ -56,6 +57,7 @@ static CrossThreadRecordData toCrossThreadRecordData(const CacheStorageConnectio
 {
     return CrossThreadRecordData {
         record.identifier,
+        record.updateResponseCounter,
         record.requestHeadersGuard,
         record.request.isolatedCopy(),
         record.options.isolatedCopy(),
@@ -70,6 +72,7 @@ static CacheStorageConnection::Record fromCrossThreadRecordData(CrossThreadRecor
 {
     return CacheStorageConnection::Record {
         data.identifier,
+        data.updateResponseCounter,
         data.requestHeadersGuard,
         WTFMove(data.request),
         WTFMove(data.options),

diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index 9e22cb2..62403a7 100644 (file)
--- a/Source/WebKit/ChangeLog
+++ b/Source/WebKit/ChangeLog
@@ -1,3 +1,20 @@
+2017-08-22  Youenn Fablet  <youenn@apple.com>
+
+        [Cache API] Add support for overwriting responses with put on an existing record
+        https://bugs.webkit.org/show_bug.cgi?id=175825
+
+        Reviewed by Geoffrey Garen.
+
+        Add support for encoding/decoding the update counter.
+        Incrementing it when overwriting an existing response.
+        Storing the new body in addition to the new response.
+
+        * NetworkProcess/cache/CacheStorageEngine.cpp:
+        (WebKit::CacheStorageEngine::putRecords):
+        * Shared/WebCoreArgumentCoders.cpp:
+        (IPC::ArgumentCoder<CacheStorageConnection::Record>::encode):
+        (IPC::ArgumentCoder<CacheStorageConnection::Record>::decode):
+
 2017-08-22  Alex Christensen  <achristensen@webkit.org>
 
         Remove ChromeClient::hasOpenedPopup

diff --git a/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp b/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp
index 6e8b3bc..5d5787b 100644 (file)
--- a/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp
+++ b/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp
@@ -174,6 +174,8 @@ void CacheStorageEngine::putRecords(uint64_t cacheIdentifier, Vector<Record>&& r
                     recordIdentifiers.uncheckedAppend(identifier);
                     existingRecord.responseHeadersGuard = record.responseHeadersGuard;
                     existingRecord.response = WTFMove(record.response);
+                    existingRecord.responseBody = WTFMove(record.responseBody);
+                    ++existingRecord.updateResponseCounter;
                 }
             }
         }

diff --git a/Source/WebKit/Shared/WebCoreArgumentCoders.cpp b/Source/WebKit/Shared/WebCoreArgumentCoders.cpp
index 1ff9a12..3715be4 100644 (file)
--- a/Source/WebKit/Shared/WebCoreArgumentCoders.cpp
+++ b/Source/WebKit/Shared/WebCoreArgumentCoders.cpp
@@ -311,6 +311,7 @@ void ArgumentCoder<CacheStorageConnection::Record>::encode(Encoder& encoder, con
 
     encoder << record.responseHeadersGuard;
     encoder << record.response;
+    encoder << record.updateResponseCounter;
 
     WTF::switchOn(record.responseBody, [&](const Ref<SharedBuffer>& buffer) {
         encoder << true;
@@ -355,6 +356,10 @@ bool ArgumentCoder<CacheStorageConnection::Record>::decode(Decoder& decoder, Cac
     if (!decoder.decode(response))
         return false;
 
+    uint64_t updateResponseCounter;
+    if (!decoder.decode(updateResponseCounter))
+        return false;
+
     WebCore::CacheStorageConnection::ResponseBody responseBody;
     bool hasSharedBufferBody;
     if (!decoder.decode(hasSharedBufferBody))
@@ -386,6 +391,7 @@ bool ArgumentCoder<CacheStorageConnection::Record>::decode(Decoder& decoder, Cac
 
     record.responseHeadersGuard = responseHeadersGuard;
     record.response = WTFMove(response);
+    record.updateResponseCounter = updateResponseCounter;
     record.responseBody = WTFMove(responseBody);
 
     return true;