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

chrome/browser/ui/ash/launcher/chrome_launcher_controller_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.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
@@ skipping 159 matching lines @@
   }
 
  private:
   typedef std::map<content::WebContents*, std::string> TabToStringMap;
 
   TabToStringMap tab_id_map_;
 
   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 {
  protected:
   ChromeLauncherControllerTest() : extension_service_(NULL) {
     SetHostDesktopType(chrome::HOST_DESKTOP_TYPE_ASH);
   }
 
   virtual ~ChromeLauncherControllerTest() {
   }
@@ skipping 49 matching lines @@
                                     &error);
 
     // Fake search extension.
     extension4_ = Extension::Create(base::FilePath(), Manifest::UNPACKED,
                                     manifest,
                                     Extension::NO_FLAGS,
                                     extension_misc::kGoogleSearchAppId,
                                     &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();
     launcher_controller_.reset();
     model_.reset();
 
     BrowserWithTestWindowTest::TearDown();
   }
 
   void AddAppListLauncherItem() {
@@ skipping 39 matching lines @@
          iter != model_->items().end(); ++iter) {
       ChromeLauncherController::IDToItemControllerMap::const_iterator
           entry(controller->id_to_item_controller_map_.find(iter->id));
       if (iter->type == ash::TYPE_APP_SHORTCUT &&
           entry != controller->id_to_item_controller_map_.end()) {
         launchers->push_back(entry->second->app_id());
       }
     }
   }
 
+  // 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(
                     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()));
@@ skipping 18 matching lines @@
           result += "Unknown";
           break;
       }
     }
     return result;
   }
 
   // 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.
   scoped_refptr<Extension> extension1_;
   scoped_refptr<Extension> extension2_;
   scoped_refptr<Extension> extension3_;
   scoped_refptr<Extension> extension4_;
   scoped_ptr<ChromeLauncherController> launcher_controller_;
   scoped_ptr<TestLauncherModelObserver> model_observer_;
   scoped_ptr<ash::LauncherModel> model_;
@@ skipping 419 matching lines @@
   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(extension2_->id()));
 }
 
 // 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());
   EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));
   EXPECT_FALSE(
       launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
 
   for (int i = 0; i < 2; i++) {
     launcher_controller_->LockV1AppWithID(extension1_->id());
 
@@ skipping 129 matching lines @@
   EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[2].type);
   EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
   EXPECT_FALSE(
       launcher_controller_->IsWindowedAppInLauncher(extension1_->id()));
 
   launcher_controller_->UnpinAppWithID(extension1_->id());
 
   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());
 
   base::ListValue policy_value;
   InsertPrefValue(&policy_value, 0, extension1_->id());
   InsertPrefValue(&policy_value, 1, extension2_->id());
   profile()->GetTestingPrefService()->SetManagedPref(prefs::kPinnedLauncherApps,
                                                      policy_value.DeepCopy());
 
@@ skipping 560 matching lines @@
   ash::LauncherID id = launcher_controller_->GetLauncherIDForAppID("1");
   int app_index = model_->ItemIndexByID(id);
   EXPECT_EQ(1, app_icon_loader->fetch_count());
   EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[app_index].type);
   EXPECT_TRUE(launcher_controller_->IsAppPinned("1"));
   EXPECT_FALSE(launcher_controller_->IsAppPinned("0"));
   EXPECT_EQ(initial_size + 1, model_->items().size());
 
   launcher_controller_.reset();
   model_.reset(new ash::LauncherModel);
+  AddAppListLauncherItem();
   launcher_controller_.reset(
       ChromeLauncherController::CreateInstance(profile(), model_.get()));
   app_tab_helper = new TestAppTabHelperImpl;
   app_tab_helper->SetAppID(tab_strip_model->GetWebContentsAt(0), "1");
   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());
   ASSERT_EQ(initial_size + 1, model_->items().size());
   EXPECT_TRUE(launcher_controller_->IsAppPinned("1"));
   EXPECT_FALSE(launcher_controller_->IsAppPinned("0"));
   EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[app_index].type);
 
   launcher_controller_->UnpinAppWithID("1");
   ASSERT_EQ(initial_size, model_->items().size());
 }