Title: [285877] trunk
Revision
285877
Author
cdu...@apple.com
Date
2021-11-16 11:55:35 -0800 (Tue, 16 Nov 2021)

Log 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:

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (285876 => 285877)


--- trunk/Source/WebKit/ChangeLog	2021-11-16 19:49:47 UTC (rev 285876)
+++ trunk/Source/WebKit/ChangeLog	2021-11-16 19:55:35 UTC (rev 285877)
@@ -1,5 +1,23 @@
 2021-11-16  Chris Dumez  <cdu...@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  <cdu...@apple.com>
+
         [iOS] Do not require the web browser entitlement to opt into captive portal mode
         https://bugs.webkit.org/show_bug.cgi?id=233191
 

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (285876 => 285877)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2021-11-16 19:49:47 UTC (rev 285876)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2021-11-16 19:55:35 UTC (rev 285877)
@@ -7936,6 +7936,9 @@
     m_editorState = EditorState();
     m_cachedFontAttributesAtSelectionStart.reset();
 
+    if (terminationReason != ProcessTerminationReason::NavigationSwap)
+        m_provisionalPage = nullptr;
+
     if (terminationReason == ProcessTerminationReason::NavigationSwap)
         pageClient().processWillSwap();
     else

Modified: trunk/Tools/ChangeLog (285876 => 285877)


--- trunk/Tools/ChangeLog	2021-11-16 19:49:47 UTC (rev 285876)
+++ trunk/Tools/ChangeLog	2021-11-16 19:55:35 UTC (rev 285877)
@@ -1,3 +1,15 @@
+2021-11-16  Chris Dumez  <cdu...@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.fu...@sony.com>
 
         [Win][MiniBrowser][WK2] Show a message box for a provisional navigation failure

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (285876 => 285877)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm	2021-11-16 19:49:47 UTC (rev 285876)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm	2021-11-16 19:55:35 UTC (rev 285877)
@@ -5527,6 +5527,48 @@
     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);
+}
+
 TEST(ProcessSwap, NavigateBackAndForth)
 {
     auto processPoolConfiguration = psonProcessPoolConfiguration();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to