cros: Window cycle list (alt-tab) resets if open during desk swap.

While the flag for limiting windows to the current active desk is
enabled, if a user swaps desks while the window cycle list is open, the
contents will not update to the new active desk.

This CL makes the window cycle list reset if it is open and a desk
swap occurs while the aforementioned flag is enabled.

Test: LimitedWindowCycleControllerTest.CycleShowsActiveDeskWindows
Bug: 1067327
Change-Id: I9d940ad82a2f05ab1d2fd9e8c5115d5f3eda9826
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2398559
Commit-Queue: Jeremy Chinsen <chinsenj@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806749}

ash/wm/desks/desks_controller.cc

@@ -27,6 +27,7 @@
 #include "ash/wm/overview/overview_item.h"
 #include "ash/wm/splitview/split_view_controller.h"
 #include "ash/wm/splitview/split_view_utils.h"
+#include "ash/wm/window_cycle_controller.h"
 #include "ash/wm/window_util.h"
 #include "base/auto_reset.h"
 #include "base/check_op.h"
@@ -545,6 +546,13 @@ void DesksController::ActivateDeskInternal(const Desk* desk,
 
   MaybeUpdateShelfItems(old_active->windows(), active_desk_->windows());
 
+  // If in the middle of a window cycle gesture, reset the window cycle list
+  // contents so it contains the new active desk's windows.
+  if (features::IsAltTabLimitedToActiveDesk()) {
+    auto* window_cycle_controller = Shell::Get()->window_cycle_controller();
+    window_cycle_controller->MaybeResetCycleList();
+  }
+
   for (auto& observer : observers_)
     observer.OnDeskActivationChanged(active_desk_, old_active);
 }

ash/wm/window_cycle_controller.cc

@@ -30,7 +30,8 @@ namespace {
 
 // Returns the most recently active window from the |window_list| or nullptr
 // if the list is empty.
-aura::Window* GetActiveWindow(const WindowCycleList::WindowList& window_list) {
+aura::Window* GetActiveWindow(
+    const WindowCycleController::WindowList& window_list) {
   return window_list.empty() ? nullptr : window_list[0];
 }
 
@@ -95,18 +96,8 @@ void WindowCycleController::StartCycling() {
   // (http://crbug.com/895265).
   Shell::Get()->wallpaper_controller()->MaybeClosePreviewWallpaper();
 
-  WindowCycleList::WindowList window_list =
-      Shell::Get()->mru_window_tracker()->BuildWindowForCycleWithPipList(
-          features::IsAltTabLimitedToActiveDesk() ? kActiveDesk : kAllDesks);
-  // Window cycle list windows will handle showing their transient related
-  // windows, so if a window in |window_list| has a transient root also in
-  // |window_list|, we can remove it as the transient root will handle showing
-  // the window.
-  window_util::RemoveTransientDescendants(&window_list);
-
-  active_desk_container_id_before_cycle_ =
-      desks_util::GetActiveDeskContainerId();
-  active_window_before_window_cycle_ = GetActiveWindow(window_list);
+  WindowCycleController::WindowList window_list = CreateWindowList();
+  SaveCurrentActiveDeskAndWindow(window_list);
 
   window_cycle_list_ = std::make_unique<WindowCycleList>(window_list);
   event_filter_ = std::make_unique<WindowCycleEventFilter>();
@@ -124,6 +115,17 @@ void WindowCycleController::CancelCycling() {
   StopCycling();
 }
 
+void WindowCycleController::MaybeResetCycleList() {
+  if (!IsCycling())
+    return;
+
+  WindowCycleController::WindowList window_list = CreateWindowList();
+  SaveCurrentActiveDeskAndWindow(window_list);
+
+  DCHECK(window_cycle_list_);
+  window_cycle_list_->ReplaceWindows(window_list);
+}
+
 void WindowCycleController::StepToWindow(aura::Window* window) {
   DCHECK(window_cycle_list_);
   window_cycle_list_->StepToWindow(window);
@@ -136,6 +138,25 @@ bool WindowCycleController::IsEventInCycleView(ui::LocatedEvent* event) {
 //////////////////////////////////////////////////////////////////////////////
 // WindowCycleController, private:
 
+WindowCycleController::WindowList WindowCycleController::CreateWindowList() {
+  WindowCycleController::WindowList window_list =
+      Shell::Get()->mru_window_tracker()->BuildWindowForCycleWithPipList(
+          features::IsAltTabLimitedToActiveDesk() ? kActiveDesk : kAllDesks);
+  // Window cycle list windows will handle showing their transient related
+  // windows, so if a window in |window_list| has a transient root also in
+  // |window_list|, we can remove it as the transient root will handle showing
+  // the window.
+  window_util::RemoveTransientDescendants(&window_list);
+  return window_list;
+}
+
+void WindowCycleController::SaveCurrentActiveDeskAndWindow(
+    const WindowCycleController::WindowList& window_list) {
+  active_desk_container_id_before_cycle_ =
+      desks_util::GetActiveDeskContainerId();
+  active_window_before_window_cycle_ = GetActiveWindow(window_list);
+}
+
 void WindowCycleController::Step(Direction direction) {
   DCHECK(window_cycle_list_);
   window_cycle_list_->Step(direction);

ash/wm/window_cycle_controller.h

@@ -34,6 +34,8 @@ class WindowCycleList;
 // order.
 class ASH_EXPORT WindowCycleController {
  public:
+  using WindowList = std::vector<aura::Window*>;
+
   enum Direction { FORWARD, BACKWARD };
 
   WindowCycleController();
@@ -60,6 +62,10 @@ class ASH_EXPORT WindowCycleController {
   void CompleteCycling();
   void CancelCycling();
 
+  // If the window cycle list is open, re-construct it. Do nothing if not
+  // cycling.
+  void MaybeResetCycleList();
+
   // Skip window cycle list directly to |window|.
   void StepToWindow(aura::Window* window);
 
@@ -72,6 +78,16 @@ class ASH_EXPORT WindowCycleController {
   }
 
  private:
+  // Gets a list of windows from the currently open windows, removing windows
+  // with transient roots already in the list. The returned list of windows
+  // is used to populate the window cycle list.
+  WindowList CreateWindowList();
+
+  // Populates |active_desk_container_id_before_cycle_| and
+  // |active_window_before_window_cycle_| when the window cycle list is
+  // initialized.
+  void SaveCurrentActiveDeskAndWindow(const WindowList& window_list);
+
   // Cycles to the next or previous window based on |direction|.
   void Step(Direction direction);
 

ash/wm/window_cycle_controller_unittest.cc

@@ -850,6 +850,20 @@ TEST_F(LimitedWindowCycleControllerTest, CycleShowsActiveDeskWindows) {
   EXPECT_TRUE(base::Contains(cycle_windows, win1.get()));
   cycle_controller->CompleteCycling();
   EXPECT_EQ(win0.get(), window_util::GetActiveWindow());
+
+  // Swap desks while cycling, contents should update.
+  cycle_controller->HandleCycleWindow(WindowCycleController::FORWARD);
+  cycle_windows = GetWindows(cycle_controller);
+  EXPECT_EQ(2u, cycle_windows.size());
+  EXPECT_TRUE(base::Contains(cycle_windows, win0.get()));
+  EXPECT_TRUE(base::Contains(cycle_windows, win1.get()));
+  ActivateDesk(desk_2);
+  EXPECT_TRUE(cycle_controller->IsCycling());
+  cycle_windows = GetWindows(cycle_controller);
+  EXPECT_EQ(1u, cycle_windows.size());
+  EXPECT_TRUE(base::Contains(cycle_windows, win2.get()));
+  cycle_controller->CompleteCycling();
+  EXPECT_EQ(win2.get(), window_util::GetActiveWindow());
 }
 
 class InteractiveWindowCycleControllerTest : public WindowCycleControllerTest {

ash/wm/window_cycle_list.cc

@@ -296,6 +296,24 @@ class WindowCycleView : public views::WidgetDelegateView,
   WindowCycleView& operator=(const WindowCycleView&) = delete;
   ~WindowCycleView() override = default;
 
+  void UpdateWindows(const WindowCycleList::WindowList& windows) {
+    for (auto* window : windows) {
+      auto* view = mirror_container_->AddChildView(
+          std::make_unique<WindowCycleItemView>(window));
+      window_view_map_[window] = view;
+
+      no_previews_set_.insert(view);
+    }
+
+    // Resize the widget.
+    aura::Window* root_window = Shell::GetRootWindowForNewWindows();
+    gfx::Rect widget_rect = root_window->GetBoundsInScreen();
+    widget_rect.ClampToCenteredSize(GetPreferredSize());
+    GetWidget()->SetBounds(widget_rect);
+
+    SetTargetWindow(windows[0]);
+  }
+
   void FadeInLayer() {
     DCHECK(GetWidget());
 
@@ -514,6 +532,25 @@ WindowCycleList::~WindowCycleList() {
   Shell::Get()->frame_throttling_controller()->EndThrottling();
 }
 
+void WindowCycleList::ReplaceWindows(const WindowList& windows) {
+  if (windows.empty()) {
+    Shell::Get()->window_cycle_controller()->CancelCycling();
+    return;
+  }
+
+  for (auto* existing_window : windows_)
+    RemoveWindow(existing_window);
+
+  current_index_ = 0;
+  windows_ = windows;
+
+  for (auto* new_window : windows_)
+    new_window->AddObserver(this);
+
+  if (ShouldShowUi() && cycle_view_)
+    cycle_view_->UpdateWindows(windows_);
+}
+
 void WindowCycleList::Step(int offset) {
   if (windows_.empty())
     return;
@@ -580,6 +617,29 @@ void WindowCycleList::DisableInitialDelayForTesting() {
 }
 
 void WindowCycleList::OnWindowDestroying(aura::Window* window) {
+  RemoveWindow(window);
+
+  if (cycle_view_ && windows_.empty()) {
+    // This deletes us.
+    Shell::Get()->window_cycle_controller()->CancelCycling();
+  }
+}
+
+void WindowCycleList::OnDisplayMetricsChanged(const display::Display& display,
+                                              uint32_t changed_metrics) {
+  if (cycle_ui_widget_ &&
+      display.id() ==
+          display::Screen::GetScreen()
+              ->GetDisplayNearestWindow(cycle_ui_widget_->GetNativeWindow())
+              .id() &&
+      (changed_metrics & (DISPLAY_METRIC_BOUNDS | DISPLAY_METRIC_ROTATION))) {
+    Shell::Get()->window_cycle_controller()->CancelCycling();
+    // |this| is deleted.
+    return;
+  }
+}
+
+void WindowCycleList::RemoveWindow(aura::Window* window) {
   window->RemoveObserver(this);
 
   WindowList::iterator i = std::find(windows_.begin(), windows_.end(), window);
@@ -596,25 +656,6 @@ void WindowCycleList::OnWindowDestroying(aura::Window* window) {
     auto* new_target_window =
         windows_.empty() ? nullptr : windows_[current_index_];
     cycle_view_->HandleWindowDestruction(window, new_target_window);
-    if (windows_.empty()) {
-      // This deletes us.
-      Shell::Get()->window_cycle_controller()->CancelCycling();
-      return;
-    }
-  }
-}
-
-void WindowCycleList::OnDisplayMetricsChanged(const display::Display& display,
-                                              uint32_t changed_metrics) {
-  if (cycle_ui_widget_ &&
-      display.id() ==
-          display::Screen::GetScreen()
-              ->GetDisplayNearestWindow(cycle_ui_widget_->GetNativeWindow())
-              .id() &&
-      (changed_metrics & (DISPLAY_METRIC_BOUNDS | DISPLAY_METRIC_ROTATION))) {
-    Shell::Get()->window_cycle_controller()->CancelCycling();
-    // |this| is deleted.
-    return;
   }
 }
 

ash/wm/window_cycle_list.h

@@ -42,6 +42,10 @@ class ASH_EXPORT WindowCycleList : public aura::WindowObserver,
   WindowCycleList& operator=(const WindowCycleList&) = delete;
   ~WindowCycleList() override;
 
+  // Removes the existing windows and replaces them with |windows|. If
+  // |windows| is empty, cancels cycling.
+  void ReplaceWindows(const WindowList& windows);
+
   // Cycles to the next or previous window based on |direction|.
   void Step(WindowCycleController::Direction direction);
 
@@ -75,6 +79,10 @@ class ASH_EXPORT WindowCycleList : public aura::WindowObserver,
   void OnDisplayMetricsChanged(const display::Display& display,
                                uint32_t changed_metrics) override;
 
+  // Removes |window| from the window list. Also removes the window from
+  // |cycle_view_| if |cycle_view_| exists.
+  void RemoveWindow(aura::Window* window);
+
   // Returns true if the window list overlay should be shown.
   bool ShouldShowUi();