Changeset 211894 in webkit

Timestamp:

Feb 8, 2017 1:11:32 PM (7 years ago)

Author:

Antti Koivisto

Message:

Allow speculative redirects
​https://bugs.webkit.org/show_bug.cgi?id=167982

Reviewed by Andreas Kling.

If speculative loader hits a redirect it will drop it on the floor. We should use it instead.

• NetworkProcess/NetworkResourceLoader.cpp:

(WebKit::NetworkResourceLoader::continueWillSendRequest):
(WebKit::NetworkResourceLoader::dispatchWillSendRequestForCacheEntry):

Reset m_isWaitingContinueWillSendRequestForCachedRedirect bit immediately.

• NetworkProcess/cache/NetworkCache.cpp:

(WebKit::NetworkCache::Cache::makeRedirectEntry):

Factor to a function.

(WebKit::NetworkCache::Cache::storeRedirect):

• NetworkProcess/cache/NetworkCache.h:

(WebKit::NetworkCache::Cache::speculativeLoadManager):

• NetworkProcess/cache/NetworkCacheEntry.cpp:

(WebKit::NetworkCache::Entry::Entry):

Use std::optional instead std::unique_ptr for the redirect request.

(WebKit::NetworkCache::Entry::decodeStorageRecord):

• NetworkProcess/cache/NetworkCacheEntry.h:

(WebKit::NetworkCache::Entry::redirectRequest):

• NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp:

(WebKit::NetworkCache::SpeculativeLoad::willSendRedirectedRequest):

Make a cache entry for speculative redirect.
Redirect is not actually performed, the target resource will have a separate
speculative entry.

(WebKit::NetworkCache::SpeculativeLoad::didFinishLoading):
(WebKit::NetworkCache::SpeculativeLoad::didFailLoading):
(WebKit::NetworkCache::SpeculativeLoad::didComplete):

Protect against multiple completions.

(WebKit::NetworkCache::SpeculativeLoad::abort): Deleted.

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

(WebKit::NetworkCache::SpeculativeLoadManager::retrieve):

Make successful retrieves asynchronous to avoid re-entrancy problems.

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/NetworkResourceLoader.cpp (modified) (3 diffs)
• NetworkProcess/cache/NetworkCache.cpp (modified) (2 diffs)
• NetworkProcess/cache/NetworkCache.h (modified) (1 diff)
• NetworkProcess/cache/NetworkCacheEntry.cpp (modified) (3 diffs)
• NetworkProcess/cache/NetworkCacheEntry.h (modified) (2 diffs)
• NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp (modified) (4 diffs)
• NetworkProcess/cache/NetworkCacheSpeculativeLoad.h (modified) (2 diffs)
• NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp (modified) (1 diff)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2017-02-08  Antti Koivisto  <antti@apple.com>
+
+        Allow speculative redirects
+        https://bugs.webkit.org/show_bug.cgi?id=167982
+
+        Reviewed by Andreas Kling.
+
+        If speculative loader hits a redirect it will drop it on the floor. We should use it instead.
+
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::continueWillSendRequest):
+        (WebKit::NetworkResourceLoader::dispatchWillSendRequestForCacheEntry):
+
+            Reset m_isWaitingContinueWillSendRequestForCachedRedirect bit immediately.
+
+        * NetworkProcess/cache/NetworkCache.cpp:
+        (WebKit::NetworkCache::Cache::makeRedirectEntry):
+
+            Factor to a function.
+
+        (WebKit::NetworkCache::Cache::storeRedirect):
+        * NetworkProcess/cache/NetworkCache.h:
+        (WebKit::NetworkCache::Cache::speculativeLoadManager):
+        * NetworkProcess/cache/NetworkCacheEntry.cpp:
+        (WebKit::NetworkCache::Entry::Entry):
+
+            Use std::optional instead std::unique_ptr for the redirect request.
+
+        (WebKit::NetworkCache::Entry::decodeStorageRecord):
+        * NetworkProcess/cache/NetworkCacheEntry.h:
+        (WebKit::NetworkCache::Entry::redirectRequest):
+        * NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp:
+        (WebKit::NetworkCache::SpeculativeLoad::willSendRedirectedRequest):
+
+            Make a cache entry for speculative redirect.
+            Redirect is not actually performed, the target resource will have a separate
+            speculative entry.
+
+        (WebKit::NetworkCache::SpeculativeLoad::didFinishLoading):
+        (WebKit::NetworkCache::SpeculativeLoad::didFailLoading):
+        (WebKit::NetworkCache::SpeculativeLoad::didComplete):
+
+            Protect against multiple completions.
+
+        (WebKit::NetworkCache::SpeculativeLoad::abort): Deleted.
+        * NetworkProcess/cache/NetworkCacheSpeculativeLoad.h:
+        * NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp:
+        (WebKit::NetworkCache::SpeculativeLoadManager::retrieve):
+
+            Make successful retrieves asynchronous to avoid re-entrancy problems.
+
 2017-02-08  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp

@@ -477 +477 @@
 #if ENABLE(NETWORK_CACHE)
     if (m_isWaitingContinueWillSendRequestForCachedRedirect) {
+        m_isWaitingContinueWillSendRequestForCachedRedirect = false;
+
         LOG(NetworkCache, "(NetworkProcess) Retrieving cached redirect");
 
@@ -484 +486 @@
             startNetworkLoad(newRequest);
 
-        m_isWaitingContinueWillSendRequestForCachedRedirect = false;
         return;
     }
@@ -644 +645 @@
 {
     ASSERT(entry->redirectRequest());
+    ASSERT(!m_isWaitingContinueWillSendRequestForCachedRedirect);
+
     LOG(NetworkCache, "(NetworkProcess) Executing cached redirect");
 

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

@@ -396 +396 @@
 }
 
+std::unique_ptr<Entry> Cache::makeRedirectEntry(const WebCore::ResourceRequest& request, const WebCore::ResourceResponse& response, const WebCore::ResourceRequest& redirectRequest)
+{
+    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, Function<void (MappedBody&)>&& completionHandler)
 {
@@ -455 +460 @@
     }
 
-    std::unique_ptr<Entry> cacheEntry = std::make_unique<Entry>(makeCacheKey(request), response, redirectRequest, WebCore::collectVaryingRequestHeaders(request, response));
-
+    auto cacheEntry = makeRedirectEntry(request, response, redirectRequest);
     auto record = cacheEntry->encodeAsStorageRecord();
 

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

@@ -126 +126 @@
     
     std::unique_ptr<Entry> makeEntry(const WebCore::ResourceRequest&, const WebCore::ResourceResponse&, RefPtr<WebCore::SharedBuffer>&&);
+    std::unique_ptr<Entry> makeRedirectEntry(const WebCore::ResourceRequest&, const WebCore::ResourceResponse&, const WebCore::ResourceRequest& redirectRequest);
 
     void dumpContentsToFile();
 
     String recordsPath() const;
+
+#if ENABLE(NETWORK_CACHE_SPECULATIVE_REVALIDATION)
+    SpeculativeLoadManager* speculativeLoadManager() { return m_speculativeLoadManager.get(); }
+#endif
 
 private:

trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheEntry.cpp

@@ -55 +55 @@
 {
     ASSERT(m_key.type() == "Resource");
+
+    m_redirectRequest.emplace();
+    m_redirectRequest->setAsIsolatedCopy(redirectRequest);
     // Redirect body is not needed even if exists.
-
-    m_redirectRequest = std::make_unique<WebCore::ResourceRequest>();
-    m_redirectRequest->setAsIsolatedCopy(redirectRequest);
     m_redirectRequest->setHTTPBody(nullptr);
 }
@@ -67 +67 @@
     , m_response(other.m_response)
     , m_varyingRequestHeaders(other.m_varyingRequestHeaders)
+    , m_redirectRequest(other.m_redirectRequest)
     , m_buffer(other.m_buffer)
     , m_sourceStorageRecord(other.m_sourceStorageRecord)
@@ -130 +131 @@
 
     if (isRedirect) {
-        entry->m_redirectRequest = std::make_unique<WebCore::ResourceRequest>();
+        entry->m_redirectRequest.emplace();
         if (!entry->m_redirectRequest->decodeWithoutPlatformData(decoder))
             return nullptr;

trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheEntry.h

@@ -60 +60 @@
 
     WebCore::SharedBuffer* buffer() const;
-    const WebCore::ResourceRequest* redirectRequest() const { return m_redirectRequest.get(); }
+    const std::optional<WebCore::ResourceRequest>& redirectRequest() const { return m_redirectRequest; }
 
 #if ENABLE(SHAREABLE_RESOURCE)
@@ -84 +84 @@
     Vector<std::pair<String, String>> m_varyingRequestHeaders;
 
-    std::unique_ptr<WebCore::ResourceRequest> m_redirectRequest;
+    std::optional<WebCore::ResourceRequest> m_redirectRequest;
     mutable RefPtr<WebCore::SharedBuffer> m_buffer;
 #if ENABLE(SHAREABLE_RESOURCE)

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

@@ -68 +68 @@
 }
 
-void SpeculativeLoad::willSendRedirectedRequest(ResourceRequest&&, ResourceRequest&& redirectRequest, ResourceResponse&& redirectResponse)
+void SpeculativeLoad::willSendRedirectedRequest(ResourceRequest&& request, ResourceRequest&& redirectRequest, ResourceResponse&& redirectResponse)
 {
-    LOG(NetworkCacheSpeculativePreloading, "(NetworkProcess) Speculative revalidation for %s hit a redirect, aborting the load.", redirectResponse.url().string().utf8().data());
-    // We drop speculative revalidations if they redirect for now as we would need to notify WebCore of such redirects.
-    abort();
+    LOG(NetworkCacheSpeculativePreloading, "Speculative redirect %s -> %s", request.url().string().utf8().data(), redirectRequest.url().string().utf8().data());
+
+    m_cacheEntry = NetworkCache::singleton().storeRedirect(request, redirectResponse, redirectRequest);
+    // Create a synthetic cache entry if we can't store.
+    if (!m_cacheEntry)
+        m_cacheEntry = NetworkCache::singleton().makeRedirectEntry(request, redirectResponse, redirectRequest);
+
+    auto load = WTFMove(m_networkLoad);
+
+    // Don't follow the redirect. The redirect target will be registered for speculative load when it is loaded.
+    didComplete();
+
+    // This causes call to didFailLoading().
+    if (load)
+        load->continueWillSendRequest({ });
 }
 
@@ -107 +119 @@
 void SpeculativeLoad::didFinishLoading(double finishTime)
 {
+    if (m_didComplete)
+        return;
     if (!m_cacheEntry && m_bufferedDataForCache) {
-        m_cacheEntry = NetworkCache::singleton().store(m_originalRequest, m_response, WTFMove(m_bufferedDataForCache), [](auto& mappedBody) { });
+        m_cacheEntry = NetworkCache::singleton().store(m_originalRequest, m_response, m_bufferedDataForCache.copyRef(), [](auto& mappedBody) { });
         // Create a synthetic cache entry if we can't store.
         if (!m_cacheEntry && m_response.httpStatusCode() == 200)
@@ -126 +140 @@
 void SpeculativeLoad::didFailLoading(const ResourceError&)
 {
+    if (m_didComplete)
+        return;
     m_cacheEntry = nullptr;
 
-    didComplete();
-}
-
-void SpeculativeLoad::abort()
-{
-    if (m_networkLoad)
-        m_networkLoad->cancel();
-
-    m_cacheEntry = nullptr;
     didComplete();
 }
@@ -144 +151 @@
     RELEASE_ASSERT(RunLoop::isMain());
 
+    if (m_didComplete)
+        return;
+    m_didComplete = true;
     m_networkLoad = nullptr;
 

trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoad.h

@@ -65 +65 @@
     void didFailLoading(const WebCore::ResourceError&) override;
 
-    void abort();
     void didComplete();
 
@@ -78 +77 @@
     RefPtr<WebCore::SharedBuffer> m_bufferedDataForCache;
     std::unique_ptr<NetworkCache::Entry> m_cacheEntry;
+    bool m_didComplete { false };
 };
 

trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp

@@ -346 +346 @@
 {
     if (auto preloadedEntry = m_preloadedEntries.take(storageKey)) {
-        completionHandler(preloadedEntry->takeCacheEntry());
+        RunLoop::main().dispatch([completionHandler = WTFMove(completionHandler), cacheEntry = preloadedEntry->takeCacheEntry()] () mutable {
+            completionHandler(WTFMove(cacheEntry));
+        });
         return;
     }