Changeset 275487 in webkit

Timestamp:

Apr 5, 2021 9:03:31 PM (3 years ago)

Author:

commit-queue@webkit.org

Message:

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):

Location:

trunk

Files:

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

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+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>
 

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

@@ -30 +30 @@
 #import "AuthenticationChallengeDispositionCocoa.h"
 #import "CompletionHandlerCallChecker.h"
+#import "NetworkProcessProxy.h"
 #import "ShouldGrandfatherStatistics.h"
 #import "WKHTTPCookieStoreInternal.h"
@@ -725 +726 @@
 }
 
++ (BOOL)_defaultNetworkProcessExists
+{
+    return !!WebKit::NetworkProcessProxy::defaultNetworkProcess();
+}
+
 @end

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

@@ -99 +99 @@
 + (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

trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp

@@ -102 +102 @@
 }
 
-static RefPtr<NetworkProcessProxy>& defaultProcess()
+RefPtr<NetworkProcessProxy>& NetworkProcessProxy::defaultNetworkProcess()
 {
     static NeverDestroyed<RefPtr<NetworkProcessProxy>> process;
@@ -108 +108 @@
 }
 
-Ref<NetworkProcessProxy> NetworkProcessProxy::defaultNetworkProcess()
-{
-    if (!defaultProcess())
-        defaultProcess() = NetworkProcessProxy::create();
-    return *defaultProcess();
+Ref<NetworkProcessProxy> NetworkProcessProxy::ensureDefaultNetworkProcess()
+{
+    if (!defaultNetworkProcess())
+        defaultNetworkProcess() = NetworkProcessProxy::create();
+    return *defaultNetworkProcess();
 }
 
@@ -311 +311 @@
     m_uploadActivity = WTF::nullopt;
 
-    if (defaultProcess() == this)
-        defaultProcess() = nullptr;
+    if (defaultNetworkProcess() == this)
+        defaultNetworkProcess() = nullptr;
 
     Ref<NetworkProcessProxy> protectedThis(*this);
@@ -1280 +1280 @@
     if (canSendMessage())
         send(Messages::NetworkProcess::DestroySession { websiteDataStore.sessionID() }, 0);
+
+    if (m_websiteDataStores.computesEmpty())
+        defaultNetworkProcess() = nullptr;
 }
 

trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h

@@ -109 +109 @@
     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();

trunk/Source/WebKit/UIProcess/WebProcessPool.cpp

@@ -377 +377 @@
         process->shutDown();
     }
+
+    if (processPools().isEmpty() && !!NetworkProcessProxy::defaultNetworkProcess())
+        NetworkProcessProxy::defaultNetworkProcess() = nullptr;
 }
 

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

@@ -201 +201 @@
 #else
     UNUSED_PARAM(sessionID);
-    return NetworkProcessProxy::defaultNetworkProcess();
+    return NetworkProcessProxy::ensureDefaultNetworkProcess();
 #endif
 }

trunk/Tools/ChangeLog

@@ +1 @@
+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>
 

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

@@ -37 +37 @@
 #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()]);
@@ -83 +83 @@
 }
 
-TEST(WebKit, NetworkProcessLaunchOnlyWhenNecessary)
+TEST(NetworkProcess, LaunchOnlyWhenNecessary)
 {
     auto webView = adoptNS([WKWebView new]);
@@ -92 +92 @@
 }
 
-TEST(WebKit, NetworkProcessCrashWhenNotAssociatedWithDataStore)
+TEST(NetworkProcess, CrashWhenNotAssociatedWithDataStore)
 {
     pid_t networkProcessPID = 0;
@@ -121 +121 @@
     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();
+}