Changeset 239759 in webkit

Timestamp:

Jan 8, 2019, 5:28:13 PM (6 years ago)

Author:

achristensen@apple.com

Message:

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):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/NetworkResourceLoader.cpp (modified) (1 diff)
• NetworkProcess/cache/CacheStorageEngine.cpp (modified) (1 diff)
• NetworkProcess/cache/CacheStorageEngine.h (modified) (2 diffs)
• NetworkProcess/cache/NetworkCache.cpp (modified) (6 diffs)
• NetworkProcess/cache/NetworkCache.h (modified) (2 diffs)
• NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp (modified) (1 diff)
• NetworkProcess/cache/NetworkCacheStorage.cpp (modified) (2 diffs)
• NetworkProcess/cache/NetworkCacheStorage.h (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp

@@ -795 +795 @@
         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
     });

trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp

@@ -74 +74 @@
 }
 
-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)) {

trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.h

@@ -60 +60 @@
     ~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>)>&&);
@@ -121 +121 @@
 
     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&&);
 

trunk/Source/WebKit/NetworkProcess/cache/NetworkCache.cpp

@@ -373 +373 @@
 }
 
-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);
@@ -393 +393 @@
             m_statistics->recordNotCachingResponse(key, storeDecision);
 
-        completionHandler(nullptr);
         return nullptr;
     }
@@ -409 +408 @@
         }
 #endif
-        completionHandler(&mappedBody);
+        completionHandler(mappedBody);
         LOG(NetworkCache, "(NetworkProcess) stored");
     });
@@ -476 +475 @@
 }
 
-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.
@@ -580 +579 @@
 }
 
-void Cache::clear(WallTime modifiedSince, CompletionHandler<void()>&& completionHandler)
+void Cache::clear(WallTime modifiedSince, Function<void()>&& completionHandler)
 {
     LOG(NetworkCache, "(NetworkProcess) clearing cache");
@@ -603 +602 @@
 }
 
-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() };

trunk/Source/WebKit/NetworkProcess/cache/NetworkCache.h

@@ -115 +115 @@
         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);
@@ -125 +125 @@
         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);
     

trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp

@@ -122 +122 @@
         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()))

trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp

@@ -114 +114 @@
     { }
 
-    ~WriteOperation()
-    {
-        if (completionHandler)
-            completionHandler(0);
-    }
-
     Ref<Storage> storage;
 
@@ -892 +886 @@
     ASSERT(!record.key.isNull());
 
-    if (!m_capacity) {
-        completionHandler(0);
-        return;
-    }
+    if (!m_capacity)
+        return;
 
     auto writeOperation = std::make_unique<WriteOperation>(*this, record, WTFMove(mappedBodyHandler), WTFMove(completionHandler));

trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h

@@ -83 +83 @@
 
     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&);