[Compositor threaded scrollbar scrolling] Jittery thumb fix.

This CL fixes an issue with scrollbar thumb drags being jittery. The
pointermove(s) are not Vsync aligned. When the first GSU is processed,
it gets queued in the compositor thread event queue. A second request
(to get a GSU for a pointermove) within the same frame will end up
calculating an incorrect delta (as ComputeThumbQuadRect would not have
accounted for the delta in the first GSU that was not yet dispatched).
This leads to generating a GSU with a delta that is larger than what it
is supposed to be. When these queued GSUs are finally dispatched, the
subsequent pointermove will make the thumb jump backward and this
problem continues on leading to a jittery thumb drag experience. To fix
this, we process only 1 GSU per frame when thumb drag is in progress.

Bug: 986174
Change-Id: Ied8103ffc447966fc15d0052c5295f896dcbcc29
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1770088
Commit-Queue: Rahul Arakeri <arakeri@microsoft.com>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691973}

cc/input/scrollbar_controller.cc

@@ -27,11 +27,11 @@ ScrollbarController::ScrollbarController(
       scrollbar_scroll_is_active_(false),
       currently_captured_scrollbar_(nullptr),
       previous_pointer_position_(gfx::PointF(0, 0)),
+      drag_processed_for_current_frame_(false),
       cancelable_autoscroll_task_(nullptr) {}
 
 void ScrollbarController::WillBeginImplFrame() {
-  // Since this function deals only with autoscrolling (for now), early out if
-  // there's no autoscroll in progress.
+  drag_processed_for_current_frame_ = false;
   if (!autoscroll_state_.has_value())
     return;
 
@@ -167,8 +167,11 @@ InputHandlerPointerResult ScrollbarController::HandleMouseMove(
   previous_pointer_position_ = position_in_widget;
   InputHandlerPointerResult scroll_result;
 
-  // If a thumb drag is not in progress, there's no point in continuing on.
-  if (!drag_anchor_relative_to_thumb_.has_value())
+  // If a thumb drag is not in progress or if a GSU was already produced for a
+  // thumb drag in this frame, there's no point in continuing on. Please see the
+  // header file for details.
+  if (!drag_anchor_relative_to_thumb_.has_value() ||
+      drag_processed_for_current_frame_)
     return scroll_result;
 
   const ScrollNode* currently_scrolling_node =
@@ -197,6 +200,7 @@ InputHandlerPointerResult ScrollbarController::HandleMouseMove(
       ui::input_types::ScrollGranularity::kScrollByPrecisePixel;
   scroll_result.type = PointerResultType::kScrollbarScroll;
   scroll_result.scroll_offset = gfx::ScrollOffset(clamped_scroll_offset);
+  drag_processed_for_current_frame_ = true;
 
   return scroll_result;
 }

cc/input/scrollbar_controller.h

@@ -107,6 +107,15 @@ class CC_EXPORT ScrollbarController {
   // when a drag has started. It is empty if a thumb drag is *not* in progress.
   base::Optional<gfx::Vector2dF> drag_anchor_relative_to_thumb_;
 
+  // Used to track if a GSU was processed for the current frame or not. Without
+  // this, thumb drag will appear jittery. The reason this happens is because
+  // when the first GSU is processed, it gets queued in the compositor thread
+  // event queue. So a second request within the same frame will end up
+  // calculating an incorrect delta (as ComputeThumbQuadRect would not have
+  // accounted for the delta in the first GSU that was not yet dispatched and
+  // pointermoves are not VSync aligned).
+  bool drag_processed_for_current_frame_;
+
   std::unique_ptr<base::CancelableClosure> cancelable_autoscroll_task_;
 };
 

cc/trees/layer_tree_host_impl_unittest.cc

@@ -12238,6 +12238,98 @@ TEST_F(LayerTreeHostImplTest, ScrollAnimatedWhileZoomed) {
   }
 }
 
+TEST_F(LayerTreeHostImplTest, SingleGSUForScrollbarThumbDragPerFrame) {
+  LayerTreeSettings settings = DefaultSettings();
+  settings.compositor_threaded_scrollbar_scrolling = true;
+  CreateHostImpl(settings, CreateLayerTreeFrameSink());
+
+  // Setup the viewport.
+  const gfx::Size viewport_size = gfx::Size(360, 600);
+  const gfx::Size content_size = gfx::Size(345, 3800);
+  CreateBasicVirtualViewportLayers(viewport_size, content_size);
+  LayerImpl* scroll_layer = host_impl_->OuterViewportScrollLayer();
+
+  // Set up the scrollbar and its dimensions.
+  LayerTreeImpl* layer_tree_impl = host_impl_->active_tree();
+  std::unique_ptr<PaintedScrollbarLayerImpl> scrollbar =
+      PaintedScrollbarLayerImpl::Create(layer_tree_impl, 99, VERTICAL, false,
+                                        true);
+  const gfx::Size scrollbar_size = gfx::Size(15, 600);
+  scrollbar->SetBounds(scrollbar_size);
+  scrollbar->test_properties()->position = gfx::PointF(345, 0);
+  scrollbar->SetScrollElementId(scroll_layer->element_id());
+  scrollbar->SetDrawsContent(true);
+  scrollbar->SetHitTestable(true);
+  scrollbar->test_properties()->opacity = 1.f;
+
+  // Set up the thumb dimensions.
+  scrollbar->SetThumbThickness(15);
+  scrollbar->SetThumbLength(50);
+  scrollbar->SetTrackStart(15);
+  scrollbar->SetTrackLength(575);
+
+  // Set up scrollbar arrows.
+  scrollbar->SetBackButtonRect(
+      gfx::Rect(gfx::Point(345, 0), gfx::Size(15, 15)));
+  scrollbar->SetForwardButtonRect(
+      gfx::Rect(gfx::Point(345, 570), gfx::Size(15, 15)));
+
+  // Add the scrollbar to the outer viewport.
+  scroll_layer->test_properties()->AddChild(std::move(scrollbar));
+
+  host_impl_->active_tree()->BuildLayerListAndPropertyTreesForTesting();
+  host_impl_->ScrollBegin(BeginState(gfx::Point(350, 18)).get(),
+                          InputHandler::SCROLLBAR);
+  TestInputHandlerClient input_handler_client;
+  host_impl_->BindToClient(&input_handler_client);
+
+  // ------------------------- Start frame 0 -------------------------
+  base::TimeTicks start_time =
+      base::TimeTicks() + base::TimeDelta::FromMilliseconds(200);
+  viz::BeginFrameArgs begin_frame_args =
+      viz::CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 1);
+  begin_frame_args.frame_time = start_time;
+  begin_frame_args.sequence_number++;
+  host_impl_->WillBeginImplFrame(begin_frame_args);
+
+  // MouseDown on the thumb should not produce a scroll.
+  InputHandlerPointerResult result =
+      host_impl_->MouseDown(gfx::PointF(350, 18));
+  EXPECT_EQ(result.scroll_offset.y(), 0u);
+
+  // The first request for a GSU should be processed as expected.
+  result = host_impl_->MouseMoveAt(gfx::Point(350, 19));
+  EXPECT_GT(result.scroll_offset.y(), 0u);
+
+  // A second request for a GSU within the same frame should be ignored as it
+  // will cause the thumb drag to become jittery. The reason this happens is
+  // because when the first GSU is processed, it gets queued in the compositor
+  // thread event queue. So a second request within the same frame will end up
+  // calculating an incorrect delta (as ComputeThumbQuadRect would not have
+  // accounted for the delta in the first GSU that was not yet dispatched).
+  result = host_impl_->MouseMoveAt(gfx::Point(350, 20));
+  EXPECT_EQ(result.scroll_offset.y(), 0u);
+  host_impl_->DidFinishImplFrame();
+
+  // ------------------------- Start frame 1 -------------------------
+  begin_frame_args.frame_time =
+      start_time + base::TimeDelta::FromMilliseconds(250);
+  begin_frame_args.sequence_number++;
+  host_impl_->WillBeginImplFrame(begin_frame_args);
+
+  // MouseMove for a new frame gets processed as usual.
+  result = host_impl_->MouseMoveAt(gfx::Point(350, 21));
+  EXPECT_GT(result.scroll_offset.y(), 0u);
+
+  // MouseUp is not expected to have a delta.
+  result = host_impl_->MouseUp(gfx::PointF(350, 21));
+  EXPECT_EQ(result.scroll_offset.y(), 0u);
+
+  // Tear down the LayerTreeHostImpl before the InputHandlerClient.
+  host_impl_->ReleaseLayerTreeFrameSink();
+  host_impl_ = nullptr;
+}
+
 TEST_F(LayerTreeHostImplTest, SecondScrollAnimatedBeginNotIgnored) {
   const gfx::Size content_size(1000, 1000);
   const gfx::Size viewport_size(50, 100);