Preserve pressed part of scrollbar when scroll anchoring.

When we are applying scroll anchoring we should not update the pressed
part of the scrollbar as it leads to unstable scrolling.

Bug: 1043674
Change-Id: Id19f33626be2c2607351bb208f1b4f340477db8c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2010053
Commit-Queue: Robert Flack <flackr@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733975}

third_party/blink/renderer/core/layout/scroll_anchor_test.cc

@@ -5,6 +5,7 @@
 #include "third_party/blink/renderer/core/layout/scroll_anchor.h"
 
 #include "build/build_config.h"
+#include "third_party/blink/public/common/input/web_mouse_event.h"
 #include "third_party/blink/renderer/core/dom/static_node_list.h"
 #include "third_party/blink/renderer/core/frame/root_frame_viewport.h"
 #include "third_party/blink/renderer/core/frame/visual_viewport.h"
@@ -28,6 +29,11 @@ class ScrollAnchorTest : public testing::WithParamInterface<bool>,
   ScrollAnchorTest() : ScopedLayoutNGForTest(GetParam()) {}
 
  protected:
+  void SetUp() override {
+    EnableCompositing();
+    RenderingTest::SetUp();
+  }
+
   void Update() {
     // TODO(skobes): Use SimTest instead of RenderingTest and move into
     // Source/web?
@@ -79,6 +85,52 @@ class ScrollAnchorTest : public testing::WithParamInterface<bool>,
         GetDocument().QuerySelectorAll(AtomicString(serialized.selector));
     EXPECT_EQ(ele_list->length(), 1u);
   }
+
+  Scrollbar* VerticalScrollbarForElement(Element* element) {
+    return ToLayoutBox(element->GetLayoutObject())
+        ->GetScrollableArea()
+        ->VerticalScrollbar();
+  }
+
+  void MouseDownOnVerticalScrollbar(Scrollbar* scrollbar) {
+    DCHECK_EQ(true, scrollbar->GetTheme().AllowsHitTest());
+    int thumb_center = scrollbar->GetTheme().ThumbPosition(*scrollbar) +
+                       scrollbar->GetTheme().ThumbLength(*scrollbar) / 2;
+    scrollbar_drag_point_ =
+        gfx::PointF(scrollbar->GetScrollableArea()
+                        ->ConvertFromScrollbarToContainingEmbeddedContentView(
+                            *scrollbar, IntPoint(0, thumb_center)));
+    scrollbar->MouseDown(blink::WebMouseEvent(
+        blink::WebInputEvent::kMouseDown, *scrollbar_drag_point_,
+        *scrollbar_drag_point_, blink::WebPointerProperties::Button::kLeft, 0,
+        blink::WebInputEvent::kNoModifiers, base::TimeTicks::Now()));
+  }
+
+  void MouseDragVerticalScrollbar(Scrollbar* scrollbar, float scroll_delta_y) {
+    DCHECK(scrollbar_drag_point_);
+    ScrollableArea* scroller = scrollbar->GetScrollableArea();
+    scrollbar_drag_point_->Offset(
+        0, scroll_delta_y *
+               (scrollbar->GetTheme().TrackLength(*scrollbar) -
+                scrollbar->GetTheme().ThumbLength(*scrollbar)) /
+               (scroller->MaximumScrollOffset().Height() -
+                scroller->MinimumScrollOffset().Height()));
+    scrollbar->MouseMoved(blink::WebMouseEvent(
+        blink::WebInputEvent::kMouseMove, *scrollbar_drag_point_,
+        *scrollbar_drag_point_, blink::WebPointerProperties::Button::kLeft, 0,
+        blink::WebInputEvent::kNoModifiers, base::TimeTicks::Now()));
+  }
+
+  void MouseUpOnVerticalScrollbar(Scrollbar* scrollbar) {
+    DCHECK(scrollbar_drag_point_);
+    scrollbar->MouseDown(blink::WebMouseEvent(
+        blink::WebInputEvent::kMouseUp, *scrollbar_drag_point_,
+        *scrollbar_drag_point_, blink::WebPointerProperties::Button::kLeft, 0,
+        blink::WebInputEvent::kNoModifiers, base::TimeTicks::Now()));
+    scrollbar_drag_point_.reset();
+  }
+
+  base::Optional<gfx::PointF> scrollbar_drag_point_;
 };
 
 INSTANTIATE_TEST_SUITE_P(All, ScrollAnchorTest, testing::Bool());
@@ -328,6 +380,51 @@ TEST_P(ScrollAnchorTest, AnchorWithLayerInScrollingDiv) {
   EXPECT_EQ(250, scroller->ScrollOffsetInt().Height());
 }
 
+TEST_P(ScrollAnchorTest, AnchorWhileDraggingScrollbar) {
+  // Dragging the scrollbar is inherently inaccurate. Allow many pixels slop in
+  // the scroll position.
+  const int kScrollbarDragAccuracy = 10;
+  USE_NON_OVERLAY_SCROLLBARS();
+  SetBodyInnerHTML(R"HTML(
+    <style>
+        #scroller { overflow: scroll; width: 500px; height: 400px; }
+        div { height: 100px }
+        #block2 { overflow: hidden }
+        #space { height: 1000px; }
+    </style>
+    <div id='scroller'><div id='space'>
+    <div id='block1'>abc</div>
+    <div id='block2'>def</div>
+    </div></div>
+  )HTML");
+  Element* scroller_element = GetDocument().getElementById("scroller");
+  ScrollableArea* scroller = ScrollerForElement(scroller_element);
+
+  Element* block1 = GetDocument().getElementById("block1");
+  Element* block2 = GetDocument().getElementById("block2");
+
+  Scrollbar* scrollbar = VerticalScrollbarForElement(scroller_element);
+  scroller->MouseEnteredScrollbar(*scrollbar);
+  MouseDownOnVerticalScrollbar(scrollbar);
+  MouseDragVerticalScrollbar(scrollbar, 150);
+  EXPECT_NEAR(150, scroller->GetScrollOffset().Height(),
+              kScrollbarDragAccuracy);
+
+  // In this layout pass we will anchor to #block2 which has its own PaintLayer.
+  SetHeight(block1, 200);
+  EXPECT_NEAR(250, scroller->ScrollOffsetInt().Height(),
+              kScrollbarDragAccuracy);
+  EXPECT_EQ(block2->GetLayoutObject(),
+            GetScrollAnchor(scroller).AnchorObject());
+
+  // If we continue dragging the scroller should scroll from the newly anchored
+  // position.
+  MouseDragVerticalScrollbar(scrollbar, 10);
+  EXPECT_NEAR(260, scroller->ScrollOffsetInt().Height(),
+              kScrollbarDragAccuracy);
+  MouseUpOnVerticalScrollbar(scrollbar);
+}
+
 // Verify that a nested scroller with a div that has its own PaintLayer can be
 // removed without causing a crash. This test passes if it doesn't crash.
 TEST_P(ScrollAnchorTest, RemoveScrollerWithLayerInScrollingDiv) {

third_party/blink/renderer/core/scroll/scrollable_area.cc

@@ -379,9 +379,9 @@ void ScrollableArea::ScrollOffsetChanged(const ScrollOffset& offset,
   // invalidated to reflect the new thumb offset, even if the theme did not
   // invalidate any individual part.
   if (Scrollbar* horizontal_scrollbar = this->HorizontalScrollbar())
-    horizontal_scrollbar->OffsetDidChange();
+    horizontal_scrollbar->OffsetDidChange(scroll_type);
   if (Scrollbar* vertical_scrollbar = this->VerticalScrollbar())
-    vertical_scrollbar->OffsetDidChange();
+    vertical_scrollbar->OffsetDidChange(scroll_type);
 
   ScrollOffset delta = GetScrollOffset() - old_offset;
   // TODO(skobes): Should we exit sooner when the offset has not changed?

third_party/blink/renderer/core/scroll/scrollbar.cc

@@ -141,7 +141,7 @@ int Scrollbar::Maximum() const {
                                               : max_offset.Height();
 }
 
-void Scrollbar::OffsetDidChange() {
+void Scrollbar::OffsetDidChange(ScrollType scroll_type) {
   DCHECK(scrollable_area_);
 
   float position = ScrollableAreaCurrentPos();
@@ -157,9 +157,13 @@ void Scrollbar::OffsetDidChange() {
                                                         position);
   SetNeedsPaintInvalidation(invalid_parts);
 
-  if (pressed_part_ == kThumbPart)
+  // Don't update the pressed position if scroll anchoring takes place as
+  // otherwise the next thumb movement will undo anchoring.
+  if (pressed_part_ == kThumbPart &&
+      scroll_type != ScrollType::kAnchoringScroll) {
     SetPressedPos(pressed_pos_ + GetTheme().ThumbPosition(*this) -
                   old_thumb_position);
+  }
 }
 
 void Scrollbar::DisconnectFromScrollableArea() {

third_party/blink/renderer/core/scroll/scrollbar.h

@@ -111,7 +111,7 @@ class CORE_EXPORT Scrollbar : public GarbageCollected<Scrollbar>,
 
   // Called by the ScrollableArea when the scroll offset changes.
   // Will trigger paint invalidation if required.
-  void OffsetDidChange();
+  void OffsetDidChange(ScrollType scroll_type);
 
   virtual void DisconnectFromScrollableArea();
   ScrollableArea* GetScrollableArea() const { return scrollable_area_; }

third_party/blink/renderer/core/scroll/scrollbar_theme_overlay_test.cc

@@ -4,6 +4,7 @@
 
 #include "third_party/blink/renderer/core/scroll/scrollbar_theme_overlay.h"
 
+#include "third_party/blink/renderer/core/scroll/scroll_types.h"
 #include "third_party/blink/renderer/core/scroll/scrollbar_test_suite.h"
 #include "third_party/blink/renderer/platform/testing/testing_platform_support_with_mock_scheduler.h"
 
@@ -58,8 +59,8 @@ TEST_F(ScrollbarThemeOverlayTest, PaintInvalidation) {
   // should cause a "general" invalidation for non-composited scrollbars.
   // Ensure the horizontal scrollbar is unaffected.
   mock_scrollable_area->UpdateScrollOffset(ScrollOffset(0, 5), kUserScroll);
-  vertical_scrollbar->OffsetDidChange();
-  horizontal_scrollbar->OffsetDidChange();
+  vertical_scrollbar->OffsetDidChange(ScrollType::kUserScroll);
+  horizontal_scrollbar->OffsetDidChange(ScrollType::kUserScroll);
   EXPECT_FALSE(vertical_scrollbar->ThumbNeedsRepaint());
   EXPECT_FALSE(vertical_scrollbar->TrackNeedsRepaint());
   EXPECT_TRUE(mock_scrollable_area->VerticalScrollbarNeedsPaintInvalidation());
@@ -71,8 +72,8 @@ TEST_F(ScrollbarThemeOverlayTest, PaintInvalidation) {
   // Try the horizontal scrollbar.
   mock_scrollable_area->ClearNeedsPaintInvalidationForScrollControls();
   mock_scrollable_area->UpdateScrollOffset(ScrollOffset(5, 5), kUserScroll);
-  horizontal_scrollbar->OffsetDidChange();
-  vertical_scrollbar->OffsetDidChange();
+  horizontal_scrollbar->OffsetDidChange(ScrollType::kUserScroll);
+  vertical_scrollbar->OffsetDidChange(ScrollType::kUserScroll);
   EXPECT_FALSE(vertical_scrollbar->ThumbNeedsRepaint());
   EXPECT_FALSE(vertical_scrollbar->TrackNeedsRepaint());
   EXPECT_FALSE(mock_scrollable_area->VerticalScrollbarNeedsPaintInvalidation());

third_party/blink/renderer/core/testing/DEPS

@@ -7,6 +7,7 @@ include_rules = [
     "+content/renderer/compositor",
     "+content/test",
     "+gpu/command_buffer/client/gles2_interface.h",
+    "+ui/events/blink/blink_event_util.h"
 ]
 
 specific_include_rules = {

third_party/blink/renderer/core/testing/core_unit_test_helper.cc

@@ -6,11 +6,13 @@
 
 #include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_core.h"
 #include "third_party/blink/renderer/core/html/html_iframe_element.h"
+#include "third_party/blink/renderer/core/input/event_handler.h"
 #include "third_party/blink/renderer/core/page/page.h"
 #include "third_party/blink/renderer/core/scroll/scrollbar_theme.h"
 #include "third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
 #include "third_party/blink/renderer/platform/loader/fetch/memory_cache.h"
+#include "ui/events/blink/blink_event_util.h"
 
 namespace blink {
 
@@ -36,6 +38,27 @@ void LocalFrameClientWithParent::Detached(FrameDetachType) {
       ->DidDetachChild();
 }
 
+void RenderingTestChromeClient::InjectGestureScrollEvent(
+    LocalFrame& local_frame,
+    WebGestureDevice device,
+    const gfx::Vector2dF& delta,
+    ScrollGranularity granularity,
+    CompositorElementId scrollable_area_element_id,
+    WebInputEvent::Type injected_type) {
+  // Directly handle injected gesture scroll events. In a real browser, these
+  // would be added to the event queue and handled asynchronously but immediate
+  // handling is sufficient to test scrollbar dragging.
+  std::unique_ptr<WebGestureEvent> gesture_event =
+      ui::GenerateInjectedScrollGesture(injected_type, base::TimeTicks::Now(),
+                                        device, gfx::PointF(0, 0), delta,
+                                        granularity);
+  if (injected_type == WebInputEvent::Type::kGestureScrollBegin) {
+    gesture_event->data.scroll_begin.scrollable_area_element_id =
+        scrollable_area_element_id.GetStableId();
+  }
+  local_frame.GetEventHandler().HandleGestureEvent(*gesture_event);
+}
+
 RenderingTestChromeClient& RenderingTest::GetChromeClient() const {
   DEFINE_STATIC_LOCAL(Persistent<RenderingTestChromeClient>, client,
                       (MakeGarbageCollected<RenderingTestChromeClient>()));

third_party/blink/renderer/core/testing/core_unit_test_helper.h

@@ -97,6 +97,13 @@ class RenderingTestChromeClient : public EmptyChromeClient {
     return device_emulation_transform_;
   }
 
+  void InjectGestureScrollEvent(LocalFrame& local_frame,
+                                WebGestureDevice device,
+                                const gfx::Vector2dF& delta,
+                                ScrollGranularity granularity,
+                                CompositorElementId scrollable_area_element_id,
+                                WebInputEvent::Type injected_type) override;
+
  private:
   std::unique_ptr<LayerTreeHostEmbedder> layer_tree_;
   TransformationMatrix device_emulation_transform_;