Changeset 240178 in webkit

Timestamp:

Jan 18, 2019 3:02:55 PM (5 years ago)

Author:

Chris Dumez

Message:

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 Alex Christensen.

Source/WebKit:

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):

Tools:

Add layout test coverage.

• TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+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 Alex Christensen.
+
+        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  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.cpp

@@ -85 +85 @@
 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());

trunk/Tools/ChangeLog

@@ +1 @@
+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 Alex Christensen.
+
+        Add layout test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2019-01-18  Jer Noble  <jer.noble@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

@@ -29 +29 @@
 #import "Test.h"
 #import "TestNavigationDelegate.h"
+#import <WebKit/WKContentRuleListStore.h>
 #import <WebKit/WKNavigationDelegatePrivate.h>
 #import <WebKit/WKNavigationPrivate.h>
@@ -3921 +3922 @@
 #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