Refactoring installer shortcut deletion; adding dedicated shortcut update feature.

chrome/installer/util/shell_util_unittest.cc

Index: chrome/installer/util/shell_util_unittest.cc
diff --git a/chrome/installer/util/shell_util_unittest.cc b/chrome/installer/util/shell_util_unittest.cc
index 25dff4c1f3129af5df37d7b57a35d2034b1823dc..bb0ac1ef16f066f531ed209c915fda872f18c34e 100644
--- a/chrome/installer/util/shell_util_unittest.cc
+++ b/chrome/installer/util/shell_util_unittest.cc
@@ -77,6 +77,23 @@ class ShellUtilShortcutTest : public testing::Test {
     test_properties_->set_dual_mode(true);
   }
 
+  // Creates a shortcut in |temp_dir| using |test_properties_|. The shortcut

[gab, 2013/05/02 21:23:59]

This is not actually true, your method always creates it on the desktop,
temp_dir_ is only used for the verification.

[huangs, 2013/05/02 21:59:35]

Moot; undoing change.

+  // name is specified by |name|, or obtained from |dist_| if |name| is empty.
+  base::FilePath CreateTestShortcutHelper(

[gab, 2013/05/02 21:23:59]

I feel this method makes the tests slightly less readable and doesn't really
save that much code duplication, please remove it.

I realize that doing more than this would be more involved and I'm ok with
keeping the task of deduping some of this test code for another day.

[huangs, 2013/05/02 21:59:35]

Done.

+      const base::ScopedTempDir& temp_dir,
+      const string16& name) {
+    bool res = ShellUtil::CreateOrUpdateShortcut(

[gab, 2013/05/02 21:23:59]

s/res/result

[huangs, 2013/05/02 21:59:35]

Okay, do so in the future.

+        ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
+        *test_properties_,
+        ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS);
+    DCHECK(res);
+    string16 shortcut_name(!name.empty() ? name :
+                           dist_->GetAppShortCutName() + installer::kLnkExt);
+    base::FilePath shortcut_path(temp_dir.path().Append(shortcut_name));
+    DCHECK(file_util::PathExists(shortcut_path));
+    return shortcut_path;
+  }
+
   // Validates that the shortcut at |location| matches |properties| (and
   // implicit default properties) for |dist|.
   // Note: This method doesn't verify the |pin_to_taskbar| property as it
@@ -347,56 +364,24 @@ TEST_F(ShellUtilShortcutTest, CreateAlwaysUserWithSystemLevelPresent) {
 }
 
 TEST_F(ShellUtilShortcutTest, RemoveChromeShortcut) {
-  ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
-                  ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
-                  *test_properties_,
-                  ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
-
-  string16 shortcut_name(dist_->GetAppShortCutName() + installer::kLnkExt);
-  base::FilePath shortcut_path(fake_user_desktop_.path().Append(shortcut_name));
-  ASSERT_TRUE(file_util::PathExists(shortcut_path));
+  base::FilePath shortcut_path(
+      CreateTestShortcutHelper(fake_user_desktop_, L""));
 
-  ASSERT_TRUE(ShellUtil::RemoveShortcut(
-      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, chrome_exe_,
-      ShellUtil::CURRENT_USER, NULL));
+  ASSERT_TRUE(ShellUtil::RemoveShortcuts(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::CURRENT_USER,
+      chrome_exe_));
   ASSERT_FALSE(file_util::PathExists(shortcut_path));
   ASSERT_TRUE(file_util::PathExists(shortcut_path.DirName()));
 }
 
 TEST_F(ShellUtilShortcutTest, RemoveSystemLevelChromeShortcut) {
   test_properties_->level = ShellUtil::SYSTEM_LEVEL;
-  ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
-                  ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
-                  *test_properties_,
-                  ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
-
-  string16 shortcut_name(dist_->GetAppShortCutName() + installer::kLnkExt);
   base::FilePath shortcut_path(
-      fake_common_desktop_.path().Append(shortcut_name));
-  ASSERT_TRUE(file_util::PathExists(shortcut_path));
+      CreateTestShortcutHelper(fake_common_desktop_, L""));
 
-  ASSERT_TRUE(ShellUtil::RemoveShortcut(
-      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, chrome_exe_,
-      ShellUtil::SYSTEM_LEVEL, NULL));
-  ASSERT_FALSE(file_util::PathExists(shortcut_path));
-  ASSERT_TRUE(file_util::PathExists(shortcut_path.DirName()));
-}
-
-TEST_F(ShellUtilShortcutTest, RemoveChromeShortcutWithSpecialName) {
-  static const wchar_t kSpecialName[] = L"I'm special";
-  test_properties_->set_shortcut_name(kSpecialName);
-  ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
-                  ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
-                  *test_properties_,
-                  ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
-
-  string16 shortcut_name(string16(kSpecialName).append(installer::kLnkExt));
-  base::FilePath shortcut_path(fake_user_desktop_.path().Append(shortcut_name));
-  ASSERT_TRUE(file_util::PathExists(shortcut_path));
-
-  ASSERT_TRUE(ShellUtil::RemoveShortcut(
-      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, chrome_exe_,
-      ShellUtil::CURRENT_USER, &string16(kSpecialName)));
+  ASSERT_TRUE(ShellUtil::RemoveShortcuts(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::SYSTEM_LEVEL,
+      chrome_exe_));
   ASSERT_FALSE(file_util::PathExists(shortcut_path));
   ASSERT_TRUE(file_util::PathExists(shortcut_path.DirName()));
 }
@@ -406,35 +391,88 @@ TEST_F(ShellUtilShortcutTest, RemoveMultipleChromeShortcuts) {
   const wchar_t kShortcutName2[] = L"Chrome 2";
 
   test_properties_->set_shortcut_name(kShortcutName1);
-  ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
-                  ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
-                  *test_properties_,
-                  ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
-  string16 shortcut1_name(
-      string16(kShortcutName1).append(installer::kLnkExt));
   base::FilePath shortcut1_path(
-      fake_user_desktop_.path().Append(shortcut1_name));
-  ASSERT_TRUE(file_util::PathExists(shortcut1_path));
+      CreateTestShortcutHelper(fake_user_desktop_,
+          string16(kShortcutName1).append(installer::kLnkExt)));
 
   test_properties_->set_shortcut_name(kShortcutName2);
   test_properties_->set_arguments(L"--profile-directory=\"Profile 2\"");
-  ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
-                  ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
-                  *test_properties_,
-                  ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
-  string16 shortcut2_name(string16(kShortcutName2).append(installer::kLnkExt));
   base::FilePath shortcut2_path(
-      fake_user_desktop_.path().Append(shortcut2_name));
-  ASSERT_TRUE(file_util::PathExists(shortcut2_path));
+      CreateTestShortcutHelper(fake_user_desktop_,
+          string16(kShortcutName2).append(installer::kLnkExt)));
 
-  ASSERT_TRUE(ShellUtil::RemoveShortcut(
-      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, chrome_exe_,
-      ShellUtil::CURRENT_USER, NULL));
+  ASSERT_TRUE(ShellUtil::RemoveShortcuts(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::CURRENT_USER,
+      chrome_exe_));
   ASSERT_FALSE(file_util::PathExists(shortcut1_path));
   ASSERT_FALSE(file_util::PathExists(shortcut2_path));
   ASSERT_TRUE(file_util::PathExists(shortcut1_path.DirName()));
 }
 
+TEST_F(ShellUtilShortcutTest, UpdateChromeShortcut) {
+  base::FilePath shortcut_path(
+      CreateTestShortcutHelper(fake_user_desktop_, L""));
+
+  base::FilePath new_exe = temp_dir_.path().Append(L"manganese.exe");
+  ShellUtil::ShortcutProperties properties(ShellUtil::CURRENT_USER);
+  properties.set_target(new_exe);
+
+  ASSERT_TRUE(ShellUtil::UpdateShortcuts(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::CURRENT_USER,
+      chrome_exe_, properties));
+  base::FilePath target_path;
+  ASSERT_TRUE(base::win::ResolveShortcut(shortcut_path, &target_path, NULL));

[gab, 2013/05/02 21:23:59]

Please use ValidateChromeShortcut to validate all of this shortcut's properties
(and in other tests below -- to make sure no other properties were affected
instead of only verifying the target -- see other tests for
Create/Replace/Update that do this already in this file).

[huangs, 2013/05/02 21:59:35]

Not using ValidateChromeShortcut for now, per discussion.  But added TODO's.

+  ASSERT_EQ(new_exe, target_path);
+}
+
+TEST_F(ShellUtilShortcutTest, UpdateSystemLevelChromeShortcut) {
+  test_properties_->level = ShellUtil::SYSTEM_LEVEL;
+  base::FilePath shortcut_path(
+      CreateTestShortcutHelper(fake_common_desktop_, L""));
+
+  base::FilePath new_exe = temp_dir_.path().Append(L"manganese.exe");
+  ShellUtil::ShortcutProperties properties(ShellUtil::CURRENT_USER);
+  properties.set_target(new_exe);
+
+  ASSERT_TRUE(ShellUtil::UpdateShortcuts(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::SYSTEM_LEVEL,
+      chrome_exe_, properties));
+  base::FilePath target_path;
+  ASSERT_TRUE(base::win::ResolveShortcut(shortcut_path, &target_path, NULL));
+  ASSERT_EQ(new_exe, target_path);
+}
+
+TEST_F(ShellUtilShortcutTest, UpdateMultipleChromeShortcuts) {
+  const wchar_t kShortcutName1[] = L"Chrome 1";
+  const wchar_t kShortcutName2[] = L"Chrome 2";
+
+  test_properties_->set_shortcut_name(kShortcutName1);
+  base::FilePath shortcut1_path(
+      CreateTestShortcutHelper(fake_user_desktop_,
+          string16(kShortcutName1).append(installer::kLnkExt)));
+
+  test_properties_->set_shortcut_name(kShortcutName2);
+  test_properties_->set_arguments(L"--profile-directory=\"Profile 2\"");
+  base::FilePath shortcut2_path(
+      CreateTestShortcutHelper(fake_user_desktop_,
+          string16(kShortcutName2).append(installer::kLnkExt)));
+
+  base::FilePath new_exe = temp_dir_.path().Append(L"manganese.exe");
+  ShellUtil::ShortcutProperties properties(ShellUtil::CURRENT_USER);
+  properties.set_target(new_exe);
+
+  ASSERT_TRUE(ShellUtil::UpdateShortcuts(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::CURRENT_USER,
+      chrome_exe_, properties));
+
+  base::FilePath target_path1;
+  ASSERT_TRUE(base::win::ResolveShortcut(shortcut1_path, &target_path1, NULL));
+  ASSERT_EQ(new_exe, target_path1);
+  base::FilePath target_path2;
+  ASSERT_TRUE(base::win::ResolveShortcut(shortcut2_path, &target_path2, NULL));
+  ASSERT_EQ(new_exe, target_path2);
+}
+
 TEST_F(ShellUtilShortcutTest, CreateMultipleStartMenuShortcutsAndRemoveFolder) {
   ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
                   ShellUtil::SHORTCUT_LOCATION_START_MENU,
@@ -456,9 +494,9 @@ TEST_F(ShellUtilShortcutTest, CreateMultipleStartMenuShortcutsAndRemoveFolder) {
   EXPECT_EQ(2, count);
 
   ASSERT_TRUE(file_util::PathExists(shortcut_folder));
-  ASSERT_TRUE(ShellUtil::RemoveShortcut(
-      ShellUtil::SHORTCUT_LOCATION_START_MENU, dist_, chrome_exe_,
-      ShellUtil::CURRENT_USER, NULL));
+  ASSERT_TRUE(ShellUtil::RemoveShortcuts(
+      ShellUtil::SHORTCUT_LOCATION_START_MENU, dist_, ShellUtil::CURRENT_USER,
+      chrome_exe_));
   ASSERT_FALSE(file_util::PathExists(shortcut_folder));
 }
 
@@ -482,9 +520,9 @@ TEST_F(ShellUtilShortcutTest, DontRemoveChromeShortcutIfPointsToAnotherChrome) {
   // The shortcut shouldn't be removed as it was installed pointing to
   // |other_chrome_exe| and RemoveChromeShortcut() is being told that the
   // removed shortcut should point to |chrome_exe_|.
-  ASSERT_TRUE(ShellUtil::RemoveShortcut(
-      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, chrome_exe_,
-      ShellUtil::CURRENT_USER, NULL));
+  ASSERT_TRUE(ShellUtil::RemoveShortcuts(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::CURRENT_USER,
+      chrome_exe_));
   ASSERT_TRUE(file_util::PathExists(shortcut_path));
   ASSERT_TRUE(file_util::PathExists(shortcut_path.DirName()));
 }