Changeset 239725 in webkit

Timestamp:

Jan 8, 2019, 8:43:14 AM (6 years ago)

Author:

achristensen@apple.com

Message:

Always call CompletionHandler in Cache::store
​https://bugs.webkit.org/show_bug.cgi?id=193237

Reviewed by Chris Dumez.

No change in behavior. This just fixes an assertion introduced in r239710 when the speculative loader finishes.

• NetworkProcess/NetworkResourceLoader.cpp:

(WebKit::NetworkResourceLoader::tryStoreAsCacheEntry):

• NetworkProcess/cache/NetworkCache.cpp:

(WebKit::NetworkCache::Cache::store):

• NetworkProcess/cache/NetworkCache.h:
• NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp:

(WebKit::NetworkCache::SpeculativeLoad::didFinishLoading):

Location:

trunk/Source/WebKit

Files:

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

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-01-08  Alex Christensen  <achristensen@webkit.org>
+
+        Always call CompletionHandler in Cache::store
+        https://bugs.webkit.org/show_bug.cgi?id=193237
+
+        Reviewed by Chris Dumez.
+
+        No change in behavior. This just fixes an assertion introduced in r239710 when the speculative loader finishes.
+
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::tryStoreAsCacheEntry):
+        * NetworkProcess/cache/NetworkCache.cpp:
+        (WebKit::NetworkCache::Cache::store):
+        * NetworkProcess/cache/NetworkCache.h:
+        * NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp:
+        (WebKit::NetworkCache::SpeculativeLoad::didFinishLoading):
+
 2019-01-08  Carlos Garcia Campos  <cgarcia@igalia.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.shareableResourceHandle.isNull())
+        if (!mappedBody || 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/NetworkCache.cpp

@@ -372 +372 @@
 }
 
-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, CompletionHandler<void(MappedBody*)>&& completionHandler)
 {
     ASSERT(responseData);
@@ -392 +392 @@
             m_statistics->recordNotCachingResponse(key, storeDecision);
 
+        completionHandler(nullptr);
         return nullptr;
     }
@@ -407 +408 @@
         }
 #endif
-        completionHandler(mappedBody);
+        completionHandler(&mappedBody);
         LOG(NetworkCache, "(NetworkProcess) stored");
     });

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

@@ -114 +114 @@
     using RetrieveCompletionHandler = CompletionHandler<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>&&, CompletionHandler<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);

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

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