Combine shelf platform and windowed app types.

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

 // 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 "chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <map>
@@ skipping 393 matching lines @@
                                           manifest, Extension::NO_FLAGS,
                                           ArcSupportHost::kHostAppId, &error);
   }
 
   // Creates a running V2 app (not pinned) of type |app_id|.
   virtual void CreateRunningV2App(const std::string& app_id) {
     DCHECK(!test_controller_);
     ash::ShelfID id =
         launcher_controller_->CreateAppShortcutLauncherItemWithType(
             ash::launcher::AppLauncherId(app_id), model_->item_count(),
-            ash::TYPE_PLATFORM_APP);
+            ash::TYPE_APP);
     DCHECK(id);
     // Change the created launcher controller into a V2 app controller.
     test_controller_ = new TestV2AppLauncherItemController(app_id,
         launcher_controller_.get());
     launcher_controller_->SetItemController(id, test_controller_);
   }
 
   // Sets the stage for a multi user test.
   virtual void SetUpMultiUserScenario(syncer::SyncChangeList* user_a,
                                       syncer::SyncChangeList* user_b) {
@@ skipping 251 matching lines @@
   // Get the setup of the currently shown launcher items in one string.
   // Each pinned element will start with a big letter, each running but not
   // pinned V1 app will start with a small letter and each running but not
   // pinned V2 app will start with a '*' + small letter.
   std::string GetPinnedAppStatus() {
     std::string result;
     for (int i = 0; i < model_->item_count(); i++) {
       if (!result.empty())
         result.append(", ");
       switch (model_->items()[i].type) {
-        case ash::TYPE_PLATFORM_APP:
-          result += "*";
-            // FALLTHROUGH
-        case ash::TYPE_WINDOWED_APP: {
+        case ash::TYPE_APP: {
+          if (controller()->IsPlatformApp(item().id))
+            result += "*";
           const std::string& app =
               launcher_controller_->GetAppIDForShelfID(model_->items()[i].id);
-            if (app == extension1_->id()) {
-              result += "app1";
-              EXPECT_FALSE(
-                  launcher_controller_->IsAppPinned(extension1_->id()));
-            } else if (app == extension2_->id()) {
-              result += "app2";
-              EXPECT_FALSE(
-                  launcher_controller_->IsAppPinned(extension2_->id()));
-            } else if (app == extension3_->id()) {
-              result += "app3";
-              EXPECT_FALSE(
-                  launcher_controller_->IsAppPinned(extension3_->id()));
-            } else if (app == extension4_->id()) {
-              result += "app4";
-              EXPECT_FALSE(
-                  launcher_controller_->IsAppPinned(extension4_->id()));
-            } else if (app == extension5_->id()) {
-              result += "app5";
-              EXPECT_FALSE(
-                  launcher_controller_->IsAppPinned(extension5_->id()));
-            } else if (app == extension6_->id()) {
-              result += "app6";
-              EXPECT_FALSE(
-                  launcher_controller_->IsAppPinned(extension6_->id()));
-            } else if (app == extension7_->id()) {
-              result += "app7";
-              EXPECT_FALSE(
-                  launcher_controller_->IsAppPinned(extension7_->id()));
-            } else if (app == extension8_->id()) {
-              result += "app8";
-              EXPECT_FALSE(
-                  launcher_controller_->IsAppPinned(extension8_->id()));
-            } else {
-              result += "unknown";
-            }
-            break;
+          EXPECT_FALSE(launcher_controller_->IsAppPinned(app));
+          if (app == extension1_->id()) {
+            result += "app1";
+          } else if (app == extension2_->id()) {
+            result += "app2";
+          } else if (app == extension3_->id()) {
+            result += "app3";
+          } else if (app == extension4_->id()) {
+            result += "app4";
+          } else if (app == extension5_->id()) {
+            result += "app5";
+          } else if (app == extension6_->id()) {
+            result += "app6";
+          } else if (app == extension7_->id()) {
+            result += "app7";
+          } else if (app == extension8_->id()) {
+            result += "app8";
+          } else {
+            result += "unknown";
           }
+          break;
+        }
         case ash::TYPE_APP_SHORTCUT: {
           const std::string& app =
               launcher_controller_->GetAppIDForShelfID(model_->items()[i].id);
-            if (app == extension1_->id()) {
-              result += "App1";
-              EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
-            } else if (app == extension2_->id()) {
-              result += "App2";
-              EXPECT_TRUE(launcher_controller_->IsAppPinned(extension2_->id()));
-            } else if (app == extension3_->id()) {
-              result += "App3";
-              EXPECT_TRUE(launcher_controller_->IsAppPinned(extension3_->id()));
-            } else if (app == extension4_->id()) {
-              result += "App4";
-              EXPECT_TRUE(launcher_controller_->IsAppPinned(extension4_->id()));
-            } else if (app == extension5_->id()) {
-              result += "App5";
-              EXPECT_TRUE(launcher_controller_->IsAppPinned(extension5_->id()));
-            } else if (app == extension6_->id()) {
-              result += "App6";
-              EXPECT_TRUE(launcher_controller_->IsAppPinned(extension6_->id()));
-            } else if (app == extension7_->id()) {
-              result += "App7";
-              EXPECT_TRUE(launcher_controller_->IsAppPinned(extension7_->id()));
-            } else if (app == extension8_->id()) {
-              result += "App8";
-              EXPECT_TRUE(launcher_controller_->IsAppPinned(extension8_->id()));
-            } else if (app == arc_support_host_->id()) {
-              result += "Play Store";
-              EXPECT_TRUE(
-                  launcher_controller_->IsAppPinned(arc_support_host_->id()));
-            } else {
-              bool arc_app_found = false;
-              for (const auto& arc_app : arc_test_.fake_apps()) {
-                if (app == ArcAppTest::GetAppId(arc_app)) {
-                  result += arc_app.name;
-                  arc_app_found = true;
-                  break;
-                }
+          EXPECT_TRUE(launcher_controller_->IsAppPinned(app));
+          if (app == extension1_->id()) {
+            result += "App1";
+          } else if (app == extension2_->id()) {
+            result += "App2";
+          } else if (app == extension3_->id()) {
+            result += "App3";
+          } else if (app == extension4_->id()) {
+            result += "App4";
+          } else if (app == extension5_->id()) {
+            result += "App5";
+          } else if (app == extension6_->id()) {
+            result += "App6";
+          } else if (app == extension7_->id()) {
+            result += "App7";
+          } else if (app == extension8_->id()) {
+            result += "App8";
+          } else if (app == arc_support_host_->id()) {
+            result += "Play Store";
+          } else {
+            bool arc_app_found = false;
+            for (const auto& arc_app : arc_test_.fake_apps()) {
+              if (app == ArcAppTest::GetAppId(arc_app)) {
+                result += arc_app.name;
+                arc_app_found = true;
+                break;
               }
-              if (!arc_app_found)
-                result += "unknown";
             }
-            break;
+            if (!arc_app_found)
+              result += "unknown";
           }
+          break;
+        }
         case ash::TYPE_BROWSER_SHORTCUT:
           result += "Chrome";
           break;
         case ash::TYPE_APP_LIST:
           result += "AppList";
           break;
         default:
           result += "Unknown";
           break;
       }
@@ skipping 787 matching lines @@
                                       UnloadedExtensionInfo::REASON_UPDATE);
   EXPECT_EQ("AppList, Chrome, App3", GetPinnedAppStatus());
 }
 
 // Check that simple locking of an application will 'create' a launcher item.
 TEST_F(ChromeLauncherControllerImplTest, CheckLockApps) {
   InitLauncherController();
   // Model should only contain the browser shortcut and app list items.
   EXPECT_EQ(2, model_->item_count());
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_EQ(ash::kInvalidShelfID, GetShelfIDForAppID(extension1_->id()));
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension2_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension2_->id()));
+  EXPECT_EQ(ash::kInvalidShelfID, GetShelfIDForAppID(extension2_->id()));
 
   launcher_controller_->LockV1AppWithID(extension1_->id());
 
   EXPECT_EQ(3, model_->item_count());
-  EXPECT_EQ(ash::TYPE_WINDOWED_APP, model_->items()[2].type);
+  EXPECT_EQ(ash::TYPE_APP, model_->items()[2].type);
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_TRUE(launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_NE(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension2_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension2_->id()));
+  EXPECT_EQ(0, launcher_controller_->GetShelfIDForAppID(extension2_->id()));
 
   launcher_controller_->UnlockV1AppWithID(extension1_->id());
 
   EXPECT_EQ(2, model_->item_count());
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_EQ(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension2_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension2_->id()));
+  EXPECT_EQ(0, launcher_controller_->GetShelfIDForAppID(extension2_->id()));
 }
 
 // Check that multiple locks of an application will be properly handled.
 TEST_F(ChromeLauncherControllerImplTest, CheckMultiLockApps) {
   InitLauncherController();
   // Model should only contain the browser shortcut and app list items.
   EXPECT_EQ(2, model_->item_count());
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_EQ(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
   for (int i = 0; i < 2; i++) {
     launcher_controller_->LockV1AppWithID(extension1_->id());
 
     EXPECT_EQ(3, model_->item_count());
-    EXPECT_EQ(ash::TYPE_WINDOWED_APP, model_->items()[2].type);
+    EXPECT_EQ(ash::TYPE_APP, model_->items()[2].type);
     EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
-    EXPECT_TRUE(launcher_controller_->IsWindowedAppInLauncher(
-        extension1_->id()));
+    EXPECT_NE(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
   }
 
   launcher_controller_->UnlockV1AppWithID(extension1_->id());
 
   EXPECT_EQ(3, model_->item_count());
-  EXPECT_EQ(ash::TYPE_WINDOWED_APP, model_->items()[2].type);
+  EXPECT_EQ(ash::TYPE_APP, model_->items()[2].type);
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_TRUE(launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_NE(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
   launcher_controller_->UnlockV1AppWithID(extension1_->id());
 
   EXPECT_EQ(2, model_->item_count());
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
-  EXPECT_FALSE(launcher_controller_->IsAppPinned(extension2_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_EQ(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 }
 
 // Check that already pinned items are not effected by locks.
 TEST_F(ChromeLauncherControllerImplTest, CheckAlreadyPinnedLockApps) {
   InitLauncherController();
   // Model should only contain the browser shortcut and app list items.
   EXPECT_EQ(2, model_->item_count());
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_EQ(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
-  EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
   launcher_controller_->PinAppWithID(extension1_->id());
-  EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));

[Mr4D (OOO till 08-26), 2016/11/23 23:11:51]

Hmmm. Why did you remove this test? After pinning the item should be pinned?

[msw, 2016/11/24 00:00:55]

This is redundant with line 1621, right?

[Mr4D (OOO till 08-26), 2016/11/24 02:21:33]

Yes. There it is. Never mind.

 
   EXPECT_EQ(3, model_->item_count());
   EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[2].type);
   EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_NE(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
   launcher_controller_->LockV1AppWithID(extension1_->id());
 
   EXPECT_EQ(3, model_->item_count());
   EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[2].type);
   EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_NE(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
   launcher_controller_->UnlockV1AppWithID(extension1_->id());
 
   EXPECT_EQ(3, model_->item_count());
   EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[2].type);
   EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_NE(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
   launcher_controller_->UnpinAppWithID(extension1_->id());
 
   EXPECT_EQ(2, model_->item_count());
+  EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));

[Mr4D (OOO till 08-26), 2016/11/23 23:11:51]

Note: that you also test for not being pinned after unpinning...

[msw, 2016/11/24 00:00:55]

Would you like me to add a comment here, like "Ensure the item is not pinned
after unpinning."? I ask because that seems too self-explanatory for a special
one-off comment, and in general these tests don't give step-by-step comments
like that.

[Mr4D (OOO till 08-26), 2016/11/24 02:21:33]

No need, that was meant for the comment above which is resolved. Thanks!

+  EXPECT_EQ(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 }
 
 // Check that already pinned items which get locked stay after unpinning.
 TEST_F(ChromeLauncherControllerImplTest, CheckPinnedAppsStayAfterUnlock) {
   InitLauncherController();
   // Model should only contain the browser shortcut and app list items.
   EXPECT_EQ(2, model_->item_count());
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_EQ(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
   launcher_controller_->PinAppWithID(extension1_->id());
 
   EXPECT_EQ(3, model_->item_count());
   EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[2].type);
   EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_NE(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
   launcher_controller_->LockV1AppWithID(extension1_->id());
 
   EXPECT_EQ(3, model_->item_count());
   EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[2].type);
   EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_NE(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
   launcher_controller_->UnpinAppWithID(extension1_->id());
 
   EXPECT_EQ(3, model_->item_count());
-  EXPECT_EQ(ash::TYPE_WINDOWED_APP, model_->items()[2].type);
+  EXPECT_EQ(ash::TYPE_APP, model_->items()[2].type);
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_TRUE(launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_NE(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
   launcher_controller_->UnlockV1AppWithID(extension1_->id());
 
   EXPECT_EQ(2, model_->item_count());
+  EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
+  EXPECT_EQ(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 }
 
 // Check that running applications wich are not pinned get properly restored
 // upon user change.
 TEST_F(ChromeLauncherControllerImplTest, CheckRunningAppOrder) {
   InitLauncherController();
   // Model should only contain the browser shortcut and app list items.
   EXPECT_EQ(2, model_->item_count());
 
   // Add a few running applications.
@@ skipping 544 matching lines @@
   launcher_controller_->ActivateWindowOrMinimizeIfActive(browser_window, false);
   EXPECT_TRUE(manager->IsWindowOnDesktopOfUser(window, current_user));
 }
 
 // Check that lock -> pin -> unlock -> unpin does properly transition.
 TEST_F(ChromeLauncherControllerImplTest, CheckLockPinUnlockUnpin) {
   InitLauncherController();
   // Model should only contain the browser shortcut and app list items.
   EXPECT_EQ(2, model_->item_count());
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_EQ(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
   launcher_controller_->LockV1AppWithID(extension1_->id());
 
   EXPECT_EQ(3, model_->item_count());
-  EXPECT_EQ(ash::TYPE_WINDOWED_APP, model_->items()[2].type);
+  EXPECT_EQ(ash::TYPE_APP, model_->items()[2].type);
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_TRUE(launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_NE(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
   launcher_controller_->PinAppWithID(extension1_->id());
 
   EXPECT_EQ(3, model_->item_count());
   EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[2].type);
   EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_NE(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
   launcher_controller_->UnlockV1AppWithID(extension1_->id());
 
   EXPECT_EQ(3, model_->item_count());
   EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[2].type);
   EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_FALSE(
-      launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
+  EXPECT_NE(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
   launcher_controller_->UnpinAppWithID(extension1_->id());
 
   EXPECT_EQ(2, model_->item_count());
+  EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
+  EXPECT_EQ(0, launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 }
 
 // Check that a locked (windowed V1 application) will be properly converted
 // between locked and pinned when the order gets changed through a profile /
 // policy change.
 TEST_F(ChromeLauncherControllerImplTest,
        RestoreDefaultAndLockedAppsResyncOrder) {
   InitLauncherController();
 
   syncer::SyncChangeList sync_list;
@@ skipping 1707 matching lines @@
 
   // Pinned state should not change.
   EXPECT_EQ("AppList, Chrome, App1, App2", GetPinnedAppStatus());
   launcher_controller_->UnpinAppWithID(extension2_->id());
   EXPECT_EQ("AppList, Chrome, App1", GetPinnedAppStatus());
 
   // Resume syncing and sync information overrides local copy.
   StartAppSyncService(copy_sync_list);
   EXPECT_EQ("AppList, Chrome, App1, App2", GetPinnedAppStatus());
 }