Set owning layer id on transform nodes in SPv2.

Set owning layer id on transform nodes in SPv2.

http://crrev.com/2758343002 switched LayerTreeHostImpl to
look up transform nodes by element id rather than layer id.

This led to identifying a transform animation change in
SPv2 in some cases that were not identified previously.

This in turn led to raster scale computation crashing due
to expecting an owning layer id on the involved transform
node, whereas our SPv2 compositing logic that builds the
cc transform node was not setting an owning layer id.

We should update cc-side animation logic to use element
id and property tree nodes wherever possible rather than
layers, but in interim, setting an owning layer id on the
cc transform node is an acceptable solution.

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

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

[wkorman, 2017-04-06 00:48:24]

Description was changed from

==========
Set owning layer id on transform nodes in SPv2.

BUG=707281
==========

to

==========
Set owning layer id on transform nodes in SPv2.

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

[wkorman, 2017-04-06 01:28:16]

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

[wkorman, 2017-04-06 01:46:42]

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

[commit-bot: I haz the power, 2017-04-06 01:47:10]

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-04-06 03:15:55]

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

[commit-bot: I haz the power, 2017-04-06 03:15:56]

Dry run: This issue passed the CQ dry run.

[chrishtr, 2017-04-06 17:55:42]

Is this a proposed short-term workaround while waiting for animations to be
represented
in cc only by property tree nodes?

[wkorman, 2017-04-06 18:05:14]

On 2017/04/06 17:55:42, chrishtr wrote:
> Is this a proposed short-term workaround while waiting for animations to be
> represented
> in cc only by property tree nodes?

Yes, see my comment at http://crbug.com/707281#c10.

[wkorman, 2017-04-06 18:11:19]

Description was changed from

==========
Set owning layer id on transform nodes in SPv2.

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

to

==========
Set owning layer id on transform nodes in SPv2.

http://crrev.com/2758343002 switched LayerTreeHostImpl to
look up transform nodes by element id rather than layer id.

This led to identifying a transform animation change in
SPv2 in some cases that were not identified previously.

This in turn led to raster scale computation crashing due
to expecting an owning layer id on the involved transform
node, whereas our SPv2 compositing logic that builds the
cc transform node was not setting an owning layer id.

We should update cc-side animation logic to use element
id and property tree nodes wherever possible rather than
layers, but in interim, setting an owning layer id on the
cc transform node is an acceptable solution.

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

[wkorman, 2017-04-06 18:11:40]

On 2017/04/06 18:05:14, wkorman wrote:
> On 2017/04/06 17:55:42, chrishtr wrote:
> > Is this a proposed short-term workaround while waiting for animations to be
> > represented
> > in cc only by property tree nodes?
> 
> Yes, see my comment at http://crbug.com/707281#c10.

I also updated the CL description to make clearer.

[chrishtr, 2017-04-06 18:15:22]

lgtm

https://codereview.chromium.org/2800983002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp
(right):

https://codereview.chromium.org/2800983002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp:77:
transformNode.owning_layer_id = m_rootLayer->id();
Please add a comment here and in the other instance that this is temporary,
and make sure there is a bug tracking the final solution.

[wkorman, 2017-04-06 18:48:26]

https://codereview.chromium.org/2800983002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp
(right):

https://codereview.chromium.org/2800983002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp:77:
transformNode.owning_layer_id = m_rootLayer->id();
On 2017/04/06 18:15:21, chrishtr wrote:
> Please add a comment here and in the other instance that this is temporary,
> and make sure there is a bug tracking the final solution.

Done. Note we were already setting owning_layer_id in scroll, clip, and effect
nodes. Some of those are probably not animation specific, and removal could
require other work. I am only adding comment to the two transform node refs here
for now.

[wkorman, 2017-04-06 18:48:48]

The CQ bit was checked by wkorman@chromium.org

[wkorman, 2017-04-06 18:48:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org
Link to the patchset: https://codereview.chromium.org/2800983002/#ps40001
(title: "Add comments.")

[commit-bot: I haz the power, 2017-04-06 18:49:40]

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-04-06 20:29:05]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491504528172470,
"parent_rev": "707f1a623b690a870f6378ccf074b51cdaad9556", "commit_rev":
"f03d05f724aabaf8e9db6df23bb38dbc1fe04ff6"}

[commit-bot: I haz the power, 2017-04-06 20:30:38]

Description was changed from

==========
Set owning layer id on transform nodes in SPv2.

http://crrev.com/2758343002 switched LayerTreeHostImpl to
look up transform nodes by element id rather than layer id.

This led to identifying a transform animation change in
SPv2 in some cases that were not identified previously.

This in turn led to raster scale computation crashing due
to expecting an owning layer id on the involved transform
node, whereas our SPv2 compositing logic that builds the
cc transform node was not setting an owning layer id.

We should update cc-side animation logic to use element
id and property tree nodes wherever possible rather than
layers, but in interim, setting an owning layer id on the
cc transform node is an acceptable solution.

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

to

==========
Set owning layer id on transform nodes in SPv2.

http://crrev.com/2758343002 switched LayerTreeHostImpl to
look up transform nodes by element id rather than layer id.

This led to identifying a transform animation change in
SPv2 in some cases that were not identified previously.

This in turn led to raster scale computation crashing due
to expecting an owning layer id on the involved transform
node, whereas our SPv2 compositing logic that builds the
cc transform node was not setting an owning layer id.

We should update cc-side animation logic to use element
id and property tree nodes wherever possible rather than
layers, but in interim, setting an owning layer id on the
cc transform node is an acceptable solution.

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

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

[commit-bot: I haz the power, 2017-04-06 20:30:41]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/f03d05f724aabaf8e9db6df23bb3...