Skip to content

Commit

Permalink
Revert "Change view_model.h to use size_t."
Browse files Browse the repository at this point in the history
This reverts commit 195d2db.

Reason for revert: With this CL in, the browser crashes upon launching on a debug build.

`view_model.h(96) Check failed: index < entries_.size() (0 vs. 0)` 

Original change's description:
> Change view_model.h to use size_t.
>
> Bug: 1292951
> Change-Id: I37ba2386710a6ead51fece31f368f685e1af1393
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3710687
> Reviewed-by: Yury Khmel <khmel@chromium.org>
> Reviewed-by: Andre Le <leandre@chromium.org>
> Commit-Queue: Peter Kasting <pkasting@chromium.org>
> Reviewed-by: Sammie Quon <sammiequon@chromium.org>
> Reviewed-by: Peter Boström <pbos@chromium.org>
> Reviewed-by: Stefan Kuhne <skuhne@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1017319}

Bug: 1292951
Change-Id: I3327d79c1353fd9d00edb7517b21f70e6ba10508
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3722351
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Luciano Pacheco <lucmult@chromium.org>
Auto-Submit: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Reviewed-by: Stefan Kuhne <skuhne@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1017501}
  • Loading branch information
benbeaudry authored and Chromium LUCI CQ committed Jun 24, 2022
1 parent 6b048b8 commit 983e02f
Show file tree
Hide file tree
Showing 42 changed files with 460 additions and 509 deletions.
4 changes: 2 additions & 2 deletions ash/app_list/app_list_controller_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -340,15 +340,15 @@ TEST_F(AppListControllerImplTest, CheckTabOrderAfterDragIconToShelf) {

// Pins |item2| by dragging it to ShelfView.
ShelfView* shelf_view = GetPrimaryShelf()->GetShelfViewForTesting();
ASSERT_EQ(0u, shelf_view->view_model()->view_size());
ASSERT_EQ(0, shelf_view->view_model()->view_size());
GetEventGenerator()->MoveMouseTo(item2->GetBoundsInScreen().CenterPoint());
GetEventGenerator()->PressLeftButton();
item2->FireMouseDragTimerForTest();
GetEventGenerator()->MoveMouseTo(
shelf_view->GetBoundsInScreen().CenterPoint());
ASSERT_TRUE(GetAppsGridView()->FireDragToShelfTimerForTest());
GetEventGenerator()->ReleaseLeftButton();
ASSERT_EQ(1u, shelf_view->view_model()->view_size());
ASSERT_EQ(1, shelf_view->view_model()->view_size());

// Verifies that the dragged item has the correct previous/next focusable
// view after drag.
Expand Down
24 changes: 12 additions & 12 deletions ash/app_list/app_list_presenter_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,7 @@ TEST_P(ProductivityLauncherTest, FolderItemViewNotAnimatingAfterClosingFolder) {
EXPECT_EQ(original_folder_item_bounds, folder_item_view->GetBoundsInScreen());

// No item layers are expected to be created.
for (size_t i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
for (int i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
views::View* item_view = apps_grid_view_->view_model()->view_at(i);
EXPECT_FALSE(item_view->layer()) << "at " << i;
}
Expand Down Expand Up @@ -1003,7 +1003,7 @@ TEST_P(ProductivityLauncherTest,
apps_grid_view_->GetItemViewAt(2)->GetBoundsInScreen());

// Verify that item view layers have been deleted.
for (size_t i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
for (int i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
views::View* item_view = apps_grid_view_->view_model()->view_at(i);
EXPECT_FALSE(item_view->layer()) << "at " << i;
}
Expand Down Expand Up @@ -1122,7 +1122,7 @@ TEST_P(ProductivityLauncherTest,
apps_grid_view_->GetItemViewAt(2)->GetBoundsInScreen());

// Verify that item view layers have been deleted.
for (size_t i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
for (int i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
views::View* item_view = apps_grid_view_->view_model()->view_at(i);
EXPECT_FALSE(item_view->layer()) << "at " << i;
}
Expand Down Expand Up @@ -1241,7 +1241,7 @@ TEST_P(ProductivityLauncherTest,
apps_grid_view_->GetItemViewAt(3)->GetBoundsInScreen());

// Verify that item view layers have been deleted.
for (size_t i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
for (int i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
views::View* item_view = apps_grid_view_->view_model()->view_at(i);
EXPECT_FALSE(item_view->layer()) << "at " << i;
}
Expand Down Expand Up @@ -1324,7 +1324,7 @@ TEST_P(ProductivityLauncherTest, ReorderedFolderItemDeletionDuringFolderClose) {
apps_grid_view_->GetItemViewAt(3)->GetBoundsInScreen());

// Verify that item view layers have been deleted.
for (size_t i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
for (int i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
views::View* item_view = apps_grid_view_->view_model()->view_at(i);
EXPECT_FALSE(item_view->layer()) << "at " << i;
}
Expand Down Expand Up @@ -1422,7 +1422,7 @@ TEST_P(ProductivityLauncherTest,
apps_grid_view_->GetItemViewAt(3)->GetBoundsInScreen());

// Verify that item view layers have been deleted.
for (size_t i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
for (int i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
views::View* item_view = apps_grid_view_->view_model()->view_at(i);
EXPECT_FALSE(item_view->layer()) << "at " << i;
}
Expand Down Expand Up @@ -1519,7 +1519,7 @@ TEST_P(ProductivityLauncherTest,
apps_grid_view_->GetItemViewAt(3)->GetBoundsInScreen());

// Verify that item view layers have been deleted.
for (size_t i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
for (int i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
views::View* item_view = apps_grid_view_->view_model()->view_at(i);
EXPECT_FALSE(item_view->layer()) << "at " << i;
}
Expand Down Expand Up @@ -2969,7 +2969,7 @@ TEST_P(PopulatedAppListTest, RemoveFolderItemAfterFolderCreation) {
// Verify that item layers have been destroyed after the drag operation ended.
apps_grid_test_api_->WaitForItemMoveAnimationDone();

for (size_t i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
for (int i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
views::View* item_view = apps_grid_view_->view_model()->view_at(i);
EXPECT_FALSE(item_view->layer()) << "at " << i;
}
Expand All @@ -2984,7 +2984,7 @@ TEST_P(PopulatedAppListTest, RemoveFolderItemAfterFolderCreation) {
EXPECT_EQ(expected_folder_item_view_bounds,
folder_item_view->GetBoundsInScreen());
EXPECT_TRUE(AppListIsInFolderView());
for (size_t i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
for (int i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
views::View* item_view = apps_grid_view_->view_model()->view_at(i);
EXPECT_FALSE(item_view->layer()) << "at " << i;
}
Expand Down Expand Up @@ -3043,7 +3043,7 @@ TEST_P(PopulatedAppListTest, ReparentLastFolderItemAfterFolderCreation) {
// Verify that item layers have been destroyed after the drag operation ended.
apps_grid_test_api_->WaitForItemMoveAnimationDone();

for (size_t i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
for (int i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
views::View* item_view = apps_grid_view_->view_model()->view_at(i);
EXPECT_FALSE(item_view->layer()) << "at " << i;
}
Expand All @@ -3065,7 +3065,7 @@ TEST_P(PopulatedAppListTest, ReparentLastFolderItemAfterFolderCreation) {
// Verify that item views have no layers after the folder has been opened.
apps_grid_test_api_->WaitForItemMoveAnimationDone();
EXPECT_TRUE(AppListIsInFolderView());
for (size_t i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
for (int i = 0; i < apps_grid_view_->view_model()->view_size(); ++i) {
views::View* item_view = apps_grid_view_->view_model()->view_at(i);
EXPECT_FALSE(item_view->layer()) << "at " << i;
}
Expand Down Expand Up @@ -5965,7 +5965,7 @@ TEST_P(AppListPresenterHomeLauncherTest, LayerOnSecondPage) {
// of the layers, including items on the second page.
generator->MoveMouseTo(start_point.x(), 0);
generator->ReleaseLeftButton();
for (size_t i = 0; i < apps_grid_view->view_model()->view_size(); ++i) {
for (int i = 0; i < apps_grid_view->view_model()->view_size(); ++i) {
views::View* item_view = apps_grid_view->view_model()->view_at(i);
EXPECT_FALSE(item_view->layer()) << "at " << i;
}
Expand Down
13 changes: 6 additions & 7 deletions ash/app_list/app_list_test_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ views::MenuItemView* GetReorderOptionForNonFolderItemMenu(

ash::AppListItemView* FindFolderItemView(ash::AppsGridView* apps_grid_view) {
auto* model = apps_grid_view->view_model();
for (size_t index = 0; index < model->view_size(); ++index) {
for (int index = 0; index < model->view_size(); ++index) {
ash::AppListItemView* current_view = model->view_at(index);
if (current_view->is_folder())
return current_view;
Expand All @@ -108,7 +108,7 @@ ash::AppListItemView* FindFolderItemView(ash::AppsGridView* apps_grid_view) {

ash::AppListItemView* FindNonFolderItemView(ash::AppsGridView* apps_grid_view) {
auto* model = apps_grid_view->view_model();
for (size_t index = 0; index < model->view_size(); ++index) {
for (int index = 0; index < model->view_size(); ++index) {
ash::AppListItemView* current_view = model->view_at(index);
if (!current_view->is_folder())
return current_view;
Expand Down Expand Up @@ -158,7 +158,7 @@ views::MenuItemView* ShowRootMenuAndReturn(
ui::test::EventGenerator* event_generator) {
views::MenuItemView* root_menu = nullptr;

EXPECT_GT(apps_grid_view->view_model()->view_size(), 0u);
EXPECT_GT(apps_grid_view->view_model()->view_size(), 0);

switch (menu_type) {
case AppListTestApi::MenuType::kAppListPageMenu:
Expand Down Expand Up @@ -415,7 +415,7 @@ std::u16string AppListTestApi::GetAppListItemViewName(
const std::string& item_id) {
views::ViewModelT<AppListItemView>* view_model =
GetTopLevelAppsGridView()->view_model();
for (size_t i = 0; i < view_model->view_size(); ++i) {
for (int i = 0; i < view_model->view_size(); ++i) {
AppListItemView* app_list_item_view = view_model->view_at(i);
if (app_list_item_view->item()->id() == item_id)
return app_list_item_view->title()->GetText();
Expand All @@ -426,7 +426,7 @@ std::u16string AppListTestApi::GetAppListItemViewName(
std::vector<std::string> AppListTestApi::GetTopLevelViewIdList() {
std::vector<std::string> id_list;
auto* view_model = GetTopLevelAppsGridView()->view_model();
for (size_t i = 0; i < view_model->view_size(); ++i) {
for (int i = 0; i < view_model->view_size(); ++i) {
AppListItem* app_list_item = view_model->view_at(i)->item();
if (app_list_item) {
id_list.push_back(app_list_item->id());
Expand Down Expand Up @@ -633,8 +633,7 @@ ash::AppListItemView* AppListTestApi::FindTopLevelFolderItemView() {
void AppListTestApi::VerifyTopLevelItemVisibility() {
auto* view_model = GetTopLevelAppsGridView()->view_model();
std::vector<std::string> invisible_item_names;
for (size_t view_index = 0; view_index < view_model->view_size();
++view_index) {
for (int view_index = 0; view_index < view_model->view_size(); ++view_index) {
auto* item_view = view_model->view_at(view_index);
if (!item_view->GetVisible())
invisible_item_names.push_back(item_view->item()->name());
Expand Down
10 changes: 5 additions & 5 deletions ash/app_list/paged_view_structure.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,15 @@ void PagedViewStructure::LoadFromMetadata() {
if (mode_ == Mode::kSinglePage) {
// Copy the view model to a single page.
pages_[0].reserve(view_model->view_size());
for (size_t i = 0; i < view_model->view_size(); ++i) {
for (int i = 0; i < view_model->view_size(); ++i) {
pages_[0].push_back(view_model->view_at(i));
}
return;
}

if (mode_ == Mode::kFullPages) {
// Copy the view model to N full pages.
for (size_t i = 0; i < view_model->view_size(); ++i) {
for (int i = 0; i < view_model->view_size(); ++i) {
if (pages_.back().size() ==
static_cast<size_t>(TilesPerPage(pages_.size() - 1))) {
pages_.emplace_back();
Expand Down Expand Up @@ -264,15 +264,15 @@ int PagedViewStructure::GetModelIndexFromIndex(const GridIndex& index) const {
return view_model->view_size();

AppListItemView* view = pages_[index.page][index.slot];
return view_model->GetIndexOfView(view).value();
return view_model->GetIndexOfView(view);
}

GridIndex PagedViewStructure::GetLastTargetIndex() const {
if (apps_grid_view_->view_model()->view_size() == 0)
return GridIndex(0, 0);

if (mode_ == Mode::kSinglePage || mode_ == Mode::kFullPages) {
size_t view_index = apps_grid_view_->view_model()->view_size();
int view_index = apps_grid_view_->view_model()->view_size();

// If a view in the current view model is being dragged, then ignore it.
if (apps_grid_view_->drag_view())
Expand Down Expand Up @@ -304,7 +304,7 @@ GridIndex PagedViewStructure::GetLastTargetIndexOfPage(int page_index) const {
}

const int page_size = total_pages();
DCHECK_LT(0u, apps_grid_view_->view_model()->view_size());
DCHECK_LT(0, apps_grid_view_->view_model()->view_size());
DCHECK_LE(page_index, page_size);

if (page_index == page_size)
Expand Down
2 changes: 1 addition & 1 deletion ash/app_list/views/app_list_bubble_view_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -989,7 +989,7 @@ TEST_F(AppListBubbleViewTest, DownArrowFromRecentsSelectsLastColumnInAppsGrid) {

// There are only 2 folders, and hence 2 columns, in the top level apps grid.
auto* apps_grid_view = GetAppsGridView();
ASSERT_EQ(2u, apps_grid_view->view_model()->view_size());
ASSERT_EQ(2, apps_grid_view->view_model()->view_size());

// Focus the 5th recent app.
auto* recent_apps_view = GetRecentAppsView();
Expand Down
12 changes: 6 additions & 6 deletions ash/app_list/views/app_list_main_view_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
namespace ash {
namespace {

const size_t kInitialItems = 2;
const int kInitialItems = 2;

} // namespace

Expand Down Expand Up @@ -184,7 +184,7 @@ class AppListMainViewTest : public views::ViewsTestBase,
delegate_->GetTestModel()->FindFolderItem("single_item_folder"));
EXPECT_EQ(AppListFolderItem::kItemType, folder_item->GetItemType());

EXPECT_EQ(1u, GetRootViewModel()->view_size());
EXPECT_EQ(1, GetRootViewModel()->view_size());
AppListItemView* folder_item_view =
static_cast<AppListItemView*>(GetRootViewModel()->view_at(0));
EXPECT_EQ(folder_item_view->item(), folder_item);
Expand Down Expand Up @@ -287,7 +287,7 @@ TEST_P(AppListMainViewTest, ModelChanged) {
std::unique_ptr<SearchModel> old_search_model(
delegate_->ReleaseTestSearchModel());

const size_t kReplacementItems = 5;
const int kReplacementItems = 5;
delegate_->ReplaceTestModel(kReplacementItems);
EXPECT_EQ(kReplacementItems, GetRootViewModel()->view_size());
}
Expand All @@ -300,11 +300,11 @@ TEST_P(AppListMainViewTest, DragReparentItemOntoPageSwitcher) {

// Number of apps to populate. Should provide more than 1 page of apps (5*4 =
// 20).
const size_t kNumApps = 30;
const int kNumApps = 30;
delegate_->GetTestModel()->PopulateApps(kNumApps);
GetRootGridView()->Layout();

EXPECT_EQ(1u, GetFolderViewModel()->view_size());
EXPECT_EQ(1, GetFolderViewModel()->view_size());
EXPECT_EQ(kNumApps + 1, GetRootViewModel()->view_size());

AppListItemView* dragged = StartDragForReparent(0);
Expand Down Expand Up @@ -372,7 +372,7 @@ TEST_P(AppListMainViewTest, ReparentSingleItemOntoSelf) {
GetFolderGridView()->EndDrag(false);

// The app list model should remain unchanged.
EXPECT_EQ(2u, GetRootViewModel()->view_size());
EXPECT_EQ(2, GetRootViewModel()->view_size());
EXPECT_EQ(folder_id, GetRootGridView()->GetItemViewAt(0)->item()->id());
AppListFolderItem* const folder_item =
delegate_->GetTestModel()->FindFolderItem("single_item_folder");
Expand Down
12 changes: 6 additions & 6 deletions ash/app_list/views/app_list_view_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1454,7 +1454,7 @@ TEST_P(AppListViewFocusTest, VerticalFocusTraversalInFullscreenAllAppsState) {
forward_view_list.push_back(suggestions[0]);
const views::ViewModelT<AppListItemView>* view_model =
apps_grid_view()->view_model();
for (size_t i = 0; i < view_model->view_size(); i += apps_grid_view()->cols())
for (int i = 0; i < view_model->view_size(); i += apps_grid_view()->cols())
forward_view_list.push_back(view_model->view_at(i));
forward_view_list.push_back(search_box_view()->search_box());

Expand All @@ -1463,7 +1463,7 @@ TEST_P(AppListViewFocusTest, VerticalFocusTraversalInFullscreenAllAppsState) {

std::vector<views::View*> backward_view_list;
backward_view_list.push_back(search_box_view()->search_box());
for (size_t i = view_model->view_size() - 1; i < view_model->view_size();
for (int i = view_model->view_size() - 1; i >= 0;
i -= apps_grid_view()->cols())
backward_view_list.push_back(view_model->view_at(i));
// Up key will always move focus to the last suggestion chip from first row
Expand Down Expand Up @@ -1531,7 +1531,7 @@ TEST_F(AppListViewFocusTest, VerticalFocusTraversalInFirstPageOfFolder) {
std::vector<views::View*> forward_view_list;
const views::ViewModelT<AppListItemView>* view_model =
app_list_folder_view()->items_grid_view()->view_model();
for (size_t i = 0; i < view_model->view_size();
for (int i = 0; i < view_model->view_size();
i += app_list_folder_view()->items_grid_view()->cols()) {
forward_view_list.push_back(view_model->view_at(i));
}
Expand All @@ -1548,7 +1548,7 @@ TEST_F(AppListViewFocusTest, VerticalFocusTraversalInFirstPageOfFolder) {
backward_view_list.push_back(search_box_view()->search_box());
backward_view_list.push_back(
app_list_folder_view()->folder_header_view()->GetFolderNameViewForTest());
for (size_t i = view_model->view_size() - 1; i < view_model->view_size();
for (int i = view_model->view_size() - 1; i >= 0;
i -= app_list_folder_view()->items_grid_view()->cols()) {
backward_view_list.push_back(view_model->view_at(i));
}
Expand Down Expand Up @@ -1579,7 +1579,7 @@ TEST_F(AppListViewPeekingFocusTest,
std::vector<views::View*> forward_view_list;
const views::ViewModelT<AppListItemView>* view_model =
app_list_folder_view()->items_grid_view()->view_model();
for (size_t i = kMaxItemsPerFolderPage; i < view_model->view_size();
for (int i = kMaxItemsPerFolderPage; i < view_model->view_size();
i += app_list_folder_view()->items_grid_view()->cols()) {
forward_view_list.push_back(view_model->view_at(i));
}
Expand All @@ -1596,7 +1596,7 @@ TEST_F(AppListViewPeekingFocusTest,
backward_view_list.push_back(search_box_view()->search_box());
backward_view_list.push_back(
app_list_folder_view()->folder_header_view()->GetFolderNameViewForTest());
for (size_t i = view_model->view_size() - 1; i < view_model->view_size();
for (int i = view_model->view_size() - 1; i >= 0;
i -= app_list_folder_view()->items_grid_view()->cols()) {
backward_view_list.push_back(view_model->view_at(i));
}
Expand Down
Loading

0 comments on commit 983e02f

Please sign in to comment.