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

5 files changed, 107 insertions, 56 deletions

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()));
 }