[ash-md] Adds support for gesture to move selection in overview mode

ash/wm/gestures/overview_gesture_handler.cc

Index: ash/wm/gestures/overview_gesture_handler.cc
diff --git a/ash/wm/gestures/overview_gesture_handler.cc b/ash/wm/gestures/overview_gesture_handler.cc
index 2b94b9eefe3bc64cc952241ac2f3a8434de651a7..fb3892cd1b26c884a57c2212aac8b3a9ca337897 100644
--- a/ash/wm/gestures/overview_gesture_handler.cc
+++ b/ash/wm/gestures/overview_gesture_handler.cc
@@ -14,7 +14,11 @@ namespace {
 
 // The threshold before engaging overview on a three finger swipe on the
 // touchpad.
-const float kSwipeThresholdPixels = 300;
+const float kVerticalSwipeThresholdPixels = 300;
+
+// The threshold before moving selector horizontally when swiping with the
+// three fingers on the touchpad.
+const float kHorizontalSwipeThresholdPixels = 150;

[tdanderson, 2017/02/06 20:13:57]

As I understand it you're processing ui::ET_SCROLL* event types in order to
perform all of this behavior, not ui::ET_GESTURE_SWIPE events.  So for lines
15-21 (and elsewhere in this CL) I think it would be better to use the term
"three-finger touchpad scroll" or similar instead of "swipe" to avoid confusion
about which event types we're using.

[varkha, 2017/02/08 20:41:13]

Done.

 
 }  // namespace
 
@@ -32,31 +36,45 @@ bool OverviewGestureHandler::ProcessScrollEvent(const ui::ScrollEvent& event) {
   scroll_x_ += event.x_offset();
   scroll_y_ += event.y_offset();

[tdanderson, 2017/02/06 20:13:58]

In the .h these two members are documented as "The total distance scrolled with
three fingers" which I don't think is correct. It seems as though these are just
to track how much has been scrolled in either direction up to but not including
the point where an action is triggered (enter overview, close overview, move
selection widget). When any of those three things happen then the values are
reset.

[varkha, 2017/02/08 20:41:13]

Done.

 
-  // Horizontal swiping is ignored.
-  if (std::fabs(scroll_x_) >= std::fabs(scroll_y_)) {
-    scroll_x_ = scroll_y_ = 0;
-    return false;
-  }
-
   // Only allow swipe up to enter overview, down to exit. Ignore extra swiping
   // in the wrong direction.
   WindowSelectorController* window_selector_controller =
       WmShell::Get()->window_selector_controller();
+
+  // Horizontal swiping moves selection when overview mode is active.
+  if (std::fabs(scroll_x_) >= std::fabs(scroll_y_)) {
+    if (!window_selector_controller->IsSelecting()) {
+      scroll_x_ = scroll_y_ = 0;
+      return false;
+    }
+
+    if (std::fabs(scroll_x_) < kHorizontalSwipeThresholdPixels)
+      return false;
+    window_selector_controller->IncrementSelection(
+        scroll_x_ / kHorizontalSwipeThresholdPixels);
+    scroll_x_ = scroll_y_ = 0;
+    return true;
+  }
+
   if (window_selector_controller->IsSelecting()) {
     if (scroll_y_ < 0)
       scroll_x_ = scroll_y_ = 0;
-    if (scroll_y_ < kSwipeThresholdPixels)
+    if (scroll_y_ < kVerticalSwipeThresholdPixels)
       return false;
   } else {
     if (scroll_y_ > 0)
       scroll_x_ = scroll_y_ = 0;
-    if (scroll_y_ > -kSwipeThresholdPixels)
+    if (scroll_y_ > -kVerticalSwipeThresholdPixels)
       return false;
   }
 
   // Reset scroll amount on toggling.
   scroll_x_ = scroll_y_ = 0;
   WmShell::Get()->RecordUserMetricsAction(UMA_TOUCHPAD_GESTURE_OVERVIEW);
+  if (window_selector_controller->IsSelecting() &&

[tdanderson, 2017/02/06 20:13:58]

Is checking IsSelecting() necessary here given how you've documented
AcceptSelection(), and given line 418 in window_selector.cc?

[varkha, 2017/02/08 20:41:13]

Yes, we can get here when not already in selection mode and it would be wrong to
call AcceptSelection (but right to skip and ToggleOverview().

[tdanderson, 2017/02/08 23:14:19]

Acknowledged.

+      window_selector_controller->AcceptSelection()) {
+    return true;
+  }
   window_selector_controller->ToggleOverview();
   return true;
 }