Title: [235952] trunk
- Revision
- 235952
- Author
- [email protected]
- Date
- 2018-09-12 14:23:40 -0700 (Wed, 12 Sep 2018)
Log Message
PSON: No process swap on back navigation after URL bar navigation
https://bugs.webkit.org/show_bug.cgi?id=189557
<rdar://problem/44353108>
Reviewed by Alex Christensen.
Source/WebKit:
Our logic in WebProcessPool::processForNavigationInternal() was wrongly using
WebBackForwardList::currentItem() as source item of the navigation, instead of
using Navigation::fromItem(). In case of back navigation, by the time
processForNavigation() is called, the WebBackForwardList's currentItem has already
been updated to be the target item, via a Sync IPC from the WebProcess. As a result,
the source and target items would be the same in the following check:
` if (currentItem->itemID().processIdentifier == backForwardListItem->itemID().processIdentifier)`
This would cause us to reuse the same process incorrectly. Our existing API test coverage
did not catch this because our target HistoryItem usually has a SuspendedPage and we decide
to use the SuspendedPage's process a few lines above in WebProcessPool::processForNavigationInternal().
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::processForNavigationInternal):
Tools:
Add API test coverage.
* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (235951 => 235952)
--- trunk/Source/WebKit/ChangeLog 2018-09-12 21:22:16 UTC (rev 235951)
+++ trunk/Source/WebKit/ChangeLog 2018-09-12 21:23:40 UTC (rev 235952)
@@ -1,3 +1,26 @@
+2018-09-12 Chris Dumez <[email protected]>
+
+ PSON: No process swap on back navigation after URL bar navigation
+ https://bugs.webkit.org/show_bug.cgi?id=189557
+ <rdar://problem/44353108>
+
+ Reviewed by Alex Christensen.
+
+ Our logic in WebProcessPool::processForNavigationInternal() was wrongly using
+ WebBackForwardList::currentItem() as source item of the navigation, instead of
+ using Navigation::fromItem(). In case of back navigation, by the time
+ processForNavigation() is called, the WebBackForwardList's currentItem has already
+ been updated to be the target item, via a Sync IPC from the WebProcess. As a result,
+ the source and target items would be the same in the following check:
+ ` if (currentItem->itemID().processIdentifier == backForwardListItem->itemID().processIdentifier)`
+
+ This would cause us to reuse the same process incorrectly. Our existing API test coverage
+ did not catch this because our target HistoryItem usually has a SuspendedPage and we decide
+ to use the SuspendedPage's process a few lines above in WebProcessPool::processForNavigationInternal().
+
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::processForNavigationInternal):
+
2018-09-12 Alex Christensen <[email protected]>
Make IPC::SharedBufferDataReference a type that decodes into but does not inherit from IPC::DataReference
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (235951 => 235952)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2018-09-12 21:22:16 UTC (rev 235951)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2018-09-12 21:23:40 UTC (rev 235952)
@@ -2159,8 +2159,8 @@
// If the target back/forward item and the current back/forward item originated
// in the same WebProcess then we should reuse the current WebProcess.
- if (auto* currentItem = page.backForwardList().currentItem()) {
- if (currentItem->itemID().processIdentifier == backForwardListItem->itemID().processIdentifier)
+ if (auto* fromItem = navigation.fromItem()) {
+ if (fromItem->itemID().processIdentifier == backForwardListItem->itemID().processIdentifier)
return page.process();
}
}
Modified: trunk/Tools/ChangeLog (235951 => 235952)
--- trunk/Tools/ChangeLog 2018-09-12 21:22:16 UTC (rev 235951)
+++ trunk/Tools/ChangeLog 2018-09-12 21:23:40 UTC (rev 235952)
@@ -1,3 +1,15 @@
+2018-09-12 Chris Dumez <[email protected]>
+
+ PSON: No process swap on back navigation after URL bar navigation
+ https://bugs.webkit.org/show_bug.cgi?id=189557
+ <rdar://problem/44353108>
+
+ Reviewed by Alex Christensen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
2018-09-11 Dean Jackson <[email protected]>
Header parsing for experimental and internal debug features
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (235951 => 235952)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2018-09-12 21:22:16 UTC (rev 235951)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2018-09-12 21:23:40 UTC (rev 235952)
@@ -419,6 +419,64 @@
EXPECT_TRUE(pid1 == pid3);
}
+TEST(ProcessSwap, BackWithoutSuspendedPage)
+{
+ auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+ processPoolConfiguration.get().processSwapsOnNavigation = 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]);
+ [handler addMappingFromURLString:@"pson://www.webkit.org/main.html" toData:testBytes];
+ [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+
+ RetainPtr<PSONMessageHandler> messageHandler = adoptNS([[PSONMessageHandler alloc] init]);
+ [[webViewConfiguration userContentController] addScriptMessageHandler:messageHandler.get() name:@"pson"];
+
+ auto webView1 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+ auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+ [webView1 setNavigationDelegate:delegate.get()];
+
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+ [webView1 loadRequest:request];
+
+ TestWebKitAPI::Util::run(&receivedMessage);
+ receivedMessage = false;
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto pid1 = [webView1 _webProcessIdentifier];
+ RetainPtr<_WKSessionState> sessionState = [webView1 _sessionState];
+ webView1 = nullptr;
+
+ auto webView2 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+ delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+ [webView2 setNavigationDelegate:delegate.get()];
+
+ [webView2 _restoreSessionState:sessionState.get() andNavigate:NO];
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
+ [webView2 loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto pid2 = [webView2 _webProcessIdentifier];
+
+ [webView2 goBack];
+
+ TestWebKitAPI::Util::run(&receivedMessage);
+ receivedMessage = false;
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto pid3 = [webView2 _webProcessIdentifier];
+
+ EXPECT_FALSE(pid1 == pid2);
+ EXPECT_FALSE(pid2 == pid3);
+}
+
#if PLATFORM(MAC)
TEST(ProcessSwap, CrossOriginWindowOpenNoOpener)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
