Diff
Modified: trunk/Source/WebKit/ChangeLog (275845 => 275846)
--- trunk/Source/WebKit/ChangeLog 2021-04-12 23:14:49 UTC (rev 275845)
+++ trunk/Source/WebKit/ChangeLog 2021-04-12 23:43:49 UTC (rev 275846)
@@ -1,3 +1,53 @@
+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-12 Chris Dumez <[email protected]>
Make sure AuxiliaryProcessProxy::sendMessage() is called on the main thread
Modified: trunk/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.cpp (275845 => 275846)
--- trunk/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.cpp 2021-04-12 23:14:49 UTC (rev 275845)
+++ trunk/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.cpp 2021-04-12 23:43:49 UTC (rev 275846)
@@ -34,13 +34,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());
@@ -55,6 +57,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)
@@ -61,11 +65,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));
}));
});
@@ -75,12 +79,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();
}));
});
@@ -90,12 +94,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();
}));
});
@@ -105,12 +109,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();
+ }));
});
}
@@ -370,8 +376,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);
@@ -380,6 +389,8 @@
LockHolder locker(m_server->lock());
m_server->unregisterConnection(connection->connectionToClient());
});
+
+ tryClose();
}
void WebIDBServer::postTask(Function<void()>&& task)
@@ -397,6 +408,8 @@
void WebIDBServer::close()
{
ASSERT(RunLoop::isMain());
+ if (!m_closeCallback)
+ return;
// Remove the references held by IPC::Connection.
for (auto* connection : m_connections)
@@ -413,6 +426,16 @@
CrossThreadTaskHandler::kill();
});
+
+ m_closeCallback();
}
+void WebIDBServer::tryClose()
+{
+ if (!m_connections.isEmpty() || m_dataTaskCounter.value())
+ return;
+
+ close();
+}
+
} // namespace WebKit
Modified: trunk/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.h (275845 => 275846)
--- trunk/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.h 2021-04-12 23:14:49 UTC (rev 275845)
+++ trunk/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.h 2021-04-12 23:43:49 UTC (rev 275846)
@@ -31,6 +31,7 @@
#include <WebCore/IDBServer.h>
#include <WebCore/StorageQuotaManager.h>
#include <wtf/CrossThreadTaskHandler.h>
+#include <wtf/RefCounter.h>
namespace WebCore {
class StorageQuotaManager;
@@ -43,7 +44,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);
@@ -88,18 +89,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: trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp (275845 => 275846)
--- trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp 2021-04-12 23:14:49 UTC (rev 275845)
+++ trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp 2021-04-12 23:43:49 UTC (rev 275846)
@@ -1241,6 +1241,10 @@
session->networkLoadScheduler().prioritizeLoads(loads);
}
+void NetworkConnectionToWebProcess::addIDBConnection()
+{
+ m_networkProcess->webIDBServer(m_sessionID).addConnection(m_connection.get(), m_webProcessIdentifier);
+}
} // namespace WebKit
Modified: trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h (275845 => 275846)
--- trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h 2021-04-12 23:14:49 UTC (rev 275845)
+++ trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h 2021-04-12 23:43:49 UTC (rev 275846)
@@ -186,6 +186,8 @@
void broadcastConsoleMessage(JSC::MessageSource, JSC::MessageLevel, const String& message);
+ void addIDBConnection();
+
private:
NetworkConnectionToWebProcess(NetworkProcess&, WebCore::ProcessIdentifier, PAL::SessionID, IPC::Connection::Identifier);
Modified: trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in (275845 => 275846)
--- trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in 2021-04-12 23:14:49 UTC (rev 275845)
+++ trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in 2021-04-12 23:43:49 UTC (rev 275846)
@@ -107,4 +107,6 @@
#endif
SetResourceLoadSchedulingMode(WebCore::PageIdentifier webPageID, enum:uint8_t WebCore::LoadSchedulingMode mode)
PrioritizeResourceLoads(Vector<uint64_t> loadIdentifiers)
+
+ AddIDBConnection()
}
Modified: trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp (275845 => 275846)
--- trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2021-04-12 23:14:49 UTC (rev 275845)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2021-04-12 23:43:49 UTC (rev 275846)
@@ -272,6 +272,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()
@@ -391,8 +395,6 @@
connection.setOnLineState(NetworkStateNotifier::singleton().onLine());
m_storageManagerSet->addConnection(connection.connection());
-
- webIDBServer(sessionID).addConnection(connection.connection(), identifier);
}
void NetworkProcess::clearCachedCredentials(PAL::SessionID sessionID)
@@ -552,8 +554,6 @@
#endif
m_storageManagerSet->remove(sessionID);
-
- removeWebIDBServerIfPossible(sessionID);
}
#if ENABLE(RESOURCE_LOAD_STATISTICS)
@@ -2330,10 +2330,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)
@@ -2363,24 +2366,6 @@
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);
-}
-
void NetworkProcess::syncLocalStorage(CompletionHandler<void()>&& completionHandler)
{
m_storageManagerSet->waitUntilSyncingLocalStorageFinished();
@@ -2667,10 +2652,8 @@
{
m_storageManagerSet->removeConnection(connection);
- auto* webIDBServer = m_webIDBServers.get(sessionID);
- ASSERT(webIDBServer);
- webIDBServer->removeConnection(connection);
- removeWebIDBServerIfPossible(sessionID);
+ if (auto* server = m_webIDBServers.get(sessionID))
+ server->removeConnection(connection);
}
NetworkConnectionToWebProcess* NetworkProcess::webProcessConnection(ProcessIdentifier identifier) const
Modified: trunk/Source/WebKit/WebProcess/Databases/IndexedDB/WebIDBConnectionToServer.cpp (275845 => 275846)
--- trunk/Source/WebKit/WebProcess/Databases/IndexedDB/WebIDBConnectionToServer.cpp 2021-04-12 23:14:49 UTC (rev 275845)
+++ trunk/Source/WebKit/WebProcess/Databases/IndexedDB/WebIDBConnectionToServer.cpp 2021-04-12 23:43:49 UTC (rev 275846)
@@ -59,6 +59,7 @@
WebIDBConnectionToServer::WebIDBConnectionToServer()
: m_connectionToServer(IDBClient::IDBConnectionToServer::create(*this))
{
+ send(Messages::NetworkConnectionToWebProcess::AddIDBConnection());
}
WebIDBConnectionToServer::~WebIDBConnectionToServer()
