⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 276556 in webkit


Ignore:
Timestamp:
Apr 24, 2021, 2:21:24 PM (5 years ago)
Author:
Russell Epstein
Message:

Reland r275846 with Unreviewed crash fix. rdar://77106929

Corrects crash due to bad merge.

Location:
branches/safari-611-branch/Source/WebKit
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-611-branch/Source/WebKit/ChangeLog

    r276555 r276556  
    495495            * WebProcess/WebCoreSupport/WebChromeClient.cpp:
    496496            (WebKit::WebChromeClient::createWindow):
     497
     4982021-04-15  Russell Epstein  <repstein@apple.com>
     499
     500        Cherry-pick r275846. rdar://problem/76727548
     501
     502    Create WebIDBServer only when it is needed
     503    https://bugs.webkit.org/show_bug.cgi?id=224305
     504    rdar://71962196
     505   
     506    Reviewed by Alex Christensen.
     507   
     508    Currently each WebIDBServer has a separate thread, so we don't want to create or keep WebIDBServer if it's not
     509    in use. There are two cases where network process needs a WebIDBServer:
     510    1. handle requests from UI process to collect or remove data
     511    2. handle requests from Web process to perform IDB operations
     512   
     513    Previously, we created a WebIDBServer when network process connects to a web process, but that does not mean web
     514    process will perform IDB operations and we may create a thread that's not used. To avoid this, add a new message
     515    AddIDBConnection for web process to ensure network process has WebIDBServer when it's about to perform operation.
     516   
     517    Also, previously network process removes a WebIDBServer when session is removed and WebIDBServer is not binded
     518    with any web process connection. Now we remove WebIDBServer when it's done handling requests, that is count of
     519    pending requests from UI process is 0 and WebIDBServer is not binded with web process connection. We also remove
     520    WebIDBServer at when network process is about to be destroyed (NetworkProcess::didClose) so we can break the
     521    reference cycle of NetworkProcess-WebIDBServer-IDBServer, and make sure thread exits.
     522   
     523    * NetworkProcess/IndexedDB/WebIDBServer.cpp:
     524    (WebKit::WebIDBServer::create):
     525    (WebKit::WebIDBServer::WebIDBServer):
     526    (WebKit::m_closeCallback):
     527    (WebKit::WebIDBServer::~WebIDBServer):
     528    (WebKit::WebIDBServer::getOrigins):
     529    (WebKit::WebIDBServer::closeAndDeleteDatabasesModifiedSince):
     530    (WebKit::WebIDBServer::closeAndDeleteDatabasesForOrigins):
     531    (WebKit::WebIDBServer::renameOrigin):
     532    (WebKit::WebIDBServer::removeConnection):
     533    (WebKit::WebIDBServer::close):
     534    (WebKit::WebIDBServer::tryClose):
     535    * NetworkProcess/IndexedDB/WebIDBServer.h:
     536    * NetworkProcess/NetworkConnectionToWebProcess.cpp:
     537    (WebKit::NetworkConnectionToWebProcess::addIDBConnection):
     538    * NetworkProcess/NetworkConnectionToWebProcess.h:
     539    * NetworkProcess/NetworkConnectionToWebProcess.messages.in:
     540    * NetworkProcess/NetworkProcess.cpp:
     541    (WebKit::NetworkProcess::didClose):
     542    (WebKit::NetworkProcess::createNetworkConnectionToWebProcess):
     543    (WebKit::NetworkProcess::destroySession):
     544    (WebKit::NetworkProcess::createWebIDBServer):
     545    (WebKit::NetworkProcess::connectionToWebProcessClosed):
     546    (WebKit::NetworkProcess::removeWebIDBServerIfPossible): Deleted. Move the removal code to WebIDBServer.
     547    * WebProcess/Databases/IndexedDB/WebIDBConnectionToServer.cpp:
     548    (WebKit::WebIDBConnectionToServer::WebIDBConnectionToServer):
     549   
     550    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275846 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     551
     552    2021-04-12  Sihui Liu  <sihui_liu@apple.com>
     553
     554            Create WebIDBServer only when it is needed
     555            https://bugs.webkit.org/show_bug.cgi?id=224305
     556            rdar://71962196
     557
     558            Reviewed by Alex Christensen.
     559
     560            Currently each WebIDBServer has a separate thread, so we don't want to create or keep WebIDBServer if it's not
     561            in use. There are two cases where network process needs a WebIDBServer:
     562            1. handle requests from UI process to collect or remove data
     563            2. handle requests from Web process to perform IDB operations
     564
     565            Previously, we created a WebIDBServer when network process connects to a web process, but that does not mean web
     566            process will perform IDB operations and we may create a thread that's not used. To avoid this, add a new message
     567            AddIDBConnection for web process to ensure network process has WebIDBServer when it's about to perform operation.
     568
     569            Also, previously network process removes a WebIDBServer when session is removed and WebIDBServer is not binded
     570            with any web process connection. Now we remove WebIDBServer when it's done handling requests, that is count of
     571            pending requests from UI process is 0 and WebIDBServer is not binded with web process connection. We also remove
     572            WebIDBServer at when network process is about to be destroyed (NetworkProcess::didClose) so we can break the
     573            reference cycle of NetworkProcess-WebIDBServer-IDBServer, and make sure thread exits.
     574
     575            * NetworkProcess/IndexedDB/WebIDBServer.cpp:
     576            (WebKit::WebIDBServer::create):
     577            (WebKit::WebIDBServer::WebIDBServer):
     578            (WebKit::m_closeCallback):
     579            (WebKit::WebIDBServer::~WebIDBServer):
     580            (WebKit::WebIDBServer::getOrigins):
     581            (WebKit::WebIDBServer::closeAndDeleteDatabasesModifiedSince):
     582            (WebKit::WebIDBServer::closeAndDeleteDatabasesForOrigins):
     583            (WebKit::WebIDBServer::renameOrigin):
     584            (WebKit::WebIDBServer::removeConnection):
     585            (WebKit::WebIDBServer::close):
     586            (WebKit::WebIDBServer::tryClose):
     587            * NetworkProcess/IndexedDB/WebIDBServer.h:
     588            * NetworkProcess/NetworkConnectionToWebProcess.cpp:
     589            (WebKit::NetworkConnectionToWebProcess::addIDBConnection):
     590            * NetworkProcess/NetworkConnectionToWebProcess.h:
     591            * NetworkProcess/NetworkConnectionToWebProcess.messages.in:
     592            * NetworkProcess/NetworkProcess.cpp:
     593            (WebKit::NetworkProcess::didClose):
     594            (WebKit::NetworkProcess::createNetworkConnectionToWebProcess):
     595            (WebKit::NetworkProcess::destroySession):
     596            (WebKit::NetworkProcess::createWebIDBServer):
     597            (WebKit::NetworkProcess::connectionToWebProcessClosed):
     598            (WebKit::NetworkProcess::removeWebIDBServerIfPossible): Deleted. Move the removal code to WebIDBServer.
     599            * WebProcess/Databases/IndexedDB/WebIDBConnectionToServer.cpp:
     600            (WebKit::WebIDBConnectionToServer::WebIDBConnectionToServer):
    497601
    4986022021-04-15  Russell Epstein  <repstein@apple.com>

  • branches/safari-611-branch/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.cpp

    r276555 r276556  
    3737namespace WebKit {
    3838
    39 Ref<WebIDBServer> WebIDBServer::create(PAL::SessionID sessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&& spaceRequester)
    40 {
    41     return adoptRef(*new WebIDBServer(sessionID, directory, WTFMove(spaceRequester)));
    42 }
    43 
    44 WebIDBServer::WebIDBServer(PAL::SessionID sessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&& spaceRequester)
     39Ref<WebIDBServer> WebIDBServer::create(PAL::SessionID sessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&& spaceRequester, CompletionHandler<void()>&& closeCallback)
     40{
     41    return adoptRef(*new WebIDBServer(sessionID, directory, WTFMove(spaceRequester), WTFMove(closeCallback)));
     42}
     43
     44WebIDBServer::WebIDBServer(PAL::SessionID sessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&& spaceRequester, CompletionHandler<void()>&& closeCallback)
    4545    : CrossThreadTaskHandler("com.apple.WebKit.IndexedDBServer", WTF::CrossThreadTaskHandler::AutodrainedPoolForRunLoop::Use)
     46    , m_dataTaskCounter([this](RefCounterEvent) { tryClose(); })
     47    , m_closeCallback(WTFMove(closeCallback))
    4648{
    4749    ASSERT(RunLoop::isMain());
    … …  
    5860{
    5961    ASSERT(RunLoop::isMain());
     62    // close() has to be called to make sure thread exits.
     63    ASSERT(!m_closeCallback);
    6064}
    6165
    … …  
    6468    ASSERT(RunLoop::isMain());
    6569
    66     postTask([this, protectedThis = makeRef(*this), callback = WTFMove(callback)]() mutable {
     70    postTask([this, protectedThis = makeRef(*this), callback = WTFMove(callback), token = m_dataTaskCounter.count()]() mutable {
    6771        ASSERT(!RunLoop::isMain());
    6872
    6973        LockHolder locker(m_server->lock());
    70         postTaskReply(CrossThreadTask([callback = WTFMove(callback), origins = crossThreadCopy(m_server->getOrigins())]() mutable {
     74        postTaskReply(CrossThreadTask([callback = WTFMove(callback), token = WTFMove(token), origins = crossThreadCopy(m_server->getOrigins())]() mutable {
    7175            callback(WTFMove(origins));
    7276        }));
    … …  
    7882    ASSERT(RunLoop::isMain());
    7983
    80     postTask([this, protectedThis = makeRef(*this), modificationTime, callback = WTFMove(callback)]() mutable {
     84    postTask([this, protectedThis = makeRef(*this), modificationTime, callback = WTFMove(callback), token = m_dataTaskCounter.count()]() mutable {
    8185        ASSERT(!RunLoop::isMain());
    8286
    8387        LockHolder locker(m_server->lock());
    8488        m_server->closeAndDeleteDatabasesModifiedSince(modificationTime);
    85         postTaskReply(CrossThreadTask([callback = WTFMove(callback)]() mutable {
     89        postTaskReply(CrossThreadTask([callback = WTFMove(callback), token = WTFMove(token)]() mutable {
    8690            callback();
    8791        }));
    … …  
    9397    ASSERT(RunLoop::isMain());
    9498
    95     postTask([this, protectedThis = makeRef(*this), originDatas = originDatas.isolatedCopy(), callback = WTFMove(callback)] () mutable {
     99    postTask([this, protectedThis = makeRef(*this), originDatas = originDatas.isolatedCopy(), callback = WTFMove(callback), token = m_dataTaskCounter.count()] () mutable {
    96100        ASSERT(!RunLoop::isMain());
    97101
    98102        LockHolder locker(m_server->lock());
    99103        m_server->closeAndDeleteDatabasesForOrigins(originDatas);
    100         postTaskReply(CrossThreadTask([callback = WTFMove(callback)]() mutable {
     104        postTaskReply(CrossThreadTask([callback = WTFMove(callback), token = WTFMove(token)]() mutable {
    101105            callback();
    102106        }));
    … …  
    108112    ASSERT(RunLoop::isMain());
    109113
    110     postTask([this, protectedThis = makeRef(*this), oldOrigin = oldOrigin.isolatedCopy(), newOrigin = newOrigin.isolatedCopy(), callback = WTFMove(callback)] () mutable {
     114    postTask([this, protectedThis = makeRef(*this), oldOrigin = oldOrigin.isolatedCopy(), newOrigin = newOrigin.isolatedCopy(), callback = WTFMove(callback), token = m_dataTaskCounter.count()] () mutable {
    111115        ASSERT(!RunLoop::isMain());
    112116
    113117        LockHolder locker(m_server->lock());
    114118        m_server->renameOrigin(oldOrigin, newOrigin);
    115         postTaskReply(CrossThreadTask(WTFMove(callback)));
     119        postTaskReply(CrossThreadTask([callback = WTFMove(callback), token = WTFMove(token)]() mutable {
     120            callback();
     121        }));
    116122    });
    117123}
    … …  
    373379    ASSERT(RunLoop::isMain());
    374380
    375     m_connections.remove(&connection);
    376     connection.removeThreadMessageReceiver(Messages::WebIDBServer::messageReceiverName());
     381    auto* takenConnection = m_connections.take(&connection);
     382    if (!takenConnection)
     383        return;
     384
     385    takenConnection->removeThreadMessageReceiver(Messages::WebIDBServer::messageReceiverName());
    377386    postTask([this, protectedThis = makeRef(*this), connectionID = connection.uniqueID()] {
    378387        auto connection = m_connectionMap.take(connectionID);
    … …  
    383392        m_server->unregisterConnection(connection->connectionToClient());
    384393    });
     394
     395    tryClose();
    385396}
    386397
    … …  
    400411{
    401412    ASSERT(RunLoop::isMain());
     413    if (!m_closeCallback)
     414        return;
    402415
    403416    // Remove the references held by IPC::Connection.
    … …  
    416429        CrossThreadTaskHandler::kill();
    417430    });
     431
     432    m_closeCallback();
     433}
     434
     435void WebIDBServer::tryClose()
     436{
     437    if (!m_connections.isEmpty() || m_dataTaskCounter.value())
     438        return;
     439
     440    close();
    418441}
    419442

  • branches/safari-611-branch/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.h

    r276555 r276556  
    3434#include <WebCore/StorageQuotaManager.h>
    3535#include <wtf/CrossThreadTaskHandler.h>
     36#include <wtf/RefCounter.h>
    3637
    3738namespace WebCore {
    … …  
    4647class WebIDBServer final : public CrossThreadTaskHandler, public IPC::Connection::ThreadMessageReceiverRefCounted {
    4748public:
    48     static Ref<WebIDBServer> create(PAL::SessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&&);
     49    static Ref<WebIDBServer> create(PAL::SessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&&, CompletionHandler<void()>&&);
    4950
    5051    void getOrigins(CompletionHandler<void(HashSet<WebCore::SecurityOriginData>&&)>&&);
    … …  
    9192    void close();
    9293
    93     bool hasConnection() const { return !m_connections.isEmpty(); }
    9494private:
    95     WebIDBServer(PAL::SessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&&);
     95    WebIDBServer(PAL::SessionID, const String& directory, WebCore::IDBServer::IDBServer::StorageQuotaManagerSpaceRequester&&, CompletionHandler<void()>&&);
    9696    ~WebIDBServer();
    9797
    9898    void postTask(WTF::Function<void()>&&);
     99
     100    void tryClose();
    99101
    100102    std::unique_ptr<WebCore::IDBServer::IDBServer> m_server;
    … …  
    103105    HashMap<IPC::Connection::UniqueID, std::unique_ptr<WebIDBConnectionToClient>> m_connectionMap;
    104106    HashSet<IPC::Connection*> m_connections;
     107
     108    enum DataTaskCounterType { };
     109    using DataTaskCounter = RefCounter<DataTaskCounterType>;
     110    using DataTaskCounterToken = DataTaskCounter::Token;
     111    DataTaskCounter m_dataTaskCounter;
     112    CompletionHandler<void()> m_closeCallback;
    105113};
    106114

  • branches/safari-611-branch/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp

    r276555 r276556  
    11711171}
    11721172
     1173void NetworkConnectionToWebProcess::addIDBConnection()
     1174{
     1175    m_networkProcess->webIDBServer(m_sessionID).addConnection(m_connection.get(), m_webProcessIdentifier);
     1176}
     1177
    11731178} // namespace WebKit
    11741179

  • branches/safari-611-branch/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h

    r276555 r276556  
    177177    void broadcastConsoleMessage(JSC::MessageSource, JSC::MessageLevel, const String& message);
    178178
     179    void addIDBConnection();
     180
    179181private:
    180182    NetworkConnectionToWebProcess(NetworkProcess&, WebCore::ProcessIdentifier, PAL::SessionID, IPC::Connection::Identifier);

  • branches/safari-611-branch/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in

    r276555 r276556  
    102102    UpdateActivePages(String name, Vector<String> activePagesOrigins, audit_token_t auditToken)
    103103#endif
     104
     105    AddIDBConnection()
    104106}

  • branches/safari-611-branch/Source/WebKit/NetworkProcess/NetworkProcess.cpp

    r276555 r276556  
    268268        platformFlushCookies(networkSession.sessionID(), [callbackAggregator] { });
    269269    });
     270
     271    // Make sure references to NetworkProcess in spaceRequester and closeHandler is removed.
     272    for (auto& server : m_webIDBServers.values())
     273        server->close();
    270274}
    271275
    … …  
    384388
    385389    m_storageManagerSet->addConnection(connection.connection());
    386 
    387 #if ENABLE(INDEXED_DATABASE)
    388     webIDBServer(sessionID).addConnection(connection.connection(), identifier);
    389 #endif
    390390}
    391391
    … …  
    548548    m_storageManagerSet->remove(sessionID);
    549549
    550 #if ENABLE(INDEXED_DATABASE)
    551     removeWebIDBServerIfPossible(sessionID);
    552 #endif
    553550}
    554551
    … …  
    23272324    }
    23282325
    2329     return WebIDBServer::create(sessionID, path, [this, weakThis = makeWeakPtr(this), sessionID](const auto& origin, uint64_t spaceRequested) {
    2330         RefPtr<StorageQuotaManager> storageQuotaManager = weakThis ? this->storageQuotaManager(sessionID, origin) : nullptr;
    2331         return storageQuotaManager ? storageQuotaManager->requestSpaceOnBackgroundThread(spaceRequested) : StorageQuotaManager::Decision::Deny;
    2332     });
     2326    auto spaceRequester = [protectedThis = makeRef(*this), sessionID](const auto& origin, uint64_t spaceRequested) {
     2327        return protectedThis->storageQuotaManager(sessionID, origin)->requestSpaceOnBackgroundThread(spaceRequested);
     2328    };
     2329    auto closeHandler = [protectedThis = makeRef(*this), sessionID]() {
     2330        protectedThis->m_webIDBServers.remove(sessionID);
     2331    };
     2332    return WebIDBServer::create(sessionID, path, WTFMove(spaceRequester), WTFMove(closeHandler));
    23332333}
    23342334
    … …  
    23592359    sessionStorageQuotaManager->setIDBRootPath(idbRootPath);
    23602360}
    2361 
    2362 void NetworkProcess::removeWebIDBServerIfPossible(PAL::SessionID sessionID)
    2363 {
    2364     ASSERT(RunLoop::isMain());
    2365 
    2366     auto iterator = m_webIDBServers.find(sessionID);
    2367     if (iterator == m_webIDBServers.end())
    2368         return;
    2369 
    2370     if (m_networkSessions.contains(sessionID))
    2371         return;
    2372 
    2373     if (iterator->value->hasConnection())
    2374         return;
    2375 
    2376     iterator->value->close();
    2377     m_webIDBServers.remove(iterator);
    2378 }
    2379 
    23802361#endif // ENABLE(INDEXED_DATABASE)
    23812362
    … …  
    26522633
    26532634#if ENABLE(INDEXED_DATABASE)
    2654     auto* webIDBServer = m_webIDBServers.get(sessionID);
    2655     ASSERT(webIDBServer);
    2656     webIDBServer->removeConnection(connection);
    2657     removeWebIDBServerIfPossible(sessionID);
     2635    if (auto* server = m_webIDBServers.get(sessionID))
     2636        server->removeConnection(connection);
    26582637#endif
    26592638}

  • branches/safari-611-branch/Source/WebKit/WebProcess/Databases/IndexedDB/WebIDBConnectionToServer.cpp

    r276555 r276556  
    6262    : m_connectionToServer(IDBClient::IDBConnectionToServer::create(*this))
    6363{
     64    send(Messages::NetworkConnectionToWebProcess::AddIDBConnection());
    6465}
    6566
Note: See TracChangeset for help on using the changeset viewer.