Commit ea8bab4e authored by Daniel Libby's avatar Daniel Libby Committed by Commit Bot

Use gesture injection for remaining main thread scrollbar scrolls.

Use the InjectGestureScrollEvent API to additionally generate gesture
events to scroll when dragging the thumb, tapping scrollbar parts or
handling gestures forwarded from the browser process.
Instead of invoking ScrollableArea::UserScroll or
SetScrollOffsetSingleAxis, we'll instead generate the appropriate
sequence of scroll gesture events to further unify how scrollbar scrolls
are performed and allow us to gather latency metrics on them. This is
done under the ScrollbarInjectsScrollGestures feature flag (currently
off by default).

For thumb moves, we don't know which direction the scroll will be when
we get MouseDown on the thumb part, which can cause the correct node to
not get added to the scroll chain. Because of this, we use the existing
pattern of generating the GestureScrollBegin 'on demand' when we inject
the first GestureScrollUpdate. This removes the need to inject GSB on
MouseDown, since it works the same way for mouse-down on other parts
as well.

Additionally, if we inject scroll gestures targeted at a
ScrollableArea, in ScrollManager we don't route those gestures to the
scrollbar_handling_scroll_gesture_ but instead bypass and just use
the event_target/scroll_gesture_handling_node_ which was set when the
ScrollableArea was targeted. This allows for GSB/GSU sent from the
browser process to continue to be handled by the scrollbar code (needed
in order to reverse the scroll direction and make the scroll
proportional to the scroller/scrollbar size) while the injected scroll
updates (interleaved with those from the browser process) target the
scrollable area and perform the actual scroll.

One other note: currently, gestures targeting a scrollbar do not
snap when the gesture is completed. Because the scrollbar is handling
the updates (returns true from Scrollbar::GestureEvent) the logic to
handle snapping is never executed. This change fixes this completely for
scroll gestures on scrollbars, but has the side effect of scrollbar taps
on track/buttons to not have a visual effect in the face of snapping.
The code currently just immediately injects a GSE - this is processed
before there is any visual indication of the scroll happening and thus
to the user, it doesn't look like a scroll happened.
This will be addressed in a follow up change to post a delayed task that
will inject a gesture scroll end and clear the pressed part, etc.

Bug: 954007
Change-Id: Ic8a641a3183d5613088a3392bb6bde8fe44e45e5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1633112Reviewed-by: default avatarTimothy Dresser <tdresser@chromium.org>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Commit-Queue: Daniel Libby <dlibby@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#664801}
parent 525a742c
...@@ -361,6 +361,12 @@ IN_PROC_BROWSER_TEST_F(ScrollLatencyBrowserTest, ScrollbarButtonLatency) { ...@@ -361,6 +361,12 @@ IN_PROC_BROWSER_TEST_F(ScrollLatencyBrowserTest, ScrollbarButtonLatency) {
RunScrollbarButtonLatencyTest(); RunScrollbarButtonLatencyTest();
} }
IN_PROC_BROWSER_TEST_F(ScrollLatencyBrowserTest, ScrollbarThumbDragLatency) {
LoadURL();
RunScrollbarThumbDragLatencyTest();
}
class ScrollLatencyCompositedScrollbarBrowserTest class ScrollLatencyCompositedScrollbarBrowserTest
: public ScrollLatencyBrowserTest { : public ScrollLatencyBrowserTest {
public: public:
......
...@@ -622,11 +622,25 @@ void RenderWidgetInputHandler::HandleInjectedScrollGestures( ...@@ -622,11 +622,25 @@ void RenderWidgetInputHandler::HandleInjectedScrollGestures(
ui::LatencyComponentType::INPUT_EVENT_LATENCY_RENDERER_MAIN_COMPONENT); ui::LatencyComponentType::INPUT_EVENT_LATENCY_RENDERER_MAIN_COMPONENT);
if (params.type == WebInputEvent::Type::kGestureScrollUpdate) { if (params.type == WebInputEvent::Type::kGestureScrollUpdate) {
scrollbar_latency_info.AddLatencyNumberWithTimestamp( if (input_event.GetType() != WebInputEvent::Type::kGestureScrollUpdate) {
last_injected_gesture_was_begin_ scrollbar_latency_info.AddLatencyNumberWithTimestamp(
? ui::INPUT_EVENT_LATENCY_FIRST_SCROLL_UPDATE_ORIGINAL_COMPONENT last_injected_gesture_was_begin_
: ui::INPUT_EVENT_LATENCY_SCROLL_UPDATE_ORIGINAL_COMPONENT, ? ui::INPUT_EVENT_LATENCY_FIRST_SCROLL_UPDATE_ORIGINAL_COMPONENT
original_timestamp, 1); : ui::INPUT_EVENT_LATENCY_SCROLL_UPDATE_ORIGINAL_COMPONENT,
original_timestamp, 1);
} else {
// If we're injecting a GSU in response to a GSU (touch drags of the
// scrollbar thumb in Blink handles GSUs, and reverses them with
// injected GSUs), the LatencyInfo will already have the appropriate
// SCROLL_UPDATE component set.
DCHECK(
scrollbar_latency_info.FindLatency(
ui::INPUT_EVENT_LATENCY_FIRST_SCROLL_UPDATE_ORIGINAL_COMPONENT,
nullptr) ||
scrollbar_latency_info.FindLatency(
ui::INPUT_EVENT_LATENCY_SCROLL_UPDATE_ORIGINAL_COMPONENT,
nullptr));
}
} }
std::unique_ptr<cc::SwapPromiseMonitor> swap_promise_monitor; std::unique_ptr<cc::SwapPromiseMonitor> swap_promise_monitor;
......
...@@ -871,7 +871,15 @@ WebInputEventResult ScrollManager::HandleGestureScrollEvent( ...@@ -871,7 +871,15 @@ WebInputEventResult ScrollManager::HandleGestureScrollEvent(
scrollbar = result.GetScrollbar(); scrollbar = result.GetScrollbar();
} }
if (scrollbar) { // Gesture scroll events injected by scrollbars should not be routed back to
// the scrollbar itself as they are intended to perform the scroll action on
// the scrollable area. Scrollbar injected gestures don't clear
// scrollbar_handling_scroll_gesture_ because touch-based scroll gestures need
// to continue going to the scrollbar first so that the scroll direction
// can be made proportional to the scroll thumb/ScrollableArea size and
// inverted.
if (scrollbar &&
gesture_event.SourceDevice() != WebGestureDevice::kScrollbar) {
bool should_update_capture = false; bool should_update_capture = false;
if (scrollbar->GestureEvent(gesture_event, &should_update_capture)) { if (scrollbar->GestureEvent(gesture_event, &should_update_capture)) {
if (should_update_capture) if (should_update_capture)
......
...@@ -199,7 +199,13 @@ class CORE_EXPORT Scrollbar : public GarbageCollectedFinalized<Scrollbar>, ...@@ -199,7 +199,13 @@ class CORE_EXPORT Scrollbar : public GarbageCollectedFinalized<Scrollbar>,
void StartTimerIfNeeded(TimeDelta delay); void StartTimerIfNeeded(TimeDelta delay);
void StopTimerIfNeeded(); void StopTimerIfNeeded();
void AutoscrollPressedPart(TimeDelta delay); void AutoscrollPressedPart(TimeDelta delay);
void InjectScrollGesture(WebInputEvent::Type gesture_type); bool HandleTapGesture();
bool IsScrollGestureInjectionEnabled() const;
void InjectScrollGestureForPressedPart(WebInputEvent::Type gesture_type);
void InjectGestureScrollUpdateForThumbMove(float single_axis_target_offset);
void InjectScrollGesture(WebInputEvent::Type type,
ScrollOffset delta,
ScrollGranularity granularity);
ScrollDirectionPhysical PressedPartScrollDirectionPhysical(); ScrollDirectionPhysical PressedPartScrollDirectionPhysical();
ScrollGranularity PressedPartScrollGranularity(); ScrollGranularity PressedPartScrollGranularity();
......
...@@ -162,10 +162,12 @@ function test3() { ...@@ -162,10 +162,12 @@ function test3() {
eventSender.mouseDown(); eventSender.mouseDown();
eventSender.mouseMoveTo(scrollbarCenterX, suggestionListOffset[1] + suggestionList.offsetHeight); eventSender.mouseMoveTo(scrollbarCenterX, suggestionListOffset[1] + suggestionList.offsetHeight);
eventSender.mouseUp(); eventSender.mouseUp();
scrollTopAfterWheelEvent = suggestionList.scrollTop; requestAnimationFrame(function() {
shouldBeTrue('scrollTopBeforeWheelEvent < scrollTopAfterWheelEvent'); scrollTopAfterWheelEvent = suggestionList.scrollTop;
shouldBeTrue('scrollTopBeforeWheelEvent < scrollTopAfterWheelEvent');
finishJSTest(); finishJSTest();
});
} }
</script> </script>
......
...@@ -115,10 +115,12 @@ function test3() { ...@@ -115,10 +115,12 @@ function test3() {
eventSender.mouseDown(); eventSender.mouseDown();
eventSender.mouseMoveTo(scrollbarCenterX, suggestionListOffset[1] + suggestionList.offsetHeight); eventSender.mouseMoveTo(scrollbarCenterX, suggestionListOffset[1] + suggestionList.offsetHeight);
eventSender.mouseUp(); eventSender.mouseUp();
scrollTopAfterWheelEvent = suggestionList.scrollTop; requestAnimationFrame(function() {
shouldBeTrue('scrollTopBeforeWheelEvent < scrollTopAfterWheelEvent'); scrollTopAfterWheelEvent = suggestionList.scrollTop;
shouldBeTrue('scrollTopBeforeWheelEvent < scrollTopAfterWheelEvent');
finishJSTest(); finishJSTest();
});
} }
</script> </script>
......
...@@ -142,10 +142,12 @@ function test3() { ...@@ -142,10 +142,12 @@ function test3() {
eventSender.mouseDown(); eventSender.mouseDown();
eventSender.mouseMoveTo(scrollbarCenterX, suggestionListOffset[1] + suggestionList.offsetHeight); eventSender.mouseMoveTo(scrollbarCenterX, suggestionListOffset[1] + suggestionList.offsetHeight);
eventSender.mouseUp(); eventSender.mouseUp();
scrollTopAfterWheelEvent = suggestionList.scrollTop; requestAnimationFrame(function() {
shouldBeTrue('scrollTopBeforeWheelEvent < scrollTopAfterWheelEvent'); scrollTopAfterWheelEvent = suggestionList.scrollTop;
shouldBeTrue('scrollTopBeforeWheelEvent < scrollTopAfterWheelEvent');
finishJSTest(); finishJSTest();
});
} }
</script> </script>
......
...@@ -118,10 +118,12 @@ function test3() { ...@@ -118,10 +118,12 @@ function test3() {
eventSender.mouseDown(); eventSender.mouseDown();
eventSender.mouseMoveTo(scrollbarCenterX, suggestionListOffset[1] + suggestionList.offsetHeight); eventSender.mouseMoveTo(scrollbarCenterX, suggestionListOffset[1] + suggestionList.offsetHeight);
eventSender.mouseUp(); eventSender.mouseUp();
scrollTopAfterWheelEvent = suggestionList.scrollTop; requestAnimationFrame(function() {
shouldBeTrue('scrollTopBeforeWheelEvent < scrollTopAfterWheelEvent'); scrollTopAfterWheelEvent = suggestionList.scrollTop;
shouldBeTrue('scrollTopBeforeWheelEvent < scrollTopAfterWheelEvent');
finishJSTest(); finishJSTest();
});
} }
</script> </script>
......
...@@ -161,10 +161,12 @@ function test3() { ...@@ -161,10 +161,12 @@ function test3() {
eventSender.mouseDown(); eventSender.mouseDown();
eventSender.mouseMoveTo(scrollbarCenterX, suggestionListOffset[1] + suggestionList.offsetHeight); eventSender.mouseMoveTo(scrollbarCenterX, suggestionListOffset[1] + suggestionList.offsetHeight);
eventSender.mouseUp(); eventSender.mouseUp();
scrollTopAfterWheelEvent = suggestionList.scrollTop; requestAnimationFrame(function() {
shouldBeTrue('scrollTopBeforeWheelEvent < scrollTopAfterWheelEvent'); scrollTopAfterWheelEvent = suggestionList.scrollTop;
shouldBeTrue('scrollTopBeforeWheelEvent < scrollTopAfterWheelEvent');
finishJSTest(); finishJSTest();
});
} }
</script> </script>
......
...@@ -23,6 +23,7 @@ This test verifies scroll position restores correctly when a thumb drag has been ...@@ -23,6 +23,7 @@ This test verifies scroll position restores correctly when a thumb drag has been
This test is expected to fail on Mac and Linux because those platforms don't cancel scrolling when mouse cursor is out of a certain range. It is Windows-specific behavior. This test is expected to fail on Mac and Linux because those platforms don't cancel scrolling when mouse cursor is out of a certain range. It is Windows-specific behavior.
<script src="../../resources/js-test.js"></script> <script src="../../resources/js-test.js"></script>
<script> <script>
window.jsTestIsAsync = true;
var container = document.getElementById("container"); var container = document.getElementById("container");
container.scrollLeft = 350; container.scrollLeft = 350;
...@@ -32,9 +33,13 @@ This test is expected to fail on Mac and Linux because those platforms don't can ...@@ -32,9 +33,13 @@ This test is expected to fail on Mac and Linux because those platforms don't can
eventSender.mouseDown(); eventSender.mouseDown();
eventSender.mouseMoveTo(100, 195); eventSender.mouseMoveTo(100, 195);
shouldBe("container.scrollLeft" , "0"); requestAnimationFrame(function() {
shouldBe("container.scrollLeft" , "0");
eventSender.mouseMoveTo(100, 700); eventSender.mouseMoveTo(100, 700);
shouldBe("container.scrollLeft" , "350"); requestAnimationFrame(function() {
shouldBe("container.scrollLeft" , "350");
finishJSTest();
});
});
} }
</script> </script>
<!DOCTYPE html>
<title>Scrollbar forward button must not scroll ancestor</title>
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<script src="../../../resources/gesture-util.js"></script>
<style>
.subscroller {
width:100px;
height:100px;
overflow:auto;
position:absolute;
top:0px;
left:0px;
}
.subscroller>.child {
height:400px;
}
.talldiv {
height:4000px;
}
</style>
<div class="subscroller">
<div class="child">subscroller</div>
</div>
<div class="talldiv">talldiv</div>
<script>
let clicks = 0;
window.addEventListener("click", e => clicks++);
promise_test(async () => {
// Ensure the page is ready to receive input.
await waitForCompositorCommit();
// Tap on the top of the scrollbar - this shouldn't scroll and thus
// click should be fired.
await touchTapOn(95, 5);
assert_equals(clicks, 1, "Tapping on scrollbar that doesn't scroll " +
"must fire a click event");
// Tap on the bottom of the scrollbar - this should scroll and no click
// event should be fired.
await touchTapOn(95, 95);
assert_equals(clicks, 1, "Tapping on scrollbar that doesn't scroll " +
"must fire a click event");
}, "Tapping a scrollbar part that doesn't scroll should fire a click event");
</script>
PASS container.scrollTop is within 0.02 of 0.57
PASS successfullyParsed is true PASS successfullyParsed is true
TEST COMPLETE TEST COMPLETE
PASS container.scrollTop is within 0.02 of 0.57
Tests scroll behavior when a subpixel scrollbar thumb is dragged by mouse. Tests scroll behavior when a subpixel scrollbar thumb is dragged by mouse.
...@@ -21,6 +21,7 @@ Tests scroll behavior when a subpixel scrollbar thumb is dragged by mouse. ...@@ -21,6 +21,7 @@ Tests scroll behavior when a subpixel scrollbar thumb is dragged by mouse.
</div> </div>
<script src="../../resources/js-test.js"></script> <script src="../../resources/js-test.js"></script>
<script> <script>
window.jsTestIsAsync = true;
if (window.eventSender) { if (window.eventSender) {
onload = function() { onload = function() {
var container = document.getElementById('container'); var container = document.getElementById('container');
...@@ -29,7 +30,10 @@ if (window.eventSender) { ...@@ -29,7 +30,10 @@ if (window.eventSender) {
eventSender.mouseMoveTo(340, 250); eventSender.mouseMoveTo(340, 250);
eventSender.mouseUp(); eventSender.mouseUp();
shouldBeCloseTo("container.scrollTop", "0.57", 0.02); requestAnimationFrame(() => {
shouldBeCloseTo("container.scrollTop", "0.57", 0.02);
finishJSTest();
});
}; };
} }
</script> </script>
\ No newline at end of file
...@@ -46,8 +46,10 @@ ...@@ -46,8 +46,10 @@
eventSender.mouseMoveTo(0, 145); eventSender.mouseMoveTo(0, 145);
eventSender.mouseUp(); eventSender.mouseUp();
} }
if (window.testRunner) requestAnimationFrame(function() {
document.getElementById("result").innerText = container.scrollLeft == 0 ? "PASS" : "FAIL"; if (window.testRunner)
document.getElementById("result").innerText = container.scrollLeft == 0 ? "PASS" : "FAIL";
});
</script> </script>
</body> </body>
</html> </html>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment