Fix excess items being dropped into app list folders.

This CL fixes an issue where more than the maximum number of apps can be
dragged into an app list folder due to the app list checking the model
index of the drag target rather than the actual bounds of the items in
the grid view.

BUG=419642

Review URL: https://codereview.chromium.org/623473004

Cr-Commit-Position: refs/heads/master@{#297969}

4 files changed, 67 insertions, 6 deletions

diff --git a/ui/app_list/views/apps_grid_view.cc b/ui/app_list/views/apps_grid_view.cc
index 22b7fd9..4971720 100644
--- a/ui/app_list/views/apps_grid_view.cc
+++ b/ui/app_list/views/apps_grid_view.cc
@@ -2085,7 +2085,8 @@ bool AppsGridView::EnableFolderDragDropUI() {
 }
 
 bool AppsGridView::CanDropIntoTarget(const Index& drop_target) const {
-  AppListItemView* target_view = GetViewAtIndex(drop_target);
+  AppListItemView* target_view =
+      GetViewDisplayedAtSlotOnCurrentPage(drop_target.slot);
   if (!target_view)
     return false;
 
@@ -2132,7 +2133,8 @@ gfx::Rect AppsGridView::GetExpectedTileBounds(int row, int col) const {
   return tile_bounds;
 }
 
-AppListItemView* AppsGridView::GetViewDisplayedAtSlotOnCurrentPage(int slot) {
+AppListItemView* AppsGridView::GetViewDisplayedAtSlotOnCurrentPage(
+    int slot) const {
   if (slot < 0)
     return NULL;
 
diff --git a/ui/app_list/views/apps_grid_view.h b/ui/app_list/views/apps_grid_view.h
index 1576ab0..878ebd3 100644
--- a/ui/app_list/views/apps_grid_view.h
+++ b/ui/app_list/views/apps_grid_view.h
@@ -422,7 +422,7 @@ class APP_LIST_EXPORT AppsGridView : public views::View,
   // there is no item displayed at |slot|, returns NULL. Note that this finds an
   // item *displayed* at a slot, which may differ from the item's location in
   // the model (as it may have been temporarily moved during a drag operation).
-  AppListItemView* GetViewDisplayedAtSlotOnCurrentPage(int slot);
+  AppListItemView* GetViewDisplayedAtSlotOnCurrentPage(int slot) const;
 
   // Sets state of the view with |target_index| to |is_target_folder| for
   // dropping |drag_view_|.
diff --git a/ui/app_list/views/apps_grid_view_unittest.cc b/ui/app_list/views/apps_grid_view_unittest.cc
index febc17c..b9cc401 100644
--- a/ui/app_list/views/apps_grid_view_unittest.cc
+++ b/ui/app_list/views/apps_grid_view_unittest.cc
@@ -149,9 +149,9 @@ class AppsGridViewTest : public views::ViewsTestBase {
   }
 
   // Points are in |apps_grid_view_|'s coordinates.
-  void SimulateDrag(AppsGridView::Pointer pointer,
-                    const gfx::Point& from,
-                    const gfx::Point& to) {
+  AppListItemView* SimulateDrag(AppsGridView::Pointer pointer,
+                                const gfx::Point& from,
+                                const gfx::Point& to) {
     AppListItemView* view = GetItemViewForPoint(from);
     DCHECK(view);
 
@@ -167,6 +167,7 @@ class AppsGridViewTest : public views::ViewsTestBase {
     ui::MouseEvent drag_event(ui::ET_MOUSE_DRAGGED,
                               translated_to, to, 0, 0);
     apps_grid_view_->UpdateDragFromItem(pointer, drag_event);
+    return view;
   }
 
   void SimulateKeyPress(ui::KeyboardCode key_code) {
@@ -416,6 +417,56 @@ TEST_F(AppsGridViewTest, MouseDragMaxItemsInFolder) {
   test_api_->LayoutToIdealBounds();
 }
 
+// Check that moving items around doesn't allow a drop to happen into a full
+// folder.
+TEST_F(AppsGridViewTest, MouseDragMaxItemsInFolderWithMovement) {
+  EnsureFoldersEnabled();
+
+  // Create and add a folder with 16 items in it.
+  size_t kTotalItems = kMaxFolderItems;
+  model_->CreateAndPopulateFolderWithApps(kTotalItems);
+  EXPECT_EQ(1u, model_->top_level_item_list()->item_count());
+  EXPECT_EQ(AppListFolderItem::kItemType,
+            model_->top_level_item_list()->item_at(0)->GetItemType());
+  AppListFolderItem* folder_item = static_cast<AppListFolderItem*>(
+      model_->top_level_item_list()->item_at(0));
+  EXPECT_EQ(kTotalItems, folder_item->ChildItemCount());
+
+  // Create and add another item.
+  model_->PopulateAppWithId(kTotalItems);
+  EXPECT_EQ(2u, model_->top_level_item_list()->item_count());
+  EXPECT_EQ(folder_item->id(), model_->top_level_item_list()->item_at(0)->id());
+  EXPECT_EQ(model_->GetItemName(kMaxFolderItems),
+            model_->top_level_item_list()->item_at(1)->id());
+
+  AppListItemView* folder_view =
+      GetItemViewForPoint(GetItemTileRectAt(0, 0).CenterPoint());
+
+  // Drag the new item to the left so that the grid reorders.
+  gfx::Point from = GetItemTileRectAt(0, 1).CenterPoint();
+  gfx::Point to = GetItemTileRectAt(0, 0).bottom_left();
+  AppListItemView* dragged_view = SimulateDrag(AppsGridView::MOUSE, from, to);
+  test_api_->LayoutToIdealBounds();
+
+  // The grid now looks like | blank | folder |.
+  EXPECT_EQ(NULL, GetItemViewForPoint(GetItemTileRectAt(0, 0).CenterPoint()));
+  EXPECT_EQ(folder_view,
+            GetItemViewForPoint(GetItemTileRectAt(0, 1).CenterPoint()));
+
+  // Move onto the folder and end the drag.
+  to = GetItemTileRectAt(0, 1).CenterPoint();
+  gfx::Point translated_to =
+      gfx::PointAtOffsetFromOrigin(to - dragged_view->bounds().origin());
+  ui::MouseEvent drag_event(ui::ET_MOUSE_DRAGGED, translated_to, to, 0, 0);
+  apps_grid_view_->UpdateDragFromItem(AppsGridView::MOUSE, drag_event);
+  apps_grid_view_->EndDrag(false);
+
+  // The item should not have moved into the folder.
+  EXPECT_EQ(2u, model_->top_level_item_list()->item_count());
+  EXPECT_EQ(kMaxFolderItems, folder_item->ChildItemCount());
+  test_api_->LayoutToIdealBounds();
+}
+
 TEST_F(AppsGridViewTest, MouseDragItemReorder) {
   // This test assumes Folders are enabled.
   EnsureFoldersEnabled();
diff --git a/ui/app_list/views/test/apps_grid_view_test_api.cc b/ui/app_list/views/test/apps_grid_view_test_api.cc
index 2251036..1142f11 100644
--- a/ui/app_list/views/test/apps_grid_view_test_api.cc
+++ b/ui/app_list/views/test/apps_grid_view_test_api.cc
@@ -23,6 +23,14 @@ views::View* AppsGridViewTestApi::GetViewAtModelIndex(int index) const {
 }
 
 void AppsGridViewTestApi::LayoutToIdealBounds() {
+  if (view_->reorder_timer_.IsRunning()) {
+    view_->reorder_timer_.Stop();
+    view_->OnReorderTimer();
+  }
+  if (view_->folder_dropping_timer_.IsRunning()) {
+    view_->folder_dropping_timer_.Stop();
+    view_->OnFolderDroppingTimer();
+  }
   view_->bounds_animator_.Cancel();
   view_->Layout();
 }