UIProcess crash when a prewarmed process is terminated
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Apr 2019 20:52:36 +0000 (20:52 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Apr 2019 20:52:36 +0000 (20:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196451
<rdar://problem/49245471>

Reviewed by Geoffrey Garen.

Source/WebKit:

Stop assuming that WebProcessProxy::m_websiteDataStore is non-null as this is no longer
true after r243384. For example, prewarmed WebContent processe do not get a data store
until they actually get used.

* UIProcess/API/Cocoa/WKProcessPool.mm:
(-[WKProcessPool _prewarmedProcessIdentifier]):
* UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::networkProcessIdentifier):
(WebKit::WebProcessPool::prewarmedProcessIdentifier):
* UIProcess/WebProcessPool.h:
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::didClose):
(WebKit::WebProcessProxy::didFinishLaunching):
(WebKit::WebProcessProxy::requestTermination):
(WebKit::WebProcessProxy::isReleaseLoggingAllowed const):
* UIProcess/WebProcessProxy.h:

Tools:

Add API test coverage.

* TestWebKitAPI/Tests/WebKitCocoa/ProcessPreWarming.mm:
(TEST):

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm
Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h
Source/WebKit/UIProcess/WebProcessProxy.cpp
Source/WebKit/UIProcess/WebProcessProxy.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessPreWarming.mm

index a30be45..34e81cd 100644 (file)
@@ -1,3 +1,29 @@
+2019-04-01  Chris Dumez  <cdumez@apple.com>
+
+        UIProcess crash when a prewarmed process is terminated
+        https://bugs.webkit.org/show_bug.cgi?id=196451
+        <rdar://problem/49245471>
+
+        Reviewed by Geoffrey Garen.
+
+        Stop assuming that WebProcessProxy::m_websiteDataStore is non-null as this is no longer
+        true after r243384. For example, prewarmed WebContent processe do not get a data store
+        until they actually get used.
+
+        * UIProcess/API/Cocoa/WKProcessPool.mm:
+        (-[WKProcessPool _prewarmedProcessIdentifier]):
+        * UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::networkProcessIdentifier):
+        (WebKit::WebProcessPool::prewarmedProcessIdentifier):
+        * UIProcess/WebProcessPool.h:
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::didClose):
+        (WebKit::WebProcessProxy::didFinishLaunching):
+        (WebKit::WebProcessProxy::requestTermination):
+        (WebKit::WebProcessProxy::isReleaseLoggingAllowed const):
+        * UIProcess/WebProcessProxy.h:
+
 2019-04-01  Patrick Griffis  <pgriffis@igalia.com>
 
         [GTK][WPE] Add more websitedatastore directories to web process sandbox
index c35d534..ce1773e 100644 (file)
@@ -440,6 +440,12 @@ static NSDictionary *policiesHashMapToDictionary(const HashMap<String, HashMap<S
     return _processPool->networkProcessIdentifier();
 }
 
+- (pid_t)_prewarmedProcessIdentifier
+{
+    return _processPool->prewarmedProcessIdentifier();
+}
+
+
 - (void)_syncNetworkProcessCookies
 {
     _processPool->syncNetworkProcessCookies();
index 6cbe472..f6e0361 100644 (file)
@@ -89,6 +89,7 @@
 
 // Test only.
 - (pid_t)_networkProcessIdentifier WK_API_AVAILABLE(macos(10.13), ios(11.0));
+- (pid_t)_prewarmedProcessIdentifier WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
 
 // Test only.
 - (size_t)_webProcessCount WK_API_AVAILABLE(macos(10.13), ios(11.0));
index 4c3f1e7..35a7e91 100644 (file)
@@ -1440,10 +1440,12 @@ void WebProcessPool::refreshPlugins()
 
 ProcessID WebProcessPool::networkProcessIdentifier()
 {
-    if (!m_networkProcess)
-        return 0;
+    return m_networkProcess ? m_networkProcess->processIdentifier() : 0;
+}
 
-    return m_networkProcess->processIdentifier();
+ProcessID WebProcessPool::prewarmedProcessIdentifier()
+{
+    return m_prewarmedProcess ? m_prewarmedProcess->processIdentifier() : 0;
 }
 
 void WebProcessPool::activePagesOriginsInWebProcessForTesting(ProcessID pid, CompletionHandler<void(Vector<String>&&)>&& completionHandler)
index 2157b6b..99ba476 100644 (file)
@@ -232,6 +232,7 @@ public:
     void clearSupportedPlugins();
 
     ProcessID networkProcessIdentifier();
+    ProcessID prewarmedProcessIdentifier();
     void activePagesOriginsInWebProcessForTesting(ProcessID, CompletionHandler<void(Vector<String>&&)>&&);
     bool networkProcessHasEntitlementForTesting(const String&);
 
index ea74141..c37208d 100644 (file)
@@ -648,7 +648,7 @@ void WebProcessProxy::didReceiveSyncMessage(IPC::Connection& connection, IPC::De
 
 void WebProcessProxy::didClose(IPC::Connection&)
 {
-    RELEASE_LOG_IF(m_websiteDataStore->sessionID().isAlwaysOnLoggingAllowed(), Process, "%p - WebProcessProxy didClose (web process crash)", this);
+    RELEASE_LOG_IF(isReleaseLoggingAllowed(), Process, "%p - WebProcessProxy didClose (web process crash)", this);
     processDidTerminateOrFailedToLaunch();
 }
 
@@ -757,7 +757,7 @@ void WebProcessProxy::didFinishLaunching(ProcessLauncher* launcher, IPC::Connect
     AuxiliaryProcessProxy::didFinishLaunching(launcher, connectionIdentifier);
 
     if (!IPC::Connection::identifierIsValid(connectionIdentifier)) {
-        RELEASE_LOG_IF(m_websiteDataStore->sessionID().isAlwaysOnLoggingAllowed(), Process, "%p - WebProcessProxy didFinishLaunching - invalid connection identifier (web process failed to launch)", this);
+        RELEASE_LOG_IF(isReleaseLoggingAllowed(), Process, "%p - WebProcessProxy didFinishLaunching - invalid connection identifier (web process failed to launch)", this);
         processDidTerminateOrFailedToLaunch();
         return;
     }
@@ -984,7 +984,7 @@ void WebProcessProxy::requestTermination(ProcessTerminationReason reason)
         return;
 
     auto protectedThis = makeRef(*this);
-    RELEASE_LOG_IF(m_websiteDataStore->sessionID().isAlwaysOnLoggingAllowed(), Process, "%p - WebProcessProxy::requestTermination - reason %d", this, reason);
+    RELEASE_LOG_IF(isReleaseLoggingAllowed(), Process, "%p - WebProcessProxy::requestTermination - reason %d", this, reason);
 
     AuxiliaryProcessProxy::terminate();
 
@@ -999,6 +999,11 @@ void WebProcessProxy::requestTermination(ProcessTerminationReason reason)
         page->processDidTerminate(reason);
 }
 
+bool WebProcessProxy::isReleaseLoggingAllowed() const
+{
+    return !m_websiteDataStore || m_websiteDataStore->sessionID().isAlwaysOnLoggingAllowed();
+}
+
 void WebProcessProxy::stopResponsivenessTimer()
 {
     responsivenessTimer().stop();
index a253ae5..67bc7f5 100644 (file)
@@ -358,6 +358,8 @@ private:
 
     void processDidTerminateOrFailedToLaunch();
 
+    bool isReleaseLoggingAllowed() const;
+
     // IPC::Connection::Client
     friend class WebConnectionToWebProcess;
     void didReceiveMessage(IPC::Connection&, IPC::Decoder&) override;
index e274f85..cb9ecc2 100644 (file)
@@ -1,3 +1,16 @@
+2019-04-01  Chris Dumez  <cdumez@apple.com>
+
+        UIProcess crash when a prewarmed process is terminated
+        https://bugs.webkit.org/show_bug.cgi?id=196451
+        <rdar://problem/49245471>
+
+        Reviewed by Geoffrey Garen.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessPreWarming.mm:
+        (TEST):
+
 2019-04-01  Aakash Jain  <aakash_jain@apple.com>
 
         [ews-app] Display OS and Xcode configuration in status-bubble's hover-over message
index c2e09fc..01778f4 100644 (file)
@@ -120,3 +120,24 @@ TEST(WKProcessPool, AutomaticProcessWarming)
     EXPECT_FALSE([pool _hasPrewarmedWebProcess]);
     EXPECT_EQ(2U, [pool _webPageContentProcessCount]);
 }
+
+TEST(WKProcessPool, PrewarmedProcessCrash)
+{
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().prewarmsProcessesAutomatically = NO;
+
+    auto pool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+    [pool _warmInitialProcess];
+
+    EXPECT_TRUE([pool _hasPrewarmedWebProcess]);
+    EXPECT_EQ(1U, [pool _webPageContentProcessCount]);
+
+    // Wait for prewarmed process to finish launching.
+    while (![pool _prewarmedProcessIdentifier])
+        TestWebKitAPI::Util::sleep(0.01);
+
+    kill([pool _prewarmedProcessIdentifier], 9);
+
+    while ([pool _hasPrewarmedWebProcess])
+        TestWebKitAPI::Util::sleep(0.01);
+}