Protect AppListItemList Add/Remove and fix sync bugs

ui/app_list/app_list_model_unittest.cc

Index: ui/app_list/app_list_model_unittest.cc
diff --git a/ui/app_list/app_list_model_unittest.cc b/ui/app_list/app_list_model_unittest.cc
index 339a4ab75bc26bbba9c6bf87db6bcf265f7cc46d..422b9eabd700d32c163977413e1872fa2caf093e 100644
--- a/ui/app_list/app_list_model_unittest.cc
+++ b/ui/app_list/app_list_model_unittest.cc
@@ -7,11 +7,13 @@
 #include <map>
 #include <string>
 
+#include "base/command_line.h"
 #include "base/strings/utf_string_conversions.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/app_list/app_list_folder_item.h"
 #include "ui/app_list/app_list_item.h"
 #include "ui/app_list/app_list_model_observer.h"
+#include "ui/app_list/app_list_switches.h"
 #include "ui/app_list/test/app_list_test_model.h"
 #include "ui/base/models/list_model_observer.h"
 
@@ -19,8 +21,7 @@ namespace app_list {
 
 namespace {
 
-class TestObserver : public AppListModelObserver,
-                     public AppListItemListObserver {
+class TestObserver : public AppListModelObserver {
  public:
   TestObserver()
       : status_changed_count_(0),
@@ -36,30 +37,25 @@ class TestObserver : public AppListModelObserver,
     ++status_changed_count_;
   }
 
-  // AppListItemListObserver
-  virtual void OnListItemAdded(size_t index, AppListItem* item) OVERRIDE {
+  virtual void OnAppListItemAdded(AppListItem* item) OVERRIDE {
     items_added_++;
   }
 
-  virtual void OnListItemRemoved(size_t index, AppListItem* item) OVERRIDE {
+  virtual void OnAppListItemWillBeDeleted(AppListItem* item) OVERRIDE {
     items_removed_++;
   }
 
-  virtual void OnListItemMoved(size_t from_index,
-                               size_t to_index,
-                               AppListItem* item) OVERRIDE {
+  virtual void OnAppListItemUpdated(AppListItem* item) OVERRIDE {
     items_moved_++;
   }
 
   int status_changed_count() const { return status_changed_count_; }
-  int signin_changed_count() const { return signin_changed_count_; }
   size_t items_added() { return items_added_; }
   size_t items_removed() { return items_removed_; }
   size_t items_moved() { return items_moved_; }
 
   void ResetCounts() {
     status_changed_count_ = 0;
-    signin_changed_count_ = 0;
     items_added_ = 0;
     items_removed_ = 0;
     items_moved_ = 0;
@@ -67,7 +63,6 @@ class TestObserver : public AppListModelObserver,
 
  private:
   int status_changed_count_;
-  int signin_changed_count_;
   size_t items_added_;
   size_t items_removed_;
   size_t items_moved_;
@@ -85,11 +80,9 @@ class AppListModelTest : public testing::Test {
   // testing::Test overrides:
   virtual void SetUp() OVERRIDE {
     model_.AddObserver(&observer_);
-    model_.item_list()->AddObserver(&observer_);
   }
   virtual void TearDown() OVERRIDE {
     model_.RemoveObserver(&observer_);
-    model_.item_list()->RemoveObserver(&observer_);
   }
 
  protected:
@@ -98,6 +91,11 @@ class AppListModelTest : public testing::Test {
     return item->observers_.HasObserver(folder);
   }
 
+  void AddFolderItem(AppListFolderItem* folder,
+                     const std::string& name) {
+    folder->item_list()->AddItem(model_.CreateItem(name, name));
+  }
+
   test::AppListTestModel model_;
   TestObserver observer_;
 
@@ -138,11 +136,11 @@ TEST_F(AppListModelTest, ModelFindItem) {
   const size_t num_apps = 2;
   model_.PopulateApps(num_apps);
   std::string item_name0 = model_.GetItemName(0);
-  AppListItem* item0 = model_.item_list()->FindItem(item_name0);
+  AppListItem* item0 = model_.FindItem(item_name0);
   ASSERT_TRUE(item0);
   EXPECT_EQ(item_name0, item0->id());
   std::string item_name1 = model_.GetItemName(1);
-  AppListItem* item1 = model_.item_list()->FindItem(item_name1);
+  AppListItem* item1 = model_.FindItem(item_name1);
   ASSERT_TRUE(item1);
   EXPECT_EQ(item_name1, item1->id());
 }
@@ -160,10 +158,11 @@ TEST_F(AppListModelTest, ModelAddItem) {
   AppListItem* item1 = model_.item_list()->item_at(1);
   ASSERT_TRUE(item1);
   AppListItem* item2 = model_.CreateItem("Added Item 2", "Added Item 2");
-  model_.item_list()->AddItem(item2);
-  model_.item_list()->SetItemPosition(
+  model_.AddItem(item2);
+  model_.SetItemPosition(
       item2, item0->position().CreateBetween(item1->position()));
   EXPECT_EQ(num_apps + 2, model_.item_list()->item_count());
+  EXPECT_EQ(num_apps + 2, observer_.items_added());
   EXPECT_EQ("Added Item 2", model_.item_list()->item_at(1)->id());
 }
 
@@ -175,6 +174,7 @@ TEST_F(AppListModelTest, ModelMoveItem) {
   ASSERT_EQ(num_apps + 1, model_.item_list()->item_count());
   // Move it to the position 1.
   model_.item_list()->MoveItem(num_apps, 1);
+  EXPECT_EQ(1u, observer_.items_moved());
   AppListItem* item = model_.item_list()->item_at(1);
   ASSERT_TRUE(item);
   EXPECT_EQ("Inserted Item", item->id());
@@ -184,15 +184,15 @@ TEST_F(AppListModelTest, ModelRemoveItem) {
   const size_t num_apps = 4;
   model_.PopulateApps(num_apps);
   // Remove an item in the middle.
-  model_.item_list()->DeleteItem(model_.GetItemName(1));
+  model_.DeleteItem(model_.GetItemName(1));
   EXPECT_EQ(num_apps - 1, model_.item_list()->item_count());
   EXPECT_EQ(1u, observer_.items_removed());
   // Remove the first item in the list.
-  model_.item_list()->DeleteItem(model_.GetItemName(0));
+  model_.DeleteItem(model_.GetItemName(0));
   EXPECT_EQ(num_apps - 2, model_.item_list()->item_count());
   EXPECT_EQ(2u, observer_.items_removed());
   // Remove the last item in the list.
-  model_.item_list()->DeleteItem(model_.GetItemName(num_apps - 1));
+  model_.DeleteItem(model_.GetItemName(num_apps - 1));
   EXPECT_EQ(num_apps - 3, model_.item_list()->item_count());
   EXPECT_EQ(3u, observer_.items_removed());
   // Ensure that the first item is the expected one
@@ -201,27 +201,6 @@ TEST_F(AppListModelTest, ModelRemoveItem) {
   EXPECT_EQ(model_.GetItemName(2), item0->id());
 }
 
-TEST_F(AppListModelTest, ModelRemoveItemByType) {
-  const size_t num_apps = 4;
-  model_.PopulateApps(num_apps);
-  model_.item_list()->AddItem(new AppListFolderItem("folder1"));
-  model_.item_list()->AddItem(new AppListFolderItem("folder2"));
-  model_.item_list()->DeleteItemsByType(test::AppListTestModel::kItemType);
-  EXPECT_EQ(num_apps, observer_.items_removed());
-  EXPECT_EQ(2u, model_.item_list()->item_count());
-  model_.item_list()->DeleteItemsByType(AppListFolderItem::kItemType);
-  EXPECT_EQ(num_apps + 2, observer_.items_removed());
-  EXPECT_EQ(0u, model_.item_list()->item_count());
-  // Delete all items
-  observer_.ResetCounts();
-  model_.PopulateApps(num_apps);
-  model_.item_list()->AddItem(new AppListFolderItem("folder1"));
-  model_.item_list()->AddItem(new AppListFolderItem("folder2"));
-  model_.item_list()->DeleteItemsByType(NULL /* all items */);
-  EXPECT_EQ(num_apps + 2, observer_.items_removed());
-  EXPECT_EQ(0u, model_.item_list()->item_count());
-}
-
 TEST_F(AppListModelTest, AppOrder) {
   const size_t num_apps = 5;
   model_.PopulateApps(num_apps);
@@ -245,7 +224,7 @@ TEST_F(AppListModelTest, FolderItem) {
   const size_t num_observed_apps = 4;
   for (int i = 0; static_cast<size_t>(i) < num_folder_apps; ++i) {
     std::string name = model_.GetItemName(i);
-    folder->item_list()->AddItem(model_.CreateItem(name, name));
+    AddFolderItem(folder.get(), name);
   }
   // Check that items 0 and 3 are observed.
   EXPECT_TRUE(ItemObservedByFolder(
@@ -272,5 +251,4 @@ TEST_F(AppListModelTest, FolderItem) {
   EXPECT_FALSE(ItemObservedByFolder(
       folder.get(), folder->item_list()->item_at(num_observed_apps)));
 }
-

[xiyuan, 2014/01/28 05:58:29]

nit: slightly prefer to keep one empty line after opening a namespace and before
closing it.

[stevenjb, 2014/01/28 18:28:08]

Unintentional. Done.

 }  // namespace app_list