split view: Restack windows reinserted to overview

Test: ash_unittests All/SplitViewOverviewSessionTest.SplitViewWindowReinsertedToOverviewAtCorrectPositionWhen*
Bug: None
Change-Id: I14d6ae52642af223627559b052dc8dc7ae0f6148
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2024148Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Avery Musbach <amusbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736649}

ash/wm/overview/overview_grid.cc

@@ -518,7 +518,8 @@ void OverviewGrid::AddItem(aura::Window* window,
                            bool animate,
                            const base::flat_set<OverviewItem*>& ignored_items,
                            size_t index,
-                           bool use_spawn_animation) {
+                           bool use_spawn_animation,
+                           bool restack) {
   DCHECK(!GetOverviewItemContaining(window));
   DCHECK_LE(index, window_list_.size());
 
@@ -539,6 +540,12 @@ void OverviewGrid::AddItem(aura::Window* window,
     item->OnStartingAnimationComplete();
   }
 
+  if (restack) {
+    if (reposition && animate)
+      item->set_should_restack_on_animation_end(true);
+    else
+      item->Restack();
+  }
   if (reposition)
     PositionWindows(animate, ignored_items);
 }
@@ -548,14 +555,15 @@ void OverviewGrid::AppendItem(aura::Window* window,
                               bool animate,
                               bool use_spawn_animation) {
   AddItem(window, reposition, animate, /*ignored_items=*/{},
-          window_list_.size(), use_spawn_animation);
+          window_list_.size(), use_spawn_animation, /*restack=*/false);
 }
 
 void OverviewGrid::AddItemInMruOrder(aura::Window* window,
                                      bool reposition,
-                                     bool animate) {
+                                     bool animate,
+                                     bool restack) {
   AddItem(window, reposition, animate, /*ignored_items=*/{},
-          FindInsertionIndex(window), /*use_spawn_animation=*/false);
+          FindInsertionIndex(window), /*use_spawn_animation=*/false, restack);
 }
 
 void OverviewGrid::RemoveItem(OverviewItem* overview_item,

ash/wm/overview/overview_grid.h

@@ -93,12 +93,17 @@ class ASH_EXPORT OverviewGrid : public SplitViewObserver,
   // overview is already active, it will use a special spawn animation on its
   // first position in the grid. |use_spawn_animation| has no effect if either
   // |animate| or |reposition| are false.
+  // If |reposition|, |animate|, and |restack| are all true, the stacking order
+  // will be adjusted after the animation. If |restack| is true but at least one
+  // of |reposition| and |animate| is false, the stacking order will be adjusted
+  // immediately.
   void AddItem(aura::Window* window,
                bool reposition,
                bool animate,
                const base::flat_set<OverviewItem*>& ignored_items,
                size_t index,
-               bool use_spawn_animation);
+               bool use_spawn_animation,
+               bool restack);
 
   // Similar to the above function, but adds the window to the end of the grid.
   void AppendItem(aura::Window* window,
@@ -108,7 +113,10 @@ class ASH_EXPORT OverviewGrid : public SplitViewObserver,
 
   // Like |AddItem|, but adds |window| at the correct position according to MRU
   // order.
-  void AddItemInMruOrder(aura::Window* window, bool reposition, bool animate);
+  void AddItemInMruOrder(aura::Window* window,
+                         bool reposition,
+                         bool animate,
+                         bool restack);
 
   // Removes |overview_item| from the grid. |overview_item| cannot already be
   // absent from the grid. If |item_destroying| is true, we may want to notify

ash/wm/overview/overview_item.cc

@@ -653,15 +653,7 @@ bool OverviewItem::IsDragItem() {
          overview_session_->window_drag_controller()->item() == this;
 }
 
-void OverviewItem::OnDragAnimationCompleted() {
-  // This is function is called whenever the grid repositions its windows, but
-  // we only need to restack the windows if an item was being dragged around
-  // and then released.
-  if (!should_restack_on_animation_end_)
-    return;
-
-  should_restack_on_animation_end_ = false;
-
+void OverviewItem::Restack() {
   // First stack this item's window below the snapped window if split view
   // mode is active.
   aura::Window* dragged_window = GetWindow();
@@ -1110,7 +1102,10 @@ void OverviewItem::OnWindowCloseAnimationCompleted() {
 
 void OverviewItem::OnItemSpawnedAnimationCompleted() {
   UpdateRoundedCornersAndShadow();
-  OnDragAnimationCompleted();
+  if (should_restack_on_animation_end_) {
+    Restack();
+    should_restack_on_animation_end_ = false;
+  }
   OnStartingAnimationComplete();
 }
 
@@ -1129,7 +1124,10 @@ void OverviewItem::OnItemBoundsAnimationEnded() {
     return;
 
   UpdateRoundedCornersAndShadow();
-  OnDragAnimationCompleted();
+  if (should_restack_on_animation_end_) {
+    Restack();
+    should_restack_on_animation_end_ = false;
+  }
 }
 
 void OverviewItem::PerformItemSpawnedAnimation(

ash/wm/overview/overview_item.h

@@ -158,11 +158,9 @@ class ASH_EXPORT OverviewItem : public views::ButtonListener,
   // Checks if this item is current being dragged.
   bool IsDragItem();
 
-  // Called after a positioning transform animation ends. Checks to see if the
-  // animation was triggered by a drag end event. If so, inserts the window back
-  // to its original stacking order so that the order of windows is the same as
-  // when entering overview.
-  void OnDragAnimationCompleted();
+  // Inserts the window back to its original stacking order so that the order of
+  // windows is the same as when entering overview.
+  void Restack();
 
   // Updates |phantoms_for_dragging_|. If |phantoms_for_dragging_| is null, then
   // a new object is created for it.

ash/wm/overview/overview_session.cc

@@ -406,7 +406,7 @@ void OverviewSession::AddItem(
     return;
 
   grid->AddItem(window, reposition, animate, ignored_items, index,
-                /*use_spawn_animation=*/false);
+                /*use_spawn_animation=*/false, /*restack=*/false);
   OnItemAdded(window);
 }
 
@@ -422,13 +422,15 @@ void OverviewSession::AppendItem(aura::Window* window,
   OnItemAdded(window);
 }
 
-void OverviewSession::AddItemInMruOrder(aura::Window* window, bool animate) {
+void OverviewSession::AddItemInMruOrder(aura::Window* window,
+                                        bool animate,
+                                        bool restack) {
   // Early exit if a grid already contains |window|.
   OverviewGrid* grid = GetGridWithRootWindow(window->GetRootWindow());
   if (!grid || grid->GetOverviewItemContaining(window))
     return;
 
-  grid->AddItemInMruOrder(window, /*reposition=*/true, animate);
+  grid->AddItemInMruOrder(window, /*reposition=*/true, animate, restack);
   OnItemAdded(window);
 }
 

ash/wm/overview/overview_session.h

@@ -179,8 +179,11 @@ class ASH_EXPORT OverviewSession : public display::DisplayObserver,
   void AppendItem(aura::Window* window, bool reposition, bool animate);
 
   // Similar to |AddItem| with reposition=true, but adds the window at the
-  // correct position according to MRU order.
-  void AddItemInMruOrder(aura::Window* window, bool animate);
+  // correct position according to MRU order. If |animate| and |restack| are
+  // both true, the stacking order will be adjusted after the animation. If
+  // |animate| is false and |restack| is true, the stacking order will be
+  // adjusted immediately.
+  void AddItemInMruOrder(aura::Window* window, bool animate, bool restack);
 
   // Removes |overview_item| from the corresponding grid. No items are
   // repositioned.

ash/wm/overview/overview_session_unittest.cc

@@ -5045,13 +5045,15 @@ TEST_P(SplitViewOverviewSessionTest,
 
 // Verify that if overview mode is active and the split view divider is dragged
 // all the way to the opposite edge, then the split view window is reinserted
-// into the overview grid at the correct position according to MRU order.
+// into the overview grid at the correct position according to MRU order, and
+// the stacking order is also correct.
 TEST_P(
     SplitViewOverviewSessionTest,
     SplitViewWindowReinsertedToOverviewAtCorrectPositionWhenSplitViewIsEnded) {
   const gfx::Rect bounds(400, 400);
   std::unique_ptr<aura::Window> window1(CreateWindow(bounds));
   std::unique_ptr<aura::Window> window2(CreateWindow(bounds));
+  std::unique_ptr<aura::Window> window3(CreateWindow(bounds));
   ToggleOverview();
   DragWindowTo(GetOverviewItemForWindow(window1.get()), gfx::PointF());
   DragWindowTo(GetOverviewItemForWindow(window2.get()),
@@ -5067,21 +5069,44 @@ TEST_P(
   GetEventGenerator()->DragMouseTo(0, 0);
   SkipDividerSnapAnimation();
 
+  // Verify the grid arrangement.
   ASSERT_TRUE(InOverviewSession());
-  const std::vector<aura::Window*> expected_mru_list = {window2.get(),
-                                                        window1.get()};
-  const std::vector<aura::Window*> expected_overview_list = {window2.get(),
-                                                             window1.get()};
+  const std::vector<aura::Window*> expected_mru_list = {
+      window2.get(), window1.get(), window3.get()};
+  const std::vector<aura::Window*> expected_overview_list = {
+      window2.get(), window1.get(), window3.get()};
   EXPECT_EQ(
       expected_mru_list,
       Shell::Get()->mru_window_tracker()->BuildMruWindowList(kActiveDesk));
   EXPECT_EQ(expected_overview_list,
             overview_controller()->GetWindowsListInOverviewGridsForTest());
+
+  // Verify the stacking order.
+  aura::Window* parent = window1->parent();
+  ASSERT_EQ(parent, window2->parent());
+  ASSERT_EQ(parent, window3->parent());
+  EXPECT_GT(IndexOf(GetOverviewItemForWindow(window2.get())
+                        ->item_widget()
+                        ->GetNativeWindow(),
+                    parent),
+            IndexOf(GetOverviewItemForWindow(window1.get())
+                        ->item_widget()
+                        ->GetNativeWindow(),
+                    parent));
+  EXPECT_GT(IndexOf(GetOverviewItemForWindow(window1.get())
+                        ->item_widget()
+                        ->GetNativeWindow(),
+                    parent),
+            IndexOf(GetOverviewItemForWindow(window3.get())
+                        ->item_widget()
+                        ->GetNativeWindow(),
+                    parent));
 }
 
 // Verify that if a window is dragged from overview and snapped in place of
 // another split view window, then the old split view window is reinserted into
-// the overview grid at the correct position according to MRU order.
+// the overview grid at the correct position according to MRU order, and the
+// stacking order is also correct.
 TEST_P(
     SplitViewOverviewSessionTest,
     SplitViewWindowReinsertedToOverviewAtCorrectPositionWhenAnotherWindowTakesItsPlace) {
@@ -5089,6 +5114,7 @@ TEST_P(
   std::unique_ptr<aura::Window> window1(CreateWindow(bounds));
   std::unique_ptr<aura::Window> window2(CreateWindow(bounds));
   std::unique_ptr<aura::Window> window3(CreateWindow(bounds));
+  std::unique_ptr<aura::Window> window4(CreateWindow(bounds));
   ToggleOverview();
   DragWindowTo(GetOverviewItemForWindow(window1.get()), gfx::PointF());
   DragWindowTo(GetOverviewItemForWindow(window2.get()),
@@ -5099,16 +5125,38 @@ TEST_P(
   DragWindowTo(GetOverviewItemForWindow(window3.get()), gfx::PointF());
   EXPECT_EQ(window3.get(), split_view_controller()->left_window());
 
+  // Verify the grid arrangement.
   ASSERT_TRUE(InOverviewSession());
   const std::vector<aura::Window*> expected_mru_list = {
-      window3.get(), window2.get(), window1.get()};
-  const std::vector<aura::Window*> expected_overview_list = {window2.get(),
-                                                             window1.get()};
+      window3.get(), window2.get(), window1.get(), window4.get()};
+  const std::vector<aura::Window*> expected_overview_list = {
+      window2.get(), window1.get(), window4.get()};
   EXPECT_EQ(
       expected_mru_list,
       Shell::Get()->mru_window_tracker()->BuildMruWindowList(kActiveDesk));
   EXPECT_EQ(expected_overview_list,
             overview_controller()->GetWindowsListInOverviewGridsForTest());
+
+  // Verify the stacking order.
+  aura::Window* parent = window1->parent();
+  ASSERT_EQ(parent, window2->parent());
+  ASSERT_EQ(parent, window4->parent());
+  EXPECT_GT(IndexOf(GetOverviewItemForWindow(window2.get())
+                        ->item_widget()
+                        ->GetNativeWindow(),
+                    parent),
+            IndexOf(GetOverviewItemForWindow(window1.get())
+                        ->item_widget()
+                        ->GetNativeWindow(),
+                    parent));
+  EXPECT_GT(IndexOf(GetOverviewItemForWindow(window1.get())
+                        ->item_widget()
+                        ->GetNativeWindow(),
+                    parent),
+            IndexOf(GetOverviewItemForWindow(window4.get())
+                        ->item_widget()
+                        ->GetNativeWindow(),
+                    parent));
 }
 
 // Verify that if the split view divider is dragged close to the edge, the grid

ash/wm/overview/overview_window_drag_controller.cc

@@ -605,10 +605,10 @@ OverviewWindowDragController::CompleteNormalDrag(
 
     OverviewGrid* target_grid =
         overview_session_->GetGridWithRootWindow(target_root);
-    // Add |window| to |target_grid| with reposition=false because soon we will
-    // call |OverviewSession::PositionWindows| anyway.
+    // Add |window| to |target_grid| with reposition=false and restack=false,
+    // because soon we will handle both repositioning and restacking anyway.
     target_grid->AddItemInMruOrder(window, /*reposition=*/false,
-                                   /*animate=*/false);
+                                   /*animate=*/false, /*restack=*/false);
     item_ = target_grid->GetOverviewItemContaining(window);
     // Put the new item where the old item ended, so it looks like it is the
     // same item. The following call to |OverviewSession::PositionWindows| will

ash/wm/splitview/split_view_controller.cc

@@ -1827,7 +1827,7 @@ void SplitViewController::InsertWindowToOverview(aura::Window* window,
                                                  bool animate) {
   if (!window || !GetOverviewSession())
     return;
-  GetOverviewSession()->AddItemInMruOrder(window, animate);
+  GetOverviewSession()->AddItemInMruOrder(window, animate, /*restack=*/true);
 }
 
 void SplitViewController::FinishWindowResizing(aura::Window* window) {