[webkit-changes] [236936] trunk/Source/WebKit

Mon, 08 Oct 2018 13:23:16 -0700

Title: [236936] trunk/Source/WebKit
Revision
236936
Author
[email protected]
Date
2018-10-08 13:22:00 -0700 (Mon, 08 Oct 2018)

Log Message

NetworkCache::Storage should be WTF::DestructionThread::Main
https://bugs.webkit.org/show_bug.cgi?id=190324

Reviewed by Alex Christensen.

Use WTF::DestructionThread::Main to make sure that Storage is destroyed in the main thread.
Remove the code that was making sure that any ref was destroyed in the main thread.

* NetworkProcess/cache/CacheStorageEngineCaches.cpp: Make sure the completion handler is not null.
* NetworkProcess/cache/NetworkCacheStorage.cpp:
(WebKit::NetworkCache::Storage::TraverseOperation::TraverseOperation): Make it clear that a ref is taken.
(WebKit::NetworkCache::Storage::Storage):
(WebKit::NetworkCache::Storage::synchronize): Protect 'this' when dispatching back to the main thread.
Move the code that was after dispatch before dispatch to allow moving protectedThis in the dispatch lambda.
(WebKit::NetworkCache::Storage::remove):
(WebKit::NetworkCache::Storage::traverse):
(WebKit::NetworkCache::Storage::clear):
(WebKit::NetworkCache::Storage::deleteOldVersions):
* NetworkProcess/cache/NetworkCacheStorage.h:

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (236935 => 236936)


--- trunk/Source/WebKit/ChangeLog	2018-10-08 20:18:26 UTC (rev 236935)
+++ trunk/Source/WebKit/ChangeLog	2018-10-08 20:22:00 UTC (rev 236936)
@@ -1,3 +1,25 @@
+2018-10-08  Youenn Fablet  <[email protected]>
+
+        NetworkCache::Storage should be WTF::DestructionThread::Main
+        https://bugs.webkit.org/show_bug.cgi?id=190324
+
+        Reviewed by Alex Christensen.
+
+        Use WTF::DestructionThread::Main to make sure that Storage is destroyed in the main thread.
+        Remove the code that was making sure that any ref was destroyed in the main thread.
+
+        * NetworkProcess/cache/CacheStorageEngineCaches.cpp: Make sure the completion handler is not null.
+        * NetworkProcess/cache/NetworkCacheStorage.cpp:
+        (WebKit::NetworkCache::Storage::TraverseOperation::TraverseOperation): Make it clear that a ref is taken.
+        (WebKit::NetworkCache::Storage::Storage):
+        (WebKit::NetworkCache::Storage::synchronize): Protect 'this' when dispatching back to the main thread.
+        Move the code that was after dispatch before dispatch to allow moving protectedThis in the dispatch lambda.
+        (WebKit::NetworkCache::Storage::remove):
+        (WebKit::NetworkCache::Storage::traverse):
+        (WebKit::NetworkCache::Storage::clear):
+        (WebKit::NetworkCache::Storage::deleteOldVersions):
+        * NetworkProcess/cache/NetworkCacheStorage.h:
+
 2018-10-08  Tim Horton  <[email protected]>
 
         Adjust keyboard scrolling animator to springy and semiphysical

Modified: trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp (236935 => 236936)


--- trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp	2018-10-08 20:18:26 UTC (rev 236935)
+++ trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp	2018-10-08 20:22:00 UTC (rev 236936)
@@ -348,7 +348,7 @@
     auto position = m_removedCaches.findMatching([&](const auto& item) { return item.identifier() == cache.identifier(); });
     if (position != notFound) {
         if (m_storage)
-            m_storage->remove(cache.keys(), { });
+            m_storage->remove(cache.keys(), [] { });
 
         m_removedCaches.remove(position);
         return;

Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp (236935 => 236936)


--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp	2018-10-08 20:18:26 UTC (rev 236935)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp	2018-10-08 20:22:00 UTC (rev 236936)
@@ -124,8 +124,8 @@
 struct Storage::TraverseOperation {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    TraverseOperation(Storage& storage, const String& type, OptionSet<TraverseFlag> flags, TraverseHandler&& handler)
-        : storage(storage)
+    TraverseOperation(Ref<Storage>&& storage, const String& type, OptionSet<TraverseFlag> flags, TraverseHandler&& handler)
+        : storage(WTFMove(storage))
         , type(type)
         , flags(flags)
         , handler(WTFMove(handler))
@@ -233,6 +233,8 @@
     , m_serialBackgroundIOQueue(WorkQueue::create("com.apple.WebKit.Cache.Storage.serialBackground", WorkQueue::Type::Serial, WorkQueue::QOS::Background))
     , m_blobStorage(makeBlobDirectoryPath(baseDirectoryPath), m_salt)
 {
+    ASSERT(RunLoop::isMain());
+
     deleteOldVersions();
     synchronize();
 }
@@ -325,7 +327,13 @@
         if (!shouldComputeExactRecordsSize)
             recordsSize = estimateRecordsSize(recordCount, blobCount);
 
-        RunLoop::main().dispatch([this, recordFilter = WTFMove(recordFilter), blobFilter = WTFMove(blobFilter), recordsSize]() mutable {
+        m_blobStorage.synchronize();
+
+        deleteEmptyRecordsDirectories(recordsPath());
+
+        LOG(NetworkCacheStorage, "(NetworkProcess) cache synchronization completed size=%zu recordCount=%u", recordsSize, recordCount);
+
+        RunLoop::main().dispatch([this, protectedThis = WTFMove(protectedThis), recordFilter = WTFMove(recordFilter), blobFilter = WTFMove(blobFilter), recordsSize]() mutable {
             for (auto& recordFilterKey : m_recordFilterHashesAddedDuringSynchronization)
                 recordFilter->add(recordFilterKey);
             m_recordFilterHashesAddedDuringSynchronization.clear();
@@ -340,13 +348,6 @@
             m_synchronizationInProgress = false;
         });
 
-        m_blobStorage.synchronize();
-
-        deleteEmptyRecordsDirectories(recordsPath());
-
-        LOG(NetworkCacheStorage, "(NetworkProcess) cache synchronization completed size=%zu recordCount=%u", recordsSize, recordCount);
-
-        RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis)] { });
     });
 }
 
@@ -598,11 +599,10 @@
 
     serialBackgroundIOQueue().dispatch([this, protectedThis = WTFMove(protectedThis), key] () mutable {
         deleteFiles(key);
-        RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis)] { });
     });
 }
 
-void Storage::remove(const Vector<Key>& keys, Function<void ()>&& completionHandler)
+void Storage::remove(const Vector<Key>& keys, CompletionHandler<void()>&& completionHandler)
 {
     ASSERT(RunLoop::isMain());
 
@@ -620,10 +620,7 @@
         for (auto& key : keysToRemove)
             deleteFiles(key);
 
-        RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler)] {
-            if (completionHandler)
-                completionHandler();
-        });
+        RunLoop::main().dispatch(WTFMove(completionHandler));
     });
 }
 
@@ -909,7 +906,7 @@
     ASSERT(traverseHandler);
     // Avoid non-thread safe Function copies.
 
-    auto traverseOperationPtr = std::make_unique<TraverseOperation>(*this, type, flags, WTFMove(traverseHandler));
+    auto traverseOperationPtr = std::make_unique<TraverseOperation>(makeRef(*this), type, flags, WTFMove(traverseHandler));
     auto& traverseOperation = *traverseOperationPtr;
     m_activeTraverseOperations.add(WTFMove(traverseOperationPtr));
 
@@ -997,7 +994,7 @@
     shrinkIfNeeded();
 }
 
-void Storage::clear(const String& type, WallTime modifiedSinceTime, Function<void ()>&& completionHandler)
+void Storage::clear(const String& type, WallTime modifiedSinceTime, CompletionHandler<void()>&& completionHandler)
 {
     ASSERT(RunLoop::isMain());
     LOG(NetworkCacheStorage, "(NetworkProcess) clearing cache");
@@ -1025,10 +1022,7 @@
         // This cleans unreferenced blobs.
         m_blobStorage.synchronize();
 
-        RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler)] {
-            if (completionHandler)
-                completionHandler();
-        });
+        RunLoop::main().dispatch(WTFMove(completionHandler));
     });
 }
 
@@ -1147,8 +1141,6 @@
 
             deleteDirectoryRecursively(oldVersionPath);
         });
-
-        RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis)] { });
     });
 }

Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h (236935 => 236936)


--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h	2018-10-08 20:18:26 UTC (rev 236935)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h	2018-10-08 20:22:00 UTC (rev 236936)
@@ -45,7 +45,7 @@
 
 class IOChannel;
 
-class Storage : public ThreadSafeRefCounted<Storage> {
+class Storage : public ThreadSafeRefCounted<Storage, WTF::DestructionThread::Main> {
 public:
     enum class Mode { Normal, AvoidRandomness };
     static RefPtr<Storage> open(const String& cachePath, Mode);
@@ -85,8 +85,8 @@
     void store(const Record&, MappedBodyHandler&&, CompletionHandler<void()>&& = { });
 
     void remove(const Key&);
-    void remove(const Vector<Key>&, Function<void ()>&&);
-    void clear(const String& type, WallTime modifiedSinceTime, Function<void ()>&& completionHandler);
+    void remove(const Vector<Key>&, CompletionHandler<void()>&&);
+    void clear(const String& type, WallTime modifiedSinceTime, CompletionHandler<void()>&&);
 
     struct RecordInfo {
         size_t bodySize;

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to