Commit 28720bb0 authored by Daniel Libby's avatar Daniel Libby Committed by Commit Bot

Don't queue scroll update for first wheel scroll update in sequence

crrev.com/c/1774911 changed how the first scroll update is detected.
That information is consumed in order to determine whether or not a
scroll update event should get queued and dispatched on the next
begin frame, or if it should be dispatched immediately.

The refactoring had a logic bug because it was trying to detect GSB
within the block of has_ongoing_compositor_scroll_or_pinch_ and thus the
flag was never reset.

This wasn't caught by the unittest for this behavior since the flag was
initialized to false in the constructor. I'm adding another scroll begin
update sequence to the unittest so that we have coverage on the state
transition.

Bug: 1002045
Change-Id: I781d782de794ccb0639d972325a4579e1c61abe2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1816360
Commit-Queue: Daniel Libby <dlibby@microsoft.com>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#698673}
parent 03e69111
...@@ -256,8 +256,19 @@ void InputHandlerProxy::HandleInputEventWithLatencyInfo( ...@@ -256,8 +256,19 @@ void InputHandlerProxy::HandleInputEventWithLatencyInfo(
has_ongoing_compositor_scroll_or_pinch_ has_ongoing_compositor_scroll_or_pinch_
? ONGOING_SCROLL_PINCH ? ONGOING_SCROLL_PINCH
: SCROLL_PINCH); : SCROLL_PINCH);
const auto& gesture_event = ToWebGestureEvent(event_with_callback->event());
const bool is_first_gesture_scroll_update =
!has_seen_first_gesture_scroll_update_after_begin_ &&
gesture_event.GetType() == blink::WebGestureEvent::kGestureScrollUpdate;
if (gesture_event.GetType() == blink::WebGestureEvent::kGestureScrollBegin) {
has_seen_first_gesture_scroll_update_after_begin_ = false;
} else if (gesture_event.GetType() ==
blink::WebGestureEvent::kGestureScrollUpdate) {
has_seen_first_gesture_scroll_update_after_begin_ = true;
}
if (has_ongoing_compositor_scroll_or_pinch_) { if (has_ongoing_compositor_scroll_or_pinch_) {
const auto& gesture_event = ToWebGestureEvent(event_with_callback->event());
bool is_from_set_non_blocking_touch = bool is_from_set_non_blocking_touch =
gesture_event.SourceDevice() == blink::WebGestureDevice::kTouchscreen && gesture_event.SourceDevice() == blink::WebGestureDevice::kTouchscreen &&
gesture_event.is_source_touch_event_set_non_blocking; gesture_event.is_source_touch_event_set_non_blocking;
...@@ -266,17 +277,7 @@ void InputHandlerProxy::HandleInputEventWithLatencyInfo( ...@@ -266,17 +277,7 @@ void InputHandlerProxy::HandleInputEventWithLatencyInfo(
gesture_event.GetType() == blink::WebGestureEvent::kGestureScrollEnd; gesture_event.GetType() == blink::WebGestureEvent::kGestureScrollEnd;
bool scroll_update_has_blocking_wheel_source = bool scroll_update_has_blocking_wheel_source =
gesture_event.SourceDevice() == blink::WebGestureDevice::kTouchpad && gesture_event.SourceDevice() == blink::WebGestureDevice::kTouchpad &&
gesture_event.GetType() == is_first_gesture_scroll_update;
blink::WebGestureEvent::kGestureScrollUpdate &&
!has_seen_first_gesture_scroll_update_after_begin_;
if (gesture_event.GetType() ==
blink::WebGestureEvent::kGestureScrollBegin) {
has_seen_first_gesture_scroll_update_after_begin_ = false;
} else if (gesture_event.GetType() ==
blink::WebGestureEvent::kGestureScrollUpdate) {
has_seen_first_gesture_scroll_update_after_begin_ = true;
}
if (is_from_set_non_blocking_touch || is_scroll_end_from_wheel || if (is_from_set_non_blocking_touch || is_scroll_end_from_wheel ||
scroll_update_has_blocking_wheel_source || synchronous_input_handler_) { scroll_update_has_blocking_wheel_source || synchronous_input_handler_) {
......
...@@ -1874,6 +1874,18 @@ TEST_F(InputHandlerProxyEventQueueTest, TouchpadGestureScrollEndFlushQueue) { ...@@ -1874,6 +1874,18 @@ TEST_F(InputHandlerProxyEventQueueTest, TouchpadGestureScrollEndFlushQueue) {
EXPECT_FALSE( EXPECT_FALSE(
input_handler_proxy_.gesture_scroll_on_impl_thread_for_testing()); input_handler_proxy_.gesture_scroll_on_impl_thread_for_testing());
// Starting a new scroll sequence should have the same behavior (namely that
// the first scroll update is not queued but immediately dispatched).
HandleGestureEventWithSourceDevice(WebInputEvent::kGestureScrollBegin,
blink::WebGestureDevice::kTouchpad);
HandleGestureEventWithSourceDevice(WebInputEvent::kGestureScrollUpdate,
blink::WebGestureDevice::kTouchpad, -20);
// Both GSB and the first GSU must be dispatched immediately since the first
// GSU has blocking wheel event source.
EXPECT_EQ(0ul, event_queue().size());
EXPECT_EQ(6ul, event_disposition_recorder_.size());
} }
TEST_F(InputHandlerProxyEventQueueTest, CoalescedLatencyInfo) { TEST_F(InputHandlerProxyEventQueueTest, CoalescedLatencyInfo) {
......
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