mash: Replace int ShelfIDs with AppLaunchID strings.

chrome/browser/ui/ash/launcher/arc_app_launcher_browsertest.cc

Index: chrome/browser/ui/ash/launcher/arc_app_launcher_browsertest.cc
diff --git a/chrome/browser/ui/ash/launcher/arc_app_launcher_browsertest.cc b/chrome/browser/ui/ash/launcher/arc_app_launcher_browsertest.cc
index 06aec4a95a5d9ff39637964f3e3863d8ee43be1c..8785877942d161ab336e813c4f0dd5fe6ffb9ea4 100644
--- a/chrome/browser/ui/ash/launcher/arc_app_launcher_browsertest.cc
+++ b/chrome/browser/ui/ash/launcher/arc_app_launcher_browsertest.cc
@@ -308,11 +308,11 @@ IN_PROC_BROWSER_TEST_P(ArcAppDeferredLauncherBrowserTest, StartAppDeferred) {
   if (is_pinned()) {
     shelf_model->PinAppWithID(app_id);
     const ash::ShelfID shelf_id = shelf_model->GetShelfIDForAppID(app_id);
-    EXPECT_TRUE(shelf_id);
+    EXPECT_FALSE(shelf_id.IsEmpty());
     const ash::ShelfItem* item = chrome_controller()->GetItem(shelf_id);
     EXPECT_EQ(base::UTF8ToUTF16(kTestAppName), item->title);
   } else {
-    EXPECT_FALSE(shelf_model->GetShelfIDForAppID(app_id));
+    EXPECT_TRUE(shelf_model->GetShelfIDForAppID(app_id).IsEmpty());
   }
 
   StopInstance();
@@ -326,15 +326,12 @@ IN_PROC_BROWSER_TEST_P(ArcAppDeferredLauncherBrowserTest, StartAppDeferred) {
   app_info = app_prefs()->GetApp(app_id);
   ASSERT_TRUE(app_info);
   EXPECT_FALSE(app_info->ready);
-  if (is_pinned())
-    EXPECT_TRUE(shelf_model->GetShelfIDForAppID(app_id));
-  else
-    EXPECT_FALSE(shelf_model->GetShelfIDForAppID(app_id));
+  EXPECT_NE(is_pinned(), shelf_model->GetShelfIDForAppID(app_id).IsEmpty());
 
   // Launching non-ready ARC app creates item on shelf and spinning animation.
   arc::LaunchApp(profile(), app_id, ui::EF_LEFT_MOUSE_BUTTON);
   const ash::ShelfID shelf_id = shelf_model->GetShelfIDForAppID(app_id);
-  EXPECT_TRUE(shelf_id);
+  EXPECT_FALSE(shelf_id.IsEmpty());
   const ash::ShelfItem* item = chrome_controller()->GetItem(shelf_id);
   EXPECT_EQ(base::UTF8ToUTF16(kTestAppName), item->title);
   AppAnimatedWaiter(app_id).Wait();
@@ -349,29 +346,21 @@ IN_PROC_BROWSER_TEST_P(ArcAppDeferredLauncherBrowserTest, StartAppDeferred) {
                       ->GetArcDeferredLauncher()
                       ->GetActiveTime(app_id)
                       .is_zero());
-      if (is_pinned())
-        EXPECT_TRUE(shelf_model->GetShelfIDForAppID(app_id));
-      else
-        EXPECT_FALSE(shelf_model->GetShelfIDForAppID(app_id));
+      EXPECT_NE(is_pinned(), shelf_model->GetShelfIDForAppID(app_id).IsEmpty());
       break;
     case TEST_ACTION_EXIT:
       // Just exist Chrome.
       break;
     case TEST_ACTION_CLOSE:
       // Close item during animation.
-      {
-        ash::ShelfItemDelegate* delegate = GetShelfItemDelegate(app_id);
-        ASSERT_TRUE(delegate);
-        delegate->Close();
-        EXPECT_TRUE(chrome_controller()
-                        ->GetArcDeferredLauncher()
-                        ->GetActiveTime(app_id)
-                        .is_zero());
-        if (is_pinned())
-          EXPECT_TRUE(shelf_model->GetShelfIDForAppID(app_id));
-        else
-          EXPECT_FALSE(shelf_model->GetShelfIDForAppID(app_id));
-      }
+      ash::ShelfItemDelegate* delegate = GetShelfItemDelegate(app_id);

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

Huh, I'm surprised this compiles, since there's no {} scope for the case.
Something new in C++11?

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

Why do you think the brackets are needed?

[James Cook, 2017/05/04 19:45:08]

Because you're declaring a new variable but not adding a new scope. I think this
only works because there is no trailing case label. Otherwise I think you'll get
something like:

foo.cc: In function ‘int main()’:
foo.cc:13:10: error: jump to case label [-fpermissive]
     case 3:
          ^
foo.cc:10:11: error:   crosses initialization of ‘int b’
       int b = 3;

I would add braces.

[msw, 2017/05/04 20:39:07]

Done.

+      ASSERT_TRUE(delegate);
+      delegate->Close();
+      EXPECT_TRUE(chrome_controller()
+                      ->GetArcDeferredLauncher()
+                      ->GetActiveTime(app_id)
+                      .is_zero());
+      EXPECT_NE(is_pinned(), shelf_model->GetShelfIDForAppID(app_id).IsEmpty());
       break;
   }
 }
@@ -398,26 +387,26 @@ IN_PROC_BROWSER_TEST_F(ArcAppLauncherBrowserTest, PinOnPackageUpdateAndRemove) {
   shelf_model->PinAppWithID(app_id2);
   const ash::ShelfID shelf_id1_before =
       shelf_model->GetShelfIDForAppID(app_id1);
-  EXPECT_TRUE(shelf_id1_before);
-  EXPECT_TRUE(shelf_model->GetShelfIDForAppID(app_id2));
+  EXPECT_FALSE(shelf_id1_before.IsEmpty());
+  EXPECT_FALSE(shelf_model->GetShelfIDForAppID(app_id2).IsEmpty());
 
   // Package contains only one app. App list is not shown for updated package.
   SendPackageUpdated(kTestAppPackage, false);
   // Second pin should gone.
   EXPECT_EQ(shelf_id1_before, shelf_model->GetShelfIDForAppID(app_id1));
-  EXPECT_FALSE(shelf_model->GetShelfIDForAppID(app_id2));
+  EXPECT_TRUE(shelf_model->GetShelfIDForAppID(app_id2).IsEmpty());
 
   // Package contains two apps. App list is not shown for updated package.
   SendPackageUpdated(kTestAppPackage, true);
   // Second pin should not appear.
   EXPECT_EQ(shelf_id1_before, shelf_model->GetShelfIDForAppID(app_id1));
-  EXPECT_FALSE(shelf_model->GetShelfIDForAppID(app_id2));
+  EXPECT_TRUE(shelf_model->GetShelfIDForAppID(app_id2).IsEmpty());
 
   // Package removed.
   SendPackageRemoved(kTestAppPackage);
   // No pin is expected.
-  EXPECT_FALSE(shelf_model->GetShelfIDForAppID(app_id1));
-  EXPECT_FALSE(shelf_model->GetShelfIDForAppID(app_id2));
+  EXPECT_TRUE(shelf_model->GetShelfIDForAppID(app_id1).IsEmpty());
+  EXPECT_TRUE(shelf_model->GetShelfIDForAppID(app_id2).IsEmpty());
 }
 
 // This test validates that app list is shown on new package and not shown