mash: Remove V1 app shelf item locking; monitor running status instead.

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 436 matching lines @@
         Extension::NO_FLAGS, "gggggggggggggggggggggggggggggggg", &error);
     arc_support_host_ = Extension::Create(base::FilePath(), Manifest::UNPACKED,
                                           manifest, Extension::NO_FLAGS,
                                           ArcSupportHost::kHostAppId, &error);
     extension_service_->AddExtension(extension_chrome_.get());
   }
 
   // Creates a running platform 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::AppLaunchId(app_id), model_->item_count(), 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_);
+    ash::ShelfID id = launcher_controller_->InsertAppLauncherItem(
+        test_controller_, ash::STATUS_RUNNING, model_->item_count(),
+        ash::TYPE_APP);
     DCHECK(launcher_controller_->IsPlatformApp(id));
-    launcher_controller_->SetItemStatus(id, ash::STATUS_RUNNING);
   }
 
   // Sets the stage for a multi user test.
   virtual void SetUpMultiUserScenario(syncer::SyncChangeList* user_a,
                                       syncer::SyncChangeList* user_b) {
     InitLauncherController();
     EXPECT_EQ("AppList, Chrome", GetPinnedAppStatus());
 
     // Set an empty pinned pref to begin with.
     syncer::SyncChangeList sync_list;
@@ skipping 833 matching lines @@
  private:
   DISALLOW_COPY_AND_ASSIGN(ChromeLauncherControllerImplMultiProfileWithArcTest);
 };
 
 INSTANTIATE_TEST_CASE_P(,
                         ChromeLauncherControllerImplMultiProfileWithArcTest,
                         ::testing::Bool());
 
 TEST_F(ChromeLauncherControllerImplTest, DefaultApps) {
   InitLauncherController();
-  // Model should only contain the browser shortcut and app list items.
+  // The 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_->IsAppPinned(extension2_->id()));
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension3_->id()));
 
   // Installing |extension3_| should add it to the launcher - behind the
   // chrome icon.
   extension_service_->AddExtension(extension3_.get());
   EXPECT_EQ("AppList, Chrome, App3", GetPinnedAppStatus());
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
@@ skipping 164 matching lines @@
 // order.
 TEST_F(ChromeLauncherControllerImplTest, RestoreDefaultAppsReverseOrder) {
   InitLauncherController();
 
   syncer::SyncChangeList sync_list;
   InsertAddPinChange(&sync_list, 0, extension1_->id());
   InsertAddPinChange(&sync_list, 1, extension2_->id());
   InsertAddPinChange(&sync_list, 2, extension3_->id());
   SendPinChanges(sync_list, true);
 
-  // Model should only contain the browser shortcut and app list items.
+  // The model should only contain the browser shortcut and app list items.
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension2_->id()));
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension3_->id()));
   EXPECT_EQ("AppList, Chrome", GetPinnedAppStatus());
 
   // Installing |extension3_| should add it to the shelf - behind the
   // chrome icon.
   ash::ShelfItem item;
   extension_service_->AddExtension(extension3_.get());
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
@@ skipping 17 matching lines @@
 // order.
 TEST_F(ChromeLauncherControllerImplTest, RestoreDefaultAppsRandomOrder) {
   InitLauncherController();
 
   syncer::SyncChangeList sync_list;
   InsertAddPinChange(&sync_list, 0, extension1_->id());
   InsertAddPinChange(&sync_list, 1, extension2_->id());
   InsertAddPinChange(&sync_list, 2, extension3_->id());
   SendPinChanges(sync_list, true);
 
-  // Model should only contain the browser shortcut and app list items.
+  // The model should only contain the browser shortcut and app list items.
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension2_->id()));
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension3_->id()));
   EXPECT_EQ("AppList, Chrome", GetPinnedAppStatus());
 
   // Installing |extension2_| should add it to the launcher - behind the
   // chrome icon.
   extension_service_->AddExtension(extension2_.get());
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension3_->id()));
@@ skipping 18 matching lines @@
        RestoreDefaultAppsRandomOrderChromeMoved) {
   InitLauncherController();
 
   syncer::SyncChangeList sync_list;
   InsertAddPinChange(&sync_list, 0, extension1_->id());
   InsertAddPinChange(&sync_list, 1, extension_misc::kChromeAppId);
   InsertAddPinChange(&sync_list, 2, extension2_->id());
   InsertAddPinChange(&sync_list, 3, extension3_->id());
   SendPinChanges(sync_list, true);
 
-  // Model should only contain the browser shortcut and app list items.
+  // The model should only contain the browser shortcut and app list items.
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension2_->id()));
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension3_->id()));
   EXPECT_EQ("AppList, Chrome", GetPinnedAppStatus());
 
   // Installing |extension2_| should add it to the shelf - behind the
   // chrome icon.
   ash::ShelfItem item;
   extension_service_->AddExtension(extension2_.get());
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
@@ skipping 63 matching lines @@
   extension_service_->UnloadExtension(extension2_->id(),
                                       UnloadedExtensionInfo::REASON_UNINSTALL);
   EXPECT_EQ("AppList, Chrome, App3", GetPinnedAppStatus());
 
   // Check that an update of an extension does not crash the system.
   extension_service_->UnloadExtension(extension3_->id(),
                                       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) {
+// Test the V1 app interaction flow: run it, activate it, close it.
+TEST_F(ChromeLauncherControllerImplTest, V1AppRunActivateClose) {
   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_EQ(ash::kInvalidShelfID,
-            launcher_controller_->GetShelfIDForAppID(extension1_->id()));
-  EXPECT_FALSE(launcher_controller_->IsAppPinned(extension2_->id()));
-  EXPECT_EQ(ash::kInvalidShelfID,
-            launcher_controller_->GetShelfIDForAppID(extension2_->id()));
-
-  launcher_controller_->LockV1AppWithID(extension1_->id());
-
-  EXPECT_EQ(3, model_->item_count());
-  EXPECT_EQ(ash::TYPE_APP, model_->items()[2].type);
-  EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_NE(ash::kInvalidShelfID,
-            launcher_controller_->GetShelfIDForAppID(extension1_->id()));
-  EXPECT_FALSE(launcher_controller_->IsAppPinned(extension2_->id()));
-  EXPECT_EQ(ash::kInvalidShelfID,
-            launcher_controller_->GetShelfIDForAppID(extension2_->id()));
-
-  launcher_controller_->UnlockV1AppWithID(extension1_->id());
-
-  EXPECT_EQ(2, model_->item_count());
-  EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_EQ(ash::kInvalidShelfID,
-            launcher_controller_->GetShelfIDForAppID(extension1_->id()));
-  EXPECT_FALSE(launcher_controller_->IsAppPinned(extension2_->id()));
-  EXPECT_EQ(ash::kInvalidShelfID,
-            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.
+  // The 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_EQ(ash::kInvalidShelfID,
             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_APP, model_->items()[2].type);
-    EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
-    EXPECT_NE(ash::kInvalidShelfID,
-              launcher_controller_->GetShelfIDForAppID(extension1_->id()));
-  }
-
-  launcher_controller_->UnlockV1AppWithID(extension1_->id());
-
+  // Reporting that the app is running should create a new shelf item.
+  launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_RUNNING);
   EXPECT_EQ(3, model_->item_count());
   EXPECT_EQ(ash::TYPE_APP, model_->items()[2].type);
+  EXPECT_EQ(ash::STATUS_RUNNING, model_->items()[2].status);
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
   EXPECT_NE(ash::kInvalidShelfID,
             launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
-  launcher_controller_->UnlockV1AppWithID(extension1_->id());
+  // Reporting an active status should just update the existing item.
+  launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_ACTIVE);
+  EXPECT_EQ(3, model_->item_count());
+  EXPECT_EQ(ash::STATUS_ACTIVE, model_->items()[2].status);
 
+  // Reporting that the app is closed should remove its shelf item.
+  launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_CLOSED);
+  EXPECT_EQ(2, model_->item_count());
+  EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
+  EXPECT_EQ(ash::kInvalidShelfID,
+            launcher_controller_->GetShelfIDForAppID(extension1_->id()));
+
+  // Reporting that the app is closed agani should have no effect.

[James Cook, 2017/03/31 19:13:22]

nit: again

[msw, 2017/03/31 19:36:06]

Done.

+  launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_CLOSED);
+  EXPECT_EQ(2, model_->item_count());
+}
+
+// Test the V1 app interaction flow: pin it, run it, close it, unpin it.
+TEST_F(ChromeLauncherControllerImplTest, V1AppPinRunCloseUnpin) {
+  InitLauncherController();
+  // The 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_EQ(ash::kInvalidShelfID,
+            launcher_controller_->GetShelfIDForAppID(extension1_->id()));
+
+  // Pinning the app should create a new shelf item.
+  launcher_controller_->PinAppWithID(extension1_->id());
+  EXPECT_EQ(3, model_->item_count());
+  EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);
+  EXPECT_EQ(ash::STATUS_CLOSED, model_->items()[2].status);
+  EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
+  EXPECT_NE(ash::kInvalidShelfID,
+            launcher_controller_->GetShelfIDForAppID(extension1_->id()));
+
+  // Reporting that the app is running should just update the existing item.
+  launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_RUNNING);
+  EXPECT_EQ(3, model_->item_count());
+  EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);
+  EXPECT_EQ(ash::STATUS_RUNNING, model_->items()[2].status);
+  EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
+  EXPECT_NE(ash::kInvalidShelfID,
+            launcher_controller_->GetShelfIDForAppID(extension1_->id()));
+
+  // Reporting that the app is closed should just update the existing item.
+  launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_CLOSED);
+  EXPECT_EQ(3, model_->item_count());
+  EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);
+  EXPECT_EQ(ash::STATUS_CLOSED, model_->items()[2].status);
+  EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
+  EXPECT_NE(ash::kInvalidShelfID,
+            launcher_controller_->GetShelfIDForAppID(extension1_->id()));
+
+  // Unpinning the app should remove its shelf item.
+  launcher_controller_->UnpinAppWithID(extension1_->id());
   EXPECT_EQ(2, model_->item_count());
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
   EXPECT_EQ(ash::kInvalidShelfID,
             launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 }

[James Cook, 2017/03/31 19:13:22]

nice tests, documented and easy to read blocks.

[msw, 2017/03/31 19:36:06]

Acknowledged.

 
-// Check that already pinned items are not effected by locks.
-TEST_F(ChromeLauncherControllerImplTest, CheckAlreadyPinnedLockApps) {
+// Test the V1 app interaction flow: run it, pin it, close it, unpin it.
+TEST_F(ChromeLauncherControllerImplTest, V1AppRunPinCloseUnpin) {
   InitLauncherController();
-  // Model should only contain the browser shortcut and app list items.
+  // The 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_EQ(ash::kInvalidShelfID,
             launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
+  // Reporting that the app is running should create a new shelf item.
+  launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_RUNNING);
+  EXPECT_EQ(3, model_->item_count());
+  EXPECT_EQ(ash::TYPE_APP, model_->items()[2].type);
+  EXPECT_EQ(ash::STATUS_RUNNING, model_->items()[2].status);
+  EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
+  EXPECT_NE(ash::kInvalidShelfID,
+            launcher_controller_->GetShelfIDForAppID(extension1_->id()));
+
+  // Pinning the app should just update the existing item.
   launcher_controller_->PinAppWithID(extension1_->id());
-
   EXPECT_EQ(3, model_->item_count());
   EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);
+  EXPECT_EQ(ash::STATUS_RUNNING, model_->items()[2].status);
   EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
   EXPECT_NE(ash::kInvalidShelfID,
             launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
-  launcher_controller_->LockV1AppWithID(extension1_->id());
-
+  // Reporting that the app is closed should just update the existing item.
+  launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_CLOSED);
   EXPECT_EQ(3, model_->item_count());
   EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);
+  EXPECT_EQ(ash::STATUS_CLOSED, model_->items()[2].status);
   EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
   EXPECT_NE(ash::kInvalidShelfID,
             launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
-  launcher_controller_->UnlockV1AppWithID(extension1_->id());
-
-  EXPECT_EQ(3, model_->item_count());
-  EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);
-  EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_NE(ash::kInvalidShelfID,
-            launcher_controller_->GetShelfIDForAppID(extension1_->id()));
-
+  // Unpinning the app should remove its shelf item.
   launcher_controller_->UnpinAppWithID(extension1_->id());
-
   EXPECT_EQ(2, model_->item_count());
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
   EXPECT_EQ(ash::kInvalidShelfID,
             launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 }
 
-// Check that already pinned items which get locked stay after unpinning.
-TEST_F(ChromeLauncherControllerImplTest, CheckPinnedAppsStayAfterUnlock) {
+// Test the V1 app interaction flow: pin it, run it, unpin it, close it.
+TEST_F(ChromeLauncherControllerImplTest, V1AppPinRunUnpinClose) {
   InitLauncherController();
-  // Model should only contain the browser shortcut and app list items.
+  // The 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_EQ(ash::kInvalidShelfID,
             launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
+  // Pinning the app should create a new shelf item.
   launcher_controller_->PinAppWithID(extension1_->id());
-
   EXPECT_EQ(3, model_->item_count());
   EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);
+  EXPECT_EQ(ash::STATUS_CLOSED, model_->items()[2].status);
   EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
   EXPECT_NE(ash::kInvalidShelfID,
             launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
-  launcher_controller_->LockV1AppWithID(extension1_->id());
-
+  // Reporting that the app is running should just update the existing item.
+  launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_RUNNING);
   EXPECT_EQ(3, model_->item_count());
   EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);
+  EXPECT_EQ(ash::STATUS_RUNNING, model_->items()[2].status);
   EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
   EXPECT_NE(ash::kInvalidShelfID,
             launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
+  // Unpinning the app should just update the existing item.
   launcher_controller_->UnpinAppWithID(extension1_->id());
-
   EXPECT_EQ(3, model_->item_count());
   EXPECT_EQ(ash::TYPE_APP, model_->items()[2].type);
+  EXPECT_EQ(ash::STATUS_RUNNING, model_->items()[2].status);
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
   EXPECT_NE(ash::kInvalidShelfID,
             launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 
-  launcher_controller_->UnlockV1AppWithID(extension1_->id());
-
+  // Reporting that the app is closed should remove its shelf item.
+  launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_CLOSED);
   EXPECT_EQ(2, model_->item_count());
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
   EXPECT_EQ(ash::kInvalidShelfID,
             launcher_controller_->GetShelfIDForAppID(extension1_->id()));
 }
 
-// Check that running applications wich are not pinned get properly restored
-// upon user change.
-TEST_F(ChromeLauncherControllerImplTest, CheckRunningAppOrder) {
+// Ensure unpinned V1 app ordering is properly restored after user changes.
+TEST_F(ChromeLauncherControllerImplTest, CheckRunningV1AppOrder) {
   InitLauncherController();
-  // Model should only contain the browser shortcut and app list items.
+  // The model should only contain the browser shortcut and app list items.
   EXPECT_EQ(2, model_->item_count());
 
   // Add a few running applications.
-  launcher_controller_->LockV1AppWithID(extension1_->id());
-  launcher_controller_->LockV1AppWithID(extension2_->id());
-  launcher_controller_->LockV1AppWithID(extension3_->id());
+  launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_RUNNING);
+  launcher_controller_->SetV1AppStatus(extension2_->id(), ash::STATUS_RUNNING);
+  launcher_controller_->SetV1AppStatus(extension3_->id(), ash::STATUS_RUNNING);
   EXPECT_EQ(5, model_->item_count());
   // Note that this not only checks the order of applications but also the
   // running type.
   EXPECT_EQ("AppList, Chrome, app1, app2, app3", GetPinnedAppStatus());
 
   // Remember the current order of applications for the current user.
   const AccountId& current_account_id =
       multi_user_util::GetAccountIdFromProfile(profile());
   RememberUnpinnedRunningApplicationOrder();
 
   // Switch some items and check that restoring a user which was not yet
   // remembered changes nothing.
   model_->Move(2, 3);
   EXPECT_EQ("AppList, Chrome, app2, app1, app3", GetPinnedAppStatus());
   const AccountId second_fake_account_id(
       AccountId::FromUserEmail("second-fake-user@fake.com"));
   RestoreUnpinnedRunningApplicationOrder(second_fake_account_id);
   EXPECT_EQ("AppList, Chrome, app2, app1, app3", GetPinnedAppStatus());
 
   // Restoring the stored user should however do the right thing.
   RestoreUnpinnedRunningApplicationOrder(current_account_id);
   EXPECT_EQ("AppList, Chrome, app1, app2, app3", GetPinnedAppStatus());
 
   // Switch again some items and even delete one - making sure that the missing
   // item gets properly handled.
   model_->Move(3, 4);
-  launcher_controller_->UnlockV1AppWithID(extension1_->id());
+  launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_CLOSED);
   EXPECT_EQ("AppList, Chrome, app3, app2", GetPinnedAppStatus());
   RestoreUnpinnedRunningApplicationOrder(current_account_id);
   EXPECT_EQ("AppList, Chrome, app2, app3", GetPinnedAppStatus());
 
   // Check that removing more items does not crash and changes nothing.
-  launcher_controller_->UnlockV1AppWithID(extension2_->id());
+  launcher_controller_->SetV1AppStatus(extension2_->id(), ash::STATUS_CLOSED);
   RestoreUnpinnedRunningApplicationOrder(current_account_id);
   EXPECT_EQ("AppList, Chrome, app3", GetPinnedAppStatus());
-  launcher_controller_->UnlockV1AppWithID(extension3_->id());
+  launcher_controller_->SetV1AppStatus(extension3_->id(), ash::STATUS_CLOSED);
   RestoreUnpinnedRunningApplicationOrder(current_account_id);
   EXPECT_EQ("AppList, Chrome", GetPinnedAppStatus());
 }
 
 TEST_P(ChromeLauncherControllerImplWithArcTest, ArcDeferredLaunch) {
   RecreateChromeLauncher();
 
   const arc::mojom::AppInfo& app1 = arc_test_.fake_apps()[0];
   const arc::mojom::AppInfo& app2 = arc_test_.fake_apps()[1];
   const arc::mojom::AppInfo& app3 = arc_test_.fake_apps()[2];
@@ skipping 593 matching lines @@
 
   // Transfer the window to another user's desktop and check that activating it
   // does pull it back to that user.
   manager->ShowWindowForUser(
       window, multi_user_util::GetAccountIdFromProfile(profile2));
   EXPECT_FALSE(manager->IsWindowOnDesktopOfUser(window, current_user));
   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_EQ(ash::kInvalidShelfID,
-            launcher_controller_->GetShelfIDForAppID(extension1_->id()));
-
-  launcher_controller_->LockV1AppWithID(extension1_->id());
-
-  EXPECT_EQ(3, model_->item_count());
-  EXPECT_EQ(ash::TYPE_APP, model_->items()[2].type);
-  EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_NE(ash::kInvalidShelfID,
-            launcher_controller_->GetShelfIDForAppID(extension1_->id()));
-
-  launcher_controller_->PinAppWithID(extension1_->id());
-
-  EXPECT_EQ(3, model_->item_count());
-  EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);
-  EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_NE(ash::kInvalidShelfID,
-            launcher_controller_->GetShelfIDForAppID(extension1_->id()));
-
-  launcher_controller_->UnlockV1AppWithID(extension1_->id());
-
-  EXPECT_EQ(3, model_->item_count());
-  EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);
-  EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
-  EXPECT_NE(ash::kInvalidShelfID,
-            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(ash::kInvalidShelfID,
-            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.
+// Check that a running windowed V1 application will be properly pinned and
+// unpinned when the order gets changed through a profile / policy change.
 TEST_F(ChromeLauncherControllerImplTest,
-       RestoreDefaultAndLockedAppsResyncOrder) {
+       RestoreDefaultAndRunningV1AppsResyncOrder) {
   InitLauncherController();
 
   syncer::SyncChangeList sync_list;
   InsertAddPinChange(&sync_list, 0, extension1_->id());
   InsertAddPinChange(&sync_list, 1, extension3_->id());
   SendPinChanges(sync_list, true);
 
   // The shelf layout has always one static item at the beginning (App List).
   extension_service_->AddExtension(extension1_.get());
   EXPECT_EQ("AppList, Chrome, App1", GetPinnedAppStatus());
   extension_service_->AddExtension(extension2_.get());
   // No new app icon will be generated.
   EXPECT_EQ("AppList, Chrome, App1", GetPinnedAppStatus());
 
-  // Add the app as locked app which will add it (un-pinned).
-  launcher_controller_->LockV1AppWithID(extension2_->id());
+  // Set the app status as running, which will add an unpinned item.
+  launcher_controller_->SetV1AppStatus(extension2_->id(), ash::STATUS_RUNNING);
   EXPECT_EQ("AppList, Chrome, App1, app2", GetPinnedAppStatus());
   extension_service_->AddExtension(extension3_.get());
   EXPECT_EQ("AppList, Chrome, App1, App3, app2", GetPinnedAppStatus());
 
-  // Now request to pin all items which should convert the locked item into a
-  // pinned item.
+  // Now request to pin all items, which will pin the running unpinned items.
   syncer::SyncChangeList sync_list1;
   InsertAddPinChange(&sync_list1, 0, extension3_->id());
   InsertAddPinChange(&sync_list1, 1, extension2_->id());
   InsertAddPinChange(&sync_list1, 2, extension1_->id());
   SendPinChanges(sync_list1, true);
   EXPECT_EQ("AppList, Chrome, App3, App2, App1", GetPinnedAppStatus());
 
-  // Going back to a status where there is no requirement for app 2 to be pinned
-  // should convert it back to locked but not pinned and state. The position
-  // is determined by the |ShelfModel|'s weight system and since running
-  // applications are not allowed to be mixed with shortcuts, it should show up
-  // at the end of the list.
+  // Removing the requirement for app 2 to be pinned should convert it back to
+  // running but not pinned. It should move towards the end of the shelf, after
+  // the pinned items, as determined by the |ShelfModel|'s weight system.
   syncer::SyncChangeList sync_list2;
   InsertAddPinChange(&sync_list2, 0, extension3_->id());
   InsertAddPinChange(&sync_list2, 1, extension1_->id());
   SendPinChanges(sync_list2, true);
   EXPECT_EQ("AppList, Chrome, App3, App1, app2", GetPinnedAppStatus());
 
   // Removing an item should simply close it and everything should shift.
   SendPinChanges(syncer::SyncChangeList(), true);
   EXPECT_EQ("AppList, Chrome, App3, app2", GetPinnedAppStatus());
 }
 
-// Check that a running and not pinned V2 application will be properly converted
-// between locked and pinned when the order gets changed through a profile /
-// policy change.
+// Check that a running unpinned V2 application will be properly pinned and
+// unpinned when the order gets changed through a profile / policy change.
 TEST_F(ChromeLauncherControllerImplTest,
        RestoreDefaultAndRunningV2AppsResyncOrder) {
   InitLauncherController();
   syncer::SyncChangeList sync_list0;
   InsertAddPinChange(&sync_list0, 0, extension1_->id());
   InsertAddPinChange(&sync_list0, 1, extension3_->id());
   SendPinChanges(sync_list0, true);
   // The shelf layout has always one static item at the beginning (app List).
   extension_service_->AddExtension(extension1_.get());
   EXPECT_EQ("AppList, Chrome, App1", GetPinnedAppStatus());
   extension_service_->AddExtension(extension_platform_app_.get());
   // No new app icon will be generated.
   EXPECT_EQ("AppList, Chrome, App1", GetPinnedAppStatus());
   // Add an unpinned but running V2 app.
   CreateRunningV2App(extension_platform_app_->id());
   EXPECT_EQ("AppList, Chrome, App1, *platform_app", GetPinnedAppStatus());
   extension_service_->AddExtension(extension3_.get());
   EXPECT_EQ("AppList, Chrome, App1, App3, *platform_app", GetPinnedAppStatus());
 
-  // Now request to pin all items which should convert the locked item into a
-  // pinned item.
+  // Now request to pin all items, which should pin the running unpinned item.
   syncer::SyncChangeList sync_list1;
   InsertAddPinChange(&sync_list1, 0, extension3_->id());
   InsertAddPinChange(&sync_list1, 1, extension_platform_app_->id());
   InsertAddPinChange(&sync_list1, 2, extension1_->id());
   SendPinChanges(sync_list1, true);
   EXPECT_EQ("AppList, Chrome, App3, *Platform_App, App1", GetPinnedAppStatus());
 
-  // Going back to a status where there is no requirement for the V2 app to be
-  // pinned should convert it back to running V2 app. Since the position is
-  // determined by the |ShelfModel|'s weight system, it will be after last
-  // pinned item.
+  // Removing the requirement for app 2 to be pinned should convert it back to
+  // running but not pinned. It should move towards the end of the shelf, after
+  // the pinned items, as determined by the |ShelfModel|'s weight system.
   syncer::SyncChangeList sync_list2;
   InsertAddPinChange(&sync_list2, 0, extension3_->id());
   InsertAddPinChange(&sync_list2, 1, extension1_->id());
   SendPinChanges(sync_list2, true);
   EXPECT_EQ("AppList, Chrome, App3, App1, *platform_app", GetPinnedAppStatus());
 
   // Removing an item should simply close it and everything should shift.
   syncer::SyncChangeList sync_list3;
   InsertAddPinChange(&sync_list3, 0, extension3_->id());
   SendPinChanges(sync_list3, true);
@@ skipping 396 matching lines @@
 // Check that V1 apps are correctly reflected in the launcher menu using the
 // refocus logic.
 // Note that the extension matching logic is tested by the extension system
 // and does not need a separate test here.
 TEST_F(ChromeLauncherControllerImplTest, V1AppMenuGeneration) {
   EXPECT_EQ(1U, chrome::GetTotalBrowserCount());
   EXPECT_EQ(0, browser()->tab_strip_model()->count());
 
   InitLauncherControllerWithBrowser();
 
-  // Model should only contain the browser shortcut and app list items.
+  // The model should only contain the browser shortcut and app list items.
   EXPECT_EQ(2, model_->item_count());
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension3_->id()));
 
   // Installing |extension3_| adds it to the launcher.
   ash::ShelfID gmail_id = model_->next_id();
   extension_service_->AddExtension(extension3_.get());
   EXPECT_EQ(3, model_->item_count());
   int gmail_index = model_->ItemIndexByID(gmail_id);
   EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[gmail_index].type);
   EXPECT_TRUE(launcher_controller_->IsAppPinned(extension3_->id()));
@@ skipping 1401 matching lines @@
   EXPECT_EQ(ash::SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS,
             shelf_controller->auto_hide());
   EXPECT_EQ(2u, shelf_controller->auto_hide_change_count());
 
   PrefService* prefs = profile()->GetTestingPrefService();
   EXPECT_EQ("Left", prefs->GetString(prefs::kShelfAlignmentLocal));
   EXPECT_EQ("Left", prefs->GetString(prefs::kShelfAlignment));
   EXPECT_EQ("Always", prefs->GetString(prefs::kShelfAutoHideBehaviorLocal));
   EXPECT_EQ("Always", prefs->GetString(prefs::kShelfAutoHideBehavior));
 }