Replace LayerImpl::sorting_context_id with transform reference.

Replace LayerImpl::sorting_context_id with transform reference.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

[wkorman, 2017-01-04 23:59:45]

Description was changed from

==========
Remove comment re: obsolescence of sorting context id.
==========

to

==========
Remove comment re: obsolescence of sorting context id.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[wkorman, 2017-01-05 00:04:28]

wkorman@chromium.org changed reviewers:
+ ajuma@chromium.org, chrishtr@chromium.org

[wkorman, 2017-01-05 00:04:28]

I looked into this and believe that the 3d sorting context id is still used to
implement backface visibility and to enforce 3d transform related layer
ordering.

I'm not sure what jbroman@ had in mind when he wrote it, but the code may have
changed
since then anyway, so looking at latest behavior seemed sensible.

I believe his comment was referring to this reference:

https://cs.chromium.org/chromium/src/cc/layers/render_surface_impl.cc?q=rende...

Layout tests that failed locally without this are below:

+compositing/3d-cube.html		ref html images diff	image	pass
+compositing/backface-visibility-transformed.html		images diff	image	pass
+compositing/backface-visibility/backface-visibility-3d.html		images
diff	image	pass
+compositing/backface-visibility/backface-visibility-hierarchical-transform.html		images
diff	image	pass
+compositing/backface-visibility/backface-visibility-image.html		images
diff	image	pass
+compositing/backface-visibility/backface-visibility-webgl.html		images
diff	image	pass
+compositing/perpendicular-layer-sorting.html		images diff	image	pass
+transforms/3d/general/background-visibility-layers.html		images diff	image	pass
+transforms/3d/hit-testing/backface-no-transform-hit-test.html		images
diff	image	pass
+transforms/3d/point-mapping/3d-point-mapping-2.html

A hack to remove the field and pull on the (quite long) thread also led to the
below cc unit tests failing, though it's possible I did something wrong hacking
on the test, so this is only a rough failure set:

    IntersectingQuadGLPixelTest/0.Y16VideoQuads
(../../cc/output/renderer_pixeltest.cc:1055)
    IntersectingQuadGLPixelTest/0.YUVVideoQuads
(../../cc/output/renderer_pixeltest.cc:1027)
    IntersectingQuadGLPixelTest/1.Y16VideoQuads
(../../cc/output/renderer_pixeltest.cc:1055)
    IntersectingQuadGLPixelTest/1.YUVVideoQuads
(../../cc/output/renderer_pixeltest.cc:1027)
    IntersectingQuadPixelTest/0.RenderPassQuads
(../../cc/output/renderer_pixeltest.cc:990)
    IntersectingQuadPixelTest/0.SolidColorQuads
(../../cc/output/renderer_pixeltest.cc:887)
    IntersectingQuadPixelTest/0.TexturedQuads
(../../cc/output/renderer_pixeltest.cc:920)
    IntersectingQuadPixelTest/1.RenderPassQuads
(../../cc/output/renderer_pixeltest.cc:990)
    IntersectingQuadPixelTest/1.SolidColorQuads
(../../cc/output/renderer_pixeltest.cc:887)
    IntersectingQuadPixelTest/1.TexturedQuads
(../../cc/output/renderer_pixeltest.cc:920)
    IntersectingQuadPixelTest/2.RenderPassQuads
(../../cc/output/renderer_pixeltest.cc:990)
    IntersectingQuadPixelTest/2.SolidColorQuads
(../../cc/output/renderer_pixeltest.cc:887)
    IntersectingQuadPixelTest/2.TexturedQuads
(../../cc/output/renderer_pixeltest.cc:920)
    IntersectingQuadPixelTest/3.RenderPassQuads
(../../cc/output/renderer_pixeltest.cc:990)
    IntersectingQuadPixelTest/3.SolidColorQuads
(../../cc/output/renderer_pixeltest.cc:887)
    IntersectingQuadPixelTest/3.TexturedQuads
(../../cc/output/renderer_pixeltest.cc:920)
    IntersectingQuadSoftwareTest/0.PictureQuads
(../../cc/output/renderer_pixeltest.cc:939)
    IntersectingQuadSoftwareTest/1.PictureQuads
(../../cc/output/renderer_pixeltest.cc:939)
    LayerTest.LayerPropertyChangedForSubtree
(../../cc/layers/layer_unittest.cc:243)
    LayerTreeHostCommonTest.CanRenderToSeparateSurface
(../../cc/trees/layer_tree_host_common_unittest.cc:5891)
    LayerTreeHostCommonTest.CreateRenderSurfaceWhenFlattenInsideRenderingContext
(../../cc/trees/layer_tree_host_common_unittest.cc:5863)
    LayerTreeHostCommonTest.DoNotIncludeBackfaceInvisibleLayers
(../../cc/trees/layer_tree_host_common_unittest.cc:6055)
    LayerTreeHostFiltersPixelTest.BackgroundFilterBlurOffAxis
(../../cc/trees/layer_tree_host_pixeltest_filters.cc:105)
    LayerTreeImplTest.HitTestingSameSortingContextChildWins
(../../cc/trees/layer_tree_impl_unittest.cc:908)
    LayerTreeImplTest.HitTestingSameSortingContextParentWins
(../../cc/trees/layer_tree_impl_unittest.cc:929)

[ajuma, 2017-01-05 00:22:20]

https://codereview.chromium.org/2615773003/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
(left):

https://codereview.chromium.org/2615773003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp:1178:
// queried by RenderSurfaceImpl.
In theory, I think looking up the sorting context id from a LayerImpl's
transform node instead of having this value on the LayerImpl itself should work,
since cc's property tree builder does add a new transform node whenever the
sorting context id changes, see
https://cs.chromium.org/chromium/src/cc/trees/property_tree_builder.cc?rcl=0&....

[chrishtr, 2017-01-05 01:04:03]

https://codereview.chromium.org/2615773003/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
(left):

https://codereview.chromium.org/2615773003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp:1178:
// queried by RenderSurfaceImpl.
On 2017/01/05 at 00:22:20, ajuma wrote:
> In theory, I think looking up the sorting context id from a LayerImpl's
transform node instead of having this value on the LayerImpl itself should work,
since cc's property tree builder does add a new transform node whenever the
sorting context id changes, see
https://cs.chromium.org/chromium/src/cc/trees/property_tree_builder.cc?rcl=0&....

Correct, this is how it should work. Ali, should that work already then?

[ajuma, 2017-01-05 14:13:21]

https://codereview.chromium.org/2615773003/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
(left):

https://codereview.chromium.org/2615773003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp:1178:
// queried by RenderSurfaceImpl.
On 2017/01/05 01:04:03, chrishtr wrote:
> On 2017/01/05 at 00:22:20, ajuma wrote:
> > In theory, I think looking up the sorting context id from a LayerImpl's
> transform node instead of having this value on the LayerImpl itself should
work,
> since cc's property tree builder does add a new transform node whenever the
> sorting context id changes, see
>
https://cs.chromium.org/chromium/src/cc/trees/property_tree_builder.cc?rcl=0&....
> 
> Correct, this is how it should work. Ali, should that work already then?

Yes, changing all the places in cc that use LayerImpl"s sorting_context_id to
using its transform node's sorting context id should work already (there might
be unit tests where we aren't building/rebuilding property trees, but those
should be fixable just by making sure property trees get built).

[chrishtr, 2017-01-05 17:34:00]

On 2017/01/05 at 14:13:21, ajuma wrote:
>
https://codereview.chromium.org/2615773003/diff/1/third_party/WebKit/Source/p...
> File
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
(left):
> 
>
https://codereview.chromium.org/2615773003/diff/1/third_party/WebKit/Source/p...
>
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp:1178:
// queried by RenderSurfaceImpl.
> On 2017/01/05 01:04:03, chrishtr wrote:
> > On 2017/01/05 at 00:22:20, ajuma wrote:
> > > In theory, I think looking up the sorting context id from a LayerImpl's
> > transform node instead of having this value on the LayerImpl itself should
work,
> > since cc's property tree builder does add a new transform node whenever the
> > sorting context id changes, see
> >
https://cs.chromium.org/chromium/src/cc/trees/property_tree_builder.cc?rcl=0&....
> > 
> > Correct, this is how it should work. Ali, should that work already then?
> 
> Yes, changing all the places in cc that use LayerImpl"s sorting_context_id to
using its transform node's sorting context id should work already (there might
be unit tests where we aren't building/rebuilding property trees, but those
should be fixable just by making sure property trees get built).

Great. Walter can you change this patch to do that?

[wkorman, 2017-01-05 22:32:57]

On 2017/01/05 at 17:34:00, chrishtr wrote:
> On 2017/01/05 at 14:13:21, ajuma wrote:
> >
https://codereview.chromium.org/2615773003/diff/1/third_party/WebKit/Source/p...
> > File
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
(left):
> > 
> >
https://codereview.chromium.org/2615773003/diff/1/third_party/WebKit/Source/p...
> >
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp:1178:
// queried by RenderSurfaceImpl.
> > On 2017/01/05 01:04:03, chrishtr wrote:
> > > On 2017/01/05 at 00:22:20, ajuma wrote:
> > > > In theory, I think looking up the sorting context id from a LayerImpl's
> > > transform node instead of having this value on the LayerImpl itself should
work,
> > > since cc's property tree builder does add a new transform node whenever
the
> > > sorting context id changes, see
> > >
https://cs.chromium.org/chromium/src/cc/trees/property_tree_builder.cc?rcl=0&....
> > > 
> > > Correct, this is how it should work. Ali, should that work already then?
> > 
> > Yes, changing all the places in cc that use LayerImpl"s sorting_context_id
to using its transform node's sorting context id should work already (there
might be unit tests where we aren't building/rebuilding property trees, but
those should be fixable just by making sure property trees get built).
> 
> Great. Walter can you change this patch to do that?

LayerImpl::Is3DSorted() and LayerImpl::sorting_context_id() are both used in
cc::PropertyTreeBuilder as part of building the property tree itself, and are
what end up leading to the sorting context id being in the transform node for
SPv1, so we can make the change but while we're in transition to SPv2 I
currently believe we'll need to keep the current fields and accessor
implementation as-is on Layer/LayerImpl as well.

So, we can change callsites outside of cc::PropertyTreeBuilder to use new method
variants like
Is3dSortedViaTransformNode()/GetSortingContextIdViaTransformNode(), then we
shouldn't need to set the sorting context in cc::Layer in
PaintArtifactCompositor, and once SPv2 is shipped we can delete the fields in
cc::Layer and cc::LayerImpl and the alternate method variants and change
LayerImpl::Is3DSorted() and LayerImpl::sorting_context_id() to just look up via
the transform node.

I may well be missing something obvious, sharing current understanding to
solicit guidance.

[ajuma, 2017-01-05 22:39:09]

On 2017/01/05 22:32:57, wkorman wrote:
> On 2017/01/05 at 17:34:00, chrishtr wrote:
> > On 2017/01/05 at 14:13:21, ajuma wrote:
> > >
>
https://codereview.chromium.org/2615773003/diff/1/third_party/WebKit/Source/p...
> > > File
>
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
> (left):
> > > 
> > >
>
https://codereview.chromium.org/2615773003/diff/1/third_party/WebKit/Source/p...
> > >
>
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp:1178:
> // queried by RenderSurfaceImpl.
> > > On 2017/01/05 01:04:03, chrishtr wrote:
> > > > On 2017/01/05 at 00:22:20, ajuma wrote:
> > > > > In theory, I think looking up the sorting context id from a
LayerImpl's
> > > > transform node instead of having this value on the LayerImpl itself
should
> work,
> > > > since cc's property tree builder does add a new transform node whenever
> the
> > > > sorting context id changes, see
> > > >
>
https://cs.chromium.org/chromium/src/cc/trees/property_tree_builder.cc?rcl=0&....
> > > > 
> > > > Correct, this is how it should work. Ali, should that work already then?
> > > 
> > > Yes, changing all the places in cc that use LayerImpl"s sorting_context_id
> to using its transform node's sorting context id should work already (there
> might be unit tests where we aren't building/rebuilding property trees, but
> those should be fixable just by making sure property trees get built).
> > 
> > Great. Walter can you change this patch to do that?
> 
> LayerImpl::Is3DSorted() and LayerImpl::sorting_context_id() are both used in
> cc::PropertyTreeBuilder as part of building the property tree itself, and are
> what end up leading to the sorting context id being in the transform node for
> SPv1, so we can make the change but while we're in transition to SPv2 I
> currently believe we'll need to keep the current fields and accessor
> implementation as-is on Layer/LayerImpl as well.
> 
> So, we can change callsites outside of cc::PropertyTreeBuilder to use new
method
> variants like
> Is3dSortedViaTransformNode()/GetSortingContextIdViaTransformNode(), then we
> shouldn't need to set the sorting context in cc::Layer in
> PaintArtifactCompositor, and once SPv2 is shipped we can delete the fields in
> cc::Layer and cc::LayerImpl and the alternate method variants and change
> LayerImpl::Is3DSorted() and LayerImpl::sorting_context_id() to just look up
via
> the transform node.
> 
> I may well be missing something obvious, sharing current understanding to
> solicit guidance.

We need to keep the sorting_context_id on cc::Layer for SPv1 (since
PropertyTreeBuilder uses it), but we can remove it from LayerImpl now and put on
LayerImplTestProperties instead (PropertyTreeBuilder only uses LayerImpls in
tests). See the way Layer::double_sided is handled for an example of this.

[wkorman, 2017-01-05 22:43:38]

On 2017/01/05 at 22:39:09, ajuma wrote:
> We need to keep the sorting_context_id on cc::Layer for SPv1 (since
PropertyTreeBuilder uses it), but we can remove it from LayerImpl now and put on
LayerImplTestProperties instead (PropertyTreeBuilder only uses LayerImpls in
tests). See the way Layer::double_sided is handled for an example of this.

Ah, tricky. I was looking at LayerImplTestProperties earlier. OK, let me try
that.

[wkorman, 2017-01-06 02:36:34]

Description was changed from

==========
Remove comment re: obsolescence of sorting context id.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Replace LayerImpl::sorting_context_id with transform reference.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[wkorman, 2017-01-06 02:40:28]

Updated to replace LayerImpl::sorting_context_id with reference of transform
node for same, PTAL.

[wkorman, 2017-01-06 02:40:34]

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

[commit-bot: I haz the power, 2017-01-06 02:41:05]

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

[wkorman, 2017-01-06 02:53:37]

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

[wkorman, 2017-01-06 02:53:38]

+vmpstr who helped me out with solid_color_layer_impl_unittest.cc in case he
wants to see what is afoot.

[commit-bot: I haz the power, 2017-01-06 03:52:22]

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

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

Dry run: 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_...)

[ajuma, 2017-01-06 14:59:55]

https://codereview.chromium.org/2615773003/diff/40001/cc/layers/layer_impl.cc
File cc/layers/layer_impl.cc (right):

https://codereview.chromium.org/2615773003/diff/40001/cc/layers/layer_impl.cc...
cc/layers/layer_impl.cc:1056: return (!property_trees ||
Looking at LayerTreeImpl::property_trees, I don't think this can ever be null.

https://codereview.chromium.org/2615773003/diff/40001/cc/layers/layer_impl.cc...
cc/layers/layer_impl.cc:1057: transform_tree_index() ==
TransformTree::kInvalidNodeId)
This should never be kInvalidNodeId except in tests where we haven't built
property trees, and for those tests I think it'd be better to build property
trees (or remove the call to LayerImpl::sorting_context_id if it isn't essential
to the test).

https://codereview.chromium.org/2615773003/diff/40001/cc/layers/layer_impl.h
File cc/layers/layer_impl.h (right):

https://codereview.chromium.org/2615773003/diff/40001/cc/layers/layer_impl.h#...
cc/layers/layer_impl.h:415: int sorting_context_id() const;
style nit: since this is no longer a trivial getter, it should be renamed
SortingContextId()

[wkorman, 2017-01-06 22:50:02]

https://codereview.chromium.org/2615773003/diff/40001/cc/layers/layer_impl.cc
File cc/layers/layer_impl.cc (right):

https://codereview.chromium.org/2615773003/diff/40001/cc/layers/layer_impl.cc...
cc/layers/layer_impl.cc:1056: return (!property_trees ||
On 2017/01/06 at 14:59:55, ajuma wrote:
> Looking at LayerTreeImpl::property_trees, I don't think this can ever be null.

Done.

https://codereview.chromium.org/2615773003/diff/40001/cc/layers/layer_impl.cc...
cc/layers/layer_impl.cc:1057: transform_tree_index() ==
TransformTree::kInvalidNodeId)
On 2017/01/06 at 14:59:55, ajuma wrote:
> This should never be kInvalidNodeId except in tests where we haven't built
property trees, and for those tests I think it'd be better to build property
trees (or remove the call to LayerImpl::sorting_context_id if it isn't essential
to the test).

Done.

https://codereview.chromium.org/2615773003/diff/40001/cc/layers/layer_impl.h
File cc/layers/layer_impl.h (right):

https://codereview.chromium.org/2615773003/diff/40001/cc/layers/layer_impl.h#...
cc/layers/layer_impl.h:415: int sorting_context_id() const;
On 2017/01/06 at 14:59:55, ajuma wrote:
> style nit: since this is no longer a trivial getter, it should be renamed
SortingContextId()

Renamed to GetSortingContextId() (added Get prefix per conversation on
http://crrev.com/2611253003 with you and danakj@).

https://codereview.chromium.org/2615773003/diff/60001/cc/layers/solid_color_l...
File cc/layers/solid_color_layer_impl_unittest.cc (left):

https://codereview.chromium.org/2615773003/diff/60001/cc/layers/solid_color_l...
cc/layers/solid_color_layer_impl_unittest.cc:103: TEST(SolidColorLayerImplTest,
VerifyCorrectBlendModeInQuad) {
I spent a bunch of time looking at this test and roped in ericrk@ to consult. We
believe it is testing a scenario that never happens in the wild, and so is not
relevant and we can just delete the test case.

If we build a property tree for this test case, and set blend mode in the
layer_impl test_properties(), and even if we try creating a root layer and
hanging the solid color layer off of that as a child layer, in all situations we
end up hitting this case in PropertyTreeBuilder:

https://cs.chromium.org/chromium/src/cc/trees/property_tree_builder.cc?q=prop...

which overwrites the layer draw blend mode with kSrcOver. When we have it set up
as a child layer (and so not root), we still end up creating a render surface
for it due to this case:

https://cs.chromium.org/chromium/src/cc/trees/property_tree_builder.cc?sq=pac...

which, again, means we'll overwrite it with kSrcOver.

Eric set up a simple web page test case that has a div with multiply blend mode
and I believe it ended up creating a RenderPassDrawQuad, and not even a
SolidColorDrawQuad, which would presumably be what this test would be focused on
testing in any case. So we've not found a way that, in the wild, the logic this
test was aiming to validate will be needed.

[wkorman, 2017-01-06 22:52:04]

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

[commit-bot: I haz the power, 2017-01-06 22:52:21]

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

[ajuma, 2017-01-06 23:47:08]

Thanks, lgtm

https://codereview.chromium.org/2615773003/diff/60001/cc/layers/solid_color_l...
File cc/layers/solid_color_layer_impl_unittest.cc (left):

https://codereview.chromium.org/2615773003/diff/60001/cc/layers/solid_color_l...
cc/layers/solid_color_layer_impl_unittest.cc:103: TEST(SolidColorLayerImplTest,
VerifyCorrectBlendModeInQuad) {
On 2017/01/06 22:50:02, wkorman wrote:
> I spent a bunch of time looking at this test and roped in ericrk@ to consult.
We
> believe it is testing a scenario that never happens in the wild, and so is not
> relevant and we can just delete the test case.
> 
> If we build a property tree for this test case, and set blend mode in the
> layer_impl test_properties(), and even if we try creating a root layer and
> hanging the solid color layer off of that as a child layer, in all situations
we
> end up hitting this case in PropertyTreeBuilder:
> 
>
https://cs.chromium.org/chromium/src/cc/trees/property_tree_builder.cc?q=prop...
> 
> which overwrites the layer draw blend mode with kSrcOver. When we have it set
up
> as a child layer (and so not root), we still end up creating a render surface
> for it due to this case:
> 
>
https://cs.chromium.org/chromium/src/cc/trees/property_tree_builder.cc?sq=pac...
> 
> which, again, means we'll overwrite it with kSrcOver.
> 
> Eric set up a simple web page test case that has a div with multiply blend
mode
> and I believe it ended up creating a RenderPassDrawQuad, and not even a
> SolidColorDrawQuad, which would presumably be what this test would be focused
on
> testing in any case. So we've not found a way that, in the wild, the logic
this
> test was aiming to validate will be needed.

Makes sense. See also
https://cs.chromium.org/chromium/src/cc/trees/occlusion_tracker.cc?rcl=148371...
where we DCHECK that draw_blend_mode is currently always SrcOver.

[commit-bot: I haz the power, 2017-01-07 00:28:33]

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

[commit-bot: I haz the power, 2017-01-07 00:28:34]

Dry run: This issue passed the CQ dry run.

[wkorman, 2017-01-07 00:31:11]

The CQ bit was checked by wkorman@chromium.org

[commit-bot: I haz the power, 2017-01-07 00:32: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, 2017-01-07 00:40:22]

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

[commit-bot: I haz the power, 2017-01-07 00:40:23]

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...)

[chrishtr, 2017-01-07 00:53:39]

lgtm

[wkorman, 2017-01-07 00:55:10]

The CQ bit was checked by wkorman@chromium.org

[commit-bot: I haz the power, 2017-01-07 00:55:39]

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-07 01:01:24]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1483750510465630,
"parent_rev": "4ce240b3ecb8ff7d7b902f2ccdf361411915610a", "commit_rev":
"f09921d0d061648aaf44835bd26c9802055eb2df"}

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

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

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

Prior attempt to commit was detected, but we were not able to check whether the
issue was successfully committed. Please check Git history manually and re-check
CQ or close this issue as needed.

[wkorman, 2017-01-07 02:02:24]

The CQ bit was checked by wkorman@chromium.org

[commit-bot: I haz the power, 2017-01-07 02:02: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-07 02:10:30]

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

[commit-bot: I haz the power, 2017-01-07 02:10:31]

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...)

[wkorman, 2017-01-07 20:54:01]

Submitted as Cr-Commit-Position: refs/heads/master@{#442121} but something went
awry with the commit logging, so closing manually.