Diff
Modified: trunk/Source/WebKit2/ChangeLog (211893 => 211894)
--- trunk/Source/WebKit2/ChangeLog 2017-02-08 20:40:32 UTC (rev 211893)
+++ trunk/Source/WebKit2/ChangeLog 2017-02-08 21:11:32 UTC (rev 211894)
@@ -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 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 <[email protected]>
Unreviewed build fix; FullscreenClient only avaialble on 32-bit builds.
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp (211893 => 211894)
--- trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp 2017-02-08 20:40:32 UTC (rev 211893)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp 2017-02-08 21:11:32 UTC (rev 211894)
@@ -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 (211893 => 211894)
--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp 2017-02-08 20:40:32 UTC (rev 211893)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp 2017-02-08 21:11:32 UTC (rev 211894)
@@ -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 (211893 => 211894)
--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.h 2017-02-08 20:40:32 UTC (rev 211893)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.h 2017-02-08 21:11:32 UTC (rev 211894)
@@ -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 (211893 => 211894)
--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheEntry.cpp 2017-02-08 20:40:32 UTC (rev 211893)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheEntry.cpp 2017-02-08 21:11:32 UTC (rev 211894)
@@ -54,10 +54,10 @@
, m_varyingRequestHeaders(varyingRequestHeaders)
{
ASSERT(m_key.type() == "Resource");
- // Redirect body is not needed even if exists.
- m_redirectRequest = std::make_unique<WebCore::ResourceRequest>();
+ m_redirectRequest.emplace();
m_redirectRequest->setAsIsolatedCopy(redirectRequest);
+ // Redirect body is not needed even if exists.
m_redirectRequest->setHTTPBody(nullptr);
}
@@ -66,6 +66,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 +130,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 (211893 => 211894)
--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheEntry.h 2017-02-08 20:40:32 UTC (rev 211893)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheEntry.h 2017-02-08 21:11:32 UTC (rev 211894)
@@ -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 (211893 => 211894)
--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp 2017-02-08 20:40:32 UTC (rev 211893)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp 2017-02-08 21:11:32 UTC (rev 211894)
@@ -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 (211893 => 211894)
--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoad.h 2017-02-08 20:40:32 UTC (rev 211893)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoad.h 2017-02-08 21:11:32 UTC (rev 211894)
@@ -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 (211893 => 211894)
--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp 2017-02-08 20:40:32 UTC (rev 211893)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp 2017-02-08 21:11:32 UTC (rev 211894)
@@ -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));
