[CacheStorage] Make CacheStorage::MatchAllCaches return in order

[CacheStorage] Make CacheStorage::MatchAllCaches return in order

MatchAllCaches needs to return the response in order of cache
creation. So if cache A was created before cache B, and they both have
an entry for request Z, then cache A's response for Z should be
returned.

Previously, the caches raced and the first to find an entry was
returned.

BUG=568096
Committed: https://crrev.com/a93559cf4b3891e1b03bd93f3e91bbf8b9b993c4
Cr-Commit-Position: refs/heads/master@{#383599}

[jkarlin, 2016-03-24 16:34:01]

Description was changed from

==========
[CacheStorage] Make CacheStorage::MatchAllCaches return in order

MatchAllCaches needs to return the response in order of cache
creation. So if cache A was created before cache B, and they both have
an entry for request Z, then cache A's response for Z should be
returned.

Previously, the caches raced and the first to find an entry was
returned.

BUG=568096
==========

to

==========
[CacheStorage] Make CacheStorage::MatchAllCaches return in order

MatchAllCaches needs to return the response in order of cache
creation. So if cache A was created before cache B, and they both have
an entry for request Z, then cache A's response for Z should be
returned.

Previously, the caches raced and the first to find an entry was
returned.

BUG=568096
==========

[jkarlin, 2016-03-24 16:34:01]

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

[jkarlin, 2016-03-24 16:34:10]

nhiroki: PTAL, thanks!

[nhiroki, 2016-03-25 10:53:10]

lgtm!

(It'd be better to have a layout test for this)

[jkarlin, 2016-03-25 11:25:48]

Thanks. Added a layout test, PTAL!

[nhiroki, 2016-03-25 14:48:35]

Thank you for adding the test!

https://codereview.chromium.org/1832863002/diff/60001/content/browser/cache_s...
File content/browser/cache_storage/cache_storage.h (right):

https://codereview.chromium.org/1832863002/diff/60001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.h:90: // request/response then it is
not defined which cache's response will be
"it is not defined which cache's response will be returned."

This comment needs to be updated.

https://codereview.chromium.org/1832863002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-matchAll.js
(right):

https://codereview.chromium.org/1832863002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-matchAll.js:41:
return cache.matchAll(entries.a.request)
I think you need to call |self.caches.match|.

Can you move this test into cache-storage-match.js?

[jkarlin, 2016-03-25 15:35:10]

PTAL

https://codereview.chromium.org/1832863002/diff/60001/content/browser/cache_s...
File content/browser/cache_storage/cache_storage.h (right):

https://codereview.chromium.org/1832863002/diff/60001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.h:90: // request/response then it is
not defined which cache's response will be
On 2016/03/25 14:48:35, nhiroki wrote:
> "it is not defined which cache's response will be returned."
> 
> This comment needs to be updated.

Done.

https://codereview.chromium.org/1832863002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-matchAll.js
(right):

https://codereview.chromium.org/1832863002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-matchAll.js:41:
return cache.matchAll(entries.a.request)
On 2016/03/25 14:48:35, nhiroki wrote:
> I think you need to call |self.caches.match|.
> 
> Can you move this test into cache-storage-match.js?

Thank you!! The name CacheStorage::MatchAllCache got me turned around.

[jkarlin, 2016-03-28 21:23:26]

The CQ bit was checked by jkarlin@chromium.org

[jkarlin, 2016-03-28 21:23:27]

The patchset sent to the CQ was uploaded after l-g-t-m from nhiroki@chromium.org
Link to the patchset: https://codereview.chromium.org/1832863002/#ps80001
(title: "Fix layout test")

[commit-bot: I haz the power, 2016-03-28 21:23:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1832863002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1832863002/80001

[commit-bot: I haz the power, 2016-03-28 23:13:33]

Description was changed from

==========
[CacheStorage] Make CacheStorage::MatchAllCaches return in order

MatchAllCaches needs to return the response in order of cache
creation. So if cache A was created before cache B, and they both have
an entry for request Z, then cache A's response for Z should be
returned.

Previously, the caches raced and the first to find an entry was
returned.

BUG=568096
==========

to

==========
[CacheStorage] Make CacheStorage::MatchAllCaches return in order

MatchAllCaches needs to return the response in order of cache
creation. So if cache A was created before cache B, and they both have
an entry for request Z, then cache A's response for Z should be
returned.

Previously, the caches raced and the first to find an entry was
returned.

BUG=568096
==========

[commit-bot: I haz the power, 2016-03-28 23:13:34]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-03-28 23:15:33]

Description was changed from

==========
[CacheStorage] Make CacheStorage::MatchAllCaches return in order

MatchAllCaches needs to return the response in order of cache
creation. So if cache A was created before cache B, and they both have
an entry for request Z, then cache A's response for Z should be
returned.

Previously, the caches raced and the first to find an entry was
returned.

BUG=568096
==========

to

==========
[CacheStorage] Make CacheStorage::MatchAllCaches return in order

MatchAllCaches needs to return the response in order of cache
creation. So if cache A was created before cache B, and they both have
an entry for request Z, then cache A's response for Z should be
returned.

Previously, the caches raced and the first to find an entry was
returned.

BUG=568096

Committed: https://crrev.com/a93559cf4b3891e1b03bd93f3e91bbf8b9b993c4
Cr-Commit-Position: refs/heads/master@{#383599}
==========

[commit-bot: I haz the power, 2016-03-28 23:15:34]

Patchset 5 (id:??) landed as
https://crrev.com/a93559cf4b3891e1b03bd93f3e91bbf8b9b993c4
Cr-Commit-Position: refs/heads/master@{#383599}