Removing the assumption that child->SetParent will not change Drawability

Removing the assumption that child->SetParent will not change Drawability

R=danakj

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289958

[danakj, 2014-08-14 22:00:14]

https://codereview.chromium.org/471233002/diff/1/cc/layers/ui_resource_layer_...
File cc/layers/ui_resource_layer_unittest.cc (right):

https://codereview.chromium.org/471233002/diff/1/cc/layers/ui_resource_layer_...
cc/layers/ui_resource_layer_unittest.cc:121:
EXPECT_FALSE(test_layer->DrawsContent());
Can you do this test with a test-only override of cc::Layer that changes its
drawability in SetLTH similarly to UIResourceLayer (or add this capability to
cc::FakeLayer), and move to layer_unittest? And can you verify the number of
descendants that draw content on the parent as well?

[awoloszyn, 2014-08-14 22:27:12]

On 2014/08/14 22:00:14, danakj wrote:
>
https://codereview.chromium.org/471233002/diff/1/cc/layers/ui_resource_layer_...
> File cc/layers/ui_resource_layer_unittest.cc (right):
> 
>
https://codereview.chromium.org/471233002/diff/1/cc/layers/ui_resource_layer_...
> cc/layers/ui_resource_layer_unittest.cc:121:
> EXPECT_FALSE(test_layer->DrawsContent());
> Can you do this test with a test-only override of cc::Layer that changes its
> drawability in SetLTH similarly to UIResourceLayer (or add this capability to
> cc::FakeLayer), and move to layer_unittest? And can you verify the number of
> descendants that draw content on the parent as well?

Done.

[danakj, 2014-08-15 15:40:30]

LGTM thanks!

https://codereview.chromium.org/471233002/diff/20001/cc/layers/layer_unittest.cc
File cc/layers/layer_unittest.cc (right):

https://codereview.chromium.org/471233002/diff/20001/cc/layers/layer_unittest...
cc/layers/layer_unittest.cc:59: class DrawsContentChangeLayer : public Layer {
Can you put this just above the test so it's closer to where it's used?

https://codereview.chromium.org/471233002/diff/20001/cc/layers/layer_unittest...
cc/layers/layer_unittest.cc:80: bool fake_draws_content_;
move variable below methods, and a blank line between

https://codereview.chromium.org/471233002/diff/20001/cc/layers/layer_unittest...
cc/layers/layer_unittest.cc:1182: scoped_refptr<DrawsContentChangeLayer>
child_layer =
becomes_not_draws_content

https://codereview.chromium.org/471233002/diff/20001/cc/layers/layer_unittest...
cc/layers/layer_unittest.cc:1184: scoped_refptr<DrawsContentChangeLayer>
second_child =
becomes_draws_content

[awoloszyn, 2014-08-15 15:56:46]

https://codereview.chromium.org/471233002/diff/20001/cc/layers/layer_unittest.cc
File cc/layers/layer_unittest.cc (right):

https://codereview.chromium.org/471233002/diff/20001/cc/layers/layer_unittest...
cc/layers/layer_unittest.cc:59: class DrawsContentChangeLayer : public Layer {
On 2014/08/15 15:40:30, danakj wrote:
> Can you put this just above the test so it's closer to where it's used?

Done.

https://codereview.chromium.org/471233002/diff/20001/cc/layers/layer_unittest...
cc/layers/layer_unittest.cc:80: bool fake_draws_content_;
On 2014/08/15 15:40:30, danakj wrote:
> move variable below methods, and a blank line between

Done.

https://codereview.chromium.org/471233002/diff/20001/cc/layers/layer_unittest...
cc/layers/layer_unittest.cc:1182: scoped_refptr<DrawsContentChangeLayer>
child_layer =
On 2014/08/15 15:40:30, danakj wrote:
> becomes_not_draws_content

Done.

https://codereview.chromium.org/471233002/diff/20001/cc/layers/layer_unittest...
cc/layers/layer_unittest.cc:1184: scoped_refptr<DrawsContentChangeLayer>
second_child =
On 2014/08/15 15:40:30, danakj wrote:
> becomes_draws_content

Done.

[awoloszyn, 2014-08-15 15:56:52]

The CQ bit was checked by awoloszyn@chromium.org

[commit-bot: I haz the power, 2014-08-15 15:59:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/awoloszyn@chromium.org/471233002/40001

[commit-bot: I haz the power, 2014-08-15 18:55:27]

Committed patchset #3 (40001) as 289958