During the self-destruct flow (user-level Chrome being replaced by system-level Chrome), existing code to retarget shortcuts failed to update icon executables. This CL fixes this. The heuristics are summarized below:

1. Only considering shortcuts with parameters .
2. Shortcuts whose icon file == old target (user-level chrome.exe) gets updated to new target (system-level chrome.exe).
3. Shortcut index is kept same as before.

(1) lets us delete user-level (pure) Chrome shortcut since system-level Chrome shortcut is already present. (2) lets us select migrate App Launcher shortcut (points to chrome.exe), without changing app shortcuts (points to fixed .ico files). (3) prevents App Launcher shortcut from mutating into Chrome shortcut (it assumes icon resources enumeration don't change.

To make (2) and (3) possible, we need to read the icon file and icon index. This is done by generalizing ResolveShortcut() to ResolveShortcutProperties().

The new logic requires a new flow in ShellUtils, and no longer uses the UpdateShortcutsWithArgs() flow.

BUG=326562

Review URL: https://codereview.chromium.org/108193019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@243642 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 188 insertions, 118 deletions

diff --git a/chrome/installer/setup/uninstall.cc b/chrome/installer/setup/uninstall.cc
index d11334e..4bb3e70 100644
--- a/chrome/installer/setup/uninstall.cc
+++ b/chrome/installer/setup/uninstall.cc
@@ -326,17 +326,15 @@ void RetargetUserShortcutsWithArgs(const InstallerState& installer_state,
   }
   BrowserDistribution* dist = product.distribution();
   ShellUtil::ShellChange install_level = ShellUtil::CURRENT_USER;
-  ShellUtil::ShortcutProperties updated_properties(install_level);
-  updated_properties.set_target(new_target_exe);
 
   // Retarget all shortcuts that point to |old_target_exe| from all
   // ShellUtil::ShortcutLocations.
   VLOG(1) << "Retargeting shortcuts.";
   for (int location = ShellUtil::SHORTCUT_LOCATION_FIRST;
       location < ShellUtil::NUM_SHORTCUT_LOCATIONS; ++location) {
-    if (!ShellUtil::UpdateShortcutsWithArgs(
+    if (!ShellUtil::RetargetShortcutsWithArgs(
             static_cast<ShellUtil::ShortcutLocation>(location), dist,
-            install_level, old_target_exe, updated_properties)) {
+            install_level, old_target_exe, new_target_exe)) {
       LOG(WARNING) << "Failed to retarget shortcuts in ShortcutLocation: "
                    << location;
     }
diff --git a/chrome/installer/util/shell_util.cc b/chrome/installer/util/shell_util.cc
index a2c55af..f5e3851 100644
--- a/chrome/installer/util/shell_util.cc
+++ b/chrome/installer/util/shell_util.cc
@@ -1267,12 +1267,29 @@ bool ShortcutOpDelete(const base::FilePath& shortcut_path) {
   return ret;
 }
 
-bool ShortcutOpUpdate(const base::win::ShortcutProperties& shortcut_properties,
-                      const base::FilePath& shortcut_path) {
-  bool ret = base::win::CreateOrUpdateShortcutLink(
-      shortcut_path, shortcut_properties, base::win::SHORTCUT_UPDATE_EXISTING);
-  LOG_IF(ERROR, !ret) << "Failed to update " << shortcut_path.value();
-  return ret;
+bool ShortcutOpRetarget(const base::FilePath& old_target,
+                        const base::FilePath& new_target,
+                        const base::FilePath& shortcut_path) {
+  base::win::ShortcutProperties new_prop;
+  new_prop.set_target(new_target);
+
+  // If the old icon matches old target, then update icon while keeping the old
+  // icon index. Non-fatal if we fail to get the old icon.
+  base::win::ShortcutProperties old_prop;
+  if (base::win::ResolveShortcutProperties(
+          shortcut_path,
+          base::win::ShortcutProperties::PROPERTIES_ICON,
+          &old_prop)) {
+    if (InstallUtil::ProgramCompare(old_target).EvaluatePath(old_prop.icon))
+      new_prop.set_icon(new_target, old_prop.icon_index);
+  } else {
+    LOG(ERROR) << "Failed to resolve " << shortcut_path.value();
+  }
+
+  bool result = base::win::CreateOrUpdateShortcutLink(
+        shortcut_path, new_prop, base::win::SHORTCUT_UPDATE_EXISTING);
+  LOG_IF(ERROR, !result) << "Failed to retarget " << shortcut_path.value();
+  return result;
 }
 
 // {|location|, |dist|, |level|} determine |shortcut_folder|.
@@ -2104,18 +2121,18 @@ bool ShellUtil::RemoveShortcuts(ShellUtil::ShortcutLocation location,
 }
 
 // static
-bool ShellUtil::UpdateShortcutsWithArgs(
+bool ShellUtil::RetargetShortcutsWithArgs(
     ShellUtil::ShortcutLocation location,
     BrowserDistribution* dist,
     ShellChange level,
-    const base::FilePath& target_exe,
-    const ShellUtil::ShortcutProperties& properties) {
+    const base::FilePath& old_target_exe,
+    const base::FilePath& new_target_exe) {
   if (!ShellUtil::ShortcutLocationIsSupported(location))
     return true;  // Vacuous success.
 
-  FilterTargetEq shortcut_filter(target_exe, true);
+  FilterTargetEq shortcut_filter(old_target_exe, true);
   ShortcutOperationCallback shortcut_operation(
-      base::Bind(&ShortcutOpUpdate, TranslateShortcutProperties(properties)));
+      base::Bind(&ShortcutOpRetarget, old_target_exe, new_target_exe));
   return BatchShortcutAction(shortcut_filter.AsShortcutFilterCallback(),
                              shortcut_operation, location, dist, level);
 }
diff --git a/chrome/installer/util/shell_util.h b/chrome/installer/util/shell_util.h
index 52f611f..868a275 100644
--- a/chrome/installer/util/shell_util.h
+++ b/chrome/installer/util/shell_util.h
@@ -529,18 +529,20 @@ class ShellUtil {
                               ShellChange level,
                               const base::FilePath& target_exe);
 
-  // Applies the updates in |properties| to all matching shortcuts in
-  // |location|, i.e.:
-  // - the shortcut's original target is |target_exe|,
+  // Updates the target of all shortcuts in |location| that satisfy the
+  // following:
+  // - the shortcut's original target is |old_target_exe|,
   // - the original arguments are non-empty.
+  // If the shortcut's icon points to |old_target_exe|, then it also gets
+  // redirected to |new_target_exe|.
   // Returns true if all updates to matching shortcuts are successful, including
   // the vacuous case where no matching shortcuts are found.
-  static bool UpdateShortcutsWithArgs(
+  static bool RetargetShortcutsWithArgs(
       ShellUtil::ShortcutLocation location,
       BrowserDistribution* dist,
       ShellChange level,
-      const base::FilePath& target_exe,
-      const ShellUtil::ShortcutProperties& properties);
+      const base::FilePath& old_target_exe,
+      const base::FilePath& new_target_exe);
 
   // Sets |suffix| to the base 32 encoding of the md5 hash of this user's sid
   // preceded by a dot.
diff --git a/chrome/installer/util/shell_util_unittest.cc b/chrome/installer/util/shell_util_unittest.cc
index 33c898a..76ed7de5 100644
--- a/chrome/installer/util/shell_util_unittest.cc
+++ b/chrome/installer/util/shell_util_unittest.cc
@@ -27,6 +27,8 @@
 namespace {
 
 const wchar_t kManganeseExe[] = L"manganese.exe";
+const wchar_t kIronExe[] = L"iron.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
@@ -47,6 +49,12 @@ class ShellUtilShortcutTest : public testing::Test {
     manganese_exe_ = temp_dir_.path().Append(kManganeseExe);
     EXPECT_EQ(0, file_util::WriteFile(manganese_exe_, "", 0));
 
+    iron_exe_ = temp_dir_.path().Append(kIronExe);
+    EXPECT_EQ(0, file_util::WriteFile(iron_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());
@@ -77,16 +85,14 @@ class ShellUtilShortcutTest : public testing::Test {
     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_icon(icon_path, 7);
     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) {
@@ -110,18 +116,26 @@ class ShellUtilShortcutTest : public testing::Test {
         break;
       default:
         ADD_FAILURE() << "Unknown location";
-        return;
+        return base::FilePath();
     }
 
-    base::string16 shortcut_name;
-    if (properties.has_shortcut_name()) {
-      shortcut_name = properties.shortcut_name;
-    } else {
-      shortcut_name =
-          dist_->GetShortcutName(BrowserDistribution::SHORTCUT_CHROME);
-    }
+    base::string16 shortcut_name = properties.has_shortcut_name() ?
+        properties.shortcut_name :
+        dist_->GetShortcutName(BrowserDistribution::SHORTCUT_CHROME);
     shortcut_name.append(installer::kLnkExt);
-    expected_path = expected_path.Append(shortcut_name);
+    return expected_path.Append(shortcut_name);
+  }
+
+  // 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()) {
@@ -143,7 +157,7 @@ class ShellUtilShortcutTest : public testing::Test {
       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);
@@ -186,6 +200,8 @@ class ShellUtilShortcutTest : public testing::Test {
 
   base::FilePath chrome_exe_;
   base::FilePath manganese_exe_;
+  base::FilePath iron_exe_;
+  base::FilePath other_ico_;
 };
 
 }  // namespace
@@ -368,11 +384,8 @@ TEST_F(ShellUtilShortcutTest, RemoveChromeShortcut) {
   ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
                   ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_,
                   ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
-
-  base::string16 shortcut_name(
-      dist_->GetShortcutName(BrowserDistribution::SHORTCUT_CHROME) +
-      installer::kLnkExt);
-  base::FilePath shortcut_path(fake_user_desktop_.path().Append(shortcut_name));
+  base::FilePath shortcut_path = GetExpectedShortcutPath(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_);
   ASSERT_TRUE(base::PathExists(shortcut_path));
 
   ASSERT_TRUE(ShellUtil::RemoveShortcuts(
@@ -387,12 +400,8 @@ TEST_F(ShellUtilShortcutTest, RemoveSystemLevelChromeShortcut) {
   ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
                   ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_,
                   ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
-
-  base::string16 shortcut_name(
-      dist_->GetShortcutName(BrowserDistribution::SHORTCUT_CHROME) +
-      installer::kLnkExt);
-  base::FilePath shortcut_path(
-      fake_common_desktop_.path().Append(shortcut_name));
+  base::FilePath shortcut_path = GetExpectedShortcutPath(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_);
   ASSERT_TRUE(base::PathExists(shortcut_path));
 
   ASSERT_TRUE(ShellUtil::RemoveShortcuts(
@@ -403,56 +412,60 @@ TEST_F(ShellUtilShortcutTest, RemoveSystemLevelChromeShortcut) {
 }
 
 TEST_F(ShellUtilShortcutTest, RemoveMultipleChromeShortcuts) {
-  const wchar_t kShortcutName1[] = L"Chrome 1";
-  const wchar_t kShortcutName2[] = L"Chrome 2";
-
-  test_properties_.set_shortcut_name(kShortcutName1);
+  // Shortcut 1: targets "chrome.exe"; no arguments.
+  test_properties_.set_shortcut_name(L"Chrome 1");
+  test_properties_.set_arguments(L"");
   ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
                   ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_,
                   ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
-  base::string16 shortcut1_name(
-      base::string16(kShortcutName1).append(installer::kLnkExt));
-  base::FilePath shortcut1_path(
-      fake_user_desktop_.path().Append(shortcut1_name));
+  base::FilePath shortcut1_path = GetExpectedShortcutPath(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_);
   ASSERT_TRUE(base::PathExists(shortcut1_path));
 
-  test_properties_.set_shortcut_name(kShortcutName2);
+  // Shortcut 2: targets "chrome.exe"; has arguments; icon set to "other.ico".
+  test_properties_.set_shortcut_name(L"Chrome 2");
   test_properties_.set_arguments(L"--profile-directory=\"Profile 2\"");
+  test_properties_.set_icon(other_ico_, 0);
   ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
                   ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_,
                   ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
-  base::string16 shortcut2_name(
-      base::string16(kShortcutName2).append(installer::kLnkExt));
-  base::FilePath shortcut2_path(
-      fake_user_desktop_.path().Append(shortcut2_name));
+  base::FilePath shortcut2_path = GetExpectedShortcutPath(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_);
   ASSERT_TRUE(base::PathExists(shortcut2_path));
 
+  // Shortcut 3: targets "iron.exe"; has arguments; icon set to "chrome.exe".
+  test_properties_.set_shortcut_name(L"Iron 3");
+  test_properties_.set_target(iron_exe_);
+  test_properties_.set_icon(chrome_exe_, 3);
+  ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
+                  ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_,
+                  ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
+  base::FilePath shortcut3_path = GetExpectedShortcutPath(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_);
+  ASSERT_TRUE(base::PathExists(shortcut3_path));
+
+  // Remove shortcuts that target "chrome.exe".
   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(shortcut3_path));
   ASSERT_TRUE(base::PathExists(shortcut1_path.DirName()));
 }
 
-TEST_F(ShellUtilShortcutTest, UpdateChromeShortcutsWithArgs) {
+TEST_F(ShellUtilShortcutTest, RetargetShortcutsWithArgs) {
   ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
                   ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_,
                   ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
+  ASSERT_TRUE(base::PathExists(GetExpectedShortcutPath(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_)));
 
-  base::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 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(
+  base::FilePath new_exe = manganese_exe_;
+  // Relies on the 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);
@@ -460,26 +473,19 @@ TEST_F(ShellUtilShortcutTest, UpdateChromeShortcutsWithArgs) {
                          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));
+  ASSERT_TRUE(base::PathExists(GetExpectedShortcutPath(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_)));
 
-  base::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 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(
+  base::FilePath new_exe = manganese_exe_;
+  // Relies on the 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);
@@ -487,54 +493,101 @@ TEST_F(ShellUtilShortcutTest, UpdateSystemLevelChromeShortcutsWithArgs) {
                          expected_properties);
 }
 
-TEST_F(ShellUtilShortcutTest, UpdateMultipleChromeShortcutsWithArgs) {
-  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_F(ShellUtilShortcutTest, RetargetChromeShortcutsWithArgsEmpty) {
+  // Shortcut 1: targets "chrome.exe"; no arguments.
+  test_properties_.set_shortcut_name(L"Chrome 1");
   test_properties_.set_arguments(L"");
   ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
                   ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_,
                   ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
-  base::string16 shortcut1_name(
-      base::string16(kShortcutName1).append(installer::kLnkExt));
-  base::FilePath shortcut1_path(
-      fake_user_desktop_.path().Append(shortcut1_name));
+  ASSERT_TRUE(base::PathExists(GetExpectedShortcutPath(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_)));
   ShellUtil::ShortcutProperties expected_properties1(test_properties_);
 
-  // Setup shortcut 2, which has non-empty arguments.
-  base::string16 shortcut2_args = L"--profile-directory=\"Profile 2\"";
-  test_properties_.set_shortcut_name(kShortcutName2);
-  test_properties_.set_arguments(shortcut2_args);
+  // Shortcut 2: targets "chrome.exe"; has arguments.
+  test_properties_.set_shortcut_name(L"Chrome 2");
+  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));
-  base::string16 shortcut2_name(
-      base::string16(kShortcutName2).append(installer::kLnkExt));
-  base::FilePath shortcut2_path(
-      fake_user_desktop_.path().Append(shortcut2_name));
-  ASSERT_TRUE(base::PathExists(shortcut2_path));
+  ASSERT_TRUE(base::PathExists(GetExpectedShortcutPath(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_)));
   ShellUtil::ShortcutProperties expected_properties2(test_properties_);
 
-  // Update shortcuts: target "manganese.exe" instead of "chrome.exe".
-  base::FilePath new_exe = temp_dir_.path().Append(kManganeseExe);
-  ShellUtil::ShortcutProperties updated_properties(ShellUtil::CURRENT_USER);
-  updated_properties.set_target(new_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 = manganese_exe_;
+  ASSERT_TRUE(ShellUtil::RetargetShortcutsWithArgs(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::CURRENT_USER,
+      chrome_exe_, new_exe));
+
+  // Verify shortcut 1: no change.
+  ValidateChromeShortcut(ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
+                         expected_properties1);
+
+  // Verify shortcut 2: target => "manganese.exe".
+  expected_properties2.set_target(new_exe);
+  ValidateChromeShortcut(ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
+                         expected_properties2);
+}
+
+TEST_F(ShellUtilShortcutTest, RetargetChromeShortcutsWithArgsIcon) {
+  const int kTestIconIndex1 = 3;
+  const int kTestIconIndex2 = 5;
+  const int kTestIconIndex3 = 8;
+
+  // Shortcut 1: targets "chrome.exe"; icon same as target.
+  test_properties_.set_shortcut_name(L"Chrome 1");
+  test_properties_.set_icon(test_properties_.target, kTestIconIndex1);
+  ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
+                  ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_,
+                  ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
+  ASSERT_TRUE(base::PathExists(GetExpectedShortcutPath(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_)));
+  ShellUtil::ShortcutProperties expected_properties1(test_properties_);
+
+  // Shortcut 2: targets "chrome.exe"; icon set to "other.ico".
+  test_properties_.set_shortcut_name(L"Chrome 2");
+  test_properties_.set_icon(other_ico_, kTestIconIndex2);
+  ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
+                  ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_,
+                  ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
+  ASSERT_TRUE(base::PathExists(GetExpectedShortcutPath(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_)));
+  ShellUtil::ShortcutProperties expected_properties2(test_properties_);
 
-  // Only changing shrotcuts that have non-empty arguments, i.e., shortcut 2.
-  ASSERT_TRUE(ShellUtil::UpdateShortcutsWithArgs(
+  // Shortcut 3: targets "iron.exe"; icon set to "chrome.exe".
+  test_properties_.set_target(iron_exe_);
+  test_properties_.set_shortcut_name(L"Iron 3");
+  test_properties_.set_icon(chrome_exe_, kTestIconIndex3);
+  ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
+                  ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_,
+                  ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS));
+  ASSERT_TRUE(base::PathExists(GetExpectedShortcutPath(
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_)));
+  ShellUtil::ShortcutProperties expected_properties3(test_properties_);
+
+  // Retarget shortcuts: replace "chrome.exe" with "manganese.exe".
+  // Relies on the fact that |test_properties_| has non-empty arguments.
+  base::FilePath new_exe = manganese_exe_;
+  ASSERT_TRUE(ShellUtil::RetargetShortcutsWithArgs(
       ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::CURRENT_USER,
-      chrome_exe_, updated_properties));
-  // Verify shortcut 1.
-  // |expected_properties1| was unchanged and still targets "chrome.exe", since
-  // it has empty target, yet we passed |require_args| = true.
+      chrome_exe_, new_exe));
+
+  // Verify shortcut 1: target & icon => "manganese.exe", kept same icon index.
+  expected_properties1.set_target(new_exe);
+  expected_properties1.set_icon(new_exe, kTestIconIndex1);
   ValidateChromeShortcut(ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
                          expected_properties1);
-  // Verify shortcut 2.
+
+  // Verify shortcut 2: target => "manganese.exe", kept icon.
   expected_properties2.set_target(new_exe);
   ValidateChromeShortcut(ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
                          expected_properties2);
+
+  // Verify shortcut 3: no change, since target doesn't match.
+  ValidateChromeShortcut(ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
+                         expected_properties3);
 }
 
 TEST_F(ShellUtilShortcutTest, CreateMultipleStartMenuShortcutsAndRemoveFolder) {