Introducing the WebServiceWorkerCache.

Source/modules/serviceworkers/Cache.cpp

Index: Source/modules/serviceworkers/Cache.cpp
diff --git a/Source/modules/serviceworkers/Cache.cpp b/Source/modules/serviceworkers/Cache.cpp
index 50e073fb4a83182c3fee7225023b0dcf303f6e68..771835d115a002f21ae642331d14c063dd4899cc 100644
--- a/Source/modules/serviceworkers/Cache.cpp
+++ b/Source/modules/serviceworkers/Cache.cpp
@@ -5,13 +5,107 @@
 #include "config.h"
 #include "modules/serviceworkers/Cache.h"
 
+#include "bindings/core/v8/Dictionary.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "bindings/core/v8/ScriptState.h"
+#include "modules/serviceworkers/Request.h"
+#include "modules/serviceworkers/Response.h"
+#include "public/platform/WebServiceWorkerCache.h"
 
 namespace blink {
 
 namespace {
 
+const char* cacheErrorToString(WebServiceWorkerCacheError reason)

[dominicc (has gone to gerrit), 2014/08/15 06:20:23]

Why do all of this in the blank namespace?

[gavinp, 2014/08/19 21:09:36]

It really should be moved out to a util module, together with fixing the code in
CacheStorage. See comment below.

+{
+    // FIXME: Construct correct DOM error objects rather than returning strings.

[dominicc (has gone to gerrit), 2014/08/15 06:20:23]

Do this. The spec says error objects and not strings, right?

[gavinp, 2014/08/19 21:09:36]

This is a copy of a method currently used in CacheStorage.cpp; so that
redundancy should probably be addressed as well at the same time as fixing this.
Rather than bloat this CL I'd rather land this in a downstream. 

+    switch (reason) {
+    case WebServiceWorkerCacheError::WebServiceWorkerCacheErrorNotImplemented:
+        return "not implemented";
+    case WebServiceWorkerCacheError::WebServiceWorkerCacheErrorNotFound:
+        return "not found";
+    case WebServiceWorkerCacheError::WebServiceWorkerCacheErrorExists:
+        return "entry already exists";
+    default:
+        ASSERT_NOT_REACHED();
+        return "unknown error";
+    }
+}
+
+WebServiceWorkerCache::QueryParams queryParamsFromDictionary(const Dictionary& dictionary)
+{
+    WebServiceWorkerCache::QueryParams queryParams;
+    DictionaryHelper::get(dictionary, "ignoreSearch", queryParams.ignoreSearch);
+    DictionaryHelper::get(dictionary, "ignoreMethod", queryParams.ignoreMethod);
+    DictionaryHelper::get(dictionary, "ignoreVary", queryParams.ignoreVary);
+    DictionaryHelper::get(dictionary, "prefixMatch", queryParams.prefixMatch);
+    // FIXME: Add cacheName.
+    return queryParams;
+}
+
+class CacheMatchCallbacks : public WebServiceWorkerCache::CacheMatchCallbacks {
+    WTF_MAKE_NONCOPYABLE(CacheMatchCallbacks);
+public:
+    CacheMatchCallbacks(PassRefPtr<ScriptPromiseResolver> resolver) : m_resolver(resolver) { }
+
+    virtual void onSuccess(WebServiceWorkerResponse* webResponse) OVERRIDE
+    {
+        m_resolver->resolve(Response::create(*webResponse));

[dominicc (has gone to gerrit), 2014/08/15 06:20:23]

Why not reset the pointer after this? It will detect a potential bug--multiple
resolutions--earlier. Also might ameliorate the leak if CacheMatchCallbacks is
leaked. Ditto below.

[gavinp, 2014/08/19 21:09:36]

Good idea.

+    }
+
+    virtual void onError(WebServiceWorkerCacheError* reason) OVERRIDE
+    {
+        m_resolver->reject(cacheErrorToString(*reason));
+    }
+
+private:
+    const RefPtr<ScriptPromiseResolver> m_resolver;

[dominicc (has gone to gerrit), 2014/08/15 06:20:23]

Why const?

[gavinp, 2014/08/19 21:09:36]

Up until we reset, it was never modified during the object lifetime, so const
seemed a good way to convey it. But with the reset, it's not const any more, so
this is moot.

+};
+
+class CacheWithResponsesCallbacks : public WebServiceWorkerCache::CacheWithResponsesCallbacks {
+    WTF_MAKE_NONCOPYABLE(CacheWithResponsesCallbacks);
+public:
+    CacheWithResponsesCallbacks(PassRefPtr<ScriptPromiseResolver> resolver) : m_resolver(resolver) { }
+
+    virtual void onSuccess(WebVector<WebServiceWorkerResponse>* webResponses) OVERRIDE
+    {
+        Vector<RefPtrWillBeRawPtr<Response> > responses;
+        for (size_t i = 0; i < webResponses->size(); ++i)
+            responses.append(Response::create((*webResponses)[i]));
+        m_resolver->resolve(responses);
+    }
+
+    virtual void onError(WebServiceWorkerCacheError* reason) OVERRIDE
+    {
+        m_resolver->reject(cacheErrorToString(*reason));
+    }
+
+private:
+    const RefPtr<ScriptPromiseResolver> m_resolver;
+};
+
+class CacheWithRequestsCallbacks : public WebServiceWorkerCache::CacheWithRequestsCallbacks {
+    WTF_MAKE_NONCOPYABLE(CacheWithRequestsCallbacks);
+public:
+    CacheWithRequestsCallbacks(PassRefPtr<ScriptPromiseResolver> resolver) : m_resolver(resolver) { }
+
+    virtual void onSuccess(WebVector<WebServiceWorkerRequest>* webRequests) OVERRIDE
+    {
+        Vector<RefPtrWillBeRawPtr<Request> > requests;
+        for (size_t i = 0; i < webRequests->size(); ++i)
+            requests.append(Request::create((*webRequests)[i]));
+        m_resolver->resolve(requests);
+    }
+
+    virtual void onError(WebServiceWorkerCacheError* reason) OVERRIDE
+    {
+        m_resolver->reject(cacheErrorToString(*reason));
+    }
+
+private:
+    const RefPtr<ScriptPromiseResolver> m_resolver;
+};
+
 ScriptPromise rejectAsNotImplemented(ScriptState* scriptState)
 {
     RefPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
@@ -21,42 +115,86 @@ ScriptPromise rejectAsNotImplemented(ScriptState* scriptState)
     return promise;
 }
 
-}
+} // namespace
 
 PassRefPtrWillBeRawPtr<Cache> Cache::fromWebServiceWorkerCache(WebServiceWorkerCache* webCache)
 {
-    return adoptRefWillBeNoop(new Cache(webCache));
+    // This management mechanism ensures that we get the same Cache* object for every WebServiceWorkerCache object, so the

[dominicc (has gone to gerrit), 2014/08/15 06:20:23]

Can this be tightened up? Something like:

Ensure that we get the same Cache* for the same WebServiceWorkerCache object, to
make == work in JavaScript. The raw pointer back to the blink::Cache is reset in
the destructor.

[gavinp, 2014/08/19 21:09:37]

I've just removed this comment, it was recapitulating the discussion in the
interface.

+    // == operator in Javascript will work correctly. The lifetime is safe despite this WebServiceWorkerCache not adding a ref, since
+    // in our destructor we call WebServiceWorkerCache::notifyDone(), registering our destruction.

[dominicc (has gone to gerrit), 2014/08/15 06:20:23]

I'm worried about how the object equality works.

Aren't caches per-origin? What happens when one cache is exposed to two
different workers in the same renderer?

[gavinp, 2014/08/19 21:09:36]

It turns out, in the content side version of this CL, we're actually quite
careful to only have one WebServiceWorkerCache object per cache_id, but that's
on a _per ServiceWorker_ basis, so two different ServiceWorkers in the same
renderer will have difference WebServiceWorkerCache objects and thus different
Cache objects, so we're safe.

+    if (Cache* cache = static_cast<Cache*>(webCache->proxyInterface()))
+        return cache;
+    return create(webCache);
 }
 
-// FIXME: Implement these methods.
-ScriptPromise Cache::match(ScriptState* scriptState, Request* request, const Dictionary& queryParams)
+Cache::~Cache()

[dominicc (has gone to gerrit), 2014/08/15 06:20:23]

Line 124 suggests this calls notifyDone; what am I missing?

[gavinp, 2014/08/19 21:09:36]

Vestigal comment, removed. It was just a recapitulation anyway. There's no more
notifyDone() since we use the destructor.

 {
-    return rejectAsNotImplemented(scriptState);
 }
 
-ScriptPromise Cache::match(ScriptState* scriptState, const String& requestString, const Dictionary& queryParams)
+ScriptPromise Cache::match(ScriptState* scriptState, Request* originalRequest, const Dictionary& queryParamsDict)
 {
-    return rejectAsNotImplemented(scriptState);
+    TrackExceptionState exceptionState;
+    RefPtrWillBeRawPtr<Request> request = Request::create(scriptState->executionContext(), originalRequest, exceptionState);
+    if (exceptionState.hadException()) {
+        // FIXME: We should throw the caught error.

[dominicc (has gone to gerrit), 2014/08/15 06:20:23]

Why should you throw it? I don't see anywhere it says to do this...

[gavinp, 2014/08/19 21:09:36]

Good catch, that's true for match().

Almost all of the other functions on Cache do throw exceptions...

+        return ScriptPromise::reject(scriptState, V8ThrowException::createTypeError(exceptionState.message(), scriptState->isolate()));

[dominicc (has gone to gerrit), 2014/08/15 06:20:23]

Why type error?

[gavinp, 2014/08/19 21:09:36]

Earlier revision of the spec perhaps? Now fixed.

+    }
+    return matchImpl(scriptState, request, queryParamsDict);
 }
 
-ScriptPromise Cache::matchAll(ScriptState* scriptState, Request* request, const Dictionary& queryParams)
+ScriptPromise Cache::match(ScriptState* scriptState, const String& requestString, const Dictionary& queryParamsDict)
 {
-    return rejectAsNotImplemented(scriptState);
+    TrackExceptionState exceptionState;
+    RefPtrWillBeRawPtr<Request> request = Request::create(scriptState->executionContext(), requestString, exceptionState);
+    if (exceptionState.hadException()) {
+        // FIXME: We should throw the caught error.
+        return ScriptPromise::reject(scriptState, V8ThrowException::createTypeError(exceptionState.message(), scriptState->isolate()));
+    }
+    return matchImpl(scriptState, request, queryParamsDict);
 }
 
-ScriptPromise Cache::matchAll(ScriptState* scriptState, const String& requestString, const Dictionary& queryParams)
+ScriptPromise Cache::matchAll(ScriptState* scriptState, Request* originalRequest, const Dictionary& queryParamsDict)
 {
-    return rejectAsNotImplemented(scriptState);
+    TrackExceptionState exceptionState;
+    RefPtrWillBeRawPtr<Request> request = Request::create(scriptState->executionContext(), originalRequest, exceptionState);
+    if (exceptionState.hadException()) {
+        // FIXME: We should throw the caught error.
+        return ScriptPromise::reject(scriptState, V8ThrowException::createTypeError(exceptionState.message(), scriptState->isolate()));
+    }
+    return matchAllImpl(scriptState, request, queryParamsDict);
 }
 
-ScriptPromise Cache::add(ScriptState* scriptState, Request* request)
+ScriptPromise Cache::matchAll(ScriptState* scriptState, const String& requestString, const Dictionary& queryParamsDict)
 {
-    return rejectAsNotImplemented(scriptState);
+    TrackExceptionState exceptionState;
+    RefPtrWillBeRawPtr<Request> request = Request::create(scriptState->executionContext(), requestString, exceptionState);
+    if (exceptionState.hadException()) {
+        // FIXME: We should throw the caught error.
+        return ScriptPromise::reject(scriptState, V8ThrowException::createTypeError(exceptionState.message(), scriptState->isolate()));
+    }
+    return matchAllImpl(scriptState, request, queryParamsDict);
+}
+
+ScriptPromise Cache::add(ScriptState* scriptState, Request* originalRequest)
+{
+    TrackExceptionState exceptionState;
+    RefPtrWillBeRawPtr<Request> request = Request::create(scriptState->executionContext(), originalRequest, exceptionState);
+    if (exceptionState.hadException()) {
+        // FIXME: We should throw the caught error.
+        return ScriptPromise::reject(scriptState, V8ThrowException::createTypeError(exceptionState.message(), scriptState->isolate()));
+    }
+    return addImpl(scriptState, request);
 }
 
 ScriptPromise Cache::add(ScriptState* scriptState, const String& requestString)
 {
-    return rejectAsNotImplemented(scriptState);
+    TrackExceptionState exceptionState;
+    RefPtrWillBeRawPtr<Request> request = Request::create(scriptState->executionContext(), requestString, exceptionState);
+    if (exceptionState.hadException()) {
+        // FIXME: We should throw the caught error.
+        return ScriptPromise::reject(scriptState, V8ThrowException::createTypeError(exceptionState.message(), scriptState->isolate()));
+    }
+    return addImpl(scriptState, request);
 }
 
 ScriptPromise Cache::addAll(ScriptState* scriptState, const WillBeHeapVector<ScriptValue>& rawRequests)
@@ -64,44 +202,164 @@ ScriptPromise Cache::addAll(ScriptState* scriptState, const WillBeHeapVector<Scr
     return rejectAsNotImplemented(scriptState);
 }
 
-ScriptPromise Cache::deleteFunction(ScriptState* scriptState, Request* request, const Dictionary& queryParams)
+ScriptPromise Cache::deleteFunction(ScriptState* scriptState, Request* originalRequest, const Dictionary& queryParamsDict)
 {
-    return rejectAsNotImplemented(scriptState);
+    TrackExceptionState exceptionState;
+    RefPtrWillBeRawPtr<Request> request = Request::create(scriptState->executionContext(), originalRequest, exceptionState);
+    if (exceptionState.hadException()) {
+        // FIXME: We should throw the caught error.
+        return ScriptPromise::reject(scriptState, V8ThrowException::createTypeError(exceptionState.message(), scriptState->isolate()));
+    }
+    return deleteImpl(scriptState, request, queryParamsDict);
 }
 
-ScriptPromise Cache::deleteFunction(ScriptState* scriptState, const String& requestString, const Dictionary& queryParams)
+ScriptPromise Cache::deleteFunction(ScriptState* scriptState, const String& requestString, const Dictionary& queryParamsDict)
 {
-    return rejectAsNotImplemented(scriptState);
+    TrackExceptionState exceptionState;
+    RefPtrWillBeRawPtr<Request> request = Request::create(scriptState->executionContext(), requestString, exceptionState);
+    if (exceptionState.hadException()) {
+        // FIXME: We should throw the caught error.
+        return ScriptPromise::reject(scriptState, V8ThrowException::createTypeError(exceptionState.message(), scriptState->isolate()));
+    }
+    return deleteImpl(scriptState, request, queryParamsDict);
 }
 
-ScriptPromise Cache::put(ScriptState* scriptState, Request* request, Response*)
+ScriptPromise Cache::put(ScriptState* scriptState, Request* originalRequest, Response* response)
 {
-    return rejectAsNotImplemented(scriptState);
+    TrackExceptionState exceptionState;
+    RefPtrWillBeRawPtr<Request> request = Request::create(scriptState->executionContext(), originalRequest, exceptionState);
+    if (exceptionState.hadException()) {
+        // FIXME: We should throw the caught error.
+        return ScriptPromise::reject(scriptState, V8ThrowException::createTypeError(exceptionState.message(), scriptState->isolate()));
+    }
+    return putImpl(scriptState, request, response);
 }
 
-ScriptPromise Cache::put(ScriptState* scriptState, const String& requestString, Response*)
+ScriptPromise Cache::put(ScriptState* scriptState, const String& requestString, Response* response)
 {
-    return rejectAsNotImplemented(scriptState);
+    TrackExceptionState exceptionState;
+    RefPtrWillBeRawPtr<Request> request = Request::create(scriptState->executionContext(), requestString, exceptionState);
+    if (exceptionState.hadException()) {
+        // FIXME: We should throw the caught error.
+        return ScriptPromise::reject(scriptState, V8ThrowException::createTypeError(exceptionState.message(), scriptState->isolate()));
+    }
+    return putImpl(scriptState, request, response);
 }
 
 ScriptPromise Cache::keys(ScriptState* scriptState)
 {
-    return rejectAsNotImplemented(scriptState);
+    return keysImpl(scriptState, RefPtrWillBeRawPtr<Request>(), Dictionary());
+}
+
+ScriptPromise Cache::keys(ScriptState* scriptState, Request* originalRequest, const Dictionary& queryParamsDict)
+{
+    TrackExceptionState exceptionState;
+    RefPtrWillBeRawPtr<Request> request = Request::create(scriptState->executionContext(), originalRequest, exceptionState);
+    if (exceptionState.hadException()) {
+        // FIXME: We should throw the caught error.
+        return ScriptPromise::reject(scriptState, V8ThrowException::createTypeError(exceptionState.message(), scriptState->isolate()));
+    }
+    return keysImpl(scriptState);
+}
+
+ScriptPromise Cache::keys(ScriptState* scriptState, const String& requestString, const Dictionary& queryParamsDict)
+{
+    TrackExceptionState exceptionState;
+    RefPtrWillBeRawPtr<Request> request = Request::create(scriptState->executionContext(), requestString, exceptionState);
+    if (exceptionState.hadException()) {
+        // FIXME: We should throw the caught error.
+        return ScriptPromise::reject(scriptState, V8ThrowException::createTypeError(exceptionState.message(), scriptState->isolate()));
+    }
+    return keysImpl(scriptState, request, queryParamsDict);
+}
+
+Cache::Cache(WebServiceWorkerCache* webCache) : m_webCache(adoptPtr(webCache))
+{
+    ScriptWrappable::init(this);
+    ASSERT(m_webCache->proxyInterface() == 0);
+    m_webCache->setProxyInterface(this);
 }
 
-ScriptPromise Cache::keys(ScriptState* scriptState, Request* request, const Dictionary& queryParams)
+PassRefPtrWillBeRawPtr<Cache> Cache::create(WebServiceWorkerCache* webCache)
+{
+    return adoptRefWillBeNoop(new Cache(webCache));
+}
+
+ScriptPromise Cache::matchImpl(ScriptState* scriptState, PassRefPtrWillBeRawPtr<Request> request, const Dictionary& queryParamsDict)
+{
+    WebServiceWorkerRequest webRequest;
+    request->populateWebServiceWorkerRequest(webRequest);
+
+    RefPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
+    const ScriptPromise promise = resolver->promise();
+    m_webCache->dispatchMatch(new CacheMatchCallbacks(resolver), webRequest, queryParamsFromDictionary(queryParamsDict));
+    return promise;
+}
+
+ScriptPromise Cache::matchAllImpl(ScriptState* scriptState, PassRefPtrWillBeRawPtr<Request> request, const Dictionary& queryParamsDict)
+{
+    WebServiceWorkerRequest webRequest;
+    request->populateWebServiceWorkerRequest(webRequest);
+
+    RefPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
+    const ScriptPromise promise = resolver->promise();
+    m_webCache->dispatchMatchAll(new CacheWithResponsesCallbacks(resolver), webRequest, queryParamsFromDictionary(queryParamsDict));
+    return promise;
+}
+
+ScriptPromise Cache::addImpl(ScriptState* scriptState, PassRefPtrWillBeRawPtr<Request>)
 {
     return rejectAsNotImplemented(scriptState);
 }
 
-ScriptPromise Cache::keys(ScriptState* scriptState, const String& requestString, const Dictionary& queryParams)
+ScriptPromise Cache::addAllImpl(ScriptState* scriptState, Vector<RefPtrWillBeRawPtr<Request> >)
 {
     return rejectAsNotImplemented(scriptState);
 }
 
-Cache::Cache(WebServiceWorkerCache* webCache) : m_webCache(webCache)
+ScriptPromise Cache::deleteImpl(ScriptState* scriptState, PassRefPtrWillBeRawPtr<Request> request, const Dictionary& queryParamsDict)
 {
-    ScriptWrappable::init(this);
+    WebVector<WebServiceWorkerCache::BatchOperation> batchOperations(size_t(1));
+    batchOperations[0].operationType = WebServiceWorkerCache::WebServiceWorkerCacheOperationTypeDelete;
+    request->populateWebServiceWorkerRequest(batchOperations[0].request);
+    batchOperations[0].matchParams = queryParamsFromDictionary(queryParamsDict);
+
+    RefPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
+    const ScriptPromise promise = resolver->promise();
+    m_webCache->dispatchBatch(new CacheWithResponsesCallbacks(resolver), batchOperations);
+    return promise;
+}
+
+ScriptPromise Cache::putImpl(ScriptState* scriptState, PassRefPtrWillBeRawPtr<Request> request, Response* response)
+{
+    WebVector<WebServiceWorkerCache::BatchOperation> batchOperations(size_t(1));
+    batchOperations[0].operationType = WebServiceWorkerCache::WebServiceWorkerCacheOperationTypePut;
+    request->populateWebServiceWorkerRequest(batchOperations[0].request);
+    response->populateWebServiceWorkerResponse(batchOperations[0].response);
+
+    RefPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
+    const ScriptPromise promise = resolver->promise();
+    m_webCache->dispatchBatch(new CacheWithResponsesCallbacks(resolver), batchOperations);
+    return promise;
+}
+
+ScriptPromise Cache::keysImpl(ScriptState* scriptState)
+{
+    RefPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
+    const ScriptPromise promise = resolver->promise();
+    m_webCache->dispatchKeys(new CacheWithRequestsCallbacks(resolver), 0, WebServiceWorkerCache::QueryParams());
+    return promise;
+}
+
+ScriptPromise Cache::keysImpl(ScriptState* scriptState, PassRefPtrWillBeRawPtr<Request> request, const Dictionary& queryParamsDict)
+{
+    WebServiceWorkerRequest webRequest;
+    request->populateWebServiceWorkerRequest(webRequest);
+
+    RefPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
+    const ScriptPromise promise = resolver->promise();
+    m_webCache->dispatchKeys(new CacheWithRequestsCallbacks(resolver), 0, WebServiceWorkerCache::QueryParams());
+    return promise;
 }
 
 } // namespace blink