- Revision
- 286397
- Author
- [email protected]
- Date
- 2021-12-01 16:18:08 -0800 (Wed, 01 Dec 2021)
Log Message
Cherry-pick r285720. rdar://problem/83941760
WebKit is unable to recover if a WebProcess gets terminated while it is launching
https://bugs.webkit.org/show_bug.cgi?id=233001
<rdar://85302938>
Reviewed by Brent Fulgham.
Source/WebKit:
While investigating <rdar://83941760>, I found that the WebAuthn Process would get
jetsammed, which would cause us to call WebProcessPool::terminateAllWebContentProcesses().
I also noticed that if one of these WebProcesses was still launching at the time
of the termination, then the WebProcessProxy / WebPageProxy would keep thinking the
WebProcess is still launching and would never attempt to relaunch it. This would result
in a blank and unresponsive WKWebView which wouldn't be able to do any loads.
The issue was due to ProcessLauncher::terminateProcess() calling invalidate(), which
it would not only terminate the XPC connection, it would also null out m_client. As a
result, we wouldn't notify the client that the process failed to launch. To address
the issue, I move the XPC connection termination logic out of invalidate() and into
its own terminateXPCConnection() function. I then called terminateXPCConnection()
instead of invalidate() inside ProcessLauncher::terminateProcess().
* UIProcess/API/Cocoa/WKProcessPool.mm:
(-[WKProcessPool _terminateAllWebContentProcesses]):
* UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
* UIProcess/Launcher/ProcessLauncher.cpp:
(WebKit::ProcessLauncher::invalidate):
* UIProcess/Launcher/ProcessLauncher.h:
* UIProcess/Launcher/mac/ProcessLauncherMac.mm:
(WebKit::ProcessLauncher::terminateProcess):
(WebKit::ProcessLauncher::platformInvalidate):
(WebKit::ProcessLauncher::terminateXPCConnection):
Tools:
Add API test coverage, this test was timing out before the fix.
* TestWebKitAPI/Tests/WebKitCocoa/WebProcessTerminate.mm:
(TEST):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@285720 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-612-branch/Source/WebKit/ChangeLog (286396 => 286397)
--- branches/safari-612-branch/Source/WebKit/ChangeLog 2021-12-02 00:18:04 UTC (rev 286396)
+++ branches/safari-612-branch/Source/WebKit/ChangeLog 2021-12-02 00:18:08 UTC (rev 286397)
@@ -1,5 +1,84 @@
2021-12-01 Alan Coon <[email protected]>
+ Cherry-pick r285720. rdar://problem/83941760
+
+ WebKit is unable to recover if a WebProcess gets terminated while it is launching
+ https://bugs.webkit.org/show_bug.cgi?id=233001
+ <rdar://85302938>
+
+ Reviewed by Brent Fulgham.
+
+ Source/WebKit:
+
+ While investigating <rdar://83941760>, I found that the WebAuthn Process would get
+ jetsammed, which would cause us to call WebProcessPool::terminateAllWebContentProcesses().
+ I also noticed that if one of these WebProcesses was still launching at the time
+ of the termination, then the WebProcessProxy / WebPageProxy would keep thinking the
+ WebProcess is still launching and would never attempt to relaunch it. This would result
+ in a blank and unresponsive WKWebView which wouldn't be able to do any loads.
+
+ The issue was due to ProcessLauncher::terminateProcess() calling invalidate(), which
+ it would not only terminate the XPC connection, it would also null out m_client. As a
+ result, we wouldn't notify the client that the process failed to launch. To address
+ the issue, I move the XPC connection termination logic out of invalidate() and into
+ its own terminateXPCConnection() function. I then called terminateXPCConnection()
+ instead of invalidate() inside ProcessLauncher::terminateProcess().
+
+ * UIProcess/API/Cocoa/WKProcessPool.mm:
+ (-[WKProcessPool _terminateAllWebContentProcesses]):
+ * UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
+ * UIProcess/Launcher/ProcessLauncher.cpp:
+ (WebKit::ProcessLauncher::invalidate):
+ * UIProcess/Launcher/ProcessLauncher.h:
+ * UIProcess/Launcher/mac/ProcessLauncherMac.mm:
+ (WebKit::ProcessLauncher::terminateProcess):
+ (WebKit::ProcessLauncher::platformInvalidate):
+ (WebKit::ProcessLauncher::terminateXPCConnection):
+
+ Tools:
+
+ Add API test coverage, this test was timing out before the fix.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/WebProcessTerminate.mm:
+ (TEST):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@285720 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2021-11-12 Chris Dumez <[email protected]>
+
+ WebKit is unable to recover if a WebProcess gets terminated while it is launching
+ https://bugs.webkit.org/show_bug.cgi?id=233001
+ <rdar://85302938>
+
+ Reviewed by Brent Fulgham.
+
+ While investigating <rdar://83941760>, I found that the WebAuthn Process would get
+ jetsammed, which would cause us to call WebProcessPool::terminateAllWebContentProcesses().
+ I also noticed that if one of these WebProcesses was still launching at the time
+ of the termination, then the WebProcessProxy / WebPageProxy would keep thinking the
+ WebProcess is still launching and would never attempt to relaunch it. This would result
+ in a blank and unresponsive WKWebView which wouldn't be able to do any loads.
+
+ The issue was due to ProcessLauncher::terminateProcess() calling invalidate(), which
+ it would not only terminate the XPC connection, it would also null out m_client. As a
+ result, we wouldn't notify the client that the process failed to launch. To address
+ the issue, I move the XPC connection termination logic out of invalidate() and into
+ its own terminateXPCConnection() function. I then called terminateXPCConnection()
+ instead of invalidate() inside ProcessLauncher::terminateProcess().
+
+ * UIProcess/API/Cocoa/WKProcessPool.mm:
+ (-[WKProcessPool _terminateAllWebContentProcesses]):
+ * UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
+ * UIProcess/Launcher/ProcessLauncher.cpp:
+ (WebKit::ProcessLauncher::invalidate):
+ * UIProcess/Launcher/ProcessLauncher.h:
+ * UIProcess/Launcher/mac/ProcessLauncherMac.mm:
+ (WebKit::ProcessLauncher::terminateProcess):
+ (WebKit::ProcessLauncher::platformInvalidate):
+ (WebKit::ProcessLauncher::terminateXPCConnection):
+
+2021-12-01 Alan Coon <[email protected]>
+
Cherry-pick r285619. rdar://problem/83941760
We should not kill all WebContent processes whenever the WebAuthn process crashes
Modified: branches/safari-612-branch/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm (286396 => 286397)
--- branches/safari-612-branch/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm 2021-12-02 00:18:04 UTC (rev 286396)
+++ branches/safari-612-branch/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm 2021-12-02 00:18:08 UTC (rev 286397)
@@ -595,4 +595,9 @@
_processPool->setUsesOnlyHIDGamepadProviderForTesting(usesHIDProvider);
}
+- (void)_terminateAllWebContentProcesses
+{
+ _processPool->terminateAllWebContentProcesses();
+}
+
@end
Modified: branches/safari-612-branch/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h (286396 => 286397)
--- branches/safari-612-branch/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h 2021-12-02 00:18:04 UTC (rev 286396)
+++ branches/safari-612-branch/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h 2021-12-02 00:18:08 UTC (rev 286397)
@@ -98,6 +98,8 @@
// Test only.
- (pid_t)_prewarmedProcessIdentifier WK_API_AVAILABLE(macos(10.15), ios(13.0));
+- (void)_terminateAllWebContentProcesses;
+
// Test only.
- (size_t)_webProcessCount WK_API_AVAILABLE(macos(10.13), ios(11.0));
- (BOOL)_hasPrewarmedWebProcess WK_API_AVAILABLE(macos(10.14.4), ios(12.2));
Modified: branches/safari-612-branch/Source/WebKit/UIProcess/Launcher/ProcessLauncher.cpp (286396 => 286397)
--- branches/safari-612-branch/Source/WebKit/UIProcess/Launcher/ProcessLauncher.cpp 2021-12-02 00:18:04 UTC (rev 286396)
+++ branches/safari-612-branch/Source/WebKit/UIProcess/Launcher/ProcessLauncher.cpp 2021-12-02 00:18:08 UTC (rev 286397)
@@ -61,7 +61,7 @@
void ProcessLauncher::invalidate()
{
- m_client = 0;
+ m_client = nullptr;
platformInvalidate();
}
Modified: branches/safari-612-branch/Source/WebKit/UIProcess/Launcher/ProcessLauncher.h (286396 => 286397)
--- branches/safari-612-branch/Source/WebKit/UIProcess/Launcher/ProcessLauncher.h 2021-12-02 00:18:04 UTC (rev 286396)
+++ branches/safari-612-branch/Source/WebKit/UIProcess/Launcher/ProcessLauncher.h 2021-12-02 00:18:08 UTC (rev 286397)
@@ -124,6 +124,10 @@
void platformInvalidate();
+#if PLATFORM(COCOA)
+ void terminateXPCConnection();
+#endif
+
Client* m_client;
#if PLATFORM(COCOA)
Modified: branches/safari-612-branch/Source/WebKit/UIProcess/Launcher/mac/ProcessLauncherMac.mm (286396 => 286397)
--- branches/safari-612-branch/Source/WebKit/UIProcess/Launcher/mac/ProcessLauncherMac.mm 2021-12-02 00:18:04 UTC (rev 286396)
+++ branches/safari-612-branch/Source/WebKit/UIProcess/Launcher/mac/ProcessLauncherMac.mm 2021-12-02 00:18:08 UTC (rev 286397)
@@ -324,7 +324,7 @@
void ProcessLauncher::terminateProcess()
{
if (m_isLaunching) {
- invalidate();
+ terminateXPCConnection();
return;
}
@@ -337,6 +337,11 @@
void ProcessLauncher::platformInvalidate()
{
+ terminateXPCConnection();
+}
+
+void ProcessLauncher::terminateXPCConnection()
+{
if (!m_xpcConnection)
return;
Modified: branches/safari-612-branch/Tools/ChangeLog (286396 => 286397)
--- branches/safari-612-branch/Tools/ChangeLog 2021-12-02 00:18:04 UTC (rev 286396)
+++ branches/safari-612-branch/Tools/ChangeLog 2021-12-02 00:18:08 UTC (rev 286397)
@@ -1,5 +1,64 @@
2021-12-01 Alan Coon <[email protected]>
+ Cherry-pick r285720. rdar://problem/83941760
+
+ WebKit is unable to recover if a WebProcess gets terminated while it is launching
+ https://bugs.webkit.org/show_bug.cgi?id=233001
+ <rdar://85302938>
+
+ Reviewed by Brent Fulgham.
+
+ Source/WebKit:
+
+ While investigating <rdar://83941760>, I found that the WebAuthn Process would get
+ jetsammed, which would cause us to call WebProcessPool::terminateAllWebContentProcesses().
+ I also noticed that if one of these WebProcesses was still launching at the time
+ of the termination, then the WebProcessProxy / WebPageProxy would keep thinking the
+ WebProcess is still launching and would never attempt to relaunch it. This would result
+ in a blank and unresponsive WKWebView which wouldn't be able to do any loads.
+
+ The issue was due to ProcessLauncher::terminateProcess() calling invalidate(), which
+ it would not only terminate the XPC connection, it would also null out m_client. As a
+ result, we wouldn't notify the client that the process failed to launch. To address
+ the issue, I move the XPC connection termination logic out of invalidate() and into
+ its own terminateXPCConnection() function. I then called terminateXPCConnection()
+ instead of invalidate() inside ProcessLauncher::terminateProcess().
+
+ * UIProcess/API/Cocoa/WKProcessPool.mm:
+ (-[WKProcessPool _terminateAllWebContentProcesses]):
+ * UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
+ * UIProcess/Launcher/ProcessLauncher.cpp:
+ (WebKit::ProcessLauncher::invalidate):
+ * UIProcess/Launcher/ProcessLauncher.h:
+ * UIProcess/Launcher/mac/ProcessLauncherMac.mm:
+ (WebKit::ProcessLauncher::terminateProcess):
+ (WebKit::ProcessLauncher::platformInvalidate):
+ (WebKit::ProcessLauncher::terminateXPCConnection):
+
+ Tools:
+
+ Add API test coverage, this test was timing out before the fix.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/WebProcessTerminate.mm:
+ (TEST):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@285720 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2021-11-12 Chris Dumez <[email protected]>
+
+ WebKit is unable to recover if a WebProcess gets terminated while it is launching
+ https://bugs.webkit.org/show_bug.cgi?id=233001
+ <rdar://85302938>
+
+ Reviewed by Brent Fulgham.
+
+ Add API test coverage, this test was timing out before the fix.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/WebProcessTerminate.mm:
+ (TEST):
+
+2021-12-01 Alan Coon <[email protected]>
+
Cherry-pick r285619. rdar://problem/83941760
We should not kill all WebContent processes whenever the WebAuthn process crashes
Modified: branches/safari-612-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebProcessTerminate.mm (286396 => 286397)
--- branches/safari-612-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebProcessTerminate.mm 2021-12-02 00:18:04 UTC (rev 286396)
+++ branches/safari-612-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebProcessTerminate.mm 2021-12-02 00:18:08 UTC (rev 286397)
@@ -29,6 +29,7 @@
#import "Test.h"
#import "TestNavigationDelegate.h"
+#import <WebKit/WKProcessPoolPrivate.h>
#import <WebKit/WKWebViewPrivate.h>
#import <WebKit/WebKit.h>
@@ -47,3 +48,20 @@
auto pid2 = [webView _webProcessIdentifier];
EXPECT_TRUE(pid != pid2);
}
+
+TEST(WebKit, TerminateAllProcessesDuringLaunch)
+{
+ auto webView = adoptNS([WKWebView new]);
+
+ // Initiate a load to make sure the process actually launches.
+ [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"about:blank"]]];
+
+ // Call terminateAllProcesses while the process is still launching.
+ [webView.get().configuration.processPool _terminateAllWebContentProcesses];
+
+ TestWebKitAPI::Util::sleep(0.5);
+
+ // The WKWebView should be able to recover from the WebProcess termination and navigation should succeed.
+ [webView loadHTMLString:@"test" baseURL:nil];
+ [webView _test_waitForDidFinishNavigation];
+}
