Fixed problems with pin/unpin preferences changing for not running applications & icon order chang…

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

Index: chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
index 67077868e4a0ead5e5f0d9f6e41d35d34df6a924..92001d4dbb9bbf52a12e27d395a871256b7a3740 100644
--- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
+++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
@@ -177,6 +177,54 @@ class TestAppTabHelperImpl : public ChromeLauncherController::AppTabHelper {
   DISALLOW_COPY_AND_ASSIGN(TestAppTabHelperImpl);
 };
 
+// Test implementation of a V2 app launcher item controller.
+class TestV2AppLauncherItemController : public LauncherItemController {
+ public:
+  TestV2AppLauncherItemController(const std::string& app_id,
+                                  ChromeLauncherController* controller)
+      : LauncherItemController(LauncherItemController::TYPE_APP,
+                               app_id,
+                               controller) {
+  }
+
+  virtual ~TestV2AppLauncherItemController() {}
+
+  // Override for LauncherItemController:
+  virtual string16 GetTitle() OVERRIDE;
+  virtual bool IsCurrentlyShownInWindow(aura::Window* window) const OVERRIDE;
+  virtual bool IsOpen() const OVERRIDE;
+  virtual bool IsVisible() const OVERRIDE;
+  virtual void Launch(int event_flags) OVERRIDE;
+  virtual void Activate() OVERRIDE;
+  virtual void Close() OVERRIDE;
+  virtual void Clicked(const ui::Event& event) OVERRIDE;
+  virtual void OnRemoved() OVERRIDE;
+  virtual ChromeLauncherAppMenuItems GetApplicationList(
+      int event_flags) OVERRIDE;
+
+ private:
+
+  DISALLOW_COPY_AND_ASSIGN(TestV2AppLauncherItemController);
+};
+
+string16 TestV2AppLauncherItemController::GetTitle() { return string16(); }
+bool TestV2AppLauncherItemController::IsCurrentlyShownInWindow(
+    aura::Window* window) const { return true; }
+bool TestV2AppLauncherItemController::IsOpen() const { return true; }
+bool TestV2AppLauncherItemController::IsVisible() const { return true; }
+void TestV2AppLauncherItemController::Launch(int event_flags) {}
+void TestV2AppLauncherItemController::Activate() {}
+void TestV2AppLauncherItemController::Close() {}
+void TestV2AppLauncherItemController::Clicked(const ui::Event& event) {}
+void TestV2AppLauncherItemController::OnRemoved() {}
+ChromeLauncherAppMenuItems TestV2AppLauncherItemController::GetApplicationList(
+    int event_flags) {
+  ChromeLauncherAppMenuItems items;
+  items.push_back(new ChromeLauncherAppMenuItem(string16(), NULL, false));
+  items.push_back(new ChromeLauncherAppMenuItem(string16(), NULL, false));
+  return items.Pass();

[oshima, 2013/09/19 21:43:25]

can you just inline them?

[Mr4D (OOO till 08-26), 2013/09/19 22:31:50]

Done.

+}
+
 }  // namespace
 
 class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {
@@ -246,6 +294,20 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {
                                     &error);
   }
 
+  // Creates a running V2 app (not pinned) of type |app_id|.
+  virtual void CreateRunningV2App(const std::string& app_id) {
+    ash::LauncherID id =
+        launcher_controller_->CreateAppShortcutLauncherItemWithType(
+            app_id,
+            model_->item_count(),
+            ash::TYPE_PLATFORM_APP);
+    DCHECK(id);
+    // Change the created launcher controller into a V2 app controller.
+    launcher_controller_->SetItemController(id,
+        new TestV2AppLauncherItemController(app_id,
+                                            launcher_controller_.get()));
+  }
+
   virtual void TearDown() OVERRIDE {
     model_->RemoveObserver(model_observer_.get());
     model_observer_.reset();
@@ -305,10 +367,42 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {
     }
   }
 
+  // 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++) {
       switch (model_->items()[i].type) {
+        case ash::TYPE_PLATFORM_APP:
+            result+= "*";
+            // FALLTHROUGH
+        case ash::TYPE_WINDOWED_APP: {
+            const std::string& app =
+                launcher_controller_->GetAppIDForLauncherID(
+                    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 {
+              result += "unknown, ";
+            }
+            break;
+          }
         case ash::TYPE_APP_SHORTCUT: {
             const std::string& app =
                 launcher_controller_->GetAppIDForLauncherID(
@@ -347,7 +441,7 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {
   // Set the index at which the chrome icon should be.
   void SetShelfChromeIconIndex(int index) {
     profile()->GetTestingPrefService()->SetInteger(prefs::kShelfChromeIconIndex,
-                                                   index + 1);
+                                                   index);
   }
 
   // Needed for extension service & friends to work.
@@ -787,7 +881,7 @@ TEST_F(ChromeLauncherControllerTest, CheckLockApps) {
 }
 
 // Check that multiple locks of an application will be properly handled.
-TEST_F(ChromeLauncherControllerTest, CheckMukltiLockApps) {
+TEST_F(ChromeLauncherControllerTest, CheckMultiLockApps) {
   InitLauncherController();
   // Model should only contain the browser shortcut and app list items.
   EXPECT_EQ(2, model_->item_count());
@@ -937,6 +1031,114 @@ TEST_F(ChromeLauncherControllerTest, CheckLockPinUnlockUnpin) {
   EXPECT_EQ(2, model_->item_count());
 }
 
+// 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(ChromeLauncherControllerTest, RestoreDefaultAndLockedAppsResyncOrder) {
+  InitLauncherController();
+  base::ListValue policy_value0;
+  InsertPrefValue(&policy_value0, 0, extension1_->id());
+  InsertPrefValue(&policy_value0, 1, extension3_->id());
+  profile()->GetTestingPrefService()->SetUserPref(prefs::kPinnedLauncherApps,
+                                                  policy_value0.DeepCopy());
+  // The shelf layout has always one static item at the beginning (App List).
+  SetShelfChromeIconIndex(0);
+  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());
+  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.
+  base::ListValue policy_value1;
+  InsertPrefValue(&policy_value1, 0, extension3_->id());
+  InsertPrefValue(&policy_value1, 1, extension2_->id());
+  InsertPrefValue(&policy_value1, 2, extension1_->id());
+  profile()->GetTestingPrefService()->SetUserPref(prefs::kPinnedLauncherApps,
+                                                  policy_value1.DeepCopy());
+  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 |LauncherModel|'s weight system. Since at the moment
+  // the weight of a running app has the same as a shortcut, it will remain
+  // where it is. Surely note ideal, but since the sync process can shift around
+  // locations - as well as many other edge cases - this gets nearly impossible
+  // to get right.
+  // TODO(skuhne): Filed crbug.com/293761 to track of this.
+  base::ListValue policy_value2;
+  InsertPrefValue(&policy_value2, 0, extension3_->id());
+  InsertPrefValue(&policy_value2, 1, extension1_->id());
+  profile()->GetTestingPrefService()->SetUserPref(prefs::kPinnedLauncherApps,
+                                                  policy_value2.DeepCopy());
+  EXPECT_EQ("AppList, Chrome, App3, app2, App1, ", GetPinnedAppStatus());
+
+  // Removing an item should simply close it and everything should shift.
+  base::ListValue policy_value3;
+  InsertPrefValue(&policy_value3, 0, extension3_->id());
+  profile()->GetTestingPrefService()->SetUserPref(prefs::kPinnedLauncherApps,
+                                                  policy_value3.DeepCopy());
+  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.
+TEST_F(ChromeLauncherControllerTest,
+       RestoreDefaultAndRunningV2AppsResyncOrder) {
+  InitLauncherController();
+  base::ListValue policy_value0;
+  InsertPrefValue(&policy_value0, 0, extension1_->id());
+  InsertPrefValue(&policy_value0, 1, extension3_->id());
+  profile()->GetTestingPrefService()->SetUserPref(prefs::kPinnedLauncherApps,
+                                                  policy_value0.DeepCopy());
+  // The shelf layout has always one static item at the beginning (app List).
+  SetShelfChromeIconIndex(0);
+  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 an unpinned but running V2 app.
+  CreateRunningV2App(extension2_->id());
+  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.
+  base::ListValue policy_value1;
+  InsertPrefValue(&policy_value1, 0, extension3_->id());
+  InsertPrefValue(&policy_value1, 1, extension2_->id());
+  InsertPrefValue(&policy_value1, 2, extension1_->id());
+  profile()->GetTestingPrefService()->SetUserPref(prefs::kPinnedLauncherApps,
+                                                  policy_value1.DeepCopy());
+  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 running V2 app. Since the position is determined
+  // by the |LauncherModel|'s weight system, it will be after last pinned item.
+  base::ListValue policy_value2;
+  InsertPrefValue(&policy_value2, 0, extension3_->id());
+  InsertPrefValue(&policy_value2, 1, extension1_->id());
+  profile()->GetTestingPrefService()->SetUserPref(prefs::kPinnedLauncherApps,
+                                                  policy_value2.DeepCopy());
+  EXPECT_EQ("AppList, Chrome, App3, App1, *app2, ", GetPinnedAppStatus());
+
+  // Removing an item should simply close it and everything should shift.
+  base::ListValue policy_value3;
+  InsertPrefValue(&policy_value3, 0, extension3_->id());
+  profile()->GetTestingPrefService()->SetUserPref(prefs::kPinnedLauncherApps,
+                                                  policy_value3.DeepCopy());
+  EXPECT_EQ("AppList, Chrome, App3, *app2, ", GetPinnedAppStatus());
+}
+
 TEST_F(ChromeLauncherControllerTest, Policy) {
   extension_service_->AddExtension(extension1_.get());
   extension_service_->AddExtension(extension3_.get());
@@ -1517,6 +1719,7 @@ TEST_F(ChromeLauncherControllerTest, PersistPinned) {
 
   launcher_controller_.reset();
   model_.reset(new ash::LauncherModel);
+  AddAppListLauncherItem();
   launcher_controller_.reset(
       ChromeLauncherController::CreateInstance(profile(), model_.get()));
   app_tab_helper = new TestAppTabHelperImpl;
@@ -1524,7 +1727,6 @@ TEST_F(ChromeLauncherControllerTest, PersistPinned) {
   SetAppTabHelper(app_tab_helper);
   app_icon_loader = new TestAppIconLoaderImpl;
   SetAppIconLoader(app_icon_loader);
-  AddAppListLauncherItem();
   launcher_controller_->Init();
 
   EXPECT_EQ(1, app_icon_loader->fetch_count());