Skip to content

Commit

Permalink
Move all CacheStorageConnection callbacks to NativePromise
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=274726
rdar://128755510

Reviewed by Jean-Yves Avenard and Sihui Liu.

We move from completion handlers to native promise as a simplification.
We introduce a new enqueueTaskWhenSettled version that can take completion handler and a finalizer.
Covered by existing tests.

* Source/WebCore/Modules/cache/CacheStorageConnection.h:
(WebCore::CacheStorageConnection::clearMemoryRepresentation):
(WebCore::CacheStorageConnection::engineRepresentation):
* Source/WebCore/Modules/cache/DOMCache.cpp:
(WebCore::DOMCache::queryCache):
(WebCore::DOMCache::batchDeleteOperation):
(WebCore::DOMCache::batchPutOperation):
* Source/WebCore/Modules/cache/DOMCacheEngine.h:
* Source/WebCore/Modules/cache/DOMCacheStorage.cpp:
(WebCore::DOMCacheStorage::retrieveCaches):
(WebCore::DOMCacheStorage::doOpen):
(WebCore::DOMCacheStorage::doRemove):
* Source/WebCore/Modules/cache/WorkerCacheStorageConnection.cpp:
(WebCore::WorkerCacheStorageConnection::retrieveCaches):
(WebCore::WorkerCacheStorageConnection::retrieveRecords):
(WebCore::WorkerCacheStorageConnection::batchDeleteOperation):
(WebCore::WorkerCacheStorageConnection::batchPutOperation):
(WebCore::WorkerCacheStorageConnection::retrieveCachesCompleted): Deleted.
(WebCore::WorkerCacheStorageConnection::retrieveRecordsCompleted): Deleted.
(WebCore::WorkerCacheStorageConnection::deleteRecordsCompleted): Deleted.
(WebCore::WorkerCacheStorageConnection::putRecordsCompleted): Deleted.
(WebCore::WorkerCacheStorageConnection::clearPendingRequests): Deleted.
* Source/WebCore/Modules/cache/WorkerCacheStorageConnection.h:
* Source/WebCore/dom/ScriptExecutionContext.h:
(WebCore::ScriptExecutionContext::enqueueTaskWhenSettled):
* Source/WebCore/page/CacheStorageProvider.h:
* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::clearCacheStorageMemoryRepresentation):
(WebCore::Internals::cacheStorageEngineRepresentation):
* Source/WebCore/workers/WorkerGlobalScope.cpp:
(WebCore::WorkerGlobalScope::prepareForDestruction):
* Source/WebKit/WebProcess/Cache/WebCacheStorageConnection.cpp:
(WebKit::WebCacheStorageConnection::retrieveCaches):
(WebKit::WebCacheStorageConnection::retrieveRecords):
(WebKit::WebCacheStorageConnection::batchDeleteOperation):
(WebKit::WebCacheStorageConnection::batchPutOperation):
(WebKit::WebCacheStorageConnection::clearMemoryRepresentation):
(WebKit::WebCacheStorageConnection::engineRepresentation):
* Source/WebKit/WebProcess/Cache/WebCacheStorageConnection.h:

Canonical link: https://commits.webkit.org/279505@main
  • Loading branch information
@youennf
youennf committed May 30, 2024
1 parent adc45c7 commit d244968
Show file tree
Hide file tree
Showing 12 changed files with 128 additions and 163 deletions.
18 changes: 12 additions & 6 deletions Source/WebCore/Modules/cache/CacheStorageConnection.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "RetrieveRecordsOptions.h"
#include <wtf/Forward.h>
#include <wtf/HashMap.h>
#include <wtf/NativePromise.h>
#include <wtf/ThreadSafeRefCounted.h>

namespace WebCore {
Expand All @@ -45,11 +46,14 @@ class CacheStorageConnection : public ThreadSafeRefCounted<CacheStorageConnectio
virtual Ref<OpenPromise> open(const ClientOrigin&, const String& cacheName) = 0;
using RemovePromise = NativePromise<bool, DOMCacheEngine::Error>;
virtual Ref<RemovePromise> remove(DOMCacheIdentifier) = 0;
virtual void retrieveCaches(const ClientOrigin&, uint64_t updateCounter, DOMCacheEngine::CacheInfosCallback&&) = 0;
using RetrieveCachesPromise = NativePromise<DOMCacheEngine::CacheInfos, DOMCacheEngine::Error>;
virtual Ref<RetrieveCachesPromise> retrieveCaches(const ClientOrigin&, uint64_t updateCounter) = 0;

virtual void retrieveRecords(DOMCacheIdentifier, RetrieveRecordsOptions&&, DOMCacheEngine::CrossThreadRecordsCallback&&) = 0;
virtual void batchDeleteOperation(DOMCacheIdentifier, const ResourceRequest&, CacheQueryOptions&&, DOMCacheEngine::RecordIdentifiersCallback&&) = 0;
virtual void batchPutOperation(DOMCacheIdentifier, Vector<DOMCacheEngine::CrossThreadRecord>&&, DOMCacheEngine::RecordIdentifiersCallback&&) = 0;
using RetrieveRecordsPromise = NativePromise<Vector<DOMCacheEngine::CrossThreadRecord>, DOMCacheEngine::Error>;
virtual Ref<RetrieveRecordsPromise> retrieveRecords(DOMCacheIdentifier, RetrieveRecordsOptions&&) = 0;
using BatchPromise = NativePromise<Vector<uint64_t>, DOMCacheEngine::Error>;
virtual Ref<BatchPromise> batchDeleteOperation(DOMCacheIdentifier, const ResourceRequest&, CacheQueryOptions&&) = 0;
virtual Ref<BatchPromise> batchPutOperation(DOMCacheIdentifier, Vector<DOMCacheEngine::CrossThreadRecord>&&) = 0;

virtual void reference(DOMCacheIdentifier /* cacheIdentifier */) = 0;
virtual void dereference(DOMCacheIdentifier /* cacheIdentifier */) = 0;
Expand All @@ -59,8 +63,10 @@ class CacheStorageConnection : public ThreadSafeRefCounted<CacheStorageConnectio
uint64_t computeRecordBodySize(const FetchResponse&, const DOMCacheEngine::ResponseBody&);

// Used only for testing purposes.
virtual void clearMemoryRepresentation(const ClientOrigin&, DOMCacheEngine::CompletionCallback&& callback) { callback(DOMCacheEngine::Error::NotImplemented); }
virtual void engineRepresentation(CompletionHandler<void(const String&)>&& callback) { callback(String { }); }
using CompletionPromise = NativePromise<void, DOMCacheEngine::Error>;
virtual Ref<CompletionPromise> clearMemoryRepresentation(const ClientOrigin&) { return CompletionPromise::createAndReject(DOMCacheEngine::Error::NotImplemented); }
using EngineRepresentationPromise = NativePromise<String, DOMCacheEngine::Error>;
virtual Ref<EngineRepresentationPromise> engineRepresentation() { return EngineRepresentationPromise::createAndReject(DOMCacheEngine::Error::NotImplemented); }
virtual void updateQuotaBasedOnSpaceUsage(const ClientOrigin&) { }

private:
Expand Down
38 changes: 31 additions & 7 deletions Source/WebCore/Modules/cache/DOMCache.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -486,14 +486,21 @@ void DOMCache::keys(std::optional<RequestInfo>&& info, CacheQueryOptions&& optio

void DOMCache::queryCache(ResourceRequest&& request, const CacheQueryOptions& options, ShouldRetrieveResponses shouldRetrieveResponses, RecordsCallback&& callback)
{
RefPtr context = scriptExecutionContext();
if (!context) {
callback(DOMCacheEngine::convertToException(DOMCacheEngine::Error::Stopped));
return;
}

RetrieveRecordsOptions retrieveOptions { WTFMove(request), scriptExecutionContext()->crossOriginEmbedderPolicy(), *scriptExecutionContext()->securityOrigin(), options.ignoreSearch, options.ignoreMethod, options.ignoreVary, shouldRetrieveResponses == ShouldRetrieveResponses::Yes };
m_connection->retrieveRecords(m_identifier, WTFMove(retrieveOptions), [this, pendingActivity = makePendingActivity(*this), callback = WTFMove(callback)](auto&& result) mutable {

context->enqueueTaskWhenSettled(m_connection->retrieveRecords(m_identifier, WTFMove(retrieveOptions)), TaskSource::DOMManipulation, [this, pendingActivity = makePendingActivity(*this), callback = WTFMove(callback)] (auto&& result) mutable {
if (m_isStopped) {
callback(DOMCacheEngine::convertToExceptionAndLog(scriptExecutionContext(), DOMCacheEngine::Error::Stopped));
return;
}

if (!result.has_value()) {
if (!result) {
callback(DOMCacheEngine::convertToExceptionAndLog(scriptExecutionContext(), result.error()));
return;
}
Expand All @@ -502,23 +509,32 @@ void DOMCache::queryCache(ResourceRequest&& request, const CacheQueryOptions& op
return fromCrossThreadRecord(WTFMove(record));
});
callback(WTFMove(records));
}, [] (auto&& callback) {
callback(makeUnexpected(DOMCacheEngine::Error::Stopped));
});

}

void DOMCache::batchDeleteOperation(const FetchRequest& request, CacheQueryOptions&& options, CompletionHandler<void(ExceptionOr<bool>&&)>&& callback)
{
m_connection->batchDeleteOperation(m_identifier, request.internalRequest(), WTFMove(options), [this, pendingActivity = makePendingActivity(*this), callback = WTFMove(callback)](RecordIdentifiersOrError&& result) mutable {
RefPtr context = scriptExecutionContext();
if (!context) {
callback(DOMCacheEngine::convertToException(DOMCacheEngine::Error::Stopped));
return;
}

context->enqueueTaskWhenSettled(m_connection->batchDeleteOperation(m_identifier, request.internalRequest(), WTFMove(options)), TaskSource::DOMManipulation, [this, pendingActivity = makePendingActivity(*this), callback = WTFMove(callback)] (auto&& result) mutable {
if (m_isStopped) {
callback(DOMCacheEngine::convertToExceptionAndLog(scriptExecutionContext(), DOMCacheEngine::Error::Stopped));
return;
}

if (!result.has_value()) {
if (!result) {
callback(DOMCacheEngine::convertToExceptionAndLog(scriptExecutionContext(), result.error()));
return;
}
callback(!result.value().isEmpty());
}, [] (auto&& callback) {
callback(makeUnexpected(DOMCacheEngine::Error::Stopped));
});
}

Expand Down Expand Up @@ -551,20 +567,28 @@ void DOMCache::batchPutOperation(const FetchRequest& request, FetchResponse& res

void DOMCache::batchPutOperation(Vector<Record>&& records, CompletionHandler<void(ExceptionOr<void>&&)>&& callback)
{
RefPtr context = scriptExecutionContext();
if (!context) {
callback(DOMCacheEngine::convertToException(DOMCacheEngine::Error::Stopped));
return;
}

auto crossThreadRecords = WTF::map(WTFMove(records), [](Record&& record) {
return toCrossThreadRecord(WTFMove(record));
});
m_connection->batchPutOperation(m_identifier, WTFMove(crossThreadRecords), [this, pendingActivity = makePendingActivity(*this), callback = WTFMove(callback)](auto&& result) mutable {
context->enqueueTaskWhenSettled(m_connection->batchPutOperation(m_identifier, WTFMove(crossThreadRecords)), TaskSource::DOMManipulation, [this, pendingActivity = makePendingActivity(*this), callback = WTFMove(callback)] (auto&& result) mutable {
if (m_isStopped) {
callback(DOMCacheEngine::convertToExceptionAndLog(scriptExecutionContext(), DOMCacheEngine::Error::Stopped));
return;
}

if (!result.has_value()) {
if (!result) {
callback(DOMCacheEngine::convertToExceptionAndLog(scriptExecutionContext(), result.error()));
return;
}
callback({ });
}, [] (auto&& callback) {
callback(makeUnexpected(DOMCacheEngine::Error::Stopped));
});
}

Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/Modules/cache/DOMCacheEngine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ struct CrossThreadRecord {
, responseBodySize(responseBodySize)
{
}
CrossThreadRecord isolatedCopy() &&;
WEBCORE_EXPORT CrossThreadRecord isolatedCopy() &&;

uint64_t identifier;
uint64_t updateResponseCounter;
Expand Down
12 changes: 7 additions & 5 deletions Source/WebCore/Modules/cache/DOMCacheStorage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,13 +178,13 @@ void DOMCacheStorage::retrieveCaches(CompletionHandler<void(std::optional<Except
return;
}

m_connection->retrieveCaches(*origin, m_updateCounter, [this, callback = WTFMove(callback), pendingActivity = makePendingActivity(*this), connectionStorageLock = makeUnique<ConnectionStorageLock>(m_connection.copyRef(), *origin)](DOMCacheEngine::CacheInfosOrError&& result) mutable {
scriptExecutionContext()->enqueueTaskWhenSettled(m_connection->retrieveCaches(*origin, m_updateCounter), TaskSource::DOMManipulation, [this, callback = WTFMove(callback), pendingActivity = makePendingActivity(*this), connectionStorageLock = makeUnique<ConnectionStorageLock>(m_connection.copyRef(), *origin)] (auto&& result) mutable {
if (m_isStopped) {
callback(DOMCacheEngine::convertToException(DOMCacheEngine::Error::Stopped));
return;
}
RefPtr context = scriptExecutionContext();
if (!result.has_value()) {
if (!result) {
callback(DOMCacheEngine::convertToExceptionAndLog(context.get(), result.error()));
return;
}
Expand All @@ -203,6 +203,8 @@ void DOMCacheStorage::retrieveCaches(CompletionHandler<void(std::optional<Except
});
}
callback(std::nullopt);
}, [] (auto&& callback) {
callback(makeUnexpected(DOMCacheEngine::Error::Stopped));
});
}

Expand Down Expand Up @@ -239,9 +241,9 @@ void DOMCacheStorage::doOpen(const String& name, DOMPromiseDeferred<IDLInterface
return;
}

context->enqueueTaskWhenSettled(m_connection->open(*origin(), name), TaskSource::DOMManipulation, [this, name, promise = WTFMove(promise), pendingActivity = makePendingActivity(*this), connectionStorageLock = makeUnique<ConnectionStorageLock>(m_connection.copyRef(), *origin())](const DOMCacheEngine::CacheIdentifierOrError& result) mutable {
context->enqueueTaskWhenSettled(m_connection->open(*origin(), name), TaskSource::DOMManipulation, [this, name, promise = WTFMove(promise), pendingActivity = makePendingActivity(*this), connectionStorageLock = makeUnique<ConnectionStorageLock>(m_connection.copyRef(), *origin())] (auto&& result) mutable {
RefPtr context = scriptExecutionContext();
if (!result.has_value()) {
if (!result) {
promise.reject(DOMCacheEngine::convertToExceptionAndLog(context.get(), result.error()));
return;
}
Expand Down Expand Up @@ -277,7 +279,7 @@ void DOMCacheStorage::doRemove(const String& name, DOMPromiseDeferred<IDLBoolean
}

scriptExecutionContext()->enqueueTaskWhenSettled(m_connection->remove(m_caches[position]->identifier()), TaskSource::DOMManipulation, [this, promise = WTFMove(promise), pendingActivity = makePendingActivity(*this)](const auto& result) mutable {
if (!result.has_value())
if (!result)
promise.reject(DOMCacheEngine::convertToExceptionAndLog(scriptExecutionContext(), result.error()));
else
promise.resolve(result.value());
Expand Down
Loading

0 comments on commit d244968

Please sign in to comment.