Find shortcuts that don't have a profile command line set when creating 2nd profile.

chrome/browser/profiles/profile_shortcut_manager_unittest_win.cc

Index: chrome/browser/profiles/profile_shortcut_manager_unittest_win.cc
===================================================================
--- chrome/browser/profiles/profile_shortcut_manager_unittest_win.cc	(revision 169477)
+++ chrome/browser/profiles/profile_shortcut_manager_unittest_win.cc	(working copy)
@@ -5,6 +5,7 @@
 #include <objbase.h>  // For CoInitialize().
 
 #include "base/file_util.h"
+#include "base/location.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/test/scoped_path_override.h"
@@ -16,6 +17,7 @@
 #include "chrome/browser/profiles/profile_shortcut_manager.h"
 #include "chrome/browser/profiles/profile_shortcut_manager_win.h"
 #include "chrome/installer/util/browser_distribution.h"
+#include "chrome/installer/util/product.h"
 #include "chrome/installer/util/shell_util.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
@@ -36,6 +38,13 @@
         fake_user_desktop_(base::DIR_USER_DESKTOP) {
   }
 
+  FilePath CreateProfileDirectory(const string16& profile_name) {
+    const FilePath profile_path =
+        profile_info_cache_->GetUserDataDir().Append(profile_name);
+    file_util::CreateDirectoryW(profile_path);
+    return profile_path;
+  }
+
   virtual void SetUp() OVERRIDE {
     CoInitialize(NULL);
     distribution_ = BrowserDistribution::GetDistribution();
@@ -50,21 +59,15 @@
         ProfileShortcutManager::Create(profile_manager_->profile_manager()));
 
     ASSERT_TRUE(PathService::Get(base::FILE_EXE, &exe_path_));
-    dest_path_ = profile_info_cache_->GetUserDataDir();
-    dest_path_ = dest_path_.Append(FILE_PATH_LITERAL("My profile"));
-    file_util::CreateDirectoryW(dest_path_);
-    profile_name_ = L"My profile";
-
     ASSERT_TRUE(ShellUtil::GetShortcutPath(ShellUtil::SHORTCUT_LOCATION_DESKTOP,
                                            distribution_,
                                            ShellUtil::CURRENT_USER,
                                            &shortcuts_directory_));
 
-    second_dest_path_ = profile_info_cache_->GetUserDataDir();
-    second_dest_path_ =
-        second_dest_path_.Append(FILE_PATH_LITERAL("My profile 2"));
-    file_util::CreateDirectoryW(second_dest_path_);
-    second_profile_name_ = L"My profile 2";
+    profile_1_name_ = L"My profile";
+    profile_1_path_ = CreateProfileDirectory(profile_1_name_);
+    profile_2_name_ = L"My profile 2";
+    profile_2_path_ = CreateProfileDirectory(profile_2_name_);
   }
 
   virtual void TearDown() OVERRIDE {
@@ -92,31 +95,28 @@
     MessageLoop::current()->Run();
   }
 
-  void SetupDefaultProfileShortcut() {
-    ASSERT_FALSE(ProfileShortcutExistsAtDefaultPath(profile_name_));
+  void SetupDefaultProfileShortcut(const tracked_objects::Location& location) {
+    ASSERT_FALSE(ProfileShortcutExistsAtDefaultPath(profile_1_name_));

[sail, 2012/11/28 05:12:31]

did you want to add a "<< location.ToString()" here?

[Alexei Svitkine (slow), 2012/11/28 21:27:12]

Done.

     // A non-badged shortcut for chrome is automatically created with the
     // first profile (for the case when the user deletes their only profile).
-    profile_info_cache_->AddProfileToCache(dest_path_, profile_name_,
+    profile_info_cache_->AddProfileToCache(profile_1_path_, profile_1_name_,
                                            string16(), 0);
     RunPendingTasks();
     // We now have 1 profile, so we expect a new shortcut with no profile
     // information.
-    ValidateProfileShortcut(string16());
+    ValidateProfileShortcut(location, string16());
   }
 
-  void SetupAndCreateTwoShortcuts() {
-    ASSERT_EQ(0, profile_info_cache_->GetNumberOfProfiles());
-    ASSERT_FALSE(ProfileShortcutExistsAtDefaultPath(profile_name_));
-    ASSERT_FALSE(ProfileShortcutExistsAtDefaultPath(second_profile_name_));
+  void SetupAndCreateTwoShortcuts(const tracked_objects::Location& location) {
+    ASSERT_EQ(0, profile_info_cache_->GetNumberOfProfiles())
+        << location.ToString();
+    ASSERT_FALSE(ProfileShortcutExistsAtDefaultPath(profile_1_name_))
+        << location.ToString();
 
-    profile_info_cache_->AddProfileToCache(dest_path_, profile_name_,
+    profile_info_cache_->AddProfileToCache(profile_1_path_, profile_1_name_,
                                            string16(), 0);
-    profile_info_cache_->AddProfileToCache(second_dest_path_,
-                                           second_profile_name_, string16(), 0);
-    profile_shortcut_manager_->CreateProfileShortcut(second_dest_path_);
-    RunPendingTasks();
-    ValidateProfileShortcut(profile_name_);
-    ValidateProfileShortcut(second_profile_name_);
+    CreateProfileWithShortcut(FROM_HERE, profile_2_name_, profile_2_path_);

[sail, 2012/11/28 05:12:31]

why pass FROM_HERE instead of location?

[Alexei Svitkine (slow), 2012/11/28 21:27:12]

Done.

+    ValidateProfileShortcut(location, profile_1_name_);
   }
 
   // Returns the default shortcut path for this profile.
@@ -134,10 +134,10 @@
 
   // Calls base::win::ValidateShortcut() with expected properties for
   // |profile_name|'s shortcut.
-  void ValidateProfileShortcut(const string16& profile_name) {
-    EXPECT_TRUE(ProfileShortcutExistsAtDefaultPath(profile_name));
-
+  void ValidateProfileShortcut(const tracked_objects::Location& location,
+                               const string16& profile_name) {
     FilePath shortcut_path = GetDefaultShortcutPathForProfile(profile_name);
+    EXPECT_TRUE(file_util::PathExists(shortcut_path)) << location.ToString();
 
     // TODO(asvitkine): With this new struct method for VerifyShortcut you can
     // now test more properties like: arguments, icon, icon_index, and app_id.
@@ -148,6 +148,39 @@
     base::win::ValidateShortcut(shortcut_path, expected_properties);
   }
 
+  void CreateProfileWithShortcut(const tracked_objects::Location& location,
+                                 const string16& profile_name,
+                                 const FilePath& profile_path) {
+    ASSERT_FALSE(ProfileShortcutExistsAtDefaultPath(profile_name))
+        << location.ToString();
+    profile_info_cache_->AddProfileToCache(profile_path, profile_name,
+                                           string16(), 0);
+    profile_shortcut_manager_->CreateProfileShortcut(profile_path);
+    RunPendingTasks();
+    ValidateProfileShortcut(location, profile_name);
+  }
+
+  // Creates a regular (non-profile) desktop shortcut with the given name and
+  // returns its path. Fails the test if an error occurs.
+  FilePath CreateRegularShortcutWithName(
+      const tracked_objects::Location& location,
+      const string16& shortcut_name) {
+    const FilePath shortcut_path =
+        shortcuts_directory_.Append(shortcut_name + installer::kLnkExt);
+    EXPECT_FALSE(file_util::PathExists(shortcut_path)) << location.ToString();
+
+    installer::Product product(distribution_);
+    ShellUtil::ShortcutProperties properties(ShellUtil::CURRENT_USER);
+    product.AddDefaultShortcutProperties(exe_path_, &properties);
+    properties.set_shortcut_name(shortcut_name);
+    EXPECT_TRUE(ShellUtil::CreateOrUpdateShortcut(
+        ShellUtil::SHORTCUT_LOCATION_DESKTOP, distribution_, properties,
+        ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS)) << location.ToString();
+    EXPECT_TRUE(file_util::PathExists(shortcut_path)) << location.ToString();
+
+    return shortcut_path;
+  }
+
   BrowserDistribution* distribution_;
   MessageLoopForUI message_loop_;
   content::TestBrowserThread ui_thread_;
@@ -157,11 +190,11 @@
   ProfileInfoCache* profile_info_cache_;
   base::ScopedPathOverride fake_user_desktop_;
   FilePath exe_path_;
-  FilePath dest_path_;
   FilePath shortcuts_directory_;
-  string16 profile_name_;
-  FilePath second_dest_path_;
-  string16 second_profile_name_;
+  string16 profile_1_name_;
+  FilePath profile_1_path_;
+  string16 profile_2_name_;
+  FilePath profile_2_path_;
 };
 
 TEST_F(ProfileShortcutManagerTest, ShortcutFilename) {
@@ -180,111 +213,94 @@
 }
 
 TEST_F(ProfileShortcutManagerTest, DesktopShortcutsCreate) {
-  ProfileShortcutManagerTest::SetupDefaultProfileShortcut();
-
-  profile_info_cache_->AddProfileToCache(second_dest_path_,
-                                         second_profile_name_, string16(), 0);
-  profile_shortcut_manager_->CreateProfileShortcut(second_dest_path_);
-  RunPendingTasks();
-
+  SetupDefaultProfileShortcut(FROM_HERE);
+  CreateProfileWithShortcut(FROM_HERE, profile_2_name_, profile_2_path_);
   // We now have 2 profiles, so we expect a new shortcut with profile
   // information for this 2nd profile.
-  ValidateProfileShortcut(second_profile_name_);
-  ASSERT_TRUE(file_util::PathExists(second_dest_path_.Append(
+  ASSERT_TRUE(file_util::PathExists(profile_2_path_.Append(
       FILE_PATH_LITERAL("Google Profile.ico"))));

[sail, 2012/11/28 05:12:31]

should we just get rid of all these FILE_PATH_LITERALs?

[Alexei Svitkine (slow), 2012/11/28 21:27:12]

Done. I also made all of these use the constant from
profile_shortcut_manager_win.cc instead of duplicating it.

 }
 
 TEST_F(ProfileShortcutManagerTest, DesktopShortcutsUpdate) {
-  ProfileShortcutManagerTest::SetupDefaultProfileShortcut();
+  SetupDefaultProfileShortcut(FROM_HERE);
+  CreateProfileWithShortcut(FROM_HERE, profile_2_name_, profile_2_path_);
 
-  EXPECT_FALSE(ProfileShortcutExistsAtDefaultPath(second_profile_name_));
-
-  profile_info_cache_->AddProfileToCache(second_dest_path_,
-                                         second_profile_name_, string16(), 0);
-  profile_shortcut_manager_->CreateProfileShortcut(second_dest_path_);
-  RunPendingTasks();
-  ValidateProfileShortcut(second_profile_name_);
-
   // Cause an update in ProfileShortcutManager by modifying the profile info
   // cache.
   const string16 new_profile_name = L"New Profile Name";
   profile_info_cache_->SetNameOfProfileAtIndex(
-      profile_info_cache_->GetIndexOfProfileWithPath(second_dest_path_),
+      profile_info_cache_->GetIndexOfProfileWithPath(profile_2_path_),
       new_profile_name);
   RunPendingTasks();
-  EXPECT_FALSE(ProfileShortcutExistsAtDefaultPath(second_profile_name_));
-  ValidateProfileShortcut(new_profile_name);
+  EXPECT_FALSE(ProfileShortcutExistsAtDefaultPath(profile_2_name_));
+  ValidateProfileShortcut(FROM_HERE, new_profile_name);
 }
 
 TEST_F(ProfileShortcutManagerTest, DesktopShortcutsDeleteSecondToLast) {
-  ProfileShortcutManagerTest::SetupAndCreateTwoShortcuts();
+  SetupAndCreateTwoShortcuts(FROM_HERE);
 
   // Delete one shortcut.
-  profile_info_cache_->DeleteProfileFromCache(second_dest_path_);
+  profile_info_cache_->DeleteProfileFromCache(profile_2_path_);
   RunPendingTasks();
-  EXPECT_FALSE(ProfileShortcutExistsAtDefaultPath(second_profile_name_));
+  EXPECT_FALSE(ProfileShortcutExistsAtDefaultPath(profile_2_name_));
 
   // Verify that the profile name has been removed from the remaining shortcut.
-  ValidateProfileShortcut(string16());
+  ValidateProfileShortcut(FROM_HERE, string16());
   // Verify that an additional shortcut, with the default profile's name does
   // not exist.
-  EXPECT_FALSE(ProfileShortcutExistsAtDefaultPath(profile_name_));
+  EXPECT_FALSE(ProfileShortcutExistsAtDefaultPath(profile_1_name_));
 }
 
 TEST_F(ProfileShortcutManagerTest, DesktopShortcutsCreateSecond) {
-  ProfileShortcutManagerTest::SetupAndCreateTwoShortcuts();
+  SetupAndCreateTwoShortcuts(FROM_HERE);
 
   // Delete one shortcut.
-  profile_info_cache_->DeleteProfileFromCache(second_dest_path_);
+  profile_info_cache_->DeleteProfileFromCache(profile_2_path_);
   RunPendingTasks();
 
   // Verify that a default shortcut exists (no profile name/avatar).
-  ValidateProfileShortcut(string16());
-  // Verify that an additional shortcut, with the default profile's name does
+  ValidateProfileShortcut(FROM_HERE, string16());
+  // Verify that an additional shortcut, with the first profile's name does
   // not exist.
-  EXPECT_FALSE(ProfileShortcutExistsAtDefaultPath(profile_name_));
+  EXPECT_FALSE(ProfileShortcutExistsAtDefaultPath(profile_1_name_));
 
   // Create a second profile and shortcut.
-  profile_info_cache_->AddProfileToCache(second_dest_path_,
-                                         second_profile_name_, string16(), 0);
-  profile_shortcut_manager_->CreateProfileShortcut(second_dest_path_);
-  RunPendingTasks();
-  ValidateProfileShortcut(second_profile_name_);
+  CreateProfileWithShortcut(FROM_HERE, profile_2_name_, profile_2_path_);
 
   // Verify that the original shortcut received the profile's name.
-  ValidateProfileShortcut(profile_name_);
+  ValidateProfileShortcut(FROM_HERE, profile_1_name_);
   // Verify that a default shortcut no longer exists.
   EXPECT_FALSE(ProfileShortcutExistsAtDefaultPath(string16()));
 }
 
 TEST_F(ProfileShortcutManagerTest, RenamedDesktopShortcuts) {
-  ProfileShortcutManagerTest::SetupAndCreateTwoShortcuts();
+  SetupAndCreateTwoShortcuts(FROM_HERE);
 
   const FilePath old_shortcut_path =
-      GetDefaultShortcutPathForProfile(second_profile_name_);
+      GetDefaultShortcutPathForProfile(profile_2_name_);
   const FilePath new_shortcut_path =
       shortcuts_directory_.Append(L"MyChrome.lnk");
   ASSERT_TRUE(file_util::Move(old_shortcut_path, new_shortcut_path));
 
   // Ensure that a new shortcut does not get made if the old one was renamed.
-  profile_shortcut_manager_->CreateProfileShortcut(second_dest_path_);
+  profile_shortcut_manager_->CreateProfileShortcut(profile_2_path_);
   RunPendingTasks();
-  EXPECT_FALSE(ProfileShortcutExistsAtDefaultPath(second_profile_name_));
+  EXPECT_FALSE(ProfileShortcutExistsAtDefaultPath(profile_2_name_));
   EXPECT_TRUE(file_util::PathExists(new_shortcut_path));
 
   // Delete the renamed shortcut and try to create it again, which should work.
   ASSERT_TRUE(file_util::Delete(new_shortcut_path, false));
   EXPECT_FALSE(file_util::PathExists(new_shortcut_path));
-  profile_shortcut_manager_->CreateProfileShortcut(second_dest_path_);
+  profile_shortcut_manager_->CreateProfileShortcut(profile_2_path_);
   RunPendingTasks();
-  EXPECT_TRUE(ProfileShortcutExistsAtDefaultPath(second_profile_name_));
+  EXPECT_TRUE(ProfileShortcutExistsAtDefaultPath(profile_2_name_));
 }
 
 TEST_F(ProfileShortcutManagerTest, RenamedDesktopShortcutsGetDeleted) {
-  ProfileShortcutManagerTest::SetupAndCreateTwoShortcuts();
+  SetupAndCreateTwoShortcuts(FROM_HERE);
 
   const FilePath old_shortcut_path =
-      GetDefaultShortcutPathForProfile(second_profile_name_);
+      GetDefaultShortcutPathForProfile(profile_2_name_);
   const FilePath new_shortcut_path =
       shortcuts_directory_.Append(L"MyChrome.lnk");
   // Make a copy of the shortcut.
@@ -296,12 +312,12 @@
   const FilePath preserved_shortcut_path =
       shortcuts_directory_.Append(L"Preserved.lnk");
   ASSERT_TRUE(file_util::CopyFile(
-      GetDefaultShortcutPathForProfile(profile_name_),
+      GetDefaultShortcutPathForProfile(profile_1_name_),
       preserved_shortcut_path));
   EXPECT_TRUE(file_util::PathExists(preserved_shortcut_path));
 
   // Delete the profile and ensure both shortcuts were also deleted.
-  profile_info_cache_->DeleteProfileFromCache(second_dest_path_);
+  profile_info_cache_->DeleteProfileFromCache(profile_2_path_);
   RunPendingTasks();
   EXPECT_FALSE(file_util::PathExists(old_shortcut_path));
   EXPECT_FALSE(file_util::PathExists(new_shortcut_path));
@@ -309,10 +325,10 @@
 }
 
 TEST_F(ProfileShortcutManagerTest, RenamedDesktopShortcutsAfterProfileRename) {
-  ProfileShortcutManagerTest::SetupAndCreateTwoShortcuts();
+  SetupAndCreateTwoShortcuts(FROM_HERE);
 
   const FilePath old_shortcut_path =
-      GetDefaultShortcutPathForProfile(second_profile_name_);
+      GetDefaultShortcutPathForProfile(profile_2_name_);
   const FilePath new_shortcut_path =
       shortcuts_directory_.Append(L"MyChrome.lnk");
 
@@ -323,9 +339,9 @@
 
   // Now, rename the profile.
   const string16 new_profile_name = L"New profile";
-  ASSERT_NE(second_profile_name_, new_profile_name);
+  ASSERT_NE(profile_2_name_, new_profile_name);
   profile_info_cache_->SetNameOfProfileAtIndex(
-      profile_info_cache_->GetIndexOfProfileWithPath(second_dest_path_),
+      profile_info_cache_->GetIndexOfProfileWithPath(profile_2_path_),
       new_profile_name);
   RunPendingTasks();
 
@@ -338,3 +354,68 @@
   EXPECT_TRUE(file_util::PathExists(updated_shortcut_path));
 }
 
+TEST_F(ProfileShortcutManagerTest, UpdateShortcutWithNoFlags) {
+  SetupDefaultProfileShortcut(FROM_HERE);
+
+  // Delete the shortcut that got created for this profile and instead make
+  // a new one without any command-line flags.
+  ASSERT_TRUE(file_util::Delete(GetDefaultShortcutPathForProfile(string16()),
+                                false));
+  const FilePath regular_shortcut_path =
+      CreateRegularShortcutWithName(FROM_HERE,
+                                    distribution_->GetAppShortCutName());
+
+  // Add another profile and check that the shortcut was renamed.
+  CreateProfileWithShortcut(FROM_HERE, profile_2_name_, profile_2_path_);
+
+  EXPECT_FALSE(file_util::PathExists(regular_shortcut_path));
+  const FilePath new_shortcut_path =

[sail, 2012/11/28 05:12:31]

maybe new_profile_1_shortcut_path

[Alexei Svitkine (slow), 2012/11/28 21:27:12]

I've made ValidateProfileShortcut() check the flags and simplified this part of
the test to use, instead.

+      GetDefaultShortcutPathForProfile(profile_1_name_);
+  EXPECT_TRUE(file_util::PathExists(new_shortcut_path));
+
+  // Check that the renamed shortcut has the correct command line flags set.
+  string16 shortcut_args;

[sail, 2012/11/28 05:12:31]

maybe profile_1_shortcut_args?

[Alexei Svitkine (slow), 2012/11/28 21:27:12]

This is now done in ValidateProfileShortcut().

+  ASSERT_TRUE(base::win::ResolveShortcut(new_shortcut_path, NULL,
+                                         &shortcut_args));
+  const string16 expected_args =
+      profiles::internal::CreateProfileShortcutFlags(profile_1_path_);
+  EXPECT_NE(string16::npos, shortcut_args.find(expected_args));
+}
+
+TEST_F(ProfileShortcutManagerTest, UpdateTwoShortcutsWithNoFlags) {
+  SetupDefaultProfileShortcut(FROM_HERE);
+
+  // Delete the shortcut that got created for this profile and instead make
+  // two new ones without any command-line flags.
+  ASSERT_TRUE(file_util::Delete(GetDefaultShortcutPathForProfile(string16()),
+                                false));
+  const FilePath regular_shortcut_path =
+      CreateRegularShortcutWithName(FROM_HERE,
+                                    distribution_->GetAppShortCutName());
+  const FilePath second_shortcut_path =

[sail, 2012/11/28 05:12:31]

I find spelling out second harder to parse. How about regular_shortcut_path_1
and regular_shortcut_path_2

Oh wait. So the first one is a standard chrome shortcut and the second one is a
custom made chrome shortcut?

If so maybe name it as regular_shortcut_path, customized_regular_shortcut_path.

[Alexei Svitkine (slow), 2012/11/28 21:27:12]

Done.

+      CreateRegularShortcutWithName(FROM_HERE, L"MyChrome");
+  EXPECT_TRUE(file_util::PathExists(second_shortcut_path));

[sail, 2012/11/28 05:12:31]

don't need this

[Alexei Svitkine (slow), 2012/11/28 21:27:12]

Done.

+
+  // Add another profile and check that one shortcut was renamed and that the
+  // other shortcut was updated but kept the same name.
+  CreateProfileWithShortcut(FROM_HERE, profile_2_name_, profile_2_path_);
+  EXPECT_FALSE(file_util::PathExists(regular_shortcut_path));
+  EXPECT_TRUE(file_util::PathExists(second_shortcut_path));
+
+  const FilePath new_shortcut_path =

[sail, 2012/11/28 05:12:31]

same as above, maybe "new_profile_1_shortcut_path"

[Alexei Svitkine (slow), 2012/11/28 21:27:12]

Done.

+      GetDefaultShortcutPathForProfile(profile_1_name_);
+  EXPECT_TRUE(file_util::PathExists(new_shortcut_path));
+
+  // Check that both shortcuts have the correct command line flags set.
+  const string16 expected_args =
+      profiles::internal::CreateProfileShortcutFlags(profile_1_path_);
+  string16 shortcut_args;
+  ASSERT_TRUE(base::win::ResolveShortcut(new_shortcut_path, NULL,
+                                         &shortcut_args));
+  EXPECT_NE(string16::npos, shortcut_args.find(expected_args));
+
+  shortcut_args.clear();
+  ASSERT_TRUE(base::win::ResolveShortcut(second_shortcut_path, NULL,
+                                         &shortcut_args));
+  EXPECT_NE(string16::npos, shortcut_args.find(expected_args));
+}