Implemented swipe to close in overview mode.

ash/wm/overview/transparent_activate_window_button.cc

Index: ash/wm/overview/transparent_activate_window_button.cc
diff --git a/ash/wm/overview/transparent_activate_window_button.cc b/ash/wm/overview/transparent_activate_window_button.cc
index a50e104e2062d8179d6097875e87919bf0941d74..a9d14a29389bbd2b584868bccd0485bf5f5983ec 100644
--- a/ash/wm/overview/transparent_activate_window_button.cc
+++ b/ash/wm/overview/transparent_activate_window_button.cc
@@ -6,9 +6,11 @@
 
 #include <vector>
 
+#include "ash/ash_switches.h"
 #include "ash/shell.h"
 #include "ash/shell_window_ids.h"
 #include "ash/wm/overview/transparent_activate_window_button_delegate.h"
+#include "base/command_line.h"
 #include "ui/events/event.h"
 #include "ui/events/gesture_event_details.h"
 #include "ui/views/controls/button/custom_button.h"
@@ -19,6 +21,13 @@ namespace ash {
 namespace {
 const char kTransparentButtonName[] = "TransparentButton";
 
+// The default close window distance minimum.
+const int kDefaultCloseWindowDistanceMinimum = 100;

[flackr, 2014/11/07 19:10:52]

Looks like this will never actually be used since the first thing we do is
override it.

[bruthig, 2015/01/02 16:49:17]

That is correct.  I'm not sure if you are suggesting a change here or not?

+
+// The minimum fling velocity which will cause a window to be closed.  Unit is
+// pixels per second.
+const float kMinimumFlingVelocity = 4000.0f;

[jonross, 2014/11/12 23:57:03]

Won't be needed if we decide to just close on fling events.

[bruthig, 2015/01/02 16:49:17]

Windows are closed too easily if using all flings.  I consulted stevet@ and we
decided that a minimum was necessary.

+
 // Initializes the event handler transparent window.
 views::Widget* InitEventHandler(aura::Window* root_window) {
   views::Widget* widget = new views::Widget;
@@ -52,6 +61,10 @@ class TransparentButton : public views::CustomButton {
 
   TransparentActivateWindowButtonDelegate* delegate() { return delegate_; }
 
+  void set_close_window_distance_minimum(int distance) {
+    close_window_distance_minimum_ = distance;

[flackr, 2014/11/07 19:10:52]

I think setting the close distance to a proportion of the button's size is
reasonable and would save us the complication of passing down the value from
WindowSelector.

[bruthig, 2015/01/02 16:49:17]

Not 100% sure I understand but I will take a shot.  I will assume you are
suggesting that the close_window_distance_minimum should be calculated within
the TransparentActivateWindowButton.

Keep in mind the only size that the TransparentActivateWindowButton has access
to is of the transparent overlay and when there are multiple windows this
overlay's size is not the same as the entire WindowSelectorItem's size.  I think
it would be kind of odd if a smaller panel window closed at a different absolute
distance than a larger window that took up the entire WindowSelectorItem's space
when in the exact same overview layout.

I'm not sure why passing in values like this is considered a complication?

[jonross, 2015/01/06 15:02:27]

Can the size of the WindowSelectionItem be accessed by the
TransparentActivateWindowButton? If so this could be determined within this
class if passing it around is a concern.

I wouldn't want different sized panels to close at different fling distances.

+  }
+
   // views::View:
   virtual void OnGestureEvent(ui::GestureEvent* event) override;
   virtual void OnMouseEvent(ui::MouseEvent* event) override;
@@ -63,6 +76,21 @@ class TransparentButton : public views::CustomButton {
  private:
   TransparentActivateWindowButtonDelegate* delegate_;
 
+  // True if the swipe to close feature is disabled.
+  const bool swipe_to_close_disabled_;

[flackr, 2014/11/07 19:10:52]

Follow the pattern used by kEnableTouchFeedback to avoid having lots of copies
of the flag value:
https://code.google.com/p/chromium/codesearch#chromium/src/ui/base/ui_base_sw...

[pkotwicz, 2014/12/21 00:09:32]

Drive by:

I dislike the pattern used with kEnableTouchFeedback because it is impossible to
test the behavior with both return values of switches::IsTouchFeedbackEnabled()
in the same test suite

It is possible to test the behavior with both return values of 
base::CommandLine::ForCurrentProcess()->HasSwitch() because the command line is
reset for each test case. TestClientInitializer does the resetting.

There are ways of avoiding having a copy of the flag in each TransparentButton
class (e.g. store the value in WindowSelectorController). I don't find any of
these ways clearly better than a member variable in TransparentButton though.

[bruthig, 2015/01/02 16:49:17]

This is a very good example of how statics are problematic for testing ;)

I will defer changing this until we have agreement.

[jonross, 2015/01/06 15:02:27]

It does add a complexity, however you can have one test class for the main
behaviour, and then extend it to provide the other command line flag.

Definitely not optimal though.

+
+  // The original X location for a tap down event. |original_x_| is in the local
+  // coordinate space as |this|.
+  float original_x_;
+
+  // The velocity of the current fling gesture.  A fling is active iff
+  // |flint_velocity_x_| has a value greater than 0.
+  float fling_velocity_x_;
+
+  // The minimum distance, in pixels, for which a touch drag should cause a
+  // window to be closed.
+  int close_window_distance_minimum_;
+
   DISALLOW_COPY_AND_ASSIGN(TransparentButton);
 };
 
@@ -70,7 +98,11 @@ TransparentButton::TransparentButton(
     views::ButtonListener* listener,
     TransparentActivateWindowButtonDelegate* delegate)
     : CustomButton(listener),
-      delegate_(delegate) {
+      delegate_(delegate),
+      swipe_to_close_disabled_(CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kAshDisableSwipeToCloseInOverviewMode)),
+      fling_velocity_x_(0.0f),
+      close_window_distance_minimum_(kDefaultCloseWindowDistanceMinimum) {
 }
 
 TransparentButton::~TransparentButton() {
@@ -81,6 +113,8 @@ void TransparentButton::OnMouseEvent(ui::MouseEvent* event) {
   // TransparentActivateWindowButton.
   if (GetWidget()->HasCapture() && event->type() == ui::ET_MOUSE_PRESSED) {
     GetWidget()->ReleaseCapture();
+    if (!swipe_to_close_disabled_)
+      delegate_->CancelScroll();
     event->StopPropagation();
     return;
   }
@@ -102,6 +136,40 @@ void TransparentButton::OnGestureEvent(ui::GestureEvent* event) {
     GetWidget()->ReleaseCapture();
   }
 
+  if (swipe_to_close_disabled_) {
+    CustomButton::OnGestureEvent(event);
+    return;
+  }
+
+  int delta_x = event->x() - original_x_;
+
+  switch (event->type()) {
+    case ui::ET_GESTURE_TAP_DOWN:
+      original_x_ = event->x();
+      break;
+    case ui::ET_GESTURE_SCROLL_BEGIN:
+    case ui::ET_GESTURE_SCROLL_UPDATE:
+      delegate_->Scroll(delta_x);
+      break;
+    case ui::ET_SCROLL_FLING_START:
+    case ui::ET_SCROLL_FLING_CANCEL:

[flackr, 2014/11/07 19:10:52]

I don't think we need to worry about fling cancel since as I understand
fling_start is after the finger lifts and fling cancel would be if they again
put their finger down to stop the fling. I think closing as soon as the finger
lifts up rather than worrying about a re-tap is simpler and more intuitive.

[bruthig, 2015/01/02 16:49:17]

Done.

+      // event->details().velocity_x() will be zero when event->type() is
+      // ui::ET_SCROLL_FLING_CANCEL.
+      fling_velocity_x_ = fabs(event->details().velocity_x());

[flackr, 2014/11/07 19:10:52]

Why do we need to save the fling velocity? If I remember correctly the fling is
generated after the finger lifts up so we can immediately evaluate at that point
whether it translates into a close or not and immediately respond to it.

[jonross, 2014/11/12 23:57:03]

Currently used on line 162 to determine if fling is fast enough to close.

Do we actually get slow flings?

[bruthig, 2015/01/02 16:49:17]

It's easier IMO to only evaluate for one event type whether the window should
close or not.  If we also evaluate and possibly close during a
ET_GESTURE_FLING_START event then we would need to keep track of whether the
window was closed or not when we are evaluating during the ET_GESTURE_END event
because we shouldn't attempt to close (or cancel scroll) on a window which has
already been closed.

+      break;
+    case ui::ET_GESTURE_END:
+      if (abs(delta_x) > close_window_distance_minimum_ ||
+          fling_velocity_x_ > kMinimumFlingVelocity) {
+        delegate_->Close();
+      } else {
+        delegate_->CancelScroll();
+      }
+      original_x_ = 0;
+      fling_velocity_x_ = 0.0f;
+      break;
+    default:
+      break;
+  }
   CustomButton::OnGestureEvent(event);
   event->StopPropagation();
 }
@@ -126,6 +194,11 @@ void TransparentActivateWindowButton::SetBounds(const gfx::Rect& bounds) {
   event_handler_widget_->SetBounds(bounds);
 }
 
+void TransparentActivateWindowButton::SetCloseWindowDistanceMinimum(
+    int distance) {
+  transparent_button_->set_close_window_distance_minimum(distance);
+}
+
 void TransparentActivateWindowButton::SendFocusAlert() const {
   event_handler_widget_->GetContentsView()->
       NotifyAccessibilityEvent(ui::AX_EVENT_FOCUS, true);