Diff
Modified: branches/safari-611-branch/Source/WebKit/ChangeLog (276555 => 276556)
--- branches/safari-611-branch/Source/WebKit/ChangeLog 2021-04-24 21:14:23 UTC (rev 276555)
+++ branches/safari-611-branch/Source/WebKit/ChangeLog 2021-04-24 21:21:24 UTC (rev 276556)
@@ -497,6 +497,110 @@
2021-04-15 Russell Epstein <[email protected]>
+ Cherry-pick r275846. rdar://problem/76727548
+
+ Create WebIDBServer only when it is needed
+ https://bugs.webkit.org/show_bug.cgi?id=224305
+ rdar://71962196
+
+ Reviewed by Alex Christensen.
+
+ Currently each WebIDBServer has a separate thread, so we don't want to create or keep WebIDBServer if it's not
+ in use. There are two cases where network process needs a WebIDBServer:
+ 1. handle requests from UI process to collect or remove data
+ 2. handle requests from Web process to perform IDB operations
+
+ Previously, we created a WebIDBServer when network process connects to a web process, but that does not mean web
+ process will perform IDB operations and we may create a thread that's not used. To avoid this, add a new message
+ AddIDBConnection for web process to ensure network process has WebIDBServer when it's about to perform operation.
+
+ Also, previously network process removes a WebIDBServer when session is removed and WebIDBServer is not binded
+ with any web process connection. Now we remove WebIDBServer when it's done handling requests, that is count of
+ pending requests from UI process is 0 and WebIDBServer is not binded with web process connection. We also remove
+ WebIDBServer at when network process is about to be destroyed (NetworkProcess::didClose) so we can break the
+ reference cycle of NetworkProcess-WebIDBServer-IDBServer, and make sure thread exits.
+
+ * NetworkProcess/IndexedDB/WebIDBServer.cpp:
+ (WebKit::WebIDBServer::create):
+ (WebKit::WebIDBServer::WebIDBServer):
+ (WebKit::m_closeCallback):
+ (WebKit::WebIDBServer::~WebIDBServer):
+ (WebKit::WebIDBServer::getOrigins):
+ (WebKit::WebIDBServer::closeAndDeleteDatabasesModifiedSince):
+ (WebKit::WebIDBServer::closeAndDeleteDatabasesForOrigins):
+ (WebKit::WebIDBServer::renameOrigin):
+ (WebKit::WebIDBServer::removeConnection):
+ (WebKit::WebIDBServer::close):
+ (WebKit::WebIDBServer::tryClose):
+ * NetworkProcess/IndexedDB/WebIDBServer.h:
+ * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+ (WebKit::NetworkConnectionToWebProcess::addIDBConnection):
+ * NetworkProcess/NetworkConnectionToWebProcess.h:
+ * NetworkProcess/NetworkConnectionToWebProcess.messages.in:
+ * NetworkProcess/NetworkProcess.cpp:
+ (WebKit::NetworkProcess::didClose):
+ (WebKit::NetworkProcess::createNetworkConnectionToWebProcess):
+ (WebKit::NetworkProcess::destroySession):
+ (WebKit::NetworkProcess::createWebIDBServer):
+ (WebKit::NetworkProcess::connectionToWebProcessClosed):
+ (WebKit::NetworkProcess::removeWebIDBServerIfPossible): Deleted. Move the removal code to WebIDBServer.
+ * WebProcess/Databases/IndexedDB/WebIDBConnectionToServer.cpp:
+ (WebKit::WebIDBConnectionToServer::WebIDBConnectionToServer):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275846 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2021-04-12 Sihui Liu <[email protected]>
+
+ Create WebIDBServer only when it is needed
+ https://bugs.webkit.org/show_bug.cgi?id=224305
+ rdar://71962196
+
+ Reviewed by Alex Christensen.
+
+ Currently each WebIDBServer has a separate thread, so we don't want to create or keep WebIDBServer if it's not
+ in use. There are two cases where network process needs a WebIDBServer:
+ 1. handle requests from UI process to collect or remove data
+ 2. handle requests from Web process to perform IDB operations
+
+ Previously, we created a WebIDBServer when network process connects to a web process, but that does not mean web
+ process will perform IDB operations and we may create a thread that's not used. To avoid this, add a new message
+ AddIDBConnection for web process to ensure network process has WebIDBServer when it's about to perform operation.
+
+ Also, previously network process removes a WebIDBServer when session is removed and WebIDBServer is not binded
+ with any web process connection. Now we remove WebIDBServer when it's done handling requests, that is count of
+ pending requests from UI process is 0 and WebIDBServer is not binded with web process connection. We also remove
+ WebIDBServer at when network process is about to be destroyed (NetworkProcess::didClose) so we can break the
+ reference cycle of NetworkProcess-WebIDBServer-IDBServer, and make sure thread exits.
+
+ * NetworkProcess/IndexedDB/WebIDBServer.cpp:
+ (WebKit::WebIDBServer::create):
+ (WebKit::WebIDBServer::WebIDBServer):
+ (WebKit::m_closeCallback):
+ (WebKit::WebIDBServer::~WebIDBServer):
+ (WebKit::WebIDBServer::getOrigins):
+ (WebKit::WebIDBServer::closeAndDeleteDatabasesModifiedSince):
+ (WebKit::WebIDBServer::closeAndDeleteDatabasesForOrigins):
+ (WebKit::WebIDBServer::renameOrigin):
+ (WebKit::WebIDBServer::removeConnection):
+ (WebKit::WebIDBServer::close):
+ (WebKit::WebIDBServer::tryClose):
+ * NetworkProcess/IndexedDB/WebIDBServer.h:
+ * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+ (WebKit::NetworkConnectionToWebProcess::addIDBConnection):
+ * NetworkProcess/NetworkConnectionToWebProcess.h:
+ * NetworkProcess/NetworkConnectionToWebProcess.messages.in:
+ * NetworkProcess/NetworkProcess.cpp:
+ (WebKit::NetworkProcess::didClose):
+ (WebKit::NetworkProcess::createNetworkConnectionToWebProcess):
+ (WebKit::NetworkProcess::destroySession):
+ (WebKit::NetworkProcess::createWebIDBServer):
+ (WebKit::NetworkProcess::connectionToWebProcessClosed):
+ (WebKit::NetworkProcess::removeWebIDBServerIfPossible): Deleted. Move the removal code to WebIDBServer.
+ * WebProcess/Databases/IndexedDB/WebIDBConnectionToServer.cpp:
+ (WebKit::WebIDBConnectionToServer::WebIDBConnectionToServer):
+
+2021-04-15 Russell Epstein <[email protected]>
+
Cherry-pick r275680. rdar://problem/76727557
Cache NSInvocation objects on _WKRemoteObjectInterface
Modified: branches/safari-611-branch/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.cpp (276555 => 276556)
--- branches/safari-611-branch/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.cpp 2021-04-24 21:14:23 UTC (rev 276555)
+++ branches/safari-611-branch/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.cpp 2021-04-24 21:21:24 UTC (rev 276556)
@@ -36,13 +36,15 @@
namespace WebKit {
-Ref<WebIDBServer> WebIDBServer::create(PAL::SessionID sessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&& spaceRequester)
+Ref<WebIDBServer> WebIDBServer::create(PAL::SessionID sessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&& spaceRequester, CompletionHandler<void()>&& closeCallback)
{
- return adoptRef(*new WebIDBServer(sessionID, directory, WTFMove(spaceRequester)));
+ return adoptRef(*new WebIDBServer(sessionID, directory, WTFMove(spaceRequester), WTFMove(closeCallback)));
}
-WebIDBServer::WebIDBServer(PAL::SessionID sessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&& spaceRequester)
+WebIDBServer::WebIDBServer(PAL::SessionID sessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&& spaceRequester, CompletionHandler<void()>&& closeCallback)
: CrossThreadTaskHandler("com.apple.WebKit.IndexedDBServer", WTF::CrossThreadTaskHandler::AutodrainedPoolForRunLoop::Use)
+ , m_dataTaskCounter([this](RefCounterEvent) { tryClose(); })
+ , m_closeCallback(WTFMove(closeCallback))
{
ASSERT(RunLoop::isMain());
@@ -57,6 +59,8 @@
WebIDBServer::~WebIDBServer()
{
ASSERT(RunLoop::isMain());
+ // close() has to be called to make sure thread exits.
+ ASSERT(!m_closeCallback);
}
void WebIDBServer::getOrigins(CompletionHandler<void(HashSet<WebCore::SecurityOriginData>&&)>&& callback)
@@ -63,11 +67,11 @@
{
ASSERT(RunLoop::isMain());
- postTask([this, protectedThis = makeRef(*this), callback = WTFMove(callback)]() mutable {
+ postTask([this, protectedThis = makeRef(*this), callback = WTFMove(callback), token = m_dataTaskCounter.count()]() mutable {
ASSERT(!RunLoop::isMain());
LockHolder locker(m_server->lock());
- postTaskReply(CrossThreadTask([callback = WTFMove(callback), origins = crossThreadCopy(m_server->getOrigins())]() mutable {
+ postTaskReply(CrossThreadTask([callback = WTFMove(callback), token = WTFMove(token), origins = crossThreadCopy(m_server->getOrigins())]() mutable {
callback(WTFMove(origins));
}));
});
@@ -77,12 +81,12 @@
{
ASSERT(RunLoop::isMain());
- postTask([this, protectedThis = makeRef(*this), modificationTime, callback = WTFMove(callback)]() mutable {
+ postTask([this, protectedThis = makeRef(*this), modificationTime, callback = WTFMove(callback), token = m_dataTaskCounter.count()]() mutable {
ASSERT(!RunLoop::isMain());
LockHolder locker(m_server->lock());
m_server->closeAndDeleteDatabasesModifiedSince(modificationTime);
- postTaskReply(CrossThreadTask([callback = WTFMove(callback)]() mutable {
+ postTaskReply(CrossThreadTask([callback = WTFMove(callback), token = WTFMove(token)]() mutable {
callback();
}));
});
@@ -92,12 +96,12 @@
{
ASSERT(RunLoop::isMain());
- postTask([this, protectedThis = makeRef(*this), originDatas = originDatas.isolatedCopy(), callback = WTFMove(callback)] () mutable {
+ postTask([this, protectedThis = makeRef(*this), originDatas = originDatas.isolatedCopy(), callback = WTFMove(callback), token = m_dataTaskCounter.count()] () mutable {
ASSERT(!RunLoop::isMain());
LockHolder locker(m_server->lock());
m_server->closeAndDeleteDatabasesForOrigins(originDatas);
- postTaskReply(CrossThreadTask([callback = WTFMove(callback)]() mutable {
+ postTaskReply(CrossThreadTask([callback = WTFMove(callback), token = WTFMove(token)]() mutable {
callback();
}));
});
@@ -107,12 +111,14 @@
{
ASSERT(RunLoop::isMain());
- postTask([this, protectedThis = makeRef(*this), oldOrigin = oldOrigin.isolatedCopy(), newOrigin = newOrigin.isolatedCopy(), callback = WTFMove(callback)] () mutable {
+ postTask([this, protectedThis = makeRef(*this), oldOrigin = oldOrigin.isolatedCopy(), newOrigin = newOrigin.isolatedCopy(), callback = WTFMove(callback), token = m_dataTaskCounter.count()] () mutable {
ASSERT(!RunLoop::isMain());
LockHolder locker(m_server->lock());
m_server->renameOrigin(oldOrigin, newOrigin);
- postTaskReply(CrossThreadTask(WTFMove(callback)));
+ postTaskReply(CrossThreadTask([callback = WTFMove(callback), token = WTFMove(token)]() mutable {
+ callback();
+ }));
});
}
@@ -372,8 +378,11 @@
{
ASSERT(RunLoop::isMain());
- m_connections.remove(&connection);
- connection.removeThreadMessageReceiver(Messages::WebIDBServer::messageReceiverName());
+ auto* takenConnection = m_connections.take(&connection);
+ if (!takenConnection)
+ return;
+
+ takenConnection->removeThreadMessageReceiver(Messages::WebIDBServer::messageReceiverName());
postTask([this, protectedThis = makeRef(*this), connectionID = connection.uniqueID()] {
auto connection = m_connectionMap.take(connectionID);
@@ -382,6 +391,8 @@
LockHolder locker(m_server->lock());
m_server->unregisterConnection(connection->connectionToClient());
});
+
+ tryClose();
}
void WebIDBServer::postTask(Function<void()>&& task)
@@ -399,6 +410,8 @@
void WebIDBServer::close()
{
ASSERT(RunLoop::isMain());
+ if (!m_closeCallback)
+ return;
// Remove the references held by IPC::Connection.
for (auto* connection : m_connections)
@@ -415,7 +428,17 @@
CrossThreadTaskHandler::kill();
});
+
+ m_closeCallback();
}
+void WebIDBServer::tryClose()
+{
+ if (!m_connections.isEmpty() || m_dataTaskCounter.value())
+ return;
+
+ close();
+}
+
} // namespace WebKit
#endif
Modified: branches/safari-611-branch/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.h (276555 => 276556)
--- branches/safari-611-branch/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.h 2021-04-24 21:14:23 UTC (rev 276555)
+++ branches/safari-611-branch/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.h 2021-04-24 21:21:24 UTC (rev 276556)
@@ -33,6 +33,7 @@
#include <WebCore/IDBServer.h>
#include <WebCore/StorageQuotaManager.h>
#include <wtf/CrossThreadTaskHandler.h>
+#include <wtf/RefCounter.h>
namespace WebCore {
class StorageQuotaManager;
@@ -45,7 +46,7 @@
class WebIDBServer final : public CrossThreadTaskHandler, public IPC::Connection::ThreadMessageReceiverRefCounted {
public:
- static Ref<WebIDBServer> create(PAL::SessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&&);
+ static Ref<WebIDBServer> create(PAL::SessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&&, CompletionHandler<void()>&&);
void getOrigins(CompletionHandler<void(HashSet<WebCore::SecurityOriginData>&&)>&&);
void closeAndDeleteDatabasesModifiedSince(WallTime, CompletionHandler<void()>&& callback);
@@ -90,18 +91,25 @@
void dispatchToThread(WTF::Function<void()>&&);
void close();
- bool hasConnection() const { return !m_connections.isEmpty(); }
private:
- WebIDBServer(PAL::SessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&&);
+ WebIDBServer(PAL::SessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&&, CompletionHandler<void()>&&);
~WebIDBServer();
void postTask(WTF::Function<void()>&&);
+ void tryClose();
+
std::unique_ptr<WebCore::IDBServer::IDBServer> m_server;
bool m_isSuspended { false };
HashMap<IPC::Connection::UniqueID, std::unique_ptr<WebIDBConnectionToClient>> m_connectionMap;
HashSet<IPC::Connection*> m_connections;
+
+ enum DataTaskCounterType { };
+ using DataTaskCounter = RefCounter<DataTaskCounterType>;
+ using DataTaskCounterToken = DataTaskCounter::Token;
+ DataTaskCounter m_dataTaskCounter;
+ CompletionHandler<void()> m_closeCallback;
};
} // namespace WebKit
Modified: branches/safari-611-branch/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp (276555 => 276556)
--- branches/safari-611-branch/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp 2021-04-24 21:14:23 UTC (rev 276555)
+++ branches/safari-611-branch/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp 2021-04-24 21:21:24 UTC (rev 276556)
@@ -1170,6 +1170,11 @@
networkProcess().setCORSDisablingPatterns(pageIdentifier, WTFMove(patterns));
}
+void NetworkConnectionToWebProcess::addIDBConnection()
+{
+ m_networkProcess->webIDBServer(m_sessionID).addConnection(m_connection.get(), m_webProcessIdentifier);
+}
+
} // namespace WebKit
#undef NETWORK_PROCESS_MESSAGE_CHECK_COMPLETION
Modified: branches/safari-611-branch/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h (276555 => 276556)
--- branches/safari-611-branch/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h 2021-04-24 21:14:23 UTC (rev 276555)
+++ branches/safari-611-branch/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h 2021-04-24 21:21:24 UTC (rev 276556)
@@ -176,6 +176,8 @@
void broadcastConsoleMessage(JSC::MessageSource, JSC::MessageLevel, const String& message);
+ void addIDBConnection();
+
private:
NetworkConnectionToWebProcess(NetworkProcess&, WebCore::ProcessIdentifier, PAL::SessionID, IPC::Connection::Identifier);
Modified: branches/safari-611-branch/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in (276555 => 276556)
--- branches/safari-611-branch/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in 2021-04-24 21:14:23 UTC (rev 276555)
+++ branches/safari-611-branch/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in 2021-04-24 21:21:24 UTC (rev 276556)
@@ -101,4 +101,6 @@
#if PLATFORM(MAC)
UpdateActivePages(String name, Vector<String> activePagesOrigins, audit_token_t auditToken)
#endif
+
+ AddIDBConnection()
}
Modified: branches/safari-611-branch/Source/WebKit/NetworkProcess/NetworkProcess.cpp (276555 => 276556)
--- branches/safari-611-branch/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2021-04-24 21:14:23 UTC (rev 276555)
+++ branches/safari-611-branch/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2021-04-24 21:21:24 UTC (rev 276556)
@@ -267,6 +267,10 @@
forEachNetworkSession([&] (auto& networkSession) {
platformFlushCookies(networkSession.sessionID(), [callbackAggregator] { });
});
+
+ // Make sure references to NetworkProcess in spaceRequester and closeHandler is removed.
+ for (auto& server : m_webIDBServers.values())
+ server->close();
}
void NetworkProcess::didCreateDownload()
@@ -383,10 +387,6 @@
connection.setOnLineState(NetworkStateNotifier::singleton().onLine());
m_storageManagerSet->addConnection(connection.connection());
-
-#if ENABLE(INDEXED_DATABASE)
- webIDBServer(sessionID).addConnection(connection.connection(), identifier);
-#endif
}
void NetworkProcess::clearCachedCredentials(PAL::SessionID sessionID)
@@ -547,9 +547,6 @@
m_storageManagerSet->remove(sessionID);
-#if ENABLE(INDEXED_DATABASE)
- removeWebIDBServerIfPossible(sessionID);
-#endif
}
#if ENABLE(RESOURCE_LOAD_STATISTICS)
@@ -2326,10 +2323,13 @@
path = m_idbDatabasePaths.get(sessionID);
}
- return WebIDBServer::create(sessionID, path, [this, weakThis = makeWeakPtr(this), sessionID](const auto& origin, uint64_t spaceRequested) {
- RefPtr<StorageQuotaManager> storageQuotaManager = weakThis ? this->storageQuotaManager(sessionID, origin) : nullptr;
- return storageQuotaManager ? storageQuotaManager->requestSpaceOnBackgroundThread(spaceRequested) : StorageQuotaManager::Decision::Deny;
- });
+ auto spaceRequester = [protectedThis = makeRef(*this), sessionID](const auto& origin, uint64_t spaceRequested) {
+ return protectedThis->storageQuotaManager(sessionID, origin)->requestSpaceOnBackgroundThread(spaceRequested);
+ };
+ auto closeHandler = [protectedThis = makeRef(*this), sessionID]() {
+ protectedThis->m_webIDBServers.remove(sessionID);
+ };
+ return WebIDBServer::create(sessionID, path, WTFMove(spaceRequester), WTFMove(closeHandler));
}
WebIDBServer& NetworkProcess::webIDBServer(PAL::SessionID sessionID)
@@ -2358,25 +2358,6 @@
ASSERT(sessionStorageQuotaManager);
sessionStorageQuotaManager->setIDBRootPath(idbRootPath);
}
-
-void NetworkProcess::removeWebIDBServerIfPossible(PAL::SessionID sessionID)
-{
- ASSERT(RunLoop::isMain());
-
- auto iterator = m_webIDBServers.find(sessionID);
- if (iterator == m_webIDBServers.end())
- return;
-
- if (m_networkSessions.contains(sessionID))
- return;
-
- if (iterator->value->hasConnection())
- return;
-
- iterator->value->close();
- m_webIDBServers.remove(iterator);
-}
-
#endif // ENABLE(INDEXED_DATABASE)
void NetworkProcess::syncLocalStorage(CompletionHandler<void()>&& completionHandler)
@@ -2651,10 +2632,8 @@
m_storageManagerSet->removeConnection(connection);
#if ENABLE(INDEXED_DATABASE)
- auto* webIDBServer = m_webIDBServers.get(sessionID);
- ASSERT(webIDBServer);
- webIDBServer->removeConnection(connection);
- removeWebIDBServerIfPossible(sessionID);
+ if (auto* server = m_webIDBServers.get(sessionID))
+ server->removeConnection(connection);
#endif
}
Modified: branches/safari-611-branch/Source/WebKit/WebProcess/Databases/IndexedDB/WebIDBConnectionToServer.cpp (276555 => 276556)
--- branches/safari-611-branch/Source/WebKit/WebProcess/Databases/IndexedDB/WebIDBConnectionToServer.cpp 2021-04-24 21:14:23 UTC (rev 276555)
+++ branches/safari-611-branch/Source/WebKit/WebProcess/Databases/IndexedDB/WebIDBConnectionToServer.cpp 2021-04-24 21:21:24 UTC (rev 276556)
@@ -61,6 +61,7 @@
WebIDBConnectionToServer::WebIDBConnectionToServer()
: m_connectionToServer(IDBClient::IDBConnectionToServer::create(*this))
{
+ send(Messages::NetworkConnectionToWebProcess::AddIDBConnection());
}
WebIDBConnectionToServer::~WebIDBConnectionToServer()
