cc : Fix effect tree index bug in cc::LayerImpl::Opacity

cc : Fix effect tree index bug in cc::LayerImpl::Opacity

Since LayerImpl::Opacity of scrollbar layer can be called during layer's
PushPropertiesTo, we can't use the effect tree index in
LayerImpl::Opacity as its the new value but the new property trees
are pushed only after layer PushProperties.

BUG=616401
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/52c00185e5194c59dbbad339394eef256cd2b872
Cr-Commit-Position: refs/heads/master@{#397341}

[jaydasika, 2016-06-01 20:46:45]

Description was changed from

==========
cc : Fix effect tree index bug in cc::LayerImpl::Opacity

Since LayerImpl::Opacity of scrollbar layer can be called during layer's
PushPropertiesTo, we can't use the effect tree index in
LayerImpl::Opacity as its the new value but the new property trees
are pushed only after layer PushProperties.

BUG=616401
==========

to

==========
cc : Fix effect tree index bug in cc::LayerImpl::Opacity

Since LayerImpl::Opacity of scrollbar layer can be called during layer's
PushPropertiesTo, we can't use the effect tree index in
LayerImpl::Opacity as its the new value but the new property trees
are pushed only after layer PushProperties.

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

[jaydasika, 2016-06-01 20:50:27]

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

[jaydasika, 2016-06-01 20:50:28]

I couldn't exactly repro the bug (as there was no test case in the bug), but
looking at the crash stack, I think this should fix the bug.

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

https://codereview.chromium.org/2033453002/diff/1/cc/trees/layer_tree_host_im...
cc/trees/layer_tree_host_impl_unittest.cc:2900: EffectNode* pending_tree_node =
This test will crash if we use effect_tree_index in LayerImpl::Opacity. The
crash stack is the same as the one reported in the bug.

[ajuma, 2016-06-01 21:09:54]

Just a question about the test.

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

https://codereview.chromium.org/2033453002/diff/1/cc/trees/layer_tree_host_im...
cc/trees/layer_tree_host_impl_unittest.cc:2893:
container->test_properties()->force_render_surface = true;
Why do we need a surface here?

[jaydasika, 2016-06-01 21:12:15]

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

https://codereview.chromium.org/2033453002/diff/1/cc/trees/layer_tree_host_im...
cc/trees/layer_tree_host_impl_unittest.cc:2893:
container->test_properties()->force_render_surface = true;
On 2016/06/01 21:09:54, ajuma wrote:
> Why do we need a surface here?

Don't need a surface. I added this so that container gets an effect node and
scrollbar layer's effect tree index changes.

[ajuma, 2016-06-01 21:17:09]

Thanks, lgtm

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

https://codereview.chromium.org/2033453002/diff/1/cc/trees/layer_tree_host_im...
cc/trees/layer_tree_host_impl_unittest.cc:2893:
container->test_properties()->force_render_surface = true;
On 2016/06/01 21:12:15, jaydasika wrote:
> On 2016/06/01 21:09:54, ajuma wrote:
> > Why do we need a surface here?
> 
> Don't need a surface. I added this so that container gets an effect node and
> scrollbar layer's effect tree index changes.

Ah, makes sense. Please add a comment about that.

[jaydasika, 2016-06-01 21:23:48]

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

https://codereview.chromium.org/2033453002/diff/1/cc/trees/layer_tree_host_im...
cc/trees/layer_tree_host_impl_unittest.cc:2893:
container->test_properties()->force_render_surface = true;
On 2016/06/01 21:17:09, ajuma wrote:
> On 2016/06/01 21:12:15, jaydasika wrote:
> > On 2016/06/01 21:09:54, ajuma wrote:
> > > Why do we need a surface here?
> > 
> > Don't need a surface. I added this so that container gets an effect node and
> > scrollbar layer's effect tree index changes.
> 
> Ah, makes sense. Please add a comment about that.

Done.

[jaydasika, 2016-06-01 21:23:52]

The CQ bit was checked by jaydasika@chromium.org

[jaydasika, 2016-06-01 21:23:52]

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/2033453002/#ps20001
(title: " ")

[commit-bot: I haz the power, 2016-06-01 21:24:24]

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

[commit-bot: I haz the power, 2016-06-02 02:04:34]

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

[commit-bot: I haz the power, 2016-06-02 02:04:35]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jaydasika, 2016-06-02 02:05:31]

The CQ bit was checked by jaydasika@chromium.org

[commit-bot: I haz the power, 2016-06-02 02:06:30]

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

[commit-bot: I haz the power, 2016-06-02 02:52:16]

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

[commit-bot: I haz the power, 2016-06-02 02:52:18]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jaydasika, 2016-06-02 02:56:47]

The CQ bit was checked by jaydasika@chromium.org

[commit-bot: I haz the power, 2016-06-02 02:57:11]

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

[commit-bot: I haz the power, 2016-06-02 03:44:10]

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

[commit-bot: I haz the power, 2016-06-02 03:44:11]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jaydasika, 2016-06-02 04:15:46]

The CQ bit was checked by jaydasika@chromium.org

[commit-bot: I haz the power, 2016-06-02 04:16:05]

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

[commit-bot: I haz the power, 2016-06-02 05:01:16]

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

[commit-bot: I haz the power, 2016-06-02 05:01:17]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jaydasika, 2016-06-02 07:39:32]

The CQ bit was checked by jaydasika@chromium.org

[commit-bot: I haz the power, 2016-06-02 07:39:57]

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

[commit-bot: I haz the power, 2016-06-02 08:57:36]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-02 08:58:42]

Description was changed from

==========
cc : Fix effect tree index bug in cc::LayerImpl::Opacity

Since LayerImpl::Opacity of scrollbar layer can be called during layer's
PushPropertiesTo, we can't use the effect tree index in
LayerImpl::Opacity as its the new value but the new property trees
are pushed only after layer PushProperties.

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

to

==========
cc : Fix effect tree index bug in cc::LayerImpl::Opacity

Since LayerImpl::Opacity of scrollbar layer can be called during layer's
PushPropertiesTo, we can't use the effect tree index in
LayerImpl::Opacity as its the new value but the new property trees
are pushed only after layer PushProperties.

BUG=616401
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/52c00185e5194c59dbbad339394eef256cd2b872
Cr-Commit-Position: refs/heads/master@{#397341}
==========

[commit-bot: I haz the power, 2016-06-02 08:58:43]

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