Skip to content

Commit

Permalink
A GSU with blocking wheel source shouldn't wait for Vsync signal.
Browse files Browse the repository at this point in the history
For a touchpad based gesture event, we want to know whether the source
wheel event was handled blocking or non-blocking, and we can flush the
vsync queue immediately if it was blocking.

Bug: 526463
Test: InputHandlerProxyEventQueueTest.TouchpadGestureScrollEndFlushQueue
Change-Id: I051f042dc7da6f901d135a5f7785a97d25581d8e
Reviewed-on: https://chromium-review.googlesource.com/849313
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Sahel Sharifymoghaddam <sahel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526847}
  • Loading branch information
Sahel Sharify authored and Commit Bot committed Jan 3, 2018
1 parent 3672c40 commit ff58622
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 25 deletions.
10 changes: 6 additions & 4 deletions content/renderer/input/input_handler_wrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ InputHandlerWrapper::InputHandlerWrapper(
bool enable_smooth_scrolling)
: input_handler_manager_(input_handler_manager),
routing_id_(routing_id),
input_handler_proxy_(input_handler.get(),
this,
base::FeatureList::IsEnabled(
features::kTouchpadAndWheelScrollLatching)),
input_handler_proxy_(
input_handler.get(),
this,
base::FeatureList::IsEnabled(
features::kTouchpadAndWheelScrollLatching),
base::FeatureList::IsEnabled(features::kAsyncWheelEvents)),
main_task_runner_(main_task_runner),
render_widget_(render_widget) {
DCHECK(input_handler);
Expand Down
3 changes: 2 additions & 1 deletion content/renderer/input/widget_input_handler_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,8 @@ void WidgetInputHandlerManager::InitOnCompositorThread(
bool smooth_scroll_enabled) {
input_handler_proxy_ = std::make_unique<ui::InputHandlerProxy>(
input_handler.get(), this,
base::FeatureList::IsEnabled(features::kTouchpadAndWheelScrollLatching));
base::FeatureList::IsEnabled(features::kTouchpadAndWheelScrollLatching),
base::FeatureList::IsEnabled(features::kAsyncWheelEvents));
input_handler_proxy_->set_smooth_scroll_enabled(smooth_scroll_enabled);
}

Expand Down
18 changes: 16 additions & 2 deletions ui/events/blink/input_handler_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ namespace ui {
InputHandlerProxy::InputHandlerProxy(
cc::InputHandler* input_handler,
InputHandlerProxyClient* client,
bool touchpad_and_wheel_scroll_latching_enabled)
bool touchpad_and_wheel_scroll_latching_enabled,
bool async_wheel_events_enabled)
: client_(client),
input_handler_(input_handler),
synchronous_input_handler_(nullptr),
Expand All @@ -153,10 +154,13 @@ InputHandlerProxy::InputHandlerProxy(
smooth_scroll_enabled_(false),
touchpad_and_wheel_scroll_latching_enabled_(
touchpad_and_wheel_scroll_latching_enabled),
async_wheel_events_enabled_(touchpad_and_wheel_scroll_latching_enabled &&
async_wheel_events_enabled),
touch_result_(kEventDispositionUndefined),
mouse_wheel_result_(kEventDispositionUndefined),
current_overscroll_params_(nullptr),
has_ongoing_compositor_scroll_fling_pinch_(false),
is_first_gesture_scroll_update_(false),
tick_clock_(std::make_unique<base::DefaultTickClock>()) {
DCHECK(client);
input_handler_->BindToClient(this,
Expand Down Expand Up @@ -215,8 +219,17 @@ void InputHandlerProxy::HandleInputEventWithLatencyInfo(
bool is_scroll_end_from_wheel =
gesture_event.source_device == blink::kWebGestureDeviceTouchpad &&
gesture_event.GetType() == blink::WebGestureEvent::kGestureScrollEnd;
bool scroll_update_has_blocking_wheel_source =
gesture_event.source_device == blink::kWebGestureDeviceTouchpad &&
gesture_event.GetType() ==
blink::WebGestureEvent::kGestureScrollUpdate &&
(!async_wheel_events_enabled_ || is_first_gesture_scroll_update_);
if (gesture_event.GetType() ==
blink::WebGestureEvent::kGestureScrollUpdate) {
is_first_gesture_scroll_update_ = false;
}
if (is_from_set_non_blocking_touch || is_scroll_end_from_wheel ||
synchronous_input_handler_) {
scroll_update_has_blocking_wheel_source || synchronous_input_handler_) {
// 1. Gesture events was already delayed by blocking events in rAF aligned
// queue. We want to avoid additional one frame delay by flushing the
// VSync queue immediately.
Expand Down Expand Up @@ -285,6 +298,7 @@ void InputHandlerProxy::DispatchSingleInputEvent(

switch (event_with_callback->event().GetType()) {
case blink::WebGestureEvent::kGestureScrollBegin:
is_first_gesture_scroll_update_ = true;
case blink::WebGestureEvent::kGestureFlingStart:
case blink::WebGestureEvent::kGesturePinchBegin:
case blink::WebGestureEvent::kGestureScrollUpdate:
Expand Down
5 changes: 4 additions & 1 deletion ui/events/blink/input_handler_proxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ class InputHandlerProxy : public cc::InputHandlerClient,
public:
InputHandlerProxy(cc::InputHandler* input_handler,
InputHandlerProxyClient* client,
bool touchpad_and_wheel_scroll_latching_enabled);
bool touchpad_and_wheel_scroll_latching_enabled,
bool async_wheel_events_enabled);
~InputHandlerProxy() override;

InputScrollElasticityController* scroll_elasticity_controller() {
Expand Down Expand Up @@ -233,6 +234,7 @@ class InputHandlerProxy : public cc::InputHandlerClient,

bool smooth_scroll_enabled_;
const bool touchpad_and_wheel_scroll_latching_enabled_;
const bool async_wheel_events_enabled_;

// The merged result of the last touch event with previous touch events.
// This value will get returned for subsequent TouchMove events to allow
Expand All @@ -253,6 +255,7 @@ class InputHandlerProxy : public cc::InputHandlerClient,

std::unique_ptr<CompositorThreadEventQueue> compositor_event_queue_;
bool has_ongoing_compositor_scroll_fling_pinch_;
bool is_first_gesture_scroll_update_;

std::unique_ptr<base::TickClock> tick_clock_;

Expand Down
62 changes: 45 additions & 17 deletions ui/events/blink/input_handler_proxy_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -358,10 +358,12 @@ class TestInputHandlerProxy : public InputHandlerProxy {
public:
TestInputHandlerProxy(cc::InputHandler* input_handler,
InputHandlerProxyClient* client,
bool touchpad_and_wheel_scroll_latching_enabled)
bool touchpad_and_wheel_scroll_latching_enabled,
bool async_wheel_events_enabled)
: InputHandlerProxy(input_handler,
client,
touchpad_and_wheel_scroll_latching_enabled) {}
touchpad_and_wheel_scroll_latching_enabled,
async_wheel_events_enabled) {}
void RecordMainThreadScrollingReasonsForTest(blink::WebGestureDevice device,
uint32_t reasons) {
RecordMainThreadScrollingReasons(device, reasons);
Expand All @@ -380,17 +382,20 @@ class InputHandlerProxyTest
: public testing::Test,
public testing::WithParamInterface<InputHandlerProxyTestType> {
public:
InputHandlerProxyTest(bool touchpad_and_wheel_scroll_latching_enabled = true)
InputHandlerProxyTest(bool touchpad_and_wheel_scroll_latching_enabled = true,
bool async_wheel_events_enabled = true)
: synchronous_root_scroll_(GetParam() == ROOT_SCROLL_SYNCHRONOUS_HANDLER),
install_synchronous_handler_(
GetParam() == ROOT_SCROLL_SYNCHRONOUS_HANDLER ||
GetParam() == CHILD_SCROLL_SYNCHRONOUS_HANDLER),
expected_disposition_(InputHandlerProxy::DID_HANDLE),
touchpad_and_wheel_scroll_latching_enabled_(
touchpad_and_wheel_scroll_latching_enabled) {
touchpad_and_wheel_scroll_latching_enabled),
async_wheel_events_enabled_(async_wheel_events_enabled) {
input_handler_.reset(
new TestInputHandlerProxy(&mock_input_handler_, &mock_client_,
touchpad_and_wheel_scroll_latching_enabled_));
touchpad_and_wheel_scroll_latching_enabled_,
async_wheel_events_enabled_));
scroll_result_did_scroll_.did_scroll = true;
scroll_result_did_not_scroll_.did_scroll = false;

Expand Down Expand Up @@ -508,13 +513,14 @@ class InputHandlerProxyTest
cc::InputHandlerScrollResult scroll_result_did_scroll_;
cc::InputHandlerScrollResult scroll_result_did_not_scroll_;
bool touchpad_and_wheel_scroll_latching_enabled_;
bool async_wheel_events_enabled_;
};

class InputHandlerProxyWithoutWheelScrollLatchingTest
: public InputHandlerProxyTest {
public:
InputHandlerProxyWithoutWheelScrollLatchingTest()
: InputHandlerProxyTest(false) {}
: InputHandlerProxyTest(false, false) {}
};

class InputHandlerProxyEventQueueTest : public testing::TestWithParam<bool> {
Expand All @@ -527,10 +533,12 @@ class InputHandlerProxyEventQueueTest : public testing::TestWithParam<bool> {

void SetUp() override {
bool wheel_scroll_latching_enabled = GetParam();
async_wheel_events_enabled_ = wheel_scroll_latching_enabled;
event_disposition_recorder_.clear();
latency_info_recorder_.clear();
input_handler_proxy_ = std::make_unique<TestInputHandlerProxy>(
&mock_input_handler_, &mock_client_, wheel_scroll_latching_enabled);
&mock_input_handler_, &mock_client_, wheel_scroll_latching_enabled,
async_wheel_events_enabled_);
if (input_handler_proxy_->compositor_event_queue_)
input_handler_proxy_->compositor_event_queue_ =
std::make_unique<CompositorThreadEventQueue>();
Expand Down Expand Up @@ -610,6 +618,7 @@ class InputHandlerProxyEventQueueTest : public testing::TestWithParam<bool> {
testing::StrictMock<MockInputHandlerProxyClient> mock_client_;
std::vector<InputHandlerProxy::EventDisposition> event_disposition_recorder_;
std::vector<ui::LatencyInfo> latency_info_recorder_;
bool async_wheel_events_enabled_;

base::MessageLoop loop_;
base::WeakPtrFactory<InputHandlerProxyEventQueueTest> weak_ptr_factory_;
Expand Down Expand Up @@ -2602,7 +2611,8 @@ TEST_P(InputHandlerProxyTest, DidReceiveInputEvent_ForFlingTouchscreen) {
mock_client;
input_handler_.reset(
new TestInputHandlerProxy(&mock_input_handler_, &mock_client,
touchpad_and_wheel_scroll_latching_enabled_));
touchpad_and_wheel_scroll_latching_enabled_,
async_wheel_events_enabled_));
if (install_synchronous_handler_) {
EXPECT_CALL(mock_input_handler_, RequestUpdateForSynchronousInputHandler())
.Times(1);
Expand Down Expand Up @@ -2644,7 +2654,7 @@ TEST(SynchronousInputHandlerProxyTest, StartupShutdown) {
testing::StrictMock<MockInputHandlerProxyClient> mock_client;
testing::StrictMock<MockSynchronousInputHandler>
mock_synchronous_input_handler;
ui::InputHandlerProxy proxy(&mock_input_handler, &mock_client, false);
ui::InputHandlerProxy proxy(&mock_input_handler, &mock_client, false, false);

// When adding a SynchronousInputHandler, immediately request an
// UpdateRootLayerStateForSynchronousInputHandler() call.
Expand All @@ -2670,7 +2680,7 @@ TEST(SynchronousInputHandlerProxyTest, UpdateRootLayerState) {
testing::StrictMock<MockInputHandlerProxyClient> mock_client;
testing::StrictMock<MockSynchronousInputHandler>
mock_synchronous_input_handler;
ui::InputHandlerProxy proxy(&mock_input_handler, &mock_client, false);
ui::InputHandlerProxy proxy(&mock_input_handler, &mock_client, false, false);

proxy.SetOnlySynchronouslyAnimateRootFlings(&mock_synchronous_input_handler);

Expand All @@ -2695,7 +2705,7 @@ TEST(SynchronousInputHandlerProxyTest, SetOffset) {
testing::StrictMock<MockInputHandlerProxyClient> mock_client;
testing::StrictMock<MockSynchronousInputHandler>
mock_synchronous_input_handler;
ui::InputHandlerProxy proxy(&mock_input_handler, &mock_client, false);
ui::InputHandlerProxy proxy(&mock_input_handler, &mock_client, false, false);

proxy.SetOnlySynchronouslyAnimateRootFlings(&mock_synchronous_input_handler);

Expand Down Expand Up @@ -3324,8 +3334,6 @@ TEST_P(InputHandlerProxyEventQueueTest, TouchpadGestureScrollEndFlushQueue) {

EXPECT_CALL(mock_input_handler_, ScrollBegin(testing::_, testing::_))
.WillRepeatedly(testing::Return(kImplThreadScrollState));
EXPECT_CALL(mock_input_handler_, SetNeedsAnimateInput())
.Times(::testing::AtLeast(1));
EXPECT_CALL(
mock_input_handler_,
ScrollBy(testing::Property(&cc::ScrollState::delta_y, testing::Gt(0))))
Expand All @@ -3339,16 +3347,36 @@ TEST_P(InputHandlerProxyEventQueueTest, TouchpadGestureScrollEndFlushQueue) {
HandleGestureEventWithSourceDevice(WebInputEvent::kGestureScrollUpdate,
blink::kWebGestureDeviceTouchpad, -20);

// GSB will be dispatched immediately, GSU will be queued.
EXPECT_EQ(1ul, event_queue().size());
EXPECT_EQ(1ul, event_disposition_recorder_.size());
// Both GSB and the first GSU will be dispatched immediately since the first
// GSU has blocking wheel event source.
EXPECT_EQ(0ul, event_queue().size());
EXPECT_EQ(2ul, event_disposition_recorder_.size());

// When async_wheel_events_enabled_ the rest of the GSU events will get queued
// since they have non-blocking wheel event source.
if (async_wheel_events_enabled_) {
EXPECT_CALL(mock_input_handler_, SetNeedsAnimateInput())
.Times(::testing::AtLeast(1));
HandleGestureEventWithSourceDevice(WebInputEvent::kGestureScrollUpdate,
blink::kWebGestureDeviceTouchpad, -20);
EXPECT_EQ(1ul, event_queue().size());
EXPECT_EQ(2ul, event_disposition_recorder_.size());
}

// Touchpad GSE will flush the queue.
HandleGestureEventWithSourceDevice(WebInputEvent::kGestureScrollEnd,
blink::kWebGestureDeviceTouchpad);

EXPECT_EQ(0ul, event_queue().size());
EXPECT_EQ(3ul, event_disposition_recorder_.size());
if (async_wheel_events_enabled_) {
// GSB, GSU(with blocking wheel source), GSU(with non-blocking wheel
// source), and GSE are the sent events.
EXPECT_EQ(4ul, event_disposition_recorder_.size());
} else {
// GSB, GSU(with blocking wheel source), and GSE are the sent events.
EXPECT_EQ(3ul, event_disposition_recorder_.size());
}

EXPECT_FALSE(
input_handler_proxy_->gesture_scroll_on_impl_thread_for_testing());
}
Expand Down

0 comments on commit ff58622

Please sign in to comment.