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

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

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())

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_;

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

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());
   }
 

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_);