Changeset 275846 in webkit


Ignore:
Timestamp:
Apr 12, 2021 4:43:49 PM (3 years ago)
Author:
sihui_liu@apple.com
Message:

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

Location:
trunk/Source/WebKit
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r275839 r275846  
     12021-04-12  Sihui Liu  <sihui_liu@apple.com>
     2
     3        Create WebIDBServer only when it is needed
     4        https://bugs.webkit.org/show_bug.cgi?id=224305
     5        rdar://71962196
     6
     7        Reviewed by Alex Christensen.
     8
     9        Currently each WebIDBServer has a separate thread, so we don't want to create or keep WebIDBServer if it's not
     10        in use. There are two cases where network process needs a WebIDBServer:
     11        1. handle requests from UI process to collect or remove data
     12        2. handle requests from Web process to perform IDB operations
     13
     14        Previously, we created a WebIDBServer when network process connects to a web process, but that does not mean web
     15        process will perform IDB operations and we may create a thread that's not used. To avoid this, add a new message
     16        AddIDBConnection for web process to ensure network process has WebIDBServer when it's about to perform operation.
     17
     18        Also, previously network process removes a WebIDBServer when session is removed and WebIDBServer is not binded
     19        with any web process connection. Now we remove WebIDBServer when it's done handling requests, that is count of
     20        pending requests from UI process is 0 and WebIDBServer is not binded with web process connection. We also remove
     21        WebIDBServer at when network process is about to be destroyed (NetworkProcess::didClose) so we can break the
     22        reference cycle of NetworkProcess-WebIDBServer-IDBServer, and make sure thread exits.
     23
     24        * NetworkProcess/IndexedDB/WebIDBServer.cpp:
     25        (WebKit::WebIDBServer::create):
     26        (WebKit::WebIDBServer::WebIDBServer):
     27        (WebKit::m_closeCallback):
     28        (WebKit::WebIDBServer::~WebIDBServer):
     29        (WebKit::WebIDBServer::getOrigins):
     30        (WebKit::WebIDBServer::closeAndDeleteDatabasesModifiedSince):
     31        (WebKit::WebIDBServer::closeAndDeleteDatabasesForOrigins):
     32        (WebKit::WebIDBServer::renameOrigin):
     33        (WebKit::WebIDBServer::removeConnection):
     34        (WebKit::WebIDBServer::close):
     35        (WebKit::WebIDBServer::tryClose):
     36        * NetworkProcess/IndexedDB/WebIDBServer.h:
     37        * NetworkProcess/NetworkConnectionToWebProcess.cpp:
     38        (WebKit::NetworkConnectionToWebProcess::addIDBConnection):
     39        * NetworkProcess/NetworkConnectionToWebProcess.h:
     40        * NetworkProcess/NetworkConnectionToWebProcess.messages.in:
     41        * NetworkProcess/NetworkProcess.cpp:
     42        (WebKit::NetworkProcess::didClose):
     43        (WebKit::NetworkProcess::createNetworkConnectionToWebProcess):
     44        (WebKit::NetworkProcess::destroySession):
     45        (WebKit::NetworkProcess::createWebIDBServer):
     46        (WebKit::NetworkProcess::connectionToWebProcessClosed):
     47        (WebKit::NetworkProcess::removeWebIDBServerIfPossible): Deleted. Move the removal code to WebIDBServer.
     48        * WebProcess/Databases/IndexedDB/WebIDBConnectionToServer.cpp:
     49        (WebKit::WebIDBConnectionToServer::WebIDBConnectionToServer):
     50
    1512021-04-12  Chris Dumez  <cdumez@apple.com>
    252

  • trunk/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.cpp

    r275799 r275846  
    3535namespace WebKit {
    3636
    37 Ref<WebIDBServer> WebIDBServer::create(PAL::SessionID sessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&& spaceRequester)
    38 {
    39     return adoptRef(*new WebIDBServer(sessionID, directory, WTFMove(spaceRequester)));
    40 }
    41 
    42 WebIDBServer::WebIDBServer(PAL::SessionID sessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&& spaceRequester)
     37Ref<WebIDBServer> WebIDBServer::create(PAL::SessionID sessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&& spaceRequester, CompletionHandler<void()>&& closeCallback)
     38{
     39    return adoptRef(*new WebIDBServer(sessionID, directory, WTFMove(spaceRequester), WTFMove(closeCallback)));
     40}
     41
     42WebIDBServer::WebIDBServer(PAL::SessionID sessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&& spaceRequester, CompletionHandler<void()>&& closeCallback)
    4343    : CrossThreadTaskHandler("com.apple.WebKit.IndexedDBServer", WTF::CrossThreadTaskHandler::AutodrainedPoolForRunLoop::Use)
     44    , m_dataTaskCounter([this](RefCounterEvent) { tryClose(); })
     45    , m_closeCallback(WTFMove(closeCallback))
    4446{
    4547    ASSERT(RunLoop::isMain());
    … …  
    5658{
    5759    ASSERT(RunLoop::isMain());
     60    // close() has to be called to make sure thread exits.
     61    ASSERT(!m_closeCallback);
    5862}
    5963
    … …  
    6266    ASSERT(RunLoop::isMain());
    6367
    64     postTask([this, protectedThis = makeRef(*this), callback = WTFMove(callback)]() mutable {
     68    postTask([this, protectedThis = makeRef(*this), callback = WTFMove(callback), token = m_dataTaskCounter.count()]() mutable {
    6569        ASSERT(!RunLoop::isMain());
    6670
    6771        LockHolder locker(m_server->lock());
    68         postTaskReply(CrossThreadTask([callback = WTFMove(callback), origins = crossThreadCopy(m_server->getOrigins())]() mutable {
     72        postTaskReply(CrossThreadTask([callback = WTFMove(callback), token = WTFMove(token), origins = crossThreadCopy(m_server->getOrigins())]() mutable {
    6973            callback(WTFMove(origins));
    7074        }));
    … …  
    7680    ASSERT(RunLoop::isMain());
    7781
    78     postTask([this, protectedThis = makeRef(*this), modificationTime, callback = WTFMove(callback)]() mutable {
     82    postTask([this, protectedThis = makeRef(*this), modificationTime, callback = WTFMove(callback), token = m_dataTaskCounter.count()]() mutable {
    7983        ASSERT(!RunLoop::isMain());
    8084
    8185        LockHolder locker(m_server->lock());
    8286        m_server->closeAndDeleteDatabasesModifiedSince(modificationTime);
    83         postTaskReply(CrossThreadTask([callback = WTFMove(callback)]() mutable {
     87        postTaskReply(CrossThreadTask([callback = WTFMove(callback), token = WTFMove(token)]() mutable {
    8488            callback();
    8589        }));
    … …  
    9195    ASSERT(RunLoop::isMain());
    9296
    93     postTask([this, protectedThis = makeRef(*this), originDatas = originDatas.isolatedCopy(), callback = WTFMove(callback)] () mutable {
     97    postTask([this, protectedThis = makeRef(*this), originDatas = originDatas.isolatedCopy(), callback = WTFMove(callback), token = m_dataTaskCounter.count()] () mutable {
    9498        ASSERT(!RunLoop::isMain());
    9599
    96100        LockHolder locker(m_server->lock());
    97101        m_server->closeAndDeleteDatabasesForOrigins(originDatas);
    98         postTaskReply(CrossThreadTask([callback = WTFMove(callback)]() mutable {
     102        postTaskReply(CrossThreadTask([callback = WTFMove(callback), token = WTFMove(token)]() mutable {
    99103            callback();
    100104        }));
    … …  
    106110    ASSERT(RunLoop::isMain());
    107111
    108     postTask([this, protectedThis = makeRef(*this), oldOrigin = oldOrigin.isolatedCopy(), newOrigin = newOrigin.isolatedCopy(), callback = WTFMove(callback)] () mutable {
     112    postTask([this, protectedThis = makeRef(*this), oldOrigin = oldOrigin.isolatedCopy(), newOrigin = newOrigin.isolatedCopy(), callback = WTFMove(callback), token = m_dataTaskCounter.count()] () mutable {
    109113        ASSERT(!RunLoop::isMain());
    110114
    111115        LockHolder locker(m_server->lock());
    112116        m_server->renameOrigin(oldOrigin, newOrigin);
    113         postTaskReply(CrossThreadTask(WTFMove(callback)));
     117        postTaskReply(CrossThreadTask([callback = WTFMove(callback), token = WTFMove(token)]() mutable {
     118            callback();
     119        }));
    114120    });
    115121}
    … …  
    371377    ASSERT(RunLoop::isMain());
    372378
    373     m_connections.remove(&connection);
    374     connection.removeThreadMessageReceiver(Messages::WebIDBServer::messageReceiverName());
     379    auto* takenConnection = m_connections.take(&connection);
     380    if (!takenConnection)
     381        return;
     382
     383    takenConnection->removeThreadMessageReceiver(Messages::WebIDBServer::messageReceiverName());
    375384    postTask([this, protectedThis = makeRef(*this), connectionID = connection.uniqueID()] {
    376385        auto connection = m_connectionMap.take(connectionID);
    … …  
    381390        m_server->unregisterConnection(connection->connectionToClient());
    382391    });
     392
     393    tryClose();
    383394}
    384395
    … …  
    398409{
    399410    ASSERT(RunLoop::isMain());
     411    if (!m_closeCallback)
     412        return;
    400413
    401414    // Remove the references held by IPC::Connection.
    … …  
    414427        CrossThreadTaskHandler::kill();
    415428    });
     429
     430    m_closeCallback();
     431}
     432
     433void WebIDBServer::tryClose()
     434{
     435    if (!m_connections.isEmpty() || m_dataTaskCounter.value())
     436        return;
     437
     438    close();
    416439}
    417440

  • trunk/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.h

    r275799 r275846  
    3232#include <WebCore/StorageQuotaManager.h>
    3333#include <wtf/CrossThreadTaskHandler.h>
     34#include <wtf/RefCounter.h>
    3435
    3536namespace WebCore {
    … …  
    4445class WebIDBServer final : public CrossThreadTaskHandler, public IPC::Connection::ThreadMessageReceiverRefCounted {
    4546public:
    46     static Ref<WebIDBServer> create(PAL::SessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&&);
     47    static Ref<WebIDBServer> create(PAL::SessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&&, CompletionHandler<void()>&&);
    4748
    4849    void getOrigins(CompletionHandler<void(HashSet<WebCore::SecurityOriginData>&&)>&&);
    … …  
    8990    void close();
    9091
    91     bool hasConnection() const { return !m_connections.isEmpty(); }
    9292private:
    93     WebIDBServer(PAL::SessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&&);
     93    WebIDBServer(PAL::SessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&&, CompletionHandler<void()>&&);
    9494    ~WebIDBServer();
    9595
    9696    void postTask(WTF::Function<void()>&&);
     97
     98    void tryClose();
    9799
    98100    std::unique_ptr<WebCore::IDBServer::IDBServer> m_server;
    … …  
    101103    HashMap<IPC::Connection::UniqueID, std::unique_ptr<WebIDBConnectionToClient>> m_connectionMap;
    102104    HashSet<IPC::Connection*> m_connections;
     105
     106    enum DataTaskCounterType { };
     107    using DataTaskCounter = RefCounter<DataTaskCounterType>;
     108    using DataTaskCounterToken = DataTaskCounter::Token;
     109    DataTaskCounter m_dataTaskCounter;
     110    CompletionHandler<void()> m_closeCallback;
    103111};
    104112

  • trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp

    r275799 r275846  
    12421242}
    12431243
     1244void NetworkConnectionToWebProcess::addIDBConnection()
     1245{
     1246    m_networkProcess->webIDBServer(m_sessionID).addConnection(m_connection.get(), m_webProcessIdentifier);
     1247}
    12441248
    12451249} // namespace WebKit

  • trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h

    r275799 r275846  
    187187    void broadcastConsoleMessage(JSC::MessageSource, JSC::MessageLevel, const String& message);
    188188
     189    void addIDBConnection();
     190
    189191private:
    190192    NetworkConnectionToWebProcess(NetworkProcess&, WebCore::ProcessIdentifier, PAL::SessionID, IPC::Connection::Identifier);

  • trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in

    r275799 r275846  
    108108    SetResourceLoadSchedulingMode(WebCore::PageIdentifier webPageID, enum:uint8_t WebCore::LoadSchedulingMode mode)
    109109    PrioritizeResourceLoads(Vector<uint64_t> loadIdentifiers)
     110
     111    AddIDBConnection()
    110112}

  • trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp

    r275799 r275846  
    273273        platformFlushCookies(networkSession.sessionID(), [callbackAggregator] { });
    274274    });
     275
     276    // Make sure references to NetworkProcess in spaceRequester and closeHandler is removed.
     277    for (auto& server : m_webIDBServers.values())
     278        server->close();
    275279}
    276280
    … …  
    392396
    393397    m_storageManagerSet->addConnection(connection.connection());
    394 
    395     webIDBServer(sessionID).addConnection(connection.connection(), identifier);
    396398}
    397399
    … …  
    553555
    554556    m_storageManagerSet->remove(sessionID);
    555 
    556     removeWebIDBServerIfPossible(sessionID);
    557557}
    558558
    … …  
    23312331    }
    23322332
    2333     return WebIDBServer::create(sessionID, path, [this, weakThis = makeWeakPtr(this), sessionID](const auto& origin, uint64_t spaceRequested) {
    2334         RefPtr<StorageQuotaManager> storageQuotaManager = weakThis ? this->storageQuotaManager(sessionID, origin) : nullptr;
    2335         return storageQuotaManager ? storageQuotaManager->requestSpaceOnBackgroundThread(spaceRequested) : StorageQuotaManager::Decision::Deny;
    2336     });
     2333    auto spaceRequester = [protectedThis = makeRef(*this), sessionID](const auto& origin, uint64_t spaceRequested) {
     2334        return protectedThis->storageQuotaManager(sessionID, origin)->requestSpaceOnBackgroundThread(spaceRequested);
     2335    };
     2336    auto closeHandler = [protectedThis = makeRef(*this), sessionID]() {
     2337        protectedThis->m_webIDBServers.remove(sessionID);
     2338    };
     2339    return WebIDBServer::create(sessionID, path, WTFMove(spaceRequester), WTFMove(closeHandler));
    23372340}
    23382341
    … …  
    23622365    ASSERT(sessionStorageQuotaManager);
    23632366    sessionStorageQuotaManager->setIDBRootPath(idbRootPath);
    2364 }
    2365 
    2366 void NetworkProcess::removeWebIDBServerIfPossible(PAL::SessionID sessionID)
    2367 {
    2368     ASSERT(RunLoop::isMain());
    2369 
    2370     auto iterator = m_webIDBServers.find(sessionID);
    2371     if (iterator == m_webIDBServers.end())
    2372         return;
    2373 
    2374     if (m_networkSessions.contains(sessionID))
    2375         return;
    2376 
    2377     if (iterator->value->hasConnection())
    2378         return;
    2379 
    2380     iterator->value->close();
    2381     m_webIDBServers.remove(iterator);
    23822367}
    23832368
    … …  
    26682653    m_storageManagerSet->removeConnection(connection);
    26692654
    2670     auto* webIDBServer = m_webIDBServers.get(sessionID);
    2671     ASSERT(webIDBServer);
    2672     webIDBServer->removeConnection(connection);
    2673     removeWebIDBServerIfPossible(sessionID);
     2655    if (auto* server = m_webIDBServers.get(sessionID))
     2656        server->removeConnection(connection);
    26742657}
    26752658

  • trunk/Source/WebKit/WebProcess/Databases/IndexedDB/WebIDBConnectionToServer.cpp

    r275799 r275846  
    6060    : m_connectionToServer(IDBClient::IDBConnectionToServer::create(*this))
    6161{
     62    send(Messages::NetworkConnectionToWebProcess::AddIDBConnection());
    6263}
    6364
Note: See TracChangeset for help on using the changeset viewer.