Refactoring installer shortcut deletion; adding dedicated shortcut update feature.

chrome/installer/util/shell_util_unittest.cc

 // Copyright (c) 2012 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/installer/util/shell_util.h"
 
 #include <vector>
 
 #include "base/base_paths.h"
 #include "base/base_paths_win.h"
@@ skipping 59 matching lines @@
     test_properties_.reset(
         new ShellUtil::ShortcutProperties(ShellUtil::CURRENT_USER));
     test_properties_->set_target(chrome_exe_);
     test_properties_->set_arguments(L"--test --chrome");
     test_properties_->set_description(L"Makes polar bears dance.");
     test_properties_->set_icon(icon_path, 0);
     test_properties_->set_app_id(L"Polar.Bear");
     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
   // implies real (non-mocked) state which is flaky to test.
   void ValidateChromeShortcut(
       ShellUtil::ShortcutLocation location,
       BrowserDistribution* dist,
       const ShellUtil::ShortcutProperties& properties) {
     base::FilePath expected_path;
     switch (location) {
@@ skipping 250 matching lines @@
   test_properties_->level = ShellUtil::CURRENT_USER;
   ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
                   ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
                   *test_properties_,
                   ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
   ASSERT_TRUE(file_util::PathExists(
       fake_user_desktop_.path().Append(shortcut_name)));
 }
 
 TEST_F(ShellUtilShortcutTest, RemoveChromeShortcut) {
-  ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
-                  ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
-                  *test_properties_,
-                  ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
+  base::FilePath shortcut_path(
+      CreateTestShortcutHelper(fake_user_desktop_, L""));
 
-  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));
-
-  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));
+  base::FilePath shortcut_path(
+      CreateTestShortcutHelper(fake_common_desktop_, L""));
 
-  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));
-
-  ASSERT_TRUE(ShellUtil::RemoveShortcut(
-      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, chrome_exe_,
-      ShellUtil::SYSTEM_LEVEL, NULL));
+  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()));
 }
-
-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_FALSE(file_util::PathExists(shortcut_path));
-  ASSERT_TRUE(file_util::PathExists(shortcut_path.DirName()));
-}
 
 TEST_F(ShellUtilShortcutTest, RemoveMultipleChromeShortcuts) {
   const wchar_t kShortcutName1[] = L"Chrome 1";
   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,
                   dist_, *test_properties_,
                   ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
   test_properties_->set_shortcut_name(L"A second shortcut");
   ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
                   ShellUtil::SHORTCUT_LOCATION_START_MENU,
                   dist_, *test_properties_,
                   ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
 
   base::FilePath shortcut_folder(
       fake_start_menu_.path().Append(dist_->GetAppShortCutName()));
   file_util::FileEnumerator file_counter(shortcut_folder, false,
                                          file_util::FileEnumerator::FILES);
   int count = 0;
   while (!file_counter.Next().empty())
     ++count;
   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));
 }
 
 TEST_F(ShellUtilShortcutTest, DontRemoveChromeShortcutIfPointsToAnotherChrome) {
   base::ScopedTempDir other_exe_dir;
   ASSERT_TRUE(other_exe_dir.CreateUniqueTempDir());
   base::FilePath other_chrome_exe =
       other_exe_dir.path().Append(installer::kChromeExe);
   EXPECT_EQ(0, file_util::WriteFile(other_chrome_exe, "", 0));
 
   test_properties_->set_target(other_chrome_exe);
   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));
 
   // 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()));
 }
 
 TEST(ShellUtilTest, BuildAppModelIdBasic) {
   std::vector<string16> components;
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   const string16 base_app_id(dist->GetBaseAppId());
   components.push_back(base_app_id);
   ASSERT_EQ(base_app_id, ShellUtil::BuildAppModelId(components));
@@ skipping 62 matching lines @@
 
   const string16 expected[] = { L"", L"MY", L"MZXQ", L"MZXW6", L"MZXW6YQ",
                                 L"MZXW6YTB", L"MZXW6YTBOI"};
 
   // Run the tests, with one more letter in the input every pass.
   for (int i = 0; i < arraysize(expected); ++i) {
     ASSERT_EQ(expected[i],
               ShellUtil::ByteArrayToBase32(test_array, i));
   }
 }