Protect AppListItemList Add/Remove and fix sync bugs

ui/app_list/cocoa/apps_grid_controller_unittest.mm

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/mac/foundation_util.h"
 #include "base/mac/scoped_nsobject.h"
 #include "base/strings/utf_string_conversions.h"
 #include "skia/ext/skia_utils_mac.h"
 #import "testing/gtest_mac.h"
 #include "ui/app_list/app_list_constants.h"
@@ skipping 468 matching lines @@
   model()->CreateAndAddItem("Item 2");
   ASSERT_EQ(3u, item_list->item_count());
   EXPECT_EQ(3u, [apps_grid_controller_ itemCount]);
   EXPECT_EQ(std::string("|Item 0,Item 1,Item 2|"), GetViewContent());
 
   // Test adding an item whose position is in the middle.
   app_list::AppListItem* item0 = item_list->item_at(0);
   app_list::AppListItem* item1 = item_list->item_at(1);
   app_list::AppListItem* item3 =
       model()->CreateItem("Item Three", "Item Three");
-  item_list->AddItem(item3);
+  model()->AddItem(item3);
   item_list->SetItemPosition(
       item3, item0->position().CreateBetween(item1->position()));
   EXPECT_EQ(4u, [apps_grid_controller_ itemCount]);
   EXPECT_EQ(std::string("|Item 0,Item Three,Item 1,Item 2|"), GetViewContent());
 }
 
 TEST_F(AppsGridControllerTest, ModelMove) {
   model()->PopulateApps(3);
   EXPECT_EQ(3u, [[GetPageAt(0) content] count]);
   EXPECT_EQ(std::string("|Item 0,Item 1,Item 2|"), GetViewContent());
 
   // Test swapping items (e.g. rearranging via sync).
   model()->item_list()->MoveItem(1, 2);
   EXPECT_EQ(std::string("|Item 0,Item 2,Item 1|"), GetViewContent());
 }
 
 TEST_F(AppsGridControllerTest, ModelRemove) {
   model()->PopulateApps(3);
   EXPECT_EQ(3u, [[GetPageAt(0) content] count]);
   EXPECT_EQ(std::string("|Item 0,Item 1,Item 2|"), GetViewContent());
 
   // Test removing an item at the end.
-  model()->item_list()->DeleteItem("Item 2");
+  model()->DeleteItem("Item 2");
   EXPECT_EQ(2u, [apps_grid_controller_ itemCount]);
   EXPECT_EQ(std::string("|Item 0,Item 1|"), GetViewContent());
 
   // Test removing in the middle.
   model()->CreateAndAddItem("Item 2");
   EXPECT_EQ(3u, [apps_grid_controller_ itemCount]);
   EXPECT_EQ(std::string("|Item 0,Item 1,Item 2|"), GetViewContent());
-  model()->item_list()->DeleteItem("Item 1");
+  model()->DeleteItem("Item 1");
   EXPECT_EQ(2u, [apps_grid_controller_ itemCount]);
   EXPECT_EQ(std::string("|Item 0,Item 2|"), GetViewContent());
 }
 
-TEST_F(AppsGridControllerTest, ModelRemoveAlll) {

[tapted, 2014/01/28 23:33:54]

Just a note, this test case initially existed to ensure there was coverage for
ListModelObserver::ListItemsRemoved(size_t start, size_t count) when count != 1
(it should have a comment to that effect - sorry). That's not relevant any more,
since AppListItemListObserver can only remove one item at a time (and would have
become irrelevant when the line below changed to DeleteItemsByType in
http://crrev.com/231923 ).

I guess it's still good to have a test that gives some coverage when the "last"
item is removed from the model.

+TEST_F(AppsGridControllerTest, ModelRemoveSeveral) {
   model()->PopulateApps(3);
   EXPECT_EQ(3u, [[GetPageAt(0) content] count]);
   EXPECT_EQ(std::string("|Item 0,Item 1,Item 2|"), GetViewContent());
 
   // Test removing multiple items via the model.
-  model()->item_list()->DeleteItemsByType(NULL /* all items */);
+  model()->DeleteItem("Item 0");
+  model()->DeleteItem("Item 1");
+  model()->DeleteItem("Item 2");
   EXPECT_EQ(0u, [apps_grid_controller_ itemCount]);
   EXPECT_EQ(std::string("||"), GetViewContent());
 }
 
 TEST_F(AppsGridControllerTest, ModelRemovePage) {
   app_list::AppListItemList* item_list = model()->item_list();
 
   model()->PopulateApps(kItemsPerPage + 1);
   ASSERT_EQ(kItemsPerPage + 1, item_list->item_count());
   EXPECT_EQ(kItemsPerPage + 1, [apps_grid_controller_ itemCount]);
   EXPECT_EQ(2u, [apps_grid_controller_ pageCount]);
 
   // Test removing the last item when there is one item on the second page.
   app_list::AppListItem* last_item = item_list->item_at(kItemsPerPage);
-  item_list->DeleteItem(last_item->id());
+  model()->DeleteItem(last_item->id());
   EXPECT_EQ(kItemsPerPage, item_list->item_count());
   EXPECT_EQ(kItemsPerPage, [apps_grid_controller_ itemCount]);
   EXPECT_EQ(1u, [apps_grid_controller_ pageCount]);
 }
 
 // Test install progress bars, and install flow with item highlighting.
 TEST_F(AppsGridControllerTest, ItemInstallProgress) {
   ReplaceTestModel(kItemsPerPage + 1);
   EXPECT_EQ(2u, [apps_grid_controller_ pageCount]);
   EXPECT_EQ(0u, [apps_grid_controller_ visiblePage]);
@@ skipping 398 matching lines @@
   EXPECT_EQ(1u, [apps_grid_controller_ scheduledScrollPage]);
   [drag_manager onMouseUp:at_center];
   EXPECT_EQ(2u, [apps_grid_controller_ visiblePage]);
   EXPECT_EQ(2u, [apps_grid_controller_ scheduledScrollPage]);
 
   [apps_grid_controller_ setPaginationObserver:nil];
 }
 
 TEST_F(AppsGridControllerTest, ContextMenus) {
   AppListItemWithMenu* item_two_model = new AppListItemWithMenu("Item Two");
-  model()->item_list()->AddItem(new AppListItemWithMenu("Item One"));
-  model()->item_list()->AddItem(item_two_model);
+  model()->AddItem(new AppListItemWithMenu("Item One"));
+  model()->AddItem(item_two_model);
   EXPECT_EQ(2u, [apps_grid_controller_ itemCount]);
 
   NSCollectionView* page = [apps_grid_controller_ collectionViewAtPageIndex:0];
   NSEvent* mouse_at_cell_0 = MouseEventInCell(page, 0);
   NSEvent* mouse_at_cell_1 = MouseEventInCell(page, 1);
 
   NSMenu* menu = [page menuForEvent:mouse_at_cell_0];
   EXPECT_EQ(1, [menu numberOfItems]);
   EXPECT_NSEQ(@"Menu For: Item One", [[menu itemAtIndex:0] title]);
 
@@ skipping 10 matching lines @@
   // Test that a button being held down with the left button does not also show
   // a context menu.
   [GetItemViewAt(0) highlight:YES];
   EXPECT_FALSE([page menuForEvent:mouse_at_cell_0]);
   [GetItemViewAt(0) highlight:NO];
   EXPECT_TRUE([page menuForEvent:mouse_at_cell_0]);
 }
 
 }  // namespace test
 }  // namespace app_list