Call SAC::DidScrollUpdate only for compositor-triggered scrolls.

cc/trees/layer_tree_host_impl_unittest.cc

 // Copyright 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/trees/layer_tree_host_impl.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <cmath>
@@ skipping 2901 matching lines @@
     std::unique_ptr<SolidColorScrollbarLayerImpl> scrollbar =
         SolidColorScrollbarLayerImpl::Create(host_impl_->active_tree(), 400,
                                              VERTICAL, 10, 0, false, true);
     scrollbar->test_properties()->opacity = 0.f;
     EXPECT_FLOAT_EQ(0.f, scrollbar->test_properties()->opacity);
 
     LayerImpl* scroll = host_impl_->active_tree()->OuterViewportScrollLayer();
     LayerImpl* root = host_impl_->active_tree()->InnerViewportContainerLayer();
     scrollbar->SetScrollElementId(scroll->element_id());
     root->test_properties()->AddChild(std::move(scrollbar));
+    scroll->set_needs_show_scrollbars(true);
     host_impl_->active_tree()->BuildPropertyTreesForTesting();
     host_impl_->active_tree()->DidBecomeActive();
+    host_impl_->active_tree()->ShowScrollbars();
     DrawFrame();
 
     // SetScrollElementId will initialize the scrollbar which will cause it to
     // show and request a redraw.
     did_request_redraw_ = false;
   }
 
   void RunTest(LayerTreeSettings::ScrollbarAnimator animator) {
     LayerTreeSettings settings = DefaultSettings();
     settings.scrollbar_animator = animator;
@@ skipping 115 matching lines @@
       host_impl_->Animate();
       EXPECT_TRUE(did_request_next_frame_);
       did_request_next_frame_ = false;
       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_->DidFinishImplFrame();
     }
 
-    // Setting the scroll offset outside a scroll should also cause the
-    // scrollbar to appear and to schedule a scrollbar animation.
+    // Setting the scroll offset outside a scroll should not cause the
+    // scrollbar to appear or schedule a scrollbar animation.
     if (host_impl_->active_tree()
             ->property_trees()
             ->scroll_tree.UpdateScrollOffsetBaseForTesting(
                 host_impl_->InnerViewportScrollLayer()->id(),
                 gfx::ScrollOffset(5, 5)))
       host_impl_->active_tree()->DidUpdateScrollOffset(
           host_impl_->InnerViewportScrollLayer()->id());
     EXPECT_FALSE(did_request_next_frame_);
     EXPECT_FALSE(did_request_redraw_);
-    if (expecting_animations) {
-      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();
-    } else {
-      EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
-      EXPECT_TRUE(animation_task_.Equals(base::Closure()));
-    }
-
-    if (expecting_animations) {
-      // Scrolling should have stopped the animation, so we should not be
-      // getting redraws.
-      begin_frame_args =
-          CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 5, fake_now);
-      host_impl_->WillBeginImplFrame(begin_frame_args);
-      host_impl_->Animate();
-      EXPECT_FALSE(did_request_next_frame_);
-      did_request_next_frame_ = false;
-      EXPECT_FALSE(did_request_redraw_);
-      did_request_redraw_ = false;
-      host_impl_->DidFinishImplFrame();
-    }
-
-    // For Andrdoid, scrollbar animation is not triggered unnecessarily.
-    // For Aura Overlay Scrollbar, scrollbar appears even if scroll offset did
-    // not change.
-    host_impl_->ScrollBegin(BeginState(gfx::Point()).get(),
-                            InputHandler::WHEEL);
-    host_impl_->ScrollBy(UpdateState(gfx::Point(), gfx::Vector2dF(5, 0)).get());
-    EXPECT_FALSE(did_request_next_frame_);
-    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_->ScrollEnd(EndState().get());
-    EXPECT_FALSE(did_request_next_frame_);
-    EXPECT_FALSE(did_request_redraw_);
-    if (animator == LayerTreeSettings::AURA_OVERLAY) {
-      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();
-    } else {
-      EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
-      EXPECT_TRUE(animation_task_.Equals(base::Closure()));
-    }
-
     // Changing page scale triggers scrollbar animation.
     host_impl_->active_tree()->PushPageScaleFromMainThread(1.f, 1.f, 4.f);
     host_impl_->active_tree()->SetPageScaleOnActiveTree(1.1f);
     EXPECT_FALSE(did_request_next_frame_);
     EXPECT_FALSE(did_request_redraw_);
     if (expecting_animations) {
       EXPECT_EQ(base::TimeDelta::FromMilliseconds(20),
                 requested_animation_delay_);
       EXPECT_FALSE(animation_task_.Equals(base::Closure()));
       requested_animation_delay_ = base::TimeDelta();
@@ skipping 129 matching lines @@
   std::unique_ptr<SolidColorScrollbarLayerImpl> scrollbar =
       SolidColorScrollbarLayerImpl::Create(host_impl_->pending_tree(), 400,
                                            VERTICAL, 10, 0, false, true);
   scrollbar->test_properties()->opacity = 0.f;
   LayerImpl* scroll = host_impl_->pending_tree()->OuterViewportScrollLayer();
   LayerImpl* container =
       host_impl_->pending_tree()->InnerViewportContainerLayer();
   scrollbar->SetScrollElementId(scroll->element_id());
   container->test_properties()->AddChild(std::move(scrollbar));
   host_impl_->pending_tree()->PushPageScaleFromMainThread(1.f, 1.f, 1.f);
+  scroll->set_needs_show_scrollbars(true);
   host_impl_->pending_tree()->BuildPropertyTreesForTesting();
   host_impl_->ActivateSyncTree();
 
   ScrollbarAnimationController* scrollbar_controller =
       host_impl_->ScrollbarAnimationControllerForElementId(
           scroll->element_id());
 
   // Scrollbars will flash shown but we should have a fade out animation
   // queued. Run it and fade out the scrollbars.
   {
@@ skipping 120 matching lines @@
                          root_scroll->element_id()));
   vert_1_scrollbar->SetScrollElementId(root_scroll->element_id());
   EXPECT_EQ(1ul, host_impl_->ScrollbarsFor(root_scroll->element_id()).size());
   EXPECT_TRUE(host_impl_->ScrollbarAnimationControllerForElementId(
       root_scroll->element_id()));
   horiz_1_scrollbar->SetScrollElementId(root_scroll->element_id());
   EXPECT_EQ(2ul, host_impl_->ScrollbarsFor(root_scroll->element_id()).size());
   EXPECT_TRUE(host_impl_->ScrollbarAnimationControllerForElementId(
       root_scroll->element_id()));
 
-  // Changing one of the viewport layers should result in a scrollbar animation
-  // update.
+  // Scrolling the viewport should result in a scrollbar animation update.
   animation_task_ = base::Closure();
-  host_impl_->active_tree()->InnerViewportContainerLayer()->SetBoundsDelta(
-      gfx::Vector2dF(10, 10));
-  EXPECT_FALSE(animation_task_.Equals(base::Closure()));
-  animation_task_ = base::Closure();
-  host_impl_->active_tree()->OuterViewportScrollLayer()->SetCurrentScrollOffset(
-      gfx::ScrollOffset(10, 10));
-  EXPECT_FALSE(animation_task_.Equals(base::Closure()));
-  animation_task_ = base::Closure();
-  host_impl_->active_tree()->InnerViewportScrollLayer()->SetCurrentScrollOffset(
-      gfx::ScrollOffset(10, 10));
+  host_impl_->ScrollBegin(BeginState(gfx::Point()).get(), InputHandler::WHEEL);
+  host_impl_->ScrollBy(UpdateState(gfx::Point(), gfx::Vector2d(10, 10)).get());
+  host_impl_->ScrollEnd(EndState().get());
   EXPECT_FALSE(animation_task_.Equals(base::Closure()));
   animation_task_ = base::Closure();
 
   // Check scrollbar registration on a sublayer.
   child->SetScrollClipLayer(child_clip->id());
   child->SetElementId(LayerIdToElementIdForTesting(child->id()));
   ElementId child_scroll_element_id = child->element_id();
   child_clip->test_properties()->AddChild(std::move(child));
   root_scroll->test_properties()->AddChild(std::move(child_clip));
   EXPECT_EQ(0ul, host_impl_->ScrollbarsFor(child_scroll_element_id).size());
   EXPECT_EQ(nullptr, host_impl_->ScrollbarAnimationControllerForElementId(
                          child_scroll_element_id));
   vert_2_scrollbar->SetScrollElementId(child_scroll_element_id);
   EXPECT_EQ(1ul, host_impl_->ScrollbarsFor(child_scroll_element_id).size());
   EXPECT_TRUE(host_impl_->ScrollbarAnimationControllerForElementId(
       child_scroll_element_id));
   horiz_2_scrollbar->SetScrollElementId(child_scroll_element_id);
   EXPECT_EQ(2ul, host_impl_->ScrollbarsFor(child_scroll_element_id).size());
   EXPECT_TRUE(host_impl_->ScrollbarAnimationControllerForElementId(
       child_scroll_element_id));
 
   // Changing one of the child layers should result in a scrollbar animation
   // update.
   animation_task_ = base::Closure();
   child_clip_ptr->SetBounds(gfx::Size(200, 200));
-  EXPECT_FALSE(animation_task_.Equals(base::Closure()));
-  animation_task_ = base::Closure();
-  child_ptr->SetCurrentScrollOffset(gfx::ScrollOffset(10, 10));
+  child_ptr->set_needs_show_scrollbars(true);
+  host_impl_->active_tree()->ShowScrollbars();
   EXPECT_FALSE(animation_task_.Equals(base::Closure()));
   animation_task_ = base::Closure();
 
   // Check scrollbar unregistration.
   vert_1_scrollbar.reset();
   EXPECT_EQ(1ul, host_impl_->ScrollbarsFor(root_scroll->element_id()).size());
   EXPECT_TRUE(host_impl_->ScrollbarAnimationControllerForElementId(
       root_scroll->element_id()));
   horiz_1_scrollbar.reset();
   EXPECT_EQ(0ul, host_impl_->ScrollbarsFor(root_scroll->element_id()).size());
@@ skipping 8801 matching lines @@
   host_impl_->MouseMoveAt(gfx::Point(10, 150));
   EXPECT_TRUE(scrollbar_1_animation_controller->MouseIsNearScrollbar(VERTICAL));
   EXPECT_TRUE(scrollbar_1_animation_controller->MouseIsOverScrollbar(VERTICAL));
   EXPECT_FALSE(
       scrollbar_2_animation_controller->MouseIsNearScrollbar(VERTICAL));
   EXPECT_FALSE(
       scrollbar_2_animation_controller->MouseIsOverScrollbar(VERTICAL));
 
   // Capture scrollbar_1, then move mouse to scrollbar_2's layer, should post an
   // event to fade out scrollbar_1.
+  scrollbar_1_animation_controller->DidScrollUpdate();

[bokan, 2017/04/25 22:56:48]

Do you know why this wasn't previously needed?

[skobes, 2017/04/27 00:46:31]

The test was counting on the scrollbar to be made initially visible through
LayerTreeImpl::RegisterScrollbar during the call to
scrollbar_1->SetScrollElementId.

RegisterScrollbar no longer triggers SAC::DidScrollUpdate, so we have to call it
manually here.

(Note that we still want scrollbars to be initially visible, but we can't
enforce this through RegisterScrollbar anymore, since it may occur before the
scroll bounds are pushed, in which case SAC::DidScrollUpdate would have no
effect due to SAC::ApplyOpacityToScrollbars setting effective_opacity to 0. 
Instead we rely on showing from main when the first layout calls
PLSA::VisibleSizeChanged.)

[bokan, 2017/04/27 19:38:58]

Got it, thanks for the explanation.

   animation_task_ = base::Closure();
 
   host_impl_->MouseDown();
   host_impl_->MouseMoveAt(gfx::Point(100, 150));
   host_impl_->MouseUp();
 
   EXPECT_FALSE(animation_task_.Equals(base::Closure()));
 
   // Near scrollbar_1, then mouse down and up, should not post an event to fade
   // out scrollbar_1.
@@ skipping 93 matching lines @@
     else
       EXPECT_FALSE(tile->HasRasterTask());
   }
   Region expected_invalidation(
       raster_source->GetRectForImage(checkerable_image->uniqueID()));
   EXPECT_EQ(expected_invalidation, *(root->GetPendingInvalidation()));
 }
 
 }  // namespace
 }  // namespace cc