Installer: adding ResolveShortcutProperties(); updating shortcut icons during shortcut migration.

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"
 #include "base/file_util.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/md5.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/test/scoped_path_override.h"
 #include "base/test/test_shortcut_win.h"
 #include "base/win/shortcut.h"
 #include "base/win/windows_version.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/product.h"
 #include "chrome/installer/util/util_constants.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
 const wchar_t kManganeseExe[] = L"manganese.exe";
+const wchar_t kOtherIco[] = L"other.ico";
 
 // TODO(huangs): Separate this into generic shortcut tests and Chrome-specific
 // tests. Specifically, we should not overly rely on getting shortcut properties
 // from product_->AddDefaultShortcutProperties().
 class ShellUtilShortcutTest : public testing::Test {
  protected:
   ShellUtilShortcutTest() : test_properties_(ShellUtil::CURRENT_USER) {}
 
   virtual void SetUp() OVERRIDE {
     dist_ = BrowserDistribution::GetDistribution();
     ASSERT_TRUE(dist_ != NULL);
     product_.reset(new installer::Product(dist_));
 
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
     chrome_exe_ = temp_dir_.path().Append(installer::kChromeExe);
     EXPECT_EQ(0, file_util::WriteFile(chrome_exe_, "", 0));
 
     manganese_exe_ = temp_dir_.path().Append(kManganeseExe);
     EXPECT_EQ(0, file_util::WriteFile(manganese_exe_, "", 0));
 
+    other_ico_ = temp_dir_.path().Append(kOtherIco);
+    EXPECT_EQ(0, file_util::WriteFile(other_ico_, "", 0));
+
     ASSERT_TRUE(fake_user_desktop_.CreateUniqueTempDir());
     ASSERT_TRUE(fake_common_desktop_.CreateUniqueTempDir());
     ASSERT_TRUE(fake_user_quick_launch_.CreateUniqueTempDir());
     ASSERT_TRUE(fake_default_user_quick_launch_.CreateUniqueTempDir());
     ASSERT_TRUE(fake_start_menu_.CreateUniqueTempDir());
     ASSERT_TRUE(fake_common_start_menu_.CreateUniqueTempDir());
     user_desktop_override_.reset(
         new base::ScopedPathOverride(base::DIR_USER_DESKTOP,
                                      fake_user_desktop_.path()));
     common_desktop_override_.reset(
@@ skipping 15 matching lines @@
     base::FilePath icon_path;
     base::CreateTemporaryFileInDir(temp_dir_.path(), &icon_path);
     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);
   }
 
-  // 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(
+  // Returns the expected path of a test shortcut. Returns an empty FilePath on
+  // failure.
+  base::FilePath GetExpectedShortcutPath(
       ShellUtil::ShortcutLocation location,
       BrowserDistribution* dist,
       const ShellUtil::ShortcutProperties& properties) {
     base::FilePath expected_path;
     switch (location) {
       case ShellUtil::SHORTCUT_LOCATION_DESKTOP:
         expected_path = (properties.level == ShellUtil::CURRENT_USER) ?
             fake_user_desktop_.path() : fake_common_desktop_.path();
         break;
       case ShellUtil::SHORTCUT_LOCATION_QUICK_LAUNCH:
         expected_path = (properties.level == ShellUtil::CURRENT_USER) ?
             fake_user_quick_launch_.path() :
             fake_default_user_quick_launch_.path();
         break;
       case ShellUtil::SHORTCUT_LOCATION_START_MENU_CHROME_DIR:
         expected_path = (properties.level == ShellUtil::CURRENT_USER) ?
             fake_start_menu_.path() : fake_common_start_menu_.path();
         expected_path = expected_path.Append(
             dist_->GetStartMenuShortcutSubfolder(
                 BrowserDistribution::SUBFOLDER_CHROME));
         break;
       default:
         ADD_FAILURE() << "Unknown location";
-        return;
+        return base::FilePath();
     }
 
-    string16 shortcut_name;
-    if (properties.has_shortcut_name()) {
-      shortcut_name = properties.shortcut_name;
-    } else {
-      shortcut_name =
-          dist_->GetShortcutName(BrowserDistribution::SHORTCUT_CHROME);
-    }
+    string16 shortcut_name = properties.has_shortcut_name() ?
+        properties.shortcut_name :
+            dist_->GetShortcutName(BrowserDistribution::SHORTCUT_CHROME);

[gab, 2014/01/02 21:01:46]

I typically indent the two items of a ternary operator at the same level (just
like with &&/||).

i.e., unindent 4 spaces.

[huangs, 2014/01/02 23:22:37]

Done.

     shortcut_name.append(installer::kLnkExt);
-    expected_path = expected_path.Append(shortcut_name);
+    return expected_path.Append(shortcut_name);
+  }
+
+  // Helper to create a test shortcut on the (fake) desktop, and returns the
+  // path to the new shortcut.
+  base::FilePath CreateTestDesktopShortcut(
+      const ShellUtil::ShortcutProperties& properties) {
+    ShellUtil::ShortcutLocation location = ShellUtil::SHORTCUT_LOCATION_DESKTOP;
+    EXPECT_TRUE(ShellUtil::CreateOrUpdateShortcut(
+        location, dist_, properties, ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
+    return GetExpectedShortcutPath(location, dist_, properties);
+  }
+
+  // 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(
+        GetExpectedShortcutPath(location, dist, properties));
 
     base::win::ShortcutProperties expected_properties;
     if (properties.has_target()) {
       expected_properties.set_target(properties.target);
       expected_properties.set_working_dir(properties.target.DirName());
     } else {
       expected_properties.set_target(chrome_exe_);
       expected_properties.set_working_dir(chrome_exe_.DirName());
     }
 
     if (properties.has_arguments())
       expected_properties.set_arguments(properties.arguments);
     else
       expected_properties.set_arguments(string16());
 
     if (properties.has_description())
       expected_properties.set_description(properties.description);
     else
       expected_properties.set_description(dist->GetAppDescription());
 
     if (properties.has_icon()) {
-      expected_properties.set_icon(properties.icon, 0);
+      expected_properties.set_icon(properties.icon, properties.icon_index);
     } else {
       int icon_index = dist->GetIconIndex(BrowserDistribution::SHORTCUT_CHROME);
       expected_properties.set_icon(chrome_exe_, icon_index);
     }
 
     if (properties.has_app_id()) {
       expected_properties.set_app_id(properties.app_id);
     } else {
       // Tests are always seen as user-level installs in ShellUtil.
       expected_properties.set_app_id(ShellUtil::GetBrowserModelId(dist, true));
@@ skipping 22 matching lines @@
   base::ScopedTempDir fake_common_start_menu_;
   scoped_ptr<base::ScopedPathOverride> user_desktop_override_;
   scoped_ptr<base::ScopedPathOverride> common_desktop_override_;
   scoped_ptr<base::ScopedPathOverride> user_quick_launch_override_;
   scoped_ptr<base::ScopedPathOverride> default_user_quick_launch_override_;
   scoped_ptr<base::ScopedPathOverride> start_menu_override_;
   scoped_ptr<base::ScopedPathOverride> common_start_menu_override_;
 
   base::FilePath chrome_exe_;
   base::FilePath manganese_exe_;
+  base::FilePath other_ico_;
 };
 
 }  // namespace
 
 TEST_F(ShellUtilShortcutTest, GetShortcutPath) {
   base::FilePath path;
   ShellUtil::GetShortcutPath(ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
                              ShellUtil::CURRENT_USER, &path);
   EXPECT_EQ(fake_user_desktop_.path(), path);
   ShellUtil::GetShortcutPath(ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
@@ skipping 159 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(base::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));
-
-  string16 shortcut_name(
-      dist_->GetShortcutName(BrowserDistribution::SHORTCUT_CHROME) +
-      installer::kLnkExt);
-  base::FilePath shortcut_path(fake_user_desktop_.path().Append(shortcut_name));
-  ASSERT_TRUE(base::PathExists(shortcut_path));
+  base::FilePath shortcut_path = CreateTestDesktopShortcut(test_properties_);

[gab, 2014/01/02 21:01:46]

So I really like that the refactoring is getting rid of building the
shortcut_name everywhere by extracting GetExpectedShortcutPath() out of
ValidateChromeShortcut(), but I still don't like the helper method
(CreateTestDesktopShortcut()) itself because:

1) It hides the calling convention to ShellUtil::CreateOrUpdateShortcut; I like
having it here as it is closely related to, e.g., the call to
ShellUtil::RemoveShortcuts below; and it makes the test code easy to read.

2) The helper method really just bundles two independent things: 
 - Creating the shortcut
 - Getting the expected path
I don't see why a common method is required for both.

I think we get rid of most of the boiler plate with GetExpectedShortcutPath()
and don't think a special helper to create a specific type of shortcut
(user-level desktop shortcut) is necessary.

[huangs, 2014/01/02 23:22:37]

If we get rid of the routine, we would replace each

  EXPECT_TRUE(base::PathExists(CreateTestDesktopShortcut(test_properties_)));

with (some spacing fix may be needed):

  EXPECT_TRUE(ShellUtil::CreateOrUpdateShortcut(
                  ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
                  test_properties_, ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
  EXPECT_TRUE(base::PathExists(GetExpectedShortcutPath(
                  ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
                  test_properties_)));

The case where we save the path is similar. I like the helper method because:

(1) It shortens code and increases uniformity.
(2) It removes distraction from the "main point" of tests. Note that the
shortcut creation tests were left alone; anyone who wants to understand the
nuances of shortcut creation should look at those.
(3) It's easier to modify en mass, e.g., if we change param to
CreateOrUpdateShortcut(), we have fewer places to change.

As we both feel strongly about this, let's defer the decision to grt@.  :)

+  EXPECT_TRUE(base::PathExists(shortcut_path));
 
   ASSERT_TRUE(ShellUtil::RemoveShortcuts(
       ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::CURRENT_USER,
       chrome_exe_));
   ASSERT_FALSE(base::PathExists(shortcut_path));
   ASSERT_TRUE(base::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_->GetShortcutName(BrowserDistribution::SHORTCUT_CHROME) +
-      installer::kLnkExt);
-  base::FilePath shortcut_path(
-      fake_common_desktop_.path().Append(shortcut_name));
-  ASSERT_TRUE(base::PathExists(shortcut_path));
+  base::FilePath shortcut_path = CreateTestDesktopShortcut(test_properties_);
+  EXPECT_TRUE(base::PathExists(shortcut_path));
 
   ASSERT_TRUE(ShellUtil::RemoveShortcuts(
       ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::SYSTEM_LEVEL,
       chrome_exe_));
   ASSERT_FALSE(base::PathExists(shortcut_path));
   ASSERT_TRUE(base::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(base::PathExists(shortcut1_path));
+  base::FilePath shortcut1_path = CreateTestDesktopShortcut(test_properties_);
+  EXPECT_TRUE(base::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(base::PathExists(shortcut2_path));
+  base::FilePath shortcut2_path = CreateTestDesktopShortcut(test_properties_);
+  EXPECT_TRUE(base::PathExists(shortcut2_path));
 
   ASSERT_TRUE(ShellUtil::RemoveShortcuts(
       ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::CURRENT_USER,
       chrome_exe_));
   ASSERT_FALSE(base::PathExists(shortcut1_path));
   ASSERT_FALSE(base::PathExists(shortcut2_path));
   ASSERT_TRUE(base::PathExists(shortcut1_path.DirName()));
 }
 
-TEST_F(ShellUtilShortcutTest, UpdateChromeShortcutsWithArgs) {
-  ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
-                  ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_,
-                  ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
-
-  string16 shortcut_name(
-      dist_->GetShortcutName(BrowserDistribution::SHORTCUT_CHROME) +
-      installer::kLnkExt);
-  base::FilePath shortcut_path(fake_user_desktop_.path().Append(shortcut_name));
-  ASSERT_TRUE(base::PathExists(shortcut_path));
+TEST_F(ShellUtilShortcutTest, RetargetShortcutsWithArgs) {
+  EXPECT_TRUE(base::PathExists(CreateTestDesktopShortcut(test_properties_)));
 
   base::FilePath new_exe = temp_dir_.path().Append(kManganeseExe);
-  ShellUtil::ShortcutProperties updated_properties(ShellUtil::CURRENT_USER);
-  updated_properties.set_target(new_exe);
-  // |updated_properties| has arguments.
-  ASSERT_TRUE(ShellUtil::UpdateShortcutsWithArgs(
+  // Relies on fact that |test_properties_| has non-empty arguments.
+  ASSERT_TRUE(ShellUtil::RetargetShortcutsWithArgs(
       ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::CURRENT_USER,
-      chrome_exe_, updated_properties));
+      chrome_exe_, new_exe));
 
   ShellUtil::ShortcutProperties expected_properties(test_properties_);
   expected_properties.set_target(new_exe);
   ValidateChromeShortcut(ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
                          expected_properties);
 }
 
-TEST_F(ShellUtilShortcutTest, UpdateSystemLevelChromeShortcutsWithArgs) {
+TEST_F(ShellUtilShortcutTest, RetargetSystemLevelChromeShortcutsWithArgs) {
   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_->GetShortcutName(BrowserDistribution::SHORTCUT_CHROME) +
-      installer::kLnkExt);
-  base::FilePath shortcut_path(
-      fake_common_desktop_.path().Append(shortcut_name));
-  ASSERT_TRUE(base::PathExists(shortcut_path));
+  EXPECT_TRUE(base::PathExists(CreateTestDesktopShortcut(test_properties_)));
 
   base::FilePath new_exe = temp_dir_.path().Append(kManganeseExe);
-  ShellUtil::ShortcutProperties updated_properties(ShellUtil::CURRENT_USER);
-  updated_properties.set_target(new_exe);
-  // |updated_properties| has arguments.
-  ASSERT_TRUE(ShellUtil::UpdateShortcutsWithArgs(
+  // Relies on fact that |test_properties_| has non-empty arguments.
+  ASSERT_TRUE(ShellUtil::RetargetShortcutsWithArgs(
       ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::SYSTEM_LEVEL,
-      chrome_exe_, updated_properties));
+      chrome_exe_, new_exe));
 
   ShellUtil::ShortcutProperties expected_properties(test_properties_);
   expected_properties.set_target(new_exe);
   ValidateChromeShortcut(ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
                          expected_properties);
 }
 
-TEST_F(ShellUtilShortcutTest, UpdateMultipleChromeShortcutsWithArgs) {
+TEST_F(ShellUtilShortcutTest, RetargetChromeShortcutsWithArgsEmpty) {
   const wchar_t kShortcutName1[] = L"Chrome 1";
   const wchar_t kShortcutName2[] = L"Chrome 2";
 
   // Setup shortcut 1, which has empty arguments.
   test_properties_.set_shortcut_name(kShortcutName1);
   test_properties_.set_arguments(L"");
-  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));
+  EXPECT_TRUE(base::PathExists(CreateTestDesktopShortcut(test_properties_)));
   ShellUtil::ShortcutProperties expected_properties1(test_properties_);
 
   // Setup shortcut 2, which has non-empty arguments.
-  string16 shortcut2_args = L"--profile-directory=\"Profile 2\"";
   test_properties_.set_shortcut_name(kShortcutName2);
-  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(base::PathExists(shortcut2_path));
+  test_properties_.set_arguments(L"--profile-directory=\"Profile 2\"");
+  EXPECT_TRUE(base::PathExists(CreateTestDesktopShortcut(test_properties_)));
   ShellUtil::ShortcutProperties expected_properties2(test_properties_);
 
-  // Update shortcuts: target "manganese.exe" instead of "chrome.exe".
+  // Retarget shortcuts: replace "chrome.exe" with "manganese.exe". Only
+  // shortcuts with non-empty arguments (i.e., shortcut 2) gets updated.
   base::FilePath new_exe = temp_dir_.path().Append(kManganeseExe);
-  ShellUtil::ShortcutProperties updated_properties(ShellUtil::CURRENT_USER);
-  updated_properties.set_target(new_exe);
-
-  // Only changing shrotcuts that have non-empty arguments, i.e., shortcut 2.
-  ASSERT_TRUE(ShellUtil::UpdateShortcutsWithArgs(
+  ASSERT_TRUE(ShellUtil::RetargetShortcutsWithArgs(
       ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::CURRENT_USER,
-      chrome_exe_, updated_properties));
+      chrome_exe_, new_exe));
   // Verify shortcut 1.
-  // |expected_properties1| was unchanged and still targets "chrome.exe", since
+  // |expected_properties1| is unchanged and still targets "chrome.exe", since
   // it has empty target, yet we passed |require_args| = true.
   ValidateChromeShortcut(ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
                          expected_properties1);
-  // Verify shortcut 2.
+  // Verify shortcut 2, which now targets "manganese.exe".
   expected_properties2.set_target(new_exe);
   ValidateChromeShortcut(ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
                          expected_properties2);
 }
+
+TEST_F(ShellUtilShortcutTest, RetargetChromeShortcutsWithArgsIcon) {
+  const wchar_t kShortcutName1[] = L"Chrome 1";
+  const wchar_t kShortcutName2[] = L"Chrome 2";
+
+  // Setup shortcut 1, which has icon set to "chrome.exe".
+  test_properties_.set_shortcut_name(kShortcutName1);
+  test_properties_.set_icon(chrome_exe_, 3);
+  EXPECT_TRUE(base::PathExists(CreateTestDesktopShortcut(test_properties_)));
+  ShellUtil::ShortcutProperties expected_properties1(test_properties_);
+
+  // Setup shortcut 2, which has icon set to "other.ico".
+  test_properties_.set_shortcut_name(kShortcutName2);
+  test_properties_.set_icon(other_ico_, 0);
+  EXPECT_TRUE(base::PathExists(CreateTestDesktopShortcut(test_properties_)));
+  ShellUtil::ShortcutProperties expected_properties2(test_properties_);
+
+  // Retarget shortcuts: replace "chrome.exe" with "manganese.exe".
+  // Relies on fact that |test_properties_| has non-empty arguments.
+  base::FilePath new_exe = temp_dir_.path().Append(kManganeseExe);
+  ASSERT_TRUE(ShellUtil::RetargetShortcutsWithArgs(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::CURRENT_USER,
+      chrome_exe_, new_exe));
+  // Verify shortcut 1: icon now targets "manganese.exe", with same icon index.
+  expected_properties1.set_target(new_exe);
+  expected_properties1.set_icon(new_exe, 3);
+  ValidateChromeShortcut(ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
+                         expected_properties1);
+  // Verify shortcut 2: icon remains unchanged.
+  expected_properties2.set_target(new_exe);
+  expected_properties2.set_icon(other_ico_, 0);
+  ValidateChromeShortcut(ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
+                         expected_properties2);
+}
 
 TEST_F(ShellUtilShortcutTest, CreateMultipleStartMenuShortcutsAndRemoveFolder) {
   ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
                   ShellUtil::SHORTCUT_LOCATION_START_MENU_CHROME_DIR,
                   dist_, test_properties_,
                   ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
   ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
                   ShellUtil::SHORTCUT_LOCATION_START_MENU_CHROME_APPS_DIR,
                   dist_, test_properties_,
                   ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
@@ skipping 167 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));
   }
 }