Title: [244799] trunk
- Revision
- 244799
- Author
- [email protected]
- Date
- 2019-04-30 13:20:00 -0700 (Tue, 30 Apr 2019)
Log Message
Only use a related page's process if that page has not been closed yet
https://bugs.webkit.org/show_bug.cgi?id=197393
<rdar://problem/50302423>
Reviewed by Tim Horton.
Source/WebKit:
We should not attempt to use a related page's process if that related page has already been closed.
Once closed, a page's process is invalid and trying to launch a new process for the closed page
leads to crashes such as the one in the radar.
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::launchProcess):
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::createWebPage):
Tools:
Add API test coverage.
* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (244798 => 244799)
--- trunk/Source/WebKit/ChangeLog 2019-04-30 20:13:09 UTC (rev 244798)
+++ trunk/Source/WebKit/ChangeLog 2019-04-30 20:20:00 UTC (rev 244799)
@@ -1,3 +1,20 @@
+2019-04-30 Chris Dumez <[email protected]>
+
+ Only use a related page's process if that page has not been closed yet
+ https://bugs.webkit.org/show_bug.cgi?id=197393
+ <rdar://problem/50302423>
+
+ Reviewed by Tim Horton.
+
+ We should not attempt to use a related page's process if that related page has already been closed.
+ Once closed, a page's process is invalid and trying to launch a new process for the closed page
+ leads to crashes such as the one in the radar.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::launchProcess):
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::createWebPage):
+
2019-04-30 Tim Horton <[email protected]>
Long-standing rare crash under -[WKWebView _web_immediateActionAnimationControllerForHitTestResultInternal...]
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (244798 => 244799)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-04-30 20:13:09 UTC (rev 244798)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-04-30 20:20:00 UTC (rev 244799)
@@ -727,7 +727,8 @@
auto& processPool = m_process->processPool();
- if (auto* relatedPage = m_configuration->relatedPage())
+ auto* relatedPage = m_configuration->relatedPage();
+ if (relatedPage && !relatedPage->isClosed())
m_process = relatedPage->ensureRunningProcess();
else
m_process = processPool.processForRegistrableDomain(m_websiteDataStore.get(), this, registrableDomain);
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (244798 => 244799)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2019-04-30 20:13:09 UTC (rev 244798)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2019-04-30 20:20:00 UTC (rev 244799)
@@ -1210,7 +1210,8 @@
}
RefPtr<WebProcessProxy> process;
- if (pageConfiguration->relatedPage()) {
+ auto* relatedPage = pageConfiguration->relatedPage();
+ if (relatedPage && !relatedPage->isClosed()) {
// Sharing processes, e.g. when creating the page via window.open().
process = &pageConfiguration->relatedPage()->ensureRunningProcess();
// We do not support several WebsiteDataStores sharing a single process.
Modified: trunk/Tools/ChangeLog (244798 => 244799)
--- trunk/Tools/ChangeLog 2019-04-30 20:13:09 UTC (rev 244798)
+++ trunk/Tools/ChangeLog 2019-04-30 20:20:00 UTC (rev 244799)
@@ -1,3 +1,15 @@
+2019-04-30 Chris Dumez <[email protected]>
+
+ Only use a related page's process if that page has not been closed yet
+ https://bugs.webkit.org/show_bug.cgi?id=197393
+ <rdar://problem/50302423>
+
+ Reviewed by Tim Horton.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
2019-04-30 Aakash Jain <[email protected]>
[ews-build] Enable Bindings tests queue on new EWS
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (244798 => 244799)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-04-30 20:13:09 UTC (rev 244798)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-04-30 20:20:00 UTC (rev 244799)
@@ -4709,6 +4709,66 @@
EXPECT_EQ(pid1, pid2); // WebViews are related so they should share the same process.
}
+TEST(ProcessSwap, ReloadRelatedWebViewAfterCrash)
+{
+ auto processPoolConfiguration = psonProcessPoolConfiguration();
+ auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+ auto webView1Configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ [webView1Configuration setProcessPool:processPool.get()];
+ auto handler = adoptNS([[PSONScheme alloc] init]);
+ [webView1Configuration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+
+ auto webView1 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webView1Configuration.get()]);
+ auto delegate = adoptNS([[TestNavigationDelegate alloc] init]);
+ __block bool didCrash = false;
+ [delegate setWebContentProcessDidTerminate:^(WKWebView *view) {
+ [view reload];
+ didCrash = true;
+ }];
+ [delegate setDidFinishNavigation:^(WKWebView *, WKNavigation *) {
+ done = true;
+ }];
+
+ [webView1 setNavigationDelegate:delegate.get()];
+
+ auto webView2Configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ [webView2Configuration setProcessPool:processPool.get()];
+ [webView2Configuration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+ webView2Configuration.get()._relatedWebView = webView1.get(); // webView2 will be related to webView1 and webView1's URL will be used for process swap decision.
+ auto webView2 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webView2Configuration.get()]);
+ [webView2 setNavigationDelegate:delegate.get()];
+
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main1.html"]];
+ [webView1 loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto pid1 = [webView1 _webProcessIdentifier];
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main2.html"]];
+ [webView2 loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto pid2 = [webView2 _webProcessIdentifier];
+
+ EXPECT_EQ(pid1, pid2); // WebViews are related so they should share the same process.
+
+ [webView1 _close];
+ webView1 = nullptr;
+
+ kill(pid1, 9);
+
+ TestWebKitAPI::Util::run(&didCrash);
+ didCrash = false;
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+}
+
TEST(ProcessSwap, TerminatedSuspendedPageProcess)
{
auto processPoolConfiguration = psonProcessPoolConfiguration();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
