WKWebSiteDataStore.removeDataOfTypes should wait until disk cache files are actually...
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 7 Jul 2017 07:22:12 +0000 (07:22 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 7 Jul 2017 07:22:12 +0000 (07:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174224
<rdar://problem/33067545>

Reviewed by Sam Weinig.

Currently we dispatch file deletion operations to a background queue and call the completion
handler without waiting for the I/O to complete.

* NetworkProcess/NetworkProcess.cpp:
(WebKit::clearDiskCacheEntries):

    Call a new version of NetworkCache::remove() for bulk deletion.
    Note that it is fine to call this with an empty vector.

* NetworkProcess/cache/NetworkCache.cpp:
(WebKit::NetworkCache::Cache::remove):

    Bulk deletion with a completion handler.

(WebKit::NetworkCache::Cache::deleteFiles): Added.

    Factor to a helper function.

* NetworkProcess/cache/NetworkCache.h:
* NetworkProcess/cache/NetworkCacheStorage.cpp:
(WebKit::NetworkCache::Storage::remove):

    Remove files for all the provided keys in a queue and invoke the completion handler in the main thread when done.

* NetworkProcess/cache/NetworkCacheStorage.h:

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

Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/NetworkProcess.cpp
Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp
Source/WebKit2/NetworkProcess/cache/NetworkCache.h
Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp
Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.h

index 68fc147..f1b8bc5 100644 (file)
@@ -1,3 +1,37 @@
+2017-07-07  Antti Koivisto  <antti@apple.com>
+
+        WKWebSiteDataStore.removeDataOfTypes should wait until disk cache files are actually removed before invoking completion handler
+        https://bugs.webkit.org/show_bug.cgi?id=174224
+        <rdar://problem/33067545>
+
+        Reviewed by Sam Weinig.
+
+        Currently we dispatch file deletion operations to a background queue and call the completion
+        handler without waiting for the I/O to complete.
+
+        * NetworkProcess/NetworkProcess.cpp:
+        (WebKit::clearDiskCacheEntries):
+
+            Call a new version of NetworkCache::remove() for bulk deletion.
+            Note that it is fine to call this with an empty vector.
+
+        * NetworkProcess/cache/NetworkCache.cpp:
+        (WebKit::NetworkCache::Cache::remove):
+
+            Bulk deletion with a completion handler.
+
+        (WebKit::NetworkCache::Cache::deleteFiles): Added.
+
+            Factor to a helper function.
+
+        * NetworkProcess/cache/NetworkCache.h:
+        * NetworkProcess/cache/NetworkCacheStorage.cpp:
+        (WebKit::NetworkCache::Storage::remove):
+
+            Remove files for all the provided keys in a queue and invoke the completion handler in the main thread when done.
+
+        * NetworkProcess/cache/NetworkCacheStorage.h:
+
 2017-07-06  Chris Dumez  <cdumez@apple.com>
 
         Fix bad usage of static variables in ResourceLoadStatisticsStore
index a59e271..32d6b39 100644 (file)
@@ -449,10 +449,7 @@ static void clearDiskCacheEntries(const Vector<SecurityOriginData>& origins, Fun
                 return;
             }
 
-            for (auto& key : cacheKeysToDelete)
-                NetworkCache::singleton().remove(key);
-
-            RunLoop::main().dispatch(WTFMove(completionHandler));
+            NetworkCache::singleton().remove(cacheKeysToDelete, WTFMove(completionHandler));
             return;
         });
 
index 6fdb2cb..4fee10e 100644 (file)
@@ -505,6 +505,13 @@ void Cache::remove(const WebCore::ResourceRequest& request)
     remove(makeCacheKey(request));
 }
 
+void Cache::remove(const Vector<Key>& keys, Function<void ()>&& completionHandler)
+{
+    ASSERT(isEnabled());
+
+    m_storage->remove(keys, WTFMove(completionHandler));
+}
+
 void Cache::traverse(Function<void (const TraversalEntry*)>&& traverseHandler)
 {
     ASSERT(isEnabled());
index 1ab2e22..651406f 100644 (file)
@@ -121,6 +121,7 @@ public:
     void traverse(Function<void (const TraversalEntry*)>&&);
     void remove(const Key&);
     void remove(const WebCore::ResourceRequest&);
+    void remove(const Vector<Key>&, Function<void ()>&&);
 
     void clear();
     void clear(std::chrono::system_clock::time_point modifiedSince, Function<void ()>&& completionHandler);
index e1c37ee..f53aa6b 100644 (file)
@@ -555,11 +555,41 @@ void Storage::remove(const Key& key)
     removeFromPendingWriteOperations(key);
 
     serialBackgroundIOQueue().dispatch([this, key] {
-        WebCore::deleteFile(recordPathForKey(key));
-        m_blobStorage.remove(blobPathForKey(key));
+        deleteFiles(key);
     });
 }
 
+void Storage::remove(const Vector<Key>& keys, Function<void ()>&& completionHandler)
+{
+    ASSERT(RunLoop::isMain());
+
+    Vector<Key> keysToRemove;
+    keysToRemove.reserveInitialCapacity(keys.size());
+
+    for (auto& key : keys) {
+        if (!mayContain(key))
+            continue;
+        removeFromPendingWriteOperations(key);
+        keysToRemove.uncheckedAppend(key);
+    }
+
+    serialBackgroundIOQueue().dispatch([this, keysToRemove = WTFMove(keysToRemove), completionHandler = WTFMove(completionHandler)] () mutable {
+        for (auto& key : keysToRemove)
+            deleteFiles(key);
+
+        if (completionHandler)
+            RunLoop::main().dispatch(WTFMove(completionHandler));
+    });
+}
+
+void Storage::deleteFiles(const Key& key)
+{
+    ASSERT(!RunLoop::isMain());
+
+    WebCore::deleteFile(recordPathForKey(key));
+    m_blobStorage.remove(blobPathForKey(key));
+}
+
 void Storage::updateFileModificationTime(const String& path)
 {
     serialBackgroundIOQueue().dispatch([path = path.isolatedCopy()] {
index af655da..21f7629 100644 (file)
@@ -68,6 +68,7 @@ public:
     void store(const Record&, MappedBodyHandler&&);
 
     void remove(const Key&);
+    void remove(const Vector<Key>&, Function<void ()>&&);
     void clear(const String& type, std::chrono::system_clock::time_point modifiedSinceTime, Function<void ()>&& completionHandler);
 
     struct RecordInfo {
@@ -143,6 +144,7 @@ private:
     bool mayContainBlob(const Key&) const;
 
     void addToRecordFilter(const Key&);
+    void deleteFiles(const Key&);
 
     const String m_basePath;
     const String m_recordsPath;