app_list: Fix possible out of bounds index.

ui/app_list/views/apps_grid_view.cc

Index: ui/app_list/views/apps_grid_view.cc
diff --git a/ui/app_list/views/apps_grid_view.cc b/ui/app_list/views/apps_grid_view.cc
index e8c60f58eda899c85948ea8536ee797800f21884..a9eb79b119b262fd6f17ebd452ce7f412c783a54 100644
--- a/ui/app_list/views/apps_grid_view.cc
+++ b/ui/app_list/views/apps_grid_view.cc
@@ -348,6 +348,9 @@ AppsGridView::AppsGridView(AppsGridViewDelegate* delegate,
       selected_view_(NULL),
       drag_view_(NULL),
       drag_start_page_(-1),
+#if defined(OS_WIN)
+      use_synchronous_drag_(true),
+#endif
       drag_pointer_(NONE),
       drop_attempt_(DROP_FOR_NONE),
       drag_and_drop_host_(NULL),
@@ -402,6 +405,8 @@ void AppsGridView::ResetForShowApps() {
   for (int i = 0; i < view_model_.view_size(); ++i) {
     view_model_.view_at(i)->SetVisible(true);
   }
+  DCHECK_EQ(item_list_->item_count(),
+            static_cast<size_t>(view_model_.view_size()));
 }
 
 void AppsGridView::SetModel(AppListModel* model) {
@@ -477,7 +482,7 @@ void AppsGridView::InitiateDrag(AppListItemView* view,
 
 void AppsGridView::StartSettingUpSynchronousDrag() {
 #if defined(OS_WIN)
-  if (!delegate_)
+  if (!delegate_ || !use_synchronous_drag_)
     return;
 
   // Folders can't be integrated with the OS.
@@ -812,7 +817,9 @@ void AppsGridView::ClearDragState() {
   if (drag_view_) {
     drag_view_->OnDragEnded();
     if (IsDraggingForReparentInRootLevelGridView()) {
-      DeleteItemViewAtIndex(view_model_.GetIndexOfView(drag_view_));
+      const int drag_view_index = view_model_.GetIndexOfView(drag_view_);
+      DCHECK_EQ(view_model_.view_size() - 1, drag_view_index);
+      DeleteItemViewAtIndex(drag_view_index);
     }
   }
   drag_view_ = NULL;
@@ -939,6 +946,9 @@ bool AppsGridView::OnKeyReleased(const ui::KeyEvent& event) {
 void AppsGridView::ViewHierarchyChanged(
     const ViewHierarchyChangedDetails& details) {
   if (!details.is_add && details.parent == this) {
+    // The view being delete should not have reference in |view_model_|.
+    DCHECK_EQ(-1, view_model_.GetIndexOfView(details.child));
+
     if (selected_view_ == details.child)
       selected_view_ = NULL;
     if (activated_folder_item_view_ == details.child)
@@ -1693,16 +1703,23 @@ void AppsGridView::ReparentItemForReorder(views::View* item_view,
   AppListFolderItem* source_folder =
       static_cast<AppListFolderItem*>(item_list_->FindItem(source_folder_id));
 
+  int target_model_index = GetModelIndexFromIndex(target);
+
   // Remove the source folder view if there is only 1 item in it, since the
   // source folder will be deleted after its only child item removed from it.
-  if (source_folder->ChildItemCount() == 1u)
-    DeleteItemViewAtIndex(
-        view_model_.GetIndexOfView(activated_folder_item_view()));
+  if (source_folder->ChildItemCount() == 1u) {
+    const int deleted_folder_index =
+        view_model_.GetIndexOfView(activated_folder_item_view());
+    DeleteItemViewAtIndex(deleted_folder_index);
+
+    // Adjust |target_model_index| if it is beyond the deleted folder index.
+    if (target_model_index >= deleted_folder_index)

[xiyuan, 2014/05/27 19:57:26]

This fixes the bad index for view_model_.Move below. It is needed because
model_->MoveItemToFolderAt removes |source_folder| from model and
|target_model_index| needs to be adjusted if |source_folder| is before
|target_model_index|.

+      --target_model_index;
+  }
 
   // Move the item from its parent folder to top level item list.
   // Must move to target_model_index, the location we expect the target item
   // to be, not the item location we want to insert before.
-  int target_model_index = GetModelIndexFromIndex(target);
   int current_model_index = view_model_.GetIndexOfView(item_view);
   syncer::StringOrdinal target_position;
   if (target_model_index < static_cast<int>(item_list_->item_count()))
@@ -1804,7 +1821,11 @@ void AppsGridView::RemoveLastItemFromReparentItemFolderIfNecessary(
 
   // Create a new item view for the last item in folder.
   size_t last_item_index;
-  item_list_->FindItemIndex(last_item->id(), &last_item_index);
+  if (!item_list_->FindItemIndex(last_item->id(), &last_item_index) ||

[xiyuan, 2014/05/27 19:57:26]

Speculative fix for calling view_model_.Add with a bad index.

+      last_item_index > static_cast<size_t>(view_model_.view_size())) {
+    NOTREACHED();
+    return;
+  }
   views::View* last_item_view = CreateViewForItemAtIndex(last_item_index);
   view_model_.Add(last_item_view, last_item_index);
   AddChildView(last_item_view);
@@ -2016,7 +2037,7 @@ AppsGridView::Index AppsGridView::GetNearestTileForDragView() {
 
   // If user drags an item across pages to the last page, and targets it
   // to the last empty slot on it, push the last item for re-ordering.
-  if (IsFirstEmptySlot(nearest_tile) && d_min < d_reorder) {
+  if (IsLastPossibleDropTarget(nearest_tile) && d_min < d_reorder) {
     drop_attempt_ = DROP_FOR_REORDER;
     nearest_tile.slot = nearest_tile.slot - 1;
     return nearest_tile;
@@ -2119,14 +2140,8 @@ gfx::Rect AppsGridView::GetTileBounds(int row, int col) const {
   return tile_rect;
 }
 
-bool AppsGridView::IsFirstEmptySlot(const Index& index) const {
-  // Don't count the hidden drag_view_ created from the item_dragged out of a
-  // folder during re-parenting into the total number of views that are visible
-  // on all grid view pages.
-  int total_views = IsDraggingForReparentInRootLevelGridView()
-                        ? view_model_.view_size() - 1
-                        : view_model_.view_size();
-  int last_possible_slot = (total_views - 1) % tiles_per_page();
+bool AppsGridView::IsLastPossibleDropTarget(const Index& index) const {
+  int last_possible_slot = view_model_.view_size() % tiles_per_page();

[xiyuan, 2014/05/27 19:57:26]

No need to special case for dragging out of a folder. This allows dropping at
the last slot.

   return (index.page == pagination_model_->total_pages() - 1 &&
           index.slot == last_possible_slot + 1);
 }