Attachment 371167 Details for Bug 198469 – Patch

[patch] Patch

bug-198469-20190602155928.patch (text/plain), 12.23 KB, created by Andy Estes on 2019-06-02 15:59:29 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Andy Estes

Created: 2019-06-02 15:59:29 PDT

Size: 12.23 KB

patch

obsolete

>Subversion Revision: 246013
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 2458cfb74339378bebf54199c9dd048de52a7fdf..0f9240927287eee48898bca6eb03da86a715f670 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2019-06-02  Andy Estes  <aestes@apple.com>
>+
>+        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 NOBODY (OOPS!).
>+
>+        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-05-31  Youenn Fablet  <youenn@apple.com>
> 
>         Add an option to mute audio capture automatically when page is not visible
>diff --git a/Source/WebCore/loader/DocumentLoader.cpp b/Source/WebCore/loader/DocumentLoader.cpp
>index d1d98138d0e7f3b97a04749299edb5c0e5540fcd..7ddcbc0ba006a0ec56dc942ca4b62397c2972e62 100644
>--- a/Source/WebCore/loader/DocumentLoader.cpp
>+++ b/Source/WebCore/loader/DocumentLoader.cpp
>@@ -942,9 +942,16 @@ void DocumentLoader::continueAfterContentPolicy(PolicyAction policy)
>         } 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.
>+        ASSERT(m_mainResource);
>+        stopLoadingForPolicyChange();
>         return;
>     }
>     case PolicyAction::StopAllLoads:
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 288349033ceec728bdcd2b28d92272549e0c674c..10a76d26fb99546628e6f23e3e09727e99ca70e5 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,24 @@
>+2019-06-02  Andy Estes  <aestes@apple.com>
>+
>+        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 NOBODY (OOPS!).
>+
>+        * 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-05-31  Sihui Liu  <sihui_liu@apple.com>
> 
>         TestWebKitAPI.WKWebView.LocalStorageProcessSuspends is flaky
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/Download.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/Download.mm
>index c6c553716ac1300bd604f342756be00dbe763b04..fda6336ddd5078ef4ea22d15daa5bd4eee408851 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/Download.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/Download.mm
>@@ -914,4 +914,99 @@ TEST(_WKDownload, DownloadMonitorReturnToForeground)
> 
> } // 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
>+{
>+    ASSERT(!_didFinishNavigation);
>+    _didFailProvisionalNavigation = YES;
>+    if (_responsePolicy != _WKNavigationResponsePolicyBecomeDownload || _didStartDownload)
>+        isDone = true;
>+}
>+
>+- (void)webView:(WKWebView *)webView didFinishNavigation:(WKNavigation *)navigation
>+{
>+    ASSERT(!_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"];
>+    [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);
>+
>+    [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 unregister];
>+}
>+
> #endif // PLATFORM(MAC) || PLATFORM(IOS)
>diff --git a/Tools/TestWebKitAPI/cocoa/TestProtocol.h b/Tools/TestWebKitAPI/cocoa/TestProtocol.h
>index c88b2dd861180b705d2a632fdaac98f4f333bd0a..299c0d5b9c2dabab0c61488cf94a3a0fcf667be7 100644
>--- a/Tools/TestWebKitAPI/cocoa/TestProtocol.h
>+++ b/Tools/TestWebKitAPI/cocoa/TestProtocol.h
>@@ -31,6 +31,7 @@
> + (void)registerWithScheme:(NSString *)scheme;
> + (void)unregister;
> + (NSString *)scheme;
>+@property (nonatomic, class, copy) NSDictionary<NSString *, NSString *> *additionalResponseHeaders;
> @end
> 
> #endif // TestProtocol_h
>diff --git a/Tools/TestWebKitAPI/cocoa/TestProtocol.mm b/Tools/TestWebKitAPI/cocoa/TestProtocol.mm
>index d949c6fe5fa541fc75d3c778a67bdc5a591fc885..607a75309a4883cbbac75fc73816af9797ac69c3 100644
>--- a/Tools/TestWebKitAPI/cocoa/TestProtocol.mm
>+++ b/Tools/TestWebKitAPI/cocoa/TestProtocol.mm
>@@ -68,6 +68,23 @@ + (void)unregister
>     testScheme = nil;
> }
> 
>+static NSDictionary<NSString *, NSString *> *& additionalResponseHeaders()
>+{
>+    static NSDictionary<NSString *, NSString *> *additionalResponseHeaders;
>+    return 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,18 +103,26 @@ - (void)startLoading
>         return;
>     }
> 
>+    if ([requestURL.host isEqualToString:@"redirect"]) {
>+        NSData *data = [@"PASS" 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 = [NSData dataWithContentsOfURL:[NSBundle.mainBundle URLForResource:requestURL.lastPathComponent.stringByDeletingPathExtension withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
>     else
>         data = [@"PASS" 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 (NSDictionary *additionalHeaders = additionalResponseHeaders())
>+        [responseHeaders addEntriesFromDictionary:additionalHeaders];
> 
>-    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];

Actions: View | Formatted Diff | Diff

Attachments on bug 198469: 371167 | 371171 | 371174 | 371197 | 371202