Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(20)

Unified Diff: ui/app_list/views/apps_grid_view.cc

Issue 298963004: app_list: Fix possible out of bounds index. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: DCHECK -> CHECK Created 6 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « ui/app_list/views/apps_grid_view.h ('k') | ui/app_list/views/test/apps_grid_view_test_api.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..52034d8c024b4de49027bdab98b83935e43982c5 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);
}
+ CHECK_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_);
+ CHECK_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_|.
+ CHECK_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)
+ --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) ||
+ 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();
return (index.page == pagination_model_->total_pages() - 1 &&
index.slot == last_possible_slot + 1);
}
« no previous file with comments | « ui/app_list/views/apps_grid_view.h ('k') | ui/app_list/views/test/apps_grid_view_test_api.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698