CacheStorage: Implement Cache.addAll() behind a flag

CacheStorage: Implement Cache.addAll() behind a flag

Spec: https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#cache-addAll
Intent-to-Ship: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/xGnKs60X33w

BUG=427652
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=198203

[nhiroki, 2015-06-26 03:18:24]

Patchset #1 (id:1) has been deleted

[nhiroki, 2015-06-26 03:18:32]

Patchset #1 (id:20001) has been deleted

[nhiroki, 2015-06-26 03:18:38]

Patchset #1 (id:40001) has been deleted

[nhiroki, 2015-06-26 03:23:30]

Patchset #1 (id:60001) has been deleted

[nhiroki, 2015-06-26 03:23:39]

Patchset #1 (id:80001) has been deleted

[nhiroki, 2015-06-26 03:39:56]

Patchset #1 (id:100001) has been deleted

[nhiroki, 2015-06-26 03:40:03]

Patchset #1 (id:120001) has been deleted

[nhiroki, 2015-06-26 03:54:18]

Patchset #1 (id:140001) has been deleted

[nhiroki, 2015-06-26 03:54:24]

Patchset #1 (id:160001) has been deleted

[nhiroki, 2015-06-26 03:54:30]

Patchset #1 (id:180001) has been deleted

[nhiroki, 2015-06-26 03:54:36]

Patchset #1 (id:200001) has been deleted

[nhiroki, 2015-06-26 03:54:42]

Patchset #1 (id:220001) has been deleted

[nhiroki, 2015-06-26 05:48:19]

Patchset #1 (id:240001) has been deleted

[nhiroki, 2015-06-26 05:48:38]

Patchset #1 (id:240002) has been deleted

[nhiroki, 2015-06-26 05:48:44]

Patchset #1 (id:270001) has been deleted

[jkarlin, 2015-06-26 19:40:27]

jkarlin@chromium.org changed reviewers:
+ jkarlin@chromium.org

[jkarlin, 2015-06-26 19:40:29]

lgtm with nits, thanks for doing this!

https://codereview.chromium.org/1124403005/diff/290001/LayoutTests/http/tests...
File LayoutTests/http/tests/cachestorage/script-tests/cache-add.js (right):

https://codereview.chromium.org/1124403005/diff/290001/LayoutTests/http/tests...
LayoutTests/http/tests/cachestorage/script-tests/cache-add.js:112:
urls.map(function(url) { return cache.match(url); }));
This and all of the Promise.all( lines, 2 space indent?

I'm really unclear as to what javascript style this file uses.

https://codereview.chromium.org/1124403005/diff/290001/Source/modules/cachest...
File Source/modules/cachestorage/Cache.cpp (right):

https://codereview.chromium.org/1124403005/diff/290001/Source/modules/cachest...
Source/modules/cachestorage/Cache.cpp:396: class ScriptPromiseAll final : public
GarbageCollectedFinalized<ScriptPromiseAll> {
I'm sad that we have to go through V8 to wait on the fetch promise. Perhaps add
a TODO to make this all C++ one day?

https://codereview.chromium.org/1124403005/diff/290001/Source/modules/cachest...
Source/modules/cachestorage/Cache.cpp:468: v8::Local<v8::Function>
createFulfillFunction(size_t index)
The create functions should be private

https://codereview.chromium.org/1124403005/diff/290001/Source/modules/cachest...
Source/modules/cachestorage/Cache.cpp:489: bool m_isResolvedOrRejected = false;
I believe 'settled' is the term for a promise that is either resolved or
rejected. So please use m_isSettled.

[nhiroki, 2015-07-02 09:31:55]

Thank you for reviewing!

https://codereview.chromium.org/1124403005/diff/290001/LayoutTests/http/tests...
File LayoutTests/http/tests/cachestorage/script-tests/cache-add.js (right):

https://codereview.chromium.org/1124403005/diff/290001/LayoutTests/http/tests...
LayoutTests/http/tests/cachestorage/script-tests/cache-add.js:112:
urls.map(function(url) { return cache.match(url); }));
On 2015/06/26 19:40:28, jkarlin_OOO wrote:
> This and all of the Promise.all( lines, 2 space indent?

Right. Fixed.

> I'm really unclear as to what javascript style this file uses.

My change is based on this:
http://www.chromium.org/blink/serviceworker/testing#TOC-Layout-Tests-Style

https://codereview.chromium.org/1124403005/diff/290001/Source/modules/cachest...
File Source/modules/cachestorage/Cache.cpp (right):

https://codereview.chromium.org/1124403005/diff/290001/Source/modules/cachest...
Source/modules/cachestorage/Cache.cpp:396: class ScriptPromiseAll final : public
GarbageCollectedFinalized<ScriptPromiseAll> {
On 2015/06/26 19:40:28, jkarlin_OOO wrote:
> I'm sad that we have to go through V8 to wait on the fetch promise. Perhaps
add
> a TODO to make this all C++ one day?

Done.

https://codereview.chromium.org/1124403005/diff/290001/Source/modules/cachest...
Source/modules/cachestorage/Cache.cpp:468: v8::Local<v8::Function>
createFulfillFunction(size_t index)
On 2015/06/26 19:40:28, jkarlin_OOO wrote:
> The create functions should be private

Done.

https://codereview.chromium.org/1124403005/diff/290001/Source/modules/cachest...
Source/modules/cachestorage/Cache.cpp:489: bool m_isResolvedOrRejected = false;
On 2015/06/26 19:40:28, jkarlin_OOO wrote:
> I believe 'settled' is the term for a promise that is either resolved or
> rejected. So please use m_isSettled.

Done.

[nhiroki, 2015-07-02 09:45:32]

nhiroki@chromium.org changed reviewers:
+ haraken@chromium.org

[nhiroki, 2015-07-02 09:46:34]

+haraken@ for RuntimeEnabledFeatures.in. PTAL, thanks.

[haraken, 2015-07-02 09:49:01]

RuntimeEnabledFeatures.in LGTM

[nhiroki, 2015-07-02 09:53:58]

The CQ bit was checked by nhiroki@chromium.org

[nhiroki, 2015-07-02 09:53:58]

The patchset sent to the CQ was uploaded after l-g-t-m from jkarlin@chromium.org
Link to the patchset: https://codereview.chromium.org/1124403005/#ps330001
(title: "fix test failure")

[commit-bot: I haz the power, 2015-07-02 09:54:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1124403005/330001

[commit-bot: I haz the power, 2015-07-02 09:57:39]

Committed patchset #4 (id:330001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=198203