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 338 matching lines @@
 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));
 
-  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));
 
-  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()));
 }
 
 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));
+
+  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));
+
+  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) {
+  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 new_exe = temp_dir_.path().Append(L"manganese.exe");
+  ShellUtil::ShortcutProperties updated_properties(ShellUtil::CURRENT_USER);
+  updated_properties.set_target(new_exe);
+
+  ASSERT_TRUE(ShellUtil::UpdateShortcuts(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::CURRENT_USER,
+      chrome_exe_, updated_properties));
+
+  // TODO(huangs): Fix ValidateChromeShortcut() and use it.
+  base::FilePath target_path;
+  ASSERT_TRUE(base::win::ResolveShortcut(shortcut_path, &target_path, NULL));
+  ASSERT_EQ(new_exe, target_path);
+}
+
+TEST_F(ShellUtilShortcutTest, UpdateSystemLevelChromeShortcut) {
+  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));
+
+  base::FilePath new_exe = temp_dir_.path().Append(L"manganese.exe");
+  ShellUtil::ShortcutProperties updated_properties(ShellUtil::CURRENT_USER);
+  updated_properties.set_target(new_exe);
+
+  ASSERT_TRUE(ShellUtil::UpdateShortcuts(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::SYSTEM_LEVEL,
+      chrome_exe_, updated_properties));
+
+  // TODO(huangs): Fix ValidateChromeShortcut() and use it.

[gab, 2013/05/13 04:12:15]

And the fact that we checked it in with SHORTCUT_REPLACE_EXISTING instead of
SHORTCUT_UPDATE_EXISTING just proves my point here that this test should really
test that the update only affected the desired properties and that all other
properties remained the same (as all other tests in this file do) -- I was
originally satisfied with this for now because it at least tested one unmodified
property (arguments), but I forgot arguments are special cased and ported over
when overwriting shortcuts to avoid overwriting custom arguments some people add
on their shortcuts....

Please fix this as well, it would have catched this issue prior to checking it
in :(.

+  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);
   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));
 
+  string16 shortcut2_args = L"--profile-directory=\"Profile 2\"";
   test_properties_->set_shortcut_name(kShortcutName2);
-  test_properties_->set_arguments(L"--profile-directory=\"Profile 2\"");
+  test_properties_->set_arguments(shortcut2_args);
   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));
 
-  ASSERT_TRUE(ShellUtil::RemoveShortcut(
-      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, chrome_exe_,
-      ShellUtil::CURRENT_USER, NULL));
-  ASSERT_FALSE(file_util::PathExists(shortcut1_path));
-  ASSERT_FALSE(file_util::PathExists(shortcut2_path));
-  ASSERT_TRUE(file_util::PathExists(shortcut1_path.DirName()));
+  base::FilePath new_exe = temp_dir_.path().Append(L"manganese.exe");
+  ShellUtil::ShortcutProperties updated_properties(ShellUtil::CURRENT_USER);
+  updated_properties.set_target(new_exe);
+
+  ASSERT_TRUE(ShellUtil::UpdateShortcuts(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::CURRENT_USER,
+      chrome_exe_, updated_properties));
+
+  // TODO(huangs): Fix ValidateChromeShortcut() and use it.
+  base::FilePath target_path1;
+  ASSERT_TRUE(base::win::ResolveShortcut(shortcut1_path, &target_path1, NULL));
+  ASSERT_EQ(new_exe, target_path1);
+
+  base::FilePath target_path2;
+  string16 shortcut2_args_read;
+  ASSERT_TRUE(base::win::ResolveShortcut(shortcut2_path, &target_path2,
+                                         &shortcut2_args_read));
+  ASSERT_EQ(new_exe, target_path2);
+  ASSERT_EQ(shortcut2_args, shortcut2_args_read);
 }
 
 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));
   }
 }