Touchpad and wheel scroll latching for ChromeOS 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/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/trace_event/trace_event.h"
 #include "ui/events/base_event_utils.h"
@@ skipping 189 matching lines @@
               0, 0);
           client_->ForwardGestureEventWithLatencyInfo(scroll_update, latency);
         }
 
         if (momentum_phase_ended) {
           // Send GSE with if scroll latching is enabled and no fling is going
           // to happen next.
           SendScrollEnd(scroll_update, false);
         } else if (scroll_phase_ended || !has_phase_info) {
           // If scroll latching is enabled and a fling might happen next, or
           // no phase info exists, start the scroll_end_timer_.

[tdresser, 2017/01/09 18:25:19]

The scroll end timer should only be running if latching is enabled, right?

[sahel, 2017/01/09 20:02:01]

Right, it should be running, but we need to reset it which is done in start.

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

For my own understanding: there's a comment above "Only OSX populates the phase
and momentumPhase", does this mean that momentum_phase_ended and
scroll_phase_ended will always be false here for non-OSX (but we'll come in here
through |!has_phase_info|?

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

correct.

           scroll_end_timer_.Start(
               FROM_HERE,
               base::TimeDelta::FromMilliseconds(scroll_transaction_ms_),
               base::Bind(&MouseWheelEventQueue::SendScrollEnd,
                          base::Unretained(this), scroll_update, false));
         }
       }
 
     } else {  // !enable_scroll_latching_
       if (needs_update || !empty_sequence) {
@@ skipping 50 matching lines @@
       scroll_end_timer_.Reset();
       task.Run();
     }
     scrolling_device_ = gesture_event.event.sourceDevice;
   } else if (scrolling_device_ == gesture_event.event.sourceDevice &&
              (gesture_event.event.type ==
                   blink::WebInputEvent::GestureScrollEnd ||
               gesture_event.event.type ==
                   blink::WebInputEvent::GestureFlingStart)) {
     scrolling_device_ = blink::WebGestureDeviceUninitialized;
-    if (scroll_end_timer_.IsRunning())
-      scroll_end_timer_.Reset();
+    if (scroll_end_timer_.IsRunning()) {

[tdresser, 2017/01/09 18:25:19]

This should only happen if latching is enabled, right? Can we DCHECK that
latching is enabled?

[sahel, 2017/01/09 20:02:01]

If the latching is not enabled then the timeout for the timer will be zero, is
it possible (in some rare cases) to have the timer running when Fling arrives?
If not, then you are right we can add a DCHECK and get rid of the else part.

[tdresser, 2017/01/10 16:19:02]

I missed that we do start the timer when latching is disabled.

This LGTM.

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

I don't actually know how we implement task queues, but I'd assume that a
0-delay timer would get queued but might be behind an already queued FlingStart.
(unless they're in separate queues and timers get priority)

Perhaps if latching is disabled, rather than queuing the 0-delay timer, we
should just synchronously run it?

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

I don't know if timer tasks have priority or not, maybe @tdresser can help with
the answer.
The timer starts even when the latching is disabled. I don't know if we always
want to have zero timeout in this case or not, if the answer is yes, then we can
get rid of the timer when latching is disabled.

+      if (enable_scroll_latching_) {
+        // Don't send the pending ScrollEnd if a fling is happening.
+        // The next wheel event will still need a ScrollBegin.
+        scroll_end_timer_.Stop();
+        needs_scroll_begin_ = true;

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

If the event is a GFS, do we still need a GSB?

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

Yes, this is for making the gesture event sequences for touchscreen and touchpad
scrolls similar.
For touchpad we will have GSB (GSU)xn GFS
we won't send a GSE after/before GFS and that's why needs_scroll_begin_ is set
to true directly for the next GSU. The next GSU won't be for this scroll
sequence because when we are handling touchpad fling, we don't generate GSU
events after the fling. 

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

Ah, ok. Thanks for the explanation.

+        needs_scroll_end_ = false;
+      } else {
+        scroll_end_timer_.Reset();
+      }
+    }
   }
 }
 
 void MouseWheelEventQueue::TryForwardNextEventToRenderer() {
   TRACE_EVENT0("input", "MouseWheelEventQueue::TryForwardNextEventToRenderer");
 
   if (wheel_queue_.empty() || event_sent_for_gesture_ack_)
     return;
 
   event_sent_for_gesture_ack_ = std::move(wheel_queue_.front());
@@ skipping 46 matching lines @@
   scroll_begin.data.scrollBegin.deltaHintUnits =
       gesture_update.data.scrollUpdate.deltaUnits;
 
   needs_scroll_begin_ = false;
   needs_scroll_end_ = true;
   client_->ForwardGestureEventWithLatencyInfo(
       scroll_begin, ui::LatencyInfo(ui::SourceEventType::WHEEL));
 }
 
 }  // namespace content