Refactoring installer shortcut deletion; adding dedicated shortcut update feature.

chrome/installer/util/shell_util.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.
 //
 // This file defines functions that integrate Chrome in Windows shell. These
 // functions can be used by Chrome as well as Chrome installer. All of the
 // work is done by the local functions defined in anonymous namespace in
 // this class.
 
 #include "chrome/installer/util/shell_util.h"
 
 #include <windows.h>
 #include <shlobj.h>
 
 #include <limits>
 #include <string>
 
+#include "base/bind.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/md5.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "base/path_service.h"
 #include "base/string16.h"
@@ skipping 1124 matching lines @@
   const base::win::Version windows_version = base::win::GetVersion();
 
   if (windows_version >= base::win::VERSION_WIN8)
     return ProbeCurrentDefaultHandlers(protocols, num_protocols);
   else if (windows_version >= base::win::VERSION_VISTA)
     return ProbeAppIsDefaultHandlers(protocols, num_protocols);
 
   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 base::FilePath& shortcut_path,
-                               const base::FilePath& target_exe,
-                               bool delete_folder) {
-  base::FilePath target_path;
-  if (!base::win::ResolveShortcut(shortcut_path, &target_path, NULL))
+// (Windows 8+) Finds and stores an app shortcuts folder path in *|path|.
+// Returns true on success.
+bool GetAppShortcutsFolder(BrowserDistribution* dist,
+                           ShellUtil::ShellChange level,
+                           base::FilePath *path) {
+  DCHECK(path);
+  if (base::win::GetVersion() < base::win::VERSION_WIN8)

[grt (UTC plus 2), 2013/05/01 13:35:58]

this check is performed in base_paths_win, so it isn't needed here. you could
make it a DCHECK if you really wanted to, or make the LOG on 1174 at level
DFATAL rather than ERROR.

[huangs, 2013/05/01 15:56:29]

With ShortcutLocationIsExpected(), code will not flow here for non-Windows 8.
Adding DCHECK(), since this is a specialized routine.

     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);
+  base::FilePath folder;
+  if (!PathService::Get(base::DIR_APP_SHORTCUTS, &folder) || folder.empty()) {

[grt (UTC plus 2), 2013/05/01 13:35:58]

now that i've looked, i see that PathService will never return true and an empty
path. please fix here and elsewhere.

[huangs, 2013/05/01 15:56:29]

Done.

+    LOG(ERROR) << "Could not get application shortcuts location.";
+    return false;
   }
 
-  // The shortcut at |shortcut_path| doesn't point to |target_exe|, act as if
-  // our shortcut had been deleted.
+  folder = folder.Append(ShellUtil::GetBrowserModelId(
+                             dist, level == ShellUtil::CURRENT_USER));
+  if (!file_util::DirectoryExists(folder)) {
+    VLOG(1) << "No start screen shortcuts.";
+    return false;
+  }
+
+  *path = folder;
   return true;
 }
 
+typedef base::Callback<bool(const base::FilePath&)> FileOperationCallback;
+
+// Shortcut operations for BatchShortcutAction().
+
+bool ShortcutOpUnpin(const base::FilePath& shortcut_path) {
+  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).";
+    // No error, since shortcut might not be pinned.
+  }
+  return true;
+}
+
+bool ShortcutOpDelete(const base::FilePath& shortcut_path) {
+  bool ret = file_util::Delete(shortcut_path, false);
+  LOG_IF(ERROR, !ret) << "Failed to remove " << shortcut_path.value();
+  return ret;
+}
+
+bool ShortcutOpSetTarget(const base::FilePath& target_path,
+                         const base::FilePath& shortcut_path) {
+  base::win::ShortcutProperties shortcut_properties;
+  shortcut_properties.set_target(target_path);
+  shortcut_properties.set_working_dir(target_path.DirName());
+  bool ret = base::win::CreateOrUpdateShortcutLink(
+      shortcut_path, shortcut_properties, base::win::SHORTCUT_REPLACE_EXISTING);
+  LOG_IF(ERROR, !ret) << "Failed to retarget " << shortcut_path.value();
+  return ret;
+}
+
+// {|location|, |dist|, |level|} determine |shortcut_folder|.
+// Applies |shortcut_operation| to each shortcut in |shortcut_folder| that
+// targets |target_exe|.
+// Returns true if all operations are successful. All intended operations are
+// attempted even if failures occur.
+bool BatchShortcutAction(const FileOperationCallback& shortcut_operation,
+                         ShellUtil::ShortcutLocation location,
+                         BrowserDistribution* dist,
+                         ShellUtil::ShellChange level,
+                         const base::FilePath& target_exe) {
+  DCHECK(!shortcut_operation.is_null());
+  base::FilePath shortcut_folder;
+  if (!ShellUtil::GetShortcutPath(location, dist, level, &shortcut_folder)) {
+    LOG(WARNING) << "Cannot find path at location " << location;

[grt (UTC plus 2), 2013/05/01 13:35:58]

isn't this going to log warnings on Vista/Win7 and below for locations that
aren't supported on those OSes? also for Win8 if there are no app shortcuts, no?

[huangs, 2013/05/01 15:56:29]

This CL is really a means to an end for a high priority CL (I need
RetargetShortcuts()). To reduce risk, I want to preserve as much old behaviour,
but looks like we diverged from that...

Meanwhile, the warning should not appear, now that ShortcutLocationIsExpected()
is added. But it's good to log it in case of a goof. Also, I don't a DCHECK
should be used (this is a common routine).

+    return false;
+  }
+
+  bool success = true;
+  InstallUtil::ProgramCompare target_compare(target_exe);
+  file_util::FileEnumerator enumerator(
+      shortcut_folder, false, file_util::FileEnumerator::FILES,
+      string16(L"*") + installer::kLnkExt);
+  base::FilePath target_path;
+  for (base::FilePath shortcut_path = enumerator.Next();
+       !shortcut_path.empty();
+       shortcut_path = enumerator.Next()) {
+    if (base::win::ResolveShortcut(shortcut_path, &target_path, NULL)) {
+      if (target_compare.EvaluatePath(target_path) &&
+          !shortcut_operation.Run(shortcut_path)) {
+        success = false;
+      }
+    } else {
+      LOG(ERROR) << "Cannot resolve shortcut at " << shortcut_path.value();
+      success = false;
+    }
+  }
+  return success;
+}
+
+// Removes folder spsecified by {|location|, |dist|, |level|}.
+bool RemoveShortcutFolder(ShellUtil::ShortcutLocation location,
+                          BrowserDistribution* dist,
+                          ShellUtil::ShellChange level) {
+  base::FilePath shortcut_folder;
+  if (!ShellUtil::GetShortcutPath(location, dist, level, &shortcut_folder)) {
+    LOG(WARNING) << "Cannot find path at location " << location;

[grt (UTC plus 2), 2013/05/01 13:35:58]

same comment: isn't this going to log warnings on Vista/Win7 and below for
locations that aren't supported on those OSes? also for Win8 if there are no app
shortcuts, no?

[huangs, 2013/05/01 15:56:29]

Same response.  :)

Do you mean "for non-Win8"? Otherwise I made a mistake.

[grt (UTC plus 2), 2013/05/01 16:33:43]

No. TASKBAR_PINS does not apply to pre-Win7, APP_SHORTCUTS does not apply to
pre-Win8.

[huangs, 2013/05/01 17:42:09]

Right. I was thinking of the phrase "isn't this going to log warnings for
non-Win8 if there are no app shortcuts, no?"

The ShortcutLocationIsSupported() check from RemoveShortcuts() will limit the
case of SHORTCUT_LOCATION_APP_SHORTCUTS to Windows 8+.

+    return false;
+  }
+  if (!file_util::Delete(shortcut_folder, true)) {
+    LOG(ERROR) << "Cannot remove folder " << shortcut_folder.value();
+    return false;
+  }
+  return true;
+}
+
 }  // namespace
 
 const wchar_t* ShellUtil::kRegDefaultIcon = L"\\DefaultIcon";
 const wchar_t* ShellUtil::kRegShellPath = L"\\shell";
 const wchar_t* ShellUtil::kRegShellOpen = L"\\shell\\open\\command";
 const wchar_t* ShellUtil::kRegStartMenuInternet =
     L"Software\\Clients\\StartMenuInternet";
 const wchar_t* ShellUtil::kRegClasses = L"Software\\Classes";
 const wchar_t* ShellUtil::kRegRegisteredApplications =
     L"Software\\RegisteredApplications";
@@ skipping 46 matching lines @@
                                               const string16& chrome_exe,
                                               const string16& suffix) {
   return QuickIsChromeRegistered(dist, chrome_exe, suffix,
                                  CONFIRM_SHELL_REGISTRATION_IN_HKLM);
 }
 
 bool ShellUtil::GetShortcutPath(ShellUtil::ShortcutLocation location,
                                 BrowserDistribution* dist,
                                 ShellChange level,
                                 base::FilePath* path) {
+  DCHECK(path);
   int dir_key = -1;
   bool add_folder_for_dist = false;
   switch (location) {
     case SHORTCUT_LOCATION_DESKTOP:
       dir_key = (level == CURRENT_USER) ? base::DIR_USER_DESKTOP :
                                           base::DIR_COMMON_DESKTOP;
       break;
     case SHORTCUT_LOCATION_QUICK_LAUNCH:
       dir_key = (level == CURRENT_USER) ? base::DIR_USER_QUICK_LAUNCH :
                                           base::DIR_DEFAULT_USER_QUICK_LAUNCH;
       break;
     case SHORTCUT_LOCATION_START_MENU:
       dir_key = (level == CURRENT_USER) ? base::DIR_START_MENU :
                                           base::DIR_COMMON_START_MENU;
       add_folder_for_dist = true;
       break;
+    case SHORTCUT_LOCATION_TASKBAR_PINS:
+      if (base::win::GetVersion() < base::win::VERSION_WIN7)

[grt (UTC plus 2), 2013/05/01 13:35:58]

isn't this unnecessary with your change to base_paths_win.cc?

[huangs, 2013/05/01 15:56:29]

Done (removed).

+        return false;
+
+      dir_key = base::DIR_TASKBAR_PINS;
+      break;
+    case SHORTCUT_LOCATION_APP_SHORTCUTS:
+      return GetAppShortcutsFolder(dist, level, path);
+
     default:
       NOTREACHED();
       return false;
   }
 
   if (!PathService::Get(dir_key, path) || path->empty()) {
     NOTREACHED() << dir_key;
     return false;
   }
 
   if (add_folder_for_dist)
     *path = path->Append(dist->GetAppShortCutName());
 
   return true;
 }
 
 bool ShellUtil::CreateOrUpdateShortcut(
     ShellUtil::ShortcutLocation location,

[grt (UTC plus 2), 2013/05/01 13:35:58]

if i'm reading the code correctly, this function will NOTREACHED and return
false if |location| is SHORTCUT_LOCATION_APP_SHORTCUTS and the user had never
created an app shortcut. please at least make note of this in the doc comment.
it's probably worth removing the NOTREACHED's as well since a crash in this case
isn't desirable.

[huangs, 2013/05/01 15:56:29]

I'm tempted to add

  if (!ShortcutLocationIsExpected(location))
    return false;

on the top, but decided not risk shortcuts not being created. Removing this
particular NOTREACHED().

     BrowserDistribution* dist,
     const ShellUtil::ShortcutProperties& properties,
     ShellUtil::ShortcutOperation operation) {
   DCHECK(dist);
   // |pin_to_taskbar| is only acknowledged when first creating the shortcut.
   DCHECK(!properties.pin_to_taskbar ||
          operation == SHELL_SHORTCUT_CREATE_ALWAYS ||
          operation == SHELL_SHORTCUT_CREATE_IF_NO_SYSTEM_LEVEL);
 
   base::FilePath user_shortcut_path;
   base::FilePath system_shortcut_path;
-  if (!GetShortcutPath(location, dist, SYSTEM_LEVEL, &system_shortcut_path) ||
-      system_shortcut_path.empty()) {
+  if (!GetShortcutPath(location, dist, SYSTEM_LEVEL, &system_shortcut_path)) {
     NOTREACHED();
     return false;
   }
 
   string16 shortcut_name(ExtractShortcutNameFromProperties(dist, properties));
   system_shortcut_path = system_shortcut_path.Append(shortcut_name);
 
   base::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)) {
     // 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.
-    if (!GetShortcutPath(location, dist, CURRENT_USER, &user_shortcut_path) ||
-        user_shortcut_path.empty()) {
+    if (!GetShortcutPath(location, dist, CURRENT_USER, &user_shortcut_path)) {
       NOTREACHED();
       return false;
     }
     user_shortcut_path = user_shortcut_path.Append(shortcut_name);
     chosen_path = &user_shortcut_path;
   } else {
     // Do not install any shortcut if we are told to install a user-level
     // shortcut, but the system-level variant of that shortcut is present.
     // Other actions (e.g., pinning) can still happen with respect to the
     // existing system-level shortcut however.
@@ skipping 518 matching lines @@
   } else if (elevate_if_not_admin &&
              base::win::GetVersion() >= base::win::VERSION_VISTA) {
     // Elevate to do the whole job
     return ElevateAndRegisterChrome(dist, chrome_exe, suffix, protocol);
   } else {
     // Admin rights are required to register capabilities before Windows 8.
     return false;
   }
 }
 
-bool ShellUtil::RemoveShortcut(ShellUtil::ShortcutLocation location,
-                               BrowserDistribution* dist,
-                               const base::FilePath& target_exe,
-                               ShellChange level,
-                               const string16* shortcut_name) {
-  const bool delete_folder = (location == SHORTCUT_LOCATION_START_MENU);
+// static
+bool ShellUtil::RemoveShortcuts(ShellUtil::ShortcutLocation location,
+                                BrowserDistribution* dist,
+                                ShellChange level,
+                                const base::FilePath& target_exe) {
+  switch (location) {
+    case SHORTCUT_LOCATION_START_MENU:  // Falls through.
+    case SHORTCUT_LOCATION_APP_SHORTCUTS:
+      return RemoveShortcutFolder(location, dist, level);
 
-  base::FilePath shortcut_folder;
-  if (!GetShortcutPath(location, dist, level, &shortcut_folder) ||
-      shortcut_folder.empty()) {
-    NOTREACHED();
-    return false;
-  }
+    case SHORTCUT_LOCATION_TASKBAR_PINS:
+      return BatchShortcutAction(base::Bind(&ShortcutOpUnpin), location, dist,
+                                 level, target_exe);
 
-  if (!delete_folder && !shortcut_name) {
-    file_util::FileEnumerator enumerator(shortcut_folder, false,
-        file_util::FileEnumerator::FILES);
-    bool had_failures = false;
-    for (base::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);
-  const base::FilePath shortcut_path(
-      shortcut_folder.Append(shortcut_base_name));
-  if (!file_util::PathExists(shortcut_path))
-    return true;
-
-  return MaybeRemoveShortcutAtPath(shortcut_path, target_exe, delete_folder);
-}
-
-void ShellUtil::RemoveTaskbarShortcuts(const string16& target_exe) {
-  if (base::win::GetVersion() < base::win::VERSION_WIN7)
-    return;
-
-  base::FilePath taskbar_pins_path;
-  if (!PathService::Get(base::DIR_TASKBAR_PINS, &taskbar_pins_path) ||
-      !file_util::PathExists(taskbar_pins_path)) {
-    LOG(ERROR) << "Couldn't find path to taskbar pins.";
-    return;
-  }
-
-  file_util::FileEnumerator shortcuts_enum(
-      taskbar_pins_path, false,
-      file_util::FileEnumerator::FILES, FILE_PATH_LITERAL("*.lnk"));
-
-  base::FilePath target_path(target_exe);
-  InstallUtil::ProgramCompare target_compare(target_path);
-  for (base::FilePath shortcut_path = shortcuts_enum.Next();
-       !shortcut_path.empty();
-       shortcut_path = shortcuts_enum.Next()) {
-    base::FilePath read_target;
-    if (!base::win::ResolveShortcut(shortcut_path, &read_target, NULL)) {
-      LOG(ERROR) << "Couldn't resolve shortcut at " << shortcut_path.value();
-      continue;
-    }
-    if (target_compare.EvaluatePath(read_target)) {
-      // Unpin this shortcut if it points to |target_exe|.
-      base::win::TaskbarUnpinShortcutLink(shortcut_path.value().c_str());
-    }
+    default:
+      return BatchShortcutAction(base::Bind(&ShortcutOpDelete), location, dist,
+                                 level, target_exe);
   }
 }
 
-void ShellUtil::RemoveStartScreenShortcuts(BrowserDistribution* dist,
-                                           const string16& target_exe) {
-  if (base::win::GetVersion() < base::win::VERSION_WIN8)
-    return;
-
-  base::FilePath app_shortcuts_path;
-  if (!PathService::Get(base::DIR_APP_SHORTCUTS, &app_shortcuts_path)) {
-    LOG(ERROR) << "Could not get application shortcuts location to delete"
-               << " start screen shortcuts.";
-    return;
-  }
-
-  app_shortcuts_path = app_shortcuts_path.Append(
-      GetBrowserModelId(dist,
-                        InstallUtil::IsPerUserInstall(target_exe.c_str())));
-  if (!file_util::DirectoryExists(app_shortcuts_path)) {
-    VLOG(1) << "No start screen shortcuts to delete.";
-    return;
-  }
-
-  VLOG(1) << "Removing start screen shortcuts from "
-          << app_shortcuts_path.value();
-  if (!file_util::Delete(app_shortcuts_path, true)) {
-    LOG(ERROR) << "Failed to remove start screen shortcuts from "
-               << app_shortcuts_path.value();
-  }
+// static
+bool ShellUtil::RetargetShortcuts(ShellUtil::ShortcutLocation location,
+                                  BrowserDistribution* dist,
+                                  ShellChange level,
+                                  const base::FilePath& old_target_exe,
+                                  const base::FilePath& new_target_exe) {
+  return BatchShortcutAction(base::Bind(&ShortcutOpSetTarget, new_target_exe),
+                             location, dist, level, old_target_exe);
 }
 
 bool ShellUtil::GetUserSpecificRegistrySuffix(string16* suffix) {
   // Use a thread-safe cache for the user's suffix.
   static base::LazyInstance<UserSpecificRegistrySuffix>::Leaky suffix_instance =
       LAZY_INSTANCE_INITIALIZER;
   return suffix_instance.Get().GetSuffix(suffix);
 }
 
 bool ShellUtil::GetOldUserSpecificRegistrySuffix(string16* suffix) {
@@ skipping 49 matching lines @@
     // are any left...).
     if (free_bits >= 8 && next_byte_index < size) {
       free_bits -= 8;
       bit_stream += bytes[next_byte_index++] << free_bits;
     }
   }
 
   DCHECK_EQ(ret.length(), encoded_length);
   return ret;
 }