Fix for rapidly clicking on composited scrollbar.

A "mouse click" is comprised of a mousedown and a mouseup. During a
mousedown on a scrollbar arrow, a GSB and a GSU are queued up in the
CompositorThreadEventQueue. On mouseup, a GSE gets added to the queue.
These gesture events are VSync aligned. However, if the user is rapidly
clicking on the arrow, we will get another mousedown before the queued
up GSE (from the first mouse click) has had a chance to be processed.
This new mousedown now ends up calling InputHandlerScrollEnd and that
in turn sets handling_gesture_on_impl_thread_ to false. When the queued
up GSE from the first mouse click finally gets dispatched, it notices
that handling_gesture_on_impl_thread_ is false and hence, leads to
DCHECK(!currently_active_gesture_device_.has_value()) failing. This CL
fixes the bug by clearing currently_active_gesture_device_ when the
ongoing scroll ends.

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

third_party/blink/renderer/platform/widget/input/input_handler_proxy.cc

@@ -1020,6 +1020,10 @@ InputHandlerProxy::HandleGestureScrollUpdate(
   return scroll_result.did_scroll ? DID_HANDLE : DROP_EVENT;
 }
 
+// TODO(arakeri): Ensure that redudant GSE(s) in the CompositorThreadEventQueue
+// are handled gracefully. (i.e currently, when an ongoing scroll needs to end,
+// we call RecordScrollEnd and InputHandlerScrollEnd synchronously. Ideally, we
+// should end the scroll when the GSB is being handled).
 InputHandlerProxy::EventDisposition InputHandlerProxy::HandleGestureScrollEnd(
     const WebGestureEvent& gesture_event) {
   TRACE_EVENT0("input", "InputHandlerProxy::HandleGestureScrollEnd");
@@ -1053,6 +1057,9 @@ InputHandlerProxy::EventDisposition InputHandlerProxy::HandleGestureScrollEnd(
 void InputHandlerProxy::InputHandlerScrollEnd() {
   input_handler_->ScrollEnd(/*should_snap=*/true);
   handling_gesture_on_impl_thread_ = false;
+
+  DCHECK(!gesture_pinch_in_progress_);
+  currently_active_gesture_device_ = base::nullopt;
 }
 
 InputHandlerProxy::EventDisposition InputHandlerProxy::HitTestTouchEvent(

third_party/blink/renderer/platform/widget/input/input_handler_proxy_unittest.cc

@@ -554,7 +554,7 @@ class InputHandlerProxyEventQueueTest : public testing::Test {
 // Tests that changing source devices mid gesture scroll is handled gracefully.
 // For example, when a touch scroll is in progress and the user initiates a
 // scrollbar scroll before the touch scroll has had a chance to dispatch a GSE.
-TEST_P(InputHandlerProxyTest, NestedGestureBasedScrolls) {
+TEST_P(InputHandlerProxyTest, NestedGestureBasedScrollsDifferentSourceDevice) {
   // Touchpad initiates a scroll.
   EXPECT_CALL(mock_input_handler_, ScrollBegin(_, _))
       .WillOnce(testing::Return(kImplThreadScrollState));
@@ -1381,6 +1381,77 @@ TEST_P(InputHandlerProxyTest, HitTestTouchEventNonNullTouchAction) {
   VERIFY_AND_RESET_MOCKS();
 }
 
+// Tests that multiple mousedown(s) on scrollbar are handled gracefully and
+// don't fail any DCHECK(s).
+TEST_F(InputHandlerProxyEventQueueTest,
+       NestedGestureBasedScrollsSameSourceDevice) {
+  // Start with mousedown. Expect CompositorThreadEventQueue to contain [GSB,
+  // GSU].
+  EXPECT_CALL(mock_input_handler_, SetNeedsAnimateInput());
+  HandleMouseEvent(WebInputEvent::Type::kMouseDown);
+  EXPECT_EQ(2ul, event_queue().size());
+  EXPECT_EQ(event_queue()[0]->event().GetType(),
+            WebInputEvent::Type::kGestureScrollBegin);
+  EXPECT_EQ(event_queue()[1]->event().GetType(),
+            WebInputEvent::Type::kGestureScrollUpdate);
+
+  EXPECT_CALL(mock_input_handler_, ScrollBegin(_, _))
+      .WillOnce(Return(kImplThreadScrollState));
+  EXPECT_CALL(mock_input_handler_, ScrollingShouldSwitchtoMainThread())
+      .WillOnce(Return(false));
+  EXPECT_CALL(mock_input_handler_, ScrollUpdate(_, _)).Times(1);
+
+  DeliverInputForBeginFrame();
+  Mock::VerifyAndClearExpectations(&mock_input_handler_);
+
+  // A mouseup adds a GSE to the CompositorThreadEventQueue.
+  EXPECT_CALL(mock_input_handler_, SetNeedsAnimateInput());
+  HandleMouseEvent(WebInputEvent::Type::kMouseUp);
+  Mock::VerifyAndClearExpectations(&mock_input_handler_);
+
+  EXPECT_EQ(1ul, event_queue().size());
+  EXPECT_EQ(event_queue()[0]->event().GetType(),
+            WebInputEvent::Type::kGestureScrollEnd);
+
+  // Called when a mousedown is being handled as it tries to end the ongoing
+  // scroll.
+  EXPECT_CALL(mock_input_handler_, RecordScrollEnd(_)).Times(1);
+  EXPECT_CALL(mock_input_handler_, ScrollEnd(true)).Times(1);
+
+  // A mousedown occurs on the scrollbar *before* the GSE is dispatched.
+  HandleMouseEvent(WebInputEvent::Type::kMouseDown);
+  Mock::VerifyAndClearExpectations(&mock_input_handler_);
+
+  EXPECT_EQ(3ul, event_queue().size());
+  EXPECT_EQ(event_queue()[1]->event().GetType(),
+            WebInputEvent::Type::kGestureScrollBegin);
+  EXPECT_EQ(event_queue()[2]->event().GetType(),
+            WebInputEvent::Type::kGestureScrollUpdate);
+
+  // Called when the GSE is being handled. (Note that ScrollEnd isn't called
+  // when the GSE is being handled as the GSE gets dropped in
+  // HandleGestureScrollEnd because handling_gesture_on_impl_thread_ is false)
+  EXPECT_CALL(mock_input_handler_, RecordScrollEnd(_)).Times(1);
+  EXPECT_CALL(mock_input_handler_, ScrollBegin(_, _))
+      .WillOnce(Return(kImplThreadScrollState));
+  EXPECT_CALL(mock_input_handler_, ScrollingShouldSwitchtoMainThread())
+      .WillOnce(Return(false));
+  EXPECT_CALL(mock_input_handler_, ScrollUpdate(_, _)).Times(1);
+
+  DeliverInputForBeginFrame();
+  Mock::VerifyAndClearExpectations(&mock_input_handler_);
+
+  // Finally, a mouseup ends the scroll.
+  EXPECT_CALL(mock_input_handler_, SetNeedsAnimateInput());
+  HandleMouseEvent(WebInputEvent::Type::kMouseUp);
+
+  EXPECT_CALL(mock_input_handler_, RecordScrollEnd(_)).Times(1);
+  EXPECT_CALL(mock_input_handler_, ScrollEnd(true)).Times(1);
+
+  DeliverInputForBeginFrame();
+  Mock::VerifyAndClearExpectations(&mock_input_handler_);
+}
+
 // Tests that the whitelisted touch action is correctly set when a touch is
 // made non blocking due to an ongoing fling. https://crbug.com/1048098.
 TEST_F(InputHandlerProxyEventQueueTest, AckTouchActionNonBlockingForFling) {