Add a hide_layer_and_subtree() flag to cc::Layer

Add a hide_layer_and_subtree() flag to cc::Layer

This flag does what it says. We can use this flag to simplify ui::Layer
as it was recursively calling SetIsDrawable() instead, and now can just
SetHideLayerAndSubtree() on a single layer.

This flag is needed to allow readbacks of layers that are have hidden
visibility (possibly due to an ancestor).

Tests:
LayerTest.CheckPropertyChangeCausesCorrectBehavior
LayerImplTest.VerifyLayerChangesAreTrackedProperly
LayerTreeHostCommonTest.SubtreeHidden_SingleLayer
LayerTreeHostCommonTest.SubtreeHidden_SingleLayerImpl
LayerTreeHostCommonTest.SubtreeHidden_TwoLayers
LayerTreeHostCommonTest.SubtreeHidden_TwoLayersImpl

R=enne, piman
BUG=242572
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=208241

[danakj, 2013-06-19 23:34:59]

enne@ please review cc/
piman@ please review ui/

[danakj, 2013-06-19 23:37:22]

Oh, I never push properties the flag do I. Clearly I need a test for that.

[enne (OOO), 2013-06-19 23:48:22]

lgtm, once the flag gets pushed in PushPropertiesTo.

[danakj, 2013-06-20 00:13:31]

New patch with PushProps and a test, and a bug fix for the root layer not saving
paint properties if it !DrawsContent in LTHCommon

[enne (OOO), 2013-06-20 00:17:47]

https://codereview.chromium.org/16896017/diff/6001/cc/trees/layer_tree_host_c...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/16896017/diff/6001/cc/trees/layer_tree_host_c...
cc/trees/layer_tree_host_common.cc:1504: if (!IsRootLayer(layer) &&
sorting_start_index == descendants.size())
If the root layer doesn't draw, why does it need to save paint properties?

[danakj, 2013-06-20 00:20:04]

https://codereview.chromium.org/16896017/diff/6001/cc/trees/layer_tree_host_c...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/16896017/diff/6001/cc/trees/layer_tree_host_c...
cc/trees/layer_tree_host_common.cc:1504: if (!IsRootLayer(layer) &&
sorting_start_index == descendants.size())
On 2013/06/20 00:17:47, enne wrote:
> If the root layer doesn't draw, why does it need to save paint properties?

Because otherwise the impl layer doesn't get the real values pushed. Right now
that's just bounds, so the impl root layer always has bounds 0x0 even though we
have a bounds on the main thread. That seems wrong, though not hugely
problematic in practice.

[enne (OOO), 2013-06-20 00:24:21]

On 2013/06/20 00:20:04, danakj wrote:
>
https://codereview.chromium.org/16896017/diff/6001/cc/trees/layer_tree_host_c...
> File cc/trees/layer_tree_host_common.cc (right):
> 
>
https://codereview.chromium.org/16896017/diff/6001/cc/trees/layer_tree_host_c...
> cc/trees/layer_tree_host_common.cc:1504: if (!IsRootLayer(layer) &&
> sorting_start_index == descendants.size())
> On 2013/06/20 00:17:47, enne wrote:
> > If the root layer doesn't draw, why does it need to save paint properties?
> 
> Because otherwise the impl layer doesn't get the real values pushed. Right now
> that's just bounds, so the impl root layer always has bounds 0x0 even though
we
> have a bounds on the main thread. That seems wrong, though not hugely
> problematic in practice.

Sure, but this is also true for any other layer that doesn't get drawn?  Why is
the root layer special here?

[danakj, 2013-06-20 00:27:20]

On Wed, Jun 19, 2013 at 8:24 PM, <enne@chromium.org> wrote:

> On 2013/06/20 00:20:04, danakj wrote:
>
> https://codereview.chromium.**org/16896017/diff/6001/cc/**
>
trees/layer_tree_host_common.**cc<https://codereview.chromium.org/16896017/diff/6001/cc/trees/layer_tree_host_common.cc>
>
>> File cc/trees/layer_tree_host_**common.cc (right):
>>
>
>
> https://codereview.chromium.**org/16896017/diff/6001/cc/**
>
trees/layer_tree_host_common.**cc#newcode1504<https://codereview.chromium.org/16896017/diff/6001/cc/trees/layer_tree_host_common.cc#newcode1504>
>
>> cc/trees/layer_tree_host_**common.cc:1504: if (!IsRootLayer(layer) &&
>> sorting_start_index == descendants.size())
>> On 2013/06/20 00:17:47, enne wrote:
>> > If the root layer doesn't draw, why does it need to save paint
>> properties?
>>
>
>  Because otherwise the impl layer doesn't get the real values pushed.
>> Right now
>> that's just bounds, so the impl root layer always has bounds 0x0 even
>> though
>>
> we
>
>> have a bounds on the main thread. That seems wrong, though not hugely
>> problematic in practice.
>>
>
> Sure, but this is also true for any other layer that doesn't get drawn?
>  Why is
> the root layer special here?
>

Oh.. it's true! I've moved the Save call instead.


>
>
https://codereview.chromium.**org/16896017/<https://codereview.chromium.org/1...
>

[enne (OOO), 2013-06-20 00:38:39]

lgtmforreal

[piman, 2013-06-20 00:41:05]

LGTM!

[danakj, 2013-06-20 00:57:11]

+sky Please take a look at ui/aura/window.cc.

We no longer have a recursively set flag on ui::Layer, so aura::Window needs to
walk up to the root instead.

Thanks!

[danakj, 2013-06-20 01:19:59]

On 2013/06/20 00:57:11, danakj wrote:
> +sky Please take a look at ui/aura/window.cc.
> 
> We no longer have a recursively set flag on ui::Layer, so aura::Window needs
to
> walk up to the root instead.
> 
> Thanks!

sky, please have a look at the related changes in ui/views as well for the test.
Thanks.

[danakj, 2013-06-20 15:20:50]

Ah, instead of moving this IsDrawn() walk up the parents to all the call sites,
just reverting that Layer::IsDrawn() method to what it used to be when sky@
added it to ui::Layer, so it does the tree walk, and no changes are needed
anywhere else.

[commit-bot: I haz the power, 2013-06-20 15:21:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/16896017/34001

[commit-bot: I haz the power, 2013-06-20 19:40:20]

Change committed as 207508

[commit-bot: I haz the power, 2013-06-24 15:49:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/16896017/51001

[commit-bot: I haz the power, 2013-06-24 18:32:20]

Change committed as 208241