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

This reverts commit dc64daa2.

Reason for revert: Consistent failures on ASan/LSan builds:
https://ci.chromium.org/p/chromium/builders/ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/38493

==7494==ERROR: AddressSanitizer: container-overflow on address 0x60200014be98 at pc 0x55bf547a29f5 bp 0x7ffe684dac20 sp 0x7ffe684dac18
READ of size 8 at 0x60200014be98 thread T0
    #0 0x55bf547a29f4 in ash::WindowCycleList::ReplaceWindows(std::__1::vector<aura::Window*, std::__1::allocator<aura::Window*> > const&) ./../../ash/wm/window_cycle_list.cc:541
    #1 0x55bf547a29f4 in ?? ??:0
    #2 0x55bf5479fb9c in ash::WindowCycleController::MaybeResetCycleList() ./../../ash/wm/window_cycle_controller.cc:126
    #3 0x55bf5479fb9c in ?? ??:0
    #4 0x55bf5467e1a5 in ash::DesksController::ActivateDeskInternal(ash::Desk const*, bool) ./../../ash/wm/desks/desks_controller.cc:553
    #5 0x55bf5467e1a5 in ?? ??:0
    #6 0x55bf54688fb2 in ash::DeskActivationAnimation::PrepareDeskForScreenshot(int) ./../../ash/wm/desks/desk_animation_impl.cc:171
    #7 0x55bf54688fb2 in ?? ??:0
    #8 0x55bf54689402 in ash::DeskActivationAnimation::OnStartingDeskScreenshotTakenInternal(int) ./../../ash/wm/desks/desk_animation_impl.cc:128
    #9 0x55bf54689402 in ?? ??:0
    #10 0x55bf5468722c in ash::DeskAnimationBase::OnStartingDeskScreenshotTaken(int) ./../../ash/wm/desks/desk_animation_base.cc:96

Original change's description:
> 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}

TBR=xdai@chromium.org,chinsenj@chromium.org

Change-Id: I60f4c419f33a2c0f5d749b50cfb2eda2ee06cb44
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1067327
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2410029Reviewed-by: Sergey Poromov <poromov@chromium.org>
Commit-Queue: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806937}

ash/wm/desks/desks_controller.cc

@@ -27,7 +27,6 @@
 #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"
@@ -546,13 +545,6 @@ 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,8 +30,7 @@ namespace {
 
 // Returns the most recently active window from the |window_list| or nullptr
 // if the list is empty.
-aura::Window* GetActiveWindow(
-    const WindowCycleController::WindowList& window_list) {
+aura::Window* GetActiveWindow(const WindowCycleList::WindowList& window_list) {
   return window_list.empty() ? nullptr : window_list[0];
 }
 
@@ -96,8 +95,18 @@ void WindowCycleController::StartCycling() {
   // (http://crbug.com/895265).
   Shell::Get()->wallpaper_controller()->MaybeClosePreviewWallpaper();
 
-  WindowCycleController::WindowList window_list = CreateWindowList();
-  SaveCurrentActiveDeskAndWindow(window_list);
+  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);
 
   window_cycle_list_ = std::make_unique<WindowCycleList>(window_list);
   event_filter_ = std::make_unique<WindowCycleEventFilter>();
@@ -115,17 +124,6 @@ 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);
@@ -138,25 +136,6 @@ 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,8 +34,6 @@ class WindowCycleList;
 // order.
 class ASH_EXPORT WindowCycleController {
  public:
-  using WindowList = std::vector<aura::Window*>;
-
   enum Direction { FORWARD, BACKWARD };
 
   WindowCycleController();
@@ -62,10 +60,6 @@ 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);
 
@@ -78,16 +72,6 @@ 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,20 +850,6 @@ 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,24 +296,6 @@ 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());
 
@@ -532,25 +514,6 @@ 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;
@@ -617,29 +580,6 @@ 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);
@@ -656,6 +596,25 @@ void WindowCycleList::RemoveWindow(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,10 +42,6 @@ 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);
 
@@ -79,10 +75,6 @@ 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();