Touchpad scroll shouldn't interfere with timer-based wheel scrolling.

On ChromeOS there are two different ui events for wheel scrolling:
ui:ET_SCROLL is the event type when the user scrolls with the touchpad
and ui::ET_MOUSEWHEEL is the event type when the user scrolls using an
external mouse wheel device. Wheel scroll latching works differently for
these two types of wheel scrolling:
For touchpad scrolling the sequence starts when the user touches the
touchpad (on a GFC) and ends when the user lifts their fingers (on a
GFS); While for wheel scrolling the sequence starts with the first wheel
event and ends when no wheel event has arrived for the past 500ms.

We always receive a GFC when the user puts their finger down on the
touchpad, but we receive a GFS only at the end of a scroll. So without this
cl if the user lifts their fingers from touchpad without scrolling,
scrolling with mouse wheel will get stuck to the same scrolling sequence
till the next time that the user touches the touchpad again.

This cl makes sure that these two logic for latching don't interfere
with each other: On arrival of a ui::ET_MOUSEWHEEL we reset the
touchpad scrolling state and when the user puts their finger on the
touchpad we end the timer-based latching sequence before setting the
touchpad_scroll_phase_state_.

Bug: 838457
Test: *.ScrollingWithExternalMouseBreaksTouchpadScrollLatching
Change-Id: Id5466e5645f08e6bf44c00f4471dbb3bbf9afc89
Reviewed-on: https://chromium-review.googlesource.com/1052853
Commit-Queue: Sahel Sharifymoghaddam <sahel@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557912}

content/browser/renderer_host/input/mouse_wheel_phase_handler.cc

@@ -14,7 +14,7 @@ MouseWheelPhaseHandler::MouseWheelPhaseHandler(
     RenderWidgetHostViewBase* const host_view)
     : host_view_(host_view),
       mouse_wheel_end_dispatch_timeout_(kDefaultMouseWheelLatchingTransaction),
-      scroll_phase_state_(SCROLL_STATE_UNKNOWN) {}
+      touchpad_scroll_phase_state_(TOUCHPAD_SCROLL_STATE_UNKNOWN) {}
 
 void MouseWheelPhaseHandler::AddPhaseIfNeededAndScheduleEndEvent(
     blink::WebMouseWheelEvent& mouse_wheel_event,
@@ -43,8 +43,8 @@ void MouseWheelPhaseHandler::AddPhaseIfNeededAndScheduleEndEvent(
       IgnorePendingWheelEndEvent();
     }
   } else {  // !has_phase
-    switch (scroll_phase_state_) {
-      case SCROLL_STATE_UNKNOWN: {
+    switch (touchpad_scroll_phase_state_) {
+      case TOUCHPAD_SCROLL_STATE_UNKNOWN: {
         mouse_wheel_event.has_synthetic_phase = true;
         // Break the latching when the location difference between the current
         // and the initial wheel event positions exceeds the maximum allowed
@@ -75,11 +75,11 @@ void MouseWheelPhaseHandler::AddPhaseIfNeededAndScheduleEndEvent(
         }
         break;
       }
-      case SCROLL_MAY_BEGIN:
+      case TOUCHPAD_SCROLL_MAY_BEGIN:
         mouse_wheel_event.phase = blink::WebMouseWheelEvent::kPhaseBegan;
-        scroll_phase_state_ = SCROLL_IN_PROGRESS;
+        touchpad_scroll_phase_state_ = TOUCHPAD_SCROLL_IN_PROGRESS;
         break;
-      case SCROLL_IN_PROGRESS:
+      case TOUCHPAD_SCROLL_IN_PROGRESS:
         mouse_wheel_event.phase = blink::WebMouseWheelEvent::kPhaseChanged;
         break;
       default:
@@ -103,15 +103,15 @@ void MouseWheelPhaseHandler::IgnorePendingWheelEndEvent() {
   mouse_wheel_end_dispatch_timer_.Stop();
 }
 
-void MouseWheelPhaseHandler::ResetScrollSequence() {
-  scroll_phase_state_ = SCROLL_STATE_UNKNOWN;
+void MouseWheelPhaseHandler::ResetTouchpadScrollSequence() {
+  touchpad_scroll_phase_state_ = TOUCHPAD_SCROLL_STATE_UNKNOWN;
 }
 
-void MouseWheelPhaseHandler::SendWheelEndIfNeeded() {
-  if (scroll_phase_state_ == SCROLL_IN_PROGRESS) {
+void MouseWheelPhaseHandler::SendWheelEndForTouchpadScrollingIfNeeded() {
+  if (touchpad_scroll_phase_state_ == TOUCHPAD_SCROLL_IN_PROGRESS) {
     RenderWidgetHostImpl* widget_host = host_view_->host();
     if (!widget_host) {
-      ResetScrollSequence();
+      ResetTouchpadScrollSequence();
       return;
     }
 
@@ -120,11 +120,18 @@ void MouseWheelPhaseHandler::SendWheelEndIfNeeded() {
     SendSyntheticWheelEventWithPhaseEnded(should_route_event);
   }
 
-  ResetScrollSequence();
+  ResetTouchpadScrollSequence();
 }
 
-void MouseWheelPhaseHandler::ScrollingMayBegin() {
-  scroll_phase_state_ = SCROLL_MAY_BEGIN;
+void MouseWheelPhaseHandler::TouchpadScrollingMayBegin() {
+  // End the timer-based wheel scroll sequence before starting a touchpad scroll
+  // sequence.
+  if (mouse_wheel_end_dispatch_timer_.IsRunning()) {
+    DCHECK_EQ(TOUCHPAD_SCROLL_STATE_UNKNOWN, touchpad_scroll_phase_state_);
+    DispatchPendingWheelEndEvent();
+  }
+
+  touchpad_scroll_phase_state_ = TOUCHPAD_SCROLL_MAY_BEGIN;
 }
 
 void MouseWheelPhaseHandler::SendSyntheticWheelEventWithPhaseEnded(
@@ -168,7 +175,7 @@ bool MouseWheelPhaseHandler::IsWithinSlopRegion(
   // This function is called to check if breaking timer-based wheel scroll
   // latching sequence is needed or not, and timer-based wheel scroll latching
   // happens only when scroll state is unknown.
-  DCHECK(scroll_phase_state_ == SCROLL_STATE_UNKNOWN);
+  DCHECK(touchpad_scroll_phase_state_ == TOUCHPAD_SCROLL_STATE_UNKNOWN);
   gfx::Vector2dF current_wheel_location(wheel_event.PositionInWidget().x,
                                         wheel_event.PositionInWidget().y);
   return (current_wheel_location - first_wheel_location_).LengthSquared() <
@@ -180,7 +187,7 @@ bool MouseWheelPhaseHandler::HasDifferentModifiers(
   // This function is called to check if breaking timer-based wheel scroll
   // latching sequence is needed or not, and timer-based wheel scroll latching
   // happens only when scroll state is unknown.
-  DCHECK(scroll_phase_state_ == SCROLL_STATE_UNKNOWN);
+  DCHECK(touchpad_scroll_phase_state_ == TOUCHPAD_SCROLL_STATE_UNKNOWN);
   return wheel_event.GetModifiers() != initial_wheel_event_.GetModifiers();
 }
 
@@ -189,7 +196,7 @@ bool MouseWheelPhaseHandler::ShouldBreakLatchingDueToDirectionChange(
   // This function is called to check if breaking timer-based wheel scroll
   // latching sequence is needed or not, and timer-based wheel scroll latching
   // happens only when scroll state is unknown.
-  DCHECK(scroll_phase_state_ == SCROLL_STATE_UNKNOWN);
+  DCHECK(touchpad_scroll_phase_state_ == TOUCHPAD_SCROLL_STATE_UNKNOWN);
   if (first_scroll_update_ack_state_ != FirstScrollUpdateAckState::kNotConsumed)
     return false;
 

content/browser/renderer_host/input/mouse_wheel_phase_handler.h

@@ -29,15 +29,18 @@ constexpr base::TimeDelta kMaximumTimeBetweenPhaseEndedAndMomentumPhaseBegan =
 const double kWheelLatchingSlopRegion = 10.0;
 
 // On ChromeOS wheel events don't have phase information; However, whenever the
-// user puts down or lifts their fingers a GFC or GFS is received.
-enum ScrollPhaseState {
+// user puts down their fingers on touchpad a GFC is received and at the end of
+// touchpad scrolling when the user lifts their fingers a GFS is received. This
+// enum tracks the current state of the touchpad scrolling by listening to GFC
+// and GFS events.
+enum TouchpadScrollPhaseState {
   // Scrolling with normal mouse wheels doesn't give any information about the
   // state of scrolling.
-  SCROLL_STATE_UNKNOWN = 0,
+  TOUCHPAD_SCROLL_STATE_UNKNOWN = 0,
   // Shows that the user has put their fingers down and a scroll may start.
-  SCROLL_MAY_BEGIN,
+  TOUCHPAD_SCROLL_MAY_BEGIN,
   // Scrolling has started and the user hasn't lift their fingers, yet.
-  SCROLL_IN_PROGRESS,
+  TOUCHPAD_SCROLL_IN_PROGRESS,
 };
 
 enum class FirstScrollUpdateAckState {
@@ -49,6 +52,9 @@ enum class FirstScrollUpdateAckState {
   kNotConsumed,
 };
 
+// The MouseWheelPhaseHandler is responsible for adding the proper phase to
+// wheel events. Phase information is necessary for wheel scrolling since it
+// shows the start and end of a scrolling sequence.
 class MouseWheelPhaseHandler {
  public:
   MouseWheelPhaseHandler(RenderWidgetHostViewBase* const host_view);
@@ -59,9 +65,9 @@ class MouseWheelPhaseHandler {
       bool should_route_event);
   void DispatchPendingWheelEndEvent();
   void IgnorePendingWheelEndEvent();
-  void ResetScrollSequence();
-  void SendWheelEndIfNeeded();
-  void ScrollingMayBegin();
+  void ResetTouchpadScrollSequence();
+  void SendWheelEndForTouchpadScrollingIfNeeded();
+  void TouchpadScrollingMayBegin();
 
   // Used to set the timer timeout for testing.
   void set_mouse_wheel_end_dispatch_timeout(base::TimeDelta timeout) {
@@ -89,7 +95,7 @@ class MouseWheelPhaseHandler {
   base::OneShotTimer mouse_wheel_end_dispatch_timer_;
   base::TimeDelta mouse_wheel_end_dispatch_timeout_;
   blink::WebMouseWheelEvent last_mouse_wheel_event_;
-  ScrollPhaseState scroll_phase_state_;
+  TouchpadScrollPhaseState touchpad_scroll_phase_state_;
   // This is used to break the timer based latching when the difference between
   // the locations of the first wheel event and the current wheel event is
   // larger than some threshold. The variable value is only valid while the

content/browser/renderer_host/render_widget_host_view_aura_unittest.cc

@@ -645,6 +645,10 @@ class RenderWidgetHostViewAuraTest : public testing::Test {
     return delegates_.back().get();
   }
 
+  MouseWheelPhaseHandler* GetMouseWheelPhaseHandler() const {
+    return view_->GetMouseWheelPhaseHandler();
+  }
+
   // Sets the |view| active in TextInputManager with the given |type|. |type|
   // cannot be ui::TEXT_INPUT_TYPE_NONE.
   // Must not be called in the destruction path of |view|.
@@ -2493,6 +2497,81 @@ TEST_F(RenderWidgetHostViewAuraAsyncWheelEventsEnabledTest,
   TouchpadFlingStartResetsWheelPhaseState();
 }
 
+TEST_F(RenderWidgetHostViewAuraAsyncWheelEventsEnabledTest,
+       ScrollingWithExternalMouseBreaksTouchpadScrollLatching) {
+  // The test is valid only when wheel scroll latching is enabled.
+  if (wheel_scrolling_mode_ == kWheelScrollingModeNone)
+    return;
+
+  // Set the mouse_wheel_phase_handler_ timer timeout to a large value to make
+  // sure that the timer is still running when we are checking for the pending
+  // wheel end event after sending ui::MouseWheelEvent.
+  view_->event_handler()->set_mouse_wheel_wheel_phase_handler_timeout(
+      TestTimeouts::action_max_timeout());
+
+  view_->InitAsChild(nullptr);
+  view_->Show();
+  sink_->ClearMessages();
+
+  // When the user puts their fingers down a GFC is receieved.
+  ui::ScrollEvent fling_cancel(ui::ET_SCROLL_FLING_CANCEL, gfx::Point(2, 2),
+                               ui::EventTimeForNow(), 0, 0, 0, 0, 0, 2);
+  view_->OnScrollEvent(&fling_cancel);
+
+  // Start touchpad scrolling by sending a ui::ET_SCROLL event.
+  ui::ScrollEvent scroll0(ui::ET_SCROLL, gfx::Point(2, 2),
+                          ui::EventTimeForNow(), 0, 0, 5, 0, 5, 2);
+  view_->OnScrollEvent(&scroll0);
+  base::RunLoop().RunUntilIdle();
+  MockWidgetInputHandler::MessageVector events =
+      GetAndResetDispatchedMessages();
+
+  const WebMouseWheelEvent* wheel_event =
+      static_cast<const WebMouseWheelEvent*>(
+          events[0]->ToEvent()->Event()->web_event.get());
+  EXPECT_EQ("MouseWheel", GetMessageNames(events));
+  EXPECT_EQ(WebMouseWheelEvent::kPhaseBegan, wheel_event->phase);
+  events[0]->ToEvent()->CallCallback(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+
+  // The mouse_wheel_phase_handler's timer won't be running during touchpad
+  // scroll.
+  EXPECT_FALSE(GetMouseWheelPhaseHandler()->HasPendingWheelEndEvent());
+
+  // ACK the GSB and GSU events generated from the first touchpad wheel event.
+  events = GetAndResetDispatchedMessages();
+  EXPECT_EQ("GestureScrollBegin GestureScrollUpdate", GetMessageNames(events));
+  const WebGestureEvent* gesture_event = static_cast<const WebGestureEvent*>(
+      events[0]->ToEvent()->Event()->web_event.get());
+  EXPECT_EQ(WebInputEvent::kGestureScrollBegin, gesture_event->GetType());
+  events[0]->ToEvent()->CallCallback(INPUT_EVENT_ACK_STATE_CONSUMED);
+  gesture_event = static_cast<const WebGestureEvent*>(
+      events[1]->ToEvent()->Event()->web_event.get());
+  EXPECT_EQ(WebInputEvent::kGestureScrollUpdate, gesture_event->GetType());
+  events[1]->ToEvent()->CallCallback(INPUT_EVENT_ACK_STATE_CONSUMED);
+
+  // Start mouse wheel scrolling by sending a ui::ET_MOUSEWHEEL event. This
+  // should end the touchpad scrolling sequence and start a new timer-based
+  // wheel scrolling sequence.
+  ui::MouseWheelEvent wheel(gfx::Vector2d(0, 5), gfx::Point(2, 2),
+                            gfx::Point(2, 2), ui::EventTimeForNow(), 0, 0);
+  view_->OnMouseEvent(&wheel);
+  base::RunLoop().RunUntilIdle();
+  events = GetAndResetDispatchedMessages();
+  EXPECT_EQ("MouseWheel GestureScrollEnd MouseWheel", GetMessageNames(events));
+  EXPECT_TRUE(events[0]->ToEvent());
+  wheel_event = static_cast<const WebMouseWheelEvent*>(
+      events[0]->ToEvent()->Event()->web_event.get());
+  EXPECT_EQ(WebMouseWheelEvent::kPhaseEnded, wheel_event->phase);
+  EXPECT_TRUE(events[2]->ToEvent());
+  wheel_event = static_cast<const WebMouseWheelEvent*>(
+      events[2]->ToEvent()->Event()->web_event.get());
+  EXPECT_EQ(WebMouseWheelEvent::kPhaseBegan, wheel_event->phase);
+
+  // The mouse_wheel_phase_handler's timer will be running during mouse wheel
+  // scroll.
+  EXPECT_TRUE(GetMouseWheelPhaseHandler()->HasPendingWheelEndEvent());
+}
+
 void RenderWidgetHostViewAuraTest::
     GSBWithTouchSourceStopsWheelScrollSequence() {
   // The test is valid only when wheel scroll latching is enabled.

content/browser/renderer_host/render_widget_host_view_event_handler.cc

@@ -368,9 +368,14 @@ void RenderWidgetHostViewEventHandler::OnMouseEvent(ui::MouseEvent* event) {
 
     if (mouse_wheel_event.delta_x != 0 || mouse_wheel_event.delta_y != 0) {
       bool should_route_event = ShouldRouteEvent(event);
-      if (host_view_->wheel_scroll_latching_enabled())
+      if (host_view_->wheel_scroll_latching_enabled()) {
+        // End the touchpad scrolling sequence (if such exists) before handling
+        // a ui::ET_MOUSEWHEEL event.
+        mouse_wheel_phase_handler_.SendWheelEndForTouchpadScrollingIfNeeded();
+
         mouse_wheel_phase_handler_.AddPhaseIfNeededAndScheduleEndEvent(
             mouse_wheel_event, should_route_event);
+      }
       if (should_route_event) {
         host_->delegate()->GetInputEventRouter()->RouteMouseWheelEvent(
             host_view_, &mouse_wheel_event, *event->latency());
@@ -466,11 +471,11 @@ void RenderWidgetHostViewEventHandler::OnScrollEvent(ui::ScrollEvent* event) {
     if (event->type() == ui::ET_SCROLL_FLING_START) {
       RecordAction(base::UserMetricsAction("TrackpadScrollFling"));
       // The user has lifted their fingers.
-      mouse_wheel_phase_handler_.ResetScrollSequence();
+      mouse_wheel_phase_handler_.ResetTouchpadScrollSequence();
     } else if (event->type() == ui::ET_SCROLL_FLING_CANCEL) {
       // The user has put their fingers down.
       DCHECK_EQ(blink::kWebGestureDeviceTouchpad, gesture_event.SourceDevice());
-      mouse_wheel_phase_handler_.ScrollingMayBegin();
+      mouse_wheel_phase_handler_.TouchpadScrollingMayBegin();
     }
   }
 
@@ -568,7 +573,7 @@ void RenderWidgetHostViewEventHandler::OnGestureEvent(ui::GestureEvent* event) {
       // wheel based send a synthetic wheel event with kPhaseEnded to cancel
       // the current scroll.
       mouse_wheel_phase_handler_.DispatchPendingWheelEndEvent();
-      mouse_wheel_phase_handler_.SendWheelEndIfNeeded();
+      mouse_wheel_phase_handler_.SendWheelEndForTouchpadScrollingIfNeeded();
     } else if (event->type() == ui::ET_SCROLL_FLING_START) {
       RecordAction(base::UserMetricsAction("TouchscreenScrollFling"));
     }
@@ -580,7 +585,7 @@ void RenderWidgetHostViewEventHandler::OnGestureEvent(ui::GestureEvent* event) {
       // correct phase info when some of the wheel events get ignored while a
       // touchscreen scroll is going on.
       mouse_wheel_phase_handler_.IgnorePendingWheelEndEvent();
-      mouse_wheel_phase_handler_.ResetScrollSequence();
+      mouse_wheel_phase_handler_.ResetTouchpadScrollSequence();
     }
 
     if (ShouldRouteEvent(event)) {