Touchpad and wheel scroll latching for ChromeOS behind flag.

ui/events/blink/input_handler_proxy.cc

Index: ui/events/blink/input_handler_proxy.cc
diff --git a/ui/events/blink/input_handler_proxy.cc b/ui/events/blink/input_handler_proxy.cc
index fcd5b33aa14e40e0a52a464a801bac8369433f1f..76e33472d0b57a35e128b77eb5f666aeea4d5172 100644
--- a/ui/events/blink/input_handler_proxy.cc
+++ b/ui/events/blink/input_handler_proxy.cc
@@ -254,6 +254,7 @@ InputHandlerProxy::InputHandlerProxy(cc::InputHandler* input_handler,
       has_fling_animation_started_(false),
       smooth_scroll_enabled_(false),
       uma_latency_reporting_enabled_(base::TimeTicks::IsHighResolution()),
+      touchpad_and_wheel_scroll_latching_enabled_(false),
       touch_start_result_(kEventDispositionUndefined),
       current_overscroll_params_(nullptr),
       has_ongoing_compositor_scroll_pinch_(false),
@@ -613,53 +614,51 @@ InputHandlerProxy::EventDisposition InputHandlerProxy::HandleMouseWheel(
   }
 }
 
-InputHandlerProxy::EventDisposition InputHandlerProxy::ScrollByMouseWheel(
+InputHandlerProxy::EventDisposition InputHandlerProxy::FlingScrollByMouseWheel(
     const WebMouseWheelEvent& wheel_event,
     cc::EventListenerProperties listener_properties) {
   DCHECK(listener_properties == cc::EventListenerProperties::kPassive ||
          listener_properties == cc::EventListenerProperties::kNone);
 
-  // TODO(ccameron): The rail information should be pushed down into
-  // InputHandler.
-  gfx::Vector2dF scroll_delta(
-      wheel_event.railsMode != WebInputEvent::RailsModeVertical
-          ? -wheel_event.deltaX
-          : 0,
-      wheel_event.railsMode != WebInputEvent::RailsModeHorizontal
-          ? -wheel_event.deltaY
-          : 0);
-
-  if (wheel_event.scrollByPage) {
-    // TODO(jamesr): We don't properly handle scroll by page in the compositor
-    // thread, so punt it to the main thread. http://crbug.com/236639
-    RecordMainThreadScrollingReasons(
-        blink::WebGestureDeviceTouchpad,
-        cc::MainThreadScrollingReason::kPageBasedScrolling);
-    return DID_NOT_HANDLE;
+  DCHECK(!wheel_event.railsMode);

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

Is this because the TODO was resolved?

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

No it's not, it is because this function is only called in touchpadScrollFling
and the  synthetic_wheel event there doesn't have vertical/horizontal rail mode.

+  gfx::Vector2dF scroll_delta(-wheel_event.deltaX, -wheel_event.deltaY);
 
-  } else if (ShouldAnimate(wheel_event.hasPreciseScrollingDeltas)) {
-    base::TimeTicks event_time =
-        base::TimeTicks() +
-        base::TimeDelta::FromSecondsD(wheel_event.timeStampSeconds);
-    base::TimeDelta delay = base::TimeTicks::Now() - event_time;
-    cc::InputHandler::ScrollStatus scroll_status =
-        input_handler_->ScrollAnimated(gfx::Point(wheel_event.x, wheel_event.y),
-                                       scroll_delta, delay);
+  DCHECK(!wheel_event.scrollByPage);

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

The bug linked with the TODO(jamesr) is still open. Why is it ok to DCHECK this
now?

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

Similarly, this part of the code was never executed.

+  DCHECK(wheel_event.hasPreciseScrollingDeltas);
+  if (touchpad_and_wheel_scroll_latching_enabled_) {
+    if (gesture_scroll_on_impl_thread_) {

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

nit: 

if (!gesture_scroll_on_impl_thread_) 
  return DID_NOT_HANDLE;

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

Done.

+      TRACE_EVENT_INSTANT2("input",
+                           "InputHandlerProxy::handle_input wheel scroll",
+                           TRACE_EVENT_SCOPE_THREAD, "deltaX", scroll_delta.x(),
+                           "deltaY", scroll_delta.y());
 
-    RecordMainThreadScrollingReasons(
-        blink::WebGestureDeviceTouchpad,
-        scroll_status.main_thread_scrolling_reasons);
+      cc::ScrollStateData scroll_state_update_data;
+      scroll_state_update_data.delta_x = scroll_delta.x();
+      scroll_state_update_data.delta_y = scroll_delta.y();
+      scroll_state_update_data.position_x = wheel_event.x;
+      scroll_state_update_data.position_y = wheel_event.y;

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

This code is duplicated from below. Instead of copying it up here, find a way to
reuse it. e.g. Presumably you want to avoid the ScrollBegin, just guard it in
|!touchpad_and_wheel_scroll_latching_enabled_|

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

I initially did that in mouse_wheel_event_queue. I got this review comment
saying that since we will get rid of the flag, copying some parts of code is
prefered because this way it's easier to delete the
|!touchpad_and_wheel_scroll_latching_enabled_| code path.

I can still change the code to avoid copying but I will have more fine grain
if(touchpad...) conditions.

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

Ok, if it's going away eventually duplication is fine.

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

Acknowledged.

+      cc::ScrollState scroll_state_update(scroll_state_update_data);
 
-    switch (scroll_status.thread) {
-      case cc::InputHandler::SCROLL_ON_IMPL_THREAD:
-        return DID_HANDLE;
-      case cc::InputHandler::SCROLL_IGNORED:
-        return DROP_EVENT;
-      default:
-        return DID_NOT_HANDLE;
-    }
+      cc::InputHandlerScrollResult scroll_result =
+          input_handler_->ScrollBy(&scroll_state_update);
+      HandleOverscroll(gfx::Point(wheel_event.x, wheel_event.y), scroll_result,
+                       false);
+      if (scroll_result.did_scroll) {
+        return listener_properties == cc::EventListenerProperties::kPassive
+                   ? DID_HANDLE_NON_BLOCKING
+                   : DID_HANDLE;
+      }
+      // End of fling.
+      cc::ScrollStateData scroll_state_end_data;
+      scroll_state_end_data.is_ending = true;
+      cc::ScrollState scroll_state_end(scroll_state_end_data);
+      input_handler_->ScrollEnd(&scroll_state_end);
 
-  } else {
+      return DROP_EVENT;
+    } else {
+      return DID_NOT_HANDLE;
+    }
+  } else {  // !touchpad_and_wheel_scroll_latching_enabled_
     cc::ScrollStateData scroll_state_begin_data;
     scroll_state_begin_data.position_x = wheel_event.x;
     scroll_state_begin_data.position_y = wheel_event.y;
@@ -880,7 +879,8 @@ InputHandlerProxy::EventDisposition InputHandlerProxy::HandleGestureFlingStart(
 
   switch (scroll_status.thread) {
     case cc::InputHandler::SCROLL_ON_IMPL_THREAD: {
-      if (gesture_event.sourceDevice == blink::WebGestureDeviceTouchpad) {
+      if (!touchpad_and_wheel_scroll_latching_enabled_ &&
+          gesture_event.sourceDevice == blink::WebGestureDeviceTouchpad) {
         scroll_state.set_is_ending(true);
         input_handler_->ScrollEnd(&scroll_state);
       }
@@ -1416,7 +1416,7 @@ bool InputHandlerProxy::TouchpadFlingScroll(
       synthetic_wheel.globalX = fling_parameters_.globalPoint.x;
       synthetic_wheel.globalY = fling_parameters_.globalPoint.y;
 
-      disposition = ScrollByMouseWheel(synthetic_wheel, properties);
+      disposition = FlingScrollByMouseWheel(synthetic_wheel, properties);
 
       // Send the event over to the main thread.
       if (disposition == DID_HANDLE_NON_BLOCKING) {