Windows: Remove desktop profile shortcuts (and any others pointing to the exe) on uninstall.

This is a re-land of https://codereview.chromium.org/11693010/, which failed on the tree on XP bots. The issue was that ProgramCompare::Evaluate() was parsing its value parameter is a command-line, which meant it was truncating it at the first space. This updated CL adds an EvaluatePath() function that takes a FilePath and doesn't try to parse it as a command-line. 

BUG=146636
TEST=Uninstalling Chrome removes any present profile shortcuts on the desktop.

Review URL: https://chromiumcodereview.appspot.com/11743022

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

9 files changed, 120 insertions, 74 deletions

diff --git a/chrome/browser/chrome_browser_main_win.cc b/chrome/browser/chrome_browser_main_win.cc
index 67850ba..b76a20a 100644
--- a/chrome/browser/chrome_browser_main_win.cc
+++ b/chrome/browser/chrome_browser_main_win.cc
@@ -135,9 +135,6 @@ int DoUninstallTasks(bool chrome_still_running) {
     VLOG(1) << "Executing uninstall actions";
     if (!first_run::RemoveSentinel())
       VLOG(1) << "Failed to delete sentinel file.";
-    // We want to remove user level shortcuts and we only care about the ones
-    // created by us and not by the installer so |alternate| is false.
-    BrowserDistribution* dist = BrowserDistribution::GetDistribution();
     FilePath chrome_exe;
     if (PathService::Get(base::FILE_EXE, &chrome_exe)) {
       ShellUtil::ShortcutLocation user_shortcut_locations[] = {
@@ -145,15 +142,15 @@ int DoUninstallTasks(bool chrome_still_running) {
         ShellUtil::SHORTCUT_LOCATION_QUICK_LAUNCH,
         ShellUtil::SHORTCUT_LOCATION_START_MENU,
       };
+      BrowserDistribution* dist = BrowserDistribution::GetDistribution();
       for (size_t i = 0; i < arraysize(user_shortcut_locations); ++i) {
-        if (!ShellUtil::RemoveShortcut(
-                user_shortcut_locations[i], dist, chrome_exe.value(),
-                ShellUtil::CURRENT_USER, NULL)) {
+        if (!ShellUtil::RemoveShortcut(user_shortcut_locations[i], dist,
+                                       chrome_exe, ShellUtil::CURRENT_USER,
+                                       NULL)) {
           VLOG(1) << "Failed to delete shortcut at location "
                   << user_shortcut_locations[i];
         }
       }
-      // TODO(rlp): Cleanup profiles shortcuts.
     } else {
       NOTREACHED();
     }
diff --git a/chrome/browser/profiles/profile_shortcut_manager_win.cc b/chrome/browser/profiles/profile_shortcut_manager_win.cc
index 7ff8009..8bb0e43 100644
--- a/chrome/browser/profiles/profile_shortcut_manager_win.cc
+++ b/chrome/browser/profiles/profile_shortcut_manager_win.cc
@@ -271,8 +271,7 @@ void DeleteDesktopShortcutsAndIconFile(const FilePath& profile_path,
     const string16 shortcut_name =
         shortcuts[i].BaseName().RemoveExtension().value();
     ShellUtil::RemoveShortcut(ShellUtil::SHORTCUT_LOCATION_DESKTOP,
-                              distribution, chrome_exe.value(),
-                              ShellUtil::CURRENT_USER,
+                              distribution, chrome_exe, ShellUtil::CURRENT_USER,
                               &shortcut_name);
   }
 
diff --git a/chrome/installer/setup/install.cc b/chrome/installer/setup/install.cc
index 7aa2319..0574eee 100644
--- a/chrome/installer/setup/install.cc
+++ b/chrome/installer/setup/install.cc
@@ -281,7 +281,7 @@ void CleanupLegacyShortcuts(const InstallerState& installer_state,
 
   if (installer_state.system_install()) {
     ShellUtil::RemoveShortcut(
-        ShellUtil::SHORTCUT_LOCATION_QUICK_LAUNCH, dist, chrome_exe.value(),
+        ShellUtil::SHORTCUT_LOCATION_QUICK_LAUNCH, dist, chrome_exe,
         ShellUtil::SYSTEM_LEVEL, NULL);
   }
 }
diff --git a/chrome/installer/setup/uninstall.cc b/chrome/installer/setup/uninstall.cc
index e8a81eb..bbfb231 100644
--- a/chrome/installer/setup/uninstall.cc
+++ b/chrome/installer/setup/uninstall.cc
@@ -325,10 +325,10 @@ void CloseChromeFrameHelperProcess() {
 
 // Deletes shortcuts at |install_level| from Start menu, Desktop,
 // Quick Launch, taskbar, and secondary tiles on the Start Screen (Win8+).
-// Only shortcuts pointing to |target| will be removed.
+// Only shortcuts pointing to |target_exe| will be removed.
 void DeleteShortcuts(const InstallerState& installer_state,
                      const Product& product,
-                     const string16& target_exe) {
+                     const FilePath& target_exe) {
   BrowserDistribution* dist = product.distribution();
 
   // The per-user shortcut for this user, if present on a system-level install,
@@ -367,10 +367,10 @@ void DeleteShortcuts(const InstallerState& installer_state,
   // it is possible for shortcuts to remain pinned while their parent shortcut
   // has been deleted or changed to point to another |target_exe|. Make sure all
   // pinned-to-taskbar shortcuts that point to |target_exe| are unpinned.
-  ShellUtil::RemoveTaskbarShortcuts(target_exe);
+  ShellUtil::RemoveTaskbarShortcuts(target_exe.value());
 
   ShellUtil::RemoveStartScreenShortcuts(product.distribution(),
-                                        target_exe);
+                                        target_exe.value());
 }
 
 bool ScheduleParentAndGrandparentForDeletion(const FilePath& path) {
@@ -1139,14 +1139,13 @@ InstallStatus UninstallProduct(const InstallationState& original_state,
     auto_launch_util::DisableAllAutoStartFeatures(
         ASCIIToUTF16(chrome::kInitialProfile));
 
-    DeleteShortcuts(installer_state, product, chrome_exe);
+    DeleteShortcuts(installer_state, product, FilePath(chrome_exe));
 
   } else if (product.is_chrome_app_host()) {
     // TODO(huangs): Remove this check once we have system-level App Host.
     DCHECK(!installer_state.system_install());
-    const string16 app_host_exe(
-        installer_state.target_path().Append(installer::kChromeAppHostExe)
-            .value());
+    const FilePath app_host_exe(
+        installer_state.target_path().Append(installer::kChromeAppHostExe));
     DeleteShortcuts(installer_state, product, app_host_exe);
   }
 
diff --git a/chrome/installer/util/install_util.cc b/chrome/installer/util/install_util.cc
index d98a30b..3c8504e 100644
--- a/chrome/installer/util/install_util.cc
+++ b/chrome/installer/util/install_util.cc
@@ -569,8 +569,12 @@ bool InstallUtil::ProgramCompare::Evaluate(const string16& value) const {
     return false;
   }
 
+  return EvaluatePath(program);
+}
+
+bool InstallUtil::ProgramCompare::EvaluatePath(const FilePath& path) const {
   // Try the simple thing first: do the paths happen to match?
-  if (FilePath::CompareEqualIgnoreCase(path_to_match_.value(), program.value()))
+  if (FilePath::CompareEqualIgnoreCase(path_to_match_.value(), path.value()))
     return true;
 
   // If the paths don't match and we couldn't open the expected file, we've done
@@ -582,7 +586,7 @@ bool InstallUtil::ProgramCompare::Evaluate(const string16& value) const {
   base::win::ScopedHandle handle;
   BY_HANDLE_FILE_INFORMATION info = {};
 
-  return (OpenForInfo(program, &handle) &&
+  return (OpenForInfo(path, &handle) &&
           GetInfo(handle, &info) &&
           info.dwVolumeSerialNumber == file_info_.dwVolumeSerialNumber &&
           info.nFileIndexHigh == file_info_.nFileIndexHigh &&
diff --git a/chrome/installer/util/install_util.h b/chrome/installer/util/install_util.h
index c7c4495..e0ac014 100644
--- a/chrome/installer/util/install_util.h
+++ b/chrome/installer/util/install_util.h
@@ -184,6 +184,7 @@ class InstallUtil {
     explicit ProgramCompare(const FilePath& path_to_match);
     virtual ~ProgramCompare();
     virtual bool Evaluate(const string16& value) const OVERRIDE;
+    bool EvaluatePath(const FilePath& path) const;
 
    protected:
     static bool OpenForInfo(const FilePath& path,
diff --git a/chrome/installer/util/shell_util.cc b/chrome/installer/util/shell_util.cc
index c6a1982..1383a72 100644
--- a/chrome/installer/util/shell_util.cc
+++ b/chrome/installer/util/shell_util.cc
@@ -1154,6 +1154,35 @@ ShellUtil::DefaultState ProbeProtocolHandlers(
   return ProbeOpenCommandHandlers(protocols, num_protocols);
 }
 
+// Removes shortcut at |shortcut_path| if it is a shortcut that points to
+// |target_exe|. If |delete_folder| is true, deletes the parent folder of
+// the shortcut completely. Returns true if either the shortcut was deleted
+// successfully or if the shortcut did not point to |target_exe|.
+bool MaybeRemoveShortcutAtPath(const FilePath& shortcut_path,
+                               const FilePath& target_exe,
+                               bool delete_folder) {
+  FilePath target_path;
+  if (!base::win::ResolveShortcut(shortcut_path, &target_path, NULL))
+    return false;
+
+  if (InstallUtil::ProgramCompare(target_exe).EvaluatePath(target_path)) {
+    // Unpin the shortcut if it was ever pinned by the user or the installer.
+    VLOG(1) << "Trying to unpin " << shortcut_path.value();
+    if (!base::win::TaskbarUnpinShortcutLink(shortcut_path.value().c_str())) {
+      VLOG(1) << shortcut_path.value()
+              << " wasn't pinned (or the unpin failed).";
+    }
+    if (delete_folder)
+      return file_util::Delete(shortcut_path.DirName(), true);
+    else
+      return file_util::Delete(shortcut_path, false);
+  }
+
+  // The shortcut at |shortcut_path| doesn't point to |target_exe|, act as if
+  // our shortcut had been deleted.
+  return true;
+}
+
 }  // namespace
 
 const wchar_t* ShellUtil::kRegDefaultIcon = L"\\DefaultIcon";
@@ -1277,13 +1306,13 @@ bool ShellUtil::CreateOrUpdateShortcut(
   user_shortcut_path = user_shortcut_path.Append(shortcut_name);
   system_shortcut_path = system_shortcut_path.Append(shortcut_name);
 
-  FilePath *chosen_path;
+  FilePath* chosen_path;
   bool should_install_shortcut = true;
   if (properties.level == SYSTEM_LEVEL) {
     // Install the system-level shortcut if requested.
     chosen_path = &system_shortcut_path;
   } else if (operation != SHELL_SHORTCUT_CREATE_IF_NO_SYSTEM_LEVEL ||
-             !file_util::PathExists(system_shortcut_path)){
+             !file_util::PathExists(system_shortcut_path)) {
     // Otherwise install the user-level shortcut, unless the system-level
     // variant of this shortcut is present on the machine and |operation| states
     // not to create a user-level shortcut in that case.
@@ -1795,10 +1824,10 @@ bool ShellUtil::RegisterChromeForProtocol(BrowserDistribution* dist,
 
 bool ShellUtil::RemoveShortcut(ShellUtil::ShortcutLocation location,
                                BrowserDistribution* dist,
-                               const string16& target_exe,
+                               const FilePath& target_exe,
                                ShellChange level,
                                const string16* shortcut_name) {
-  bool delete_folder = (location == SHORTCUT_LOCATION_START_MENU);
+  const bool delete_folder = (location == SHORTCUT_LOCATION_START_MENU);
 
   FilePath shortcut_folder;
   if (!GetShortcutPath(location, dist, level, &shortcut_folder) ||
@@ -1807,43 +1836,29 @@ bool ShellUtil::RemoveShortcut(ShellUtil::ShortcutLocation location,
     return false;
   }
 
-  string16 shortcut_base_name(
+  if (!delete_folder && !shortcut_name) {
+    file_util::FileEnumerator enumerator(shortcut_folder, false,
+        file_util::FileEnumerator::FILES);
+    bool had_failures = false;
+    for (FilePath path = enumerator.Next(); !path.empty();
+         path = enumerator.Next()) {
+      if (path.Extension() != installer::kLnkExt)
+        continue;
+
+      if (!MaybeRemoveShortcutAtPath(path, target_exe, delete_folder))
+        had_failures = true;
+    }
+    return !had_failures;
+  }
+
+  const string16 shortcut_base_name(
       (shortcut_name ? *shortcut_name : dist->GetAppShortCutName()) +
       installer::kLnkExt);
-  FilePath shortcut_path(shortcut_folder.Append(shortcut_base_name));
-
+  const FilePath shortcut_path(shortcut_folder.Append(shortcut_base_name));
   if (!file_util::PathExists(shortcut_path))
     return true;
 
-  base::win::ScopedComPtr<IShellLink> i_shell_link;
-  base::win::ScopedComPtr<IPersistFile> i_persist_file;
-  wchar_t read_target[MAX_PATH] = {};
-  if (FAILED(i_shell_link.CreateInstance(CLSID_ShellLink, NULL,
-                                         CLSCTX_INPROC_SERVER)) ||
-      FAILED(i_persist_file.QueryFrom(i_shell_link)) ||
-      FAILED(i_persist_file->Load(shortcut_path.value().c_str(), STGM_READ)) ||
-      FAILED(i_shell_link->GetPath(read_target, MAX_PATH, NULL,
-                                   SLGP_SHORTPATH))) {
-    NOTREACHED();
-    return false;
-  }
-
-  if (InstallUtil::ProgramCompare(FilePath(target_exe)).Evaluate(read_target)) {
-    // Unpin the shortcut if it was ever pinned by the user or the installer.
-    VLOG(1) << "Trying to unpin " << shortcut_path.value();
-    if (!base::win::TaskbarUnpinShortcutLink(shortcut_path.value().c_str())) {
-      VLOG(1) << shortcut_path.value()
-              << " wasn't pinned (or the unpin failed).";
-    }
-    if (delete_folder)
-      return file_util::Delete(shortcut_folder, true);
-    else
-      return file_util::Delete(shortcut_path, false);
-  }
-
-  // The shortcut at |shortcut_path| doesn't point to |target_exe|, act as if
-  // our shortcut had been deleted.
-  return true;
+  return MaybeRemoveShortcutAtPath(shortcut_path, target_exe, delete_folder);
 }
 
 void ShellUtil::RemoveTaskbarShortcuts(const string16& target_exe) {
@@ -1870,7 +1885,7 @@ void ShellUtil::RemoveTaskbarShortcuts(const string16& target_exe) {
       LOG(ERROR) << "Couldn't resolve shortcut at " << shortcut_path.value();
       continue;
     }
-    if (target_compare.Evaluate(read_target.value())) {
+    if (target_compare.EvaluatePath(read_target)) {
       // Unpin this shortcut if it points to |target_exe|.
       base::win::TaskbarUnpinShortcutLink(shortcut_path.value().c_str());
     }
diff --git a/chrome/installer/util/shell_util.h b/chrome/installer/util/shell_util.h
index 98439c3..0c9823e 100644
--- a/chrome/installer/util/shell_util.h
+++ b/chrome/installer/util/shell_util.h
@@ -497,21 +497,21 @@ class ShellUtil {
                                         const string16& protocol,
                                         bool elevate_if_not_admin);
 
-  // Removes installed shortcut at |location|.
-  // |chrome_exe|: The path to the chrome.exe being uninstalled; the shortcut
-  // will only be deleted if its target is also |chrome_exe|.
+  // Removes installed shortcut(s) at |location|.
+  // |target_exe|: Shortcut target exe; shortcuts will only be deleted when
+  // their target is |target_exe|.
   // |level|: CURRENT_USER to remove the per-user shortcut and SYSTEM_LEVEL to
   // remove the all-users shortcut.
   // |shortcut_name|: If non-null, remove the shortcut named |shortcut_name| at
-  // location; otherwise remove the default shortcut at |location|.
-  // If |location| is SHORTCUT_LOCATION_START_MENU the shortcut folder specific
+  // location; otherwise remove all shortcuts to |target_exe| at |location|.
+  // If |location| is SHORTCUT_LOCATION_START_MENU, the shortcut folder specific
   // to |dist| is deleted.
-  // Also attempts to unpin the removed shortcut from the taskbar.
-  // Returns true if the shortcut was successfully deleted (or there is no
-  // shortcut at |location| pointing to |chrome_exe|).
+  // Also attempts to unpin the removed shortcut(s) from the taskbar.
+  // Returns true if the shortcut(s) were successfully deleted (or there were
+  // none at |location| pointing to |target_exe|).
   static bool RemoveShortcut(ShellUtil::ShortcutLocation location,
                              BrowserDistribution* dist,
-                             const string16& target_exe,
+                             const FilePath& target_exe,
                              ShellChange level,
                              const string16* shortcut_name);
 
diff --git a/chrome/installer/util/shell_util_unittest.cc b/chrome/installer/util/shell_util_unittest.cc
index dd52fcf..9db9fac 100644
--- a/chrome/installer/util/shell_util_unittest.cc
+++ b/chrome/installer/util/shell_util_unittest.cc
@@ -357,7 +357,7 @@ TEST_F(ShellUtilShortcutTest, RemoveChromeShortcut) {
   ASSERT_TRUE(file_util::PathExists(shortcut_path));
 
   ASSERT_TRUE(ShellUtil::RemoveShortcut(
-      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, chrome_exe_.value(),
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, chrome_exe_,
       ShellUtil::CURRENT_USER, NULL));
   ASSERT_FALSE(file_util::PathExists(shortcut_path));
   ASSERT_TRUE(file_util::PathExists(shortcut_path.DirName()));
@@ -375,7 +375,7 @@ TEST_F(ShellUtilShortcutTest, RemoveSystemLevelChromeShortcut) {
   ASSERT_TRUE(file_util::PathExists(shortcut_path));
 
   ASSERT_TRUE(ShellUtil::RemoveShortcut(
-      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, chrome_exe_.value(),
+      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()));
@@ -394,12 +394,43 @@ TEST_F(ShellUtilShortcutTest, RemoveChromeShortcutWithSpecialName) {
   ASSERT_TRUE(file_util::PathExists(shortcut_path));
 
   ASSERT_TRUE(ShellUtil::RemoveShortcut(
-      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, chrome_exe_.value(),
+      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));
+  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));
+  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()));
+}
+
 TEST_F(ShellUtilShortcutTest, CreateMultipleStartMenuShortcutsAndRemoveFolder) {
   ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut(
                   ShellUtil::SHORTCUT_LOCATION_START_MENU,
@@ -413,8 +444,8 @@ TEST_F(ShellUtilShortcutTest, CreateMultipleStartMenuShortcutsAndRemoveFolder) {
 
   FilePath shortcut_folder(
       fake_start_menu_.path().Append(dist_->GetAppShortCutName()));
-  file_util::FileEnumerator file_counter (shortcut_folder, false,
-                                          file_util::FileEnumerator::FILES);
+  file_util::FileEnumerator file_counter(shortcut_folder, false,
+                                         file_util::FileEnumerator::FILES);
   int count = 0;
   while (!file_counter.Next().empty())
     ++count;
@@ -422,7 +453,7 @@ TEST_F(ShellUtilShortcutTest, CreateMultipleStartMenuShortcutsAndRemoveFolder) {
 
   ASSERT_TRUE(file_util::PathExists(shortcut_folder));
   ASSERT_TRUE(ShellUtil::RemoveShortcut(
-      ShellUtil::SHORTCUT_LOCATION_START_MENU, dist_, chrome_exe_.value(),
+      ShellUtil::SHORTCUT_LOCATION_START_MENU, dist_, chrome_exe_,
       ShellUtil::CURRENT_USER, NULL));
   ASSERT_FALSE(file_util::PathExists(shortcut_folder));
 }
@@ -448,8 +479,8 @@ TEST_F(ShellUtilShortcutTest, DontRemoveChromeShortcutIfPointsToAnotherChrome) {
   // |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_.value(), ShellUtil::CURRENT_USER, NULL));
+      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, chrome_exe_,
+      ShellUtil::CURRENT_USER, NULL));
   ASSERT_TRUE(file_util::PathExists(shortcut_path));
   ASSERT_TRUE(file_util::PathExists(shortcut_path.DirName()));
 }