Overview: fix the animation regression.

If one side of the screen is occupied by a split window, and the other
side of the screen is occupied by overview, select an overview window
should only animate the selected overview window to snap, and the other
windows should not animate.

Bug: 866178
Change-Id: I1cb243ad61791a240227c6395770643603726a33
Reviewed-on: https://chromium-review.googlesource.com/1145836
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577230}

ash/wm/overview/window_grid.cc

@@ -462,7 +462,7 @@ WindowSelectorItem* WindowGrid::GetWindowSelectorItemContaining(
   return nullptr;
 }
 
-void WindowGrid::AddItem(aura::Window* window) {
+void WindowGrid::AddItem(aura::Window* window, bool reposition) {
   DCHECK(!GetWindowSelectorItemContaining(window));
 
   window_observer_.Add(window);
@@ -478,10 +478,12 @@ void WindowGrid::AddItem(aura::Window* window) {
     window_list_.back()->set_should_animate_when_exiting(false);
   }
 
-  PositionWindows(/*animate=*/true);
+  if (reposition)
+    PositionWindows(/*animate=*/true);
 }
 
-void WindowGrid::RemoveItem(WindowSelectorItem* selector_item) {
+void WindowGrid::RemoveItem(WindowSelectorItem* selector_item,
+                            bool reposition) {
   auto iter =
       GetWindowSelectorItemIterContainingWindow(selector_item->GetWindow());
   if (iter != window_list_.end()) {
@@ -491,7 +493,8 @@ void WindowGrid::RemoveItem(WindowSelectorItem* selector_item) {
     window_list_.erase(iter);
   }
 
-  PositionWindows(/*animate=*/true);
+  if (reposition)
+    PositionWindows(/*animate=*/true);
 }
 
 void WindowGrid::FilterItems(const base::string16& pattern) {
@@ -571,7 +574,8 @@ void WindowGrid::OnWindowDragStarted(aura::Window* dragged_window) {
   DCHECK_EQ(dragged_window->GetRootWindow(), root_window_);
   DCHECK(!new_selector_item_widget_);
   new_selector_item_widget_ = CreateNewSelectorItemWidget(dragged_window);
-  window_selector_->AddItem(new_selector_item_widget_->GetNativeWindow());
+  window_selector_->AddItem(new_selector_item_widget_->GetNativeWindow(),
+                            /*reposition=*/true);
 
   // Stack the newly added window item below |dragged_window|.
   DCHECK_EQ(dragged_window->parent(),
@@ -649,20 +653,28 @@ void WindowGrid::OnWindowDragEnded(aura::Window* dragged_window,
   DCHECK_EQ(dragged_window->GetRootWindow(), root_window_);
   DCHECK(new_selector_item_widget_.get());
 
-  // Check to see if the dragged window needs to be added to overview.
+  // Check to see if the dragged window needs to be added to overview. If so,
+  // add it to overview without repositioning the grid. It will be done at the
+  // end of this function.
   if (SelectedWindow()) {
     if (IsNewSelectorItemWindow(SelectedWindow()->GetWindow()))
-      window_selector_->AddItem(dragged_window);
+      window_selector_->AddItem(dragged_window, /*reposition=*/false);
     SelectedWindow()->set_selected(false);
     selection_widget_.reset();
   }
 
-  window_selector_->RemoveWindowSelectorItem(GetWindowSelectorItemContaining(
-      new_selector_item_widget_->GetNativeWindow()));
+  window_selector_->RemoveWindowSelectorItem(
+      GetWindowSelectorItemContaining(
+          new_selector_item_widget_->GetNativeWindow()),
+      /*reposition=*/false);
   new_selector_item_widget_.reset();
 
   // Called to reset caption and title visibility after dragging.
   OnSelectorItemDragEnded();
+
+  // Need to call PositionWindows() here as the above two functions AddItem()
+  // and RemoveWindowSelectorItem() are called without repositioning windows.
+  PositionWindows(/*animate=*/true);
 }
 
 bool WindowGrid::IsNewSelectorItemWindow(aura::Window* window) const {

ash/wm/overview/window_grid.h

@@ -88,11 +88,13 @@ class ASH_EXPORT WindowGrid : public aura::WindowObserver,
       const aura::Window* window) const;
 
   // Adds |window| to the grid. Intended to be used by split view. |window|
-  // cannot already be on the grid.
-  void AddItem(aura::Window* window);
+  // cannot already be on the grid. If |reposition| is true, reposition all
+  // window items in the grid after adding the item.
+  void AddItem(aura::Window* window, bool reposition);
 
-  // Removes |selector_item| from the grid.
-  void RemoveItem(WindowSelectorItem* selector_item);
+  // Removes |selector_item| from the grid. If |reprosition| is ture, reposition
+  // all window items in the grid after removing the item.
+  void RemoveItem(WindowSelectorItem* selector_item, bool reposition);
 
   // Dims the items whose titles do not contain |pattern| and prevents their
   // selection. The pattern has its accents removed and is converted to

ash/wm/overview/window_selector.cc

@@ -296,7 +296,7 @@ void WindowSelector::Init(const WindowList& windows,
       window_grid->PositionWindows(/*animate=*/true);
       // Reset |should_animate_when_entering_| in order to animate during
       // overview mode, such as dragging animations.
-        window_grid->ResetWindowListAnimationStates();
+      window_grid->ResetWindowListAnimationStates();
     }
 
     // Image used for text filter textfield.
@@ -340,25 +340,25 @@ void WindowSelector::Shutdown() {
 
   size_t remaining_items = 0;
   for (std::unique_ptr<WindowGrid>& window_grid : grid_list_) {
-      // During shutdown, do not animate all windows in overview if we need to
-      // animate the snapped window.
-      if (split_view_controller->IsSplitViewModeActive() &&
-          split_view_controller->GetDefaultSnappedWindow()->GetRootWindow() ==
-              window_grid->root_window() &&
-          split_view_controller->snapped_window_animation_observer()) {
-        // OverviewWindowAnimationObserver is used to obseve the snapped window
-        // animation. And the windows in |window_grid| will restore their
-        // transform when the snapped window completes its animation.
-        window_grid->set_window_animation_observer(
-            split_view_controller->snapped_window_animation_observer());
-        window_grid->SetWindowListNotAnimatedWhenExiting();
-      } else {
-        window_grid->SetWindowListAnimationStates(
-            selected_item_ && selected_item_->window_grid() == window_grid.get()
-                ? selected_item_
-                : nullptr,
-            OverviewTransition::kExit);
-      }
+    // During shutdown, do not animate all windows in overview if we need to
+    // animate the snapped window.
+    if (split_view_controller->IsSplitViewModeActive() &&
+        split_view_controller->GetDefaultSnappedWindow()->GetRootWindow() ==
+            window_grid->root_window() &&
+        split_view_controller->snapped_window_animation_observer()) {
+      // OverviewWindowAnimationObserver is used to obseve the snapped window
+      // animation. And the windows in |window_grid| will restore their
+      // transform when the snapped window completes its animation.
+      window_grid->set_window_animation_observer(
+          split_view_controller->snapped_window_animation_observer());
+      window_grid->SetWindowListNotAnimatedWhenExiting();
+    } else {
+      window_grid->SetWindowListAnimationStates(
+          selected_item_ && selected_item_->window_grid() == window_grid.get()
+              ? selected_item_
+              : nullptr,
+          OverviewTransition::kExit);
+    }
     for (const auto& window_selector_item : window_grid->window_list())
       window_selector_item->RestoreWindow(/*reset_transform=*/true);
     remaining_items += window_grid->size();
@@ -521,13 +521,13 @@ WindowGrid* WindowSelector::GetGridWithRootWindow(aura::Window* root_window) {
   return nullptr;
 }
 
-void WindowSelector::AddItem(aura::Window* window) {
+void WindowSelector::AddItem(aura::Window* window, bool reposition) {
   // Early exit if a grid already contains |window|.
   WindowGrid* grid = GetGridWithRootWindow(window->GetRootWindow());
   if (!grid || grid->GetWindowSelectorItemContaining(window))
     return;
 
-  grid->AddItem(window);
+  grid->AddItem(window, reposition);
   ++num_items_;
 
   // Transfer focus from |window| to the text widget, to match the behavior of
@@ -535,7 +535,8 @@ void WindowSelector::AddItem(aura::Window* window) {
   wm::ActivateWindow(GetTextFilterWidgetWindow());
 }
 
-void WindowSelector::RemoveWindowSelectorItem(WindowSelectorItem* item) {
+void WindowSelector::RemoveWindowSelectorItem(WindowSelectorItem* item,
+                                              bool reposition) {
   if (item->GetWindow()->HasObserver(this)) {
     item->GetWindow()->RemoveObserver(this);
     observed_windows_.erase(item->GetWindow());
@@ -546,7 +547,7 @@ void WindowSelector::RemoveWindowSelectorItem(WindowSelectorItem* item) {
   // Remove |item| from the corresponding grid.
   for (std::unique_ptr<WindowGrid>& grid : grid_list_) {
     if (grid->GetWindowSelectorItemContaining(item->GetWindow())) {
-      grid->RemoveItem(item);
+      grid->RemoveItem(item, reposition);
       --num_items_;
       break;
     }

ash/wm/overview/window_selector.h

@@ -108,17 +108,24 @@ class ASH_EXPORT WindowSelector : public display::DisplayObserver,
   WindowGrid* GetGridWithRootWindow(aura::Window* root_window);
 
   // Add |window| to the grid in |grid_list_| with the same root window. Does
-  // nothing if the grid already contains |window|. This may be called in two
-  // scenarioes: 1) when a item in split view mode was previously snapped but
-  // should now be returned to the window grid (e.g. split view divider dragged
-  // to either edge, or a window is snapped to a postion that already has a
-  // snapped window); 2) when a window (not from overview) is dragged while
-  // overview is open and the window is dropped on the new selector item, the
-  // dragged window is then added to the overview.
-  void AddItem(aura::Window* window);
-
-  // Removes the window selector item from the overview window grid.
-  void RemoveWindowSelectorItem(WindowSelectorItem* item);
+  // nothing if the grid already contains |window|. And if |reposition| is true,
+  // re-position all windows in the target window grid. This may be called in
+  // two scenarioes: 1) when a item in split view mode was previously snapped
+  // but should now be returned to the window grid (e.g. split view divider
+  // dragged to either edge, or a window is snapped to a postion that already
+  // has a snapped window); 2) when a window (not from overview) is dragged
+  // while overview is open and the window is dropped on the new selector item,
+  // the dragged window is then added to the overview.
+  void AddItem(aura::Window* window, bool reposition);
+
+  // Removes the window selector item from the overview window grid. And if
+  // |reposition| is true, re-position all windows in the target window grid.
+  // This may be called in two scenarioes: 1) when a user drags an overview item
+  // to snap to one side of the screen, the item should be removed from the
+  // overview grid; 2) when a window (not from overview) ends its dragging while
+  // overview is open, the new selector item should be removed. Note in both
+  // cases, the windows in the window grid do not need to be repositioned.
+  void RemoveWindowSelectorItem(WindowSelectorItem* item, bool reposition);
 
   void InitiateDrag(WindowSelectorItem* item,
                     const gfx::Point& location_in_screen);

ash/wm/splitview/split_view_controller.cc

@@ -1391,9 +1391,11 @@ void SplitViewController::RemoveWindowFromOverviewIfApplicable(
   overview_window_item_bounds_map_[window] = item->target_bounds();
 
   // Remove it from the grid. The transform will be reset later after the
-  // window is snapped.
+  // window is snapped. Note the remaining windows in overview don't need to be
+  // repositioned in this case as they have been positioned to the right place
+  // during dragging.
   item->RestoreWindow(/*reset_transform=*/false);
-  window_selector->RemoveWindowSelectorItem(item);
+  window_selector->RemoveWindowSelectorItem(item, /*reposition=*/false);
 }
 
 aura::Window* SplitViewController::GetSnappedWindowAt(
@@ -1412,7 +1414,7 @@ void SplitViewController::InsertWindowToOverview(aura::Window* window) {
   if (!window || !Shell::Get()->window_selector_controller()->IsSelecting())
     return;
   Shell::Get()->window_selector_controller()->window_selector()->AddItem(
-      window);
+      window, /*reposition=*/true);
 }
 
 void SplitViewController::StartOverview() {

ash/wm/splitview/split_view_controller_unittest.cc

@@ -2040,7 +2040,8 @@ TEST_F(SplitViewTabDraggingTest, DragSnappedWindowWhileOverviewOpen) {
       window_selector->GetGridWithRootWindow(window2->GetRootWindow());
   ASSERT_TRUE(current_grid);
   window_selector->RemoveWindowSelectorItem(
-      current_grid->GetWindowSelectorItemContaining(window2.get()));
+      current_grid->GetWindowSelectorItemContaining(window2.get()),
+      /*reposition=*/false);
   resizer = StartDrag(window2.get(), window1.get());
   ASSERT_TRUE(resizer.get());
   EXPECT_EQ(GetIndicatorState(resizer.get()), IndicatorState::kNone);
@@ -2076,7 +2077,8 @@ TEST_F(SplitViewTabDraggingTest, DragSnappedWindowWhileOverviewOpen) {
   // causing overview mode to end.
   // Remove |window1| from overview first before tab dragging.
   window_selector->RemoveWindowSelectorItem(
-      current_grid->GetWindowSelectorItemContaining(window1.get()));
+      current_grid->GetWindowSelectorItemContaining(window1.get()),
+      /*reposition=*/false);
   resizer = StartDrag(window1.get(), window2.get());
   ASSERT_TRUE(resizer.get());
   DragWindowTo(resizer.get(), gfx::Point(600, 500));