Changeset 238368 in webkit

Timestamp:

Nov 18, 2018 11:14:51 PM (5 years ago)

Author:

Chris Dumez

Message:

[PSON] Received an invalid message "WebPageProxy.DidPerformClientRedirect" from the web process
​https://bugs.webkit.org/show_bug.cgi?id=191828

Reviewed by Antti Koivisto.

Source/WebKit:

WebPageProxy::didPerformClientRedirect() had a MESSAGE_CHECK_URL() for both the source
and the destination URL. The macro calls WebProcessProxy::checkURLReceivedFromWebProcess()
to make sure that both URLs were actually loaded by this WebContent process.

However, now that we're enabled process-swap on cross-site navigation, these checks are no
longer fully valid. If the client-side redirect is cross-site, then the source and destination
URLs get loaded by 2 different WebContent processes. Since didPerformClientRedirect() gets
called after the redirect and gets set by the new WebContent process, only the destination URL
is guaranteed to have been loaded by this process. The source URL may have been loaded in
another process so this patch drops the message check for the source URL.

• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::didPerformClientRedirect):

Tools:

Add API test coverage.

• TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
• TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
• TestWebKitAPI/Tests/WebKitCocoa/client-side-redirect.html: Added.

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/WebPageProxy.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (modified) (4 diffs)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/client-side-redirect.html (added)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2018-11-18  Chris Dumez  <cdumez@apple.com>
+
+        [PSON] Received an invalid message "WebPageProxy.DidPerformClientRedirect" from the web process
+        https://bugs.webkit.org/show_bug.cgi?id=191828
+
+        Reviewed by Antti Koivisto.
+
+        WebPageProxy::didPerformClientRedirect() had a MESSAGE_CHECK_URL() for both the source
+        and the destination URL. The macro calls WebProcessProxy::checkURLReceivedFromWebProcess()
+        to make sure that both URLs were actually loaded by this WebContent process.
+
+        However, now that we're enabled process-swap on cross-site navigation, these checks are no
+        longer fully valid. If the client-side redirect is cross-site, then the source and destination
+        URLs get loaded by 2 different WebContent processes. Since didPerformClientRedirect() gets
+        called after the redirect and gets set by the new WebContent process, only the destination URL
+        is guaranteed to have been loaded by this process. The source URL may have been loaded in
+        another process so this patch drops the message check for the source URL.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::didPerformClientRedirect):
+
 2018-11-18  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
 

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -4443 +4443 @@
     MESSAGE_CHECK(frame);
     MESSAGE_CHECK(frame->page() == this);
-
-    MESSAGE_CHECK_URL(sourceURLString);
     MESSAGE_CHECK_URL(destinationURLString);
 

trunk/Tools/ChangeLog

@@ +1 @@
+2018-11-18  Chris Dumez  <cdumez@apple.com>
+
+        [PSON] Received an invalid message "WebPageProxy.DidPerformClientRedirect" from the web process
+        https://bugs.webkit.org/show_bug.cgi?id=191828
+
+        Reviewed by Antti Koivisto.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+        * TestWebKitAPI/Tests/WebKitCocoa/client-side-redirect.html: Added.
+
 2018-11-17  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

@@ -588 +588 @@
                 83779C381F82FECE007CDA8A /* VisitedLinkStore.mm in Sources */ = {isa = PBXBuildFile; fileRef = 83779C371F82FEB0007CDA8A /* VisitedLinkStore.mm */; };
                 837A35F11D9A1E7D00663C57 /* DownloadRequestBlobURL.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 837A35F01D9A1E6400663C57 /* DownloadRequestBlobURL.html */; };
+                839AA35F21A26B0300980DD6 /* client-side-redirect.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 839AA35E21A26ACD00980DD6 /* client-side-redirect.html */; };
                 83B6DE6F1EE75221001E792F /* RestoreSessionState.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 83B6DE6E1EE7520F001E792F /* RestoreSessionState.cpp */; };
                 83BAEE8D1EF4625500DDE894 /* PluginLoadClientPolicies.mm in Sources */ = {isa = PBXBuildFile; fileRef = 83BAEE8C1EF4625500DDE894 /* PluginLoadClientPolicies.mm */; };
@@ -999 +1000 @@
                                 2EFF06C71D886A580004BB30 /* change-video-source-on-end.html in Copy Resources */,
                                 9BD4239C1E04C01C00200395 /* chinese-character-with-image.html in Copy Resources */,
+                                839AA35F21A26B0300980DD6 /* client-side-redirect.html in Copy Resources */,
                                 1A50AA201A2A51FC00F4C345 /* close-from-within-create-page.html in Copy Resources */,
                                 9B270FEE1DDC2C0B002D53F3 /* closed-shadow-tree-test.html in Copy Resources */,
@@ -1737 +1739 @@
                 83779C371F82FEB0007CDA8A /* VisitedLinkStore.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = VisitedLinkStore.mm; sourceTree = "<group>"; };
                 837A35F01D9A1E6400663C57 /* DownloadRequestBlobURL.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = DownloadRequestBlobURL.html; sourceTree = "<group>"; };
+                839AA35E21A26ACD00980DD6 /* client-side-redirect.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "client-side-redirect.html"; sourceTree = "<group>"; };
                 83B6DE6E1EE7520F001E792F /* RestoreSessionState.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = RestoreSessionState.cpp; sourceTree = "<group>"; };
                 83B88A331C80056D00BB2418 /* HTMLParserIdioms.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = HTMLParserIdioms.cpp; sourceTree = "<group>"; };
@@ -2721 +2724 @@
                                 2EFF06C41D8867700004BB30 /* change-video-source-on-click.html */,
                                 2EFF06C61D886A560004BB30 /* change-video-source-on-end.html */,
+                                839AA35E21A26ACD00980DD6 /* client-side-redirect.html */,
                                 E5036F77211BC22800BFDBE2 /* color-drop.html */,
                                 F4B825D61EF4DBD4006E417F /* compressed-files.zip */,

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

@@ -1458 +1458 @@
 }
 
+TEST(ProcessSwap, CrossSiteClientSideRedirectFromFileURL)
+{
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().processSwapsOnNavigation = YES;
+    processPoolConfiguration.get().prewarmsProcessesAutomatically = YES;
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [webViewConfiguration setProcessPool:processPool.get()];
+    auto handler = adoptNS([[PSONScheme alloc] init]);
+    [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()];
+
+    willPerformClientRedirect = false;
+    didPerformClientRedirect = false;
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"client-side-redirect" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto pid1 = [webView _webProcessIdentifier];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto pid2 = [webView _webProcessIdentifier];
+    EXPECT_NE(pid1, pid2);
+
+    EXPECT_EQ(2U, [processPool _webProcessCountIgnoringPrewarmed]);
+    EXPECT_TRUE(willPerformClientRedirect);
+    EXPECT_TRUE(didPerformClientRedirect);
+}
+
 static void runNavigationWithLockedHistoryTest(ShouldEnablePSON shouldEnablePSON)
 {