Changeset 266798 in webkit

Timestamp:

Sep 9, 2020, 12:56:58 PM (6 years ago)

Author:

beidson@apple.com

Message:

Make sure WKWebsiteDataStore operations reuse existing process pools even when all WKWebViews have closed.
<rdar://problem/62978295> and ​https://bugs.webkit.org/show_bug.cgi?id=216317

Reviewed by Geoffrey Garen.

Source/WebKit:

Covered by new API test.

When WebsiteDataStores are gathering all the NetworkProcesses they might need to message, they miss some
obvious candidates if there are no longer any related WKWebViews.

Fix that by tracking which sessions a NetworkProcess knows about.

• UIProcess/Network/NetworkProcessProxy.cpp:

(WebKit::NetworkProcessProxy::addSession):
(WebKit::NetworkProcessProxy::hasSession const):
(WebKit::NetworkProcessProxy::removeSession):

• UIProcess/Network/NetworkProcessProxy.h:

• UIProcess/WebsiteData/WebsiteDataStore.cpp:

(WebKit::WebsiteDataStore::isAssociatedProcessPool const):

Tools:

• TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm:

(TestWebKitAPI::TEST):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp (modified) (2 diffs)
• Source/WebKit/UIProcess/Network/NetworkProcessProxy.h (modified) (2 diffs)
• Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm (modified) (3 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-09-09  Brady Eidson  <beidson@apple.com>
+
+        Make sure WKWebsiteDataStore operations reuse existing process pools even when all WKWebViews have closed.
+        <rdar://problem/62978295> and https://bugs.webkit.org/show_bug.cgi?id=216317
+
+        Reviewed by Geoffrey Garen.
+
+        Covered by new API test.
+        
+        When WebsiteDataStores are gathering all the NetworkProcesses they might need to message, they miss some
+        obvious candidates if there are no longer any related WKWebViews.
+        
+        Fix that by tracking which sessions a NetworkProcess knows about.
+
+        * UIProcess/Network/NetworkProcessProxy.cpp:
+        (WebKit::NetworkProcessProxy::addSession):
+        (WebKit::NetworkProcessProxy::hasSession const):
+        (WebKit::NetworkProcessProxy::removeSession):
+        * UIProcess/Network/NetworkProcessProxy.h:
+
+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+        (WebKit::WebsiteDataStore::isAssociatedProcessPool const):
+
 2020-09-09  Chris Dumez  <cdumez@apple.com>
 

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

@@ -1274 +1274 @@
 void NetworkProcessProxy::addSession(Ref<WebsiteDataStore>&& store)
 {
+    m_sessionIDs.add(store->sessionID());
+
     if (canSendMessage())
         send(Messages::NetworkProcess::AddWebsiteDataStore { store->parameters() }, 0);
@@ -1284 +1286 @@
 }
 
+bool NetworkProcessProxy::hasSession(PAL::SessionID sessionID) const
+{
+    return m_sessionIDs.contains(sessionID);
+}
+
 void NetworkProcessProxy::removeSession(PAL::SessionID sessionID)
 {
+    m_sessionIDs.remove(sessionID);
+
     if (canSendMessage())
         send(Messages::NetworkProcess::DestroySession { sessionID }, 0);

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

@@ -213 +213 @@
 
     void addSession(Ref<WebsiteDataStore>&&);
+    bool hasSession(PAL::SessionID) const;
     void removeSession(PAL::SessionID);
     
@@ -348 +349 @@
     };
 #endif
+
+    HashSet<PAL::SessionID> m_sessionIDs;
 };
 

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

@@ -2000 +2000 @@
     if (auto* processPoolDataStore = processPool.websiteDataStore())
         return processPoolDataStore == this;
+    if (auto* networkProcessProxy = processPool.networkProcess())
+        return networkProcessProxy->hasSession(m_sessionID);
     return false;
 }

trunk/Tools/ChangeLog

@@ +1 @@
+2020-09-09  Brady Eidson  <beidson@apple.com>
+
+        Make sure WKWebsiteDataStore operations reuse existing process pools even when all WKWebViews have closed.
+        <rdar://problem/62978295> and https://bugs.webkit.org/show_bug.cgi?id=216317
+
+        Reviewed by Geoffrey Garen.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm:
+        (TestWebKitAPI::TEST):
+
 2020-09-09  Angelos Oikonomopoulos  <angelos@igalia.com>
 

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

@@ -26 +26 @@
 #import "config.h"
 
+#import "HTTPServer.h"
 #import "PlatformUtilities.h"
 #import "TCPServer.h"
@@ -31 +32 @@
 #import "TestWKWebView.h"
 #import <WebKit/WKProcessPoolPrivate.h>
+#import <WebKit/WKWebViewPrivate.h>
 #import <WebKit/WKWebsiteDataRecordPrivate.h>
 #import <WebKit/WKWebsiteDataStorePrivate.h>
@@ -310 +312 @@
 }
 
-}
+TEST(WebKit, ClearCustomDataStoreNoWebViews)
+{
+    HTTPServer server([connectionCount = 0] (Connection connection) mutable {
+        ++connectionCount;
+        connection.receiveHTTPRequest([connection, connectionCount] (Vector<char>&& request) {
+            switch (connectionCount) {
+            case 1:
+                connection.send(
+                    "HTTP/1.1 200 OK\r\n"
+                    "Content-Length: 5\r\n"
+                    "Set-Cookie: a=b\r\n"
+                    "Connection: close\r\n"
+                    "\r\n"
+                    "Hello");
+                break;
+            case 2:
+                EXPECT_FALSE(strstr(request.data(), "Cookie: a=b\r\n"));
+                connection.send(
+                    "HTTP/1.1 200 OK\r\n"
+                    "Content-Length: 5\r\n"
+                    "Connection: close\r\n"
+                    "\r\n"
+                    "Hello");
+                break;
+            default:
+                ASSERT_NOT_REACHED();
+            }
+        });
+    });
+
+
+    NSURL *fileURL = [NSURL fileURLWithPath:@"/tmp/testcookiefile.cookie"];
+    auto configuration = adoptNS([[_WKWebsiteDataStoreConfiguration alloc] init]);
+    [configuration _setCookieStorageFile:fileURL];
+
+    auto dataStore = adoptNS([[WKWebsiteDataStore alloc] _initWithConfiguration:configuration.get()]);
+    auto viewConfiguration = adoptNS([WKWebViewConfiguration new]);
+    [viewConfiguration setWebsiteDataStore:dataStore.get()];
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 100, 100) configuration:viewConfiguration.get() addToWindow:YES]);
+
+    auto *url = [NSURL URLWithString:[NSString stringWithFormat:@"http://127.0.0.1:%d/index.html", server.port()]];
+
+    [webView synchronouslyLoadRequest:[NSURLRequest requestWithURL:url]];
+    [webView _close];
+    webView = nil;
+
+    // Now that the WebView is closed, remove all website data.
+    // Then recreate a WebView with the same configuration to confirm the website data was removed.
+    static bool done;
+    [dataStore removeDataOfTypes:[WKWebsiteDataStore allWebsiteDataTypes] modifiedSince:[NSDate distantPast] completionHandler:^{
+        done = true;
+    }];
+    Util::run(&done);
+    done = false;
+
+    webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 100, 100) configuration:viewConfiguration.get() addToWindow:YES]);
+    [webView synchronouslyLoadRequest:[NSURLRequest requestWithURL:url]];
+}
+
+} // namespace TestWebKitAPI