Diff
Modified: trunk/LayoutTests/ChangeLog (252777 => 252778)
--- trunk/LayoutTests/ChangeLog 2019-11-22 17:08:01 UTC (rev 252777)
+++ trunk/LayoutTests/ChangeLog 2019-11-22 17:25:29 UTC (rev 252778)
@@ -1,3 +1,18 @@
+2019-11-22 Chris Dumez <[email protected]>
+
+ Speculative loading sometimes happens too early and is missing login cookies
+ https://bugs.webkit.org/show_bug.cgi?id=204305
+ <rdar://problem/57063840>
+
+ Reviewed by Antti Koivisto.
+
+ Extend layout test coverage to make sure that the validation request contains the latest cookies
+ set by the main resource.
+
+ * http/tests/cache/disk-cache/speculative-validation/resources/validation-request-frame.php:
+ * http/tests/cache/disk-cache/speculative-validation/validation-request-expected.txt:
+ * http/tests/cache/disk-cache/speculative-validation/validation-request.html:
+
2019-11-22 Per Arne Vollan <[email protected]>
Layout Test storage/indexeddb/modern/new-database-after-user-delete.html is flaky
Modified: trunk/LayoutTests/http/tests/cache/disk-cache/speculative-validation/resources/validation-request-frame.php (252777 => 252778)
--- trunk/LayoutTests/http/tests/cache/disk-cache/speculative-validation/resources/validation-request-frame.php 2019-11-22 17:08:01 UTC (rev 252777)
+++ trunk/LayoutTests/http/tests/cache/disk-cache/speculative-validation/resources/validation-request-frame.php 2019-11-22 17:25:29 UTC (rev 252778)
@@ -2,9 +2,15 @@
header('Content-Type: text/html');
header('Cache-Control: max-age=0');
header('Etag: 123456789');
-
+$cookie = "speculativeRequestValidation=" . uniqid();
+header('Set-Cookie: ' . $cookie);
?>
<!DOCTYPE html>
<body>
<script src=""
+<script>
+<?php
+echo "sentSetCookieHeader = '" . $cookie . "';";
+?>
+</script>
</body>
Modified: trunk/LayoutTests/http/tests/cache/disk-cache/speculative-validation/validation-request-expected.txt (252777 => 252778)
--- trunk/LayoutTests/http/tests/cache/disk-cache/speculative-validation/validation-request-expected.txt 2019-11-22 17:08:01 UTC (rev 252777)
+++ trunk/LayoutTests/http/tests/cache/disk-cache/speculative-validation/validation-request-expected.txt 2019-11-22 17:25:29 UTC (rev 252778)
@@ -4,6 +4,7 @@
PASS validationRequestHeader('If-None-Match') is "123456789"
+PASS isCookieHeaderCorrect is true
PASS validationRequestHeader('Accept') is initialHeaderValues['Accept']
PASS validationRequestHeader('Accept-Encoding') is initialHeaderValues['Accept-Encoding']
PASS validationRequestHeader('Accept-Language') is initialHeaderValues['Accept-Language']
Modified: trunk/LayoutTests/http/tests/cache/disk-cache/speculative-validation/validation-request.html (252777 => 252778)
--- trunk/LayoutTests/http/tests/cache/disk-cache/speculative-validation/validation-request.html 2019-11-22 17:08:01 UTC (rev 252777)
+++ trunk/LayoutTests/http/tests/cache/disk-cache/speculative-validation/validation-request.html 2019-11-22 17:25:29 UTC (rev 252778)
@@ -37,6 +37,8 @@
if (state == "speculativeRevalidation") {
// Validate the HTTP headers of the speculative validation request.
shouldBeEqualToString("validationRequestHeader('If-None-Match')", "123456789");
+ isCookieHeaderCorrect = validationRequestHeader('Cookie') === document.getElementById("testFrame").contentWindow.sentSetCookieHeader;
+ shouldBeTrue("isCookieHeaderCorrect");
for (var i = 0; i < headersToCheck.length; i++) {
headerToCheck = headersToCheck[i];
Modified: trunk/Source/WebKit/ChangeLog (252777 => 252778)
--- trunk/Source/WebKit/ChangeLog 2019-11-22 17:08:01 UTC (rev 252777)
+++ trunk/Source/WebKit/ChangeLog 2019-11-22 17:25:29 UTC (rev 252778)
@@ -1,3 +1,39 @@
+2019-11-22 Chris Dumez <[email protected]>
+
+ Speculative loading sometimes happens too early and is missing login cookies
+ https://bugs.webkit.org/show_bug.cgi?id=204305
+ <rdar://problem/57063840>
+
+ Reviewed by Antti Koivisto.
+
+ Speculative loads were issued before receiving the response from the main resource. However,
+ the main resource may set important cookies that are thus missing from the speculative requests.
+
+ To address the issue we now delay speculative loads for first-party subresources until we've
+ received the response from the main resource. To avoid regressing PLT, we still warm up the
+ first-party subresources from disk right away and preconnect to the server.
+
+ No new tests, extended existing test.
+
+ * NetworkProcess/NetworkResourceLoader.cpp:
+ (WebKit::NetworkResourceLoader::didReceiveResponse):
+ (WebKit::NetworkResourceLoader::didReceiveMainResourceResponse):
+ (WebKit::NetworkResourceLoader::didRetrieveCacheEntry):
+ * NetworkProcess/NetworkResourceLoader.h:
+ * NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp:
+ (WebKit::NetworkCache::SpeculativeLoadManager::PendingFrameLoad::didReceiveMainResourceResponse const):
+ (WebKit::NetworkCache::SpeculativeLoadManager::PendingFrameLoad::markMainResourceResponseAsReceived):
+ (WebKit::NetworkCache::SpeculativeLoadManager::PendingFrameLoad::addPostMainResourceResponseTask):
+ (WebKit::NetworkCache::SpeculativeLoadManager::shouldRegisterLoad):
+ (WebKit::NetworkCache::SpeculativeLoadManager::registerLoad):
+ (WebKit::NetworkCache::SpeculativeLoadManager::registerMainResourceLoadResponse):
+ (WebKit::NetworkCache::SpeculativeLoadManager::preconnectForSubresource):
+ (WebKit::NetworkCache::SpeculativeLoadManager::revalidateSubresource):
+ * NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.h:
+ * NetworkProcess/cache/NetworkCacheSubresourcesEntry.cpp:
+ (WebKit::NetworkCache::SubresourceInfo::isFirstParty const):
+ * NetworkProcess/cache/NetworkCacheSubresourcesEntry.h:
+
2019-11-22 Carlos Garcia Campos <[email protected]>
[GTK][WPE] RemoteInspector: use sockets instead of DBus
Modified: trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp (252777 => 252778)
--- trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp 2019-11-22 17:08:01 UTC (rev 252777)
+++ trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp 2019-11-22 17:25:29 UTC (rev 252778)
@@ -30,6 +30,7 @@
#include "FormDataReference.h"
#include "Logging.h"
#include "NetworkCache.h"
+#include "NetworkCacheSpeculativeLoadManager.h"
#include "NetworkConnectionToWebProcess.h"
#include "NetworkConnectionToWebProcessMessages.h"
#include "NetworkLoad.h"
@@ -466,6 +467,9 @@
{
RELEASE_LOG_IF_ALLOWED("didReceiveResponse: (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", httpStatusCode = %d, length = %" PRId64 ")", m_parameters.webPageID.toUInt64(), m_parameters.webFrameID.toUInt64(), m_parameters.identifier, receivedResponse.httpStatusCode(), receivedResponse.expectedContentLength());
+ if (isMainResource())
+ didReceiveMainResourceResponse(receivedResponse);
+
m_response = WTFMove(receivedResponse);
if (shouldCaptureExtraNetworkLoadMetrics() && m_networkLoadChecker) {
@@ -900,10 +904,21 @@
});
}
+void NetworkResourceLoader::didReceiveMainResourceResponse(const WebCore::ResourceResponse& response)
+{
+#if ENABLE(NETWORK_CACHE_SPECULATIVE_REVALIDATION)
+ if (auto* speculativeLoadManager = m_cache ? m_cache->speculativeLoadManager() : nullptr)
+ speculativeLoadManager->registerMainResourceLoadResponse(globalFrameID(), originalRequest(), response);
+#endif
+}
+
void NetworkResourceLoader::didRetrieveCacheEntry(std::unique_ptr<NetworkCache::Entry> entry)
{
auto response = entry->response();
+ if (isMainResource())
+ didReceiveMainResourceResponse(response);
+
if (isMainResource() && shouldInterruptLoadForCSPFrameAncestorsOrXFrameOptions(response)) {
response = sanitizeResponseIfPossible(WTFMove(response), ResourceResponse::SanitizationType::CrossOriginSafe);
send(Messages::WebResourceLoader::StopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied { response });
Modified: trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.h (252777 => 252778)
--- trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.h 2019-11-22 17:08:01 UTC (rev 252777)
+++ trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.h 2019-11-22 17:25:29 UTC (rev 252778)
@@ -155,6 +155,7 @@
void startNetworkLoad(WebCore::ResourceRequest&&, FirstLoad);
void restartNetworkLoad(WebCore::ResourceRequest&&);
void continueDidReceiveResponse();
+ void didReceiveMainResourceResponse(const WebCore::ResourceResponse&);
enum class LoadResult {
Unknown,
Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp (252777 => 252778)
--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp 2019-11-22 17:08:01 UTC (rev 252777)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp 2019-11-22 17:25:29 UTC (rev 252778)
@@ -33,6 +33,7 @@
#include "NetworkCacheSpeculativeLoad.h"
#include "NetworkCacheSubresourcesEntry.h"
#include "NetworkProcess.h"
+#include "PreconnectTask.h"
#include <WebCore/DiagnosticLoggingKeys.h>
#include <pal/HysteresisActivity.h>
#include <wtf/HashCountedSet.h>
@@ -203,6 +204,16 @@
saveToDiskIfReady();
}
+ bool didReceiveMainResourceResponse() const { return m_didReceiveMainResourceResponse; }
+ void markMainResourceResponseAsReceived()
+ {
+ m_didReceiveMainResourceResponse = true;
+ for (auto& task : m_postMainResourceResponseTasks)
+ task();
+ }
+
+ void addPostMainResourceResponseTask(Function<void()>&& task) { m_postMainResourceResponseTasks.append(WTFMove(task)); }
+
private:
PendingFrameLoad(Storage& storage, const Key& mainResourceKey, WTF::Function<void()>&& loadCompletionHandler)
: m_storage(storage)
@@ -242,8 +253,10 @@
WTF::Function<void()> m_loadCompletionHandler;
PAL::HysteresisActivity m_loadHysteresisActivity;
std::unique_ptr<SubresourcesEntry> m_existingEntry;
+ Vector<Function<void()>> m_postMainResourceResponseTasks;
bool m_didFinishLoad { false };
bool m_didRetrieveExistingEntry { false };
+ bool m_didReceiveMainResourceResponse { false };
};
SpeculativeLoadManager::SpeculativeLoadManager(Cache& cache, Storage& storage)
@@ -323,15 +336,22 @@
addResult.iterator->value->append(WTFMove(completionHandler));
}
+bool SpeculativeLoadManager::shouldRegisterLoad(const WebCore::ResourceRequest& request)
+{
+ if (request.httpMethod() != "GET")
+ return false;
+ if (!request.httpHeaderField(HTTPHeaderName::Range).isEmpty())
+ return false;
+ return true;
+}
+
void SpeculativeLoadManager::registerLoad(const GlobalFrameID& frameID, const ResourceRequest& request, const Key& resourceKey)
{
ASSERT(RunLoop::isMain());
ASSERT(request.url().protocolIsInHTTPFamily());
- if (request.httpMethod() != "GET")
+ if (!shouldRegisterLoad(request))
return;
- if (!request.httpHeaderField(HTTPHeaderName::Range).isEmpty())
- return;
auto isMainResource = request.requester() == ResourceRequest::Requester::Main;
if (isMainResource) {
@@ -362,6 +382,18 @@
pendingFrameLoad->registerSubresourceLoad(request, resourceKey);
}
+void SpeculativeLoadManager::registerMainResourceLoadResponse(const GlobalFrameID& frameID, const WebCore::ResourceRequest& request, const WebCore::ResourceResponse& response)
+{
+ if (!shouldRegisterLoad(request))
+ return;
+
+ if (response.isRedirection())
+ return;
+
+ if (auto* pendingFrameLoad = m_pendingFrameLoads.get(frameID))
+ pendingFrameLoad->markMainResourceResponseAsReceived();
+}
+
void SpeculativeLoadManager::addPreloadedEntry(std::unique_ptr<Entry> entry, const GlobalFrameID& frameID, Optional<ResourceRequest>&& revalidationRequest)
{
ASSERT(entry);
@@ -417,6 +449,26 @@
return true;
}
+void SpeculativeLoadManager::preconnectForSubresource(const SubresourceInfo& subresourceInfo, Entry* entry, const GlobalFrameID& frameID)
+{
+#if ENABLE(SERVER_PRECONNECT)
+ NetworkLoadParameters parameters;
+ parameters.webPageProxyID = frameID.webPageProxyID;
+ parameters.webPageID = frameID.webPageID;
+ parameters.webFrameID = frameID.frameID;
+ parameters.storedCredentialsPolicy = StoredCredentialsPolicy::Use;
+ parameters.contentSniffingPolicy = ContentSniffingPolicy::DoNotSniffContent;
+ parameters.contentEncodingSniffingPolicy = ContentEncodingSniffingPolicy::Sniff;
+ parameters.shouldPreconnectOnly = PreconnectOnly::Yes;
+ parameters.request = constructRevalidationRequest(subresourceInfo.key(), subresourceInfo, entry);
+ new PreconnectTask(m_cache.networkProcess(), m_cache.sessionID(), WTFMove(parameters), [](const WebCore::ResourceError&) { });
+#else
+ UNUSED_PARAM(subresourceInfo);
+ UNUSED_PARAM(entry);
+ UNUSED_PARAM(frameID);
+#endif
+}
+
void SpeculativeLoadManager::revalidateSubresource(const SubresourceInfo& subresourceInfo, std::unique_ptr<Entry> entry, const GlobalFrameID& frameID)
{
ASSERT(!entry || entry->needsValidation());
@@ -427,6 +479,18 @@
if (!key.range().isEmpty())
return;
+ auto* pendingLoad = m_pendingFrameLoads.get(frameID);
+
+ // Delay first-party speculative loads until we've received the response for the main resource, in case the main resource
+ // response sets cookies that are needed for subsequent loads.
+ if (pendingLoad && !pendingLoad->didReceiveMainResourceResponse() && subresourceInfo.isFirstParty()) {
+ preconnectForSubresource(subresourceInfo, entry.get(), frameID);
+ pendingLoad->addPostMainResourceResponseTask([this, subresourceInfo, entry = WTFMove(entry), frameID]() mutable {
+ revalidateSubresource(subresourceInfo, WTFMove(entry), frameID);
+ });
+ return;
+ }
+
ResourceRequest revalidationRequest = constructRevalidationRequest(key, subresourceInfo, entry.get());
LOG(NetworkCacheSpeculativePreloading, "(NetworkProcess) Speculatively revalidating '%s':", key.identifier().utf8().data());
Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.h (252777 => 252778)
--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.h 2019-11-22 17:08:01 UTC (rev 252777)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.h 2019-11-22 17:25:29 UTC (rev 252778)
@@ -50,6 +50,7 @@
~SpeculativeLoadManager();
void registerLoad(const GlobalFrameID&, const WebCore::ResourceRequest&, const Key& resourceKey);
+ void registerMainResourceLoadResponse(const GlobalFrameID&, const WebCore::ResourceRequest&, const WebCore::ResourceResponse&);
typedef Function<void (std::unique_ptr<Entry>)> RetrieveCompletionHandler;
@@ -59,10 +60,12 @@
private:
class PreloadedEntry;
+ static bool shouldRegisterLoad(const WebCore::ResourceRequest&);
void addPreloadedEntry(std::unique_ptr<Entry>, const GlobalFrameID&, Optional<WebCore::ResourceRequest>&& revalidationRequest = WTF::nullopt);
void preloadEntry(const Key&, const SubresourceInfo&, const GlobalFrameID&);
void retrieveEntryFromStorage(const SubresourceInfo&, RetrieveCompletionHandler&&);
void revalidateSubresource(const SubresourceInfo&, std::unique_ptr<Entry>, const GlobalFrameID&);
+ void preconnectForSubresource(const SubresourceInfo&, Entry*, const GlobalFrameID&);
bool satisfyPendingRequests(const Key&, Entry*);
void retrieveSubresourcesEntry(const Key& storageKey, WTF::Function<void (std::unique_ptr<SubresourcesEntry>)>&&);
void startSpeculativeRevalidation(const GlobalFrameID&, SubresourcesEntry&);
Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSubresourcesEntry.cpp (252777 => 252778)
--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSubresourcesEntry.cpp 2019-11-22 17:08:01 UTC (rev 252777)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSubresourcesEntry.cpp 2019-11-22 17:25:29 UTC (rev 252778)
@@ -81,6 +81,12 @@
return true;
}
+bool SubresourceInfo::isFirstParty() const
+{
+ RegistrableDomain firstPartyDomain { m_firstPartyForCookies };
+ return firstPartyDomain.matches(URL(URL(), key().identifier()));
+}
+
Storage::Record SubresourcesEntry::encodeAsStorageRecord() const
{
WTF::Persistence::Encoder encoder;
Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSubresourcesEntry.h (252777 => 252778)
--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSubresourcesEntry.h 2019-11-22 17:08:01 UTC (rev 252777)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSubresourcesEntry.h 2019-11-22 17:25:29 UTC (rev 252778)
@@ -58,6 +58,8 @@
void setNonTransient() { m_isTransient = false; }
+ bool isFirstParty() const;
+
private:
Key m_key;
WallTime m_lastSeen;
