REGRESSION(r267763) NetworkProcess never terminates

https://bugs.webkit.org/show_bug.cgi?id=224191
<rdar://problem/76124590>

Patch by Alex Christensen <achristensen@webkit.org> on 2021-04-05
Reviewed by Chris Dumez.

Source/WebKit:

Before r267763, when a WebProcessPool was deallocated, the NetworkProcess it owned was terminated.
Since then, once you start using a NetworkProcess, it will be kept until your app closes or it crashes.
To reclaim these resources in a way similar to how we did before, we now terminate the network process in two situations:
1. If all WebsiteDataStores associated with it are deallocated.  This happens if you have never used the default WKWebsiteDataStore.
2. If all WebProcessPools are deallocated.  This can still happen if you do use the default WKWebsiteDataStore, which is never deallocated.

Covered by API tests.

* UIProcess/API/Cocoa/WKWebsiteDataStore.mm:
(+[WKWebsiteDataStore _defaultNetworkProcessExists]):
* UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h:
* UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::defaultNetworkProcessExists):
(WebKit::NetworkProcessProxy::removeSession):
* UIProcess/Network/NetworkProcessProxy.h:
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::~WebProcessPool):

Tools:

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

Canonical link: https://commits.webkit.org/236145@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275487 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog

@@ -1,3 +1,29 @@
+2021-04-05  Alex Christensen  <achristensen@webkit.org>
+
+        REGRESSION(r267763) NetworkProcess never terminates
+        https://bugs.webkit.org/show_bug.cgi?id=224191
+        <rdar://problem/76124590>
+
+        Reviewed by Chris Dumez.
+
+        Before r267763, when a WebProcessPool was deallocated, the NetworkProcess it owned was terminated.
+        Since then, once you start using a NetworkProcess, it will be kept until your app closes or it crashes.
+        To reclaim these resources in a way similar to how we did before, we now terminate the network process in two situations:
+        1. If all WebsiteDataStores associated with it are deallocated.  This happens if you have never used the default WKWebsiteDataStore.
+        2. If all WebProcessPools are deallocated.  This can still happen if you do use the default WKWebsiteDataStore, which is never deallocated.
+
+        Covered by API tests.
+
+        * UIProcess/API/Cocoa/WKWebsiteDataStore.mm:
+        (+[WKWebsiteDataStore _defaultNetworkProcessExists]):
+        * UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h:
+        * UIProcess/Network/NetworkProcessProxy.cpp:
+        (WebKit::NetworkProcessProxy::defaultNetworkProcessExists):
+        (WebKit::NetworkProcessProxy::removeSession):
+        * UIProcess/Network/NetworkProcessProxy.h:
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::~WebProcessPool):
+
 2021-04-05  Alex Christensen  <achristensen@webkit.org>
 
         Reduce crash inside getAuditToken

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

@@ -29,6 +29,7 @@
 #import "APIString.h"
 #import "AuthenticationChallengeDispositionCocoa.h"
 #import "CompletionHandlerCallChecker.h"
+#import "NetworkProcessProxy.h"
 #import "ShouldGrandfatherStatistics.h"
 #import "WKHTTPCookieStoreInternal.h"
 #import "WKNSArray.h"
@@ -724,4 +725,9 @@ - (BOOL)_networkProcessExists
     return !!_websiteDataStore->networkProcessIfExists();
 }
 
++ (BOOL)_defaultNetworkProcessExists
+{
+    return !!WebKit::NetworkProcessProxy::defaultNetworkProcess();
+}
+
 @end

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

@@ -98,6 +98,7 @@ typedef NS_OPTIONS(NSUInteger, _WKWebsiteDataStoreFetchOptions) {
 - (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));
++ (BOOL)_defaultNetworkProcessExists WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
 
 @end
 

Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp

@@ -101,17 +101,17 @@ Vector<Ref<NetworkProcessProxy>> NetworkProcessProxy::allNetworkProcesses()
     return processes;
 }
 
-static RefPtr<NetworkProcessProxy>& defaultProcess()
+RefPtr<NetworkProcessProxy>& NetworkProcessProxy::defaultNetworkProcess()
 {
     static NeverDestroyed<RefPtr<NetworkProcessProxy>> process;
     return process.get();
 }
 
-Ref<NetworkProcessProxy> NetworkProcessProxy::defaultNetworkProcess()
+Ref<NetworkProcessProxy> NetworkProcessProxy::ensureDefaultNetworkProcess()
 {
-    if (!defaultProcess())
-        defaultProcess() = NetworkProcessProxy::create();
-    return *defaultProcess();
+    if (!defaultNetworkProcess())
+        defaultNetworkProcess() = NetworkProcessProxy::create();
+    return *defaultNetworkProcess();
 }
 
 void NetworkProcessProxy::terminate()
@@ -310,8 +310,8 @@ void NetworkProcessProxy::networkProcessDidTerminate(TerminationReason reason)
 
     m_uploadActivity = WTF::nullopt;
 
-    if (defaultProcess() == this)
-        defaultProcess() = nullptr;
+    if (defaultNetworkProcess() == this)
+        defaultNetworkProcess() = nullptr;
 
     Ref<NetworkProcessProxy> protectedThis(*this);
     for (auto* processPool : WebProcessPool::allProcessPools())
@@ -1279,6 +1279,9 @@ void NetworkProcessProxy::removeSession(WebsiteDataStore& websiteDataStore)
 
     if (canSendMessage())
         send(Messages::NetworkProcess::DestroySession { websiteDataStore.sessionID() }, 0);
+
+    if (m_websiteDataStores.computesEmpty())
+        defaultNetworkProcess() = nullptr;
 }
 
 WebsiteDataStore* NetworkProcessProxy::websiteDataStoreFromSessionID(PAL::SessionID sessionID)

Source/WebKit/UIProcess/Network/NetworkProcessProxy.h

@@ -108,7 +108,8 @@ class NetworkProcessProxy final : public AuxiliaryProcessProxy, private ProcessT
     using DomainInNeedOfStorageAccess = WebCore::RegistrableDomain;
     using OpenerDomain = WebCore::RegistrableDomain;
 
-    static Ref<NetworkProcessProxy> defaultNetworkProcess();
+    static Ref<NetworkProcessProxy> ensureDefaultNetworkProcess();
+    static RefPtr<NetworkProcessProxy>& defaultNetworkProcess();
     static Ref<NetworkProcessProxy> create() { return adoptRef(*new NetworkProcessProxy); }
     ~NetworkProcessProxy();
 

Source/WebKit/UIProcess/WebProcessPool.cpp

@@ -376,6 +376,9 @@ WebProcessPool::~WebProcessPool()
 
         process->shutDown();
     }
+
+    if (processPools().isEmpty() && !!NetworkProcessProxy::defaultNetworkProcess())
+        NetworkProcessProxy::defaultNetworkProcess() = nullptr;
 }
 
 void WebProcessPool::initializeClient(const WKContextClientBase* client)

Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp

@@ -200,7 +200,7 @@ static Ref<NetworkProcessProxy> networkProcessForSession(PAL::SessionID sessionI
     return NetworkProcessProxy::create();
 #else
     UNUSED_PARAM(sessionID);
-    return NetworkProcessProxy::defaultNetworkProcess();
+    return NetworkProcessProxy::ensureDefaultNetworkProcess();
 #endif
 }
 

Tools/ChangeLog

@@ -1,3 +1,14 @@
+2021-04-05  Alex Christensen  <achristensen@webkit.org>
+
+        REGRESSION(r267763) NetworkProcess never terminates
+        https://bugs.webkit.org/show_bug.cgi?id=224191
+        <rdar://problem/76124590>
+
+        Reviewed by Chris Dumez.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/NetworkProcess.mm:
+        (TEST):
+
 2021-04-05  Alex Christensen  <achristensen@webkit.org>
 
         Resurrect Mac CMake build

Tools/TestWebKitAPI/Tests/WebKitCocoa/NetworkProcess.mm

@@ -36,7 +36,7 @@
 #import <wtf/RetainPtr.h>
 #import <wtf/Vector.h>
 
-TEST(WebKit, NetworkProcessEntitlements)
+TEST(NetworkProcess, Entitlements)
 {
     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:adoptNS([[WKWebViewConfiguration alloc] init]).get()]);
     [webView synchronouslyLoadTestPageNamed:@"simple"];
@@ -82,7 +82,7 @@ HTTPServer server([&] (Connection connection) {
     checkReferer([NSURL URLWithString:shorterHost], shorterHost.UTF8String);
 }
 
-TEST(WebKit, NetworkProcessLaunchOnlyWhenNecessary)
+TEST(NetworkProcess, LaunchOnlyWhenNecessary)
 {
     auto webView = adoptNS([WKWebView new]);
     [webView configuration].websiteDataStore._resourceLoadStatisticsEnabled = YES;
@@ -91,7 +91,7 @@ HTTPServer server([&] (Connection connection) {
     EXPECT_FALSE([[webView configuration].websiteDataStore _networkProcessExists]);
 }
 
-TEST(WebKit, NetworkProcessCrashWhenNotAssociatedWithDataStore)
+TEST(NetworkProcess, CrashWhenNotAssociatedWithDataStore)
 {
     pid_t networkProcessPID = 0;
     @autoreleasepool {
@@ -120,3 +120,28 @@ HTTPServer server([&] (Connection connection) {
     [webView synchronouslyLoadTestPageNamed:@"simple"];
     EXPECT_NE(networkProcessPID, [webView configuration].websiteDataStore._networkProcessIdentifier);
 }
+
+TEST(NetworkProcess, TerminateWhenUnused)
+{
+    RetainPtr<WKProcessPool> retainedPool;
+    @autoreleasepool {
+        auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+        configuration.get().websiteDataStore = [WKWebsiteDataStore nonPersistentDataStore];
+        retainedPool = configuration.get().processPool;
+        auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 0, 0) configuration:configuration.get()]);
+        [webView synchronouslyLoadTestPageNamed:@"simple"];
+        EXPECT_TRUE([WKWebsiteDataStore _defaultNetworkProcessExists]);
+    }
+    while ([WKWebsiteDataStore _defaultNetworkProcessExists])
+        TestWebKitAPI::Util::spinRunLoop();
+
+    retainedPool = nil;
+
+    @autoreleasepool {
+        auto webView = adoptNS([WKWebView new]);
+        [webView synchronouslyLoadTestPageNamed:@"simple"];
+        EXPECT_TRUE([WKWebsiteDataStore _defaultNetworkProcessExists]);
+    }
+    while ([WKWebsiteDataStore _defaultNetworkProcessExists])
+        TestWebKitAPI::Util::spinRunLoop();
+}