We should not speculatively revalidate cached redirects
https://bugs.webkit.org/show_bug.cgi?id=156548
<rdar://problem/25583886>
Reviewed by Darin Adler.
Source/WebKit2:
Stop speculatively revalidating cached redirects. This matches matches
the behavior in NetworkCache's makeUseDecision() which reuses cached
redirects only if they do not need revalidation.
This was breaking fonts.css loading on stripe.com because the
SpeculativeLoadManager would wrongly speculatively revalidate the
redirect and then serve a 302 response the NetworkResourceLoader
when the actual request came in. This would cause us to not follow
the redirect.
* NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp:
(WebKit::NetworkCache::SpeculativeLoad::willSendRedirectedRequest):
Abort the speculative load if it hits a redirect. This is the safe thing
to do in this case, as we are supposed to do a hand-shake with WebCore
in such case.
(WebKit::NetworkCache::SpeculativeLoad::didReceiveResponse):
Let successful validations fall through instead of calling didComplete()
early. This matches what is not in NetworkResourceLoader. This way,
didFinishLoading() ends up getting called for both successful and
unsuccessful (i.e. did not return a 302 status code) network validation.
(WebKit::NetworkCache::SpeculativeLoad::didFinishLoading):
- Stop dealing with redirects as we abort the load as soon as we hit a
redirect now.
- Stop asserting that m_cacheEntryForValidation is null now that this
is called for successful validations as well.
(WebKit::NetworkCache::SpeculativeLoad::abort):
New method that aborts the network loads, calls the completion handler
and clean up. It is called in the case we hit a redirect while
revalidating.
* NetworkProcess/cache/NetworkCacheSpeculativeLoad.h:
Drop m_redirectChainCacheStatus member as we no longer deal with
redirects.
* NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp:
(WebKit::NetworkCache::SpeculativeLoadManager::retrieveEntryFromStorage):
If the resource needs revalidation AND is a cached redirect, then do not
use it. This matches what is done in NetworkCache's makeUseDecision().
Tools:
Re-enable speculative loading in the context of layout tests. This was
turned off by mistake when speculative loading was turned into a
setting recently.
* WebKitTestRunner/TestController.cpp:
(WTR::TestController::generatePageConfiguration):
LayoutTests:
Add layout test to make sure that speculative loading does not break
redirects. This replicates the issue seen with fonts.css on stripe.com.
* http/tests/cache/disk-cache/speculative-validation/cacheable-redirect-expected.txt: Added.
* http/tests/cache/disk-cache/speculative-validation/cacheable-redirect.html: Added.
* http/tests/cache/disk-cache/speculative-validation/resources/cacheable-redirect-frame.php: Added.
* http/tests/cache/disk-cache/speculative-validation/resources/css-to-revalidate.php: Added.
* http/tests/cache/disk-cache/speculative-validation/resources/redirect-to-css.php: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@199521 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp b/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp
index 8f93fa3..9c2ee1a 100644
--- a/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp
+++ b/Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp
@@ -425,8 +425,14 @@
return true;
}
- if (responseNeedsRevalidation(response, entry->timeStamp()))
+ if (responseNeedsRevalidation(response, entry->timeStamp())) {
+ // Do not use cached redirects that have expired.
+ if (entry->redirectRequest()) {
+ completionHandler(nullptr);
+ return true;
+ }
entry->setNeedsValidation(true);
+ }
completionHandler(WTFMove(entry));
return true;
