Don't treat minimized windows as lower priority in MRU list (for Alt+Tab,

Overview, etc.)

BUG=635762
TBR=derat@chromium.org

Review-Url: https://codereview.chromium.org/2258703002
Cr-Commit-Position: refs/heads/master@{#415316}

ash/common/wm/mru_window_tracker.cc

@@ -34,14 +34,6 @@ void AddTrackedWindows(WmWindow* root,
   windows->insert(windows->end(), children.begin(), children.end());
 }
 
-// Returns whether |w1| should be considered less recently used than |w2|. This
-// is used for a stable sort to move minimized windows to the LRU end of the
-// list.
-bool CompareWindowState(WmWindow* w1, WmWindow* w2) {
-  return w1->GetWindowState()->IsMinimized() &&
-         !w2->GetWindowState()->IsMinimized();
-}
-
 // Returns a list of windows ordered by their stacking order.
 // If |mru_windows| is passed, these windows are moved to the front of the list.
 // It uses the given |should_include_window_predicate| to determine whether to
@@ -95,9 +87,6 @@ MruWindowTracker::WindowList BuildWindowListInternal(
     }
   }
 
-  // Move minimized windows to the beginning (LRU end) of the list.
-  std::stable_sort(windows.begin(), windows.end(), CompareWindowState);
-
   // Window cycling expects the topmost window at the front of the list.
   std::reverse(windows.begin(), windows.end());
 

ash/common/wm/overview/window_grid.cc

@@ -392,8 +392,10 @@ WindowGrid::WindowGrid(WmWindow* root_window,
                        WindowSelector* window_selector)
     : root_window_(root_window),
       window_selector_(window_selector),
+      window_observer_(this),
       selected_index_(0),
-      num_columns_(0) {
+      num_columns_(0),
+      prepared_for_overview_(false) {
   std::vector<WmWindow*> windows_in_root;
   for (auto* window : windows) {
     if (window->GetRootWindow() == root_window)
@@ -409,16 +411,12 @@ WindowGrid::WindowGrid(WmWindow* root_window,
                                     window_selector_->text_filter_bottom());
   }
   for (auto* window : windows_in_root) {
-    window->AddObserver(this);
-    observed_windows_.insert(window);
+    window_observer_.Add(window);
     window_list_.push_back(new WindowSelectorItem(window, window_selector_));
   }
 }
 
-WindowGrid::~WindowGrid() {
-  for (WmWindow* window : observed_windows_)
-    window->RemoveObserver(this);
-}
+WindowGrid::~WindowGrid() {}
 
 void WindowGrid::Shutdown() {
   if (shield_widget_) {
@@ -453,6 +451,7 @@ void WindowGrid::PrepareForOverview() {
     InitShieldWidget();
   for (auto iter = window_list_.begin(); iter != window_list_.end(); ++iter)
     (*iter)->PrepareForOverview();
+  prepared_for_overview_ = true;
 }
 
 void WindowGrid::PositionWindowsMD(bool animate) {
@@ -787,8 +786,7 @@ void WindowGrid::WindowClosing(WindowSelectorItem* window) {
 }
 
 void WindowGrid::OnWindowDestroying(WmWindow* window) {
-  window->RemoveObserver(this);
-  observed_windows_.erase(window);
+  window_observer_.Remove(window);
   ScopedVector<WindowSelectorItem>::iterator iter =
       std::find_if(window_list_.begin(), window_list_.end(),
                    WindowSelectorItemComparator(window));
@@ -821,6 +819,12 @@ void WindowGrid::OnWindowDestroying(WmWindow* window) {
 void WindowGrid::OnWindowBoundsChanged(WmWindow* window,
                                        const gfx::Rect& old_bounds,
                                        const gfx::Rect& new_bounds) {
+  // During preparation, window bounds can change (e.g. by unminimizing a
+  // window). Ignore bounds change notifications in this case; we'll reposition
+  // soon.
+  if (!prepared_for_overview_)
+    return;
+
   auto iter = std::find_if(window_list_.begin(), window_list_.end(),
                            WindowSelectorItemComparator(window));
   DCHECK(iter != window_list_.end());

ash/common/wm/overview/window_grid.h

@@ -15,6 +15,7 @@
 #include "ash/common/wm_window_observer.h"
 #include "base/macros.h"
 #include "base/memory/scoped_vector.h"
+#include "base/scoped_observer.h"
 
 namespace views {
 class Widget;
@@ -172,8 +173,7 @@ class ASH_EXPORT WindowGrid : public WmWindowObserver {
   // Vector containing all the windows in this grid.
   ScopedVector<WindowSelectorItem> window_list_;
 
-  // Vector containing the observed windows.
-  std::set<WmWindow*> observed_windows_;
+  ScopedObserver<WmWindow, WindowGrid> window_observer_;
 
   // Widget that darkens the screen background.
   std::unique_ptr<views::Widget> shield_widget_;
@@ -190,6 +190,9 @@ class ASH_EXPORT WindowGrid : public WmWindowObserver {
   // Number of columns in the grid.
   size_t num_columns_;
 
+  // True only after all windows have been prepared for overview.
+  bool prepared_for_overview_;
+
   DISALLOW_COPY_AND_ASSIGN(WindowGrid);
 };
 

ash/mus/wm/mru_window_tracker_unittest.cc

@@ -46,8 +46,7 @@ TEST_F(MruWindowTrackerTest, Basic) {
   EXPECT_EQ(w3, window_list[2]);
 }
 
-// Test that minimized windows are considered least recently used (but kept in
-// correct relative order).
+// Test that minimized windows are not treated specially.
 TEST_F(MruWindowTrackerTest, MinimizedWindowsAreLru) {
   WmWindow* w1 = CreateTestWindow();
   WmWindow* w2 = CreateTestWindow();
@@ -66,15 +65,13 @@ TEST_F(MruWindowTrackerTest, MinimizedWindowsAreLru) {
   w4->GetWindowState()->Minimize();
   w5->GetWindowState()->Minimize();
 
-  // Expect the relative order of minimized windows to remain the same, but all
-  // minimized windows to be at the end of the list.
   WmWindow::Windows window_list = mru_window_tracker()->BuildMruWindowList();
-  EXPECT_EQ(w2, window_list[0]);
-  EXPECT_EQ(w3, window_list[1]);
-  EXPECT_EQ(w6, window_list[2]);
-  EXPECT_EQ(w1, window_list[3]);
-  EXPECT_EQ(w4, window_list[4]);
-  EXPECT_EQ(w5, window_list[5]);
+  EXPECT_EQ(w1, window_list[0]);
+  EXPECT_EQ(w2, window_list[1]);
+  EXPECT_EQ(w3, window_list[2]);
+  EXPECT_EQ(w4, window_list[3]);
+  EXPECT_EQ(w5, window_list[4]);
+  EXPECT_EQ(w6, window_list[5]);
 }
 
 // Tests that windows being dragged are only in the WindowList once.

ash/wm/mru_window_tracker_unittest.cc

@@ -51,8 +51,7 @@ TEST_F(MruWindowTrackerTest, Basic) {
   EXPECT_EQ(w3.get(), window_list[2]);
 }
 
-// Test that minimized windows are considered least recently used (but kept in
-// correct relative order).
+// Test that minimized windows are not treated specially.
 TEST_F(MruWindowTrackerTest, MinimizedWindowsAreLru) {
   std::unique_ptr<aura::Window> w1(CreateWindow());
   std::unique_ptr<aura::Window> w2(CreateWindow());
@@ -71,16 +70,18 @@ TEST_F(MruWindowTrackerTest, MinimizedWindowsAreLru) {
   wm::GetWindowState(w4.get())->Minimize();
   wm::GetWindowState(w5.get())->Minimize();
 
-  // Expect the relative order of minimized windows to remain the same, but all
-  // minimized windows to be at the end of the list.
+  // By minimizing the first window, we activate w2 which will move it to the
+  // front of the MRU queue.
+  EXPECT_TRUE(wm::GetWindowState(w2.get())->IsActive());
+
   aura::Window::Windows window_list =
       WmWindowAura::ToAuraWindows(mru_window_tracker()->BuildMruWindowList());
   EXPECT_EQ(w2.get(), window_list[0]);
-  EXPECT_EQ(w3.get(), window_list[1]);
-  EXPECT_EQ(w6.get(), window_list[2]);
-  EXPECT_EQ(w1.get(), window_list[3]);
-  EXPECT_EQ(w4.get(), window_list[4]);
-  EXPECT_EQ(w5.get(), window_list[5]);
+  EXPECT_EQ(w1.get(), window_list[1]);
+  EXPECT_EQ(w3.get(), window_list[2]);
+  EXPECT_EQ(w4.get(), window_list[3]);
+  EXPECT_EQ(w5.get(), window_list[4]);
+  EXPECT_EQ(w6.get(), window_list[5]);
 }
 
 // Tests that windows being dragged are only in the WindowList once.