Title: [240178] trunk
Revision
240178
Author
cdu...@apple.com
Date
2019-01-18 15:02:55 -0800 (Fri, 18 Jan 2019)

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

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (240177 => 240178)


--- trunk/Source/WebKit/ChangeLog	2019-01-18 22:55:56 UTC (rev 240177)
+++ trunk/Source/WebKit/ChangeLog	2019-01-18 23:02:55 UTC (rev 240178)
@@ -1,3 +1,28 @@
+2019-01-18  Chris Dumez  <cdu...@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.no...@apple.com>
 
         SDK_VARIANT build destinations should be separate from non-SDK_VARIANT builds

Modified: trunk/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.cpp (240177 => 240178)


--- trunk/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.cpp	2019-01-18 22:55:56 UTC (rev 240177)
+++ trunk/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.cpp	2019-01-18 23:02:55 UTC (rev 240178)
@@ -84,11 +84,9 @@
 
 void WebUserContentControllerProxy::addProcess(WebProcessProxy& webProcessProxy, WebPageCreationParameters& parameters)
 {
-    if (!m_processes.add(&webProcessProxy).isNewEntry)
-        return;
+    if (m_processes.add(&webProcessProxy).isNewEntry)
+        webProcessProxy.addMessageReceiver(Messages::WebUserContentControllerProxy::messageReceiverName(), identifier().toUInt64(), *this);
 
-    webProcessProxy.addMessageReceiver(Messages::WebUserContentControllerProxy::messageReceiverName(), identifier().toUInt64(), *this);
-
     ASSERT(parameters.userContentWorlds.isEmpty());
     for (const auto& world : m_userContentWorlds)
         parameters.userContentWorlds.append(std::make_pair(world.key->identifier(), world.key->name()));

Modified: trunk/Tools/ChangeLog (240177 => 240178)


--- trunk/Tools/ChangeLog	2019-01-18 22:55:56 UTC (rev 240177)
+++ trunk/Tools/ChangeLog	2019-01-18 23:02:55 UTC (rev 240178)
@@ -1,3 +1,15 @@
+2019-01-18  Chris Dumez  <cdu...@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.no...@apple.com>
 
         SDK_VARIANT build destinations should be separate from non-SDK_VARIANT builds

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (240177 => 240178)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm	2019-01-18 22:55:56 UTC (rev 240177)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm	2019-01-18 23:02:55 UTC (rev 240178)
@@ -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 @@
 
 #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=""
+</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
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to