cc: Commit property trees to the compositor thread

cc: Commit property trees to the compositor thread

This patch is ugly plumbing and sad template boilerplate.

It pushes property trees and and property tree-derived values (like tree
indices and transform offsets) to the compositor trees.  It also
calculates (but does not verify) visible rects using property trees on
the compositor thread.  In order to not crash, this patch also has to
build property trees for tests that only have a LayerImpl-side structure
and don't have property trees that get committed from a Layer tree.

These property trees cannot be verified yet as they are not updated from
compositor-side changes like scrolling or animation.

BUG=386793
Committed: https://crrev.com/e95b154d48067914e65a091aaee0b78359f7f08f
Cr-Commit-Position: refs/heads/master@{#325904}

[enne (OOO), 2015-04-16 21:36:41]

enne@chromium.org changed reviewers:
+ vollick@chromium.org

[ajuma, 2015-04-16 21:58:14]

ajuma@chromium.org changed reviewers:
+ ajuma@chromium.org

[ajuma, 2015-04-16 21:58:15]

lgtm

https://codereview.chromium.org/1097583002/diff/1/cc/layers/layer.h
File cc/layers/layer.h (right):

https://codereview.chromium.org/1097583002/diff/1/cc/layers/layer.h#newcode480
cc/layers/layer.h:480: // TODO(enne): should these set needs push properties?
I think they'll need to, since they could change without anything else setting
needs push properties on this layer.

[enne (OOO), 2015-04-16 22:16:54]

https://codereview.chromium.org/1097583002/diff/1/cc/layers/layer.h
File cc/layers/layer.h (right):

https://codereview.chromium.org/1097583002/diff/1/cc/layers/layer.h#newcode480
cc/layers/layer.h:480: // TODO(enne): should these set needs push properties?
On 2015/04/16 21:58:14, ajuma wrote:
> I think they'll need to, since they could change without anything else setting
> needs push properties on this layer.

Done.  I didn't add any "if (new != old) push" logic, since these things really
only get set during a tree rebuild, so there's probably other pushing happening.
 I can add conditionals if y'all think that's best.

[Ian Vollick, 2015-04-17 13:30:07]

On 2015/04/16 22:16:54, enne wrote:
> https://codereview.chromium.org/1097583002/diff/1/cc/layers/layer.h
> File cc/layers/layer.h (right):
> 
> https://codereview.chromium.org/1097583002/diff/1/cc/layers/layer.h#newcode480
> cc/layers/layer.h:480: // TODO(enne): should these set needs push properties?
> On 2015/04/16 21:58:14, ajuma wrote:
> > I think they'll need to, since they could change without anything else
setting
> > needs push properties on this layer.
> 
> Done.  I didn't add any "if (new != old) push" logic, since these things
really
> only get set during a tree rebuild, so there's probably other pushing
happening.
>  I can add conditionals if y'all think that's best.

Nah, this is fine. lgtm.

[Ian Vollick, 2015-04-17 15:25:06]

The CQ bit was checked by vollick@chromium.org

[Ian Vollick, 2015-04-17 15:25:07]

The patchset sent to the CQ was uploaded after l-g-t-m from ajuma@chromium.org
Link to the patchset: https://codereview.chromium.org/1097583002/#ps20001
(title: "Now with more push properties")

[commit-bot: I haz the power, 2015-04-17 15:26:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1097583002/20001

[commit-bot: I haz the power, 2015-04-17 15:33:31]

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

[commit-bot: I haz the power, 2015-04-17 15:33:31]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[enne (OOO), 2015-04-17 20:59:36]

https://codereview.chromium.org/1097583002/diff/40001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_unittest.cc (left):

https://codereview.chromium.org/1097583002/diff/40001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest.cc:3448: child_->SetPosition(gfx::Point(1,
1));
This code used to adjust the offset of children which would cause a property
push.

This in practice will cause more push properties when any parent moves around.

Alternatively, maybe "offset from parent" is a draw property-style piece of data
that should be recomputed on the compositor thread?

[ajuma, 2015-04-17 22:53:11]

https://codereview.chromium.org/1097583002/diff/40001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_unittest.cc (left):

https://codereview.chromium.org/1097583002/diff/40001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest.cc:3448: child_->SetPosition(gfx::Point(1,
1));
On 2015/04/17 20:59:36, enne wrote:
> This code used to adjust the offset of children which would cause a property
> push.
> 
> This in practice will cause more push properties when any parent moves around.
> 
> Alternatively, maybe "offset from parent" is a draw property-style piece of
data
> that should be recomputed on the compositor thread?

If these extra pushes turn out to be costly, perhaps another alternative would
be to have a finer-grained PushPropertyTreeProperties instead of pushing all the
properties? My hand-wavy guess is that this would turn out to be cheaper than
re-computing all the offsets on the compositor side (since pushing would mean
visiting some of the layers when offsets change and copying an offset at each
visited layer, but re-computing on the compositor side would involve visiting
all of the layers after every commit and doing some math that's more complex
that just copying an offset).

[enne (OOO), 2015-04-17 23:00:06]

That seems reasonable.  Just curious what y'all thought about that tradeoff.

I'll commit this then, if y'all are ok with that?

[ajuma, 2015-04-17 23:09:54]

On 2015/04/17 23:00:06, enne wrote:
> That seems reasonable.  Just curious what y'all thought about that tradeoff.
> 
> I'll commit this then, if y'all are ok with that?

Sgtm!

[enne (OOO), 2015-04-17 23:13:43]

The CQ bit was checked by enne@chromium.org

[enne (OOO), 2015-04-17 23:13:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
vollick@chromium.org, ajuma@chromium.org
Link to the patchset: https://codereview.chromium.org/1097583002/#ps40001
(title: "Rebased on top of needs_rebuild patch")

[commit-bot: I haz the power, 2015-04-17 23:14:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1097583002/40001

[commit-bot: I haz the power, 2015-04-18 00:46:41]

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

[commit-bot: I haz the power, 2015-04-18 00:46:42]

Try jobs failed on following builders:
  win8_chromium_rel on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)

[enne (OOO), 2015-04-20 20:03:15]

The CQ bit was checked by enne@chromium.org

[enne (OOO), 2015-04-20 20:03:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
vollick@chromium.org, ajuma@chromium.org
Link to the patchset: https://codereview.chromium.org/1097583002/#ps60001
(title: "Win bool conversion compile fix")

[commit-bot: I haz the power, 2015-04-20 20:04:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1097583002/60001

[commit-bot: I haz the power, 2015-04-20 20:35:59]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-20 20:37:30]

Patchset 4 (id:??) landed as
https://crrev.com/e95b154d48067914e65a091aaee0b78359f7f08f
Cr-Commit-Position: refs/heads/master@{#325904}