Changeset 272376 in webkit

Timestamp:

Feb 4, 2021, 10:10:39 AM (4 years ago)

Author:

achristensen@apple.com

Message:

REGRESSION(r267763) Network process launches earlier than before
​https://bugs.webkit.org/show_bug.cgi?id=221384
<rdar://problem/73507706>

Patch by Alex Christensen <​achristensen@apple.com> on 2021-02-04
Reviewed by Chris Dumez.

Source/WebKit:

Before r267763 we could have an app launch, allocate a WKWebView, and set _resourceLoadStatisticsEnabled
all without having a network process launch.

When setting _resourceLoadStatisticsEnabled, we don't need to launch a network process if we haven't already
because when we do, WebsiteDataStore::parameters will send the correct value. I added a test that verifies this doesn't
launch the network process if it hasn't already been launched.

We also don't need to update the process assertion or the process suspension state if there is no network process.

sendNetworkProcessXPCEndpointToWebProcess is also called when opening Safari on iOS and would be the next thing to
unnecessarily launch the network process during app initialization. I manually verified that if we add an early return
if there is no network process yet, we do send the XPC endpoint to the web process when the network process does launch
through the call site in sendNetworkProcessXPCEndpointToAllWebProcesses.

I verified on my phone that this is the minimal change needed for Safari on iOS to not launch the network process before
application:didFinishLaunchingWithOptions: is called as it did before r267763.

• UIProcess/Cocoa/WebProcessPoolCocoa.mm:

(WebKit::WebProcessPool::updateProcessSuppressionState):

• UIProcess/WebProcessPool.cpp:

(WebKit::WebProcessPool::updateProcessAssertions):

• UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:

(WebKit::WebsiteDataStore::sendNetworkProcessXPCEndpointToWebProcess):

• UIProcess/WebsiteData/WebsiteDataStore.cpp:

(WebKit::WebsiteDataStore::setResourceLoadStatisticsEnabled):

• UIProcess/WebsiteData/WebsiteDataStore.h:

(WebKit::WebsiteDataStore::networkProcessIfExists):

Tools:

• TestWebKitAPI/Tests/WebKitCocoa/NetworkProcess.mm:

(TEST):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h (modified) (1 diff)
• Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm (modified) (1 diff)
• Source/WebKit/UIProcess/WebProcessPool.cpp (modified) (1 diff)
• Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp (modified) (2 diffs)
• Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/NetworkProcess.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-02-04  Alex Christensen  <achristensen@apple.com>
+
+        REGRESSION(r267763) Network process launches earlier than before
+        https://bugs.webkit.org/show_bug.cgi?id=221384
+        <rdar://problem/73507706>
+
+        Reviewed by Chris Dumez.
+
+        Before r267763 we could have an app launch, allocate a WKWebView, and set _resourceLoadStatisticsEnabled
+        all without having a network process launch.
+
+        When setting _resourceLoadStatisticsEnabled, we don't need to launch a network process if we haven't already
+        because when we do, WebsiteDataStore::parameters will send the correct value.  I added a test that verifies this doesn't
+        launch the network process if it hasn't already been launched.
+
+        We also don't need to update the process assertion or the process suspension state if there is no network process.
+
+        sendNetworkProcessXPCEndpointToWebProcess is also called when opening Safari on iOS and would be the next thing to
+        unnecessarily launch the network process during app initialization.  I manually verified that if we add an early return
+        if there is no network process yet, we do send the XPC endpoint to the web process when the network process does launch
+        through the call site in sendNetworkProcessXPCEndpointToAllWebProcesses.
+
+        I verified on my phone that this is the minimal change needed for Safari on iOS to not launch the network process before
+        application:didFinishLaunchingWithOptions: is called as it did before r267763.
+
+        * UIProcess/Cocoa/WebProcessPoolCocoa.mm:
+        (WebKit::WebProcessPool::updateProcessSuppressionState):
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::updateProcessAssertions):
+        * UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:
+        (WebKit::WebsiteDataStore::sendNetworkProcessXPCEndpointToWebProcess):
+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+        (WebKit::WebsiteDataStore::setResourceLoadStatisticsEnabled):
+        * UIProcess/WebsiteData/WebsiteDataStore.h:
+        (WebKit::WebsiteDataStore::networkProcessIfExists):
+
 2021-02-04  Aditya Keerthi  <akeerthi@apple.com>
 

trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm

@@ -701 +701 @@
 }
 
+- (BOOL)_networkProcessExists
+{
+    return !!_websiteDataStore->networkProcessIfExists();
+}
+
 @end

trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h

@@ -97 +97 @@
 - (pid_t)_networkProcessIdentifier WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
 + (void)_makeNextNetworkProcessLaunchFailForTesting WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
+- (BOOL)_networkProcessExists WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
 
 @end

trunk/Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm

@@ -142 +142 @@
 {
     WebsiteDataStore::forEachWebsiteDataStore([enabled = processSuppressionEnabled()] (WebsiteDataStore& dataStore) {
-        dataStore.networkProcess().setProcessSuppressionEnabled(enabled);
+        if (auto* networkProcess = dataStore.networkProcessIfExists())
+            networkProcess->setProcessSuppressionEnabled(enabled);
     });
 

trunk/Source/WebKit/UIProcess/WebProcessPool.cpp

@@ -1719 +1719 @@
 {
     WebsiteDataStore::forEachWebsiteDataStore([] (WebsiteDataStore& dataStore) {
-        dataStore.networkProcess().updateProcessAssertion();
+        if (auto* networkProcess = dataStore.networkProcessIfExists())
+            networkProcess->updateProcessAssertion();
     });
 #if ENABLE(GPU_PROCESS)

trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp

@@ -1991 +1991 @@
         resolveDirectoriesIfNecessary();
         
-        networkProcess().send(Messages::NetworkProcess::SetResourceLoadStatisticsEnabled(m_sessionID, true), 0);
+        if (m_networkProcess)
+            m_networkProcess->send(Messages::NetworkProcess::SetResourceLoadStatisticsEnabled(m_sessionID, true), 0);
         for (auto& processPool : processPools())
             processPool->sendToAllProcessesForSession(Messages::WebProcess::SetResourceLoadStatisticsEnabled(true), m_sessionID);
@@ -1997 +1998 @@
     }
 
-    networkProcess().send(Messages::NetworkProcess::SetResourceLoadStatisticsEnabled(m_sessionID, false), 0);
+    if (m_networkProcess)
+        m_networkProcess->send(Messages::NetworkProcess::SetResourceLoadStatisticsEnabled(m_sessionID, false), 0);
     for (auto& processPool : processPools())
         processPool->sendToAllProcessesForSession(Messages::WebProcess::SetResourceLoadStatisticsEnabled(false), m_sessionID);

trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h

@@ -123 +123 @@
     NetworkProcessProxy& networkProcess() const;
     NetworkProcessProxy& networkProcess();
+    NetworkProcessProxy* networkProcessIfExists() { return m_networkProcess.get(); }
 
     static WebsiteDataStore* existingDataStoreForSessionID(PAL::SessionID);

trunk/Tools/ChangeLog

@@ +1 @@
+2021-02-04  Alex Christensen  <achristensen@apple.com>
+
+        REGRESSION(r267763) Network process launches earlier than before
+        https://bugs.webkit.org/show_bug.cgi?id=221384
+        <rdar://problem/73507706>
+
+        Reviewed by Chris Dumez.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/NetworkProcess.mm:
+        (TEST):
+
 2021-02-04  Philippe Normand  <pnormand@igalia.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/NetworkProcess.mm

@@ -79 +79 @@
     checkReferer([NSURL URLWithString:shorterHost], shorterHost.UTF8String);
 }
+
+TEST(WebKit, NetworkProcessLaunchOnlyWhenNecessary)
+{
+    auto webView = [[WKWebView new] autorelease];
+    webView.configuration.websiteDataStore._resourceLoadStatisticsEnabled = YES;
+    EXPECT_FALSE([webView.configuration.websiteDataStore _networkProcessExists]);
+}