Attachment 359529 Details for Bug 193588 – Patch

[patch] Patch

bug-193588-20190118125240.patch (text/plain), 9.10 KB, created by Chris Dumez on 2019-01-18 12:52:42 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-01-18 12:52:42 PST

Size: 9.10 KB

patch

obsolete

>Subversion Revision: 240161
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 04862d5776b4a7121ef54357aee32c8fe5d7fc2e..e63a600a540a69376f66d1965fbdd187f49275e5 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,28 @@
>+2019-01-18  Chris Dumez  <cdumez@apple.com>
>+
>+        Regression(PSON) Content blockers are sometimes lost on back navigation cross-site
>+        https://bugs.webkit.org/show_bug.cgi?id=193588
>+        <rdar://problem/47131566>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When the WebPageProxy needs to create initialization parameters for its WebPage in the
>+        WebContent process, it calls WebProcessProxy::addWebUserContentControllerProxy()
>+        which calls WebUserContentControllerProxy::addProcess(). This last call is supposed to
>+        register the WebProcessProxy with the WebUserContentControllerProxy and adding the
>+        contentRuleLists to the WebPageCreationParameters. The issue is that if the
>+        WebUserContentControllerProxy already knows about this WebProcessProxy, it would return
>+        early and not populate the WebPageCreationParameters.
>+
>+        In PSON world, when navigating back to a page that failed to enter page cache, we reuse
>+        the process where we previously loaded the page but re-create a new WebPage on the
>+        WebContent process site. When this happens, WebUserContentControllerProxy would not
>+        add the contentRuleLists to the WebPageCreationParameters and the new WebPage in the
>+        previously-suspended process would be missing them.
>+
>+        * UIProcess/UserContent/WebUserContentControllerProxy.cpp:
>+        (WebKit::WebUserContentControllerProxy::addProcess):
>+
> 2019-01-18  Chris Dumez  <cdumez@apple.com>
> 
>         Regression(PSON) Scroll position is not always restored properly when navigating back
>diff --git a/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.cpp b/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.cpp
>index d46b19bc4f85d0f44b81c4885454dcf218b24609..751ace51aa976dbeb3f13256d50f86780df73353 100644
>--- a/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.cpp
>+++ b/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.cpp
>@@ -84,10 +84,8 @@ WebUserContentControllerProxy::~WebUserContentControllerProxy()
> 
> void WebUserContentControllerProxy::addProcess(WebProcessProxy& webProcessProxy, WebPageCreationParameters& parameters)
> {
>-    if (!m_processes.add(&webProcessProxy).isNewEntry)
>-        return;
>-
>-    webProcessProxy.addMessageReceiver(Messages::WebUserContentControllerProxy::messageReceiverName(), identifier().toUInt64(), *this);
>+    if (m_processes.add(&webProcessProxy).isNewEntry)
>+        webProcessProxy.addMessageReceiver(Messages::WebUserContentControllerProxy::messageReceiverName(), identifier().toUInt64(), *this);
> 
>     ASSERT(parameters.userContentWorlds.isEmpty());
>     for (const auto& world : m_userContentWorlds)
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index e107a2dd6ef4d1867aa954f0e977aef2f5cdd2d3..8d13123ae7747617cdfa293156d2cd9b83bedbe9 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-01-18  Chris Dumez  <cdumez@apple.com>
>+
>+        Regression(PSON) Content blockers are sometimes lost on back navigation cross-site
>+        https://bugs.webkit.org/show_bug.cgi?id=193588
>+        <rdar://problem/47131566>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add layout test coverage.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
>+
> 2019-01-18  Chris Dumez  <cdumez@apple.com>
> 
>         Regression(PSON) Scroll position is not always restored properly when navigating back
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>index afa29c6a712f07c83b914b80a08c4ce6f09375ec..fda46a597c8d3c03e3d04834dc31ea6250a2e230 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>@@ -28,6 +28,7 @@
> #import "PlatformUtilities.h"
> #import "Test.h"
> #import "TestNavigationDelegate.h"
>+#import <WebKit/WKContentRuleListStore.h>
> #import <WebKit/WKNavigationDelegatePrivate.h>
> #import <WebKit/WKNavigationPrivate.h>
> #import <WebKit/WKPreferencesPrivate.h>
>@@ -3920,4 +3921,118 @@ TEST(ProcessSwap, ScrollPositionRestoration)
> 
> #endif // PLATFORM(MAC)
> 
>+static NSString *blockmeFilter = @"[{\"action\":{\"type\":\"block\"},\"trigger\":{\"url-filter\":\".*blockme.html\"}}]";
>+
>+static const char* contentBlockingAfterProcessSwapTestBytes = R"PSONRESOURCE(
>+<body>
>+<script>
>+let wasSubframeLoaded = false;
>+// Pages with dedicated workers do not go into page cache.
>+var myWorker = new Worker('worker.js');
>+</script>
>+<iframe src="blockme.html"></iframe>
>+</body>
>+)PSONRESOURCE";
>+
>+static const char* markSubFrameAsLoadedTestBytes = R"PSONRESOURCE(
>+<script>
>+top.wasSubframeLoaded = true;
>+</script>
>+)PSONRESOURCE";
>+
>+TEST(ProcessSwap, ContentBlockingAfterProcessSwap)
>+{
>+    [[WKContentRuleListStore defaultStore] removeContentRuleListForIdentifier:@"ContentBlockingAfterProcessSwapExtension" completionHandler:^(NSError *error) {
>+        done = true;
>+    }];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
>+    [processPoolConfiguration setProcessSwapsOnNavigation:YES];
>+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
>+
>+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
>+    [webViewConfiguration setProcessPool:processPool.get()];
>+
>+    __block bool doneCompiling = false;
>+    [[WKContentRuleListStore defaultStore] compileContentRuleListForIdentifier:@"ContentBlockingAfterProcessSwapExtension" encodedContentRuleList:blockmeFilter completionHandler:^(WKContentRuleList *ruleList, NSError *error) {
>+
>+        EXPECT_NOT_NULL(ruleList);
>+        EXPECT_NULL(error);
>+
>+        [webViewConfiguration.get().userContentController addContentRuleList:ruleList];
>+
>+        doneCompiling = true;
>+    }];
>+    TestWebKitAPI::Util::run(&doneCompiling);
>+
>+    auto handler = adoptNS([[PSONScheme alloc] init]);
>+    [handler addMappingFromURLString:@"pson://www.webkit.org/main.html" toData:contentBlockingAfterProcessSwapTestBytes];
>+    [handler addMappingFromURLString:@"pson://www.webkit.org/blockme.html" toData:markSubFrameAsLoadedTestBytes];
>+    [handler addMappingFromURLString:@"pson://www.apple.com/main.html" toData:contentBlockingAfterProcessSwapTestBytes];
>+    [handler addMappingFromURLString:@"pson://www.apple.com/blockme.html" toData:markSubFrameAsLoadedTestBytes];
>+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
>+
>+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
>+    auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
>+    [webView setNavigationDelegate:delegate.get()];
>+
>+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
>+    [webView loadRequest:request];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    [webView evaluateJavaScript:@"window.wasSubframeLoaded ? 'FAIL' : 'PASS'" completionHandler: [&] (id result, NSError *error) {
>+        NSString *blockSuccess = (NSString *)result;
>+        EXPECT_WK_STREQ(@"PASS", blockSuccess);
>+        done = true;
>+    }];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
>+    [webView loadRequest:request];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    [webView evaluateJavaScript:@"window.wasSubframeLoaded ? 'FAIL' : 'PASS'" completionHandler: [&] (id result, NSError *error) {
>+        NSString *blockSuccess = (NSString *)result;
>+        EXPECT_WK_STREQ(@"PASS", blockSuccess);
>+        done = true;
>+    }];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    [webView goBack];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    [webView evaluateJavaScript:@"window.wasSubframeLoaded ? 'FAIL' : 'PASS'" completionHandler: [&] (id result, NSError *error) {
>+        NSString *blockSuccess = (NSString *)result;
>+        EXPECT_WK_STREQ(@"PASS", blockSuccess);
>+        done = true;
>+    }];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    [webView goForward];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    [webView evaluateJavaScript:@"window.wasSubframeLoaded ? 'FAIL' : 'PASS'" completionHandler: [&] (id result, NSError *error) {
>+        NSString *blockSuccess = (NSString *)result;
>+        EXPECT_WK_STREQ(@"PASS", blockSuccess);
>+        done = true;
>+    }];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    [[WKContentRuleListStore defaultStore] removeContentRuleListForIdentifier:@"ContentBlockingAfterProcessSwapExtension" completionHandler:^(NSError *error) {
>+        done = true;
>+    }];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+}
>+
> #endif // WK_API_ENABLED

Actions: View | Formatted Diff | Diff

Attachments on bug 193588: 359529