Implement CachedMetadata handling on MojoAsyncResourceHandler

Implement CachedMetadata handling on MojoAsyncResourceHandler

With this CL, MojoAsyncResourcehandler dispatches the cached metadata if it
is attached with the net::URLRequest.

BUG=603396
Review-Url: https://codereview.chromium.org/2629513003
Cr-Commit-Position: refs/heads/master@{#444302}
Committed: https://chromium.googlesource.com/chromium/src/+/2a525f9d4ab7f144b179ed013482e3ec98dfd6d0

[yhirano, 2017-01-12 06:51:11]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 06:51:34]

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

[yhirano, 2017-01-12 07:01:59]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 07:02:23]

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

[yhirano, 2017-01-12 07:06:20]

yhirano@chromium.org changed reviewers:
+ tzik@chromium.org

[yhirano, 2017-01-12 07:32:02]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 07:32:27]

Dry run: 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, 2017-01-12 08:54:14]

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

[commit-bot: I haz the power, 2017-01-12 08:54:15]

Dry run: This issue passed the CQ dry run.

[tzik, 2017-01-13 02:41:23]

lgtm

https://codereview.chromium.org/2629513003/diff/40001/content/browser/loader/...
File content/browser/loader/mojo_async_resource_handler_unittest.cc (right):

https://codereview.chromium.org/2629513003/diff/40001/content/browser/loader/...
content/browser/loader/mojo_async_resource_handler_unittest.cc:252: metadata_ =
metadata;
std::move()

[yhirano, 2017-01-13 12:34:53]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-13 12:35:09]

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

[yhirano, 2017-01-13 12:36:13]

https://codereview.chromium.org/2629513003/diff/40001/content/browser/loader/...
File content/browser/loader/mojo_async_resource_handler_unittest.cc (right):

https://codereview.chromium.org/2629513003/diff/40001/content/browser/loader/...
content/browser/loader/mojo_async_resource_handler_unittest.cc:252: metadata_ =
metadata;
On 2017/01/13 02:41:23, tzik wrote:
> std::move()

Done.

[commit-bot: I haz the power, 2017-01-13 12:36:48]

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

[commit-bot: I haz the power, 2017-01-13 12:36:49]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[yhirano, 2017-01-13 12:55:52]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-13 12:56:01]

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

[yhirano, 2017-01-13 12:56:13]

yhirano@chromium.org changed reviewers:
+ dcheng@chromium.org, horo@chromium.org, jam@chromium.org, mmenke@chromium.org

[yhirano, 2017-01-13 12:56:13]

+mmenke@ for content/browser/loader
+horo@ for content/browser/service_worker and content/renderer/service_worker
+dcheng@ for content/common/url_loader.mojom
+jam@ for content overall

[commit-bot: I haz the power, 2017-01-13 15:22:03]

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

[commit-bot: I haz the power, 2017-01-13 15:22:04]

Dry run: This issue passed the CQ dry run.

[mmenke, 2017-01-13 15:55:04]

Quick comment.  I'll do a more thorough review this afternoon.

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
File content/browser/loader/mojo_async_resource_handler.cc (right):

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
content/browser/loader/mojo_async_resource_handler.cc:190:
std::vector<uint8_t>(ptr, ptr + metadata->size()));
Not sure how big this tends to be, but can we somehow avoid a double-copy here? 
(Once to the vector, once to the IPC buffer)

[yhirano, 2017-01-13 16:13:11]

Description was changed from

==========
Implement CachedMetadata handling on MojoAsyncResourceHandler

With this CL, MojoAsyncResourcehandler dispatches the cached metadata if it
is attached with the net::URLRequest.

BUG=603396
==========

to

==========
Implement CachedMetadata handling on MojoAsyncResourceHandler

With this CL, MojoAsyncResourcehandler dispatches the cached metadata if it
is attached with the net::URLRequest.

BUG=603396
==========

[yhirano, 2017-01-13 16:13:11]

yhirano@chromium.org changed reviewers:
+ yzshen@chromium.org

[yhirano, 2017-01-13 16:14:25]

On 2017/01/13 15:55:04, mmenke wrote:
> Quick comment.  I'll do a more thorough review this afternoon.
> 
>
https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
> File content/browser/loader/mojo_async_resource_handler.cc (right):
> 
>
https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
> content/browser/loader/mojo_async_resource_handler.cc:190:
> std::vector<uint8_t>(ptr, ptr + metadata->size()));
> Not sure how big this tends to be, but can we somehow avoid a double-copy
here? 
> (Once to the vector, once to the IPC buffer)

Hmm, I don't know. +yzshen@: Is there any way to avoid the copy?

[jam, 2017-01-13 23:12:00]

lgtm

[dcheng, 2017-01-14 11:30:17]

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
File content/browser/loader/mojo_async_resource_handler.cc (right):

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
content/browser/loader/mojo_async_resource_handler.cc:190:
std::vector<uint8_t>(ptr, ptr + metadata->size()));
On 2017/01/13 15:55:04, mmenke wrote:
> Not sure how big this tends to be, but can we somehow avoid a double-copy
here? 
> (Once to the vector, once to the IPC buffer)

This could be an interesting Mojo feature request. Mojo has something called
CArray (roughly analogous to StringPiece), but usually that's used in
StructTraits. So I suspect that if we were to wrap this in a struct with
typemapping, we would be able to remove a copy... but it would be nice not to
have to do that.

https://codereview.chromium.org/2629513003/diff/70001/content/child/url_loade...
File content/child/url_loader_client_impl.cc (right):

https://codereview.chromium.org/2629513003/diff/70001/content/child/url_loade...
content/child/url_loader_client_impl.cc:141: request_id_, std::vector<char>(ptr,
ptr + data.size())));
And I guess this is another copy =(

Is there any way we can be consistent with our types here? It's uint8_t on the
sending side and char on the receiving side.

[horo, 2017-01-16 01:48:16]

service_worker/ LGTM

[yhirano, 2017-01-16 05:25:46]

https://codereview.chromium.org/2629513003/diff/70001/content/child/url_loade...
File content/child/url_loader_client_impl.cc (right):

https://codereview.chromium.org/2629513003/diff/70001/content/child/url_loade...
content/child/url_loader_client_impl.cc:141: request_id_, std::vector<char>(ptr,
ptr + data.size())));
On 2017/01/14 11:30:17, dcheng wrote:
> And I guess this is another copy =(
> 

This is needed to reuse the current ResourceDispatcher code. I want to do that
during migration unless there's a significant performance regression. tzik@ will
measure the performance in this quarter, so I would like to fix performance
regressions after that. Are you fine with the plan?

> Is there any way we can be consistent with our types here? It's uint8_t on the
> sending side and char on the receiving side.

It's char on both sending and receiving sides. You can see reinterpret_cast here
and in MojoAsyncResourceHandler.

[yhirano, 2017-01-16 05:40:41]

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
File content/browser/loader/mojo_async_resource_handler.cc (right):

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
content/browser/loader/mojo_async_resource_handler.cc:190:
std::vector<uint8_t>(ptr, ptr + metadata->size()));
On 2017/01/14 11:30:17, dcheng wrote:
> On 2017/01/13 15:55:04, mmenke wrote:
> > Not sure how big this tends to be, but can we somehow avoid a double-copy
> here? 
> > (Once to the vector, once to the IPC buffer)
> 
> This could be an interesting Mojo feature request. Mojo has something called
> CArray (roughly analogous to StringPiece), but usually that's used in
> StructTraits. So I suspect that if we were to wrap this in a struct with
> typemapping, we would be able to remove a copy... but it would be nice not to
> have to do that.

How about adding net::IOBufferWithSize support to IPC?

[mmenke, 2017-01-17 18:39:57]

LGTM!

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
File content/browser/loader/mojo_async_resource_handler.cc (right):

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
content/browser/loader/mojo_async_resource_handler.cc:187: const uint8_t* ptr =
reinterpret_cast<const uint8_t*>(metadata->data());
nit:  Maybe ptr -> data?  data isn't exactly a detailed description, but "ptr"
seems even worse.

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
content/browser/loader/mojo_async_resource_handler.cc:190:
std::vector<uint8_t>(ptr, ptr + metadata->size()));
On 2017/01/16 05:40:41, yhirano wrote:
> On 2017/01/14 11:30:17, dcheng wrote:
> > On 2017/01/13 15:55:04, mmenke wrote:
> > > Not sure how big this tends to be, but can we somehow avoid a double-copy
> > here? 
> > > (Once to the vector, once to the IPC buffer)
> > 
> > This could be an interesting Mojo feature request. Mojo has something called
> > CArray (roughly analogous to StringPiece), but usually that's used in
> > StructTraits. So I suspect that if we were to wrap this in a struct with
> > typemapping, we would be able to remove a copy... but it would be nice not
to
> > have to do that.
> 
> How about adding net::IOBufferWithSize support to IPC?

It's a weird enough type that I don't think that's a great idea (What if we have
another net::IOBuffer subtype and a length, for instance?).  Some general way to
send an array + length as a vector without a copy seems like it would be better,
to me.

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
File content/browser/loader/mojo_async_resource_handler_unittest.cc (right):

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
content/browser/loader/mojo_async_resource_handler_unittest.cc:432:
url_loader_client_.cached_metadata().size()));
Suggest moving this std::string call into
TestURLLoaderClient::OnReceiveCachedMetadata

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
File content/browser/loader/test_url_loader_client.cc (right):

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
content/browser/loader/test_url_loader_client.cc:49: const std::vector<uint8_t>&
data) {
EXPECT_FALSE(has_received_cached_metadata_);
EXPECT_TRUE(cached_metadata_.empty());
EXPECT_TRUE(has_received_response_);
EXPECT_TRUE(response_body_.empty());
EXPECT_FALSE(response_body_.empty(has_received_completion_));

Can also add EXPECT_FALSE(has_received_cached_metadata_); to the events that
occur after this (Complete, StartLoadingResponseBody, DataDownloaded)

https://codereview.chromium.org/2629513003/diff/70001/content/child/url_loade...
File content/child/url_loader_client_impl.cc (right):

https://codereview.chromium.org/2629513003/diff/70001/content/child/url_loade...
content/child/url_loader_client_impl.cc:139: const char* ptr =
reinterpret_cast<const char*>(data.data());
Again, suggest replacing ptr with data

[mmenke, 2017-01-17 18:40:52]

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
File content/browser/loader/mojo_async_resource_handler.cc (right):

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
content/browser/loader/mojo_async_resource_handler.cc:191: }
Also, include <vector> (Same probably goes for both test cc files as well.

[dcheng, 2017-01-18 00:23:53]

mojo lgtm with a nit. hopefully we can make this more efficient with the right
primitives (though some of this is related to the fact that we aren't consistent
about storing data buffers as vector<uint8_t>)

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
File content/browser/loader/mojo_async_resource_handler_unittest.cc (right):

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
content/browser/loader/mojo_async_resource_handler_unittest.cc:397:
strncpy(metadata->data(), "hello", 5);
Please use memcpy (strncpy doesn't null terminate, which is normally desired. We
don't want that here, but that makes it harder to audit usage of strncpy
throughout the codebase)

https://codereview.chromium.org/2629513003/diff/70001/content/child/url_loade...
File content/child/url_loader_client_impl.cc (right):

https://codereview.chromium.org/2629513003/diff/70001/content/child/url_loade...
content/child/url_loader_client_impl.cc:141: request_id_, std::vector<char>(ptr,
ptr + data.size())));
On 2017/01/16 05:25:46, yhirano wrote:
> On 2017/01/14 11:30:17, dcheng wrote:
> > And I guess this is another copy =(
> > 
> 
> This is needed to reuse the current ResourceDispatcher code. I want to do that
> during migration unless there's a significant performance regression. tzik@
will
> measure the performance in this quarter, so I would like to fix performance
> regressions after that. Are you fine with the plan?
> 
> > Is there any way we can be consistent with our types here? It's uint8_t on
the
> > sending side and char on the receiving side.
> 
> It's char on both sending and receiving sides. You can see reinterpret_cast
here
> and in MojoAsyncResourceHandler.

Sort of... we have to reinterpret_cast the original buffer to a uint8_t* to
construct a std::vector<uint8_t> out of it, and then we have to reinterpret_cast
it back on the other side.

It seems like we should be fixing the representation here to use
std::vector<uint8_t> throughout (though that's obviously out of scope of this
patch)

[yhirano, 2017-01-18 06:17:00]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 06:17:28]

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

[yhirano, 2017-01-18 06:22:43]

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
File content/browser/loader/mojo_async_resource_handler.cc (right):

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
content/browser/loader/mojo_async_resource_handler.cc:187: const uint8_t* ptr =
reinterpret_cast<const uint8_t*>(metadata->data());
On 2017/01/17 18:39:57, mmenke wrote:
> nit:  Maybe ptr -> data?  data isn't exactly a detailed description, but "ptr"
> seems even worse.

Done.

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
content/browser/loader/mojo_async_resource_handler.cc:190:
std::vector<uint8_t>(ptr, ptr + metadata->size()));
On 2017/01/17 18:39:57, mmenke wrote:
> On 2017/01/16 05:40:41, yhirano wrote:
> > On 2017/01/14 11:30:17, dcheng wrote:
> > > On 2017/01/13 15:55:04, mmenke wrote:
> > > > Not sure how big this tends to be, but can we somehow avoid a
double-copy
> > > here? 
> > > > (Once to the vector, once to the IPC buffer)
> > > 
> > > This could be an interesting Mojo feature request. Mojo has something
called
> > > CArray (roughly analogous to StringPiece), but usually that's used in
> > > StructTraits. So I suspect that if we were to wrap this in a struct with
> > > typemapping, we would be able to remove a copy... but it would be nice not
> to
> > > have to do that.
> > 
> > How about adding net::IOBufferWithSize support to IPC?
> 
> It's a weird enough type that I don't think that's a great idea (What if we
have
> another net::IOBuffer subtype and a length, for instance?).  Some general way
to
> send an array + length as a vector without a copy seems like it would be
better,
> to me.

OK, let's leave it be. We'll be able to remove the copy when migrating
parameters. Note that we have this copy in AsyncResourceHandler too.

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
content/browser/loader/mojo_async_resource_handler.cc:191: }
On 2017/01/17 18:40:52, mmenke wrote:
> Also, include <vector> (Same probably goes for both test cc files as well.

Done.

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
File content/browser/loader/mojo_async_resource_handler_unittest.cc (right):

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
content/browser/loader/mojo_async_resource_handler_unittest.cc:397:
strncpy(metadata->data(), "hello", 5);
On 2017/01/18 00:23:52, dcheng wrote:
> Please use memcpy (strncpy doesn't null terminate, which is normally desired.
We
> don't want that here, but that makes it harder to audit usage of strncpy
> throughout the codebase)

Done.

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
content/browser/loader/mojo_async_resource_handler_unittest.cc:432:
url_loader_client_.cached_metadata().size()));
On 2017/01/17 18:39:57, mmenke wrote:
> Suggest moving this std::string call into
> TestURLLoaderClient::OnReceiveCachedMetadata

Done.

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
File content/browser/loader/test_url_loader_client.cc (right):

https://codereview.chromium.org/2629513003/diff/70001/content/browser/loader/...
content/browser/loader/test_url_loader_client.cc:49: const std::vector<uint8_t>&
data) {
On 2017/01/17 18:39:57, mmenke wrote:
> EXPECT_FALSE(has_received_cached_metadata_);
> EXPECT_TRUE(cached_metadata_.empty());
> EXPECT_TRUE(has_received_response_);
> EXPECT_TRUE(response_body_.empty());
> EXPECT_FALSE(response_body_.empty(has_received_completion_));
> 
> Can also add EXPECT_FALSE(has_received_cached_metadata_); to the events that
> occur after this (Complete, StartLoadingResponseBody, DataDownloaded)

I added former ones. I'll add latter ones in
https://codereview.chromium.org/2633123002/.

https://codereview.chromium.org/2629513003/diff/70001/content/child/url_loade...
File content/child/url_loader_client_impl.cc (right):

https://codereview.chromium.org/2629513003/diff/70001/content/child/url_loade...
content/child/url_loader_client_impl.cc:139: const char* ptr =
reinterpret_cast<const char*>(data.data());
On 2017/01/17 18:39:57, mmenke wrote:
> Again, suggest replacing ptr with data

I'm using data_ptr as we are already using the name.

https://codereview.chromium.org/2629513003/diff/70001/content/child/url_loade...
content/child/url_loader_client_impl.cc:141: request_id_, std::vector<char>(ptr,
ptr + data.size())));
On 2017/01/18 00:23:53, dcheng wrote:
> On 2017/01/16 05:25:46, yhirano wrote:
> > On 2017/01/14 11:30:17, dcheng wrote:
> > > And I guess this is another copy =(
> > > 
> > 
> > This is needed to reuse the current ResourceDispatcher code. I want to do
that
> > during migration unless there's a significant performance regression. tzik@
> will
> > measure the performance in this quarter, so I would like to fix performance
> > regressions after that. Are you fine with the plan?
> > 
> > > Is there any way we can be consistent with our types here? It's uint8_t on
> the
> > > sending side and char on the receiving side.
> > 
> > It's char on both sending and receiving sides. You can see reinterpret_cast
> here
> > and in MojoAsyncResourceHandler.
> 
> Sort of... we have to reinterpret_cast the original buffer to a uint8_t* to
> construct a std::vector<uint8_t> out of it, and then we have to
reinterpret_cast
> it back on the other side.
> 
> It seems like we should be fixing the representation here to use
> std::vector<uint8_t> throughout (though that's obviously out of scope of this
> patch)

Acknowledged.

[yhirano, 2017-01-18 06:22:55]

The CQ bit was unchecked by yhirano@chromium.org

[yhirano, 2017-01-18 06:22:55]

The CQ bit was checked by yhirano@chromium.org

[yhirano, 2017-01-18 06:22:56]

The patchset sent to the CQ was uploaded after l-g-t-m from tzik@chromium.org,
jam@chromium.org, dcheng@chromium.org, horo@chromium.org, mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2629513003/#ps90001
(title: "fix")

[commit-bot: I haz the power, 2017-01-18 06:23:03]

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

[yhirano, 2017-01-18 06:46:45]

The CQ bit was unchecked by yhirano@chromium.org

[yhirano, 2017-01-18 06:48:35]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 06:48:50]

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

[yhirano, 2017-01-18 06:50:19]

The CQ bit was checked by yhirano@chromium.org

[yhirano, 2017-01-18 06:50:19]

The CQ bit was unchecked by yhirano@chromium.org

[yhirano, 2017-01-18 06:50:19]

The patchset sent to the CQ was uploaded after l-g-t-m from tzik@chromium.org,
jam@chromium.org, dcheng@chromium.org, horo@chromium.org, mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2629513003/#ps110001
(title: "fix")

[commit-bot: I haz the power, 2017-01-18 06:50:36]

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, 2017-01-18 08:16:49]

CQ is committing da patch.

Bot data: {"patchset_id": 110001, "attempt_start_ts": 1484722219409510,
"parent_rev": "f0512977b1a5996b5fc734527726a54aa22f583e", "commit_rev":
"2a525f9d4ab7f144b179ed013482e3ec98dfd6d0"}

[commit-bot: I haz the power, 2017-01-18 08:17:20]

Description was changed from

==========
Implement CachedMetadata handling on MojoAsyncResourceHandler

With this CL, MojoAsyncResourcehandler dispatches the cached metadata if it
is attached with the net::URLRequest.

BUG=603396
==========

to

==========
Implement CachedMetadata handling on MojoAsyncResourceHandler

With this CL, MojoAsyncResourcehandler dispatches the cached metadata if it
is attached with the net::URLRequest.

BUG=603396

Review-Url: https://codereview.chromium.org/2629513003
Cr-Commit-Position: refs/heads/master@{#444302}
Committed:
https://chromium.googlesource.com/chromium/src/+/2a525f9d4ab7f144b179ed013482...
==========

[commit-bot: I haz the power, 2017-01-18 08:17:22]

Committed patchset #7 (id:110001) as
https://chromium.googlesource.com/chromium/src/+/2a525f9d4ab7f144b179ed013482...