Always call CompletionHandler in Cache::store
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Jan 2019 16:43:14 +0000 (16:43 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Jan 2019 16:43:14 +0000 (16:43 +0000)
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):

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

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

index 310d6a7..e04b29e 100644 (file)
@@ -1,3 +1,20 @@
+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>
 
         Unreviewed. Update OptionsGTK.cmake and NEWS for 2.23.2 release
index 4b46bee..9e4771c 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.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
     });
 }
index 9388209..1d3542f 100644 (file)
@@ -371,7 +371,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, CompletionHandler<void(MappedBody*)>&& completionHandler)
 {
     ASSERT(responseData);
 
@@ -391,6 +391,7 @@ std::unique_ptr<Entry> Cache::store(const WebCore::ResourceRequest& request, con
         if (m_statistics)
             m_statistics->recordNotCachingResponse(key, storeDecision);
 
+        completionHandler(nullptr);
         return nullptr;
     }
 
@@ -406,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");
     });
 
index 7c3006c..aca4436 100644 (file)
@@ -113,7 +113,7 @@ public:
     };
     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);
 
index 7e7baf5..7dfc7ec 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 1a8df36..b91fea9 100644 (file)
@@ -884,8 +884,10 @@ void Storage::store(const Record& record, MappedBodyHandler&& mappedBodyHandler,
     ASSERT(RunLoop::isMain());
     ASSERT(!record.key.isNull());
 
-    if (!m_capacity)
+    if (!m_capacity) {
+        completionHandler(0);
         return;
+    }
 
     auto writeOperation = std::make_unique<WriteOperation>(*this, record, WTFMove(mappedBodyHandler), WTFMove(completionHandler));
     m_pendingWriteOperations.prepend(WTFMove(writeOperation));