Wheel scroll latching enabled behind flag.

content/browser/renderer_host/input/mouse_wheel_event_queue.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/input/mouse_wheel_event_queue.h"
 
 #include "base/metrics/histogram_macros.h"
 #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;
 using blink::WebMouseWheelEvent;
 using ui::LatencyInfo;
 
 namespace content {
 
 // This class represents a single queued mouse wheel event. Its main use
 // is that it is reported via trace events.
@@ skipping 13 matching lines @@
 };
 
 MouseWheelEventQueue::MouseWheelEventQueue(MouseWheelEventQueueClient* client,
                                            int64_t scroll_transaction_ms)
     : client_(client),
       needs_scroll_begin_(true),
       needs_scroll_end_(false),
       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() {
   if (!wheel_queue_.empty())
     STLDeleteElements(&wheel_queue_);
 }
 
 void MouseWheelEventQueue::QueueEvent(
     const MouseWheelEventWithLatencyInfo& event) {
   TRACE_EVENT0("input", "MouseWheelEventQueue::QueueEvent");
@@ skipping 108 matching lines @@
     // because the events generated will be a GSB (non-synthetic) and GSE
     // (non-synthetic). This situation arises when OSX generates double
     // phase end information.
     bool empty_sequence =
         !needs_update && needs_scroll_begin_ && current_phase_ended;
 
     if (needs_update || !empty_sequence) {
       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.
         SendScrollBegin(scroll_update, true);
       }
 
       if (needs_update) {
         ui::LatencyInfo latency = ui::LatencyInfo();
         latency.AddLatencyNumber(
             ui::INPUT_EVENT_LATENCY_GENERATE_SCROLL_UPDATE_FROM_MOUSE_WHEEL, 0,
             0);
         client_->ForwardGestureEventWithLatencyInfo(scroll_update, latency);
       }
 
       if (current_phase_ended) {
         // Non-synthetic GSEs are sent when the current phase is canceled or
         // ended.
         SendScrollEnd(scroll_update, false);
       } else if (has_phase_info) {
         // 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,
             base::TimeDelta::FromMilliseconds(scroll_transaction_ms_),
             base::Bind(&MouseWheelEventQueue::SendScrollEnd,
                        base::Unretained(this), scroll_update, false));
       }
     }
   }
 
@@ skipping 78 matching lines @@
   scroll_begin.data.scrollBegin.targetViewport = false;
   scroll_begin.data.scrollBegin.deltaHintUnits =
       gesture_update.data.scrollUpdate.deltaUnits;
 
   needs_scroll_begin_ = false;
   needs_scroll_end_ = true;
   client_->ForwardGestureEventWithLatencyInfo(scroll_begin, ui::LatencyInfo());
 }
 
 }  // namespace content