Title: [211869] trunk/Source/WebKit2
Revision
211869
Author
[email protected]
Date
2017-02-08 07:10:07 -0800 (Wed, 08 Feb 2017)

Log 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.

* NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::continueWillSendRequest):
(WebKit::NetworkResourceLoader::dispatchWillSendRequestForCacheEntry):

    Reset the 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 cache entry for speculative redirect.
    The redirect is not actually performed, the target resource will have separate
    a 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.

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (211868 => 211869)


--- trunk/Source/WebKit2/ChangeLog	2017-02-08 12:39:24 UTC (rev 211868)
+++ trunk/Source/WebKit2/ChangeLog	2017-02-08 15:10:07 UTC (rev 211869)
@@ -1,3 +1,54 @@
+2017-02-08  Antti Koivisto  <[email protected]>
+
+        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.
+
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::continueWillSendRequest):
+        (WebKit::NetworkResourceLoader::dispatchWillSendRequestForCacheEntry):
+
+            Reset the 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 cache entry for speculative redirect.
+            The redirect is not actually performed, the target resource will have separate
+            a 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-07  Brent Fulgham  <[email protected]>
 
         Correct initialization of Resource Load Statistics for use in WebKitTestRunner

Modified: trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp (211868 => 211869)


--- trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp	2017-02-08 12:39:24 UTC (rev 211868)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp	2017-02-08 15:10:07 UTC (rev 211869)
@@ -476,6 +476,8 @@
 
 #if ENABLE(NETWORK_CACHE)
     if (m_isWaitingContinueWillSendRequestForCachedRedirect) {
+        m_isWaitingContinueWillSendRequestForCachedRedirect = false;
+
         LOG(NetworkCache, "(NetworkProcess) Retrieving cached redirect");
 
         if (canUseCachedRedirect(newRequest))
@@ -483,7 +485,6 @@
         else
             startNetworkLoad(newRequest);
 
-        m_isWaitingContinueWillSendRequestForCachedRedirect = false;
         return;
     }
 #endif
@@ -643,6 +644,8 @@
 void NetworkResourceLoader::dispatchWillSendRequestForCacheEntry(std::unique_ptr<NetworkCache::Entry> entry)
 {
     ASSERT(entry->redirectRequest());
+    ASSERT(!m_isWaitingContinueWillSendRequestForCachedRedirect);
+
     LOG(NetworkCache, "(NetworkProcess) Executing cached redirect");
 
     ++m_redirectCount;

Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp (211868 => 211869)


--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp	2017-02-08 12:39:24 UTC (rev 211868)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp	2017-02-08 15:10:07 UTC (rev 211869)
@@ -395,6 +395,11 @@
     return std::make_unique<Entry>(makeCacheKey(request), response, WTFMove(responseData), WebCore::collectVaryingRequestHeaders(request, response));
 }
 
+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)
 {
     ASSERT(isEnabled());
@@ -454,8 +459,7 @@
         return nullptr;
     }
 
-    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();
 
     m_storage->store(record, nullptr);

Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.h (211868 => 211869)


--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.h	2017-02-08 12:39:24 UTC (rev 211868)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.h	2017-02-08 15:10:07 UTC (rev 211869)
@@ -125,11 +125,16 @@
     void storeData(const DataKey&,  const uint8_t* data, size_t);
     
     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:
     Cache() = default;
     ~Cache() = delete;

Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheEntry.cpp (211868 => 211869)


--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheEntry.cpp	2017-02-08 12:39:24 UTC (rev 211868)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheEntry.cpp	2017-02-08 15:10:07 UTC (rev 211869)
@@ -52,12 +52,10 @@
     , m_timeStamp(std::chrono::system_clock::now())
     , m_response(response)
     , m_varyingRequestHeaders(varyingRequestHeaders)
+    , m_redirectRequest(redirectRequest)
 {
     ASSERT(m_key.type() == "Resource");
     // Redirect body is not needed even if exists.
-
-    m_redirectRequest = std::make_unique<WebCore::ResourceRequest>();
-    m_redirectRequest->setAsIsolatedCopy(redirectRequest);
     m_redirectRequest->setHTTPBody(nullptr);
 }
 
@@ -66,6 +64,7 @@
     , m_timeStamp(other.m_timeStamp)
     , 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)
 {
@@ -129,7 +128,7 @@
         return nullptr;
 
     if (isRedirect) {
-        entry->m_redirectRequest = std::make_unique<WebCore::ResourceRequest>();
+        entry->m_redirectRequest.emplace();
         if (!entry->m_redirectRequest->decodeWithoutPlatformData(decoder))
             return nullptr;
     }

Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheEntry.h (211868 => 211869)


--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheEntry.h	2017-02-08 12:39:24 UTC (rev 211868)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheEntry.h	2017-02-08 15:10:07 UTC (rev 211869)
@@ -59,7 +59,7 @@
     const Vector<std::pair<String, String>>& varyingRequestHeaders() const { return m_varyingRequestHeaders; }
 
     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)
     ShareableResource::Handle& shareableResourceHandle() const;
@@ -83,7 +83,7 @@
     WebCore::ResourceResponse m_response;
     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)
     mutable ShareableResource::Handle m_shareableResourceHandle;

Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp (211868 => 211869)


--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp	2017-02-08 12:39:24 UTC (rev 211868)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp	2017-02-08 15:10:07 UTC (rev 211869)
@@ -67,11 +67,23 @@
     ASSERT(!m_networkLoad);
 }
 
-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({ });
 }
 
 auto SpeculativeLoad::didReceiveResponse(ResourceResponse&& receivedResponse) -> ShouldContinueDidReceiveResponse
@@ -106,8 +118,10 @@
 
 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)
             m_cacheEntry = NetworkCache::singleton().makeEntry(m_originalRequest, m_response, WTFMove(m_bufferedDataForCache));
@@ -125,24 +139,20 @@
 
 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();
-}
-
 void SpeculativeLoad::didComplete()
 {
     RELEASE_ASSERT(RunLoop::isMain());
 
+    if (m_didComplete)
+        return;
+    m_didComplete = true;
     m_networkLoad = nullptr;
 
     // Make sure speculatively revalidated resources do not get validated by the NetworkResourceLoader again.

Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoad.h (211868 => 211869)


--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoad.h	2017-02-08 12:39:24 UTC (rev 211868)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoad.h	2017-02-08 15:10:07 UTC (rev 211869)
@@ -64,7 +64,6 @@
     void didFinishLoading(double finishTime) override;
     void didFailLoading(const WebCore::ResourceError&) override;
 
-    void abort();
     void didComplete();
 
     GlobalFrameID m_frameID;
@@ -77,6 +76,7 @@
 
     RefPtr<WebCore::SharedBuffer> m_bufferedDataForCache;
     std::unique_ptr<NetworkCache::Entry> m_cacheEntry;
+    bool m_didComplete { false };
 };
 
 } // namespace NetworkCache

Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp (211868 => 211869)


--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp	2017-02-08 12:39:24 UTC (rev 211868)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp	2017-02-08 15:10:07 UTC (rev 211869)
@@ -345,7 +345,9 @@
 void SpeculativeLoadManager::retrieve(const Key& storageKey, RetrieveCompletionHandler&& completionHandler)
 {
     if (auto preloadedEntry = m_preloadedEntries.take(storageKey)) {
-        completionHandler(preloadedEntry->takeCacheEntry());
+        RunLoop::main().dispatch([completionHandler = WTFMove(completionHandler), cacheEntry = preloadedEntry->takeCacheEntry()] () mutable {
+            completionHandler(WTFMove(cacheEntry));
+        });
         return;
     }
     ASSERT(m_pendingPreloads.contains(storageKey));
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to