Fix CompletionHandler assertions introduced today.
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Jan 2019 01:28:13 +0000 (01:28 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Jan 2019 01:28:13 +0000 (01:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193237

This reverts part of r239710 and all of r239725, r239738, and r239748 which unsuccessfully tried to fix all the assertions.
This code is a mess that will have to be cleaned up later.

* NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::tryStoreAsCacheEntry):
* NetworkProcess/cache/CacheStorageEngine.cpp:
(WebKit::CacheStorage::Engine::from):
* NetworkProcess/cache/CacheStorageEngine.h:
* NetworkProcess/cache/NetworkCache.cpp:
(WebKit::NetworkCache::Cache::store):
(WebKit::NetworkCache::Cache::remove):
(WebKit::NetworkCache::Cache::traverse):
(WebKit::NetworkCache::Cache::clear):
(WebKit::NetworkCache::Cache::retrieveData):
* NetworkProcess/cache/NetworkCache.h:
* NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp:
(WebKit::NetworkCache::SpeculativeLoad::didFinishLoading):
* NetworkProcess/cache/NetworkCacheStorage.cpp:
(WebKit::NetworkCache::Storage::store):
(WebKit::NetworkCache::Storage::WriteOperation::~WriteOperation): Deleted.
* NetworkProcess/cache/NetworkCacheStorage.h:
(WebKit::NetworkCache::Storage::store):

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

Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp
Source/WebKit/NetworkProcess/cache/CacheStorageEngine.h
Source/WebKit/NetworkProcess/cache/NetworkCache.cpp
Source/WebKit/NetworkProcess/cache/NetworkCache.h
Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp
Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp
Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h

index 907ecc8..dcb7b35 100644 (file)
@@ -1,3 +1,31 @@
+2019-01-08  Alex Christensen  <achristensen@webkit.org>
+
+        Fix CompletionHandler assertions introduced today.
+        https://bugs.webkit.org/show_bug.cgi?id=193237
+
+        This reverts part of r239710 and all of r239725, r239738, and r239748 which unsuccessfully tried to fix all the assertions.
+        This code is a mess that will have to be cleaned up later.
+
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::tryStoreAsCacheEntry):
+        * NetworkProcess/cache/CacheStorageEngine.cpp:
+        (WebKit::CacheStorage::Engine::from):
+        * NetworkProcess/cache/CacheStorageEngine.h:
+        * NetworkProcess/cache/NetworkCache.cpp:
+        (WebKit::NetworkCache::Cache::store):
+        (WebKit::NetworkCache::Cache::remove):
+        (WebKit::NetworkCache::Cache::traverse):
+        (WebKit::NetworkCache::Cache::clear):
+        (WebKit::NetworkCache::Cache::retrieveData):
+        * NetworkProcess/cache/NetworkCache.h:
+        * NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp:
+        (WebKit::NetworkCache::SpeculativeLoad::didFinishLoading):
+        * NetworkProcess/cache/NetworkCacheStorage.cpp:
+        (WebKit::NetworkCache::Storage::store):
+        (WebKit::NetworkCache::Storage::WriteOperation::~WriteOperation): Deleted.
+        * NetworkProcess/cache/NetworkCacheStorage.h:
+        (WebKit::NetworkCache::Storage::store):
+
 2019-01-08  Jiewen Tan  <jiewen_tan@apple.com>
 
         [Mac] Layout Test http/wpt/webauthn/public-key-credential-create-success-hid.https.html and http/wpt/webauthn/public-key-credential-get-success-hid.https.html are flaky
index 7090598..7f8bda3 100644 (file)
@@ -794,12 +794,12 @@ void NetworkResourceLoader::tryStoreAsCacheEntry()
     if (!m_bufferedDataForCache)
         return;
 
-    m_cache->store(m_networkLoad->currentRequest(), m_response, WTFMove(m_bufferedDataForCache), [loader = makeRef(*this)](auto* mappedBody) mutable {
+    m_cache->store(m_networkLoad->currentRequest(), m_response, WTFMove(m_bufferedDataForCache), [loader = makeRef(*this)](auto& mappedBody) mutable {
 #if ENABLE(SHAREABLE_RESOURCE)
-        if (!mappedBody || mappedBody->shareableResourceHandle.isNull())
+        if (mappedBody.shareableResourceHandle.isNull())
             return;
         LOG(NetworkCache, "(NetworkProcess) sending DidCacheResource");
-        loader->send(Messages::NetworkProcessConnection::DidCacheResource(loader->originalRequest(), mappedBody->shareableResourceHandle, loader->sessionID()));
+        loader->send(Messages::NetworkProcessConnection::DidCacheResource(loader->originalRequest(), mappedBody.shareableResourceHandle, loader->sessionID()));
 #endif
     });
 }
index e1043ed..801d22e 100644 (file)
@@ -73,7 +73,7 @@ Engine::~Engine()
         callback(Data { }, 1);
 }
 
-void Engine::from(NetworkProcess& networkProcess, PAL::SessionID sessionID, CompletionHandler<void(Engine&)>&& callback)
+void Engine::from(NetworkProcess& networkProcess, PAL::SessionID sessionID, Function<void(Engine&)>&& callback)
 {
     if (auto* engine = networkProcess.findCacheEngine(sessionID)) {
         callback(*engine);
index b497e69..1ebcb73 100644 (file)
@@ -59,7 +59,7 @@ class Engine : public RefCounted<Engine>, public CanMakeWeakPtr<Engine> {
 public:
     ~Engine();
 
-    static void from(NetworkProcess&, PAL::SessionID, CompletionHandler<void(Engine&)>&&);
+    static void from(NetworkProcess&, PAL::SessionID, Function<void(Engine&)>&&);
     static void destroyEngine(NetworkProcess&, PAL::SessionID);
     static void fetchEntries(NetworkProcess&, PAL::SessionID, bool shouldComputeSize, CompletionHandler<void(Vector<WebsiteData::Entry>)>&&);
 
@@ -120,11 +120,11 @@ private:
     void initialize(WebCore::DOMCacheEngine::CompletionCallback&&);
 
     using CachesOrError = Expected<std::reference_wrapper<Caches>, WebCore::DOMCacheEngine::Error>;
-    using CachesCallback = CompletionHandler<void(CachesOrError&&)>;
+    using CachesCallback = Function<void(CachesOrError&&)>;
     void readCachesFromDisk(const WebCore::ClientOrigin&, CachesCallback&&);
 
     using CacheOrError = Expected<std::reference_wrapper<Cache>, WebCore::DOMCacheEngine::Error>;
-    using CacheCallback = CompletionHandler<void(CacheOrError&&)>;
+    using CacheCallback = Function<void(CacheOrError&&)>;
     void readCache(uint64_t cacheIdentifier, CacheCallback&&);
 
     Cache* cache(uint64_t cacheIdentifier);
index 54b65aa..ce35ad2 100644 (file)
@@ -372,7 +372,7 @@ std::unique_ptr<Entry> Cache::makeRedirectEntry(const WebCore::ResourceRequest&
     return std::make_unique<Entry>(makeCacheKey(request), response, redirectRequest, WebCore::collectVaryingRequestHeaders(request, response));
 }
 
-std::unique_ptr<Entry> Cache::store(const WebCore::ResourceRequest& request, const WebCore::ResourceResponse& response, RefPtr<WebCore::SharedBuffer>&& responseData, CompletionHandler<void(MappedBody*)>&& completionHandler)
+std::unique_ptr<Entry> Cache::store(const WebCore::ResourceRequest& request, const WebCore::ResourceResponse& response, RefPtr<WebCore::SharedBuffer>&& responseData, Function<void(MappedBody&)>&& completionHandler)
 {
     ASSERT(responseData);
 
@@ -392,7 +392,6 @@ std::unique_ptr<Entry> Cache::store(const WebCore::ResourceRequest& request, con
         if (m_statistics)
             m_statistics->recordNotCachingResponse(key, storeDecision);
 
-        completionHandler(nullptr);
         return nullptr;
     }
 
@@ -408,7 +407,7 @@ std::unique_ptr<Entry> Cache::store(const WebCore::ResourceRequest& request, con
             mappedBody.shareableResource->createHandle(mappedBody.shareableResourceHandle);
         }
 #endif
-        completionHandler(&mappedBody);
+        completionHandler(mappedBody);
         LOG(NetworkCache, "(NetworkProcess) stored");
     });
 
@@ -475,12 +474,12 @@ void Cache::remove(const WebCore::ResourceRequest& request)
     remove(makeCacheKey(request));
 }
 
-void Cache::remove(const Vector<Key>& keys, CompletionHandler<void()>&& completionHandler)
+void Cache::remove(const Vector<Key>& keys, Function<void()>&& completionHandler)
 {
     m_storage->remove(keys, WTFMove(completionHandler));
 }
 
-void Cache::traverse(CompletionHandler<void(const TraversalEntry*)>&& traverseHandler)
+void Cache::traverse(Function<void(const TraversalEntry*)>&& traverseHandler)
 {
     // Protect against clients making excessive traversal requests.
     const unsigned maximumTraverseCount = 3;
@@ -579,7 +578,7 @@ void Cache::deleteDumpFile()
     });
 }
 
-void Cache::clear(WallTime modifiedSince, CompletionHandler<void()>&& completionHandler)
+void Cache::clear(WallTime modifiedSince, Function<void()>&& completionHandler)
 {
     LOG(NetworkCache, "(NetworkProcess) clearing cache");
 
@@ -602,7 +601,7 @@ String Cache::recordsPath() const
     return m_storage->recordsPath();
 }
 
-void Cache::retrieveData(const DataKey& dataKey, CompletionHandler<void(const uint8_t*, size_t)> completionHandler)
+void Cache::retrieveData(const DataKey& dataKey, Function<void(const uint8_t*, size_t)> completionHandler)
 {
     Key key { dataKey, m_storage->salt() };
     m_storage->retrieve(key, 4, [completionHandler = WTFMove(completionHandler)] (auto record, auto) mutable {
index 44d2716..3e47861 100644 (file)
@@ -114,9 +114,9 @@ public:
 
         WTF_MAKE_FAST_ALLOCATED;
     };
-    using RetrieveCompletionHandler = CompletionHandler<void(std::unique_ptr<Entry>, const RetrieveInfo&)>;
+    using RetrieveCompletionHandler = Function<void(std::unique_ptr<Entry>, const RetrieveInfo&)>;
     void retrieve(const WebCore::ResourceRequest&, const GlobalFrameID&, RetrieveCompletionHandler&&);
-    std::unique_ptr<Entry> store(const WebCore::ResourceRequest&, const WebCore::ResourceResponse&, RefPtr<WebCore::SharedBuffer>&&, CompletionHandler<void(MappedBody*)>&&);
+    std::unique_ptr<Entry> store(const WebCore::ResourceRequest&, const WebCore::ResourceResponse&, RefPtr<WebCore::SharedBuffer>&&, Function<void(MappedBody&)>&&);
     std::unique_ptr<Entry> storeRedirect(const WebCore::ResourceRequest&, const WebCore::ResourceResponse&, const WebCore::ResourceRequest& redirectRequest, Optional<Seconds> maxAgeCap);
     std::unique_ptr<Entry> update(const WebCore::ResourceRequest&, const GlobalFrameID&, const Entry&, const WebCore::ResourceResponse& validatingResponse);
 
@@ -124,15 +124,15 @@ public:
         const Entry& entry;
         const Storage::RecordInfo& recordInfo;
     };
-    void traverse(CompletionHandler<void(const TraversalEntry*)>&&);
+    void traverse(Function<void(const TraversalEntry*)>&&);
     void remove(const Key&);
     void remove(const WebCore::ResourceRequest&);
-    void remove(const Vector<Key>&, CompletionHandler<void()>&&);
+    void remove(const Vector<Key>&, Function<void()>&&);
 
     void clear();
-    void clear(WallTime modifiedSince, CompletionHandler<void()>&&);
+    void clear(WallTime modifiedSince, Function<void()>&&);
 
-    void retrieveData(const DataKey&, CompletionHandler<void(const uint8_t*, size_t)>);
+    void retrieveData(const DataKey&, Function<void(const uint8_t*, size_t)>);
     void storeData(const DataKey&,  const uint8_t* data, size_t);
     
     std::unique_ptr<Entry> makeEntry(const WebCore::ResourceRequest&, const WebCore::ResourceResponse&, RefPtr<WebCore::SharedBuffer>&&);
index 7dfc7ec..7e7baf5 100644 (file)
@@ -121,7 +121,7 @@ void SpeculativeLoad::didFinishLoading(const WebCore::NetworkLoadMetrics&)
     if (m_didComplete)
         return;
     if (!m_cacheEntry && m_bufferedDataForCache) {
-        m_cacheEntry = m_cache->store(m_originalRequest, m_response, m_bufferedDataForCache.copyRef(), [](auto* mappedBody) { });
+        m_cacheEntry = m_cache->store(m_originalRequest, m_response, m_bufferedDataForCache.copyRef(), [](auto& mappedBody) { });
         // Create a synthetic cache entry if we can't store.
         if (!m_cacheEntry && isStatusCodeCacheableByDefault(m_response.httpStatusCode()))
             m_cacheEntry = m_cache->makeEntry(m_originalRequest, m_response, WTFMove(m_bufferedDataForCache));
index 63327f4..4ea7430 100644 (file)
@@ -113,12 +113,6 @@ public:
         , completionHandler(WTFMove(completionHandler))
     { }
 
-    ~WriteOperation()
-    {
-        if (completionHandler)
-            completionHandler(0);
-    }
-
     Ref<Storage> storage;
 
     const Record record;
@@ -891,10 +885,8 @@ void Storage::store(const Record& record, MappedBodyHandler&& mappedBodyHandler,
     ASSERT(RunLoop::isMain());
     ASSERT(!record.key.isNull());
 
-    if (!m_capacity) {
-        completionHandler(0);
+    if (!m_capacity)
         return;
-    }
 
     auto writeOperation = std::make_unique<WriteOperation>(*this, record, WTFMove(mappedBodyHandler), WTFMove(completionHandler));
     m_pendingWriteOperations.prepend(WTFMove(writeOperation));
index 30a6540..c354581 100644 (file)
@@ -82,7 +82,7 @@ public:
     void retrieve(const Key&, unsigned priority, RetrieveCompletionHandler&&);
 
     using MappedBodyHandler = Function<void (const Data& mappedBody)>;
-    void store(const Record&, MappedBodyHandler&&, CompletionHandler<void(int)>&& = [] (int) { });
+    void store(const Record&, MappedBodyHandler&&, CompletionHandler<void(int)>&& = { });
 
     void remove(const Key&);
     void remove(const Vector<Key>&, CompletionHandler<void()>&&);