Regression(PSON) crash under WebPageProxy::didReceiveServerRedirectForProvisionalLoad...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Nov 2018 19:52:36 +0000 (19:52 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Nov 2018 19:52:36 +0000 (19:52 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238565 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebPageProxy.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

index d532549..d2077d7 100644 (file)
@@ -1,3 +1,21 @@
+2018-11-27  Chris Dumez  <cdumez@apple.com>
+
+        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  <jiewen_tan@apple.com>
 
         Disallow loading webarchives as iframes
index 77608c7..af2935d 100644 (file)
@@ -2704,8 +2704,12 @@ void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, R
             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 @@ void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, R
             // 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::processWillBecomeForeground()
 void WebPageProxy::resetState(ResetStateReason resetStateReason)
 {
     m_mainFrame = nullptr;
+    m_mainFrameCreationHandler = nullptr;
+    m_mainFrameWindowCreationHandler = nullptr;
+
 #if PLATFORM(COCOA)
     m_scrollingPerformanceData = nullptr;
 #endif
index 415ce9a..b894484 100644 (file)
@@ -1,3 +1,15 @@
+2018-11-27  Chris Dumez  <cdumez@apple.com>
+
+        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  <aakash_jain@apple.com>
 
         [ews-app] Add support to communicate with Buildbot (Follow-up fix)
index db664f4..b07a139 100644 (file)
@@ -485,6 +485,57 @@ TEST(ProcessSwap, LoadAfterPolicyDecision)
         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]);