Title: [228935] trunk
Revision
228935
Author
commit-qu...@webkit.org
Date
2018-02-22 14:50:19 -0800 (Thu, 22 Feb 2018)

Log Message

CacheStorage::Engine::Caches::writeRecord is not always calling the completion handler
https://bugs.webkit.org/show_bug.cgi?id=183055

Patch by Youenn Fablet <you...@apple.com> on 2018-02-22
Reviewed by Chris Dumez.

Source/WebKit:

Add a completion handler to Storage::store.
Use it instead in Caches::writeRecord.
This ensures that the Cache add/put promise will be called once all write operations have been done.

* NetworkProcess/cache/CacheStorageEngineCaches.cpp:
(WebKit::CacheStorage::Caches::writeRecord):
* NetworkProcess/cache/NetworkCacheStorage.cpp:
(WebKit::NetworkCache::Storage::WriteOperation::WriteOperation):
(WebKit::NetworkCache::Storage::finishWriteOperation):
(WebKit::NetworkCache::Storage::store):
* NetworkProcess/cache/NetworkCacheStorage.h:
(WebKit::NetworkCache::Storage::store):

LayoutTests:

* http/tests/cache-storage/resources/cache-persistency-iframe.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (228934 => 228935)


--- trunk/LayoutTests/ChangeLog	2018-02-22 22:36:47 UTC (rev 228934)
+++ trunk/LayoutTests/ChangeLog	2018-02-22 22:50:19 UTC (rev 228935)
@@ -1,3 +1,12 @@
+2018-02-22  Youenn Fablet  <you...@apple.com>
+
+        CacheStorage::Engine::Caches::writeRecord is not always calling the completion handler
+        https://bugs.webkit.org/show_bug.cgi?id=183055
+
+        Reviewed by Chris Dumez.
+
+        * http/tests/cache-storage/resources/cache-persistency-iframe.html:
+
 2018-02-22  Chris Dumez  <cdu...@apple.com>
 
         Document.open() cancels existing provisional load but not navigation policy check

Modified: trunk/LayoutTests/http/tests/cache-storage/resources/cache-persistency-iframe.html (228934 => 228935)


--- trunk/LayoutTests/http/tests/cache-storage/resources/cache-persistency-iframe.html	2018-02-22 22:36:47 UTC (rev 228934)
+++ trunk/LayoutTests/http/tests/cache-storage/resources/cache-persistency-iframe.html	2018-02-22 22:50:19 UTC (rev 228935)
@@ -3,22 +3,18 @@
 <body>
     <script>
 var cache;
-function doTest()
+async function doTest()
 {
     if (window.location.hash === "#check") {
-        self.caches.keys().then(keys => {
-            window.parent.postMessage(keys.length === 1 && keys[0] === "testCacheName", "*");
-        });
+        let keys = await self.caches.keys();
+        window.parent.postMessage(keys.length === 1 && keys[0] === "testCacheName", "*");
         return;
     }
 
     if (window.location.hash === "#remove") {
-        self.caches.open("testCacheName").then(c => {
-            cache = c
-            self.caches.delete("testCacheName").then(() => {
-                window.parent.postMessage("removed", "*");
-            });
-        });
+        let cache = await self.caches.open("testCacheName");
+        await self.caches.delete("testCacheName");
+        window.parent.postMessage("removed", "*");
         return;
     }
 
@@ -25,9 +21,9 @@
     var cacheName = "testCacheName";
     if (window.location.hash.indexOf("#name=") === 0)
         cacheName = window.location.hash.substring(6);
-    self.caches.open(cacheName).then(() => {
-        window.parent.postMessage("ready", "*");
-    });
+    let cache = await self.caches.open(cacheName);
+    await cache.put(new Request('testurl'), new Response('test body'));
+    window.parent.postMessage("ready", "*");
 }
 doTest();
     </script>

Modified: trunk/Source/WebKit/ChangeLog (228934 => 228935)


--- trunk/Source/WebKit/ChangeLog	2018-02-22 22:36:47 UTC (rev 228934)
+++ trunk/Source/WebKit/ChangeLog	2018-02-22 22:50:19 UTC (rev 228935)
@@ -1,3 +1,23 @@
+2018-02-22  Youenn Fablet  <you...@apple.com>
+
+        CacheStorage::Engine::Caches::writeRecord is not always calling the completion handler
+        https://bugs.webkit.org/show_bug.cgi?id=183055
+
+        Reviewed by Chris Dumez.
+
+        Add a completion handler to Storage::store.
+        Use it instead in Caches::writeRecord.
+        This ensures that the Cache add/put promise will be called once all write operations have been done.
+
+        * NetworkProcess/cache/CacheStorageEngineCaches.cpp:
+        (WebKit::CacheStorage::Caches::writeRecord):
+        * NetworkProcess/cache/NetworkCacheStorage.cpp:
+        (WebKit::NetworkCache::Storage::WriteOperation::WriteOperation):
+        (WebKit::NetworkCache::Storage::finishWriteOperation):
+        (WebKit::NetworkCache::Storage::store):
+        * NetworkProcess/cache/NetworkCacheStorage.h:
+        (WebKit::NetworkCache::Storage::store):
+
 2018-02-22  Ryosuke Niwa  <rn...@webkit.org>
 
         Add an entitlement check for service worker on iOS

Modified: trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp (228934 => 228935)


--- trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp	2018-02-22 22:36:47 UTC (rev 228934)
+++ trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp	2018-02-22 22:50:19 UTC (rev 228935)
@@ -470,10 +470,11 @@
 
     if (!shouldPersist()) {
         m_volatileStorage.set(recordInformation.key, WTFMove(record));
+        callback(std::nullopt);
         return;
     }
 
-    m_storage->store(Cache::encode(recordInformation, record), [protectedStorage = makeRef(*m_storage), callback = WTFMove(callback)](const Data&) {
+    m_storage->store(Cache::encode(recordInformation, record), { }, [protectedStorage = makeRef(*m_storage), callback = WTFMove(callback)]() {
         callback(std::nullopt);
     });
 }

Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp (228934 => 228935)


--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp	2018-02-22 22:36:47 UTC (rev 228934)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp	2018-02-22 22:50:19 UTC (rev 228935)
@@ -100,15 +100,17 @@
 struct Storage::WriteOperation {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    WriteOperation(Storage& storage, const Record& record, MappedBodyHandler&& mappedBodyHandler)
+    WriteOperation(Storage& storage, const Record& record, MappedBodyHandler&& mappedBodyHandler, CompletionHandler<void()>&& completionHandler)
         : storage(storage)
         , record(record)
         , mappedBodyHandler(WTFMove(mappedBodyHandler))
+        , completionHandler(WTFMove(completionHandler))
     { }
     Ref<Storage> storage;
 
     const Record record;
     const MappedBodyHandler mappedBodyHandler;
+    CompletionHandler<void()> completionHandler;
 
     std::atomic<unsigned> activeCount { 0 };
 };
@@ -800,6 +802,9 @@
 
     auto protectedThis = makeRef(*this);
 
+    if (writeOperation.completionHandler)
+        writeOperation.completionHandler();
+
     m_activeWriteOperations.remove(&writeOperation);
     dispatchPendingWriteOperations();
 
@@ -832,7 +837,7 @@
     dispatchPendingReadOperations();
 }
 
-void Storage::store(const Record& record, MappedBodyHandler&& mappedBodyHandler)
+void Storage::store(const Record& record, MappedBodyHandler&& mappedBodyHandler, CompletionHandler<void()>&& completionHandler)
 {
     ASSERT(RunLoop::isMain());
     ASSERT(!record.key.isNull());
@@ -840,7 +845,7 @@
     if (!m_capacity)
         return;
 
-    auto writeOperation = std::make_unique<WriteOperation>(*this, record, WTFMove(mappedBodyHandler));
+    auto writeOperation = std::make_unique<WriteOperation>(*this, record, WTFMove(mappedBodyHandler), WTFMove(completionHandler));
     m_pendingWriteOperations.prepend(WTFMove(writeOperation));
 
     // Add key to the filter already here as we do lookups from the pending operations too.

Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h (228934 => 228935)


--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h	2018-02-22 22:36:47 UTC (rev 228934)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h	2018-02-22 22:50:19 UTC (rev 228935)
@@ -30,6 +30,7 @@
 #include "NetworkCacheKey.h"
 #include <WebCore/Timer.h>
 #include <wtf/BloomFilter.h>
+#include <wtf/CompletionHandler.h>
 #include <wtf/Deque.h>
 #include <wtf/Function.h>
 #include <wtf/HashSet.h>
@@ -62,7 +63,7 @@
     void retrieve(const Key&, unsigned priority, RetrieveCompletionHandler&&);
 
     typedef Function<void (const Data& mappedBody)> MappedBodyHandler;
-    void store(const Record&, MappedBodyHandler&&);
+    void store(const Record&, MappedBodyHandler&&, CompletionHandler<void()>&& = { });
 
     void remove(const Key&);
     void remove(const Vector<Key>&, Function<void ()>&&);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to