home_screen: Fix position/stacking on window drag from shelf to overview

When you drag a window from the shelf and drop into overview, it shall
be positioned correctly and stacked correctly.

Test: ash_unittests DragWindowFromShelfControllerTest.DropsIntoOverviewAtCorrectPosition
Change-Id: I198084bfe644c70de00d6824623106dff1776f63
Fixed: 1047821
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2032797Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Avery Musbach <amusbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738255}

ash/home_screen/drag_window_from_shelf_controller_unittest.cc

@@ -17,10 +17,12 @@
 #include "ash/test/ash_test_base.h"
 #include "ash/wallpaper/wallpaper_view.h"
 #include "ash/wallpaper/wallpaper_widget_controller.h"
+#include "ash/wm/mru_window_tracker.h"
 #include "ash/wm/overview/overview_constants.h"
 #include "ash/wm/overview/overview_controller.h"
 #include "ash/wm/overview/overview_grid.h"
 #include "ash/wm/overview/overview_item.h"
+#include "ash/wm/overview/overview_test_util.h"
 #include "ash/wm/splitview/split_view_constants.h"
 #include "ash/wm/splitview/split_view_controller.h"
 #include "ash/wm/tablet_mode/tablet_mode_controller_test_api.h"
@@ -29,10 +31,25 @@
 #include "base/test/metrics/histogram_tester.h"
 #include "ui/compositor/scoped_animation_duration_scale_mode.h"
 #include "ui/gfx/geometry/point_f.h"
+#include "ui/views/widget/widget.h"
 #include "ui/wm/core/window_util.h"
 
 namespace ash {
 
+namespace {
+
+// Helper function to get the index of |child|, given its parent window
+// |parent|.
+int IndexOf(aura::Window* child, aura::Window* parent) {
+  aura::Window::Windows children = parent->children();
+  auto it = std::find(children.begin(), children.end(), child);
+  DCHECK(it != children.end());
+
+  return static_cast<int>(std::distance(children.begin(), it));
+}
+
+}  // namespace
+
 class DragWindowFromShelfControllerTest : public AshTestBase {
  public:
   DragWindowFromShelfControllerTest() = default;
@@ -986,4 +1003,66 @@ TEST_F(DragWindowFromShelfControllerTest,
             window_util::GetActiveWindow());
 }
 
+// Test that when the dragged window is dropped into overview, it is positioned
+// and stacked correctly.
+TEST_F(DragWindowFromShelfControllerTest, DropsIntoOverviewAtCorrectPosition) {
+  std::unique_ptr<aura::Window> window1 = CreateTestWindow();
+  std::unique_ptr<aura::Window> window2 = CreateTestWindow();
+  std::unique_ptr<aura::Window> window3 = CreateTestWindow();
+  ToggleOverview();
+  ui::test::EventGenerator* generator = GetEventGenerator();
+  generator->MoveMouseTo(gfx::ToRoundedPoint(
+      GetOverviewItemForWindow(window1.get())->target_bounds().CenterPoint()));
+  generator->DragMouseTo(0, 400);
+  generator->MoveMouseTo(gfx::ToRoundedPoint(
+      GetOverviewItemForWindow(window2.get())->target_bounds().CenterPoint()));
+  generator->DragMouseTo(799, 400);
+  EXPECT_EQ(window1.get(), split_view_controller()->left_window());
+  EXPECT_EQ(window2.get(), split_view_controller()->right_window());
+  ToggleOverview();
+  StartDrag(window1.get(),
+            Shelf::ForWindow(Shell::GetPrimaryRootWindow())
+                ->GetIdealBounds()
+                .left_center(),
+            HotseatState::kExtended);
+  Drag(gfx::Point(200, 200), 1.f, 1.f);
+  DragWindowFromShelfControllerTestApi().WaitUntilOverviewIsShown(
+      window_drag_controller());
+  EndDrag(gfx::Point(200, 200), base::nullopt);
+
+  // Verify the grid arrangement.
+  OverviewController* overview_controller = Shell::Get()->overview_controller();
+  ASSERT_TRUE(overview_controller->InOverviewSession());
+  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));
+}
+
 }  // namespace ash

ash/wm/overview/overview_grid.cc

@@ -1864,12 +1864,14 @@ size_t OverviewGrid::GetOverviewItemIndex(OverviewItem* item) const {
 }
 
 size_t OverviewGrid::FindInsertionIndex(const aura::Window* window) {
-  DCHECK(!GetDropTarget());
   size_t index = 0u;
   for (aura::Window* mru_window :
        Shell::Get()->mru_window_tracker()->BuildMruWindowList(kActiveDesk)) {
-    if (index == window_list_.size() || mru_window == window)
+    if (index == size() ||
+        IsDropTargetWindow(window_list_[index]->GetWindow()) ||
+        mru_window == window) {
       return index;
+    }
     // As we iterate over the whole MRU window list, the windows in this grid
     // will be encountered in the same order, but possibly with other windows in
     // between. Ignore those other windows, and only increment |index| when we
@@ -1909,8 +1911,8 @@ void OverviewGrid::AddDraggedWindowIntoOverviewOnDragEnd(
     dragged_window->ClearProperty(kCanAttachToAnotherWindowKey);
   }
 
-  overview_session_->AddItem(dragged_window, /*reposition=*/false,
-                             /*animate=*/false);
+  overview_session_->AddItemInMruOrder(dragged_window, /*reposition=*/false,
+                                       /*animate=*/false, /*restack=*/true);
 }
 
 // Returns the desks widget bounds in root, given the screen bounds of the

ash/wm/overview/overview_item.cc

@@ -670,6 +670,11 @@ void OverviewItem::Restack() {
   // |window| and has the same parent.
   for (const std::unique_ptr<OverviewItem>& overview_item :
        overview_grid_->window_list()) {
+    // |Restack| is sometimes called when there is a drop target, but is never
+    // used to restack an item that comes after a drop target. In other words,
+    // |overview_grid_| might have a drop target, but we will break out of the
+    // for loop before reaching it.
+    DCHECK(!overview_grid_->IsDropTargetWindow(overview_item->GetWindow()));
     if (overview_item.get() == this)
       break;
     if (overview_item->GetWindow()->parent() == parent_window)

ash/wm/overview/overview_session.cc

@@ -433,6 +433,7 @@ void OverviewSession::AppendItem(aura::Window* window,
 }
 
 void OverviewSession::AddItemInMruOrder(aura::Window* window,
+                                        bool reposition,
                                         bool animate,
                                         bool restack) {
   // Early exit if a grid already contains |window|.
@@ -440,7 +441,7 @@ void OverviewSession::AddItemInMruOrder(aura::Window* window,
   if (!grid || grid->GetOverviewItemContaining(window))
     return;
 
-  grid->AddItemInMruOrder(window, /*reposition=*/true, animate, restack);
+  grid->AddItemInMruOrder(window, reposition, animate, restack);
   OnItemAdded(window);
 }
 

ash/wm/overview/overview_session.h

@@ -180,12 +180,15 @@ class ASH_EXPORT OverviewSession : public display::DisplayObserver,
   // TODO(afakhry): Expose |use_spawn_animation| if needed.
   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. 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);
+  // Like |AddItem|, but adds |window| at the correct position according to MRU
+  // order. 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 AddItemInMruOrder(aura::Window* window,
+                         bool reposition,
+                         bool animate,
+                         bool restack);
 
   // Removes |overview_item| from the corresponding grid. No items are
   // repositioned.

ash/wm/splitview/split_view_controller.cc

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

ash/wm/tablet_mode/tablet_mode_window_drag_delegate.cc

@@ -314,8 +314,11 @@ void TabletModeWindowDragDelegate::FlingOrSwipe(ui::GestureEvent* event) {
   if (event->type() == ui::ET_SCROLL_FLING_START) {
     if (ShouldFlingIntoOverview(event)) {
       DCHECK(Shell::Get()->overview_controller()->InOverviewSession());
-      Shell::Get()->overview_controller()->overview_session()->AddItem(
-          dragged_window_, /*reposition=*/true, /*animate=*/false);
+      Shell::Get()
+          ->overview_controller()
+          ->overview_session()
+          ->AddItemInMruOrder(dragged_window_, /*reposition=*/true,
+                              /*animate=*/false, /*restack=*/true);
     }
     StartFling(event);
   }