Prevent create ScrollbarAnimationController for Mac Overlay Scrollbar

Prevent create ScrollbarAnimationController for Mac Overlay Scrollbar

crbug.com/724507 is caused by my patch
(crrev.com/b50f6446eb79c4aaaac0a5eefdf90d2b4a99ce54). I assigned
`settings.scrollbar_animator = cc::LayerTreeSettings::ANDROID_OVERLAY`
for default which we assigned `LayerTreeSettings::NO_ANIMATOR` before.
Which can give mobile emulator correct settings but in
LayerTreeImpl::RegisterScrollbar -
LayerTreeHostImpl::RegisterScrollbarAnimationController we create
ScrollbarAnimationController to handle scrollbar's opacity. 

In this patch, we add GetScrollbarAnimator to ScrollbarLayerImplBase
and its subclasses. PaintedScrollbarLayerImpl::GetScrollbarAnimator can
always returns NO_ANIMATOR because AuraOverlayScrollbar is using
PaintedOverlayScrollbarLayerImpl, others can return based on
LayerTreeSettings. Then we can prevent create
ScrollbarAnimationController for Mac Overlay Scrollbar by checking
scrollbar's animator.

BUG=724507
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2918753002
Cr-Commit-Position: refs/heads/master@{#477287}
Committed: https://chromium.googlesource.com/chromium/src/+/4795906652460b283e526e6745057d77f0133980

[chaopeng, 2017-06-01 02:54:23]

Description was changed from

==========
Quick fix for Scrollbar doesn't appear with Rubberband effect

This issue introduced from b50f6446eb79c4aaaac0a5eefdf90d2b4a99ce54.
This patch revert the MacOS related changes in
b50f6446eb79c4aaaac0a5eefdf90d2b4a99ce54

BUG=724507
==========

to

==========
Quick fix for Scrollbar doesn't appear with Rubberband effect

This issue introduced from crrev.com/2870733002.
This patch revert the MacOS related changes in
crrev.com/2870733002.

BUG=724507
==========

[chaopeng, 2017-06-01 02:54:58]

chaopeng@chromium.org changed reviewers:
+ aelias@chromium.org

[chaopeng, 2017-06-01 02:54:58]

PTAL. Thank you.

[aelias_OOO_until_Jul13, 2017-06-01 03:00:45]

Doesn't this regress Mac devtools?  Can you look for a proper fix instead?

[chaopeng, 2017-06-01 03:47:51]

On 2017/06/01 03:00:45, aelias wrote:
> Doesn't this regress Mac devtools?  Can you look for a proper fix instead?

Yes, I am investigating a proper fix for this issue. I want to land and merge
this to M61 ensure Mac scrollbar works.

[aelias_OOO_until_Jul13, 2017-06-01 18:28:55]

I'd rather avoid the churn and the bug is only marked ReleaseBlock-Stable. 
Please update when you have a proper fix.

[chaopeng, 2017-06-02 19:56:31]

Description was changed from

==========
Quick fix for Scrollbar doesn't appear with Rubberband effect

This issue introduced from crrev.com/2870733002.
This patch revert the MacOS related changes in
crrev.com/2870733002.

BUG=724507
==========

to

==========
Quick fix for Scrollbar doesn't appear with Rubberband effect

This issue introduced from crrev.com/2870733002.
This patch revert the MacOS related changes in
crrev.com/2870733002.

BUG=724507
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[chaopeng, 2017-06-02 20:05:01]

Description was changed from

==========
Quick fix for Scrollbar doesn't appear with Rubberband effect

This issue introduced from crrev.com/2870733002.
This patch revert the MacOS related changes in
crrev.com/2870733002.

BUG=724507
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Prevent create ScrollbarAnimationController for Mac Overlay Scrollbar

crbug.com/724507 is caused by my patch
(crrev.com/b50f6446eb79c4aaaac0a5eefdf90d2b4a99ce54). I assigned
`settings.scrollbar_animator = cc::LayerTreeSettings::ANDROID_OVERLAY`
for default which we assigned `LayerTreeSettings::NO_ANIMATOR` before.
Which can give mobile emulator correct settings but in
LayerTreeImpl::RegisterScrollbar -
LayerTreeHostImpl::RegisterScrollbarAnimationController we create
ScrollbarAnimationController to handle scrollbar's opacity. 

In this patch, we identify Mac Overlay Scrollbar by Mac platform and
is_overlay_scrollbar and not SolidColorScrollbarLayerImpl then skip
create ScrollbarAnimationController for it.

BUG=724507
==========

[chaopeng, 2017-06-02 20:05:15]

chaopeng@chromium.org changed reviewers:
+ bokan@chromium.org

[chaopeng, 2017-06-02 20:06:14]

PTAL. Thank you.

[aelias_OOO_until_Jul13, 2017-06-02 22:02:24]

https://codereview.chromium.org/2918753002/diff/20001/cc/layers/scrollbar_lay...
File cc/layers/scrollbar_layer_impl_base.h (right):

https://codereview.chromium.org/2918753002/diff/20001/cc/layers/scrollbar_lay...
cc/layers/scrollbar_layer_impl_base.h:64: virtual bool
IsSolidColorScrollbarLayerImpl() const;
How about instead making scrollbars return one of the animator enum types to
specify which one they're compatible with?

https://codereview.chromium.org/2918753002/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl.cc (left):

https://codereview.chromium.org/2918753002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:3744: if (settings().scrollbar_animator ==
LayerTreeSettings::NO_ANIMATOR)
Why is this removed?

https://codereview.chromium.org/2918753002/diff/20001/cc/trees/layer_tree_imp...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/2918753002/diff/20001/cc/trees/layer_tree_imp...
cc/trees/layer_tree_impl.cc:514: this, [this](LayerImpl* layer) {
Please omit this unrelated whitespace change.

https://codereview.chromium.org/2918753002/diff/20001/cc/trees/layer_tree_imp...
cc/trees/layer_tree_impl.cc:1711: #if defined(OS_MACOSX)
Please find a way to avoid OS_MACOSX.  How about early-returning if either
NO_ANIMATOR or ANDROID_OVERLAY &&
!scrollbar_layer->IsSolidColorScrollbarLayerImpl()?

[chaopeng, 2017-06-05 14:32:40]

Description was changed from

==========
Prevent create ScrollbarAnimationController for Mac Overlay Scrollbar

crbug.com/724507 is caused by my patch
(crrev.com/b50f6446eb79c4aaaac0a5eefdf90d2b4a99ce54). I assigned
`settings.scrollbar_animator = cc::LayerTreeSettings::ANDROID_OVERLAY`
for default which we assigned `LayerTreeSettings::NO_ANIMATOR` before.
Which can give mobile emulator correct settings but in
LayerTreeImpl::RegisterScrollbar -
LayerTreeHostImpl::RegisterScrollbarAnimationController we create
ScrollbarAnimationController to handle scrollbar's opacity. 

In this patch, we identify Mac Overlay Scrollbar by Mac platform and
is_overlay_scrollbar and not SolidColorScrollbarLayerImpl then skip
create ScrollbarAnimationController for it.

BUG=724507
==========

to

==========
Prevent create ScrollbarAnimationController for Mac Overlay Scrollbar

crbug.com/724507 is caused by my patch
(crrev.com/b50f6446eb79c4aaaac0a5eefdf90d2b4a99ce54). I assigned
`settings.scrollbar_animator = cc::LayerTreeSettings::ANDROID_OVERLAY`
for default which we assigned `LayerTreeSettings::NO_ANIMATOR` before.
Which can give mobile emulator correct settings but in
LayerTreeImpl::RegisterScrollbar -
LayerTreeHostImpl::RegisterScrollbarAnimationController we create
ScrollbarAnimationController to handle scrollbar's opacity. 

In this patch, we identify Mac Overlay Scrollbar by Mac platform and
is_overlay_scrollbar and not SolidColorScrollbarLayerImpl then skip
create ScrollbarAnimationController for it.

BUG=724507
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[chaopeng, 2017-06-05 14:46:16]

Description was changed from

==========
Prevent create ScrollbarAnimationController for Mac Overlay Scrollbar

crbug.com/724507 is caused by my patch
(crrev.com/b50f6446eb79c4aaaac0a5eefdf90d2b4a99ce54). I assigned
`settings.scrollbar_animator = cc::LayerTreeSettings::ANDROID_OVERLAY`
for default which we assigned `LayerTreeSettings::NO_ANIMATOR` before.
Which can give mobile emulator correct settings but in
LayerTreeImpl::RegisterScrollbar -
LayerTreeHostImpl::RegisterScrollbarAnimationController we create
ScrollbarAnimationController to handle scrollbar's opacity. 

In this patch, we identify Mac Overlay Scrollbar by Mac platform and
is_overlay_scrollbar and not SolidColorScrollbarLayerImpl then skip
create ScrollbarAnimationController for it.

BUG=724507
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Prevent create ScrollbarAnimationController for Mac Overlay Scrollbar

crbug.com/724507 is caused by my patch
(crrev.com/b50f6446eb79c4aaaac0a5eefdf90d2b4a99ce54). I assigned
`settings.scrollbar_animator = cc::LayerTreeSettings::ANDROID_OVERLAY`
for default which we assigned `LayerTreeSettings::NO_ANIMATOR` before.
Which can give mobile emulator correct settings but in
LayerTreeImpl::RegisterScrollbar -
LayerTreeHostImpl::RegisterScrollbarAnimationController we create
ScrollbarAnimationController to handle scrollbar's opacity. 

In this patch, we add GetScrollbarAnimator to ScrollbarLayerImplBase
and its subclasses. PaintedScrollbarLayerImpl::GetScrollbarAnimator can
always returns NO_ANIMATOR because AuraOverlayScrollbar is using
PaintedOverlayScrollbarLayerImpl, others can return based on LayerTreeSettings.

BUG=724507
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[chaopeng, 2017-06-05 14:52:05]

Description was changed from

==========
Prevent create ScrollbarAnimationController for Mac Overlay Scrollbar

crbug.com/724507 is caused by my patch
(crrev.com/b50f6446eb79c4aaaac0a5eefdf90d2b4a99ce54). I assigned
`settings.scrollbar_animator = cc::LayerTreeSettings::ANDROID_OVERLAY`
for default which we assigned `LayerTreeSettings::NO_ANIMATOR` before.
Which can give mobile emulator correct settings but in
LayerTreeImpl::RegisterScrollbar -
LayerTreeHostImpl::RegisterScrollbarAnimationController we create
ScrollbarAnimationController to handle scrollbar's opacity. 

In this patch, we add GetScrollbarAnimator to ScrollbarLayerImplBase
and its subclasses. PaintedScrollbarLayerImpl::GetScrollbarAnimator can
always returns NO_ANIMATOR because AuraOverlayScrollbar is using
PaintedOverlayScrollbarLayerImpl, others can return based on LayerTreeSettings.

BUG=724507
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Prevent create ScrollbarAnimationController for Mac Overlay Scrollbar

crbug.com/724507 is caused by my patch
(crrev.com/b50f6446eb79c4aaaac0a5eefdf90d2b4a99ce54). I assigned
`settings.scrollbar_animator = cc::LayerTreeSettings::ANDROID_OVERLAY`
for default which we assigned `LayerTreeSettings::NO_ANIMATOR` before.
Which can give mobile emulator correct settings but in
LayerTreeImpl::RegisterScrollbar -
LayerTreeHostImpl::RegisterScrollbarAnimationController we create
ScrollbarAnimationController to handle scrollbar's opacity. 

In this patch, we add GetScrollbarAnimator to ScrollbarLayerImplBase
and its subclasses. PaintedScrollbarLayerImpl::GetScrollbarAnimator can
always returns NO_ANIMATOR because AuraOverlayScrollbar is using
PaintedOverlayScrollbarLayerImpl, others can return based on
LayerTreeSettings. Then we can prevent create
ScrollbarAnimationController for Mac Overlay Scrollbar by checking
scrollbar's animator.

BUG=724507
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[chaopeng, 2017-06-05 14:52:40]

Updated. PTAL. Thank you.

https://codereview.chromium.org/2918753002/diff/20001/cc/layers/scrollbar_lay...
File cc/layers/scrollbar_layer_impl_base.h (right):

https://codereview.chromium.org/2918753002/diff/20001/cc/layers/scrollbar_lay...
cc/layers/scrollbar_layer_impl_base.h:64: virtual bool
IsSolidColorScrollbarLayerImpl() const;
On 2017/06/02 22:02:23, aelias wrote:
> How about instead making scrollbars return one of the animator enum types to
> specify which one they're compatible with?

Yes, this would be better.

https://codereview.chromium.org/2918753002/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl.cc (left):

https://codereview.chromium.org/2918753002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:3744: if (settings().scrollbar_animator ==
LayerTreeSettings::NO_ANIMATOR)
On 2017/06/02 22:02:23, aelias wrote:
> Why is this removed?

Moved to LayerTreeImpl::RegisterScrollbar.

[aelias_OOO_until_Jul13, 2017-06-06 02:26:11]

lgtm. thanks!

[chaopeng, 2017-06-06 13:54:31]

The CQ bit was checked by chaopeng@chromium.org

[commit-bot: I haz the power, 2017-06-06 13:54:45]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[chaopeng, 2017-06-06 13:54:54]

The CQ bit was unchecked by chaopeng@chromium.org

[chaopeng, 2017-06-06 14:51:33]

The CQ bit was checked by chaopeng@chromium.org

[commit-bot: I haz the power, 2017-06-06 14:51:53]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-06 15:06:43]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496760693749350,
"parent_rev": "5af488e3095f444e1ec680c7b89dfee2c772b35b", "commit_rev":
"4795906652460b283e526e6745057d77f0133980"}

[commit-bot: I haz the power, 2017-06-06 15:06:57]

Description was changed from

==========
Prevent create ScrollbarAnimationController for Mac Overlay Scrollbar

crbug.com/724507 is caused by my patch
(crrev.com/b50f6446eb79c4aaaac0a5eefdf90d2b4a99ce54). I assigned
`settings.scrollbar_animator = cc::LayerTreeSettings::ANDROID_OVERLAY`
for default which we assigned `LayerTreeSettings::NO_ANIMATOR` before.
Which can give mobile emulator correct settings but in
LayerTreeImpl::RegisterScrollbar -
LayerTreeHostImpl::RegisterScrollbarAnimationController we create
ScrollbarAnimationController to handle scrollbar's opacity. 

In this patch, we add GetScrollbarAnimator to ScrollbarLayerImplBase
and its subclasses. PaintedScrollbarLayerImpl::GetScrollbarAnimator can
always returns NO_ANIMATOR because AuraOverlayScrollbar is using
PaintedOverlayScrollbarLayerImpl, others can return based on
LayerTreeSettings. Then we can prevent create
ScrollbarAnimationController for Mac Overlay Scrollbar by checking
scrollbar's animator.

BUG=724507
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Prevent create ScrollbarAnimationController for Mac Overlay Scrollbar

crbug.com/724507 is caused by my patch
(crrev.com/b50f6446eb79c4aaaac0a5eefdf90d2b4a99ce54). I assigned
`settings.scrollbar_animator = cc::LayerTreeSettings::ANDROID_OVERLAY`
for default which we assigned `LayerTreeSettings::NO_ANIMATOR` before.
Which can give mobile emulator correct settings but in
LayerTreeImpl::RegisterScrollbar -
LayerTreeHostImpl::RegisterScrollbarAnimationController we create
ScrollbarAnimationController to handle scrollbar's opacity. 

In this patch, we add GetScrollbarAnimator to ScrollbarLayerImplBase
and its subclasses. PaintedScrollbarLayerImpl::GetScrollbarAnimator can
always returns NO_ANIMATOR because AuraOverlayScrollbar is using
PaintedOverlayScrollbarLayerImpl, others can return based on
LayerTreeSettings. Then we can prevent create
ScrollbarAnimationController for Mac Overlay Scrollbar by checking
scrollbar's animator.

BUG=724507
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2918753002
Cr-Commit-Position: refs/heads/master@{#477287}
Committed:
https://chromium.googlesource.com/chromium/src/+/4795906652460b283e526e674505...
==========

[commit-bot: I haz the power, 2017-06-06 15:06:58]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/4795906652460b283e526e674505...