[Reland] Deleted ScrollbarLayerImplBase arg from ScrollbarController.

- Reverted change: crrev.com/c/2174809
- Reverted CL is in PS1.
- Reverted because ScrollbarController::StartAutoScrollAnimation tried
to dereference a null ScrollbarLayerImplBase.
- Relanded CL + fix is in the latest PS.

Root cause: When the user rapidly clicks on the scrollbar, a second
mousedown can arrive within the same VSync as the first mousedown.
When handling the second mousedown, the first thing that happens is
that the ScrollbarController schedules an autoscroll task. After this,
the InputHandlerProxy notices that this 2nd mousedown occurred in the
same VSync and that will lead to LayerTreeHostImpl::ScrollEnd getting
called. ScrollbarController::captured_scrollbar_metadata_ then gets
cleared by ScrollEnd. Due to this, the scrollbar layer reference that
is needed by StartAutoScrollAnimation is null. The previously scheduled
autoscroll task then tries to proceed (after 250ms) by using a null
ScrollbarLayer and fails.

Why didn't this fail before this change: This bug was masked because
when the autoscroll task was previously created, a pointer to the
scrollbar layer was directly passed from the HandlePointerDown to
StartAutoScrollAnimation. This reference always remains valid even
though the InputHandlerProxy cancelled the scroll. With the refactor,
the scrollbar layer needed for StartAutoScrollAnimation is now queried
on demand using captured_scrollbar_metadata_ which gets cleared on
ScrollEnd and this is what exposed the bug.

The fix is to clear the scheduled autoscroll task as well on ScrollEnd.

Original description:
This CL gets rid of the several ScrollbarLayerImplBase references in
the private methods of ScrollbarController. They need not be passed
around because the ScrollbarLayerImplBase can easily be looked up by
calling ScrollbarController::ScrollbarLayer (which retrieves the needed
scrollber layer by using CapturedScrollbarMetadata::scroll_element_id).

Bug: 1077093, 1078765
Change-Id: Ida68e4323e69757a7bfaeab7a7986fb64d07feb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2224606
Commit-Queue: Rahul Arakeri <arakeri@microsoft.com>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774367}

cc/input/scrollbar_controller.cc

@@ -35,7 +35,7 @@ void ScrollbarController::WillBeginImplFrame() {
 }
 
 // Retrieves the ScrollbarLayerImplBase corresponding to the stashed ElementId.
-ScrollbarLayerImplBase* ScrollbarController::ScrollbarLayer() {
+ScrollbarLayerImplBase* ScrollbarController::ScrollbarLayer() const {
   if (!captured_scrollbar_metadata_.has_value())
     return nullptr;
 
@@ -88,9 +88,9 @@ InputHandlerPointerResult ScrollbarController::HandlePointerDown(
   scroll_result.type = PointerResultType::kScrollbarScroll;
   layer_tree_host_impl_->active_tree()->UpdateScrollbarGeometries();
   const ScrollbarPart scrollbar_part =
-      GetScrollbarPartFromPointerDown(scrollbar, position_in_widget);
-  scroll_result.scroll_offset = GetScrollOffsetForScrollbarPart(
-      scrollbar, scrollbar_part, shift_modifier);
+      GetScrollbarPartFromPointerDown(position_in_widget);
+  scroll_result.scroll_offset =
+      GetScrollOffsetForScrollbarPart(scrollbar_part, shift_modifier);
   last_known_pointer_position_ = position_in_widget;
   scrollbar_scroll_is_active_ = true;
   scroll_result.scroll_units = Granularity(scrollbar_part, shift_modifier);
@@ -112,12 +112,12 @@ InputHandlerPointerResult ScrollbarController::HandlePointerDown(
     // have the potential of initiating an autoscroll (if held down for long
     // enough).
     DCHECK(scrollbar_part != ScrollbarPart::THUMB);
-    cancelable_autoscroll_task_ = std::make_unique<base::CancelableOnceClosure>(
-        base::BindOnce(&ScrollbarController::StartAutoScrollAnimation,
-                       base::Unretained(this),
-                       InitialDeltaToAutoscrollVelocity(
-                           scrollbar, scroll_result.scroll_offset),
-                       scrollbar, scrollbar_part));
+    cancelable_autoscroll_task_ =
+        std::make_unique<base::CancelableOnceClosure>(base::BindOnce(
+            &ScrollbarController::StartAutoScrollAnimation,
+            base::Unretained(this),
+            InitialDeltaToAutoscrollVelocity(scroll_result.scroll_offset),
+            scrollbar_part));
     layer_tree_host_impl_->task_runner_provider()
         ->ImplThreadTaskRunner()
         ->PostDelayedTask(FROM_HERE, cancelable_autoscroll_task_->callback(),
@@ -127,16 +127,16 @@ InputHandlerPointerResult ScrollbarController::HandlePointerDown(
 }
 
 bool ScrollbarController::SnapToDragOrigin(
-    const ScrollbarLayerImplBase* scrollbar,
-    const gfx::PointF pointer_position_in_widget) {
+    const gfx::PointF pointer_position_in_widget) const {
   // Consult the ScrollbarTheme to check if thumb snapping is supported on the
   // current platform.
+  const ScrollbarLayerImplBase* scrollbar = ScrollbarLayer();
   if (!(scrollbar && scrollbar->SupportsDragSnapBack()))
     return false;
 
   bool clipped = false;
-  const gfx::PointF pointer_position_in_layer = GetScrollbarRelativePosition(
-      scrollbar, pointer_position_in_widget, &clipped);
+  const gfx::PointF pointer_position_in_layer =
+      GetScrollbarRelativePosition(pointer_position_in_widget, &clipped);
 
   if (clipped)
     return false;
@@ -189,7 +189,7 @@ bool ScrollbarController::SnapToDragOrigin(
 
 ui::ScrollGranularity ScrollbarController::Granularity(
     const ScrollbarPart scrollbar_part,
-    const bool shift_modifier) {
+    const bool shift_modifier) const {
   const bool shift_click_on_scrollbar_track =
       shift_modifier && (scrollbar_part == ScrollbarPart::FORWARD_TRACK ||
                          scrollbar_part == ScrollbarPart::BACK_TRACK);
@@ -201,17 +201,17 @@ ui::ScrollGranularity ScrollbarController::Granularity(
   return ui::ScrollGranularity::kScrollByPixel;
 }
 
-float ScrollbarController::GetScrollDeltaForAbsoluteJump(
-    const ScrollbarLayerImplBase* scrollbar) {
+float ScrollbarController::GetScrollDeltaForAbsoluteJump() const {
   layer_tree_host_impl_->active_tree()->UpdateScrollbarGeometries();
 
   bool clipped = false;
-  const gfx::PointF pointer_position_in_layer = GetScrollbarRelativePosition(
-      scrollbar, last_known_pointer_position_, &clipped);
+  const gfx::PointF pointer_position_in_layer =
+      GetScrollbarRelativePosition(last_known_pointer_position_, &clipped);
 
   if (clipped)
     return 0;
 
+  const ScrollbarLayerImplBase* scrollbar = ScrollbarLayer();
   const float pointer_location =
       scrollbar->orientation() == ScrollbarOrientation::VERTICAL
           ? pointer_position_in_layer.y()
@@ -232,19 +232,18 @@ float ScrollbarController::GetScrollDeltaForAbsoluteJump(
 
   const float delta =
       round(std::abs(desired_thumb_origin - current_thumb_origin));
-  return delta * GetScrollerToScrollbarRatio(scrollbar);
+  return delta * GetScrollerToScrollbarRatio();
 }
 
 int ScrollbarController::GetScrollDeltaForDragPosition(
-    const ScrollbarLayerImplBase* scrollbar,
-    const gfx::PointF pointer_position_in_widget) {
+    const gfx::PointF pointer_position_in_widget) const {
+  const ScrollbarLayerImplBase* scrollbar = ScrollbarLayer();
   const float pointer_delta =
       scrollbar->orientation() == ScrollbarOrientation::VERTICAL
           ? pointer_position_in_widget.y() - drag_state_->drag_origin.y()
           : pointer_position_in_widget.x() - drag_state_->drag_origin.x();
 
-  const float new_offset =
-      pointer_delta * GetScrollerToScrollbarRatio(scrollbar);
+  const float new_offset = pointer_delta * GetScrollerToScrollbarRatio();
   const float scroll_delta = drag_state_->scroll_position_at_start_ +
                              new_offset - scrollbar->current_pos();
 
@@ -276,7 +275,7 @@ InputHandlerPointerResult ScrollbarController::HandlePointerMove(
   if (drag_processed_for_current_frame_)
     return scroll_result;
 
-  if (SnapToDragOrigin(scrollbar, position_in_widget)) {
+  if (SnapToDragOrigin(position_in_widget)) {
     const float delta =
         scrollbar->current_pos() - drag_state_->scroll_position_at_start_;
     scroll_result.scroll_units = ui::ScrollGranularity::kScrollByPrecisePixel;
@@ -302,7 +301,7 @@ InputHandlerPointerResult ScrollbarController::HandlePointerMove(
   // valid ScrollNode.
   DCHECK(target_node);
 
-  int delta = GetScrollDeltaForDragPosition(scrollbar, position_in_widget);
+  int delta = GetScrollDeltaForDragPosition(position_in_widget);
   if (drag_state_->scroller_length_at_previous_move !=
       scrollbar->scroll_layer_length()) {
     drag_state_->scroller_displacement = delta;
@@ -339,8 +338,7 @@ InputHandlerPointerResult ScrollbarController::HandlePointerMove(
   return scroll_result;
 }
 
-float ScrollbarController::GetScrollerToScrollbarRatio(
-    const ScrollbarLayerImplBase* scrollbar) {
+float ScrollbarController::GetScrollerToScrollbarRatio() const {
   // Calculating the delta by which the scroller layer should move when
   // dragging the thumb depends on the following factors:
   // - scrollbar_track_length
@@ -381,14 +379,15 @@ float ScrollbarController::GetScrollerToScrollbarRatio(
   //          |<- scrollbar_thumb_length ->|
   //
   layer_tree_host_impl_->active_tree()->UpdateScrollbarGeometries();
+  const ScrollbarLayerImplBase* scrollbar = ScrollbarLayer();
   float scroll_layer_length = scrollbar->scroll_layer_length();
   float scrollbar_track_length = scrollbar->TrackLength();
   gfx::Rect thumb_rect(scrollbar->ComputeThumbQuadRect());
   float scrollbar_thumb_length =
       scrollbar->orientation() == ScrollbarOrientation::VERTICAL
           ? thumb_rect.height()
           : thumb_rect.width();
-  int viewport_length = GetViewportLength(scrollbar);
+  int viewport_length = GetViewportLength();
 
   return (scroll_layer_length - viewport_length) /
          (scrollbar_track_length - scrollbar_thumb_length);
@@ -399,6 +398,10 @@ void ScrollbarController::ResetState() {
   drag_state_ = base::nullopt;
   autoscroll_state_ = base::nullopt;
   captured_scrollbar_metadata_ = base::nullopt;
+  if (cancelable_autoscroll_task_) {
+    cancelable_autoscroll_task_->Cancel();
+    cancelable_autoscroll_task_.reset();
+  }
 }
 
 void ScrollbarController::DidUnregisterScrollbar(ElementId element_id) {
@@ -413,12 +416,10 @@ void ScrollbarController::RecomputeAutoscrollStateIfNeeded() {
     return;
 
   layer_tree_host_impl_->active_tree()->UpdateScrollbarGeometries();
-  const ScrollbarLayerImplBase* scrollbar = ScrollbarLayer();
-  const gfx::Rect thumb_quad = scrollbar->ComputeThumbQuadRect();
 
   bool clipped;
-  gfx::PointF scroller_relative_position(GetScrollbarRelativePosition(
-      scrollbar, last_known_pointer_position_, &clipped));
+  gfx::PointF scroller_relative_position(
+      GetScrollbarRelativePosition(last_known_pointer_position_, &clipped));
 
   if (clipped)
     return;
@@ -429,6 +430,8 @@ void ScrollbarController::RecomputeAutoscrollStateIfNeeded() {
   int thumb_start = 0;
   int thumb_end = 0;
   int pointer_position = 0;
+  const ScrollbarLayerImplBase* scrollbar = ScrollbarLayer();
+  const gfx::Rect thumb_quad = scrollbar->ComputeThumbQuadRect();
   if (scrollbar->orientation() == ScrollbarOrientation::VERTICAL) {
     thumb_start = thumb_quad.y();
     thumb_end = thumb_quad.y() + thumb_quad.height();
@@ -458,50 +461,51 @@ void ScrollbarController::RecomputeAutoscrollStateIfNeeded() {
     const float scroll_layer_length = scrollbar->scroll_layer_length();
     if (autoscroll_state_->scroll_layer_length != scroll_layer_length) {
       layer_tree_host_impl_->mutator_host()->ScrollAnimationAbort();
-      StartAutoScrollAnimation(autoscroll_state_->velocity, scrollbar,
+      StartAutoScrollAnimation(autoscroll_state_->velocity,
                                autoscroll_state_->pressed_scrollbar_part);
     }
   }
 
   // The animations need to be aborted/restarted based on the pointer location
   // (i.e leaving/entering the track/arrows, reaching the track end etc). The
   // autoscroll_state_ however, needs to be reset on pointer changes.
-  const gfx::RectF scrollbar_part_rect(GetRectForScrollbarPart(
-      scrollbar, autoscroll_state_->pressed_scrollbar_part));
+  const gfx::RectF scrollbar_part_rect(
+      GetRectForScrollbarPart(autoscroll_state_->pressed_scrollbar_part));
   if (!scrollbar_part_rect.Contains(scroller_relative_position)) {
     // Stop animating if pointer moves outside the rect bounds.
     layer_tree_host_impl_->mutator_host()->ScrollAnimationAbort();
   } else if (scrollbar_part_rect.Contains(scroller_relative_position) &&
              !layer_tree_host_impl_->mutator_host()->IsElementAnimating(
                  scrollbar->scroll_element_id())) {
     // Start animating if pointer re-enters the bounds.
-    StartAutoScrollAnimation(autoscroll_state_->velocity, scrollbar,
+    StartAutoScrollAnimation(autoscroll_state_->velocity,
                              autoscroll_state_->pressed_scrollbar_part);
   }
 }
 
 // Helper to calculate the autoscroll velocity.
 float ScrollbarController::InitialDeltaToAutoscrollVelocity(
-    const ScrollbarLayerImplBase* scrollbar,
     gfx::ScrollOffset scroll_offset) const {
+  DCHECK(captured_scrollbar_metadata_.has_value());
   const float scroll_delta =
-      scrollbar->orientation() == ScrollbarOrientation::VERTICAL
+      ScrollbarLayer()->orientation() == ScrollbarOrientation::VERTICAL
           ? scroll_offset.y()
           : scroll_offset.x();
   return scroll_delta * kAutoscrollMultiplier;
 }
 
 void ScrollbarController::StartAutoScrollAnimation(
     const float velocity,
-    const ScrollbarLayerImplBase* scrollbar,
     ScrollbarPart pressed_scrollbar_part) {
   // Autoscroll and thumb drag are mutually exclusive. Both can't be active at
   // the same time.
   DCHECK(!drag_state_.has_value());
+  DCHECK(captured_scrollbar_metadata_.has_value());
   DCHECK_NE(velocity, 0);
 
   // scroll_node is set up while handling GSB. If there's no node to scroll, we
   // don't need to create any animation for it.
+  const ScrollbarLayerImplBase* scrollbar = ScrollbarLayer();
   ScrollTree& scroll_tree =
       layer_tree_host_impl_->active_tree()->property_trees()->scroll_tree;
   ScrollNode* scroll_node =
@@ -552,18 +556,13 @@ InputHandlerPointerResult ScrollbarController::HandlePointerUp(
   if (autoscroll_state_.has_value())
     layer_tree_host_impl_->mutator_host()->ScrollAnimationAbort();
 
-  if (cancelable_autoscroll_task_) {
-    cancelable_autoscroll_task_->Cancel();
-    cancelable_autoscroll_task_.reset();
-  }
-
   ResetState();
   return scroll_result;
 }
 
 // Returns the layer that is hit by the position_in_widget.
 LayerImpl* ScrollbarController::GetLayerHitByPoint(
-    const gfx::PointF position_in_widget) {
+    const gfx::PointF position_in_widget) const {
   LayerTreeImpl* active_tree = layer_tree_host_impl_->active_tree();
   gfx::Point viewport_point(position_in_widget.x(), position_in_widget.y());
 
@@ -575,8 +574,8 @@ LayerImpl* ScrollbarController::GetLayerHitByPoint(
   return layer_impl;
 }
 
-int ScrollbarController::GetViewportLength(
-    const ScrollbarLayerImplBase* scrollbar) const {
+int ScrollbarController::GetViewportLength() const {
+  const ScrollbarLayerImplBase* scrollbar = ScrollbarLayer();
   const ScrollNode* scroll_node =
       layer_tree_host_impl_->active_tree()
           ->property_trees()
@@ -588,29 +587,26 @@ int ScrollbarController::GetViewportLength(
 }
 
 int ScrollbarController::GetScrollDeltaForScrollbarPart(
-    const ScrollbarLayerImplBase* scrollbar,
     const ScrollbarPart scrollbar_part,
-    const bool shift_modifier) {
+    const bool shift_modifier) const {
   int scroll_delta = 0;
 
   switch (scrollbar_part) {
     case ScrollbarPart::BACK_BUTTON:
     case ScrollbarPart::FORWARD_BUTTON:
       if (layer_tree_host_impl_->settings().percent_based_scrolling) {
-        scroll_delta =
-            kPercentDeltaForDirectionalScroll * GetViewportLength(scrollbar);
+        scroll_delta = kPercentDeltaForDirectionalScroll * GetViewportLength();
       } else {
         scroll_delta = kPixelsPerLineStep * ScreenSpaceScaleFactor();
       }
       break;
     case ScrollbarPart::BACK_TRACK:
     case ScrollbarPart::FORWARD_TRACK: {
       if (shift_modifier) {
-        scroll_delta = GetScrollDeltaForAbsoluteJump(scrollbar);
+        scroll_delta = GetScrollDeltaForAbsoluteJump();
         break;
       }
-      scroll_delta =
-          GetViewportLength(scrollbar) * kMinFractionToStepWhenPaging;
+      scroll_delta = GetViewportLength() * kMinFractionToStepWhenPaging;
       break;
     }
     default:
@@ -634,9 +630,8 @@ float ScrollbarController::ScreenSpaceScaleFactor() const {
 }
 
 gfx::PointF ScrollbarController::GetScrollbarRelativePosition(
-    const ScrollbarLayerImplBase* scrollbar,
     const gfx::PointF position_in_widget,
-    bool* clipped) {
+    bool* clipped) const {
   gfx::Transform inverse_screen_space_transform(
       gfx::Transform::kSkipInitialization);
 
@@ -647,7 +642,7 @@ gfx::PointF ScrollbarController::GetScrollbarRelativePosition(
           ? 1.f / layer_tree_host_impl_->active_tree()->device_scale_factor()
           : 1.f;
   gfx::Transform scaled_screen_space_transform(
-      scrollbar->ScreenSpaceTransform());
+      ScrollbarLayer()->ScreenSpaceTransform());
   scaled_screen_space_transform.PostScale(scale, scale);
   if (!scaled_screen_space_transform.GetInverse(
           &inverse_screen_space_transform))
@@ -659,14 +654,14 @@ gfx::PointF ScrollbarController::GetScrollbarRelativePosition(
 
 // Determines the ScrollbarPart based on the position_in_widget.
 ScrollbarPart ScrollbarController::GetScrollbarPartFromPointerDown(
-    const ScrollbarLayerImplBase* scrollbar,
-    const gfx::PointF position_in_widget) {
+    const gfx::PointF position_in_widget) const {
   // position_in_widget needs to be transformed and made relative to the
   // scrollbar layer because hit testing assumes layer relative coordinates.
+  const ScrollbarLayerImplBase* scrollbar = ScrollbarLayer();
   bool clipped = false;
 
   const gfx::PointF scroller_relative_position(
-      GetScrollbarRelativePosition(scrollbar, position_in_widget, &clipped));
+      GetScrollbarRelativePosition(position_in_widget, &clipped));
 
   if (clipped)
     return ScrollbarPart::NO_PART;
@@ -676,8 +671,8 @@ ScrollbarPart ScrollbarController::GetScrollbarPartFromPointerDown(
 
 // Determines the corresponding rect for the given scrollbar part.
 gfx::Rect ScrollbarController::GetRectForScrollbarPart(
-    const ScrollbarLayerImplBase* scrollbar,
-    const ScrollbarPart scrollbar_part) {
+    const ScrollbarPart scrollbar_part) const {
+  const ScrollbarLayerImplBase* scrollbar = ScrollbarLayer();
   if (scrollbar_part == ScrollbarPart::BACK_BUTTON)
     return scrollbar->BackButtonRect();
   if (scrollbar_part == ScrollbarPart::FORWARD_BUTTON)
@@ -692,11 +687,11 @@ gfx::Rect ScrollbarController::GetRectForScrollbarPart(
 // Determines the scroll offsets based on the ScrollbarPart and the scrollbar
 // orientation.
 gfx::ScrollOffset ScrollbarController::GetScrollOffsetForScrollbarPart(
-    const ScrollbarLayerImplBase* scrollbar,
     const ScrollbarPart scrollbar_part,
-    const bool shift_modifier) {
+    const bool shift_modifier) const {
+  const ScrollbarLayerImplBase* scrollbar = ScrollbarLayer();
   float scroll_delta =
-      GetScrollDeltaForScrollbarPart(scrollbar, scrollbar_part, shift_modifier);
+      GetScrollDeltaForScrollbarPart(scrollbar_part, shift_modifier);
 
   // See CreateScrollStateForGesture for more information on how these values
   // will be interpreted.