Touchpad and wheel scroll latching for ChromeOS behind flag.

ui/events/blink/input_handler_proxy_unittest.cc

Index: ui/events/blink/input_handler_proxy_unittest.cc
diff --git a/ui/events/blink/input_handler_proxy_unittest.cc b/ui/events/blink/input_handler_proxy_unittest.cc
index 0b7a14b7f1f258c5e4be18b7ed319903e5248f3e..51a50ae36616b469fbabbebeffc7d50da3d34e3d 100644
--- a/ui/events/blink/input_handler_proxy_unittest.cc
+++ b/ui/events/blink/input_handler_proxy_unittest.cc
@@ -454,6 +454,10 @@ class InputHandlerProxyTest
     input_handler_->smooth_scroll_enabled_ = value;
   }
 
+  void SetTouchpadAndWheelScrollLatchingEnabled(bool value) {
+    input_handler_->touchpad_and_wheel_scroll_latching_enabled_ = value;
+  }
+
   base::HistogramTester& histogram_tester() {
     return histogram_tester_;
   }
@@ -918,6 +922,107 @@ TEST_P(InputHandlerProxyTest, GestureFlingStartedTouchpad) {
   EXPECT_EQ(expected_disposition_, input_handler_->HandleInputEvent(gesture_));
 }
 
+TEST_P(InputHandlerProxyTest, GestureFlingTouchpadScrollLatchingEnabled) {
+  SetTouchpadAndWheelScrollLatchingEnabled(true);
+  // We shouldn't send any events to the widget for this gesture.
+  expected_disposition_ = InputHandlerProxy::DID_HANDLE;
+  VERIFY_AND_RESET_MOCKS();
+
+  EXPECT_CALL(mock_input_handler_, ScrollBegin(::testing::_, ::testing::_))
+      .WillOnce(testing::Return(kImplThreadScrollState));
+
+  // HandleGestureScrollBegin will set gesture_scroll_on_impl_thread_.
+  gesture_.setType(WebInputEvent::GestureScrollBegin);
+  EXPECT_EQ(expected_disposition_, input_handler_->HandleInputEvent(gesture_));
+  EXPECT_TRUE(input_handler_->gesture_scroll_on_impl_thread_for_testing());
+
+  VERIFY_AND_RESET_MOCKS();
+
+  // On the fling start, we should schedule an animation but not actually start
+  // scrolling.
+  gesture_.setType(WebInputEvent::GestureFlingStart);
+  WebFloatPoint fling_delta = WebFloatPoint(1000, 0);
+  WebPoint fling_point = WebPoint(7, 13);
+  WebPoint fling_global_point = WebPoint(17, 23);
+  // Note that for trackpad, wheel events with the Control modifier are
+  // special (reserved for zoom), so don't set that here.
+  int modifiers = WebInputEvent::ShiftKey | WebInputEvent::AltKey;
+  gesture_ = CreateFling(blink::WebGestureDeviceTouchpad, fling_delta,
+                         fling_point, fling_global_point, modifiers);
+  EXPECT_SET_NEEDS_ANIMATE_INPUT(1);
+  EXPECT_CALL(mock_input_handler_, ScrollBegin(testing::_, testing::_))
+      .WillOnce(testing::Return(kImplThreadScrollState));
+
+  // When scroll latching is enabled, ScrollEnd shouldn't get called while
+  // handling GestureFlingStart.
+  EXPECT_CALL(mock_input_handler_, ScrollEnd(testing::_)).Times(0);
+  EXPECT_EQ(expected_disposition_, input_handler_->HandleInputEvent(gesture_));
+
+  VERIFY_AND_RESET_MOCKS();
+
+  // The first animate call should let us pick up an animation start time, but
+  // we shouldn't actually move anywhere just yet. The first frame after the
+  // fling start will typically include the last scroll from the gesture that
+  // lead to the scroll (either wheel or gesture scroll), so there should be no
+  // visible hitch.
+  EXPECT_SET_NEEDS_ANIMATE_INPUT(1);
+  EXPECT_CALL(mock_input_handler_,
+              GetEventListenerProperties(cc::EventListenerClass::kMouseWheel))

[bokan, 2017/01/10 18:53:59]

I don't think we need this check. It seems to be over-specifying the behavior
and isn't really what we're testing here. That is, if we changed the behavior so
that the first Animate call does actually scroll, that wouldn't be breaking the
latching behavior you're introducing.

[sahel, 2017/01/10 21:10:06]

Done.

+      .Times(0);
+  base::TimeTicks time = base::TimeTicks() + base::TimeDelta::FromSeconds(10);
+  Animate(time);
+
+  VERIFY_AND_RESET_MOCKS();
+
+  // The second call should start scrolling in the -X direction.
+  EXPECT_SET_NEEDS_ANIMATE_INPUT(1);
+  EXPECT_CALL(mock_input_handler_,
+              GetEventListenerProperties(cc::EventListenerClass::kMouseWheel))
+      .WillOnce(testing::Return(cc::EventListenerProperties::kNone));

[bokan, 2017/01/10 18:53:58]

Also don't need this, I think. The ScrollBy below should be enough.

[sahel, 2017/01/10 21:10:06]

I wanted to show that while ScrollBy in FlingScrollByMouseWheel returns true,
the next Animate will (indirectly) call the FlingScrollByMouseWheel and once the
ScrollBy result is false, the next Animate won't cause FlingScrollByMouseWheel
to get called anymore.

I can get rid of it, if it's not needed.

[bokan, 2017/01/10 21:25:06]

I guess I'm confused, how does GetEventListenerProperties relate to all this?

[sahel, 2017/01/12 18:53:44]

I want to check whether FlingScrollByMouseWheel is called or not. I cannot MOCK
the function, because the change for latching is done in the body of this
function. Therefore, I check GetEventListenerProperties which is called in this
function to indirectly check if the FlingScrollByMouseWheel is called or not.

+
+  // When scroll latching is enabled, ScrollBegin shouldn't get called for
+  // every tick.
+  EXPECT_CALL(mock_input_handler_, ScrollBegin(testing::_, testing::_))
+      .Times(0);
+  EXPECT_CALL(
+      mock_input_handler_,
+      ScrollBy(testing::Property(&cc::ScrollState::delta_x, testing::Lt(0))))
+      .WillOnce(testing::Return(scroll_result_did_scroll_));
+  // When scroll latching is enabled, ScrollEnd shouldn't get called for every
+  // tick.
+  EXPECT_CALL(mock_input_handler_, ScrollEnd(testing::_)).Times(0);
+  time += base::TimeDelta::FromMilliseconds(100);
+  Animate(time);
+
+  VERIFY_AND_RESET_MOCKS();
+
+  // The last call should stop scrolling.
+  EXPECT_CALL(mock_input_handler_,
+              GetEventListenerProperties(cc::EventListenerClass::kMouseWheel))
+      .WillOnce(testing::Return(cc::EventListenerProperties::kNone));
+
+  // When scroll latching is enabled, ScrollBegin shouldn't get called for
+  // every tick.
+  EXPECT_CALL(mock_input_handler_, ScrollBegin(testing::_, testing::_))
+      .Times(0);
+  EXPECT_CALL(
+      mock_input_handler_,
+      ScrollBy(testing::Property(&cc::ScrollState::delta_x, testing::Lt(0))))
+      .WillOnce(testing::Return(scroll_result_did_not_scroll_));
+  // When scroll latching is enabled, ScrollEnd gets called when the last
+  // ScrollBy did not scroll.
+  EXPECT_CALL(mock_input_handler_, ScrollEnd(testing::_));
+  time += base::TimeDelta::FromMilliseconds(100);
+  Animate(time);
+
+  VERIFY_AND_RESET_MOCKS();
+
+  // No more ticks.
+  EXPECT_CALL(mock_input_handler_,

[bokan, 2017/01/10 18:53:59]

I don't think you need this. All we do form here is clean up.

[sahel, 2017/01/10 21:10:06]

When latching is enabled, ScrollEnd is called in FlingScrollByMouseWheel only
when did_scroll is false (rather than every time after ScrollBy).

This is for showing that once scrollEnd is called, Animate won't call
FlingScrollByMouseWheel anymore. I can get read of this part if you still think
it's not needed.

[bokan, 2017/01/10 21:25:06]

But you don't call Animate after it...

[sahel, 2017/01/12 18:53:44]

I didn't know the behavior of VERIFY_AND_RESET_MOCKS()
As we discussed I moved this checking before the last VERIFY_AND_RESET_MOCKS();

+              GetEventListenerProperties(cc::EventListenerClass::kMouseWheel))
+      .Times(0);
+}
+
 TEST_P(InputHandlerProxyTest, GestureFlingOnMainThreadTouchpad) {
   // We should send all events to the widget for this gesture.
   expected_disposition_ = InputHandlerProxy::DID_NOT_HANDLE;