Wheel scroll latching enabled behind flag.

content/browser/renderer_host/input/mouse_wheel_event_queue.cc

Index: content/browser/renderer_host/input/mouse_wheel_event_queue.cc
diff --git a/content/browser/renderer_host/input/mouse_wheel_event_queue.cc b/content/browser/renderer_host/input/mouse_wheel_event_queue.cc
index d1d3bc22276279290e8cde1271e27591b76b26e3..4a0ec3f52008931ac00eff9204ee601757cd144f 100644
--- a/content/browser/renderer_host/input/mouse_wheel_event_queue.cc
+++ b/content/browser/renderer_host/input/mouse_wheel_event_queue.cc
@@ -8,6 +8,7 @@
 #include "base/stl_util.h"
 #include "base/trace_event/trace_event.h"
 #include "content/common/input/web_input_event_traits.h"
+#include "content/public/common/content_features.h"
 
 using blink::WebGestureEvent;
 using blink::WebInputEvent;
@@ -41,6 +42,12 @@ MouseWheelEventQueue::MouseWheelEventQueue(MouseWheelEventQueueClient* client,
       scroll_transaction_ms_(scroll_transaction_ms),
       scrolling_device_(blink::WebGestureDeviceUninitialized) {
   DCHECK(client);
+  touchpad_scroll_latching_ =
+      base::FeatureList::IsEnabled(features::kTouchpadScrollLatching);

[dtapuska, 2016/07/19 17:41:31]

I don't think this will build on mac. It is a little odd that we adjust the
kDefaultWheelScrollTransactionMs.

I think perhaps we should make two defaults. One for latching enabled; one with
it disabled. Then the caller can determine which to use.

Also the value is used before this method is initialized. So you are
initializing a static field inside a constructor of an object. So its first use
will be 0.

[sahel, 2016/07/19 22:47:03]

Done.

+  if (touchpad_scroll_latching_)
+    kDefaultWheelScrollTransactionMs = 100;
+  else
+    kDefaultWheelScrollTransactionMs = 0;
 }
 
 MouseWheelEventQueue::~MouseWheelEventQueue() {
@@ -169,7 +176,7 @@ void MouseWheelEventQueue::ProcessMouseWheelAck(
       if (needs_scroll_begin_) {
         // If no GSB has been sent, it will be a non-synthetic GSB.
         SendScrollBegin(scroll_update, false);
-      } else if (has_phase_info) {
+      } else if (has_phase_info && !touchpad_scroll_latching_) {

[dtapuska, 2016/07/19 17:41:31]

Is it always scroll_transaction_ms_ is 0 then touchpad_scroll_latching_ is
false.

Do we need both fields? Perhaps we can just have a method bool
touch_pad_scroll_latching() { return scroll_transaction_ms_ == 0; }

[sahel, 2016/07/19 22:47:03]

I kept the boolean value, but calculated it based on the scroll_transaction_ms_
in constructor.
The reason for keeping the boolean value is easier code change when we want to
remove the flag and have the feature always enabled.

         // If a GSB has been sent, generate a synthetic GSB if we have phase
         // information. This should be removed once crbug.com/526463 is fully
         // implemented.
@@ -192,7 +199,8 @@ void MouseWheelEventQueue::ProcessMouseWheelAck(
         // Generate a synthetic GSE for every update to force hit testing so
         // that the non-latching behavior is preserved. Remove once
         // crbug.com/526463 is fully implemented.
-        SendScrollEnd(scroll_update, true);
+        if (!touchpad_scroll_latching_)
+          SendScrollEnd(scroll_update, true);
       } else {
         scroll_end_timer_.Start(
             FROM_HERE,