- Revision
- 246043
- Author
- [email protected]
- Date
- 2019-06-03 12:39:03 -0700 (Mon, 03 Jun 2019)
Log Message
Memory-cached main resources continue to load after the client decides a content policy of PolicyAction::Download
https://bugs.webkit.org/show_bug.cgi?id=198469
<rdar://problem/50512713>
Reviewed by Youenn Fablet.
Source/WebCore:
When a document is loaded from the memory cache it does not have a main resource loader, but
DocumentLoader::continueAfterContentPolicy relies on being able to call
ResourceLoader::didFail on the main resource loader to cancel the provisional navigation
when the client decides a content policy of PolicyAction::Download.
This means that memory-cached main resources continue to load even after WebKit has started
to download the main resource. The expected behavior is for the provisional navigation to
fail once the download starts, like what happens when there is a main resource loader.
This patch teaches DocumentLoader::continueAfterContentPolicy to call
stopLoadingForPolicyChange() in the case of a null main resource loader. This will dispatch
didFailProvisionalNavigation and remove the DocumentLoader as a client of its
CachedRawResource to prevent it from delivering any cached data.
Added a new API test.
* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::continueAfterContentPolicy):
Tools:
* TestWebKitAPI/Tests/WebKitCocoa/Download.mm:
(-[TestDownloadNavigationResponseFromMemoryCacheDelegate webView:didStartProvisionalNavigation:]):
(-[TestDownloadNavigationResponseFromMemoryCacheDelegate webView:didFailProvisionalNavigation:withError:]):
(-[TestDownloadNavigationResponseFromMemoryCacheDelegate webView:didFinishNavigation:]):
(-[TestDownloadNavigationResponseFromMemoryCacheDelegate _downloadDidStart:]):
(-[TestDownloadNavigationResponseFromMemoryCacheDelegate webView:decidePolicyForNavigationResponse:decisionHandler:]):
(TEST):
* TestWebKitAPI/cocoa/TestProtocol.h:
* TestWebKitAPI/cocoa/TestProtocol.mm:
(+[TestProtocol additionalResponseHeaders]):
(+[TestProtocol setAdditionalResponseHeaders:]):
(-[TestProtocol startLoading]):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (246042 => 246043)
--- trunk/Source/WebCore/ChangeLog 2019-06-03 19:14:42 UTC (rev 246042)
+++ trunk/Source/WebCore/ChangeLog 2019-06-03 19:39:03 UTC (rev 246043)
@@ -1,3 +1,30 @@
+2019-06-02 Andy Estes <[email protected]>
+
+ Memory-cached main resources continue to load after the client decides a content policy of PolicyAction::Download
+ https://bugs.webkit.org/show_bug.cgi?id=198469
+ <rdar://problem/50512713>
+
+ Reviewed by Youenn Fablet.
+
+ When a document is loaded from the memory cache it does not have a main resource loader, but
+ DocumentLoader::continueAfterContentPolicy relies on being able to call
+ ResourceLoader::didFail on the main resource loader to cancel the provisional navigation
+ when the client decides a content policy of PolicyAction::Download.
+
+ This means that memory-cached main resources continue to load even after WebKit has started
+ to download the main resource. The expected behavior is for the provisional navigation to
+ fail once the download starts, like what happens when there is a main resource loader.
+
+ This patch teaches DocumentLoader::continueAfterContentPolicy to call
+ stopLoadingForPolicyChange() in the case of a null main resource loader. This will dispatch
+ didFailProvisionalNavigation and remove the DocumentLoader as a client of its
+ CachedRawResource to prevent it from delivering any cached data.
+
+ Added a new API test.
+
+ * loader/DocumentLoader.cpp:
+ (WebCore::DocumentLoader::continueAfterContentPolicy):
+
2019-06-03 Timothy Hatcher <[email protected]>
Tweak the text and underline color for data detected text.
Modified: trunk/Source/WebCore/loader/DocumentLoader.cpp (246042 => 246043)
--- trunk/Source/WebCore/loader/DocumentLoader.cpp 2019-06-03 19:14:42 UTC (rev 246042)
+++ trunk/Source/WebCore/loader/DocumentLoader.cpp 2019-06-03 19:39:03 UTC (rev 246043)
@@ -942,9 +942,15 @@
} else
frameLoader()->client().convertMainResourceLoadToDownload(this, sessionID, m_request, m_response);
- // It might have gone missing
- if (mainResourceLoader())
+ // The main resource might be loading from the memory cache, or its loader might have gone missing.
+ if (mainResourceLoader()) {
static_cast<ResourceLoader*>(mainResourceLoader())->didFail(interruptedForPolicyChangeError());
+ return;
+ }
+
+ // We must stop loading even if there is no main resource loader. Otherwise, we might remain
+ // the client of a CachedRawResource that will continue to send us data.
+ stopLoadingForPolicyChange();
return;
}
case PolicyAction::StopAllLoads:
Modified: trunk/Tools/ChangeLog (246042 => 246043)
--- trunk/Tools/ChangeLog 2019-06-03 19:14:42 UTC (rev 246042)
+++ trunk/Tools/ChangeLog 2019-06-03 19:39:03 UTC (rev 246043)
@@ -1,3 +1,24 @@
+2019-06-02 Andy Estes <[email protected]>
+
+ Memory-cached main resources continue to load after the client decides a content policy of PolicyAction::Download
+ https://bugs.webkit.org/show_bug.cgi?id=198469
+ <rdar://problem/50512713>
+
+ Reviewed by Youenn Fablet.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/Download.mm:
+ (-[TestDownloadNavigationResponseFromMemoryCacheDelegate webView:didStartProvisionalNavigation:]):
+ (-[TestDownloadNavigationResponseFromMemoryCacheDelegate webView:didFailProvisionalNavigation:withError:]):
+ (-[TestDownloadNavigationResponseFromMemoryCacheDelegate webView:didFinishNavigation:]):
+ (-[TestDownloadNavigationResponseFromMemoryCacheDelegate _downloadDidStart:]):
+ (-[TestDownloadNavigationResponseFromMemoryCacheDelegate webView:decidePolicyForNavigationResponse:decisionHandler:]):
+ (TEST):
+ * TestWebKitAPI/cocoa/TestProtocol.h:
+ * TestWebKitAPI/cocoa/TestProtocol.mm:
+ (+[TestProtocol additionalResponseHeaders]):
+ (+[TestProtocol setAdditionalResponseHeaders:]):
+ (-[TestProtocol startLoading]):
+
2019-06-03 Timothy Hatcher <[email protected]>
Tweak the text and underline color for data detected text.
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Download.mm (246042 => 246043)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Download.mm 2019-06-03 19:14:42 UTC (rev 246042)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Download.mm 2019-06-03 19:39:03 UTC (rev 246043)
@@ -33,6 +33,7 @@
#import "Test.h"
#import "TestProtocol.h"
#import "TestWKWebView.h"
+#import <WebKit/WKErrorPrivate.h>
#import <WebKit/WKNavigationDelegatePrivate.h>
#import <WebKit/WKProcessPoolPrivate.h>
#import <WebKit/WKUIDelegatePrivate.h>
@@ -914,4 +915,113 @@
} // namespace TestWebKitAPI
+@interface TestDownloadNavigationResponseFromMemoryCacheDelegate : NSObject <WKNavigationDelegate, _WKDownloadDelegate>
+@property (nonatomic) WKNavigationResponsePolicy responsePolicy;
+@property (nonatomic, readonly) BOOL didFailProvisionalNavigation;
+@property (nonatomic, readonly) BOOL didFinishNavigation;
+@property (nonatomic, readonly) BOOL didStartDownload;
+@end
+
+@implementation TestDownloadNavigationResponseFromMemoryCacheDelegate
+
+- (void)webView:(WKWebView *)webView didStartProvisionalNavigation:(WKNavigation *)navigation
+{
+ _didFailProvisionalNavigation = NO;
+ _didFinishNavigation = NO;
+ _didStartDownload = NO;
+}
+
+- (void)webView:(WKWebView *)webView didFailProvisionalNavigation:(WKNavigation *)navigation withError:(NSError *)error
+{
+ EXPECT_EQ(_WKErrorCodeFrameLoadInterruptedByPolicyChange, error.code);
+ EXPECT_FALSE(_didFinishNavigation);
+ EXPECT_WK_STREQ(_WKLegacyErrorDomain, error.domain);
+ _didFailProvisionalNavigation = YES;
+ if (_responsePolicy != _WKNavigationResponsePolicyBecomeDownload || _didStartDownload)
+ isDone = true;
+}
+
+- (void)webView:(WKWebView *)webView didFinishNavigation:(WKNavigation *)navigation
+{
+ EXPECT_FALSE(_didFailProvisionalNavigation);
+ _didFinishNavigation = YES;
+ if (_responsePolicy != _WKNavigationResponsePolicyBecomeDownload || _didStartDownload)
+ isDone = true;
+}
+
+- (void)_downloadDidStart:(_WKDownload *)download
+{
+ _didStartDownload = YES;
+ if (_didFailProvisionalNavigation || _didFinishNavigation)
+ isDone = true;
+}
+
+- (void)webView:(WKWebView *)webView decidePolicyForNavigationResponse:(WKNavigationResponse *)navigationResponse decisionHandler:(void (^)(WKNavigationResponsePolicy))decisionHandler
+{
+ decisionHandler(_responsePolicy);
+}
+
+@end
+
+TEST(WebKit, DownloadNavigationResponseFromMemoryCache)
+{
+ [TestProtocol registerWithScheme:@"http"];
+ TestProtocol.additionalResponseHeaders = @{ @"Cache-Control" : @"max-age=3600" };
+
+ auto delegate = adoptNS([[TestDownloadNavigationResponseFromMemoryCacheDelegate alloc] init]);
+ auto webView = adoptNS([[WKWebView alloc] init]);
+ [webView setNavigationDelegate:delegate.get()];
+ [webView configuration].processPool._downloadDelegate = delegate.get();
+
+ NSURL *firstURL = [NSURL URLWithString:@"http://bundle-html-file/simple"];
+ [delegate setResponsePolicy:WKNavigationResponsePolicyAllow];
+ [webView loadRequest:[NSURLRequest requestWithURL:firstURL]];
+ isDone = false;
+ TestWebKitAPI::Util::run(&isDone);
+ EXPECT_FALSE([delegate didFailProvisionalNavigation]);
+ EXPECT_FALSE([delegate didStartDownload]);
+ EXPECT_TRUE([delegate didFinishNavigation]);
+ EXPECT_WK_STREQ(firstURL.absoluteString, [webView URL].absoluteString);
+
+ NSURL *secondURL = [NSURL URLWithString:@"http://bundle-html-file/simple2"];
+ [delegate setResponsePolicy:_WKNavigationResponsePolicyBecomeDownload];
+ [webView loadRequest:[NSURLRequest requestWithURL:secondURL]];
+ isDone = false;
+ TestWebKitAPI::Util::run(&isDone);
+ EXPECT_FALSE([delegate didFinishNavigation]);
+ EXPECT_TRUE([delegate didFailProvisionalNavigation]);
+ EXPECT_TRUE([delegate didStartDownload]);
+ EXPECT_WK_STREQ(firstURL.absoluteString, [webView URL].absoluteString);
+
+ [delegate setResponsePolicy:WKNavigationResponsePolicyAllow];
+ [webView loadRequest:[NSURLRequest requestWithURL:secondURL]];
+ isDone = false;
+ TestWebKitAPI::Util::run(&isDone);
+ EXPECT_FALSE([delegate didFailProvisionalNavigation]);
+ EXPECT_FALSE([delegate didStartDownload]);
+ EXPECT_TRUE([delegate didFinishNavigation]);
+ EXPECT_WK_STREQ(secondURL.absoluteString, [webView URL].absoluteString);
+
+ [delegate setResponsePolicy:WKNavigationResponsePolicyAllow];
+ [webView goBack];
+ isDone = false;
+ TestWebKitAPI::Util::run(&isDone);
+ EXPECT_FALSE([delegate didFailProvisionalNavigation]);
+ EXPECT_FALSE([delegate didStartDownload]);
+ EXPECT_TRUE([delegate didFinishNavigation]);
+ EXPECT_WK_STREQ(firstURL.absoluteString, [webView URL].absoluteString);
+
+ [delegate setResponsePolicy:_WKNavigationResponsePolicyBecomeDownload];
+ [webView loadRequest:[NSURLRequest requestWithURL:secondURL]];
+ isDone = false;
+ TestWebKitAPI::Util::run(&isDone);
+ EXPECT_FALSE([delegate didFinishNavigation]);
+ EXPECT_TRUE([delegate didFailProvisionalNavigation]);
+ EXPECT_TRUE([delegate didStartDownload]);
+ EXPECT_WK_STREQ(firstURL.absoluteString, [webView URL].absoluteString);
+
+ TestProtocol.additionalResponseHeaders = nil;
+ [TestProtocol unregister];
+}
+
#endif // PLATFORM(MAC) || PLATFORM(IOS)
Modified: trunk/Tools/TestWebKitAPI/cocoa/TestProtocol.h (246042 => 246043)
--- trunk/Tools/TestWebKitAPI/cocoa/TestProtocol.h 2019-06-03 19:14:42 UTC (rev 246042)
+++ trunk/Tools/TestWebKitAPI/cocoa/TestProtocol.h 2019-06-03 19:39:03 UTC (rev 246043)
@@ -31,6 +31,7 @@
+ (void)registerWithScheme:(NSString *)scheme;
+ (void)unregister;
+ (NSString *)scheme;
+@property (nonatomic, class, copy) NSDictionary<NSString *, NSString *> *additionalResponseHeaders;
@end
#endif // TestProtocol_h
Modified: trunk/Tools/TestWebKitAPI/cocoa/TestProtocol.mm (246042 => 246043)
--- trunk/Tools/TestWebKitAPI/cocoa/TestProtocol.mm 2019-06-03 19:14:42 UTC (rev 246042)
+++ trunk/Tools/TestWebKitAPI/cocoa/TestProtocol.mm 2019-06-03 19:39:03 UTC (rev 246043)
@@ -68,6 +68,19 @@
testScheme = nil;
}
+static NSDictionary<NSString *, NSString *> *additionalResponseHeaders;
+
++ (NSDictionary<NSString *, NSString *> *)additionalResponseHeaders
+{
+ return additionalResponseHeaders;
+}
+
++ (void)setAdditionalResponseHeaders:(NSDictionary<NSString *, NSString *> *)additionalHeaders
+{
+ [additionalResponseHeaders autorelease];
+ additionalResponseHeaders = [additionalHeaders copy];
+}
+
static NSURL *createRedirectURL(NSString *query)
{
return [NSURL URLWithString:[NSString stringWithFormat:@"%@://%@", testScheme, query]];
@@ -86,6 +99,13 @@
return;
}
+ if ([requestURL.host isEqualToString:@"redirect"]) {
+ NSData *data = "" dataUsingEncoding:NSASCIIStringEncoding];
+ auto response = adoptNS([[NSURLResponse alloc] initWithURL:requestURL MIMEType:@"text/html" expectedContentLength:data.length textEncodingName:nil]);
+ [self.client URLProtocol:self wasRedirectedToRequest:[NSURLRequest requestWithURL:createRedirectURL(requestURL.query)] redirectResponse:response.get()];
+ return;
+ }
+
NSData *data;
if ([requestURL.host isEqualToString:@"bundle-html-file"])
data = "" dataWithContentsOfURL:[NSBundle.mainBundle URLForResource:requestURL.lastPathComponent.stringByDeletingPathExtension withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
@@ -92,12 +112,13 @@
else
data = "" dataUsingEncoding:NSASCIIStringEncoding];
- RetainPtr<NSURLResponse> response = adoptNS([[NSURLResponse alloc] initWithURL:requestURL MIMEType:@"text/html" expectedContentLength:data.length textEncodingName:nil]);
+ NSMutableDictionary *responseHeaders = [NSMutableDictionary dictionaryWithCapacity:2];
+ responseHeaders[@"Content-Type"] = @"text/html";
+ responseHeaders[@"Content-Length"] = [NSString stringWithFormat:@"%tu", data.length];
+ if (additionalResponseHeaders)
+ [responseHeaders addEntriesFromDictionary:additionalResponseHeaders];
- if ([requestURL.host isEqualToString:@"redirect"]) {
- [self.client URLProtocol:self wasRedirectedToRequest:[NSURLRequest requestWithURL:createRedirectURL(requestURL.query)] redirectResponse:response.get()];
- return;
- }
+ auto response = adoptNS([[NSHTTPURLResponse alloc] initWithURL:requestURL statusCode:200 HTTPVersion:@"HTTP/1.1" headerFields:responseHeaders]);
[self.client URLProtocol:self didReceiveResponse:response.get() cacheStoragePolicy:NSURLCacheStorageNotAllowed];
[self.client URLProtocol:self didLoadData:data];
