Crash under IPC::Connection::markCurrentlyDispatchedMessageAsInvalid()
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Aug 2019 17:12:43 +0000 (17:12 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Aug 2019 17:12:43 +0000 (17:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=200674
<rdar://problem/50692748>

Reviewed by Geoff Garen.

Source/WebKit:

When the client terminates a provisional process (e.g. via the [WKWebView _killWebContentProcessAndResetState]
SPI), the WebProcessProxy would notify its associated WebPageProxy objects that it had terminated but would fail
to notify its associated ProvisionalPageProxy objects. As a result, those objects would not get destroyed and
would still think that they were in the middle of a provisional load the next time a load started. This inconsistent
state would lead to crashes such as the one in the radar.

* UIProcess/ProvisionalPageProxy.cpp:
(WebKit::ProvisionalPageProxy::cancel):
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::requestTermination):

Tools:

Add API test coverage.

* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

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

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

index 1a4e4e5..63be939 100644 (file)
@@ -1,3 +1,22 @@
+2019-08-13  Chris Dumez  <cdumez@apple.com>
+
+        Crash under IPC::Connection::markCurrentlyDispatchedMessageAsInvalid()
+        https://bugs.webkit.org/show_bug.cgi?id=200674
+        <rdar://problem/50692748>
+
+        Reviewed by Geoff Garen.
+
+        When the client terminates a provisional process (e.g. via the [WKWebView _killWebContentProcessAndResetState]
+        SPI), the WebProcessProxy would notify its associated WebPageProxy objects that it had terminated but would fail
+        to notify its associated ProvisionalPageProxy objects. As a result, those objects would not get destroyed and
+        would still think that they were in the middle of a provisional load the next time a load started. This inconsistent
+        state would lead to crashes such as the one in the radar.
+
+        * UIProcess/ProvisionalPageProxy.cpp:
+        (WebKit::ProvisionalPageProxy::cancel):
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::requestTermination):
+
 2019-08-13  Youenn Fablet  <youenn@apple.com>
 
         Blob registries should be keyed by session IDs
index c37efb4..666fbd3 100644 (file)
@@ -120,6 +120,8 @@ void ProvisionalPageProxy::cancel()
     // If the provisional load started, then indicate that it failed due to cancellation by calling didFailProvisionalLoadForFrame().
     if (m_provisionalLoadURL.isEmpty())
         return;
+        
+    ASSERT(m_process->state() == WebProcessProxy::State::Running);
 
     RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "cancel: Simulating a didFailProvisionalLoadForFrame for pageID = %" PRIu64, m_page.pageID().toUInt64());
     ASSERT(m_mainFrame);
index 0b242a2..e169137 100644 (file)
@@ -1033,12 +1033,18 @@ void WebProcessProxy::requestTermination(ProcessTerminationReason reason)
     if (webConnection())
         webConnection()->didClose();
 
+    auto provisionalPages = WTF::map(m_provisionalPages, [](auto* provisionalPage) { return makeWeakPtr(provisionalPage); });
     auto pages = copyToVectorOf<RefPtr<WebPageProxy>>(m_pageMap.values());
 
     shutDown();
 
     for (auto& page : pages)
         page->processDidTerminate(reason);
+        
+    for (auto& provisionalPage : provisionalPages) {
+        if (provisionalPage)
+            provisionalPage->processDidTerminate();
+    }
 }
 
 bool WebProcessProxy::isReleaseLoggingAllowed() const
index 62ff3ee..fc68aca 100644 (file)
@@ -1,3 +1,15 @@
+2019-08-13  Chris Dumez  <cdumez@apple.com>
+
+        Crash under IPC::Connection::markCurrentlyDispatchedMessageAsInvalid()
+        https://bugs.webkit.org/show_bug.cgi?id=200674
+        <rdar://problem/50692748>
+
+        Reviewed by Geoff Garen.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2019-08-12  Takashi Komori  <Takashi.Komori@sony.com>
 
         [WTF] Thread::removeFromThreadGroup leaks weak pointers.
index 7872eb6..100e9bc 100644 (file)
@@ -640,6 +640,48 @@ TEST(ProcessSwap, KillWebContentProcessAfterServerRedirectPolicyDecision)
     done = false;
 }
 
+TEST(ProcessSwap, KillProvisionalWebContentProcessThenStartNewLoad)
+{
+    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()];
+    
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+    
+    // When the provisional load starts in the provisional process, kill the WebView's processes.
+    navigationDelegate->didStartProvisionalNavigationHandler = ^{
+        [webView _killWebContentProcessAndResetState];
+        done = true;
+    };
+    
+    // Start a new cross-site load, which should happen in a new provisional process.
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
+    [webView loadRequest:request];
+    
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+    
+    navigationDelegate->didStartProvisionalNavigationHandler = nil;
+    
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+}
+
 TEST(ProcessSwap, NoSwappingForeTLDPlus2)
 {
     auto processPoolConfiguration = psonProcessPoolConfiguration();