Title: [238565] trunk
Revision
238565
Author
[email protected]
Date
2018-11-27 11:52:36 -0800 (Tue, 27 Nov 2018)

Log Message

Regression(PSON) crash under WebPageProxy::didReceiveServerRedirectForProvisionalLoadForFrame()
https://bugs.webkit.org/show_bug.cgi?id=191983
<rdar://problem/46246863>

Reviewed by Geoffrey Garen.

Source/WebKit:

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::continueNavigationInNewProcess):
Make sure the navigation still exists in m_mainFrameCreationHandler and return early if it
does not.

(WebKit::WebPageProxy::resetState):
Clear out m_mainFrameCreationHandler / m_mainFrameWindowCreationHandler if we resetting the state
after a crash. At this point, there is no chance the WebProcess will send us the IPC that will
cause these to get called and we do not want old state to remain for future navigations.

Tools:

Add API test coverage.

* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (238564 => 238565)


--- trunk/Source/WebKit/ChangeLog	2018-11-27 19:47:17 UTC (rev 238564)
+++ trunk/Source/WebKit/ChangeLog	2018-11-27 19:52:36 UTC (rev 238565)
@@ -1,3 +1,21 @@
+2018-11-27  Chris Dumez  <[email protected]>
+
+        Regression(PSON) crash under WebPageProxy::didReceiveServerRedirectForProvisionalLoadForFrame()
+        https://bugs.webkit.org/show_bug.cgi?id=191983
+        <rdar://problem/46246863>
+
+        Reviewed by Geoffrey Garen.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::continueNavigationInNewProcess):
+        Make sure the navigation still exists in m_mainFrameCreationHandler and return early if it
+        does not.
+
+        (WebKit::WebPageProxy::resetState):
+        Clear out m_mainFrameCreationHandler / m_mainFrameWindowCreationHandler if we resetting the state
+        after a crash. At this point, there is no chance the WebProcess will send us the IPC that will
+        cause these to get called and we do not want old state to remain for future navigations.
+
 2018-11-16  Jiewen Tan  <[email protected]>
 
         Disallow loading webarchives as iframes

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (238564 => 238565)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2018-11-27 19:47:17 UTC (rev 238564)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2018-11-27 19:52:36 UTC (rev 238565)
@@ -2704,8 +2704,12 @@
             loadRequestWithNavigation(navigation, ResourceRequest { navigation->currentRequest() }, WebCore::ShouldOpenExternalURLsPolicy::ShouldAllowExternalSchemes, nullptr, ShouldTreatAsContinuingLoad::Yes);
 
         ASSERT(!m_mainFrame);
-        m_mainFrameCreationHandler = [this, protectedThis = WTFMove(protectedThis), navigation = navigation.copyRef(), request =  navigation->currentRequest(), mainFrameURL, isServerRedirect = navigation->currentRequestIsRedirect()]() mutable {
+        m_mainFrameCreationHandler = [this, protectedThis = WTFMove(protectedThis), navigationID = navigation->navigationID(), request =  navigation->currentRequest(), mainFrameURL, isServerRedirect = navigation->currentRequestIsRedirect()]() mutable {
             ASSERT(m_mainFrame);
+            // This navigation was destroyed so no need to notify of redirect.
+            if (!navigationState().navigation(navigationID))
+                return;
+
             // Restore the main frame's committed URL as some clients may rely on it until the next load is committed.
             m_mainFrame->frameLoadState().setURL(mainFrameURL);
 
@@ -2714,7 +2718,7 @@
             // In this case we have the UIProcess synthesize the redirect notification at the appropriate time.
             if (isServerRedirect) {
                 m_mainFrame->frameLoadState().didStartProvisionalLoad(request.url());
-                didReceiveServerRedirectForProvisionalLoadForFrame(m_mainFrame->frameID(), navigation->navigationID(), WTFMove(request), { });
+                didReceiveServerRedirectForProvisionalLoadForFrame(m_mainFrame->frameID(), navigationID, WTFMove(request), { });
             }
         };
 
@@ -6264,6 +6268,9 @@
 void WebPageProxy::resetState(ResetStateReason resetStateReason)
 {
     m_mainFrame = nullptr;
+    m_mainFrameCreationHandler = nullptr;
+    m_mainFrameWindowCreationHandler = nullptr;
+
 #if PLATFORM(COCOA)
     m_scrollingPerformanceData = nullptr;
 #endif

Modified: trunk/Tools/ChangeLog (238564 => 238565)


--- trunk/Tools/ChangeLog	2018-11-27 19:47:17 UTC (rev 238564)
+++ trunk/Tools/ChangeLog	2018-11-27 19:52:36 UTC (rev 238565)
@@ -1,3 +1,15 @@
+2018-11-27  Chris Dumez  <[email protected]>
+
+        Regression(PSON) crash under WebPageProxy::didReceiveServerRedirectForProvisionalLoadForFrame()
+        https://bugs.webkit.org/show_bug.cgi?id=191983
+        <rdar://problem/46246863>
+
+        Reviewed by Geoffrey Garen.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2018-11-27  Aakash Jain  <[email protected]>
 
         [ews-app] Add support to communicate with Buildbot (Follow-up fix)

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (238564 => 238565)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm	2018-11-27 19:47:17 UTC (rev 238564)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm	2018-11-27 19:52:36 UTC (rev 238565)
@@ -485,6 +485,57 @@
         TestWebKitAPI::Util::spinRunLoop();
 }
 
+TEST(ProcessSwap, KillWebContentProcessAfterServerRedirectPolicyDecision)
+{
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().processSwapsOnNavigation = YES;
+    processPoolConfiguration.get().prewarmsProcessesAutomatically = 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 addRedirectFromURLString:@"pson://www.webkit.org/main2.html" toURLString:@"pson://www.apple.com/main.html"];
+    [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()];
+
+    [webView configuration].preferences.safeBrowsingEnabled = NO;
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main1.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    __block BOOL isRedirection = NO;
+    navigationDelegate->decidePolicyForNavigationAction = ^(WKNavigationAction * action, void (^decisionHandler)(WKNavigationActionPolicy)) {
+        decisionHandler(WKNavigationActionPolicyAllow);
+        if (!isRedirection) {
+            isRedirection = YES;
+            return;
+        }
+
+        navigationDelegate->decidePolicyForNavigationAction = nil;
+        [webView _killWebContentProcessAndResetState];
+        done = true;
+    };
+
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main2.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    TestWebKitAPI::Util::spinRunLoop(10);
+    [webView reload];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+}
+
 TEST(ProcessSwap, NoSwappingForeTLDPlus2)
 {
     auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to