CacheStorage: Expand cache.keys() method.

CacheStorage: Expand cache.keys() method.

Current implementation of the method is always return all of keys even if it
takes optional parameters. According to ServiceWorkerSpec[1], the method should
take a request object and cache query options[2] as optional parameters.
This CL is expanding the method as per the spec.

[1] http://slightlyoff.github.io/ServiceWorker/spec/service_worker/#cache-keys
[2] Currenly, only supports ignoreSearch option like matchAll(), delete()

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

[zino, 2016-02-22 16:29:30]

Description was changed from

==========
keys with ignoreSearch

BUG=
==========

to

==========
keys with ignoreSearch

TODO: unittest
TODO: write a layout test for cache-keys.
TODO: write a layout test for credentials

BUG=
==========

[zino, 2016-02-24 18:25:07]

Description was changed from

==========
keys with ignoreSearch

TODO: unittest
TODO: write a layout test for cache-keys.
TODO: write a layout test for credentials

BUG=
==========

to

==========
[WIP] keys with ignoreSearch

TODO: send request object from Blink to Chromium.
TODO: unittest
TODO: write a layout test for cache-keys.
TODO: write a layout test for credentials

BUG=
==========

[zino, 2016-02-24 18:26:56]

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

[zino, 2016-02-24 18:26:57]

https://codereview.chromium.org/1719103002/diff/1/content/browser/cache_stora...
File content/browser/cache_storage/cache_storage_cache.h (left):

https://codereview.chromium.org/1719103002/diff/1/content/browser/cache_stora...
content/browser/cache_storage/cache_storage_cache.h:105: // TODO(jkarlin): Have
keys take an optional ServiceWorkerFetchRequest.
jkarlin@,

Are you working on this TODO?
If you are working on the issue, I'm waiting for you.
If not so, can I resolve the issue with this CL together?

[jkarlin, 2016-02-24 18:31:30]

I'm not working on it, go for it.

[zino, 2016-02-26 04:03:40]

Description was changed from

==========
[WIP] keys with ignoreSearch

TODO: send request object from Blink to Chromium.
TODO: unittest
TODO: write a layout test for cache-keys.
TODO: write a layout test for credentials

BUG=
==========

to

==========
[WIP] keys with ignoreSearch

TODO: unittest
TODO: write a layout test for cache-keys.
TODO: write a layout test for credentials

BUG=
==========

[zino, 2016-03-02 16:42:09]

Description was changed from

==========
[WIP] keys with ignoreSearch

TODO: unittest
TODO: write a layout test for cache-keys.
TODO: write a layout test for credentials

BUG=
==========

to

==========
CacheStorage: Expand cache.keys() method.

Current implementation of the method is always return all of keys even if it
takes optional parameters. According to ServiceWorkerSpec[1], the method should
take a request object and cache query options[2] as optional parameters.This CL
is expanding the method as per the spec and fixing a typo in Cache.idl
as well.

[1] http://slightlyoff.github.io/ServiceWorker/spec/service_worker/#cache-keys
[2] Currenly, only supports ignoreSearch option like matchAll(), delete()

BUG=
==========

[zino, 2016-03-02 16:42:24]

Description was changed from

==========
CacheStorage: Expand cache.keys() method.

Current implementation of the method is always return all of keys even if it
takes optional parameters. According to ServiceWorkerSpec[1], the method should
take a request object and cache query options[2] as optional parameters.This CL
is expanding the method as per the spec and fixing a typo in Cache.idl
as well.

[1] http://slightlyoff.github.io/ServiceWorker/spec/service_worker/#cache-keys
[2] Currenly, only supports ignoreSearch option like matchAll(), delete()

BUG=
==========

to

==========
CacheStorage: Expand cache.keys() method.

Current implementation of the method is always return all of keys even if it
takes optional parameters. According to ServiceWorkerSpec[1], the method should
take a request object and cache query options[2] as optional parameters.
This CL is expanding the method as per the spec and fixing a typo in Cache.idl
as well.

[1] http://slightlyoff.github.io/ServiceWorker/spec/service_worker/#cache-keys
[2] Currenly, only supports ignoreSearch option like matchAll(), delete()

BUG=
==========

[zino, 2016-03-02 16:44:10]

Description was changed from

==========
CacheStorage: Expand cache.keys() method.

Current implementation of the method is always return all of keys even if it
takes optional parameters. According to ServiceWorkerSpec[1], the method should
take a request object and cache query options[2] as optional parameters.
This CL is expanding the method as per the spec and fixing a typo in Cache.idl
as well.

[1] http://slightlyoff.github.io/ServiceWorker/spec/service_worker/#cache-keys
[2] Currenly, only supports ignoreSearch option like matchAll(), delete()

BUG=
==========

to

==========
CacheStorage: Expand cache.keys() method.

Current implementation of the method is always return all of keys even if it
takes optional parameters. According to ServiceWorkerSpec[1], the method should
take a request object and cache query options[2] as optional parameters.
This CL is expanding the method as per the spec and fixing a typo in Cache.idl
as well.

[1] http://slightlyoff.github.io/ServiceWorker/spec/service_worker/#cache-keys
[2] Currenly, only supports ignoreSearch option like matchAll(), delete()

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

[zino, 2016-03-02 16:44:11]

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

[zino, 2016-03-02 16:48:36]

jinho.bang@samsung.com changed reviewers:
+ philipj@opera.com

[zino, 2016-03-02 16:49:20]

Patchset #2 (id:20001) has been deleted

[zino, 2016-03-02 16:49:29]

Patchset #2 (id:40001) has been deleted

[zino, 2016-03-02 16:51:06]

Please take a look.

Thank you.

[zino, 2016-03-03 09:44:48]

On 2016/03/02 16:51:06, zino wrote:
> Please take a look.
> 
> Thank you.

FYI, I've just fixed unit test error on trybot.

[zino, 2016-03-03 12:40:52]

Patchset #5 (id:120001) has been deleted

[jkarlin, 2016-03-04 14:39:09]

Overall looking good but please see the last comment as I think another CL
should come before this one.

https://codereview.chromium.org/1719103002/diff/100001/content/browser/cache_...
File content/browser/cache_storage/cache_storage_cache.cc (right):

https://codereview.chromium.org/1719103002/diff/100001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache.cc:246: explicit
KeysContext(scoped_ptr<ServiceWorkerFetchRequest> request,
Remove explicit

https://codereview.chromium.org/1719103002/diff/100001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache.cc:1157: // loops
comment formatting, you have an extra newline here

https://codereview.chromium.org/1719103002/diff/100001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache.cc:1181: void
CacheStorageCache::KeyDidOpenEntry(
Should be KeysDidOpenEntry to show that it's a continuation of the keys
function.

https://codereview.chromium.org/1719103002/diff/100001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache.cc:1202: void
CacheStorageCache::KeyDidReadMetadata(
Should be KeysDidReadMetadata

https://codereview.chromium.org/1719103002/diff/100001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache.cc:1209: if (metadata) {
Start with the failed case and return early:

e.g.,

if (!metadata) {
  entry->Doom();
  callback.Run(CACHE_STORAGE_OK, std::move(requests));
  return;
}
...

https://codereview.chromium.org/1719103002/diff/100001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache.cc:1221: }
Lines 1210-1221 could be put in a new 
PopulateFetchRequestFromMetadata function akin to PopulateResponseMetadata (this
should be moved to anonymous namespace and renamed to
PopulateResponseFromMetadata).

namespace {

void PopulateFetchRequestFromMetadata(const CacheMetadata& metadata,
ServiceWorkerFetchRequest* request) { 
...
}

}  // namespace

https://codereview.chromium.org/1719103002/diff/100001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache.cc:1253: if
(keys_context->options.ignore_search) {
This CL adds what is essentially a third implementation of the Query Cache
algorithm. It's time to consolidate.

In a separate CL, please create something like the following and update Match
and Delete to use it:

struct RequestAndResponse {
  scoped_ptr<ServiceWorkerFetchRequest> request;
  scoped_ptr<ServiceWorkerResponse> response;
  BlobDataHandle response_body_handle;
};

using CacheQueryResults = std::vector<RequestAndResponse>;
using CacheQueryResultsCallback = base::Callback<void(CacheQueryResults)>;

void CacheStorageCache::QueryCache(scoped_ptr<ServiceWorkerFetchRequest>
request, const CacheStorageCacheQueryParams& match_params, const
CacheQueryResultsCallback& callback) {

// Perform
http://slightlyoff.github.io/ServiceWorker/spec/service_worker/#query-cache-a...
here

}

Then in this CL use your new method for Keys as well.

[zino, 2016-08-01 15:04:22]

Patchset #6 (id:160001) has been deleted

[zino, 2016-08-01 15:24:01]

Patchset #6 (id:180001) has been deleted

[zino, 2016-08-01 18:36:20]

Patchset #6 (id:200001) has been deleted

[zino, 2016-08-02 01:07:18]

Description was changed from

==========
CacheStorage: Expand cache.keys() method.

Current implementation of the method is always return all of keys even if it
takes optional parameters. According to ServiceWorkerSpec[1], the method should
take a request object and cache query options[2] as optional parameters.
This CL is expanding the method as per the spec and fixing a typo in Cache.idl
as well.

[1] http://slightlyoff.github.io/ServiceWorker/spec/service_worker/#cache-keys
[2] Currenly, only supports ignoreSearch option like matchAll(), delete()

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

to

==========
CacheStorage: Expand cache.keys() method.

Current implementation of the method is always return all of keys even if it
takes optional parameters. According to ServiceWorkerSpec[1], the method should
take a request object and cache query options[2] as optional parameters.
This CL is expanding the method as per the spec.

[1] http://slightlyoff.github.io/ServiceWorker/spec/service_worker/#cache-keys
[2] Currenly, only supports ignoreSearch option like matchAll(), delete()

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

[zino, 2016-08-02 01:09:20]

Reviewers,

Please take a look at PatchSet6.

Thank you

[nhiroki, 2016-08-02 07:31:06]

Partially reviewed. I'll take another look later.

https://codereview.chromium.org/1719103002/diff/220001/content/browser/cache_...
File content/browser/cache_storage/cache_storage_cache.cc (right):

https://codereview.chromium.org/1719103002/diff/220001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache.cc:632:
!query_cache_results->request->url.is_empty()) {
Hm... I assumed that the diskcache always returns a valid request and request's
url is not empty. Did you hit DCHECK(query_cache_results->request) in the
original code?

If request is null or url is empty, I think we should immediately proceed to the
next entry instead of trying to read metadata.

[jkarlin, 2016-08-02 12:06:20]

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

https://codereview.chromium.org/1719103002/diff/220001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache_unittest.cc:1057:
expected_keys.push_back(body_request_with_query_.url.spec());
You can use vector initializers now, so this can probably be something like:

std::vector<std::string expected_keys = {body_request_.url.spec(),
body_request_with_query_.url.spec()};

https://codereview.chromium.org/1719103002/diff/220001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache_unittest.cc:1075:
expected_keys.push_back(body_request_with_query_.url.spec());
Ditto

https://codereview.chromium.org/1719103002/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-keys.js
(right):

https://codereview.chromium.org/1719103002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-keys.js:99:
assert_request_array_equivalent(
Add a TODO to remove the "equivalent" part and delete the
assert_request_array_equivalent function once keys are returned in insertion
order.

[zino, 2016-08-02 17:37:42]

Thank you for review.

https://codereview.chromium.org/1719103002/diff/220001/content/browser/cache_...
File content/browser/cache_storage/cache_storage_cache.cc (right):

https://codereview.chromium.org/1719103002/diff/220001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache.cc:632:
!query_cache_results->request->url.is_empty()) {
On 2016/08/02 07:31:06, nhiroki wrote:
> Hm... I assumed that the diskcache always returns a valid request and
request's
> url is not empty. Did you hit DCHECK(query_cache_results->request) in the
> original code?

Yes, diskcache always returns a valid request and request's url is not empty but
The query_cache_results->request is a optional argument of
keys(request, options) or other things. So, it can be null or empty.
If the name(query_cache_results->request) is confusing, we can rename it.

In the original code, we asserted that the request is not null if only
ignore_search is true.
But it could still be null or empty url if ignore_search is false.

Also, the spec says like this (e.g. keys()):
  ...
  2. If the optional argument request is omitted, then:
    1. For each fetching record entry of its request to response map,
       in key insertion order:
      1. Add entry.[[key]] to resultArray.
  3. Else:
    ...
    4. Let requestResponseArray be the result of running Query Cache algorithm
       passing a Request object that represents r and options as the arguments.

In terms of implementation, I thought that the number 2(for each) could merge
into 
the number 4(Query Cache) in order to reduce repeated codes unnecessarily.

> 
> If request is null or url is empty, I think we should immediately proceed to
the
> next entry instead of trying to read metadata.

So, I thought we should read metadata if request is null or url is empty.
What do you think?
If you are agree with me, I'll leave comments here.
If I'm wrong, please let me know better way :)

Thank you!

[nhiroki, 2016-08-08 12:40:55]

LGTM (sorry for my delayed reply)

https://codereview.chromium.org/1719103002/diff/220001/content/browser/cache_...
File content/browser/cache_storage/cache_storage_cache.cc (right):

https://codereview.chromium.org/1719103002/diff/220001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache.cc:632:
!query_cache_results->request->url.is_empty()) {
On 2016/08/02 17:37:42, zino wrote:
> On 2016/08/02 07:31:06, nhiroki wrote:
> > Hm... I assumed that the diskcache always returns a valid request and
> request's
> > url is not empty. Did you hit DCHECK(query_cache_results->request) in the
> > original code?
> 
> Yes, diskcache always returns a valid request and request's url is not empty
but
> The query_cache_results->request is a optional argument of
> keys(request, options) or other things. So, it can be null or empty.
> If the name(query_cache_results->request) is confusing, we can rename it.

Oh, I wrongly assumed |query_cache_results->request| is returned from a disk
cache. I think current naming is appropriate and you don't have to rename it, I
misread it though :p

> In the original code, we asserted that the request is not null if only
> ignore_search is true.
> But it could still be null or empty url if ignore_search is false.
> 
> Also, the spec says like this (e.g. keys()):
>   ...
>   2. If the optional argument request is omitted, then:
>     1. For each fetching record entry of its request to response map,
>        in key insertion order:
>       1. Add entry.[[key]] to resultArray.
>   3. Else:
>     ...
>     4. Let requestResponseArray be the result of running Query Cache algorithm
>        passing a Request object that represents r and options as the
arguments.
> 
> In terms of implementation, I thought that the number 2(for each) could merge
> into 
> the number 4(Query Cache) in order to reduce repeated codes unnecessarily.
> 
> > 
> > If request is null or url is empty, I think we should immediately proceed to
> the
> > next entry instead of trying to read metadata.
> 
> So, I thought we should read metadata if request is null or url is empty.
> What do you think?
> If you are agree with me, I'll leave comments here.
> If I'm wrong, please let me know better way :)
> 
> Thank you!

Looks good. Thank you for the clarification :)

https://codereview.chromium.org/1719103002/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-keys.js
(right):

https://codereview.chromium.org/1719103002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-keys.js:43:
{ignoreSearch: true})
Args don't need 4-spaces indent.

[zino, 2016-08-08 16:36:03]

Thank you for review.

I addressed all of comments.

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

https://codereview.chromium.org/1719103002/diff/220001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache_unittest.cc:1057:
expected_keys.push_back(body_request_with_query_.url.spec());
On 2016/08/02 12:06:20, jkarlin wrote:
> You can use vector initializers now, so this can probably be something like:
> 
> std::vector<std::string expected_keys = {body_request_.url.spec(),
> body_request_with_query_.url.spec()};

Done.

https://codereview.chromium.org/1719103002/diff/220001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache_unittest.cc:1075:
expected_keys.push_back(body_request_with_query_.url.spec());
On 2016/08/02 12:06:20, jkarlin wrote:
> Ditto

Done.

https://codereview.chromium.org/1719103002/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-keys.js
(right):

https://codereview.chromium.org/1719103002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-keys.js:43:
{ignoreSearch: true})
On 2016/08/08 12:40:54, nhiroki (slow) wrote:
> Args don't need 4-spaces indent.

Done.

https://codereview.chromium.org/1719103002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-keys.js:99:
assert_request_array_equivalent(
On 2016/08/02 12:06:20, jkarlin wrote:
> Add a TODO to remove the "equivalent" part and delete the
> assert_request_array_equivalent function once keys are returned in insertion
> order.

Done.

[zino, 2016-08-09 15:14:07]

jkarlin@

Could you review this patch? Would you mind if I commit this?

[jkarlin, 2016-08-09 18:34:29]

lgtm with a couple of nits.

Sorry for the slow response, I was OOO for a few days.

Please run 'git cl format' before uploading in case I missed anything.

https://codereview.chromium.org/1719103002/diff/240001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/resources/test-helpers.js
(right):

https://codereview.chromium.org/1719103002/diff/240001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/resources/test-helpers.js:250:
// TODO(zino): Should remove this function if once keys() returns request
Remove "if"

https://codereview.chromium.org/1719103002/diff/240001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-keys.js
(right):

https://codereview.chromium.org/1719103002/diff/240001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-keys.js:45:
// assert_request_array_equivalent() if once keys() returns request
remove "if"

https://codereview.chromium.org/1719103002/diff/240001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-keys.js:104:
// assert_request_array_equivalent() if once keys() returns request
Remove "if"

[zino, 2016-08-09 21:54:00]

Thank you for review.

I ran 'git cl format' and 'git cl presubmit' but there was no error.

Thanks!

https://codereview.chromium.org/1719103002/diff/240001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/resources/test-helpers.js
(right):

https://codereview.chromium.org/1719103002/diff/240001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/resources/test-helpers.js:250:
// TODO(zino): Should remove this function if once keys() returns request
On 2016/08/09 18:34:29, jkarlin wrote:
> Remove "if"

Done.

https://codereview.chromium.org/1719103002/diff/240001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-keys.js
(right):

https://codereview.chromium.org/1719103002/diff/240001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-keys.js:45:
// assert_request_array_equivalent() if once keys() returns request
On 2016/08/09 18:34:29, jkarlin wrote:
> remove "if"

Done.

https://codereview.chromium.org/1719103002/diff/240001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-keys.js:104:
// assert_request_array_equivalent() if once keys() returns request
On 2016/08/09 18:34:29, jkarlin wrote:
> Remove "if"

Done.

[zino, 2016-08-09 21:54:14]

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

[zino, 2016-08-09 21:54:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
nhiroki@chromium.org, jkarlin@chromium.org
Link to the patchset: https://codereview.chromium.org/1719103002/#ps260001
(title: "nits")

[commit-bot: I haz the power, 2016-08-09 21:54:55]

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-08-09 23:39:02]

Description was changed from

==========
CacheStorage: Expand cache.keys() method.

Current implementation of the method is always return all of keys even if it
takes optional parameters. According to ServiceWorkerSpec[1], the method should
take a request object and cache query options[2] as optional parameters.
This CL is expanding the method as per the spec.

[1] http://slightlyoff.github.io/ServiceWorker/spec/service_worker/#cache-keys
[2] Currenly, only supports ignoreSearch option like matchAll(), delete()

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

to

==========
CacheStorage: Expand cache.keys() method.

Current implementation of the method is always return all of keys even if it
takes optional parameters. According to ServiceWorkerSpec[1], the method should
take a request object and cache query options[2] as optional parameters.
This CL is expanding the method as per the spec.

[1] http://slightlyoff.github.io/ServiceWorker/spec/service_worker/#cache-keys
[2] Currenly, only supports ignoreSearch option like matchAll(), delete()

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

[commit-bot: I haz the power, 2016-08-09 23:39:03]

Committed patchset #8 (id:260001)

[commit-bot: I haz the power, 2016-08-09 23:41:21]

Description was changed from

==========
CacheStorage: Expand cache.keys() method.

Current implementation of the method is always return all of keys even if it
takes optional parameters. According to ServiceWorkerSpec[1], the method should
take a request object and cache query options[2] as optional parameters.
This CL is expanding the method as per the spec.

[1] http://slightlyoff.github.io/ServiceWorker/spec/service_worker/#cache-keys
[2] Currenly, only supports ignoreSearch option like matchAll(), delete()

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

to

==========
CacheStorage: Expand cache.keys() method.

Current implementation of the method is always return all of keys even if it
takes optional parameters. According to ServiceWorkerSpec[1], the method should
take a request object and cache query options[2] as optional parameters.
This CL is expanding the method as per the spec.

[1] http://slightlyoff.github.io/ServiceWorker/spec/service_worker/#cache-keys
[2] Currenly, only supports ignoreSearch option like matchAll(), delete()

BUG=520784

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

[commit-bot: I haz the power, 2016-08-09 23:41:25]

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