Attachment 371197 Details for Bug 198469 – Patch

[patch] Patch

bug-198469-20190603110916.patch (text/plain), 13.04 KB, created by Andy Estes on 2019-06-03 11:09:17 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Andy Estes

Created: 2019-06-03 11:09:17 PDT

Size: 13.04 KB

patch

obsolete

>Subversion Revision: 246013
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 2458cfb74339378bebf54199c9dd048de52a7fdf..45eea2b57d06d93fcd22c2e38d25b50e4b9da321 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 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-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..719f1e8aafff909d0106b7ca9582648d89a8ab60 100644
>--- a/Source/WebCore/loader/DocumentLoader.cpp
>+++ b/Source/WebCore/loader/DocumentLoader.cpp
>@@ -942,9 +942,15 @@ 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.
>+        stopLoadingForPolicyChange();
>         return;
>     }
>     case PolicyAction::StopAllLoads:
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 288349033ceec728bdcd2b28d92272549e0c674c..bdf0bdc89f76a69ce5c4da4f708f8948a8d9a9b4 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 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-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..2e03230aaac0142c2c13206e92dc62e260da2b23 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/Download.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/Download.mm
>@@ -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 @@ 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
>+{
>+    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)
>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..6fb7eafe88e31351b1188a3f5ea27b180d8ff627 100644
>--- a/Tools/TestWebKitAPI/cocoa/TestProtocol.mm
>+++ b/Tools/TestWebKitAPI/cocoa/TestProtocol.mm
>@@ -68,6 +68,19 @@ + (void)unregister
>     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,18 +99,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 (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];

Flags:

ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

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