cc : Track ids for which active opacity always wins on property trees

cc : Track ids for which active opacity always wins on property trees

And then use this for partial sync of property trees at the time of
activation. PaintedScrollbarLayer and SolidColorScrollbarLayer currently
need the active tree opacity to win at activation.

BUG=611899
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/ae9b7a05e087905774fb29807281fa771711ff3b
Cr-Commit-Position: refs/heads/master@{#394784}

[jaydasika, 2016-05-17 23:30:07]

Description was changed from

==========
cc : Track ids for which active opacity always wins on property trees

And then use this for partial sync of property trees at the time of
activation. PaintedScrollbarLayer and SolidColorScrollbarLayer currently
need the active tree opacity to win at activation.

BUG=611899
==========

to

==========
cc : Track ids for which active opacity always wins on property trees

And then use this for partial sync of property trees at the time of
activation. PaintedScrollbarLayer and SolidColorScrollbarLayer currently
need the active tree opacity to win at activation.

BUG=611899
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[jaydasika, 2016-05-17 23:33:00]

jaydasika@chromium.org changed reviewers:
+ ajuma@chromium.org, vollick@chromium.org, weiliangc@chromium.org

[jaydasika, 2016-05-17 23:33:01]

PTAL

[ajuma, 2016-05-18 13:53:48]

https://codereview.chromium.org/1991523002/diff/1/cc/layers/scrollbar_layer_i...
File cc/layers/scrollbar_layer_impl_base.cc (right):

https://codereview.chromium.org/1991523002/diff/1/cc/layers/scrollbar_layer_i...
cc/layers/scrollbar_layer_impl_base.cc:44: LayerImpl::DidBecomeActive();
Since this just calls into the base class now, ScrollbarLayerImplBase doesn't
need to override DidBecomeActive anymore.

https://codereview.chromium.org/1991523002/diff/1/cc/trees/layer_tree_host_im...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/1991523002/diff/1/cc/trees/layer_tree_host_im...
cc/trees/layer_tree_host_impl_unittest.cc:2898:
pending_scrollbar_layer->SetNeedsPushProperties();
Is this line needed?

https://codereview.chromium.org/1991523002/diff/1/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/1991523002/diff/1/cc/trees/property_tree.cc#n...
cc/trees/property_tree.cc:2049: target_tree->effect_tree.set_needs_update(true);
This should be conditional on the opacity value changing.

https://codereview.chromium.org/1991523002/diff/1/cc/trees/property_tree_buil...
File cc/trees/property_tree_builder.cc (right):

https://codereview.chromium.org/1991523002/diff/1/cc/trees/property_tree_buil...
cc/trees/property_tree_builder.cc:823: .push_back(node.owner_id);
Are there tests for this (that break when we don't add nodes to the map here)?

[jaydasika, 2016-05-19 00:27:09]

https://codereview.chromium.org/1991523002/diff/1/cc/layers/scrollbar_layer_i...
File cc/layers/scrollbar_layer_impl_base.cc (right):

https://codereview.chromium.org/1991523002/diff/1/cc/layers/scrollbar_layer_i...
cc/layers/scrollbar_layer_impl_base.cc:44: LayerImpl::DidBecomeActive();
On 2016/05/18 13:53:48, ajuma wrote:
> Since this just calls into the base class now, ScrollbarLayerImplBase doesn't
> need to override DidBecomeActive anymore.

Done.

https://codereview.chromium.org/1991523002/diff/1/cc/trees/layer_tree_host_im...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/1991523002/diff/1/cc/trees/layer_tree_host_im...
cc/trees/layer_tree_host_impl_unittest.cc:2898:
pending_scrollbar_layer->SetNeedsPushProperties();
On 2016/05/18 13:53:48, ajuma wrote:
> Is this line needed?
I added this when I realized ScrollbarlayerImplBase::PushPropertiesTo was not
getting called during the activation here. So, scrollbar_layer->opacity() was
not really getting tested. The active opacity remains because we don't push
properties and not because PushPropertiesTo ensures it. But, given that we are
going to delete LayerImpl::opacity soon, I think its not needed.

https://codereview.chromium.org/1991523002/diff/1/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/1991523002/diff/1/cc/trees/property_tree.cc#n...
cc/trees/property_tree.cc:2049: target_tree->effect_tree.set_needs_update(true);
On 2016/05/18 13:53:48, ajuma wrote:
> This should be conditional on the opacity value changing.

Done.

https://codereview.chromium.org/1991523002/diff/1/cc/trees/property_tree_buil...
File cc/trees/property_tree_builder.cc (right):

https://codereview.chromium.org/1991523002/diff/1/cc/trees/property_tree_buil...
cc/trees/property_tree_builder.cc:823: .push_back(node.owner_id);
On 2016/05/18 13:53:48, ajuma wrote:
> Are there tests for this (that break when we don't add nodes to the map here)?

Added one.

[ajuma, 2016-05-19 14:21:25]

Thanks, lgtm

[jaydasika, 2016-05-19 16:08:40]

The CQ bit was checked by jaydasika@chromium.org

[commit-bot: I haz the power, 2016-05-19 16:08:55]

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

[commit-bot: I haz the power, 2016-05-19 16:13:06]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-05-19 16:14:35]

Description was changed from

==========
cc : Track ids for which active opacity always wins on property trees

And then use this for partial sync of property trees at the time of
activation. PaintedScrollbarLayer and SolidColorScrollbarLayer currently
need the active tree opacity to win at activation.

BUG=611899
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc : Track ids for which active opacity always wins on property trees

And then use this for partial sync of property trees at the time of
activation. PaintedScrollbarLayer and SolidColorScrollbarLayer currently
need the active tree opacity to win at activation.

BUG=611899
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/ae9b7a05e087905774fb29807281fa771711ff3b
Cr-Commit-Position: refs/heads/master@{#394784}
==========

[commit-bot: I haz the power, 2016-05-19 16:14:36]

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