mash: Replace int ShelfIDs with AppLaunchID strings.

ash/shelf/shelf_model_unittest.cc

Index: ash/shelf/shelf_model_unittest.cc
diff --git a/ash/shelf/shelf_model_unittest.cc b/ash/shelf/shelf_model_unittest.cc
index def513db41616682ecc94d5e238df329115d5273..7f5d27d61133db189deb2497a1707c32ecc50eaf 100644
--- a/ash/shelf/shelf_model_unittest.cc
+++ b/ash/shelf/shelf_model_unittest.cc
@@ -69,6 +69,7 @@ class ShelfModelTest : public testing::Test {
 
     ShelfItem item;
     item.type = TYPE_APP_LIST;
+    item.id = ShelfID("AppList");

[James Cook, 2017/05/04 16:38:49]

ditto

[msw, 2017/05/04 19:05:57]

Done.

     model_->Add(item);
     EXPECT_EQ(1, model_->item_count());
 
@@ -89,16 +90,17 @@ class ShelfModelTest : public testing::Test {
 
 TEST_F(ShelfModelTest, BasicAssertions) {
   // Add an item.
-  ShelfItem item;
-  item.type = TYPE_PINNED_APP;
-  int index = model_->Add(item);
+  ShelfItem item1;
+  item1.id = ShelfID("item1");
+  item1.type = TYPE_PINNED_APP;
+  int index = model_->Add(item1);
   EXPECT_EQ(2, model_->item_count());
   EXPECT_EQ("added=1", observer_->StateStringAndClear());
 
   // Change to a platform app item.
   ShelfID original_id = model_->items()[index].id;
-  item.type = TYPE_APP;
-  model_->Set(index, item);
+  item1.type = TYPE_APP;
+  model_->Set(index, item1);
   EXPECT_EQ(original_id, model_->items()[index].id);
   EXPECT_EQ("changed=1", observer_->StateStringAndClear());
   EXPECT_EQ(TYPE_APP, model_->items()[index].type);
@@ -109,18 +111,23 @@ TEST_F(ShelfModelTest, BasicAssertions) {
   EXPECT_EQ("removed=1", observer_->StateStringAndClear());
 
   // Add an app item.
-  item.type = TYPE_PINNED_APP;
-  index = model_->Add(item);
+  ShelfItem item2;
+  item2.id = ShelfID("item2");
+  item2.type = TYPE_PINNED_APP;
+  index = model_->Add(item2);
   observer_->StateStringAndClear();
 
-  // Change everything.
-  model_->Set(index, item);
+  // Change the item type.
+  item2.type = TYPE_APP;
+  model_->Set(index, item2);
   EXPECT_EQ("changed=1", observer_->StateStringAndClear());
-  EXPECT_EQ(TYPE_PINNED_APP, model_->items()[index].type);
+  EXPECT_EQ(TYPE_APP, model_->items()[index].type);
 
   // Add another item.
-  item.type = TYPE_PINNED_APP;
-  model_->Add(item);
+  ShelfItem item3;
+  item3.id = ShelfID("item3");
+  item3.type = TYPE_PINNED_APP;
+  model_->Add(item3);
   observer_->StateStringAndClear();
 
   // Move the second to the first.
@@ -142,6 +149,7 @@ TEST_F(ShelfModelTest, BasicAssertions) {
 TEST_F(ShelfModelTest, AddIndices) {
   // Insert browser short cut at index 1.
   ShelfItem browser_shortcut;
+  browser_shortcut.id = ShelfID("browser");
   browser_shortcut.type = TYPE_BROWSER_SHORTCUT;
   int browser_shortcut_index = model_->Add(browser_shortcut);
   EXPECT_EQ(1, browser_shortcut_index);
@@ -149,10 +157,12 @@ TEST_F(ShelfModelTest, AddIndices) {
   // App items should be after the browser shortcut.
   ShelfItem item;
   item.type = TYPE_APP;
+  item.id = ShelfID("id1");
   int platform_app_index1 = model_->Add(item);
   EXPECT_EQ(2, platform_app_index1);
 
   // Add another platform app item, it should follow first.
+  item.id = ShelfID("id2");
   int platform_app_index2 = model_->Add(item);
   EXPECT_EQ(3, platform_app_index2);
 
@@ -160,10 +170,12 @@ TEST_F(ShelfModelTest, AddIndices) {
   // TYPE_BROWSER_SHORTCUT. So TYPE_PINNED_APP is located after
   // TYPE_BROWSER_SHORTCUT.
   item.type = TYPE_PINNED_APP;
+  item.id = ShelfID("id3");
   int app_shortcut_index1 = model_->Add(item);
   EXPECT_EQ(2, app_shortcut_index1);
 
   item.type = TYPE_PINNED_APP;
+  item.id = ShelfID("id4");
   int app_shortcut_index2 = model_->Add(item);
   EXPECT_EQ(3, app_shortcut_index2);
 
@@ -172,14 +184,17 @@ TEST_F(ShelfModelTest, AddIndices) {
   // So TYPE_PINNED_APP is located at index 0. And, TYPE_BROWSER_SHORTCUT is
   // located at index 1.
   item.type = TYPE_PINNED_APP;
+  item.id = ShelfID("id5");
   int app_shortcut_index3 = model_->AddAt(1, item);
   EXPECT_EQ(1, app_shortcut_index3);
 
   item.type = TYPE_PINNED_APP;
+  item.id = ShelfID("id6");
   int app_shortcut_index4 = model_->AddAt(6, item);
   EXPECT_EQ(5, app_shortcut_index4);
 
   item.type = TYPE_PINNED_APP;
+  item.id = ShelfID("id7");
   int app_shortcut_index5 = model_->AddAt(3, item);
   EXPECT_EQ(3, app_shortcut_index5);
 
@@ -188,26 +203,32 @@ TEST_F(ShelfModelTest, AddIndices) {
 
   // Check that AddAt() figures out the correct indexes for apps and panels.
   item.type = TYPE_APP;
+  item.id = ShelfID("id8");
   int platform_app_index3 = model_->AddAt(3, item);
   EXPECT_EQ(7, platform_app_index3);
 
   item.type = TYPE_APP_PANEL;
+  item.id = ShelfID("id9");
   int app_panel_index1 = model_->AddAt(2, item);
   EXPECT_EQ(10, app_panel_index1);
 
   item.type = TYPE_APP;
+  item.id = ShelfID("id10");
   int platform_app_index4 = model_->AddAt(11, item);
   EXPECT_EQ(10, platform_app_index4);
 
   item.type = TYPE_APP_PANEL;
+  item.id = ShelfID("id11");
   int app_panel_index2 = model_->AddAt(12, item);
   EXPECT_EQ(12, app_panel_index2);
 
   item.type = TYPE_APP;
+  item.id = ShelfID("id12");
   int platform_app_index5 = model_->AddAt(7, item);
   EXPECT_EQ(7, platform_app_index5);
 
   item.type = TYPE_APP_PANEL;
+  item.id = ShelfID("id13");
   int app_panel_index3 = model_->AddAt(13, item);
   EXPECT_EQ(13, app_panel_index3);
 
@@ -223,6 +244,7 @@ TEST_F(ShelfModelTest, FirstRunningAppIndex) {
   // Insert the browser shortcut at index 1 and check that the running
   // application index would be behind it.
   ShelfItem item;
+  item.id = ShelfID("browser");
   item.type = TYPE_BROWSER_SHORTCUT;
   EXPECT_EQ(1, model_->Add(item));
   EXPECT_EQ(2, model_->FirstRunningAppIndex());
@@ -230,120 +252,103 @@ TEST_F(ShelfModelTest, FirstRunningAppIndex) {
   // Insert a panel application at the end and check that the running
   // application index would be at / before the application panel.
   item.type = TYPE_APP_PANEL;
+  item.id = ShelfID("app panel");
   EXPECT_EQ(2, model_->Add(item));
   EXPECT_EQ(2, model_->FirstRunningAppIndex());
 
   // Insert an application shortcut and make sure that the running application
   // index would be behind it.
   item.type = TYPE_PINNED_APP;
+  item.id = ShelfID("pinned app");
   EXPECT_EQ(2, model_->Add(item));
   EXPECT_EQ(3, model_->FirstRunningAppIndex());
 
   // Insert a two app items and check the first running app index.
   item.type = TYPE_APP;
+  item.id = ShelfID("app1");
   EXPECT_EQ(3, model_->Add(item));
   EXPECT_EQ(3, model_->FirstRunningAppIndex());
+  item.id = ShelfID("app2");
   EXPECT_EQ(4, model_->Add(item));
   EXPECT_EQ(3, model_->FirstRunningAppIndex());
 }
 
-// Assertions around id generation and usage.
-TEST_F(ShelfModelTest, ShelfIDTests) {
-  // Get the next to use ID counter.
-  ShelfID id = model_->next_id();
-
-  // Calling this function multiple times does not change the returned ID.
-  EXPECT_EQ(model_->next_id(), id);
-
-  // Adding another item to the list should produce a new ID.
-  ShelfItem item;
-  item.type = TYPE_APP;
-  model_->Add(item);
-  EXPECT_NE(model_->next_id(), id);
-}
-
-// This verifies that converting an existing item into a lower weight category
-// (e.g. shortcut to running but not pinned app) will move it to the proper
-// location. See crbug.com/248769.
-TEST_F(ShelfModelTest, CorrectMoveItemsWhenStateChange) {
-  // The first item is the app list and last item is the browser.
-  ShelfItem browser_shortcut;
-  browser_shortcut.type = TYPE_BROWSER_SHORTCUT;
-  int browser_shortcut_index = model_->Add(browser_shortcut);
+// Test item reordering on type/weight (eg. pinning) changes. crbug.com/248769.
+TEST_F(ShelfModelTest, ReorderOnTypeChanges) {
   EXPECT_EQ(TYPE_APP_LIST, model_->items()[0].type);
-  EXPECT_EQ(1, browser_shortcut_index);
 
-  // Add three shortcuts. They should all be moved between the two.
-  ShelfItem item;
-  item.type = TYPE_PINNED_APP;
-  int app1_index = model_->Add(item);
-  EXPECT_EQ(2, app1_index);
-  int app2_index = model_->Add(item);
-  EXPECT_EQ(3, app2_index);
-  int app3_index = model_->Add(item);
-  EXPECT_EQ(4, app3_index);
-
-  // Now change the type of the second item and make sure that it is moving
-  // behind the shortcuts.
-  item.type = TYPE_APP;
-  model_->Set(app2_index, item);
-
-  // The item should have moved in front of the app launcher.
-  EXPECT_EQ(TYPE_APP, model_->items()[4].type);
+  // Add three pinned items.
+  ShelfItem item1;
+  item1.type = TYPE_PINNED_APP;
+  item1.id = ShelfID("id1");
+  int app1_index = model_->Add(item1);
+  EXPECT_EQ(1, app1_index);
+
+  ShelfItem item2;
+  item2.type = TYPE_PINNED_APP;
+  item2.id = ShelfID("id2");
+  int app2_index = model_->Add(item2);
+  EXPECT_EQ(2, app2_index);
+
+  ShelfItem item3;
+  item3.type = TYPE_PINNED_APP;
+  item3.id = ShelfID("id3");
+  int app3_index = model_->Add(item3);
+  EXPECT_EQ(3, app3_index);
+
+  // Unpinning an item moves it behind the shortcuts.
+  EXPECT_EQ(item3.id, model_->items()[3].id);
+  item2.type = TYPE_APP;
+  model_->Set(app2_index, item2);
+  EXPECT_EQ(item2.id, model_->items()[3].id);
 }
 
 // Test conversion between ShelfID and application [launch] ids.
 TEST_F(ShelfModelTest, IdentifierConversion) {
   const std::string app_id1("app_id1");
   const std::string launch_id("launch_id");
-  const ShelfID unknown_shelf_id = 123;
 
-  // Expect kInvalidShelfID and empty app ids for input not found in the model.
-  EXPECT_EQ(kInvalidShelfID, model_->GetShelfIDForAppID(std::string()));
-  EXPECT_EQ(kInvalidShelfID, model_->GetShelfIDForAppID(app_id1));
-  EXPECT_EQ(kInvalidShelfID,
-            model_->GetShelfIDForAppIDAndLaunchID(app_id1, std::string()));
-  EXPECT_EQ(kInvalidShelfID,
-            model_->GetShelfIDForAppIDAndLaunchID(app_id1, launch_id));
-  EXPECT_TRUE(model_->GetAppIDForShelfID(kInvalidShelfID).empty());
-  EXPECT_TRUE(model_->GetAppIDForShelfID(unknown_shelf_id).empty());
+  // Expect empty ShelfIDs and app ids for input not found in the model.
+  EXPECT_TRUE(model_->GetShelfIDForAppID(std::string()).IsEmpty());
+  EXPECT_TRUE(model_->GetShelfIDForAppID(app_id1).IsEmpty());
+  EXPECT_TRUE(
+      model_->GetShelfIDForAppIDAndLaunchID(app_id1, std::string()).IsEmpty());
+  EXPECT_TRUE(
+      model_->GetShelfIDForAppIDAndLaunchID(app_id1, launch_id).IsEmpty());
+  EXPECT_TRUE(model_->GetAppIDForShelfID(ShelfID()).empty());
+  EXPECT_TRUE(model_->GetAppIDForShelfID(ShelfID("foo")).empty());
 
   // Add an example app with an app id and a launch id.
   ShelfItem item;
   item.type = TYPE_PINNED_APP;
-  item.app_launch_id = AppLaunchId(app_id1, launch_id);
-  const ShelfID assigned_shelf_id1 = model_->next_id();
+  item.id = ShelfID(app_id1, launch_id);
   const int index = model_->Add(item);
 
   // Ensure the item ids can be found and converted as expected.
-  EXPECT_NE(kInvalidShelfID, assigned_shelf_id1);
-  EXPECT_EQ(assigned_shelf_id1, model_->GetShelfIDForAppID(app_id1));
-  EXPECT_EQ(assigned_shelf_id1,
-            model_->GetShelfIDForAppIDAndLaunchID(app_id1, launch_id));
-  EXPECT_EQ(app_id1, model_->GetAppIDForShelfID(assigned_shelf_id1));
+  EXPECT_EQ(item.id, model_->GetShelfIDForAppID(app_id1));
+  EXPECT_EQ(item.id, model_->GetShelfIDForAppIDAndLaunchID(app_id1, launch_id));
+  EXPECT_NE(item.id,
+            model_->GetShelfIDForAppIDAndLaunchID(app_id1, std::string()));
+  EXPECT_EQ(app_id1, model_->GetAppIDForShelfID(item.id));
 
   // Removing the example app should again yield invalid ids.
   model_->RemoveItemAt(index);
-  EXPECT_EQ(kInvalidShelfID, model_->GetShelfIDForAppID(app_id1));
-  EXPECT_EQ(kInvalidShelfID,
-            model_->GetShelfIDForAppIDAndLaunchID(app_id1, launch_id));
-  EXPECT_TRUE(model_->GetAppIDForShelfID(assigned_shelf_id1).empty());
+  EXPECT_TRUE(model_->GetShelfIDForAppID(app_id1).IsEmpty());
+  EXPECT_TRUE(
+      model_->GetShelfIDForAppIDAndLaunchID(app_id1, launch_id).IsEmpty());
+  EXPECT_TRUE(model_->GetAppIDForShelfID(item.id).empty());
 
   // Add an example app with a different app id and no launch id.
   const std::string app_id2("app_id2");
-  item.app_launch_id = AppLaunchId(app_id2);
-  const ShelfID assigned_shelf_id2 = model_->next_id();
+  item.id = ShelfID(app_id2);
   model_->Add(item);
 
   // Ensure the item ids can be found and converted as expected.
-  EXPECT_NE(kInvalidShelfID, assigned_shelf_id2);
-  EXPECT_NE(assigned_shelf_id1, assigned_shelf_id2);
-  EXPECT_EQ(assigned_shelf_id2, model_->GetShelfIDForAppID(app_id2));
-  EXPECT_EQ(assigned_shelf_id2,
+  EXPECT_EQ(item.id, model_->GetShelfIDForAppID(app_id2));
+  EXPECT_EQ(item.id,
             model_->GetShelfIDForAppIDAndLaunchID(app_id2, std::string()));
-  EXPECT_EQ(kInvalidShelfID,
-            model_->GetShelfIDForAppIDAndLaunchID(app_id2, launch_id));
-  EXPECT_EQ(app_id2, model_->GetAppIDForShelfID(assigned_shelf_id2));
+  EXPECT_NE(item.id, model_->GetShelfIDForAppIDAndLaunchID(app_id2, launch_id));
+  EXPECT_EQ(app_id2, model_->GetAppIDForShelfID(item.id));
 }
 
 // Test pinning and unpinning a closed app, and checking if it is pinned.
@@ -359,14 +364,14 @@ TEST_F(ShelfModelTest, ClosedAppPinning) {
   EXPECT_TRUE(model_->IsAppPinned(app_id));
   EXPECT_EQ(2, model_->item_count());
   EXPECT_EQ(TYPE_PINNED_APP, model_->items()[1].type);
-  EXPECT_EQ(app_id, model_->items()[1].app_launch_id.app_id());
+  EXPECT_EQ(app_id, model_->items()[1].id.app_id());
 
   // Pinning the same app id again should have no change.
   model_->PinAppWithID(app_id);
   EXPECT_TRUE(model_->IsAppPinned(app_id));
   EXPECT_EQ(2, model_->item_count());
   EXPECT_EQ(TYPE_PINNED_APP, model_->items()[1].type);
-  EXPECT_EQ(app_id, model_->items()[1].app_launch_id.app_id());
+  EXPECT_EQ(app_id, model_->items()[1].id.app_id());
 
   // Unpinning the app should remove the item.
   model_->UnpinAppWithID(app_id);
@@ -391,48 +396,42 @@ TEST_F(ShelfModelTest, RunningAppPinning) {
   ShelfItem item;
   item.type = TYPE_APP;
   item.status = STATUS_RUNNING;
-  item.app_launch_id = AppLaunchId(app_id);
-  const ShelfID assigned_shelf_id = model_->next_id();
+  item.id = ShelfID(app_id);
   const int index = model_->Add(item);
 
   // The item should be added but not pinned.
   EXPECT_FALSE(model_->IsAppPinned(app_id));
   EXPECT_EQ(2, model_->item_count());
   EXPECT_EQ(TYPE_APP, model_->items()[index].type);
-  EXPECT_EQ(app_id, model_->items()[index].app_launch_id.app_id());
-  EXPECT_EQ(assigned_shelf_id, model_->items()[index].id);
+  EXPECT_EQ(item.id, model_->items()[index].id);
 
   // Pinning the item should just change its type.
   model_->PinAppWithID(app_id);
   EXPECT_TRUE(model_->IsAppPinned(app_id));
   EXPECT_EQ(2, model_->item_count());
   EXPECT_EQ(TYPE_PINNED_APP, model_->items()[index].type);
-  EXPECT_EQ(app_id, model_->items()[index].app_launch_id.app_id());
-  EXPECT_EQ(assigned_shelf_id, model_->items()[index].id);
+  EXPECT_EQ(item.id, model_->items()[index].id);
 
   // Pinning the same app id again should have no change.
   model_->PinAppWithID(app_id);
   EXPECT_TRUE(model_->IsAppPinned(app_id));
   EXPECT_EQ(2, model_->item_count());
   EXPECT_EQ(TYPE_PINNED_APP, model_->items()[index].type);
-  EXPECT_EQ(app_id, model_->items()[index].app_launch_id.app_id());
-  EXPECT_EQ(assigned_shelf_id, model_->items()[index].id);
+  EXPECT_EQ(item.id, model_->items()[index].id);
 
   // Unpinning the app should leave the item unpinnned but running.
   model_->UnpinAppWithID(app_id);
   EXPECT_FALSE(model_->IsAppPinned(app_id));
   EXPECT_EQ(2, model_->item_count());
   EXPECT_EQ(TYPE_APP, model_->items()[index].type);
-  EXPECT_EQ(app_id, model_->items()[index].app_launch_id.app_id());
-  EXPECT_EQ(assigned_shelf_id, model_->items()[index].id);
+  EXPECT_EQ(item.id, model_->items()[index].id);
 
   // Unpinning the same app id again should have no change.
   model_->UnpinAppWithID(app_id);
   EXPECT_FALSE(model_->IsAppPinned(app_id));
   EXPECT_EQ(2, model_->item_count());
   EXPECT_EQ(TYPE_APP, model_->items()[index].type);
-  EXPECT_EQ(app_id, model_->items()[index].app_launch_id.app_id());
-  EXPECT_EQ(assigned_shelf_id, model_->items()[index].id);
+  EXPECT_EQ(item.id, model_->items()[index].id);
 }
 
 }  // namespace ash