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..c953d5dbe450b8c7fe911efb8af8ee346e4b4baf 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)
+{
+    // FIXME: Construct correct DOM error objects rather than returning strings.
+    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));
+    }
+
+    virtual void onError(WebServiceWorkerCacheError* reason) OVERRIDE
+    {
+        m_resolver->reject(cacheErrorToString(*reason));
+    }
+
+private:
+    const RefPtr<ScriptPromiseResolver> m_resolver;
+};
+
+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,87 @@ ScriptPromise rejectAsNotImplemented(ScriptState* scriptState)
     return promise;
 }
 
-}
+} // namespace
 
 PassRefPtrWillBeRawPtr<Cache> Cache::fromWebServiceWorkerCache(WebServiceWorkerCache* webCache)
 {
+    // This management mechanism insures that we get the same Cache* object for every WebServiceWorkerCache object, so the

[dominicc (has gone to gerrit), 2014/08/11 01:40:41]

ensures

[gavinp, 2014/08/13 23:25:22]

Done.

+    // == 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.
+    if (Cache* cache = static_cast<Cache*>(webCache->proxyInterface()))
+        return cache;
     return adoptRefWillBeNoop(new Cache(webCache));

[dominicc (has gone to gerrit), 2014/08/11 01:40:41]

I'd add Cache::create; it's typical Blink, more readable here.

[gavinp, 2014/08/13 23:25:22]

Done.

 }
 
-// FIXME: Implement these methods.
-ScriptPromise Cache::match(ScriptState* scriptState, Request* request, const Dictionary& queryParams)
+Cache::~Cache()
 {
-    return rejectAsNotImplemented(scriptState);
+    m_webCache->notifyDone();
 }
 
-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/11 01:40:41]

Why? I think the spec just says to reject the Promise:

https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#cache-match

Ditto below.

[dominicc (has gone to gerrit), 2014/08/14 02:35:12]

Ping.

[gavinp, 2014/08/14 22:34:07]

Yup, you're right. This is pretty tricky； matchAll() actually behaves
differently.

The [[QueryCache]] algorithm says to rethrow exceptions if the promise
constructor throws, see (4) in
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#query-cache-...
. However, in the case of match,
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#cache-match ,
(3) says to reject with the exception.

Looking at matchAll() through, there's no discussion of catching and rejecting,
so I think we have to rethrow. Likewise for delete(), keys(), put().

This is pretty weird. I'll ping Jake to find out how intentional it is. I'll do
this in a later upload, my next upload just deals with the easy stuff.

+        return ScriptPromise::reject(scriptState, V8ThrowException::createTypeError(exceptionState.message(), scriptState->isolate()));
+    }
+    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 +203,159 @@ 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* request, const Dictionary& queryParams)
+ScriptPromise Cache::keys(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 keysImpl(scriptState);
 }
 
-ScriptPromise Cache::keys(ScriptState* scriptState, const String& requestString, const Dictionary& queryParams)
+ScriptPromise Cache::keys(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 keysImpl(scriptState, request, queryParamsDict);
 }
 
 Cache::Cache(WebServiceWorkerCache* webCache) : m_webCache(webCache)
 {
     ScriptWrappable::init(this);
+    ASSERT(m_webCache->proxyInterface() == 0);
+    m_webCache->setProxyInterface(this);

[dominicc (has gone to gerrit), 2014/08/11 01:40:41]

Who owns m_webCache?

[gavinp, 2014/08/13 23:25:22]

I've cleared this up a lot in the newest upload; I wasn't totally sure earlier.
It's owned by the Cache object. There's some comments in the WebServiceWorker*
files, do you think those are adequate?

[dominicc (has gone to gerrit), 2014/08/14 02:35:12]

I commented over there first.

To me, the embedder = Chrome, so that comment doesn't jive with this; it is
Blink that owns the WebServiceWorkerCache and not the embedder.

How do caches get removed? What happens if one page is holding onto a Cache when
that happens from another page?

[gavinp, 2014/08/14 22:34:07]

Yeah, that comment was bad, it's gone now.

So regarding caches and multiple pages, we're fine. Each service worker is in
its own service worker scope, so that's a unique arena for these cache objects.
I think we'll be fine if we add IPC on a per-scope basis announcing the end of
using a cache in that scope, which is called from the destructor. WDYT?

+}
+
+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->dispatchMatch(new CacheMatchCallbacks(resolver), webRequest, queryParamsFromDictionary(queryParamsDict));

[dominicc (has gone to gerrit), 2014/08/11 01:40:41]

Match all?

Since the signatures are the same does it make sense to have a parameter for the
cardinality?

[gavinp, 2014/08/13 23:25:22]

The return value is different, although kind of trivially (a Response vs an
array of them). But so many of these functions have nearly identical signatures;
I don't like combining them where behaviour is different, because the untwisting
on the other end gets complicated.

For instance, Match is a lower latency function than MatchAll, since the
underlying implementation doesn't have to iterate through all possible matches
once it has one. So that's pretty different.

Reasonable?

[dominicc (has gone to gerrit), 2014/08/14 02:35:12]

+1, yes, separate is simpler; simpler is better.

If the return types are different presumably mixing up the methods will usually
result in compile errors relating to the return type.

Except when that return type is some untyped container like a Promise.

Seriously though... this method is called match*All*Impl yet it dispatches just
"match". What's up with that? I'm having a hard time seeing which characters
differ from matchImpl directly above this one.

[gavinp, 2014/08/14 22:34:07]

That was a cut and paste mistake... I have now fixed it. Thanks. And, yeah,
these promise returns make static typing very confusing.

+    return promise;
+}
+
+ScriptPromise Cache::addImpl(ScriptState* scriptState, PassRefPtrWillBeRawPtr<Request>)
+{
+    return rejectAsNotImplemented(scriptState);
+}
+
+ScriptPromise Cache::addAllImpl(ScriptState* scriptState, Vector<RefPtrWillBeRawPtr<Request> >)
+{
+    return rejectAsNotImplemented(scriptState);
+}
+
+ScriptPromise Cache::deleteImpl(ScriptState* scriptState, PassRefPtrWillBeRawPtr<Request> request, const Dictionary& queryParamsDict)
+{
+    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