cros: Disable tab scrubbing while window cycle list (alt-tab) is open.

With the ongoing interactivity improvements to the window cycle list
(alt-tab) three finger horizontal trackpad scrolling was introduced as
a way for users to cycle the window cycle list. However, if the user's
most recent active window was a chrome window with multiple tabs, the
three finger swipes would also tab scrub.

To prevent both tab scrubbing and window cycle list cycling, this CL
disables tab scrubbing while a user is cycling windows.

Tests: manual + added test
Bug: 1067327
Change-Id: Ic1ffd1152c3082c2cbc4ab198f5bf0f0c4540037
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2437155
Commit-Queue: Jeremy Chinsen <chinsenj@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812304}

ash/shell_delegate.h

@@ -68,6 +68,9 @@ class ASH_EXPORT ShellDelegate {
   // Check whether the current tab of the browser window can go back.
   virtual bool CanGoBack(gfx::NativeWindow window) const = 0;
 
+  // Sets the tab scrubber |enabled_| field to |enabled|.
+  virtual void SetTabScrubberEnabled(bool enabled) = 0;
+
   // Returns true if |window| allows default touch behaviors. If false, it means
   // no default touch behavior is allowed (i.e., the touch action of window is
   // cc::TouchAction::kNone). This function is used by BackGestureEventHandler

ash/test_shell_delegate.cc

@@ -48,6 +48,10 @@ bool TestShellDelegate::CanGoBack(gfx::NativeWindow window) const {
   return can_go_back_;
 }
 
+void TestShellDelegate::SetTabScrubberEnabled(bool enabled) {
+  tab_scrubber_enabled_ = enabled;
+}
+
 bool TestShellDelegate::ShouldWaitForTouchPressAck(gfx::NativeWindow window) {
   return should_wait_for_touch_ack_;
 }

ash/test_shell_delegate.h

@@ -39,6 +39,7 @@ class TestShellDelegate : public ShellDelegate {
   CreateBackGestureContextualNudgeDelegate(
       BackGestureContextualNudgeController* controller) override;
   bool CanGoBack(gfx::NativeWindow window) const override;
+  void SetTabScrubberEnabled(bool enabled) override;
   bool ShouldWaitForTouchPressAck(gfx::NativeWindow window) override;
   void BindNavigableContentsFactory(
       mojo::PendingReceiver<content::mojom::NavigableContentsFactory> receiver)
@@ -57,6 +58,9 @@ class TestShellDelegate : public ShellDelegate {
   // True if the current top window can go back.
   bool can_go_back_ = true;
 
+  // True if the tab scrubber is enabled.
+  bool tab_scrubber_enabled_ = true;
+
   // True if when performing back gesture on the top window, we should handle
   // the event after the touch ack is received. Please refer to
   // |BackGestureEventHandler::should_wait_for_touch_ack_| for detailed

ash/wm/window_cycle_list.cc

@@ -16,6 +16,7 @@
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/public/cpp/window_properties.h"
 #include "ash/shell.h"
+#include "ash/shell_delegate.h"
 #include "ash/style/ash_color_provider.h"
 #include "ash/wm/window_mini_view.h"
 #include "ash/wm/window_preview_view.h"
@@ -38,8 +39,8 @@
 #include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_delegate.h"
-#include "ui/wm/core/window_animations.h"
 #include "ui/wm/core/coordinate_conversion.h"
+#include "ui/wm/core/window_animations.h"
 
 namespace ash {
 
@@ -520,6 +521,10 @@ WindowCycleList::WindowCycleList(const WindowList& windows)
     window->AddObserver(this);
 
   if (ShouldShowUi()) {
+    // Disable the tab scrubber so three finger scrolling doesn't scrub tabs as
+    // well.
+    Shell::Get()->shell_delegate()->SetTabScrubberEnabled(false);
+
     if (g_disable_initial_delay) {
       InitWindowCycleView();
     } else {
@@ -533,6 +538,8 @@ WindowCycleList::~WindowCycleList() {
   if (!ShouldShowUi())
     Shell::Get()->mru_window_tracker()->SetIgnoreActivations(false);
 
+  Shell::Get()->shell_delegate()->SetTabScrubberEnabled(true);
+
   for (auto* window : windows_)
     window->RemoveObserver(this);
 

chrome/browser/ui/ash/chrome_shell_delegate.cc

@@ -22,6 +22,7 @@
 #include "chrome/browser/ui/ash/chrome_screenshot_grabber.h"
 #include "chrome/browser/ui/ash/keyboard/chrome_keyboard_ui.h"
 #include "chrome/browser/ui/ash/session_util.h"
+#include "chrome/browser/ui/ash/tab_scrubber.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_navigator.h"
@@ -88,6 +89,10 @@ bool ChromeShellDelegate::CanGoBack(gfx::NativeWindow window) const {
   return contents ? contents->GetController().CanGoBack() : false;
 }
 
+void ChromeShellDelegate::SetTabScrubberEnabled(bool enabled) {
+  TabScrubber::GetInstance()->SetEnabled(enabled);
+}
+
 bool ChromeShellDelegate::AllowDefaultTouchActions(gfx::NativeWindow window) {
   content::WebContents* contents =
       GetActiveWebContentsForNativeBrowserWindow(window);

chrome/browser/ui/ash/chrome_shell_delegate.h

@@ -26,6 +26,7 @@ class ChromeShellDelegate : public ash::ShellDelegate {
       ash::BackGestureContextualNudgeController* controller) override;
   void OpenKeyboardShortcutHelpPage() const override;
   bool CanGoBack(gfx::NativeWindow window) const override;
+  void SetTabScrubberEnabled(bool enabled) override;
   bool AllowDefaultTouchActions(gfx::NativeWindow window) override;
   bool ShouldWaitForTouchPressAck(gfx::NativeWindow window) override;
   bool IsTabDrag(const ui::OSExchangeData& drop_data) override;

chrome/browser/ui/ash/tab_scrubber.cc

@@ -67,6 +67,10 @@ bool TabScrubber::IsActivationPending() {
   return activate_timer_.IsRunning();
 }
 
+void TabScrubber::SetEnabled(bool enabled) {
+  enabled_ = enabled;
+}
+
 TabScrubber::TabScrubber() {
   // TODO(mash): Add window server API to observe swipe gestures. Observing
   // gestures on browser windows is not sufficient, as this feature works when
@@ -80,6 +84,9 @@ TabScrubber::~TabScrubber() {
 }
 
 void TabScrubber::OnScrollEvent(ui::ScrollEvent* event) {
+  if (!enabled_)
+    return;
+
   if (event->type() == ui::ET_SCROLL_FLING_CANCEL ||
       event->type() == ui::ET_SCROLL_FLING_START) {
     FinishScrub(true);

chrome/browser/ui/ash/tab_scrubber.h

@@ -43,6 +43,8 @@ class TabScrubber : public ui::EventHandler,
   int highlighted_tab() const { return highlighted_tab_; }
   bool IsActivationPending();
 
+  void SetEnabled(bool enabled);
+
  private:
   friend class TabScrubberTest;
 
@@ -76,6 +78,8 @@ class TabScrubber : public ui::EventHandler,
 
   void UpdateHighlightedTab(Tab* new_tab, int new_index);
 
+  bool GetEnabledForTesting() const { return enabled_; }
+
   // Are we currently scrubbing?.
   bool scrubbing_ = false;
   // The last browser we used for scrubbing, NULL if |scrubbing_| is false and
@@ -101,6 +105,11 @@ class TabScrubber : public ui::EventHandler,
   // The time at which scrubbing started. Needed for UMA reporting of scrubbing
   // duration.
   base::TimeTicks scrubbing_start_time_;
+  // If |enabled_|, tab scrubber takes events and determines whether tabs should
+  // scrub. If not |enabled_|, tab scrubber ignores events. Should be disabled
+  // when clashing interactions can occur, like window cycle list scrolling
+  // gesture.
+  bool enabled_ = true;
 
   DISALLOW_COPY_AND_ASSIGN(TabScrubber);
 };

chrome/browser/ui/ash/tab_scrubber_browsertest.cc

@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "ash/display/event_transformation_handler.h"
+#include "ash/public/cpp/ash_features.h"
 #include "ash/shell.h"
 #include "base/command_line.h"
 #include "base/macros.h"
@@ -91,6 +92,12 @@ class TabScrubberTest : public InProcessBrowserTest,
   }
 
   // InProcessBrowserTest:
+  void SetUp() override {
+    scoped_feature_list_.InitAndEnableFeature(
+        ash::features::kInteractiveWindowCycleList);
+    InProcessBrowserTest::SetUp();
+  }
+
   void SetUpOnMainThread() override {
     TabScrubber::GetInstance()->use_default_activation_delay_ = false;
     // Disable external monitor scaling of coordinates.
@@ -216,6 +223,25 @@ class TabScrubberTest : public InProcessBrowserTest,
     browser->tab_strip_model()->RemoveObserver(this);
   }
 
+  // Sends alt-tab key press event to start the window cycle list.
+  void StartCyclingWindows(Browser* browser) {
+    auto event_generator = CreateEventGenerator(browser);
+    // Views use VKEY_MENU for both left and right Alt keys.
+    event_generator->PressKey(ui::VKEY_MENU, ui::EF_NONE);
+    event_generator->PressKey(ui::KeyboardCode::VKEY_TAB, ui::EF_ALT_DOWN);
+    event_generator->ReleaseKey(ui::KeyboardCode::VKEY_TAB, ui::EF_ALT_DOWN);
+  }
+
+  // Sends alt-tab key release event to start the window cycle list.
+  void StopCyclingWindows(Browser* browser) {
+    auto event_generator = CreateEventGenerator(browser);
+    event_generator->ReleaseKey(ui::VKEY_MENU, ui::EF_NONE);
+  }
+
+  bool IsTabScrubberEnabled() {
+    return TabScrubber::GetInstance()->GetEnabledForTesting();
+  }
+
   void AddTabs(Browser* browser, int num_tabs) {
     TabStrip* tab_strip = GetTabStrip(browser);
     for (int i = 0; i < num_tabs; ++i)
@@ -291,6 +317,8 @@ class TabScrubberTest : public InProcessBrowserTest,
     return std::make_unique<ui::test::EventGenerator>(root, window);
   }
 
+  base::test::ScopedFeatureList scoped_feature_list_;
+
   DISALLOW_COPY_AND_ASSIGN(TabScrubberTest);
 };
 
@@ -568,3 +596,33 @@ IN_PROC_BROWSER_TEST_F(TabScrubberTest, RTLMoveBefore) {
   browser()->tab_strip_model()->MoveSelectedTabsTo(2);
   EXPECT_EQ(0, TabScrubber::GetInstance()->highlighted_tab());
 }
+
+// If the window cycle list is open, the tab scrubber should be disabled.
+IN_PROC_BROWSER_TEST_F(TabScrubberTest, DisabledIfWindowCycleListOpen) {
+  AddTabs(browser(), 4);
+
+  // Create a second browser, but don't make it active.
+  Browser* browser2 = CreateBrowser(browser()->profile());
+  browser()->window()->Activate();
+  ASSERT_FALSE(browser2->window()->IsActive());
+  ASSERT_TRUE(browser()->window()->IsActive());
+
+  // Open window cycle list. It should be open now so tab scrubber should be
+  // disabled.
+  StartCyclingWindows(browser());
+  EXPECT_FALSE(IsTabScrubberEnabled());
+  Scrub(browser(), 0, EACH_TAB);
+  EXPECT_EQ(0u, activation_order_.size());
+  EXPECT_EQ(4, browser()->tab_strip_model()->active_index());
+
+  // Stop cycling. Scrub should work again.
+  StopCyclingWindows(browser());
+  EXPECT_TRUE(IsTabScrubberEnabled());
+  Scrub(browser(), 0, EACH_TAB);
+  ASSERT_EQ(4U, activation_order_.size());
+  EXPECT_EQ(3, activation_order_[0]);
+  EXPECT_EQ(2, activation_order_[1]);
+  EXPECT_EQ(1, activation_order_[2]);
+  EXPECT_EQ(0, activation_order_[3]);
+  EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
+}