cc: Keep PaintImage in DrawImage.

cc: Keep PaintImage in DrawImage.

Currently we only store the SkImage in DrawImage during discardable
meta-data generation. This change extends that to store PaintImage so
it can be used by the checker-imaging system during raster.

It also updates checker-imaging to not checker on static and partially
loaded images.

BUG=722570
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2857923004
Cr-Commit-Position: refs/heads/master@{#472379}
Committed: https://chromium.googlesource.com/chromium/src/+/d5e13bff12f02c4dc487d87451b329754ce1c1cc

[Khushal, 2017-05-03 22:40:36]

Description was changed from

==========
cc: Keep PaintImage in DrawImage.

Currently we only store the SkImage in DrawImage during discardable
meta-data generation. This change extends that to store PaintImage so
it can be used by the checker-imaging system during raster.

BUG=715886
==========

to

==========
cc: Keep PaintImage in DrawImage.

Currently we only store the SkImage in DrawImage during discardable
meta-data generation. This change extends that to store PaintImage so
it can be used by the checker-imaging system during raster.

BUG=715886
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[Khushal, 2017-05-03 22:41:02]

khushalsagar@chromium.org changed reviewers:
+ vmpstr@chromium.org

[Khushal, 2017-05-09 04:42:25]

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

[commit-bot: I haz the power, 2017-05-09 04:43:28]

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-05-09 05:36:24]

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

[commit-bot: I haz the power, 2017-05-09 05:36:25]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[vmpstr, 2017-05-09 22:42:54]

This looks good. I have a patch that is out for review that adds an extra
parameter to the ctor of PaintImage. What order did you want to land these in,
since one will have to rebase on the other.

https://codereview.chromium.org/2857923004/diff/20001/cc/paint/discardable_im...
File cc/paint/discardable_image_store.cc (right):

https://codereview.chromium.org/2857923004/diff/20001/cc/paint/discardable_im...
cc/paint/discardable_image_store.cc:245:
(*image_id_to_rect_)[paint_image.sk_image()->uniqueID()].Union(image_rect);
can you make a comment here to explain why we use the sk image id instead of the
stable id

[Khushal, 2017-05-09 22:59:30]

On 2017/05/09 22:42:54, vmpstr wrote:
> This looks good. I have a patch that is out for review that adds an extra
> parameter to the ctor of PaintImage. What order did you want to land these in,
> since one will have to rebase on the other.

I can go after your patch.

> 
>
https://codereview.chromium.org/2857923004/diff/20001/cc/paint/discardable_im...
> File cc/paint/discardable_image_store.cc (right):
> 
>
https://codereview.chromium.org/2857923004/diff/20001/cc/paint/discardable_im...
> cc/paint/discardable_image_store.cc:245:
> (*image_id_to_rect_)[paint_image.sk_image()->uniqueID()].Union(image_rect);
> can you make a comment here to explain why we use the sk image id instead of
the
> stable id

[vmpstr, 2017-05-12 22:15:42]

https://codereview.chromium.org/2857923004/diff/40001/cc/paint/image_id.h
File cc/paint/image_id.h (right):

https://codereview.chromium.org/2857923004/diff/40001/cc/paint/image_id.h#new...
cc/paint/image_id.h:16: using ImageId = PaintImage::Id;
I just want to use PaintImage::Id

https://codereview.chromium.org/2857923004/diff/40001/cc/raster/image_hijack_...
File cc/raster/image_hijack_canvas.cc (right):

https://codereview.chromium.org/2857923004/diff/40001/cc/raster/image_hijack_...
cc/raster/image_hijack_canvas.cc:19: PaintImage::Id s_canvas_paint_image_id =
PaintImage::GetNextId();
I think this should be a static int instead of GetNextId call.

https://codereview.chromium.org/2857923004/diff/40001/cc/raster/image_hijack_...
cc/raster/image_hijack_canvas.cc:102: image_decode_cache,
sk_ref_sp(const_cast<SkImage*>(image)),
image is already non const?

https://codereview.chromium.org/2857923004/diff/40001/cc/tiles/image_controll...
File cc/tiles/image_controller.cc (right):

https://codereview.chromium.org/2857923004/diff/40001/cc/tiles/image_controll...
cc/tiles/image_controller.cc:25: paint_image_id_(PaintImage::GetNextId()),
This is something that we need to talk about I think. We need to be able to
plumb ... something.

Like, I think the cache should know the stable id for us to properly clean up
partially loaded images. Also, the checker tracker has to also never checker a
skia id which was decoded by the decode() call.

[Khushal, 2017-05-13 02:45:50]

https://codereview.chromium.org/2857923004/diff/40001/cc/paint/image_id.h
File cc/paint/image_id.h (right):

https://codereview.chromium.org/2857923004/diff/40001/cc/paint/image_id.h#new...
cc/paint/image_id.h:16: using ImageId = PaintImage::Id;
On 2017/05/12 22:15:42, vmpstr wrote:
> I just want to use PaintImage::Id

Do you mind if that is in a follow-up patch?

https://codereview.chromium.org/2857923004/diff/40001/cc/raster/image_hijack_...
File cc/raster/image_hijack_canvas.cc (right):

https://codereview.chromium.org/2857923004/diff/40001/cc/raster/image_hijack_...
cc/raster/image_hijack_canvas.cc:19: PaintImage::Id s_canvas_paint_image_id =
PaintImage::GetNextId();
On 2017/05/12 22:15:42, vmpstr wrote:
> I think this should be a static int instead of GetNextId call. 

Do you mean like kNonLazyStableId?

https://codereview.chromium.org/2857923004/diff/40001/cc/tiles/image_controll...
File cc/tiles/image_controller.cc (right):

https://codereview.chromium.org/2857923004/diff/40001/cc/tiles/image_controll...
cc/tiles/image_controller.cc:25: paint_image_id_(PaintImage::GetNextId()),
On 2017/05/12 22:15:42, vmpstr wrote:
> This is something that we need to talk about I think. We need to be able to
> plumb ... something.
> 
> Like, I think the cache should know the stable id for us to properly clean up
> partially loaded images. Also, the checker tracker has to also never checker a
> skia id which was decoded by the decode() call.

We'll definitely need PaintImage plumbed to cc, instead of SkImage, for the
decode API. That shouldn't be difficult. And that should handle the
checker-imaging.

For clearing the caches, I think we should address that in the doc that
discusses unifying blink and cc's caching.

[vmpstr, 2017-05-15 15:02:55]

https://codereview.chromium.org/2857923004/diff/40001/cc/paint/image_id.h
File cc/paint/image_id.h (right):

https://codereview.chromium.org/2857923004/diff/40001/cc/paint/image_id.h#new...
cc/paint/image_id.h:16: using ImageId = PaintImage::Id;
On 2017/05/13 02:45:50, Khushal wrote:
> On 2017/05/12 22:15:42, vmpstr wrote:
> > I just want to use PaintImage::Id
> 
> Do you mind if that is in a follow-up patch?

I'd prefer we do this here.

https://codereview.chromium.org/2857923004/diff/40001/cc/raster/image_hijack_...
File cc/raster/image_hijack_canvas.cc (right):

https://codereview.chromium.org/2857923004/diff/40001/cc/raster/image_hijack_...
cc/raster/image_hijack_canvas.cc:19: PaintImage::Id s_canvas_paint_image_id =
PaintImage::GetNextId();
On 2017/05/13 02:45:50, Khushal wrote:
> On 2017/05/12 22:15:42, vmpstr wrote:
> > I think this should be a static int instead of GetNextId call. 
> 
> Do you mean like kNonLazyStableId?

Either kNonLazyStableId or some other identifier, but yes, any static value
other than GetNextId()

https://codereview.chromium.org/2857923004/diff/40001/cc/tiles/image_controll...
File cc/tiles/image_controller.cc (right):

https://codereview.chromium.org/2857923004/diff/40001/cc/tiles/image_controll...
cc/tiles/image_controller.cc:25: paint_image_id_(PaintImage::GetNextId()),
On 2017/05/13 02:45:50, Khushal wrote:
> On 2017/05/12 22:15:42, vmpstr wrote:
> > This is something that we need to talk about I think. We need to be able to
> > plumb ... something.
> > 
> > Like, I think the cache should know the stable id for us to properly clean
up
> > partially loaded images. Also, the checker tracker has to also never checker
a
> > skia id which was decoded by the decode() call.
> 
> We'll definitely need PaintImage plumbed to cc, instead of SkImage, for the
> decode API. That shouldn't be difficult. And that should handle the
> checker-imaging.
> 
> For clearing the caches, I think we should address that in the doc that
> discusses unifying blink and cc's caching.

My concern is that it doesn't feel that we're cleanly defining the boundary
where PaintImage has data generated by blink and where it's being used as a
placeholder in draw images. I think we need to tune this to ensure that we
always get the right paint image (the id specifically). I think this should
naturally fall out from removing image hijack canvas, since we'll be passing the
right id there. 

the problem is that if I want to add any code that depends on the id into the
cache (such as clearing old entries), I need to know that each call has the
right id. If we sometimes pass the right id, and sometimes we just generate a
dummy one, then we'll end up in a situation where we might be clearing things we
didn't mean to, or not clearing things that we should.

[Khushal, 2017-05-15 21:22:15]

https://codereview.chromium.org/2857923004/diff/40001/cc/paint/image_id.h
File cc/paint/image_id.h (right):

https://codereview.chromium.org/2857923004/diff/40001/cc/paint/image_id.h#new...
cc/paint/image_id.h:16: using ImageId = PaintImage::Id;
On 2017/05/15 15:02:55, vmpstr wrote:
> On 2017/05/13 02:45:50, Khushal wrote:
> > On 2017/05/12 22:15:42, vmpstr wrote:
> > > I just want to use PaintImage::Id
> > 
> > Do you mind if that is in a follow-up patch?
> 
> I'd prefer we do this here.

Done.

https://codereview.chromium.org/2857923004/diff/40001/cc/raster/image_hijack_...
File cc/raster/image_hijack_canvas.cc (right):

https://codereview.chromium.org/2857923004/diff/40001/cc/raster/image_hijack_...
cc/raster/image_hijack_canvas.cc:19: PaintImage::Id s_canvas_paint_image_id =
PaintImage::GetNextId();
On 2017/05/15 15:02:55, vmpstr wrote:
> On 2017/05/13 02:45:50, Khushal wrote:
> > On 2017/05/12 22:15:42, vmpstr wrote:
> > > I think this should be a static int instead of GetNextId call. 
> > 
> > Do you mean like kNonLazyStableId?
> 
> Either kNonLazyStableId or some other identifier, but yes, any static value
> other than GetNextId()

Done. I added a kWrongStableId to mark it clearly that this use is incorrect.
Filed a bug as well about the cases where this is being used right now.

https://codereview.chromium.org/2857923004/diff/40001/cc/raster/image_hijack_...
cc/raster/image_hijack_canvas.cc:102: image_decode_cache,
sk_ref_sp(const_cast<SkImage*>(image)),
On 2017/05/12 22:15:42, vmpstr wrote:
> image is already non const?

Done.

https://codereview.chromium.org/2857923004/diff/40001/cc/tiles/image_controll...
File cc/tiles/image_controller.cc (right):

https://codereview.chromium.org/2857923004/diff/40001/cc/tiles/image_controll...
cc/tiles/image_controller.cc:25: paint_image_id_(PaintImage::GetNextId()),
On 2017/05/15 15:02:55, vmpstr wrote:
> On 2017/05/13 02:45:50, Khushal wrote:
> > On 2017/05/12 22:15:42, vmpstr wrote:
> > > This is something that we need to talk about I think. We need to be able
to
> > > plumb ... something.
> > > 
> > > Like, I think the cache should know the stable id for us to properly clean
> up
> > > partially loaded images. Also, the checker tracker has to also never
checker
> a
> > > skia id which was decoded by the decode() call.
> > 
> > We'll definitely need PaintImage plumbed to cc, instead of SkImage, for the
> > decode API. That shouldn't be difficult. And that should handle the
> > checker-imaging.
> > 
> > For clearing the caches, I think we should address that in the doc that
> > discusses unifying blink and cc's caching.
> 
> My concern is that it doesn't feel that we're cleanly defining the boundary
> where PaintImage has data generated by blink and where it's being used as a
> placeholder in draw images. I think we need to tune this to ensure that we
> always get the right paint image (the id specifically). I think this should
> naturally fall out from removing image hijack canvas, since we'll be passing
the
> right id there. 
> 
> the problem is that if I want to add any code that depends on the id into the
> cache (such as clearing old entries), I need to know that each call has the
> right id. If we sometimes pass the right id, and sometimes we just generate a
> dummy one, then we'll end up in a situation where we might be clearing things
we
> didn't mean to, or not clearing things that we should.


ImageHijackCanvas is one use-case. The other one is the decode API.
crbug.com/722559 has info about all these use-cases right now. The plumbing we
have so far is enough for the checker-imaging system, but the hijack canvas and
decode API will definitely need to be handled before the cache can make use of
this id.

[Khushal, 2017-05-15 21:25:39]

Description was changed from

==========
cc: Keep PaintImage in DrawImage.

Currently we only store the SkImage in DrawImage during discardable
meta-data generation. This change extends that to store PaintImage so
it can be used by the checker-imaging system during raster.

BUG=715886
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: Keep PaintImage in DrawImage.

Currently we only store the SkImage in DrawImage during discardable
meta-data generation. This change extends that to store PaintImage so
it can be used by the checker-imaging system during raster.

BUG=722570
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[Khushal, 2017-05-15 21:27:34]

Description was changed from

==========
cc: Keep PaintImage in DrawImage.

Currently we only store the SkImage in DrawImage during discardable
meta-data generation. This change extends that to store PaintImage so
it can be used by the checker-imaging system during raster.

BUG=722570
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: Keep PaintImage in DrawImage.

Currently we only store the SkImage in DrawImage during discardable
meta-data generation. This change extends that to store PaintImage so
it can be used by the checker-imaging system during raster.

It also updates checker-imaging to not checker on static and partially
loaded images.

BUG=722570
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[vmpstr, 2017-05-16 17:43:27]

https://codereview.chromium.org/2857923004/diff/60001/cc/paint/discardable_im...
File cc/paint/discardable_image_store.cc (right):

https://codereview.chromium.org/2857923004/diff/60001/cc/paint/discardable_im...
cc/paint/discardable_image_store.cc:192: PaintImage
paint_image(PaintImage::kWrongStableId, sk_ref_sp(image),
Maybe call it kUnknownStableId? If it's wrong, then why are you using it? :)

https://codereview.chromium.org/2857923004/diff/60001/cc/paint/discardable_im...
cc/paint/discardable_image_store.cc:252:
(*image_id_to_rect_)[paint_image.stable_id()].Union(image_rect);
I don't think it is, but is it possible for us to have two different uniqueIDs
in the same raster source with the same stable id?

https://codereview.chromium.org/2857923004/diff/60001/cc/paint/paint_image.h
File cc/paint/paint_image.h (right):

https://codereview.chromium.org/2857923004/diff/60001/cc/paint/paint_image.h#...
cc/paint/paint_image.h:28: static const Id kWrongStableId = -2;
s/Wrong/Unknown/ and also maybe initialize id_ to this?

https://codereview.chromium.org/2857923004/diff/60001/cc/raster/image_hijack_...
File cc/raster/image_hijack_canvas.cc (right):

https://codereview.chromium.org/2857923004/diff/60001/cc/raster/image_hijack_...
cc/raster/image_hijack_canvas.cc:17: PaintImage::Id s_canvas_paint_image_id =
PaintImage::GetNextId();
My concern is just initializing globals using function calls that rely on
globals. Is there another place you can put this?

https://codereview.chromium.org/2857923004/diff/60001/cc/tiles/checker_image_...
File cc/tiles/checker_image_tracker.cc (right):

https://codereview.chromium.org/2857923004/diff/60001/cc/tiles/checker_image_...
cc/tiles/checker_image_tracker.cc:123: image_id, DecodePolicy::SYNC_PERMANENT));
Why the change from ASYNC to SYNC_PERMANENT?

https://codereview.chromium.org/2857923004/diff/60001/cc/tiles/software_image...
File cc/tiles/software_image_decode_cache.h (right):

https://codereview.chromium.org/2857923004/diff/60001/cc/tiles/software_image...
cc/tiles/software_image_decode_cache.h:268: PaintImage image);
for functions that don't store the paint image, you should pass them by const
ref.

[Khushal, 2017-05-16 21:29:13]

https://codereview.chromium.org/2857923004/diff/60001/cc/paint/discardable_im...
File cc/paint/discardable_image_store.cc (right):

https://codereview.chromium.org/2857923004/diff/60001/cc/paint/discardable_im...
cc/paint/discardable_image_store.cc:192: PaintImage
paint_image(PaintImage::kWrongStableId, sk_ref_sp(image),
On 2017/05/16 17:43:27, vmpstr wrote:
> Maybe call it kUnknownStableId? If it's wrong, then why are you using it? :)

Done.
I thought calling it wrong will give me a push to eliminate these cases sooner.
:P

https://codereview.chromium.org/2857923004/diff/60001/cc/paint/discardable_im...
cc/paint/discardable_image_store.cc:252:
(*image_id_to_rect_)[paint_image.stable_id()].Union(image_rect);
On 2017/05/16 17:43:27, vmpstr wrote:
> I don't think it is, but is it possible for us to have two different uniqueIDs
> in the same raster source with the same stable id?

It shouldn't be. The uniqueID essentially represents the current frame for a
blink::Image and all elements using that image are dirtied and repainted
together if the image changes.

https://codereview.chromium.org/2857923004/diff/60001/cc/paint/paint_image.h
File cc/paint/paint_image.h (right):

https://codereview.chromium.org/2857923004/diff/60001/cc/paint/paint_image.h#...
cc/paint/paint_image.h:28: static const Id kWrongStableId = -2;
On 2017/05/16 17:43:27, vmpstr wrote:
> s/Wrong/Unknown/ and also maybe initialize id_ to this?

Done.

https://codereview.chromium.org/2857923004/diff/60001/cc/raster/image_hijack_...
File cc/raster/image_hijack_canvas.cc (right):

https://codereview.chromium.org/2857923004/diff/60001/cc/raster/image_hijack_...
cc/raster/image_hijack_canvas.cc:17: PaintImage::Id s_canvas_paint_image_id =
PaintImage::GetNextId();
On 2017/05/16 17:43:27, vmpstr wrote:
> My concern is just initializing globals using function calls that rely on
> globals. Is there another place you can put this?

Oh, I'm sorry. Forgot to remove this. Its not necessary anymore since we use the
kUnknownStableId for all these cases.

https://codereview.chromium.org/2857923004/diff/60001/cc/tiles/checker_image_...
File cc/tiles/checker_image_tracker.cc (right):

https://codereview.chromium.org/2857923004/diff/60001/cc/tiles/checker_image_...
cc/tiles/checker_image_tracker.cc:123: image_id, DecodePolicy::SYNC_PERMANENT));
On 2017/05/16 17:43:27, vmpstr wrote:
> Why the change from ASYNC to SYNC_PERMANENT?

Because the value is not being updated in the can_checker_image = false case, so
by default it should be sync.

https://codereview.chromium.org/2857923004/diff/60001/cc/tiles/software_image...
File cc/tiles/software_image_decode_cache.h (right):

https://codereview.chromium.org/2857923004/diff/60001/cc/tiles/software_image...
cc/tiles/software_image_decode_cache.h:268: PaintImage image);
On 2017/05/16 17:43:27, vmpstr wrote:
> for functions that don't store the paint image, you should pass them by const
> ref.

Done.

[Khushal, 2017-05-16 22:10:57]

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

[commit-bot: I haz the power, 2017-05-16 22:12:29]

Dry run: CQ is trying da patch.

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

[vmpstr, 2017-05-16 22:47:32]

lgtm % perftest fixes

[Khushal, 2017-05-16 22:52:40]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-05-16 22:52:40]

The patchset sent to the CQ was uploaded after l-g-t-m from vmpstr@chromium.org
Link to the patchset: https://codereview.chromium.org/2857923004/#ps120001
(title: "perftest")

[commit-bot: I haz the power, 2017-05-16 22:54:47]

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-05-17 00:37:07]

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

[commit-bot: I haz the power, 2017-05-17 00:37:08]

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

[Khushal, 2017-05-17 00:39:41]

The CQ bit was checked by khushalsagar@chromium.org

[commit-bot: I haz the power, 2017-05-17 00:40:57]

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-05-17 03:54:56]

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

[commit-bot: I haz the power, 2017-05-17 03:54:57]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Khushal, 2017-05-17 04:27:20]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-05-17 04:27:21]

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

[commit-bot: I haz the power, 2017-05-17 04:28:21]

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-05-17 08:08:56]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1494995240640070,
"parent_rev": "8f97866e8f0c1bae205fe0f3a4107d41ba8bae28", "commit_rev":
"d5e13bff12f02c4dc487d87451b329754ce1c1cc"}

[commit-bot: I haz the power, 2017-05-17 08:09:09]

Description was changed from

==========
cc: Keep PaintImage in DrawImage.

Currently we only store the SkImage in DrawImage during discardable
meta-data generation. This change extends that to store PaintImage so
it can be used by the checker-imaging system during raster.

It also updates checker-imaging to not checker on static and partially
loaded images.

BUG=722570
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: Keep PaintImage in DrawImage.

Currently we only store the SkImage in DrawImage during discardable
meta-data generation. This change extends that to store PaintImage so
it can be used by the checker-imaging system during raster.

It also updates checker-imaging to not checker on static and partially
loaded images.

BUG=722570
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2857923004
Cr-Commit-Position: refs/heads/master@{#472379}
Committed:
https://chromium.googlesource.com/chromium/src/+/d5e13bff12f02c4dc487d87451b3...
==========

[commit-bot: I haz the power, 2017-05-17 08:09:13]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/d5e13bff12f02c4dc487d87451b3...