cc: Plumb device color space through to rasterization

cc: Plumb device color space through to rasterization

Add plumbing from LayerTree to LayerTreeImpl. Grab the color space
from the LayerTreeImpl in raster task creation in the TileManager by
using the TileManagerClient interface.

Note that while this is plumbing a valid gfx::ColorSpace, this still
has no impact on behavior, because the gfx::ColorSpace to
SkColorSpace conversion isn't in place yet. Once that is in place,
tests to query the tile resources' color spaces can be added.

BUG=44872
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/99961cb9d362a5499e3b7455d16c760db78c2108
Cr-Commit-Position: refs/heads/master@{#419525}

[ccameron, 2016-09-12 23:42:05]

Description was changed from

==========
cc: Make PictureLayerImpl Tiles use LayerTreeImpl's device color space

Is this reasonable plumbing?

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

to

==========
cc: Make PictureLayerImpl Tiles use LayerTreeImpl's device color space

Is this reasonable plumbing?

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[ccameron, 2016-09-12 23:44:24]

https://codereview.chromium.org/2336853002/diff/1/cc/tiles/tile.h
File cc/tiles/tile.h (right):

https://codereview.chromium.org/2336853002/diff/1/cc/tiles/tile.h#newcode120
cc/tiles/tile.h:120: const gfx::ColorSpace& color_space,
This didn't seem appropriate to have as part of CreateInfo because the tiling
doesn't actually impact it.

Also of note is that if you have a single tiling which uses different color
spaces within it, that's not "wrong" in any particular way -- ideally you'll
want to re-raster to a single color space eventually, but the GLRenderer has the
responsibility of ensuring that you resolve to the same final color space.

https://codereview.chromium.org/2336853002/diff/1/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/2336853002/diff/1/cc/tiles/tile_manager.cc#ne...
cc/tiles/tile_manager.cc:945: tile->color_space());
This is where we need to get the ColorSpace plumbed to.

I had the idea of having a TileManager::SetColorSpace function, but that seemed
like a not-good idea.

[ccameron, 2016-09-13 23:19:01]

Description was changed from

==========
cc: Make PictureLayerImpl Tiles use LayerTreeImpl's device color space

Is this reasonable plumbing?

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Plumb device color space through to rasterization

Add plumbing from LayerTree to LayerTreeImpl. Grab the color space
from the LayerTreeImpl in raster task creation in the TileManager by
using the TileManagerClient interface.

Note that while this is plumbing a valid gfx::ColorSpace, this still
has no impact on behavior, because the gfx::ColorSpace is never
converted to a SkColorSpace.

BUG=44872
==========

[ccameron, 2016-09-13 23:19:01]

ccameron@chromium.org changed reviewers:
+ enne@chromium.org, vmpstr@chromium.org

[ccameron, 2016-09-13 23:24:45]

Description was changed from

==========
cc: Plumb device color space through to rasterization

Add plumbing from LayerTree to LayerTreeImpl. Grab the color space
from the LayerTreeImpl in raster task creation in the TileManager by
using the TileManagerClient interface.

Note that while this is plumbing a valid gfx::ColorSpace, this still
has no impact on behavior, because the gfx::ColorSpace is never
converted to a SkColorSpace.

BUG=44872
==========

to

==========
cc: Plumb device color space through to rasterization

Add plumbing from LayerTree to LayerTreeImpl. Grab the color space
from the LayerTreeImpl in raster task creation in the TileManager by
using the TileManagerClient interface.

Note that while this is plumbing a valid gfx::ColorSpace, this still
has no impact on behavior, because the gfx::ColorSpace to
SkColorSpace conversion isn't in place yet.

BUG=44872
==========

[ccameron, 2016-09-13 23:32:47]

Description was changed from

==========
cc: Plumb device color space through to rasterization

Add plumbing from LayerTree to LayerTreeImpl. Grab the color space
from the LayerTreeImpl in raster task creation in the TileManager by
using the TileManagerClient interface.

Note that while this is plumbing a valid gfx::ColorSpace, this still
has no impact on behavior, because the gfx::ColorSpace to
SkColorSpace conversion isn't in place yet.

BUG=44872
==========

to

==========
cc: Plumb device color space through to rasterization

Add plumbing from LayerTree to LayerTreeImpl. Grab the color space
from the LayerTreeImpl in raster task creation in the TileManager by
using the TileManagerClient interface.

Note that while this is plumbing a valid gfx::ColorSpace, this still
has no impact on behavior, because the gfx::ColorSpace to
SkColorSpace conversion isn't in place yet. Once that is in place,
tests to query the tile resources' color spaces can be added.

BUG=44872
==========

[ccameron, 2016-09-13 23:34:16]

ptal: I've updated this to the suggested structure.

I also ripped out the color space plumbing from
https://codereview.chromium.org/2325773003/ and moved it here. The reason for
that is that I want to be sure that sending to the LayerTree and LayerTreeImpl
are the right things to be doing (as opposed to sending to the various Host
structures), and this gives a wider picture of that.

https://codereview.chromium.org/2336853002/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/2336853002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:1327: return sync_tree()->device_color_space();
I'd vaguely want to put the device_color_space hanging off of the
LayerTreeHostImpl instead of the LayerTreeHost, but there seemed to be little
precedent for doing such a thing. Does pulling it from the sync_tree seem
reasonable here?

https://codereview.chromium.org/2336853002/diff/20001/cc/trees/layer_tree_imp...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/2336853002/diff/20001/cc/trees/layer_tree_imp...
cc/trees/layer_tree_impl.cc:859: 
Ideally, we want to request a full re-raster here. It's not strictly required
for correctness, but "it would be nice". How do we trigger that? TileManager
doesn't seem to have a "wipe out all tiles" method that I found.

[ccameron, 2016-09-13 23:34:23]

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

[commit-bot: I haz the power, 2016-09-13 23:35:06]

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, 2016-09-14 00:55:58]

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

[commit-bot: I haz the power, 2016-09-14 00:55:59]

Dry run: This issue passed the CQ dry run.

[ccameron, 2016-09-14 22:31:49]

Ping

[vmpstr, 2016-09-14 23:00:43]

(sorry for late review)

https://codereview.chromium.org/2336853002/diff/20001/cc/test/fake_tile_manag...
File cc/test/fake_tile_manager_client.h (right):

https://codereview.chromium.org/2336853002/diff/20001/cc/test/fake_tile_manag...
cc/test/fake_tile_manager_client.h:32: gfx::ColorSpace tile_color_space_;
Make this private, please.

https://codereview.chromium.org/2336853002/diff/20001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/2336853002/diff/20001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:945: client_->GetTileColorSpace());
Can you make CreateRasterTask take the color space as a parameter, and then in
the caller code call this once outside of a loop and just pass it in? 

It feels like it's worth the extra parameter on the function so that we don't
call this in a loop...

https://codereview.chromium.org/2336853002/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/2336853002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:1327: return sync_tree()->device_color_space();
On 2016/09/13 23:34:16, ccameron wrote:
> I'd vaguely want to put the device_color_space hanging off of the
> LayerTreeHostImpl instead of the LayerTreeHost, but there seemed to be little
> precedent for doing such a thing. Does pulling it from the sync_tree seem
> reasonable here?

sync_tree might be nullptr, right? If we activate and scroll on the compositor
thread, then new tiles would have to access this

https://codereview.chromium.org/2336853002/diff/20001/cc/trees/layer_tree_imp...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/2336853002/diff/20001/cc/trees/layer_tree_imp...
cc/trees/layer_tree_impl.cc:859: 
On 2016/09/13 23:34:16, ccameron wrote:
> Ideally, we want to request a full re-raster here. It's not strictly required
> for correctness, but "it would be nice". How do we trigger that? TileManager
> doesn't seem to have a "wipe out all tiles" method that I found.

LayerTreeImpl has ReleaseResources/RecreateResources. It does feel a bit
heavyhanded, since it's also going to remove all tilings and scales and things,
but maybe we want that anyway?
https://cs.chromium.org/chromium/src/cc/trees/layer_tree_impl.cc?sq=package:c...

In general though, I feel like if we want to reraster everything, then when we
create a pending tree with a different color space we should be invalidating the
world. Wdyt?

[ccameron, 2016-09-15 06:36:31]

Description was changed from

==========
cc: Plumb device color space through to rasterization

Add plumbing from LayerTree to LayerTreeImpl. Grab the color space
from the LayerTreeImpl in raster task creation in the TileManager by
using the TileManagerClient interface.

Note that while this is plumbing a valid gfx::ColorSpace, this still
has no impact on behavior, because the gfx::ColorSpace to
SkColorSpace conversion isn't in place yet. Once that is in place,
tests to query the tile resources' color spaces can be added.

BUG=44872
==========

to

==========
cc: Plumb device color space through to rasterization

Add plumbing from LayerTree to LayerTreeImpl. Grab the color space
from the LayerTreeImpl in raster task creation in the TileManager by
using the TileManagerClient interface.

Note that while this is plumbing a valid gfx::ColorSpace, this still
has no impact on behavior, because the gfx::ColorSpace to
SkColorSpace conversion isn't in place yet. Once that is in place,
tests to query the tile resources' color spaces can be added.

BUG=44872
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[ccameron, 2016-09-15 06:36:33]

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

[commit-bot: I haz the power, 2016-09-15 06:36:47]

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, 2016-09-15 06:38:51]

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

[commit-bot: I haz the power, 2016-09-15 06:38:51]

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-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[ccameron, 2016-09-15 07:01:55]

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

[commit-bot: I haz the power, 2016-09-15 07:02:12]

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

[ccameron, 2016-09-15 07:13:42]

Thanks -- updated

https://codereview.chromium.org/2336853002/diff/20001/cc/test/fake_tile_manag...
File cc/test/fake_tile_manager_client.h (right):

https://codereview.chromium.org/2336853002/diff/20001/cc/test/fake_tile_manag...
cc/test/fake_tile_manager_client.h:32: gfx::ColorSpace tile_color_space_;
On 2016/09/14 23:00:43, vmpstr wrote:
> Make this private, please.

Changed to return-by-value, and so this isn't needed (for this moment ... it'll
probably be needed once I have the full plumbing and want to test it).

https://codereview.chromium.org/2336853002/diff/20001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/2336853002/diff/20001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:945: client_->GetTileColorSpace());
On 2016/09/14 23:00:43, vmpstr wrote:
> Can you make CreateRasterTask take the color space as a parameter, and then in
> the caller code call this once outside of a loop and just pass it in? 
> 
> It feels like it's worth the extra parameter on the function so that we don't
> call this in a loop...

Done.

https://codereview.chromium.org/2336853002/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/2336853002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:1327: return sync_tree()->device_color_space();
On 2016/09/14 23:00:43, vmpstr wrote:
> On 2016/09/13 23:34:16, ccameron wrote:
> > I'd vaguely want to put the device_color_space hanging off of the
> > LayerTreeHostImpl instead of the LayerTreeHost, but there seemed to be
little
> > precedent for doing such a thing. Does pulling it from the sync_tree seem
> > reasonable here?
> 
> sync_tree might be nullptr, right? If we activate and scroll on the compositor
> thread, then new tiles would have to access this

I was planning to have this return a gfx::ColorSpace (instead of the const
reference) and just return the default value if there were no tree (sync_tree is
"pending_tree ? pending_tree : active_tree"), but then I found that that case
wasn't hit (at least in unittests and basic browsing).

That said, now that we've lifted the call to this out of the main loop, we might
as well have it return by value.

https://codereview.chromium.org/2336853002/diff/20001/cc/trees/layer_tree_imp...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/2336853002/diff/20001/cc/trees/layer_tree_imp...
cc/trees/layer_tree_impl.cc:859: 
On 2016/09/14 23:00:43, vmpstr wrote:
> On 2016/09/13 23:34:16, ccameron wrote:
> > Ideally, we want to request a full re-raster here. It's not strictly
required
> > for correctness, but "it would be nice". How do we trigger that? TileManager
> > doesn't seem to have a "wipe out all tiles" method that I found.
> 
> LayerTreeImpl has ReleaseResources/RecreateResources. It does feel a bit
> heavyhanded, since it's also going to remove all tilings and scales and
things,
> but maybe we want that anyway?
>
https://cs.chromium.org/chromium/src/cc/trees/layer_tree_impl.cc?sq=package:c...
> 
> In general though, I feel like if we want to reraster everything, then when we
> create a pending tree with a different color space we should be invalidating
the
> world. Wdyt?

I added a Release+Recreate -- it does indeed feel heavy, but adding a way to
spare the tilings etc is probably not worth the fragility. This should only
happen when moving windows across monitors.

Is the second suggestion to add something to
LayerTreeHostImpl::CreatePendingTree?

[commit-bot: I haz the power, 2016-09-15 07:48:20]

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

[commit-bot: I haz the power, 2016-09-15 07:48:21]

Dry run: This issue passed the CQ dry run.

[vmpstr, 2016-09-15 18:44:40]

https://codereview.chromium.org/2336853002/diff/60001/cc/trees/layer_tree_imp...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/2336853002/diff/60001/cc/trees/layer_tree_imp...
cc/trees/layer_tree_impl.cc:860: ReleaseResources();
Hmm...

Have you tested this with something like a very basic page switching monitors?

My concern is that on the pending tree, we currently have only sparse tiles.
That is, when we first create a pending tree it shouldn't have any tiles on it.
So, this call kinda double makes sure of that. However, it doesn't say anything
about which tiles we should be creating on the pending tree. That's still
dictated by the invalidation.

On the active tree, this would happen on activation (right?), which means that
we'll remove all active tree tiles and then apply the pending tree tiles, which
are located where the invalidation is. That is, it won't necessarily be the full
viewport. In turn that means that we're risking a frame of checkers until tile
manager panics and figures out that it actually doesn't have ready tiles on the
active tree.

Does that reasoning make sense?

If so, I'd prefer this being something along the lines of when we create a
pending tree (sync tree) and we set a new color space, then we add an
invalidation for the full region. PictureLayer has SetNeedsDisplay which should
invalidate everything... I'm not sure how where to call that though. When we
then set the color space on the active tree during commit, we _can_ blow away
all tiles, since we know that because we're activating we're going to get the
full viewport of tiles (everything was invalidated - every tile that's needed
exists on the pending tree)

[ccameron, 2016-09-15 20:52:51]

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

[ccameron, 2016-09-15 20:53:30]

On 2016/09/15 18:44:40, vmpstr wrote:
>
https://codereview.chromium.org/2336853002/diff/60001/cc/trees/layer_tree_imp...
> File cc/trees/layer_tree_impl.cc (right):
> 
>
https://codereview.chromium.org/2336853002/diff/60001/cc/trees/layer_tree_imp...
> cc/trees/layer_tree_impl.cc:860: ReleaseResources();
> Hmm...
> 
> Have you tested this with something like a very basic page switching monitors?
> 
> My concern is that on the pending tree, we currently have only sparse tiles.
> That is, when we first create a pending tree it shouldn't have any tiles on
it.
> So, this call kinda double makes sure of that. However, it doesn't say
anything
> about which tiles we should be creating on the pending tree. That's still
> dictated by the invalidation.
> 
> On the active tree, this would happen on activation (right?), which means that
> we'll remove all active tree tiles and then apply the pending tree tiles,
which
> are located where the invalidation is. That is, it won't necessarily be the
full
> viewport. In turn that means that we're risking a frame of checkers until tile
> manager panics and figures out that it actually doesn't have ready tiles on
the
> active tree.
> 
> Does that reasoning make sense?
> 
> If so, I'd prefer this being something along the lines of when we create a
> pending tree (sync tree) and we set a new color space, then we add an
> invalidation for the full region. PictureLayer has SetNeedsDisplay which
should
> invalidate everything... I'm not sure how where to call that though. When we
> then set the color space on the active tree during commit, we _can_ blow away
> all tiles, since we know that because we're activating we're going to get the
> full viewport of tiles (everything was invalidated - every tile that's needed
> exists on the pending tree)

In that case, doing it in LayerTree::SetDeviceColorSpace would make sense -- we
can just SetNeedsDisplay on the tree if we observe that the color space has
changed. Updated this (and the tests).

[commit-bot: I haz the power, 2016-09-15 20:53:31]

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, 2016-09-15 21:47:13]

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

[commit-bot: I haz the power, 2016-09-15 21:47:14]

Dry run: This issue passed the CQ dry run.

[vmpstr, 2016-09-16 19:01:50]

Looks good, just one question

https://codereview.chromium.org/2336853002/diff/100001/cc/trees/layer_tree_im...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/2336853002/diff/100001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:861: layer_tree_host_impl_->SetFullViewportDamage();
Do you need this? I think maybe the act of activating a full invalidation will
damage stuff anyway?

[ccameron, 2016-09-16 19:58:26]

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

[commit-bot: I haz the power, 2016-09-16 19:58:53]

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

[ccameron, 2016-09-16 20:02:38]

Thanks -- updated

https://codereview.chromium.org/2336853002/diff/100001/cc/trees/layer_tree_im...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/2336853002/diff/100001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:861: layer_tree_host_impl_->SetFullViewportDamage();
On 2016/09/16 19:01:49, vmpstr wrote:
> Do you need this? I think maybe the act of activating a full invalidation will
> damage stuff anyway?

Nope, good catch. Removed.

[vmpstr, 2016-09-16 20:03:53]

lgtm

[ccameron, 2016-09-16 20:10:34]

The CQ bit was unchecked by ccameron@chromium.org

[ccameron, 2016-09-16 20:10:41]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2016-09-16 20:11:28]

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-09-16 20:21:13]

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

[commit-bot: I haz the power, 2016-09-16 20:21:15]

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

[ccameron, 2016-09-16 20:23:43]

oh, enne, can you content/renderer/gpu/OWNERS?

[enne (OOO), 2016-09-16 21:02:26]

lgtm *stamp noise*

[ccameron, 2016-09-16 23:06:30]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2016-09-16 23:07:05]

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

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

[commit-bot: I haz the power, 2016-09-16 23:16:09]

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

[ccameron, 2016-09-19 17:52:15]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2016-09-19 17:52: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, 2016-09-19 19:15:44]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-09-19 21:02:55]

Description was changed from

==========
cc: Plumb device color space through to rasterization

Add plumbing from LayerTree to LayerTreeImpl. Grab the color space
from the LayerTreeImpl in raster task creation in the TileManager by
using the TileManagerClient interface.

Note that while this is plumbing a valid gfx::ColorSpace, this still
has no impact on behavior, because the gfx::ColorSpace to
SkColorSpace conversion isn't in place yet. Once that is in place,
tests to query the tile resources' color spaces can be added.

BUG=44872
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Plumb device color space through to rasterization

Add plumbing from LayerTree to LayerTreeImpl. Grab the color space
from the LayerTreeImpl in raster task creation in the TileManager by
using the TileManagerClient interface.

Note that while this is plumbing a valid gfx::ColorSpace, this still
has no impact on behavior, because the gfx::ColorSpace to
SkColorSpace conversion isn't in place yet. Once that is in place,
tests to query the tile resources' color spaces can be added.

BUG=44872
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/99961cb9d362a5499e3b7455d16c760db78c2108
Cr-Commit-Position: refs/heads/master@{#419525}
==========

[commit-bot: I haz the power, 2016-09-19 21:02:56]

Patchset 7 (id:??) landed as
https://crrev.com/99961cb9d362a5499e3b7455d16c760db78c2108
Cr-Commit-Position: refs/heads/master@{#419525}