CacheStorage: caches.match with bad cacheName should resolve with undefined.

CacheStorage: caches.match with bad cacheName should resolve with undefined.

Make caches.match(request, options)'s promise resolve with undefined (rather
than reject) when options.cacheName is given but no corresponding Cache object
is found.

Related spec issue:
  https://github.com/slightlyoff/ServiceWorker/issues/891

BUG=610349
Committed: https://crrev.com/9916998beca540fc96101352626d49fd0193dca8
Cr-Commit-Position: refs/heads/master@{#406817}

[zino, 2016-07-20 14:37:47]

Description was changed from

==========
CacheStorage: caches.match with bad cacheName should resolve with undefined.

Make caches.match(request, options)'s promise resolve with undefined (rather
than reject) when options.cacheName is given but no corresponding Cache object
is found.

Related spec issue:
  https://github.com/slightlyoff/ServiceWorker/issues/891

BUG=610349
==========

to

==========
CacheStorage: caches.match with bad cacheName should resolve with undefined.

Make caches.match(request, options)'s promise resolve with undefined (rather
than reject) when options.cacheName is given but no corresponding Cache object
is found.

Related spec issue:
  https://github.com/slightlyoff/ServiceWorker/issues/891

BUG=610349
==========

[zino, 2016-07-20 14:37:48]

jinho.bang@samsung.com changed reviewers:
+ jkarlin@chromium.org, nhiroki@chromium.org

[zino, 2016-07-20 14:40:05]

PTAL

[nhiroki, 2016-07-20 16:05:45]

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

[nhiroki, 2016-07-20 16:05:45]

I'm somewhat concerned that this change may break some real applications.

jsbell@ and jkarlin@, any thoughts?

https://codereview.chromium.org/2161423003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-storage-match.js
(right):

https://codereview.chromium.org/2161423003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-storage-match.js:115:
'with undefined.');
It'd be better to make our tests consistent with w3c web-platform-tests[1] so
that we can easier upstream our tests to the repository later.

[1]
https://github.com/w3c/web-platform-tests/blob/master/service-workers/cache-s...

[jsbell, 2016-07-20 17:40:48]

On 2016/07/20 16:05:45, nhiroki (slow) wrote:
> I'm somewhat concerned that this change may break some real applications.
> 
> jsbell@ and jkarlin@, any thoughts?

The point was made in https://github.com/slightlyoff/ServiceWorker/issues/891
that the current behavior was causing surprising bugs, as CacheStorage.match()'s
behavior was different from Cache.match().

Relying on a rejection if the named cache did not exist would be somewhat
unusual. I can imagine a scenario: caches created on demand and match() being
used to detect that the entry is not present (resolve with undefined) or that
the cache doesn't exist yet (reject). 

But given that Gecko went ahead with this change back in May and hasn't gotten
complaints yet I think it's probably safe.

change lgtm with nhiroki@'s comment about the test addressed.

[zino, 2016-07-21 08:10:15]

nhiroki@,

I addressed test. PTAL.

https://codereview.chromium.org/2161423003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-storage-match.js
(right):

https://codereview.chromium.org/2161423003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-storage-match.js:115:
'with undefined.');
On 2016/07/20 16:05:45, nhiroki (slow) wrote:
> It'd be better to make our tests consistent with w3c web-platform-tests[1] so
> that we can easier upstream our tests to the repository later.
> 
> [1]
>
https://github.com/w3c/web-platform-tests/blob/master/service-workers/cache-s...

Done.

[nhiroki, 2016-07-21 08:56:24]

On 2016/07/20 17:40:48, jsbell wrote:
> On 2016/07/20 16:05:45, nhiroki (slow) wrote:
> > I'm somewhat concerned that this change may break some real applications.
> > 
> > jsbell@ and jkarlin@, any thoughts?
> 
> The point was made in https://github.com/slightlyoff/ServiceWorker/issues/891
> that the current behavior was causing surprising bugs, as
CacheStorage.match()'s
> behavior was different from Cache.match().
> 
> Relying on a rejection if the named cache did not exist would be somewhat
> unusual. I can imagine a scenario: caches created on demand and match() being
> used to detect that the entry is not present (resolve with undefined) or that
> the cache doesn't exist yet (reject). 
> 
> But given that Gecko went ahead with this change back in May and hasn't gotten
> complaints yet I think it's probably safe.
> 
> change lgtm with nhiroki@'s comment about the test addressed.

Thank you for the clarification. Sounds reasonable.

[nhiroki, 2016-07-21 08:56:34]

LGTM

[zino, 2016-07-21 09:49:48]

The CQ bit was checked by jinho.bang@samsung.com

[zino, 2016-07-21 09:49:49]

The patchset sent to the CQ was uploaded after l-g-t-m from jsbell@chromium.org
Link to the patchset: https://codereview.chromium.org/2161423003/#ps20001
(title: "test")

[commit-bot: I haz the power, 2016-07-21 09:50:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-21 10:58:12]

Description was changed from

==========
CacheStorage: caches.match with bad cacheName should resolve with undefined.

Make caches.match(request, options)'s promise resolve with undefined (rather
than reject) when options.cacheName is given but no corresponding Cache object
is found.

Related spec issue:
  https://github.com/slightlyoff/ServiceWorker/issues/891

BUG=610349
==========

to

==========
CacheStorage: caches.match with bad cacheName should resolve with undefined.

Make caches.match(request, options)'s promise resolve with undefined (rather
than reject) when options.cacheName is given but no corresponding Cache object
is found.

Related spec issue:
  https://github.com/slightlyoff/ServiceWorker/issues/891

BUG=610349
==========

[commit-bot: I haz the power, 2016-07-21 10:58:13]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-21 10:59:40]

Description was changed from

==========
CacheStorage: caches.match with bad cacheName should resolve with undefined.

Make caches.match(request, options)'s promise resolve with undefined (rather
than reject) when options.cacheName is given but no corresponding Cache object
is found.

Related spec issue:
  https://github.com/slightlyoff/ServiceWorker/issues/891

BUG=610349
==========

to

==========
CacheStorage: caches.match with bad cacheName should resolve with undefined.

Make caches.match(request, options)'s promise resolve with undefined (rather
than reject) when options.cacheName is given but no corresponding Cache object
is found.

Related spec issue:
  https://github.com/slightlyoff/ServiceWorker/issues/891

BUG=610349

Committed: https://crrev.com/9916998beca540fc96101352626d49fd0193dca8
Cr-Commit-Position: refs/heads/master@{#406817}
==========

[commit-bot: I haz the power, 2016-07-21 10:59:41]

Patchset 2 (id:??) landed as
https://crrev.com/9916998beca540fc96101352626d49fd0193dca8
Cr-Commit-Position: refs/heads/master@{#406817}