[Android]Increase Scrollbar fade delay on Resize.

[Android]Increase Scrollbar fade delay on Resize.

Increase Scrollbar fade delay during on Resize to 2000mS. Scrollbar
layer properties changes during page load, which is triggering
scrollbar animation continuously because of 300mS fade delay.
Increasing the fade delay to 2000mS on Resize ensures the
scrollbar animates optimally rather than animating rapidly while
loading heavy sites.

BUG=361141
Committed: https://crrev.com/07f11a816954910f11186cfe461f4bb3cfc5faaf
Cr-Commit-Position: refs/heads/master@{#298471}

[MuVen, 2014-09-29 10:10:58]

sataya.m@samsung.com changed reviewers:
+ aelias@chromium.org, danakj@chromium.org, jdduke@chromium.org

[MuVen, 2014-09-29 10:11:07]

Patchset #1 (id:1) has been deleted

[MuVen, 2014-09-29 10:13:24]

Patchset #1 (id:20001) has been deleted

[MuVen, 2014-09-29 10:13:49]

PTAL.

[aelias_OOO_until_Jul13, 2014-09-29 19:33:46]

https://codereview.chromium.org/608223002/diff/40001/cc/animation/scrollbar_a...
File cc/animation/scrollbar_animation_controller.h (right):

https://codereview.chromium.org/608223002/diff/40001/cc/animation/scrollbar_a...
cc/animation/scrollbar_animation_controller.h:34: void
SetDelayOnScrollbarAnimation(base::TimeDelta delay);
Instead of this, please have a new method "virtual void DidResizeBounds();" and
pass in the second delay in the constructor.

https://codereview.chromium.org/608223002/diff/40001/cc/layers/layer_impl.h
File cc/layers/layer_impl.h (right):

https://codereview.chromium.org/608223002/diff/40001/cc/layers/layer_impl.h#n...
cc/layers/layer_impl.h:634: bool delay_scrollbar_animation_ : 1;
Please avoid adding a member variable for transient state, everything here is
doable using local variables/arguments.

https://codereview.chromium.org/608223002/diff/40001/cc/trees/layer_tree_sett...
File cc/trees/layer_tree_settings.cc (right):

https://codereview.chromium.org/608223002/diff/40001/cc/trees/layer_tree_sett...
cc/trees/layer_tree_settings.cc:36: scrollbar_fade_extra_delay_ms(0),
Please rename to "scrollbar_fade_resize_delay_ms"

[MuVen, 2014-09-30 12:50:26]

PTAL

https://codereview.chromium.org/608223002/diff/40001/cc/animation/scrollbar_a...
File cc/animation/scrollbar_animation_controller.h (right):

https://codereview.chromium.org/608223002/diff/40001/cc/animation/scrollbar_a...
cc/animation/scrollbar_animation_controller.h:34: void
SetDelayOnScrollbarAnimation(base::TimeDelta delay);
On 2014/09/29 19:33:46, aelias wrote:
> Instead of this, please have a new method "virtual void DidResizeBounds();"
and
> pass in the second delay in the constructor.

Done.(passed the second delay in the constructor).
Instead of having one more method i have passed on_resize parameter based on
which we can set the delay time.

https://codereview.chromium.org/608223002/diff/40001/cc/layers/layer_impl.h
File cc/layers/layer_impl.h (right):

https://codereview.chromium.org/608223002/diff/40001/cc/layers/layer_impl.h#n...
cc/layers/layer_impl.h:634: bool delay_scrollbar_animation_ : 1;
On 2014/09/29 19:33:46, aelias wrote:
> Please avoid adding a member variable for transient state, everything here is
> doable using local variables/arguments.

Done.

https://codereview.chromium.org/608223002/diff/40001/cc/trees/layer_tree_sett...
File cc/trees/layer_tree_settings.cc (right):

https://codereview.chromium.org/608223002/diff/40001/cc/trees/layer_tree_sett...
cc/trees/layer_tree_settings.cc:36: scrollbar_fade_extra_delay_ms(0),
On 2014/09/29 19:33:46, aelias wrote:
> Please rename to "scrollbar_fade_resize_delay_ms"

Done.

[aelias_OOO_until_Jul13, 2014-09-30 18:55:28]

https://codereview.chromium.org/608223002/diff/60001/cc/animation/scrollbar_a...
File cc/animation/scrollbar_animation_controller.h (right):

https://codereview.chromium.org/608223002/diff/60001/cc/animation/scrollbar_a...
cc/animation/scrollbar_animation_controller.h:35: virtual void
DidScrollUpdate(bool on_resize = false);
Please remove this default argument.

https://codereview.chromium.org/608223002/diff/60001/cc/animation/scrollbar_a...
cc/animation/scrollbar_animation_controller.h:56: void PostDelayedFade(bool
on_resize = false);
Please remove this default argument.

https://codereview.chromium.org/608223002/diff/60001/cc/animation/scrollbar_a...
File cc/animation/scrollbar_animation_controller_linear_fade.h (right):

https://codereview.chromium.org/608223002/diff/60001/cc/animation/scrollbar_a...
cc/animation/scrollbar_animation_controller_linear_fade.h:27: virtual void
DidScrollUpdate(bool on_resize = false) OVERRIDE;
Default arguments are against the Chromium style guide.

https://codereview.chromium.org/608223002/diff/60001/cc/animation/scrollbar_a...
File cc/animation/scrollbar_animation_controller_thinning.h (right):

https://codereview.chromium.org/608223002/diff/60001/cc/animation/scrollbar_a...
cc/animation/scrollbar_animation_controller_thinning.h:35: virtual void
DidScrollUpdate(bool on_resize = false) OVERRIDE;
Please remove this default argument.

https://codereview.chromium.org/608223002/diff/60001/cc/layers/layer_impl.cc
File cc/layers/layer_impl.cc (right):

https://codereview.chromium.org/608223002/diff/60001/cc/layers/layer_impl.cc#...
cc/layers/layer_impl.cc:1230: bool on_resize) const {
I don't think the new argument is needed here.  You can build on
https://codereview.chromium.org/571873003/ and use the longer delay if
SetMaximum or SetVisibleToTotalLengthRatio returns true.

https://codereview.chromium.org/608223002/diff/60001/cc/layers/layer_impl.h
File cc/layers/layer_impl.h (right):

https://codereview.chromium.org/608223002/diff/60001/cc/layers/layer_impl.h#n...
cc/layers/layer_impl.h:515: void ScrollbarParametersDidChange(bool on_resize =
false);
Because you can remove the argument in SetScrollbarPosition, you can remove it
in ScrollbarParamtersDidChange as well.

[MuVen, 2014-10-01 04:56:55]

Patchset #3 (id:80001) has been deleted

[MuVen, 2014-10-01 05:00:27]

Patchset #3 (id:100001) has been deleted

[MuVen, 2014-10-01 05:32:44]

Patchset #4 (id:140001) has been deleted

[MuVen, 2014-10-01 05:33:00]

Patchset #4 (id:160001) has been deleted

[MuVen, 2014-10-01 16:44:30]

Patchset #3 (id:120001) has been deleted

[MuVen, 2014-10-01 16:45:52]

PTAL.

https://codereview.chromium.org/608223002/diff/60001/cc/animation/scrollbar_a...
File cc/animation/scrollbar_animation_controller.h (right):

https://codereview.chromium.org/608223002/diff/60001/cc/animation/scrollbar_a...
cc/animation/scrollbar_animation_controller.h:35: virtual void
DidScrollUpdate(bool on_resize = false);
On 2014/09/30 18:55:27, aelias wrote:
> Please remove this default argument.

Done.

https://codereview.chromium.org/608223002/diff/60001/cc/animation/scrollbar_a...
cc/animation/scrollbar_animation_controller.h:56: void PostDelayedFade(bool
on_resize = false);
On 2014/09/30 18:55:27, aelias wrote:
> Please remove this default argument.

Done.

https://codereview.chromium.org/608223002/diff/60001/cc/animation/scrollbar_a...
File cc/animation/scrollbar_animation_controller_linear_fade.h (right):

https://codereview.chromium.org/608223002/diff/60001/cc/animation/scrollbar_a...
cc/animation/scrollbar_animation_controller_linear_fade.h:27: virtual void
DidScrollUpdate(bool on_resize = false) OVERRIDE;
On 2014/09/30 18:55:27, aelias wrote:
> Default arguments are against the Chromium style guide.

Done.

https://codereview.chromium.org/608223002/diff/60001/cc/layers/layer_impl.cc
File cc/layers/layer_impl.cc (right):

https://codereview.chromium.org/608223002/diff/60001/cc/layers/layer_impl.cc#...
cc/layers/layer_impl.cc:1230: bool on_resize) const {
On 2014/09/30 18:55:27, aelias wrote:
> I don't think the new argument is needed here.  You can build on
> https://codereview.chromium.org/571873003/ and use the longer delay if
> SetMaximum or SetVisibleToTotalLengthRatio returns true.

Done.

https://codereview.chromium.org/608223002/diff/60001/cc/layers/layer_impl.h
File cc/layers/layer_impl.h (right):

https://codereview.chromium.org/608223002/diff/60001/cc/layers/layer_impl.h#n...
cc/layers/layer_impl.h:515: void ScrollbarParametersDidChange(bool on_resize =
false);
On 2014/09/30 18:55:28, aelias wrote:
> Because you can remove the argument in SetScrollbarPosition, you can remove it
> in ScrollbarParamtersDidChange as well.

Done.

[aelias_OOO_until_Jul13, 2014-10-01 18:41:27]

Looking better, just a few more comments.

https://codereview.chromium.org/608223002/diff/180001/cc/animation/scrollbar_...
File cc/animation/scrollbar_animation_controller_linear_fade_unittest.cc
(right):

https://codereview.chromium.org/608223002/diff/180001/cc/animation/scrollbar_...
cc/animation/scrollbar_animation_controller_linear_fade_unittest.cc:16: class
ScrollbarAnimationControllerLinearFadeTest
Please add a new test case for the new behavior in this class.

https://codereview.chromium.org/608223002/diff/180001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/608223002/diff/180001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl_unittest.cc:1438: // After a scroll, a fade
animation should be scheduled about 50ms from now.
"After pinch causes a scrollbar resize,"

https://codereview.chromium.org/608223002/diff/180001/content/renderer/gpu/re...
File content/renderer/gpu/render_widget_compositor.cc (right):

https://codereview.chromium.org/608223002/diff/180001/content/renderer/gpu/re...
content/renderer/gpu/render_widget_compositor.cc:410:
settings.scrollbar_fade_resize_delay_ms = 2000;
Please set this to 500 since the page-load fade-spam problem doesn't affect
desktop pinch zoom scrollbars.

[MuVen, 2014-10-02 08:42:48]

PTAL. Updated new testcase.

https://codereview.chromium.org/608223002/diff/180001/cc/animation/scrollbar_...
File cc/animation/scrollbar_animation_controller_linear_fade_unittest.cc
(right):

https://codereview.chromium.org/608223002/diff/180001/cc/animation/scrollbar_...
cc/animation/scrollbar_animation_controller_linear_fade_unittest.cc:16: class
ScrollbarAnimationControllerLinearFadeTest
On 2014/10/01 18:41:26, aelias wrote:
> Please add a new test case for the new behavior in this class.

Done.

https://codereview.chromium.org/608223002/diff/180001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/608223002/diff/180001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl_unittest.cc:1438: // After a scroll, a fade
animation should be scheduled about 50ms from now.
On 2014/10/01 18:41:26, aelias wrote:
> "After pinch causes a scrollbar resize,"

Done.

https://codereview.chromium.org/608223002/diff/180001/content/renderer/gpu/re...
File content/renderer/gpu/render_widget_compositor.cc (right):

https://codereview.chromium.org/608223002/diff/180001/content/renderer/gpu/re...
content/renderer/gpu/render_widget_compositor.cc:410:
settings.scrollbar_fade_resize_delay_ms = 2000;
On 2014/10/01 18:41:26, aelias wrote:
> Please set this to 500 since the page-load fade-spam problem doesn't affect
> desktop pinch zoom scrollbars.

Done.

[aelias_OOO_until_Jul13, 2014-10-03 01:57:53]

https://codereview.chromium.org/608223002/diff/200001/cc/layers/layer_impl.cc
File cc/layers/layer_impl.cc (right):

https://codereview.chromium.org/608223002/diff/200001/cc/layers/layer_impl.cc...
cc/layers/layer_impl.cc:1279: bool did_scrollbar_resize = false;
Sorry, one more thing.  I just tried it locally and the fade is delayed after
pinches and after some normal scrolls (when the top controls hide), because they
resize the clip layer.  Please make it only use the long fade when the scroll
layer changes size, not when the clip layer does.  You can do that either by
storing the scroll layer size on the scrollbar_layer, or something more like
your original approach of passing a bool to SetScrollbarPosition.

[MuVen, 2014-10-03 13:01:37]

PTAL.

https://codereview.chromium.org/608223002/diff/200001/cc/layers/layer_impl.cc
File cc/layers/layer_impl.cc (right):

https://codereview.chromium.org/608223002/diff/200001/cc/layers/layer_impl.cc...
cc/layers/layer_impl.cc:1279: bool did_scrollbar_resize = false;
Done.

[aelias_OOO_until_Jul13, 2014-10-03 19:19:00]

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

[aelias_OOO_until_Jul13, 2014-10-03 19:19:01]

lgtm, thanks.  Adding jamesr@ for OWNERS review of new setting in
render_widget_compositor.cc

[MuVen, 2014-10-06 04:41:53]

sataya.m@samsung.com changed reviewers:
- danakj@chromium.org, jdduke@chromium.org

[MuVen, 2014-10-07 02:58:48]

jamesr,

PTAL.

Thanks,

[jamesr, 2014-10-07 04:02:40]

content/renderer/ lgtm

[MuVen, 2014-10-07 04:03:19]

On 2014/10/07 04:02:40, jamesr wrote:
> content/renderer/ lgtm

thanks jamesr & aelias.

[MuVen, 2014-10-07 04:03:26]

The CQ bit was checked by sataya.m@samsung.com

[commit-bot: I haz the power, 2014-10-07 04:05:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/608223002/220001

[commit-bot: I haz the power, 2014-10-07 04:10:57]

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

[commit-bot: I haz the power, 2014-10-07 04:10:59]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/65162)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/70025)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[MuVen, 2014-10-07 04:11:39]

The CQ bit was checked by sataya.m@samsung.com

[commit-bot: I haz the power, 2014-10-07 04:13:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/608223002/220001

[commit-bot: I haz the power, 2014-10-07 04:18:35]

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

[commit-bot: I haz the power, 2014-10-07 04:18:36]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/65165)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/70027)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[MuVen, 2014-10-07 13:20:59]

The CQ bit was checked by sataya.m@samsung.com

[commit-bot: I haz the power, 2014-10-07 13:21:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/608223002/240001

[commit-bot: I haz the power, 2014-10-07 14:29:28]

Committed patchset #6 (id:240001) as 4c6006d240d3e11119143ad9c4cd9004bb96512e

[commit-bot: I haz the power, 2014-10-07 14:30:23]

Patchset 6 (id:??) landed as
https://crrev.com/07f11a816954910f11186cfe461f4bb3cfc5faaf
Cr-Commit-Position: refs/heads/master@{#298471}