Changeset 285877 in webkit

Timestamp:

Nov 16, 2021, 11:55:35 AM (5 years ago)

Author:

Chris Dumez

Message:

Crash under WebKit::WebPageProxy::commitProvisionalPage()
​https://bugs.webkit.org/show_bug.cgi?id=233199
<rdar://57659921>

Reviewed by Youenn Fablet.

Source/WebKit:

In the event where the committed WebProcess would crash while a cross-site provisional load
is going on in a provisional page / WebProcess, we would do a null dereference of the page's
drawing area when trying to commit the provisional page later on. We would also hit various
assertions in debug since the page's state gets completely reset when its WebProcess crashes.

To address the issue, we now clear the provisional page if the page's WebProcess crashes.

• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::resetStateAfterProcessExited):

Tools:

Add API test coverage.

• TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/WebPageProxy.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-11-16  Chris Dumez  <cdumez@apple.com>
+
+        Crash under WebKit::WebPageProxy::commitProvisionalPage()
+        https://bugs.webkit.org/show_bug.cgi?id=233199
+        <rdar://57659921>
+
+        Reviewed by Youenn Fablet.
+
+        In the event where the committed WebProcess would crash while a cross-site provisional load
+        is going on in a provisional page / WebProcess, we would do a null dereference of the page's
+        drawing area when trying to commit the provisional page later on. We would also hit various
+        assertions in debug since the page's state gets completely reset when its WebProcess crashes.
+
+        To address the issue, we now clear the provisional page if the page's WebProcess crashes.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::resetStateAfterProcessExited):
+
 2021-11-16  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -7937 +7937 @@
     m_cachedFontAttributesAtSelectionStart.reset();
 
+    if (terminationReason != ProcessTerminationReason::NavigationSwap)
+        m_provisionalPage = nullptr;
+
     if (terminationReason == ProcessTerminationReason::NavigationSwap)
         pageClient().processWillSwap();

trunk/Tools/ChangeLog

@@ +1 @@
+2021-11-16  Chris Dumez  <cdumez@apple.com>
+
+        Crash under WebKit::WebPageProxy::commitProvisionalPage()
+        https://bugs.webkit.org/show_bug.cgi?id=233199
+        <rdar://57659921>
+
+        Reviewed by Youenn Fablet.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2021-11-16  Fujii Hironori  <Hironori.Fujii@sony.com>
 

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

@@ -5526 +5526 @@
     EXPECT_NE(pid1, pid4);
     EXPECT_NE(pid3, pid4);
+}
+
+TEST(ProcessSwap, CommittedProcessCrashDuringCrossSiteNavigation)
+{
+    auto processPoolConfiguration = psonProcessPoolConfiguration();
+    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 navigationDelegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+    [webView setNavigationDelegate:navigationDelegate.get()];
+
+    done = false;
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto pid1 = [webView _webProcessIdentifier];
+
+    static bool didKill = false;
+    navigationDelegate->decidePolicyForNavigationAction = ^(WKNavigationAction *, void (^decisionHandler)(WKNavigationActionPolicy)) {
+        decisionHandler(WKNavigationActionPolicyAllow); // Will ask the load to proceed in a new provisional WebProcess since the navigation is cross-site.
+
+        // Simulate a crash of the committed WebProcess while the provisional navigation starts in the new provisional WebProcess.
+        dispatch_after(dispatch_time(DISPATCH_TIME_NOW, 0.2 * NSEC_PER_SEC), dispatch_get_main_queue(), ^{
+            kill(pid1, 9);
+            didKill = true;
+        });
+    };
+
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&didKill);
+
+    TestWebKitAPI::Util::sleep(0.5);
 }