CacheStorage: Add ignoreSearch option to cache.matchAll().

CacheStorage: Add ignoreSearch option to cache.matchAll().

The option that speicifies whether the matching process should ignore the query
string in the url. If the option is true, query string of request url would be
ignored when performaing a match.

Spec:
  https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#cache-query-options-dictionary

BUG=520784
Committed: https://crrev.com/8cd93e585ae7f85b41a54b9328657ac0ab48eedb
Cr-Commit-Position: refs/heads/master@{#375508}

[zino, 2016-01-14 10:45:37]

Description was changed from

==========
CacheStorage: Support ignoreSearch option.

Support ignoreSearch option.

Spec:
 
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#ca...

BUG=520784
==========

to

==========
CacheStorage: Support cache.matchAll() with ignoreSearch option.

CacheStorage: Support cache.matchAll() with ignoreSearch option.

Spec:
 
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#ca...

BUG=520784
==========

[zino, 2016-01-15 09:56:48]

Description was changed from

==========
CacheStorage: Support cache.matchAll() with ignoreSearch option.

CacheStorage: Support cache.matchAll() with ignoreSearch option.

Spec:
 
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#ca...

BUG=520784
==========

to

==========
CacheStorage: Add ignoreSearch option to cache.matchAll().

The option that speicifies whether the matching process should ignore the query
string in the url. If the option is true, query string of request url would be
ignored when performaing a match.

Spec:
 
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#ca...

BUG=520784
==========

[zino, 2016-01-15 09:58:45]

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

[zino, 2016-01-15 09:58:45]

PTAL.

[zino, 2016-01-15 10:02:40]

On 2016/01/15 09:58:45, zino wrote:
> PTAL.

FYI, we have to be able to use the option with cache.delete() and cache.keys().

But I'll separate the work into next CLs for easier implementing and reviewing.

[nhiroki, 2016-01-18 04:29:59]

Thank you for working on this.

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
File content/browser/cache_storage/cache_storage_cache.cc (right):

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:152: request_url.host() ==
key.host() && request_url.path() == key.path();
This match algorithm could be insufficient, for example, when the url contains
username/password parts. According to the "Query Cache" algorithm in the sw
spec, request/cached URLs should be serialized by the "URL serializing"
algorithm (https://url.spec.whatwg.org/#url-serializing) and then be compared.

Maybe we could achieve that by creating new KURL objects without search
parameters and compares them by KURL::operator==()?

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:218: explicit
MatchAllContext(scoped_ptr<ServiceWorkerFetchRequest> request,
Remove "explicit"

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:219: const
CacheStorageCache::ResponsesCallback& callback)
"CacheStorageCache::" prefix is not necessary.

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:339: void
CacheStorageCache::MatchAll(const ResponsesCallback& callback) {
Could you remove this indirection and directly call MatchAll(null_req,
callback)?

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:700:
URLMatchWithoutQuery(context->request->url, GURL(entry->GetKey()))) {
This combined condition is a bit confusing to me. How about checking the
|ignore_search| option to clarify that we're handling the case here.

// context->options() is CacheStorageCacheQueryParams passed from
CacheStorageDispatcherHost.
if (context->options().ignore_search) {
  DCHECK(context->request);
  if (!URLMatchWithoutQuery(...))
    ReadMetadata(...);
  else
    MatchAllProcessNextEntry(...);
  return;
}
DCHECK(!context->request);
ReadMetadata(...);

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
File content/browser/cache_storage/cache_storage_dispatcher_host.cc (right):

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_dispatcher_host.cc:254: if
(match_params.ignore_search) {
Could you add a runtime flag for this feature and disable it by default until
we're ready to ship?

https://codereview.chromium.org/1578363009/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/window/cache-matchAll-expected.txt
(right):

https://codereview.chromium.org/1578363009/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/cachestorage/window/cache-matchAll-expected.txt:2:
CONSOLE WARNING: Cache.match() does not support 'ignoreSearch' option yet. See
http://crbug.com/520784
Could you show these warning messages only when the runtime flag (see my other
comment) is disabled?

https://codereview.chromium.org/1578363009/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/worker/cache-matchAll-expected.txt
(right):

https://codereview.chromium.org/1578363009/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/cachestorage/worker/cache-matchAll-expected.txt:2:
CONSOLE WARNING: Cache.match() does not support 'ignoreSearch' option yet. See
http://crbug.com/520784
ditto.

[zino, 2016-01-19 09:50:25]

Patchset #3 (id:40001) has been deleted

[zino, 2016-01-19 13:04:09]

Patchset #3 (id:60001) has been deleted

[zino, 2016-01-19 13:46:38]

I addressed your comments.

Could you review again?

Thank you!

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
File content/browser/cache_storage/cache_storage_cache.cc (right):

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:152: request_url.host() ==
key.host() && request_url.path() == key.path();
I modified it but I couldn't write a test for it.
I'm not sure but Request object seems to contains no username/password parts.
The spec[1] and our implementation[2] says "if parsedURL includes credentials,
throw a TypeError".

[1] https://fetch.spec.whatwg.org/#dom-request (12-3)
[2] http://code.google.com/p/chromium/issues/detail?id=474439

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:218: explicit
MatchAllContext(scoped_ptr<ServiceWorkerFetchRequest> request,
On 2016/01/18 04:29:58, nhiroki wrote:
> Remove "explicit"

Done.

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:219: const
CacheStorageCache::ResponsesCallback& callback)
On 2016/01/18 04:29:58, nhiroki wrote:
> "CacheStorageCache::" prefix is not necessary.

Done.

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:339: void
CacheStorageCache::MatchAll(const ResponsesCallback& callback) {
On 2016/01/18 04:29:59, nhiroki wrote:
> Could you remove this indirection and directly call MatchAll(null_req,
> callback)?

Done.

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:700:
URLMatchWithoutQuery(context->request->url, GURL(entry->GetKey()))) {
On 2016/01/18 04:29:59, nhiroki wrote:
> This combined condition is a bit confusing to me. How about checking the
> |ignore_search| option to clarify that we're handling the case here.
> 
> // context->options() is CacheStorageCacheQueryParams passed from
> CacheStorageDispatcherHost.
> if (context->options().ignore_search) {
>   DCHECK(context->request);
>   if (!URLMatchWithoutQuery(...))
>     ReadMetadata(...);
>   else
>     MatchAllProcessNextEntry(...);
>   return;
> }
> DCHECK(!context->request);
> ReadMetadata(...);

Done.

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
File content/browser/cache_storage/cache_storage_dispatcher_host.cc (right):

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_dispatcher_host.cc:254: if
(match_params.ignore_search) {
On 2016/01/18 04:29:59, nhiroki wrote:
> Could you add a runtime flag for this feature and disable it by default until
> we're ready to ship?

Done.

https://codereview.chromium.org/1578363009/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/window/cache-matchAll-expected.txt
(right):

https://codereview.chromium.org/1578363009/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/cachestorage/window/cache-matchAll-expected.txt:2:
CONSOLE WARNING: Cache.match() does not support 'ignoreSearch' option yet. See
http://crbug.com/520784
On 2016/01/18 04:29:59, nhiroki wrote:
> Could you show these warning messages only when the runtime flag (see my other
> comment) is disabled?

Done.

https://codereview.chromium.org/1578363009/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/worker/cache-matchAll-expected.txt
(right):

https://codereview.chromium.org/1578363009/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/cachestorage/worker/cache-matchAll-expected.txt:2:
CONSOLE WARNING: Cache.match() does not support 'ignoreSearch' option yet. See
http://crbug.com/520784
On 2016/01/18 04:29:59, nhiroki wrote:
> ditto.

Done.

[nhiroki, 2016-01-20 08:56:02]

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
File content/browser/cache_storage/cache_storage_cache.cc (right):

https://codereview.chromium.org/1578363009/diff/20001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:152: request_url.host() ==
key.host() && request_url.path() == key.path();
On 2016/01/19 13:46:38, zino wrote:
> I modified it but I couldn't write a test for it.
> I'm not sure but Request object seems to contains no username/password parts.
> The spec[1] and our implementation[2] says "if parsedURL includes credentials,
> throw a TypeError".
> 
> [1] https://fetch.spec.whatwg.org/#dom-request (12-3)
> [2] http://code.google.com/p/chromium/issues/detail?id=474439

Yes, the constructor rejects such a URL.

A Request object made by a regular network request can contain
username/password, for example, clicking "<a
href='http://username:password@example.com/'>".

"LayoutTests/http/tests/cachestorage/serviceworker/credentials.html" is an
example of credential tests.

https://codereview.chromium.org/1578363009/diff/80001/content/browser/cache_s...
File content/browser/cache_storage/cache_storage_cache.cc (right):

https://codereview.chromium.org/1578363009/diff/80001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:150: bool
URLMatchWithoutQuery(const GURL& request_url, const GURL& key) {
This function name sounds a bit strange to me.

This does 2 things: (1) removing query parts from given URLs and (2) comparing
them. Such mixed functions seem to make it difficult to name this. Instead, how
about making a simple "GURL RemoveQueryParam(const GURL& url)" just to remove a
query part and calling it in MatchAllProcessNextEntry() like this?:

if (RemoveQueryParam(context->request->url) !=
RemoveQueryParam(GURL(entry->GetKey())) { ... }

https://codereview.chromium.org/1578363009/diff/80001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:667:
std::move(context->out_blob_data_handles));
Do you have any reason to change these parameters? I think it'd be common
pattern to pass explicit empty results to an error callback as the original code
does.

https://codereview.chromium.org/1578363009/diff/80001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:682:
std::move(context->out_blob_data_handles));
ditto.

https://codereview.chromium.org/1578363009/diff/80001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:702: disk_cache::Entry*
entry(*iter);
Can you move this to line 705?

https://codereview.chromium.org/1578363009/diff/80001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:706: // In this caase, we
don't need to read data.
s/casse/case

https://codereview.chromium.org/1578363009/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in (right):

https://codereview.chromium.org/1578363009/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in:37:
CacheIgnoreSearchOption status=test
This should be "experimental" because this is not only for testing and will be
shipped soon.

[zino, 2016-02-03 12:03:26]

Patchset #4 (id:100001) has been deleted

[zino, 2016-02-03 12:04:48]

PTAL.

https://codereview.chromium.org/1578363009/diff/80001/content/browser/cache_s...
File content/browser/cache_storage/cache_storage_cache.cc (right):

https://codereview.chromium.org/1578363009/diff/80001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:150: bool
URLMatchWithoutQuery(const GURL& request_url, const GURL& key) {
On 2016/01/20 08:56:02, nhiroki wrote:
> This function name sounds a bit strange to me.
> 
> This does 2 things: (1) removing query parts from given URLs and (2) comparing
> them. Such mixed functions seem to make it difficult to name this. Instead,
how
> about making a simple "GURL RemoveQueryParam(const GURL& url)" just to remove
a
> query part and calling it in MatchAllProcessNextEntry() like this?:
> 
> if (RemoveQueryParam(context->request->url) !=
> RemoveQueryParam(GURL(entry->GetKey())) { ... }

Done.

https://codereview.chromium.org/1578363009/diff/80001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:667:
std::move(context->out_blob_data_handles));
On 2016/01/20 08:56:02, nhiroki wrote:
> Do you have any reason to change these parameters? I think it'd be common
> pattern to pass explicit empty results to an error callback as the original
code
> does.

Done.

I thought that we need to avoid creating object duplicately (even if it's a
empty pointer object) if possible.
The out_responses and blob_data_handles were already initialized as empty
object.

On second thoughts, I was wrong and readability seems more important.

https://codereview.chromium.org/1578363009/diff/80001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:682:
std::move(context->out_blob_data_handles));
On 2016/01/20 08:56:02, nhiroki wrote:
> ditto.

Done.

https://codereview.chromium.org/1578363009/diff/80001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:702: disk_cache::Entry*
entry(*iter);
On 2016/01/20 08:56:02, nhiroki wrote:
> Can you move this to line 705?

Done.

https://codereview.chromium.org/1578363009/diff/80001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache.cc:706: // In this caase, we
don't need to read data.
On 2016/01/20 08:56:02, nhiroki wrote:
> s/casse/case

Done.

https://codereview.chromium.org/1578363009/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in (right):

https://codereview.chromium.org/1578363009/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in:37:
CacheIgnoreSearchOption status=test
On 2016/01/20 08:56:02, nhiroki (ooo until Feb. 4) wrote:
> This should be "experimental" because this is not only for testing and will be
> shipped soon.

Done.

[nhiroki, 2016-02-10 07:35:33]

Sorry for the late reply. Looks good overall.

https://codereview.chromium.org/1578363009/diff/120001/content/browser/cache_...
File content/browser/cache_storage/cache_storage_cache_unittest.cc (right):

https://codereview.chromium.org/1578363009/diff/120001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache_unittest.cc:388:
scoped_ptr<ServiceWorkerFetchRequest>(), CacheStorageCacheQueryParams(),
Can you add unittests for the case where 'ignore_search' is specified?

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/resources/ignore-search-with-credentials-worker.js
(right):

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/resources/ignore-search-with-credentials-worker.js:15:

nit: please remove trailing blank lines

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html
(right):

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html:9:
var request;
Probably you can move this global variable to

promise_test(function(test) {
    var service_worker;
    var script_url = ...;
    var scope = ...;
    // HERE!!
  });

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html:21:
for (var i = 0; i < requests.length; i++)
"Array.prototype.find()" may work?

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html:49:
frame.contentWindow.xhr('simple.txt?query_without_credential=test')
nit: can you wrap this line at the 80 column?

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html:52:
.then(function() {
Can you add a comment about the reason why we need to retrieve a request from
the cache storage instead of directly creating a new request object by the ctor?

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html:57:
return caches.open('ignore-search')
The cache is already opened in find_request_object(). Can you avoid the 2nd open
operation here? For example, this could be...

.then(function(frame) {
    return Promise.all([...]);
  })
.then(function() { return caches.open('ignore-search'); })
.then(function(c) {
    cache = c;
    return find_request_object(cache, 'abc:def');
  })
.then(function(request) {
    return cache.matchAll(request, { ignoreSearch : true });
  })
.then(function(result) {
    assert_equals(...);
  })

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html:62:
.then(function(result) {
s/result/results/

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html:65:
assert_equals(remove_query(request.url), remove_query(result[1].url));
assert_equals() receives 'actual' and 'expected' args in this order, so these
could be...

assert_equals(remove_query(result[0].url), remove_query(request.url));
assert_equals(remove_query(result[1].url), remove_query(request.url));

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[zino, 2016-02-11 03:35:05]

Thank you for review.

https://codereview.chromium.org/1578363009/diff/120001/content/browser/cache_...
File content/browser/cache_storage/cache_storage_cache_unittest.cc (right):

https://codereview.chromium.org/1578363009/diff/120001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache_unittest.cc:388:
scoped_ptr<ServiceWorkerFetchRequest>(), CacheStorageCacheQueryParams(),
On 2016/02/10 07:35:32, nhiroki (slow) wrote:
> Can you add unittests for the case where 'ignore_search' is specified?

Done.

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/resources/ignore-search-with-credentials-worker.js
(right):

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/resources/ignore-search-with-credentials-worker.js:15:

On 2016/02/10 07:35:32, nhiroki (slow) wrote:
> nit: please remove trailing blank lines

Done.

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html
(right):

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html:9:
var request;
On 2016/02/10 07:35:33, nhiroki (slow) wrote:
> Probably you can move this global variable to
> 
> promise_test(function(test) {
>     var service_worker;
>     var script_url = ...;
>     var scope = ...;
>     // HERE!!
>   });

Done.

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html:21:
for (var i = 0; i < requests.length; i++)
On 2016/02/10 07:35:33, nhiroki (slow) wrote:
> "Array.prototype.find()" may work?

Done.

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html:49:
frame.contentWindow.xhr('simple.txt?query_without_credential=test')
On 2016/02/10 07:35:32, nhiroki (slow) wrote:
> nit: can you wrap this line at the 80 column?

Done.

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html:52:
.then(function() {
On 2016/02/10 07:35:33, nhiroki (slow) wrote:
> Can you add a comment about the reason why we need to retrieve a request from
> the cache storage instead of directly creating a new request object by the
ctor?

Done.

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html:57:
return caches.open('ignore-search')
On 2016/02/10 07:35:32, nhiroki (slow) wrote:
> The cache is already opened in find_request_object(). Can you avoid the 2nd
open
> operation here? For example, this could be...
> 
> .then(function(frame) {
>     return Promise.all([...]);
>   })
> .then(function() { return caches.open('ignore-search'); })
> .then(function(c) {
>     cache = c;
>     return find_request_object(cache, 'abc:def');
>   })
> .then(function(request) {
>     return cache.matchAll(request, { ignoreSearch : true });
>   })
> .then(function(result) {
>     assert_equals(...);
>   })

Done.

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html:62:
.then(function(result) {
On 2016/02/10 07:35:33, nhiroki (slow) wrote:
> s/result/results/

Done.

https://codereview.chromium.org/1578363009/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html:65:
assert_equals(remove_query(request.url), remove_query(result[1].url));
On 2016/02/10 07:35:32, nhiroki (slow) wrote:
> assert_equals() receives 'actual' and 'expected' args in this order, so these
> could be...
> 
> assert_equals(remove_query(result[0].url), remove_query(request.url));
> assert_equals(remove_query(result[1].url), remove_query(request.url));
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

Done.

[nhiroki, 2016-02-12 04:18:33]

Can you upload the latest patchset? :)

[zino, 2016-02-12 08:28:41]

On 2016/02/12 04:18:33, nhiroki (slow) wrote:
> Can you upload the latest patchset? :)

Ah, sorry. I uploaded a patch set to other CL by mistake. :(
I re-uploaded it here correctly. PTAL.

Thank you!

[zino, 2016-02-12 08:29:25]

jinho.bang@samsung.com changed reviewers:
+ tkent@chromium.org

[zino, 2016-02-12 08:31:15]

+ tkent@ for Blink/Source/platform/RuntimeEnabledFeatures.in.

PTAL.

Thank you.

[zino, 2016-02-15 07:08:35]

nhiroki@,

PTAL. (if you forgot this CL.)

[nhiroki, 2016-02-15 08:55:10]

LGTM! \o/

https://codereview.chromium.org/1578363009/diff/140001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html
(right):

https://codereview.chromium.org/1578363009/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html:64:
// [1] https://fetch.spec.whatwg.org/#dom-request
Good! Thank you for adding the comment.

[zino, 2016-02-15 09:07:29]

tkent@,

Could you review this? (Especially, need your stamp for
third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in)

Thank you.

[tkent, 2016-02-15 23:02:29]

lgtm

[zino, 2016-02-15 23:31:25]

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

[commit-bot: I haz the power, 2016-02-15 23:33:00]

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

[commit-bot: I haz the power, 2016-02-15 23:35:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-15 23:35:35]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[zino, 2016-02-16 01:39:26]

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

[zino, 2016-02-16 01:39:26]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org,
nhiroki@chromium.org
Link to the patchset: https://codereview.chromium.org/1578363009/#ps160001
(title: "rebase")

[commit-bot: I haz the power, 2016-02-16 01:39:37]

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

[commit-bot: I haz the power, 2016-02-16 03:06:54]

Description was changed from

==========
CacheStorage: Add ignoreSearch option to cache.matchAll().

The option that speicifies whether the matching process should ignore the query
string in the url. If the option is true, query string of request url would be
ignored when performaing a match.

Spec:
 
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#ca...

BUG=520784
==========

to

==========
CacheStorage: Add ignoreSearch option to cache.matchAll().

The option that speicifies whether the matching process should ignore the query
string in the url. If the option is true, query string of request url would be
ignored when performaing a match.

Spec:
 
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#ca...

BUG=520784
==========

[commit-bot: I haz the power, 2016-02-16 03:06:54]

Committed patchset #6 (id:160001)

[commit-bot: I haz the power, 2016-02-16 22:51:13]

Description was changed from

==========
CacheStorage: Add ignoreSearch option to cache.matchAll().

The option that speicifies whether the matching process should ignore the query
string in the url. If the option is true, query string of request url would be
ignored when performaing a match.

Spec:
 
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#ca...

BUG=520784
==========

to

==========
CacheStorage: Add ignoreSearch option to cache.matchAll().

The option that speicifies whether the matching process should ignore the query
string in the url. If the option is true, query string of request url would be
ignored when performaing a match.

Spec:
 
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#ca...

BUG=520784

Committed: https://crrev.com/8cd93e585ae7f85b41a54b9328657ac0ab48eedb
Cr-Commit-Position: refs/heads/master@{#375508}
==========

[commit-bot: I haz the power, 2016-02-16 22:51:14]

Patchset 6 (id:??) landed as
https://crrev.com/8cd93e585ae7f85b41a54b9328657ac0ab48eedb
Cr-Commit-Position: refs/heads/master@{#375508}