Suppress scrollbar animation if the scroll gesture does not scroll

Currently, the scrollbar layer animation is triggered at the end of every scroll
gesture. However, as not every scroll gesture actually scrolls a layer, this may
trigger extraneous animations, e.g., when the user overscrolls, or presses and
lifts their finger without scrolling.  Only trigger the scrollbar animation at
the end of a scroll gesture if the gesture caused the layer to scroll.

BUG=285771

Review URL: https://chromiumcodereview.appspot.com/23978008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@221801 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 123 insertions, 26 deletions

diff --git a/cc/animation/scrollbar_animation_controller.h b/cc/animation/scrollbar_animation_controller.h
index 55d2945..94c953e 100644
--- a/cc/animation/scrollbar_animation_controller.h
+++ b/cc/animation/scrollbar_animation_controller.h
@@ -18,14 +18,13 @@ class CC_EXPORT ScrollbarAnimationController {
  public:
   virtual ~ScrollbarAnimationController() {}
 
-  virtual bool IsScrollGestureInProgress() const = 0;
   virtual bool IsAnimating() const = 0;
   virtual base::TimeDelta DelayBeforeStart(base::TimeTicks now) const = 0;
 
   virtual bool Animate(base::TimeTicks now) = 0;
   virtual void DidScrollGestureBegin() = 0;
   virtual void DidScrollGestureEnd(base::TimeTicks now) = 0;
-  virtual void DidProgrammaticallyUpdateScroll(base::TimeTicks now) = 0;
+  virtual void DidScrollUpdate(base::TimeTicks now) = 0;
 };
 
 }  // namespace cc
diff --git a/cc/animation/scrollbar_animation_controller_linear_fade.cc b/cc/animation/scrollbar_animation_controller_linear_fade.cc
index 5382712..9f23802 100644
--- a/cc/animation/scrollbar_animation_controller_linear_fade.cc
+++ b/cc/animation/scrollbar_animation_controller_linear_fade.cc
@@ -24,16 +24,13 @@ ScrollbarAnimationControllerLinearFade::ScrollbarAnimationControllerLinearFade(
     : ScrollbarAnimationController(),
       scroll_layer_(scroll_layer),
       scroll_gesture_in_progress_(false),
+      scroll_gesture_has_scrolled_(false),
       fadeout_delay_(fadeout_delay),
       fadeout_length_(fadeout_length) {}
 
 ScrollbarAnimationControllerLinearFade::
     ~ScrollbarAnimationControllerLinearFade() {}
 
-bool ScrollbarAnimationControllerLinearFade::IsScrollGestureInProgress() const {
-  return scroll_gesture_in_progress_;
-}
-
 bool ScrollbarAnimationControllerLinearFade::IsAnimating() const {
   return !last_awaken_time_.is_null();
 }
@@ -54,28 +51,35 @@ bool ScrollbarAnimationControllerLinearFade::Animate(base::TimeTicks now) {
 }
 
 void ScrollbarAnimationControllerLinearFade::DidScrollGestureBegin() {
-  scroll_layer_->SetScrollbarOpacity(1.0f);
   scroll_gesture_in_progress_ = true;
-  last_awaken_time_ = base::TimeTicks();
+  scroll_gesture_has_scrolled_ = false;
 }
 
 void ScrollbarAnimationControllerLinearFade::DidScrollGestureEnd(
     base::TimeTicks now) {
+  // The animation should not be triggered if no scrolling has occurred.
+  if (scroll_gesture_has_scrolled_)
+    last_awaken_time_ = now;
+  scroll_gesture_has_scrolled_ = false;
   scroll_gesture_in_progress_ = false;
-  last_awaken_time_ = now;
 }
 
-void ScrollbarAnimationControllerLinearFade::DidProgrammaticallyUpdateScroll(
+void ScrollbarAnimationControllerLinearFade::DidScrollUpdate(
     base::TimeTicks now) {
-  // Don't set scroll_gesture_in_progress_ as this scroll is not from a gesture
-  // and we won't receive ScrollEnd.
   scroll_layer_->SetScrollbarOpacity(1.0f);
-  last_awaken_time_ = now;
+  // The animation should only be activated if the scroll updated occurred
+  // programatically, outside the scope of a scroll gesture.
+  if (scroll_gesture_in_progress_) {
+    last_awaken_time_ = base::TimeTicks();
+    scroll_gesture_has_scrolled_ = true;
+  } else {
+    last_awaken_time_ = now;
+  }
 }
 
 float ScrollbarAnimationControllerLinearFade::OpacityAtTime(
     base::TimeTicks now) {
-  if (scroll_gesture_in_progress_)
+  if (scroll_gesture_has_scrolled_)
     return 1.0f;
 
   if (last_awaken_time_.is_null())
diff --git a/cc/animation/scrollbar_animation_controller_linear_fade.h b/cc/animation/scrollbar_animation_controller_linear_fade.h
index 9ecb3c1..74c411f 100644
--- a/cc/animation/scrollbar_animation_controller_linear_fade.h
+++ b/cc/animation/scrollbar_animation_controller_linear_fade.h
@@ -23,14 +23,13 @@ class CC_EXPORT ScrollbarAnimationControllerLinearFade
   virtual ~ScrollbarAnimationControllerLinearFade();
 
   // ScrollbarAnimationController overrides.
-  virtual bool IsScrollGestureInProgress() const OVERRIDE;
   virtual bool IsAnimating() const OVERRIDE;
   virtual base::TimeDelta DelayBeforeStart(base::TimeTicks now) const OVERRIDE;
 
   virtual bool Animate(base::TimeTicks now) OVERRIDE;
   virtual void DidScrollGestureBegin() OVERRIDE;
   virtual void DidScrollGestureEnd(base::TimeTicks now) OVERRIDE;
-  virtual void DidProgrammaticallyUpdateScroll(base::TimeTicks now) OVERRIDE;
+  virtual void DidScrollUpdate(base::TimeTicks now) OVERRIDE;
 
  protected:
   ScrollbarAnimationControllerLinearFade(LayerImpl* scroll_layer,
@@ -44,6 +43,7 @@ class CC_EXPORT ScrollbarAnimationControllerLinearFade
 
   base::TimeTicks last_awaken_time_;
   bool scroll_gesture_in_progress_;
+  bool scroll_gesture_has_scrolled_;
 
   base::TimeDelta fadeout_delay_;
   base::TimeDelta fadeout_length_;
diff --git a/cc/animation/scrollbar_animation_controller_linear_fade_unittest.cc b/cc/animation/scrollbar_animation_controller_linear_fade_unittest.cc
index 4a11ba6..4e0dab9 100644
--- a/cc/animation/scrollbar_animation_controller_linear_fade_unittest.cc
+++ b/cc/animation/scrollbar_animation_controller_linear_fade_unittest.cc
@@ -44,20 +44,43 @@ TEST_F(ScrollbarAnimationControllerLinearFadeTest, HiddenInBegin) {
   EXPECT_FLOAT_EQ(0.0f, scrollbar_layer_->opacity());
 }
 
-TEST_F(ScrollbarAnimationControllerLinearFadeTest, AwakenByScrollGesture) {
+TEST_F(ScrollbarAnimationControllerLinearFadeTest,
+       HiddenAfterNonScrollingGesture) {
+  scrollbar_controller_->DidScrollGestureBegin();
+  EXPECT_FALSE(scrollbar_controller_->IsAnimating());
+  EXPECT_FALSE(scrollbar_controller_->Animate(base::TimeTicks()));
+  EXPECT_FLOAT_EQ(0.0f, scrollbar_layer_->opacity());
+
+  base::TimeTicks time;
+  time += base::TimeDelta::FromSeconds(100);
+  EXPECT_FALSE(scrollbar_controller_->Animate(time));
+  EXPECT_FLOAT_EQ(0.0f, scrollbar_layer_->opacity());
+  scrollbar_controller_->DidScrollGestureEnd(time);
+
+  time += base::TimeDelta::FromSeconds(100);
+  EXPECT_FALSE(scrollbar_controller_->IsAnimating());
+  EXPECT_FALSE(scrollbar_controller_->Animate(time));
+  EXPECT_FLOAT_EQ(0.0f, scrollbar_layer_->opacity());
+}
+
+TEST_F(ScrollbarAnimationControllerLinearFadeTest, AwakenByScrollingGesture) {
   base::TimeTicks time;
   time += base::TimeDelta::FromSeconds(1);
   scrollbar_controller_->DidScrollGestureBegin();
-  EXPECT_TRUE(scrollbar_controller_->IsScrollGestureInProgress());
+  scrollbar_controller_->Animate(time);
+  EXPECT_FALSE(scrollbar_controller_->IsAnimating());
+  EXPECT_FLOAT_EQ(0.0f, scrollbar_layer_->opacity());
+
+  scrollbar_controller_->DidScrollUpdate(time);
   EXPECT_FALSE(scrollbar_controller_->IsAnimating());
   EXPECT_FLOAT_EQ(1.0f, scrollbar_layer_->opacity());
 
   time += base::TimeDelta::FromSeconds(100);
   scrollbar_controller_->Animate(time);
+  EXPECT_FALSE(scrollbar_controller_->IsAnimating());
   EXPECT_FLOAT_EQ(1.0f, scrollbar_layer_->opacity());
   scrollbar_controller_->DidScrollGestureEnd(time);
 
-  EXPECT_FALSE(scrollbar_controller_->IsScrollGestureInProgress());
   EXPECT_TRUE(scrollbar_controller_->IsAnimating());
   EXPECT_EQ(2, scrollbar_controller_->DelayBeforeStart(time).InSeconds());
 
@@ -80,6 +103,7 @@ TEST_F(ScrollbarAnimationControllerLinearFadeTest, AwakenByScrollGesture) {
   time += base::TimeDelta::FromSeconds(1);
 
   scrollbar_controller_->DidScrollGestureBegin();
+  scrollbar_controller_->DidScrollUpdate(time);
   scrollbar_controller_->DidScrollGestureEnd(time);
 
   time += base::TimeDelta::FromSeconds(1);
@@ -106,8 +130,7 @@ TEST_F(ScrollbarAnimationControllerLinearFadeTest, AwakenByScrollGesture) {
 TEST_F(ScrollbarAnimationControllerLinearFadeTest, AwakenByProgrammaticScroll) {
   base::TimeTicks time;
   time += base::TimeDelta::FromSeconds(1);
-  scrollbar_controller_->DidProgrammaticallyUpdateScroll(time);
-  EXPECT_FALSE(scrollbar_controller_->IsScrollGestureInProgress());
+  scrollbar_controller_->DidScrollUpdate(time);
   EXPECT_TRUE(scrollbar_controller_->IsAnimating());
   EXPECT_EQ(2, scrollbar_controller_->DelayBeforeStart(time).InSeconds());
   scrollbar_controller_->Animate(time);
@@ -116,7 +139,7 @@ TEST_F(ScrollbarAnimationControllerLinearFadeTest, AwakenByProgrammaticScroll) {
   time += base::TimeDelta::FromSeconds(1);
   scrollbar_controller_->Animate(time);
   EXPECT_FLOAT_EQ(1.0f, scrollbar_layer_->opacity());
-  scrollbar_controller_->DidProgrammaticallyUpdateScroll(time);
+  scrollbar_controller_->DidScrollUpdate(time);
 
   time += base::TimeDelta::FromSeconds(1);
   scrollbar_controller_->Animate(time);
@@ -135,7 +158,7 @@ TEST_F(ScrollbarAnimationControllerLinearFadeTest, AwakenByProgrammaticScroll) {
   EXPECT_FLOAT_EQ(1.0f / 3.0f, scrollbar_layer_->opacity());
 
   time += base::TimeDelta::FromSeconds(1);
-  scrollbar_controller_->DidProgrammaticallyUpdateScroll(time);
+  scrollbar_controller_->DidScrollUpdate(time);
   time += base::TimeDelta::FromSeconds(1);
   scrollbar_controller_->Animate(time);
   EXPECT_FLOAT_EQ(1.0f, scrollbar_layer_->opacity());
@@ -157,5 +180,67 @@ TEST_F(ScrollbarAnimationControllerLinearFadeTest, AwakenByProgrammaticScroll) {
   EXPECT_FLOAT_EQ(0.0f, scrollbar_layer_->opacity());
 }
 
+TEST_F(ScrollbarAnimationControllerLinearFadeTest,
+       AnimationPreservedByNonScrollingGesture) {
+  base::TimeTicks time;
+  time += base::TimeDelta::FromSeconds(1);
+  scrollbar_controller_->DidScrollUpdate(time);
+  EXPECT_TRUE(scrollbar_controller_->IsAnimating());
+  scrollbar_controller_->Animate(time);
+  EXPECT_FLOAT_EQ(1.0f, scrollbar_layer_->opacity());
+
+  time += base::TimeDelta::FromSeconds(3);
+  scrollbar_controller_->Animate(time);
+  EXPECT_FLOAT_EQ(2.0f / 3.0f, scrollbar_layer_->opacity());
+
+  scrollbar_controller_->DidScrollGestureBegin();
+  EXPECT_TRUE(scrollbar_controller_->IsAnimating());
+  EXPECT_FLOAT_EQ(2.0f / 3.0f, scrollbar_layer_->opacity());
+
+  time += base::TimeDelta::FromSeconds(1);
+  scrollbar_controller_->Animate(time);
+  EXPECT_FLOAT_EQ(1.0f / 3.0f, scrollbar_layer_->opacity());
+
+  scrollbar_controller_->DidScrollGestureEnd(time);
+  EXPECT_TRUE(scrollbar_controller_->IsAnimating());
+  EXPECT_FLOAT_EQ(1.0f / 3.0f, scrollbar_layer_->opacity());
+
+  time += base::TimeDelta::FromSeconds(1);
+  EXPECT_FALSE(scrollbar_controller_->Animate(time));
+  EXPECT_FLOAT_EQ(0.0f, scrollbar_layer_->opacity());
+}
+
+TEST_F(ScrollbarAnimationControllerLinearFadeTest,
+       AnimationOverriddenByScrollingGesture) {
+  base::TimeTicks time;
+  time += base::TimeDelta::FromSeconds(1);
+  scrollbar_controller_->DidScrollUpdate(time);
+  EXPECT_TRUE(scrollbar_controller_->IsAnimating());
+  scrollbar_controller_->Animate(time);
+  EXPECT_FLOAT_EQ(1.0f, scrollbar_layer_->opacity());
+
+  time += base::TimeDelta::FromSeconds(3);
+  scrollbar_controller_->Animate(time);
+  EXPECT_FLOAT_EQ(2.0f / 3.0f, scrollbar_layer_->opacity());
+
+  scrollbar_controller_->DidScrollGestureBegin();
+  EXPECT_TRUE(scrollbar_controller_->IsAnimating());
+  EXPECT_FLOAT_EQ(2.0f / 3.0f, scrollbar_layer_->opacity());
+
+  time += base::TimeDelta::FromSeconds(1);
+  scrollbar_controller_->Animate(time);
+  EXPECT_FLOAT_EQ(1.0f / 3.0f, scrollbar_layer_->opacity());
+
+  time += base::TimeDelta::FromSeconds(1);
+  scrollbar_controller_->DidScrollUpdate(time);
+  EXPECT_FALSE(scrollbar_controller_->IsAnimating());
+  EXPECT_FLOAT_EQ(1, scrollbar_layer_->opacity());
+
+  time += base::TimeDelta::FromSeconds(1);
+  scrollbar_controller_->DidScrollGestureEnd(time);
+  EXPECT_TRUE(scrollbar_controller_->IsAnimating());
+  EXPECT_FLOAT_EQ(1, scrollbar_layer_->opacity());
+}
+
 }  // namespace
 }  // namespace cc
diff --git a/cc/layers/layer_impl.cc b/cc/layers/layer_impl.cc
index aee85cf..615fcf2 100644
--- a/cc/layers/layer_impl.cc
+++ b/cc/layers/layer_impl.cc
@@ -975,9 +975,8 @@ void LayerImpl::UpdateScrollbarPositions() {
     return;
   last_scroll_offset_ = current_offset;
 
-  if (scrollbar_animation_controller_ &&
-      !scrollbar_animation_controller_->IsScrollGestureInProgress()) {
-    scrollbar_animation_controller_->DidProgrammaticallyUpdateScroll(
+  if (scrollbar_animation_controller_) {
+    scrollbar_animation_controller_->DidScrollUpdate(
         layer_tree_impl_->CurrentPhysicalTimeTicks());
   }
 
diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc
index e00b9a36..34a9686 100644
--- a/cc/trees/layer_tree_host_impl_unittest.cc
+++ b/cc/trees/layer_tree_host_impl_unittest.cc
@@ -1144,9 +1144,19 @@ TEST_F(LayerTreeHostImplTest, ScrollbarLinearFadeScheduling) {
   EXPECT_EQ(base::TimeDelta(), requested_scrollbar_animation_delay_);
   EXPECT_FALSE(did_request_redraw_);
 
+  // If no scroll happened during a scroll gesture, StartScrollbarAnimation
+  // should have no effect.
+  host_impl_->ScrollBegin(gfx::Point(), InputHandler::Wheel);
+  host_impl_->ScrollEnd();
+  host_impl_->StartScrollbarAnimation();
+  EXPECT_EQ(base::TimeDelta(), requested_scrollbar_animation_delay_);
+  EXPECT_FALSE(did_request_redraw_);
+
   // After a scroll, a fade animation should be scheduled about 20ms from now.
   host_impl_->ScrollBegin(gfx::Point(), InputHandler::Wheel);
+  host_impl_->ScrollBy(gfx::Point(), gfx::Vector2dF(5, 0));
   host_impl_->ScrollEnd();
+  did_request_redraw_ = false;
   host_impl_->StartScrollbarAnimation();
   EXPECT_LT(base::TimeDelta::FromMilliseconds(19),
             requested_scrollbar_animation_delay_);