Reland "tablet: Remove minimum time between taps for quick switch."

This is a reland of b513507c

Original change's description:
> tablet: Remove minimum time between taps for quick switch.
> 
> Previously relied on using tap_count in GestureEvent which has a minimum
> time between taps of 40ms for consecutive taps to have tap_count of 2.
> Remove that and track double taps in overview button tray.cc. Allows
> double tap from mice now.
> 
> Test: ash_unittests OverviewButtonTrayTest.*
> Bug: 817883
> Change-Id: Icce2e668223f938a7f8194c5f0a03279abff9907
> Reviewed-on: https://chromium-review.googlesource.com/956769
> Commit-Queue: Sammie Quon <sammiequon@chromium.org>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#542315}

Bug: 817883
Change-Id: Ifd6ecabf3f1702d8aeffa29a48a6475f0e6b5422
Reviewed-on: https://chromium-review.googlesource.com/958014Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542553}

ash/system/overview/overview_button_tray.cc

@@ -25,6 +25,8 @@
 
 namespace ash {
 
+constexpr base::TimeDelta OverviewButtonTray::kDoubleTapThresholdMs;
+
 OverviewButtonTray::OverviewButtonTray(Shelf* shelf)
     : TrayBackgroundView(shelf),
       icon_(new views::ImageView()),
@@ -67,28 +69,37 @@ void OverviewButtonTray::OnGestureEvent(ui::GestureEvent* event) {
 }
 
 bool OverviewButtonTray::PerformAction(const ui::Event& event) {
-  if (event.type() == ui::ET_GESTURE_TAP) {
-    if (event.AsGestureEvent()->details().tap_count() == 2) {
-      // If the second tap is not on the window selection page, that means we
-      // started on the window selection page. Ignore these double taps. (ie.
-      // treat them as single taps by ignoring the second tap)
-      if (!Shell::Get()->window_selector_controller()->IsSelecting())
-        return true;
-
-      base::RecordAction(base::UserMetricsAction("Tablet_QuickSwitch"));
-      MruWindowTracker::WindowList mru_window_list =
-          Shell::Get()->mru_window_tracker()->BuildMruWindowList();
-
-      // Switch to the second most recently used window (most recent is the
-      // current window), if it exists.
-      if (mru_window_list.size() > 1) {
-        AnimateInkDrop(views::InkDropState::DEACTIVATED, nullptr);
-        wm::GetWindowState(mru_window_list[1])->Activate();
-        return true;
-      }
+  DCHECK(event.type() == ui::ET_MOUSE_RELEASED ||
+         event.type() == ui::ET_GESTURE_TAP);
+
+  if (last_press_event_time_ &&
+      event.time_stamp() - last_press_event_time_.value() <
+          kDoubleTapThresholdMs) {
+    // Second taps should not be processed outside of overview mode. (First
+    // taps should be outside of overview).
+    DCHECK(Shell::Get()->window_selector_controller()->IsSelecting());
+
+    base::RecordAction(base::UserMetricsAction("Tablet_QuickSwitch"));
+    MruWindowTracker::WindowList mru_window_list =
+        Shell::Get()->mru_window_tracker()->BuildMruWindowList();
+
+    // Switch to the second most recently used window (most recent is the
+    // current window), if it exists.
+    if (mru_window_list.size() > 1u) {
+      AnimateInkDrop(views::InkDropState::DEACTIVATED, nullptr);
+      wm::GetWindowState(mru_window_list[1])->Activate();
+      last_press_event_time_ = base::nullopt;
+      return true;
     }
   }
 
+  // If not in overview mode record the time of this tap. A subsequent tap will
+  // be checked against this to see if we should quick switch.
+  last_press_event_time_ =
+      Shell::Get()->window_selector_controller()->IsSelecting()
+          ? base::nullopt
+          : base::make_optional(event.time_stamp());
+
   WindowSelectorController* controller =
       Shell::Get()->window_selector_controller();
   // Note: Toggling overview mode will fail if there is no window to show, the

ash/system/overview/overview_button_tray.h

@@ -29,6 +29,15 @@ class ASH_EXPORT OverviewButtonTray : public TrayBackgroundView,
                                       public ShellObserver,
                                       public TabletModeObserver {
  public:
+  // Second taps within this time will be counted as double taps. Use this
+  // instead of ui::Event's click_count and tap_count as those have a minimum
+  // time bewtween events before the second tap counts as a double tap.
+  // TODO(crbug.com/817883): We should the gesture detector double tap time or
+  // overview enter animation time, once ux decides which one to match (both are
+  // 300ms currently).
+  static constexpr base::TimeDelta kDoubleTapThresholdMs =
+      base::TimeDelta::FromMilliseconds(300);
+
   explicit OverviewButtonTray(Shelf* shelf);
   ~OverviewButtonTray() override;
 
@@ -70,6 +79,10 @@ class ASH_EXPORT OverviewButtonTray : public TrayBackgroundView,
 
   ScopedSessionObserver scoped_session_observer_;
 
+  // Stores the timestamp of the last tap event time that happened while not
+  // in overview mode. Used to check for double taps, which invoke quick switch.
+  base::Optional<base::TimeTicks> last_press_event_time_;
+
   DISALLOW_COPY_AND_ASSIGN(OverviewButtonTray);
 };
 

ash/system/overview/overview_button_tray_unittest.cc

@@ -49,18 +49,17 @@ OverviewButtonTray* GetSecondaryTray() {
       ->overview_button_tray();
 }
 
-// Helper function to perform a double tap on the overview button tray. A double
-// tap consists fot two tap gestures, one with tap count 1 and another with tap
-// count 2.
+ui::GestureEvent CreateTapEvent(
+    base::TimeDelta delta_from_start = base::TimeDelta()) {
+  return ui::GestureEvent(0, 0, 0, base::TimeTicks() + delta_from_start,
+                          ui::GestureEventDetails(ui::ET_GESTURE_TAP));
+}
+
+// Helper function to perform a double tap on the overview button tray.
 void PerformDoubleTap() {
-  ui::GestureEvent first_tap(0, 0, 0, base::TimeTicks(),
-                             ui::GestureEventDetails(ui::ET_GESTURE_TAP));
-  GetTray()->PerformAction(first_tap);
-
-  ui::GestureEventDetails second_tap_details(ui::ET_GESTURE_TAP);
-  second_tap_details.set_tap_count(2);
-  ui::GestureEvent second_tap(0, 0, 0, base::TimeTicks(), second_tap_details);
-  GetTray()->PerformAction(second_tap);
+  ui::GestureEvent tap = CreateTapEvent();
+  GetTray()->PerformAction(tap);
+  GetTray()->PerformAction(tap);
 }
 
 }  // namespace
@@ -96,7 +95,7 @@ void OverviewButtonTrayTest::NotifySessionStateChanged() {
 
 // Ensures that creation doesn't cause any crashes and adds the image icon.
 TEST_F(OverviewButtonTrayTest, BasicConstruction) {
-  EXPECT_TRUE(GetImageView(GetTray()) != NULL);
+  EXPECT_TRUE(GetImageView(GetTray()));
 }
 
 // Test that tablet mode toggle changes visibility.
@@ -118,13 +117,11 @@ TEST_F(OverviewButtonTrayTest, PerformAction) {
   // Overview Mode only works when there is a window
   std::unique_ptr<aura::Window> window(
       CreateTestWindowInShellWithBounds(gfx::Rect(5, 5, 20, 20)));
-  ui::GestureEvent tap(0, 0, 0, base::TimeTicks(),
-                       ui::GestureEventDetails(ui::ET_GESTURE_TAP));
-  GetTray()->PerformAction(tap);
+  GetTray()->PerformAction(CreateTapEvent());
   EXPECT_TRUE(Shell::Get()->window_selector_controller()->IsSelecting());
 
   // Verify tapping on the button again closes overview mode.
-  GetTray()->PerformAction(tap);
+  GetTray()->PerformAction(CreateTapEvent());
   EXPECT_FALSE(Shell::Get()->window_selector_controller()->IsSelecting());
 }
 
@@ -144,23 +141,25 @@ TEST_F(OverviewButtonTrayTest, PerformDoubleTapAction) {
   EXPECT_TRUE(wm::IsActiveWindow(window1.get()));
   EXPECT_FALSE(Shell::Get()->window_selector_controller()->IsSelecting());
 
-  // Verify that if we double tap on the window selection page, we leave the
-  // window selection page, but window 1 remains the active window.
-  ui::GestureEvent tap(0, 0, 0, base::TimeTicks(),
-                       ui::GestureEventDetails(ui::ET_GESTURE_TAP));
+  // Verify that if we double tap on the window selection page, it acts as two
+  // taps, and ends up on the window selection page again.
+  ui::GestureEvent tap = CreateTapEvent();
   ASSERT_TRUE(wm::IsActiveWindow(window1.get()));
   GetTray()->PerformAction(tap);
   ASSERT_TRUE(Shell::Get()->window_selector_controller()->IsSelecting());
   PerformDoubleTap();
-  EXPECT_TRUE(wm::IsActiveWindow(window1.get()));
-  EXPECT_FALSE(Shell::Get()->window_selector_controller()->IsSelecting());
+  EXPECT_TRUE(Shell::Get()->window_selector_controller()->IsSelecting());
 
   // Verify that if we minimize a window, double tapping the overlay tray button
-  // will bring up the window.
+  // will bring up the window, and it should be the active window.
+  GetTray()->PerformAction(tap);
+  ASSERT_TRUE(!Shell::Get()->window_selector_controller()->IsSelecting());
+  ASSERT_TRUE(wm::IsActiveWindow(window1.get()));
   wm::GetWindowState(window2.get())->Minimize();
   ASSERT_EQ(window2->layer()->GetTargetOpacity(), 0.0);
   PerformDoubleTap();
   EXPECT_EQ(window2->layer()->GetTargetOpacity(), 1.0);
+  EXPECT_TRUE(wm::IsActiveWindow(window2.get()));
 }
 
 // Tests that tapping on the control will record the user action Tray_Overview.
@@ -170,9 +169,7 @@ TEST_F(OverviewButtonTrayTest, TrayOverviewUserAction) {
   // Tapping on the control when there are no windows (and thus the user cannot
   // enter overview mode) should still record the action.
   base::UserActionTester user_action_tester;
-  ui::GestureEvent tap(0, 0, 0, base::TimeTicks(),
-                       ui::GestureEventDetails(ui::ET_GESTURE_TAP));
-  GetTray()->PerformAction(tap);
+  GetTray()->PerformAction(CreateTapEvent());
   ASSERT_FALSE(Shell::Get()->window_selector_controller()->IsSelecting());
   EXPECT_EQ(1, user_action_tester.GetActionCount(kTrayOverview));
 
@@ -180,13 +177,15 @@ TEST_F(OverviewButtonTrayTest, TrayOverviewUserAction) {
   // should record the user action.
   std::unique_ptr<aura::Window> window(
       CreateTestWindowInShellWithBounds(gfx::Rect(5, 5, 20, 20)));
-  GetTray()->PerformAction(tap);
+  GetTray()->PerformAction(
+      CreateTapEvent(OverviewButtonTray::kDoubleTapThresholdMs));
   ASSERT_TRUE(Shell::Get()->window_selector_controller()->IsSelecting());
   EXPECT_EQ(2, user_action_tester.GetActionCount(kTrayOverview));
 
   // Tapping on the control to exit overview mode should record the
   // user action.
-  GetTray()->PerformAction(tap);
+  GetTray()->PerformAction(
+      CreateTapEvent(OverviewButtonTray::kDoubleTapThresholdMs * 2));
   ASSERT_FALSE(Shell::Get()->window_selector_controller()->IsSelecting());
   EXPECT_EQ(3, user_action_tester.GetActionCount(kTrayOverview));
 }