Changeset 225481 in webkit


Ignore:
Timestamp:
Dec 4, 2017 10:13:45 AM (6 years ago)
Author:
Chris Dumez
Message:

WorkerCacheStorageConnection should handle the case of terminated workers
​https://bugs.webkit.org/show_bug.cgi?id=180304

Patch by Youenn Fablet <​youenn@apple.com> on 2017-12-04
Reviewed by Chris Dumez.

No web page observable change of behavior.

Reworked WorkerCacheStorageConnection hopping.
Instead of refing/unrefing itself, it refs the worker thread and the main thread connection.
This worker thread is then used on the way back from the main thread.

  • Modules/cache/WorkerCacheStorageConnection.cpp:

(WebCore::WorkerCacheStorageConnection::create):
(WebCore::WorkerCacheStorageConnection::WorkerCacheStorageConnection):
(WebCore::WorkerCacheStorageConnection::doOpen):
(WebCore::WorkerCacheStorageConnection::doRemove):
(WebCore::WorkerCacheStorageConnection::doRetrieveCaches):
(WebCore::WorkerCacheStorageConnection::reference):
(WebCore::WorkerCacheStorageConnection::dereference):
(WebCore::WorkerCacheStorageConnection::doRetrieveRecords):
(WebCore::WorkerCacheStorageConnection::doBatchDeleteOperation):
(WebCore::WorkerCacheStorageConnection::doBatchPutOperation):

  • Modules/cache/WorkerCacheStorageConnection.h:
  • workers/WorkerGlobalScope.cpp:

(WebCore::WorkerGlobalScope::cacheStorageConnection):

  • workers/WorkerGlobalScope.h:
Location:
trunk/Source/WebCore
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r225480 r225481  
     12017-12-04  Youenn Fablet  <youenn@apple.com>
     2
     3        WorkerCacheStorageConnection should handle the case of terminated workers
     4        https://bugs.webkit.org/show_bug.cgi?id=180304
     5
     6        Reviewed by Chris Dumez.
     7
     8        No web page observable change of behavior.
     9
     10        Reworked WorkerCacheStorageConnection hopping.
     11        Instead of refing/unrefing itself, it refs the worker thread and the main thread connection.
     12        This worker thread is then used on the way back from the main thread.
     13
     14        * Modules/cache/WorkerCacheStorageConnection.cpp:
     15        (WebCore::WorkerCacheStorageConnection::create):
     16        (WebCore::WorkerCacheStorageConnection::WorkerCacheStorageConnection):
     17        (WebCore::WorkerCacheStorageConnection::doOpen):
     18        (WebCore::WorkerCacheStorageConnection::doRemove):
     19        (WebCore::WorkerCacheStorageConnection::doRetrieveCaches):
     20        (WebCore::WorkerCacheStorageConnection::reference):
     21        (WebCore::WorkerCacheStorageConnection::dereference):
     22        (WebCore::WorkerCacheStorageConnection::doRetrieveRecords):
     23        (WebCore::WorkerCacheStorageConnection::doBatchDeleteOperation):
     24        (WebCore::WorkerCacheStorageConnection::doBatchPutOperation):
     25        * Modules/cache/WorkerCacheStorageConnection.h:
     26        * workers/WorkerGlobalScope.cpp:
     27        (WebCore::WorkerGlobalScope::cacheStorageConnection):
     28        * workers/WorkerGlobalScope.h:
     29
    1302017-12-04  Frederic Wang  <fwang@igalia.com>
    231

  • trunk/Source/WebCore/Modules/cache/WorkerCacheStorageConnection.cpp

    r223935 r225481  
    9292{
    9393    auto connection = adoptRef(*new WorkerCacheStorageConnection(scope));
    94     callOnMainThread([protectedConnection = connection.copyRef()]() mutable {
    95         ASSERT(isMainThread());
    96         protectedConnection->m_mainThreadConnection = protectedConnection->m_proxy.createCacheStorageConnection();
    97     });
     94    callOnMainThreadAndWait([workerThread = makeRef(scope.thread()), connection = connection.ptr()]() mutable {
     95        connection->m_mainThreadConnection = workerThread->workerLoaderProxy().createCacheStorageConnection();
     96    });
     97    ASSERT(connection->m_mainThreadConnection);
    9898    return connection;
    9999}
    … …  
    101101WorkerCacheStorageConnection::WorkerCacheStorageConnection(WorkerGlobalScope& scope)
    102102    : m_scope(scope)
    103     , m_proxy(m_scope.thread().workerLoaderProxy())
    104     , m_taskMode(WorkerRunLoop::defaultMode().isolatedCopy())
    105103{
    106104}
    … …  
    114112void WorkerCacheStorageConnection::doOpen(uint64_t requestIdentifier, const String& origin, const String& cacheName)
    115113{
    116     callOnMainThread([this, protectedThis = makeRef(*this), requestIdentifier, origin = origin.isolatedCopy(), cacheName = cacheName.isolatedCopy()]() mutable {
    117         ASSERT(isMainThread());
    118         ASSERT(m_mainThreadConnection);
    119 
    120         m_mainThreadConnection->open(origin, cacheName, [this, protectedThis = WTFMove(protectedThis), requestIdentifier](const CacheIdentifierOrError& result) mutable {
    121             m_proxy.postTaskForModeToWorkerGlobalScope([this, protectedThis = WTFMove(protectedThis), requestIdentifier, result](ScriptExecutionContext& context) mutable {
    122                 ASSERT_UNUSED(context, context.isWorkerGlobalScope());
    123                 openCompleted(requestIdentifier, result);
    124             }, m_taskMode);
     114    callOnMainThread([workerThread = makeRef(m_scope.thread()), mainThreadConnection = m_mainThreadConnection, requestIdentifier, origin = origin.isolatedCopy(), cacheName = cacheName.isolatedCopy()] () mutable {
     115        mainThreadConnection->open(origin, cacheName, [workerThread = WTFMove(workerThread), requestIdentifier] (const CacheIdentifierOrError& result) mutable {
     116            workerThread->runLoop().postTaskForMode([requestIdentifier, result] (auto& scope) mutable {
     117                downcast<WorkerGlobalScope>(scope).cacheStorageConnection().openCompleted(requestIdentifier, result);
     118            }, WorkerRunLoop::defaultMode());
    125119        });
    126120    });
    … …  
    129123void WorkerCacheStorageConnection::doRemove(uint64_t requestIdentifier, uint64_t cacheIdentifier)
    130124{
    131     callOnMainThread([this, protectedThis = makeRef(*this), requestIdentifier, cacheIdentifier]() mutable {
    132         ASSERT(isMainThread());
    133         ASSERT(m_mainThreadConnection);
    134 
    135         m_mainThreadConnection->remove(cacheIdentifier, [this, protectedThis = WTFMove(protectedThis), requestIdentifier, cacheIdentifier](const CacheIdentifierOrError& result) mutable {
     125    callOnMainThread([workerThread = makeRef(m_scope.thread()), mainThreadConnection = m_mainThreadConnection, requestIdentifier, cacheIdentifier] () mutable {
     126        mainThreadConnection->remove(cacheIdentifier, [workerThread = WTFMove(workerThread), requestIdentifier, cacheIdentifier] (const CacheIdentifierOrError& result) mutable {
    136127            ASSERT_UNUSED(cacheIdentifier, !result.hasValue() || result.value().identifier == cacheIdentifier);
    137             m_proxy.postTaskForModeToWorkerGlobalScope([this, protectedThis = WTFMove(protectedThis), requestIdentifier, result](ScriptExecutionContext& context) mutable {
    138                 ASSERT_UNUSED(context, context.isWorkerGlobalScope());
    139                 removeCompleted(requestIdentifier, result);
    140             }, m_taskMode);
     128            workerThread->runLoop().postTaskForMode([requestIdentifier, result] (auto& scope) mutable {
     129                downcast<WorkerGlobalScope>(scope).cacheStorageConnection().removeCompleted(requestIdentifier, result);
     130            }, WorkerRunLoop::defaultMode());
    141131        });
    142132    });
    … …  
    145135void WorkerCacheStorageConnection::doRetrieveCaches(uint64_t requestIdentifier, const String& origin, uint64_t updateCounter)
    146136{
    147     callOnMainThread([this, protectedThis = makeRef(*this), requestIdentifier, origin = origin.isolatedCopy(), updateCounter]() mutable {
    148         ASSERT(isMainThread());
    149         ASSERT(m_mainThreadConnection);
    150 
    151         m_mainThreadConnection->retrieveCaches(origin, updateCounter, [this, protectedThis = WTFMove(protectedThis), requestIdentifier](CacheInfosOrError&& result) mutable {
     137    callOnMainThread([workerThread = makeRef(m_scope.thread()), mainThreadConnection = m_mainThreadConnection, requestIdentifier, origin = origin.isolatedCopy(), updateCounter] () mutable {
     138        mainThreadConnection->retrieveCaches(origin, updateCounter, [workerThread = WTFMove(workerThread), requestIdentifier] (CacheInfosOrError&& result) mutable {
    152139            CacheInfosOrError isolatedResult;
    153140            if (!result.hasValue())
    … …  
    156143                isolatedResult = result.value().isolatedCopy();
    157144
    158             m_proxy.postTaskForModeToWorkerGlobalScope([this, protectedThis = WTFMove(protectedThis), requestIdentifier, result = WTFMove(isolatedResult)](ScriptExecutionContext& context) mutable {
    159                 ASSERT_UNUSED(context, context.isWorkerGlobalScope());
    160                 updateCaches(requestIdentifier, WTFMove(result));
    161             }, m_taskMode);
     145            workerThread->runLoop().postTaskForMode([requestIdentifier, result = WTFMove(isolatedResult)] (auto& scope) mutable {
     146                downcast<WorkerGlobalScope>(scope).cacheStorageConnection().updateCaches(requestIdentifier, WTFMove(result));
     147            }, WorkerRunLoop::defaultMode());
    162148        });
    163149    });
    … …  
    166152void WorkerCacheStorageConnection::reference(uint64_t cacheIdentifier)
    167153{
    168     callOnMainThread([this, protectedThis = makeRef(*this), cacheIdentifier]() {
    169         ASSERT(isMainThread());
    170         ASSERT(m_mainThreadConnection);
    171 
    172         m_mainThreadConnection->reference(cacheIdentifier);
     154    callOnMainThread([mainThreadConnection = m_mainThreadConnection, cacheIdentifier]() {
     155        mainThreadConnection->reference(cacheIdentifier);
    173156    });
    174157}
    … …  
    176159void WorkerCacheStorageConnection::dereference(uint64_t cacheIdentifier)
    177160{
    178     callOnMainThread([this, protectedThis = makeRef(*this), cacheIdentifier]() {
    179         ASSERT(isMainThread());
    180         ASSERT(m_mainThreadConnection);
    181 
    182         m_mainThreadConnection->dereference(cacheIdentifier);
     161    callOnMainThread([mainThreadConnection = m_mainThreadConnection, cacheIdentifier]() {
     162        mainThreadConnection->dereference(cacheIdentifier);
    183163    });
    184164}
    … …  
    211191void WorkerCacheStorageConnection::doRetrieveRecords(uint64_t requestIdentifier, uint64_t cacheIdentifier, const URL& url)
    212192{
    213     callOnMainThread([this, protectedThis = makeRef(*this), requestIdentifier, cacheIdentifier, url = url.isolatedCopy()]() mutable {
    214         ASSERT(isMainThread());
    215         ASSERT(m_mainThreadConnection);
    216 
    217         m_mainThreadConnection->retrieveRecords(cacheIdentifier, url, [this, protectedThis = WTFMove(protectedThis), requestIdentifier](RecordsOrError&& result) mutable {
    218             m_proxy.postTaskForModeToWorkerGlobalScope([this, protectedThis = WTFMove(protectedThis), result = recordsDataOrErrorFromRecords(result), requestIdentifier](ScriptExecutionContext& context) mutable {
    219                 ASSERT_UNUSED(context, context.isWorkerGlobalScope());
    220                 updateRecords(requestIdentifier, recordsOrErrorFromRecordsData(WTFMove(result)));
    221             }, m_taskMode);
     193    callOnMainThread([workerThread = makeRef(m_scope.thread()), mainThreadConnection = m_mainThreadConnection, requestIdentifier, cacheIdentifier, url = url.isolatedCopy()]() mutable {
     194        mainThreadConnection->retrieveRecords(cacheIdentifier, url, [workerThread = WTFMove(workerThread), requestIdentifier](RecordsOrError&& result) mutable {
     195            workerThread->runLoop().postTaskForMode([result = recordsDataOrErrorFromRecords(result), requestIdentifier] (auto& scope) mutable {
     196                downcast<WorkerGlobalScope>(scope).cacheStorageConnection().updateRecords(requestIdentifier, recordsOrErrorFromRecordsData(WTFMove(result)));
     197            }, WorkerRunLoop::defaultMode());
    222198        });
    223199    });
    … …  
    226202void WorkerCacheStorageConnection::doBatchDeleteOperation(uint64_t requestIdentifier, uint64_t cacheIdentifier, const ResourceRequest& request, CacheQueryOptions&& options)
    227203{
    228     callOnMainThread([this, protectedThis = makeRef(*this), requestIdentifier, cacheIdentifier, request = request.isolatedCopy(), options = options.isolatedCopy()]() mutable {
    229         ASSERT(isMainThread());
    230         ASSERT(m_mainThreadConnection);
    231 
    232         m_mainThreadConnection->batchDeleteOperation(cacheIdentifier, request, WTFMove(options), [this, protectedThis = WTFMove(protectedThis), requestIdentifier](RecordIdentifiersOrError&& result) mutable {
    233 
    234             m_proxy.postTaskForModeToWorkerGlobalScope([this, protectedThis = WTFMove(protectedThis), requestIdentifier, result = WTFMove(result)](ScriptExecutionContext& context) mutable {
    235                 ASSERT_UNUSED(context, context.isWorkerGlobalScope());
    236                 deleteRecordsCompleted(requestIdentifier, WTFMove(result));
    237             }, m_taskMode);
     204    callOnMainThread([workerThread = makeRef(m_scope.thread()), mainThreadConnection = m_mainThreadConnection, requestIdentifier, cacheIdentifier, request = request.isolatedCopy(), options = options.isolatedCopy()]() mutable {
     205        mainThreadConnection->batchDeleteOperation(cacheIdentifier, request, WTFMove(options), [workerThread = WTFMove(workerThread), requestIdentifier](RecordIdentifiersOrError&& result) mutable {
     206            workerThread->runLoop().postTaskForMode([requestIdentifier, result = WTFMove(result)] (auto& scope) mutable {
     207                downcast<WorkerGlobalScope>(scope).cacheStorageConnection().deleteRecordsCompleted(requestIdentifier, WTFMove(result));
     208            }, WorkerRunLoop::defaultMode());
    238209        });
    239210    });
    … …  
    242213void WorkerCacheStorageConnection::doBatchPutOperation(uint64_t requestIdentifier, uint64_t cacheIdentifier, Vector<Record>&& records)
    243214{
    244     callOnMainThread([this, protectedThis = makeRef(*this), requestIdentifier, cacheIdentifier, recordsData = recordsDataFromRecords(records)]() mutable {
    245         ASSERT(isMainThread());
    246         ASSERT(m_mainThreadConnection);
    247 
    248         m_mainThreadConnection->batchPutOperation(cacheIdentifier, recordsFromRecordsData(WTFMove(recordsData)), [this, protectedThis = WTFMove(protectedThis), requestIdentifier](RecordIdentifiersOrError&& result) mutable {
    249 
    250             m_proxy.postTaskForModeToWorkerGlobalScope([this, protectedThis = WTFMove(protectedThis), requestIdentifier, result = WTFMove(result)](ScriptExecutionContext& context) mutable {
    251                 ASSERT_UNUSED(context, context.isWorkerGlobalScope());
    252                 putRecordsCompleted(requestIdentifier, WTFMove(result));
    253             }, m_taskMode);
     215    callOnMainThread([workerThread = makeRef(m_scope.thread()), mainThreadConnection = m_mainThreadConnection, requestIdentifier, cacheIdentifier, recordsData = recordsDataFromRecords(records)]() mutable {
     216        mainThreadConnection->batchPutOperation(cacheIdentifier, recordsFromRecordsData(WTFMove(recordsData)), [workerThread = WTFMove(workerThread), requestIdentifier] (RecordIdentifiersOrError&& result) mutable {
     217            workerThread->runLoop().postTaskForMode([requestIdentifier, result = WTFMove(result)] (auto& scope) mutable {
     218                downcast<WorkerGlobalScope>(scope).cacheStorageConnection().putRecordsCompleted(requestIdentifier, WTFMove(result));
     219            }, WorkerRunLoop::defaultMode());
    254220        });
    255221    });

  • trunk/Source/WebCore/Modules/cache/WorkerCacheStorageConnection.h

    r221710 r225481  
    4242    explicit WorkerCacheStorageConnection(WorkerGlobalScope&);
    4343
    44     // WebCore::CacheStorageConnection
     44    // WebCore::CacheStorageConnection.
    4545    void doOpen(uint64_t requestIdentifier, const String& origin, const String& cacheName) final;
    4646    void doRemove(uint64_t requestIdentifier, uint64_t cacheIdentifier) final;
    … …  
    5656
    5757    WorkerGlobalScope& m_scope;
    58     WorkerLoaderProxy& m_proxy;
    59     String m_taskMode;
    6058
    6159    RefPtr<CacheStorageConnection> m_mainThreadConnection;

  • trunk/Source/WebCore/workers/WorkerGlobalScope.cpp

    r225470 r225481  
    396396}
    397397
    398 CacheStorageConnection& WorkerGlobalScope::cacheStorageConnection()
     398WorkerCacheStorageConnection& WorkerGlobalScope::cacheStorageConnection()
    399399{
    400400    if (!m_cacheStorageConnection)

  • trunk/Source/WebCore/workers/WorkerGlobalScope.h

    r224788 r225481  
    7272#endif
    7373
    74     CacheStorageConnection& cacheStorageConnection();
     74    WorkerCacheStorageConnection& cacheStorageConnection();
    7575
    7676    WorkerScriptController* script() { return m_script.get(); }
Note: See TracChangeset for help on using the changeset viewer.