Store AppItems as pages in AppListModel.

chrome/browser/ui/app_list/test/fast_show_pickler_unittest.cc

Index: chrome/browser/ui/app_list/test/fast_show_pickler_unittest.cc
diff --git a/chrome/browser/ui/app_list/test/fast_show_pickler_unittest.cc b/chrome/browser/ui/app_list/test/fast_show_pickler_unittest.cc
index e87fe895a0a9241c8d2b5f303b60956a656123dd..2f174a64289a755b8d036ab19276196c47213cb5 100644
--- a/chrome/browser/ui/app_list/test/fast_show_pickler_unittest.cc
+++ b/chrome/browser/ui/app_list/test/fast_show_pickler_unittest.cc
@@ -17,17 +17,22 @@ using app_list::AppListModel;
 class AppListModelPicklerUnitTest : public testing::Test {
  protected:
   void CheckIsSame(AppListModel* m1, AppListModel* m2) {
-    ASSERT_EQ(m1->apps()->item_count(), m2->apps()->item_count());
     ASSERT_EQ(m1->signed_in(), m2->signed_in());
-    for (size_t i = 0; i < m1->apps()->item_count(); i++) {
-      ASSERT_EQ(m1->apps()->GetItemAt(i)->id(),
-                m2->apps()->GetItemAt(i)->id());
-      ASSERT_EQ(m1->apps()->GetItemAt(i)->title(),
-                m2->apps()->GetItemAt(i)->title());
-      ASSERT_EQ(m1->apps()->GetItemAt(i)->full_name(),
-                m2->apps()->GetItemAt(i)->full_name());
-      CompareImages(m1->apps()->GetItemAt(i)->icon(),
-                    m2->apps()->GetItemAt(i)->icon());
+    ASSERT_EQ(m1->GetNumAppPages(), m2->GetNumAppPages());
+    for (size_t p = 0; p < m1->GetNumAppPages(); ++p) {
+      const AppListModel::AppItems& m1apps = m1->GetAppItems(p);
+      const AppListModel::AppItems& m2apps = m2->GetAppItems(p);
+      ASSERT_EQ(m1apps.item_count(), m2apps.item_count());
+      for (size_t i = 0; i < m1apps.item_count(); ++i) {
+        ASSERT_EQ(m1apps.GetItemAt(i)->id(),
+                  m2apps.GetItemAt(i)->id());
+        ASSERT_EQ(m1apps.GetItemAt(i)->title(),
+                  m2apps.GetItemAt(i)->title());
+        ASSERT_EQ(m1apps.GetItemAt(i)->full_name(),
+                  m2apps.GetItemAt(i)->full_name());
+        CompareImages(m1apps.GetItemAt(i)->icon(),
+                      m2apps.GetItemAt(i)->icon());
+      }
     }
   }
 
@@ -43,16 +48,15 @@ class AppListModelPicklerUnitTest : public testing::Test {
     }
   }
 
-  scoped_ptr<AppListModel> CopyViaPickle(AppListModel* model) {
+  void DoConsistencyChecks(AppListModel* model) {
     scoped_ptr<Pickle> pickle(
         FastShowPickler::PickleAppListModelForFastShow(model));
-    return FastShowPickler::UnpickleAppListModelForFastShow(pickle.get());
-  }
-
-  void DoConsistencyChecks(AppListModel* model) {
-    scoped_ptr<AppListModel> model2(CopyViaPickle(model));
+    ASSERT_TRUE(pickle);
+    scoped_ptr<AppListModel> model2(
+        FastShowPickler::UnpickleAppListModelForFastShow(pickle.get()));
+    ASSERT_TRUE(model2);
     AppListModel dest_model;
-    FastShowPickler::CopyOver(model2.get(), &dest_model);
+    CopyAppListModel(model2.get(), &dest_model);
 
     CheckIsSame(model, model2.get());
     CheckIsSame(model, &dest_model);
@@ -68,6 +72,27 @@ class AppListModelPicklerUnitTest : public testing::Test {
     bitmap.eraseARGB(255, 1, 2, 3);
     return gfx::ImageSkia::CreateFrom1xBitmap(bitmap);
   }
+
+  void CopyAppListItemModel(const AppListItemModel* src_item,
+                            AppListItemModel* dest_item) {
+    dest_item->SetTitleAndFullName(src_item->title(), src_item->full_name());
+    dest_item->SetIcon(src_item->icon(), src_item->has_shadow());
+  }
+
+  void CopyAppListModel(const AppListModel* src, AppListModel* dest) {

[koz (OOO until 15th September), 2013/10/18 05:17:17]

These two methods aren't just used for testing. FastShowPickler is dead code
currently but the intent of it is to unpickle a model and then copy that over an
existing model. It's true one could change AppListView to not create its own
model, then you wouldn't need to copy over it, but until then this should live
with the real code.

[stevenjb, 2013/10/18 22:14:26]

I see. I'll move it back with a comment; I'll admit it's unclear to me what this
is/was/will be used for. I had assumed this function was only used for testing
since that's the only place it was called from. Is there an Issue for the change
that will use this?

+    ASSERT_EQ(0u, dest->GetNumAppPages());
+    dest->SetSignedIn(src->signed_in());
+    for (size_t p = 0; p < src->GetNumAppPages(); ++p) {
+      const AppListModel::AppItems& apps = src->GetAppItems(p);
+      for (size_t i = 0; i < apps.item_count(); ++i) {
+        const AppListItemModel* src_item = apps.GetItemAt(i);
+        AppListItemModel* dest_item = new AppListItemModel(src_item->id());
+        CopyAppListItemModel(src_item, dest_item);
+        size_t page_idx = dest->AddItem(dest_item);
+        EXPECT_EQ(p, page_idx);
+      }
+    }
+  }
 };
 
 TEST_F(AppListModelPicklerUnitTest, EmptyModel) {
@@ -79,7 +104,7 @@ TEST_F(AppListModelPicklerUnitTest, OneItem) {
   AppListModel model;
   AppListItemModel* app1 = new AppListItemModel("abc");
   app1->SetTitleAndFullName("ht", "hello, there");
-  model.apps()->Add(app1);
+  model.AddItem(app1);
 
   DoConsistencyChecks(&model);
 }
@@ -88,11 +113,11 @@ TEST_F(AppListModelPicklerUnitTest, TwoItems) {
   AppListModel model;
   AppListItemModel* app1 = new AppListItemModel("abc");
   app1->SetTitleAndFullName("ht", "hello, there");
-  model.apps()->Add(app1);
+  model.AddItem(app1);
 
   AppListItemModel* app2 = new AppListItemModel("abc2");
   app2->SetTitleAndFullName("ht2", "hello, there 2");
-  model.apps()->Add(app2);
+  model.AddItem(app2);
 
   DoConsistencyChecks(&model);
 }
@@ -102,11 +127,11 @@ TEST_F(AppListModelPicklerUnitTest, Images) {
   AppListItemModel* app1 = new AppListItemModel("abc");
   app1->SetTitleAndFullName("ht", "hello, there");
   app1->SetIcon(MakeImage(), true);
-  model.apps()->Add(app1);
+  model.AddItem(app1);
 
   AppListItemModel* app2 = new AppListItemModel("abc2");
   app2->SetTitleAndFullName("ht2", "hello, there 2");
-  model.apps()->Add(app2);
+  model.AddItem(app2);
 
   DoConsistencyChecks(&model);
 }
@@ -116,7 +141,7 @@ TEST_F(AppListModelPicklerUnitTest, EmptyImage) {
   AppListItemModel* app1 = new AppListItemModel("abc");
   app1->SetTitleAndFullName("ht", "hello, there");
   app1->SetIcon(gfx::ImageSkia(), true);
-  model.apps()->Add(app1);
+  model.AddItem(app1);
 
   DoConsistencyChecks(&model);
 }