cc: Fix scrollbar animation tests

cc/trees/layer_tree_host_impl_unittest.cc

Index: cc/trees/layer_tree_host_impl_unittest.cc
diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc
index 110bee999392e7e66db813e375deca2ff781de9b..591cab21feb7e1ce41d95e418a9e2775348d0b4e 100644
--- a/cc/trees/layer_tree_host_impl_unittest.cc
+++ b/cc/trees/layer_tree_host_impl_unittest.cc
@@ -1563,192 +1563,149 @@ class LayerTreeHostImplOverridePhysicalTime : public LayerTreeHostImpl {
   base::TimeTicks fake_current_physical_time_;
 };
 
-#define SETUP_LAYERS_FOR_SCROLLBAR_ANIMATION_TEST()                            \
-  gfx::Size viewport_size(10, 10);                                             \
-  gfx::Size content_size(100, 100);                                            \
-                                                                               \
-  LayerTreeHostImplOverridePhysicalTime* host_impl_override_time =             \
-      new LayerTreeHostImplOverridePhysicalTime(settings, this, &proxy_,       \
-                                                shared_bitmap_manager_.get(),  \
-                                                &stats_instrumentation_);      \
-  host_impl_ = make_scoped_ptr(host_impl_override_time);                       \
-  host_impl_->InitializeRenderer(CreateOutputSurface());                       \
-  host_impl_->SetViewportSize(viewport_size);                                  \
-                                                                               \
-  scoped_ptr<LayerImpl> root =                                                 \
-      LayerImpl::Create(host_impl_->active_tree(), 1);                         \
-  root->SetBounds(viewport_size);                                              \
-                                                                               \
-  scoped_ptr<LayerImpl> scroll =                                               \
-      LayerImpl::Create(host_impl_->active_tree(), 2);                         \
-  scroll->SetScrollClipLayer(root->id());                                      \
-  scroll->PushScrollOffsetFromMainThread(gfx::ScrollOffset());                 \
-  root->SetBounds(viewport_size);                                              \
-  scroll->SetBounds(content_size);                                             \
-  scroll->SetContentBounds(content_size);                                      \
-  scroll->SetIsContainerForFixedPositionLayers(true);                          \
-                                                                               \
-  scoped_ptr<LayerImpl> contents =                                             \
-      LayerImpl::Create(host_impl_->active_tree(), 3);                         \
-  contents->SetDrawsContent(true);                                             \
-  contents->SetBounds(content_size);                                           \
-  contents->SetContentBounds(content_size);                                    \
-                                                                               \
-  scoped_ptr<SolidColorScrollbarLayerImpl> scrollbar =                         \
-      SolidColorScrollbarLayerImpl::Create(host_impl_->active_tree(), 4,       \
-                                           VERTICAL, 10, 0, false, true);      \
-  EXPECT_FLOAT_EQ(0.f, scrollbar->opacity());                                  \
-                                                                               \
-  scroll->AddChild(contents.Pass());                                           \
-  root->AddChild(scroll.Pass());                                               \
-  root->SetHasRenderSurface(true);                                             \
-  scrollbar->SetScrollLayerAndClipLayerByIds(2, 1);                            \
-  root->AddChild(scrollbar.Pass());                                            \
-                                                                               \
-  host_impl_->active_tree()->SetRootLayer(root.Pass());                        \
-  host_impl_->active_tree()->SetViewportLayersFromIds(Layer::INVALID_ID, 1, 2, \
-                                                      Layer::INVALID_ID);      \
-  host_impl_->active_tree()->DidBecomeActive();                                \
-  DrawFrame();
-
-TEST_F(LayerTreeHostImplTest, ScrollbarLinearFadeScheduling) {
-  LayerTreeSettings settings;
-  settings.scrollbar_animator = LayerTreeSettings::LINEAR_FADE;
-  settings.scrollbar_fade_delay_ms = 20;
-  settings.scrollbar_fade_duration_ms = 20;
-
-  SETUP_LAYERS_FOR_SCROLLBAR_ANIMATION_TEST();
-
-  base::TimeTicks fake_now = gfx::FrameTime::Now();
-
-  EXPECT_FALSE(did_request_animate_);
-  EXPECT_FALSE(did_request_redraw_);
-  EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
-  EXPECT_TRUE(animation_task_.Equals(base::Closure()));
+class LayerTreeHostImplTestScrollbarAnimation : public LayerTreeHostImplTest {
+ protected:
+  void SetupLayers(LayerTreeSettings settings) {
+    gfx::Size viewport_size(10, 10);
+    gfx::Size content_size(100, 100);
+
+    LayerTreeHostImplOverridePhysicalTime* host_impl_override_time =
+        new LayerTreeHostImplOverridePhysicalTime(settings, this, &proxy_,
+                                                  shared_bitmap_manager_.get(),
+                                                  &stats_instrumentation_);
+    host_impl_ = make_scoped_ptr(host_impl_override_time);
+    host_impl_->InitializeRenderer(CreateOutputSurface());
+    host_impl_->SetViewportSize(viewport_size);
 
-  // If no scroll happened during a scroll gesture, it should have no effect.
-  host_impl_->ScrollBegin(gfx::Point(), InputHandler::WHEEL);
-  host_impl_->ScrollEnd();
-  EXPECT_FALSE(did_request_animate_);
-  EXPECT_FALSE(did_request_redraw_);
-  EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
-  EXPECT_TRUE(animation_task_.Equals(base::Closure()));
+    scoped_ptr<LayerImpl> root =
+        LayerImpl::Create(host_impl_->active_tree(), 1);
+    root->SetBounds(viewport_size);
 
-  // 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(0, 5));
-  EXPECT_FALSE(did_request_animate_);
-  EXPECT_TRUE(did_request_redraw_);
-  did_request_redraw_ = false;
-  EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
-  EXPECT_TRUE(animation_task_.Equals(base::Closure()));
+    scoped_ptr<LayerImpl> scroll =
+        LayerImpl::Create(host_impl_->active_tree(), 2);
+    scroll->SetScrollClipLayer(root->id());
+    scroll->PushScrollOffsetFromMainThread(gfx::ScrollOffset());
+    root->SetBounds(viewport_size);
+    scroll->SetBounds(content_size);
+    scroll->SetContentBounds(content_size);
+    scroll->SetIsContainerForFixedPositionLayers(true);
 
-  host_impl_->ScrollEnd();
-  EXPECT_FALSE(did_request_animate_);
-  EXPECT_FALSE(did_request_redraw_);
-  EXPECT_EQ(base::TimeDelta::FromMilliseconds(20), requested_animation_delay_);
-  EXPECT_FALSE(animation_task_.Equals(base::Closure()));
+    scoped_ptr<LayerImpl> contents =
+        LayerImpl::Create(host_impl_->active_tree(), 3);
+    contents->SetDrawsContent(true);
+    contents->SetBounds(content_size);
+    contents->SetContentBounds(content_size);
 
-  fake_now += requested_animation_delay_;
-  requested_animation_delay_ = base::TimeDelta();
-  animation_task_.Run();
-  animation_task_ = base::Closure();
-  EXPECT_TRUE(did_request_animate_);
-  did_request_animate_ = false;
-  EXPECT_FALSE(did_request_redraw_);
+    scoped_ptr<SolidColorScrollbarLayerImpl> scrollbar =
+        SolidColorScrollbarLayerImpl::Create(host_impl_->active_tree(), 4,
+                                             VERTICAL, 10, 0, false, true);
+    EXPECT_FLOAT_EQ(0.f, scrollbar->opacity());
 
-  // After the fade begins, we should start getting redraws instead of a
-  // scheduled animation.
-  host_impl_->Animate(fake_now);
-  EXPECT_TRUE(did_request_animate_);
-  did_request_animate_ = false;
-  EXPECT_TRUE(did_request_redraw_);
-  did_request_redraw_ = false;
-  EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
-  EXPECT_TRUE(animation_task_.Equals(base::Closure()));
+    scroll->AddChild(contents.Pass());
+    root->AddChild(scroll.Pass());
+    root->SetHasRenderSurface(true);
+    scrollbar->SetScrollLayerAndClipLayerByIds(2, 1);
+    root->AddChild(scrollbar.Pass());
 
-  // Setting the scroll offset outside a scroll should also cause the scrollbar
-  // to appear and to schedule a fade.
-  host_impl_->InnerViewportScrollLayer()->PushScrollOffsetFromMainThread(
-      gfx::ScrollOffset(5, 5));
-  EXPECT_FALSE(did_request_animate_);
-  EXPECT_FALSE(did_request_redraw_);
-  EXPECT_EQ(base::TimeDelta::FromMilliseconds(20), requested_animation_delay_);
-  EXPECT_FALSE(animation_task_.Equals(base::Closure()));
-  requested_animation_delay_ = base::TimeDelta();
-  animation_task_ = base::Closure();
-
-  // Unnecessarily Fade animation of solid color scrollbar is not triggered.
-  host_impl_->ScrollBegin(gfx::Point(), InputHandler::WHEEL);
-  host_impl_->ScrollBy(gfx::Point(), gfx::Vector2dF(5, 0));
-  EXPECT_FALSE(did_request_animate_);
-  EXPECT_TRUE(did_request_redraw_);
-  did_request_redraw_ = false;
-  EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
-  EXPECT_TRUE(animation_task_.Equals(base::Closure()));
+    host_impl_->active_tree()->SetRootLayer(root.Pass());
+    host_impl_->active_tree()->SetViewportLayersFromIds(Layer::INVALID_ID, 1, 2,
+                                                        Layer::INVALID_ID);
+    host_impl_->active_tree()->DidBecomeActive();
+    DrawFrame();
+  }
 
-  host_impl_->ScrollEnd();
-  EXPECT_FALSE(did_request_animate_);
-  EXPECT_FALSE(did_request_redraw_);
-  EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
-  EXPECT_TRUE(animation_task_.Equals(base::Closure()));
-}
+  void RunTest(LayerTreeSettings::ScrollbarAnimator animator) {
+    LayerTreeSettings settings;
+    settings.scrollbar_animator = animator;
+    settings.scrollbar_fade_delay_ms = 20;
+    settings.scrollbar_fade_duration_ms = 20;
 
-TEST_F(LayerTreeHostImplTest, ScrollbarFadePinchZoomScrollbars) {
-  LayerTreeSettings settings;
-  settings.scrollbar_animator = LayerTreeSettings::LINEAR_FADE;
-  settings.scrollbar_fade_delay_ms = 20;
-  settings.scrollbar_fade_duration_ms = 20;
-  settings.use_pinch_zoom_scrollbars = true;
+    SetupLayers(settings);
 
-  SETUP_LAYERS_FOR_SCROLLBAR_ANIMATION_TEST();
+    base::TimeTicks fake_now = gfx::FrameTime::Now();
 
-  base::TimeTicks fake_now = gfx::FrameTime::Now();
+    EXPECT_FALSE(did_request_animate_);
+    EXPECT_FALSE(did_request_redraw_);
+    EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
+    EXPECT_TRUE(animation_task_.Equals(base::Closure()));
 
-  host_impl_->active_tree()->PushPageScaleFromMainThread(1.f, 1.f, 4.f);
+    // If no scroll happened during a scroll gesture, it should have no effect.
+    host_impl_->ScrollBegin(gfx::Point(), InputHandler::WHEEL);
+    host_impl_->ScrollEnd();
+    EXPECT_FALSE(did_request_animate_);
+    EXPECT_FALSE(did_request_redraw_);
+    EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
+    EXPECT_TRUE(animation_task_.Equals(base::Closure()));
 
-  EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
-  EXPECT_FALSE(did_request_animate_);
+    // After a scroll, a fade animation should be scheduled about 20ms from now.

[bokan, 2015/03/02 22:49:06]

Nit: this is no longer specifically for fade, please update the comments.

[sunnyps, 2015/03/03 20:47:09]

Done.

+    host_impl_->ScrollBegin(gfx::Point(), InputHandler::WHEEL);
+    host_impl_->ScrollBy(gfx::Point(), gfx::Vector2dF(0, 5));
+    EXPECT_FALSE(did_request_animate_);
+    EXPECT_TRUE(did_request_redraw_);
+    did_request_redraw_ = false;
+    EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
+    EXPECT_TRUE(animation_task_.Equals(base::Closure()));
 
-  // If no scroll happened during a scroll gesture, it should have no effect.
-  host_impl_->ScrollBegin(gfx::Point(), InputHandler::WHEEL);
-  host_impl_->ScrollEnd();
-  EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
-  EXPECT_FALSE(did_request_animate_);
-  EXPECT_TRUE(animation_task_.Equals(base::Closure()));
+    host_impl_->ScrollEnd();
+    EXPECT_FALSE(did_request_animate_);
+    EXPECT_FALSE(did_request_redraw_);
+    EXPECT_EQ(base::TimeDelta::FromMilliseconds(20),
+              requested_animation_delay_);
+    EXPECT_FALSE(animation_task_.Equals(base::Closure()));
+
+    fake_now += requested_animation_delay_;
+    requested_animation_delay_ = base::TimeDelta();
+    animation_task_.Run();
+    animation_task_ = base::Closure();
+    EXPECT_TRUE(did_request_animate_);
+    did_request_animate_ = false;
+    EXPECT_FALSE(did_request_redraw_);
 
-  // After a scroll, no fade animation should be scheduled.
-  host_impl_->ScrollBegin(gfx::Point(), InputHandler::WHEEL);
-  host_impl_->ScrollBy(gfx::Point(), gfx::Vector2dF(5, 0));
-  host_impl_->ScrollEnd();
-  did_request_redraw_ = false;
-  EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
-  EXPECT_FALSE(did_request_animate_);
-  requested_animation_delay_ = base::TimeDelta();
+    // After the fade begins, we should start getting redraws instead of a
+    // scheduled animation.
+    host_impl_->Animate(fake_now);
+    EXPECT_TRUE(did_request_animate_);
+    did_request_animate_ = false;
+    EXPECT_TRUE(did_request_redraw_);
+    did_request_redraw_ = false;
+    EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
+    EXPECT_TRUE(animation_task_.Equals(base::Closure()));
 
-  // We should not see any draw requests.
-  fake_now += base::TimeDelta::FromMilliseconds(25);
-  EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
-  EXPECT_FALSE(did_request_animate_);
+    // Setting the scroll offset outside a scroll should also cause the
+    // scrollbar to appear and to schedule a fade.
+    host_impl_->InnerViewportScrollLayer()->PushScrollOffsetFromMainThread(
+        gfx::ScrollOffset(5, 5));
+    EXPECT_FALSE(did_request_animate_);
+    EXPECT_FALSE(did_request_redraw_);
+    EXPECT_EQ(base::TimeDelta::FromMilliseconds(20),
+              requested_animation_delay_);
+    EXPECT_FALSE(animation_task_.Equals(base::Closure()));
+    requested_animation_delay_ = base::TimeDelta();
+    animation_task_ = base::Closure();
+
+    // Unnecessarily Fade animation of solid color scrollbar is not triggered.

[ajuma, 2015/02/26 15:30:45]

While you're moving this: s/Unnecessarily/Unnecessary

[sunnyps, 2015/03/02 19:19:07]

Acknowledged.

[sunnyps, 2015/03/03 20:47:09]

Done.

+    host_impl_->ScrollBegin(gfx::Point(), InputHandler::WHEEL);
+    host_impl_->ScrollBy(gfx::Point(), gfx::Vector2dF(5, 0));
+    EXPECT_FALSE(did_request_animate_);
+    EXPECT_TRUE(did_request_redraw_);
+    did_request_redraw_ = false;
+    EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
+    EXPECT_TRUE(animation_task_.Equals(base::Closure()));
 
-  // Make page scale > min so that subsequent scrolls will trigger fades.
-  host_impl_->SetPageScaleOnActiveTree(1.1f);

[ajuma, 2015/02/26 15:30:45]

Does this line have no effect on behavior? (As I understand it, the
justification for removing this test is that the use_pinch_zoom_scrollbars
setting isn't read anywhere, so this is really testing the same thing as
ScrollbarLinearFadeScheduling, but that's only true if changing page scale
doesn't change expected scrollbar animation behavior.)

[sunnyps, 2015/03/02 19:19:07]

It seems the scrollbar fade animation is triggered by the
SetPageScaleOnActiveTree call rather than being triggered by the
ScrollBegin/ScrollBy/ScrollEnd calls below as the test would have you believe.

[bokan, 2015/03/02 22:49:06]

That's not true, ScrollBy will activate the fade animation, but this used to be
gated based on the page scale. If page scale was at the minimum then we wouldn't
trigger the animation. This was to prevent overlay scrollbars from showing up
over top of page scrollbars when they're unneeded. Now we run the animation
regardless of page scale but hide the subtree when page scale is near the
minimum. See https://codereview.chromium.org/885063002/ for more details.

That said, we use the same scrollbar code on Android and desktop now so I think
this is indeed testing the same thing as ScrollbarLinearFadeScheduling

+    host_impl_->ScrollEnd();
+    EXPECT_FALSE(did_request_animate_);
+    EXPECT_FALSE(did_request_redraw_);
+    EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
+    EXPECT_TRUE(animation_task_.Equals(base::Closure()));
+  }
+};
 
-  // 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;
-  EXPECT_LT(base::TimeDelta::FromMilliseconds(19), requested_animation_delay_);
-  EXPECT_FALSE(did_request_animate_);
-  requested_animation_delay_ = base::TimeDelta();
-  animation_task_.Run();
+TEST_F(LayerTreeHostImplTestScrollbarAnimation, LinearFade) {
+  RunTest(LayerTreeSettings::LINEAR_FADE);
+}
 
-  // After the fade begins, we should start getting redraws instead of a
-  // scheduled animation.
-  fake_now += base::TimeDelta::FromMilliseconds(25);
-  host_impl_->Animate(fake_now);
-  EXPECT_TRUE(did_request_animate_);
+TEST_F(LayerTreeHostImplTestScrollbarAnimation, Thinning) {
+  RunTest(LayerTreeSettings::THINNING);
 }
 
 void LayerTreeHostImplTest::SetupMouseMoveAtWithDeviceScale(