Refactor ShellUtil shortcut code -- single multi-purpose methods as opposed to many slighlty diffe…

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"
@@ skipping 28 matching lines @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/install_util.h"
 #include "chrome/installer/util/master_preferences.h"
 #include "chrome/installer/util/master_preferences_constants.h"
 
 using base::win::RegKey;
 
 namespace {
 
+typedef ShellUtil::ChromeShortcutProperties ChromeShortcutProperties;
+
 // An enum used to tell QuickIsChromeRegistered() which level of registration
 // the caller wants to confirm.
 enum RegistrationConfirmationLevel {
   // Only look for Chrome's ProgIds.
   // This is sufficient when we are trying to determine the suffix of the
   // currently running Chrome as shell integration registrations might not be
   // present.
   CONFIRM_PROGID_REGISTRATION = 0,
   // Confirm that Chrome is fully integrated with Windows (i.e. registered with
   // Defaut Programs). These registrations can be in HKCU as of Windows 8.
@@ skipping 871 matching lines @@
                                         &entries);
   // Change the default protocol handler for current user.
   if (!AddRegistryEntries(HKEY_CURRENT_USER, entries)) {
     LOG(ERROR) << "Could not make Chrome default protocol client (XP).";
     return false;
   }
 
   return true;
 }
 
+// Returns |properties.shortcut_name| if the property is set, otherwise it
+// returns dist->GetAppShortcutName(). In any case, it makes sure the
+// return value is suffixed with ".lnk".
+string16 ExtractShortcutNameFromProperties(
+    BrowserDistribution* dist,
+    const ChromeShortcutProperties& properties) {
+  DCHECK(dist);
+  string16 shortcut_name;
+  if (properties.options & ChromeShortcutProperties::PROPERTIES_SHORTCUT_NAME)
+    shortcut_name = properties.shortcut_name;
+  else
+    shortcut_name = dist->GetAppShortCutName();
+
+  static const wchar_t lnk_ext[] = L".lnk";

[grt (UTC plus 2), 2012/10/02 13:22:30]

although the style guide doesn't call it out, i think this constant should
either be kLnkExt at the top of this function, or kLnkExt at the top of the
namespace.  i prefer the former.

[gab, 2012/10/03 15:14:59]

Adding L".lnk" in a bunch of places has been bugging me throughout this CL,
finally added it to util_constants.h.

+  if (!EndsWith(shortcut_name, lnk_ext, false))
+    shortcut_name.append(lnk_ext);
+
+  return shortcut_name;
+}
+
+// Returns a ShortcutProperties struct containing the properties to set on the
+// shortcut based on the provided ChromeShortcutProperties. If |operation| is
+// to create, some properties (e.g. icon, app_id) might be given a default if
+// not set in |properties| (see individual ChromeShortcutProperties for
+// details).
+base::win::ShortcutProperties GetShortcutPropertiesFromChromeShortcutProperties(
+    BrowserDistribution* dist,
+    const ChromeShortcutProperties& properties,
+    base::win::ShortcutOperation operation) {
+  bool create = (operation == base::win::SHORTCUT_CREATE_ALWAYS ||
+                 operation == base::win::SHORTCUT_REPLACE_EXISTING);
+  // |target| is mandatory when creating the shortcut.
+  DCHECK(!create ||
+         (properties.options & ChromeShortcutProperties::PROPERTIES_TARGET));
+
+  base::win::ShortcutProperties shortcut_properties;
+
+  bool target_is_chrome_exe = false;

[grt (UTC plus 2), 2012/10/02 13:22:30]

i wonder if there's a better way to handle default values for Chrome.  let's
discuss in person.

[gab, 2012/10/03 15:14:59]

As discussed the target is now always chrome.exe when using
CreateOrUpdateChromeShortcut (the only call site not using chrome.exe, i.e.
uninstall link pointing to setup.exe now uses
base::win::CreateOrUpdateShortcutLink() directly).

The default values are still handled here (I considered making them part of the
constructor of ChromeShortcutProperties, but that would make the constructor
non-trivial).

+  if (properties.options & ChromeShortcutProperties::PROPERTIES_TARGET) {
+    shortcut_properties.set_target(properties.target);
+    DCHECK(!properties.target.DirName().empty());
+    shortcut_properties.set_working_dir(properties.target.DirName());
+
+    if (properties.target.BaseName() == FilePath(installer::kChromeExe))

[grt (UTC plus 2), 2012/10/02 13:22:30]

BaseName().value() == installer.kChromeExe to avoid an unnecessary temporary

[gab, 2012/10/03 15:14:59]

I also debated doing that and decided not to given we are really comparing
FilePaths which should know better how to compare paths than a pure
case-sensitive string comparison (as it is now FilePaths comparison is only
case-insensitive on the drive letter, but this could arguably be made better on
Windows if ever required).

I could make this a base::strcasecmp(properties.target.BaseName().value(),
installer::kChromeExe) == 0, but this implies knowledge of Windows paths'
case-insensitivity here and thus this is logic I believe belongs in
FilePath::operator==, not here.

I don't want to change FilePath::operator== as part of this CL, but am happy to
do so in another CL preceding this one.

+      target_is_chrome_exe = true;
+  }
+
+  if (properties.options & ChromeShortcutProperties::PROPERTIES_ARGUMENTS)
+    shortcut_properties.set_arguments(properties.arguments);
+
+  if (properties.options & ChromeShortcutProperties::PROPERTIES_DESCRIPTION)
+    shortcut_properties.set_description(properties.description);
+  else if (create && target_is_chrome_exe)
+    shortcut_properties.set_description(dist->GetAppDescription());
+
+  if (properties.options & ChromeShortcutProperties::PROPERTIES_ICON) {
+    shortcut_properties.set_icon(properties.icon, 0);

[grt (UTC plus 2), 2012/10/02 13:22:30]

the icon index can't be set by users of ChromeShortcutProperties?

[gab, 2012/10/03 15:14:59]

No current caller required it, i.e. either callers set a .ico file (and thus use
0 as an icon_index) or they would use dist->GetIconIndex() when the target was
chrome.exe (which, if icon_index was specified in master_preferences, would be
overriden by the master_prefs value in the old UpdateChromeShortcut() method).

Thus I feel adding it as a ChromeShortcutProperties just adds noise for a never
used scenario. With the current structure it's fairly easy to add it later if
required anyways.

+  } else if (create) {
+    // Set the icon to |properties.target| by default when creating a shortcut.
+    // If |properties.target| is chrome.exe, set |icon_index| as per the
+    // master_preferences if specified or as per the default icon index for this
+    // distribution.
+    int icon_index = 0;
+    if (target_is_chrome_exe) {
+      installer::MasterPreferences prefs(

[grt (UTC plus 2), 2012/10/02 13:22:30]

this scares me a little bit.  let's talk about it.

[gab, 2012/10/03 15:14:59]

Yea it's ugly, just copying the old behavior from the deleted
UpdateChromeShortcut() method (see end of this diff) for now.

Not sure why we would ever overwrite this in master_prefs..? Do some
distribution deals have their own icons? I would prefer always using
dist->GetIconIndex()...

+          properties.target.DirName().AppendASCII(
+              installer::kDefaultMasterPrefs));
+      if (!prefs.GetInt(installer::master_preferences::kChromeShortcutIconIndex,
+                        &icon_index)) {
+        icon_index = dist->GetIconIndex();
+      }
+    }
+    shortcut_properties.set_icon(properties.target, icon_index);
+  }
+
+  if (properties.options & ChromeShortcutProperties::PROPERTIES_APP_ID) {
+    shortcut_properties.set_app_id(properties.app_id);
+  } else if (create && target_is_chrome_exe) {
+    // Set the AppUserModelId to this distribution's AppId by default when
+    // creating the shortcut and the target is chrome.exe.
+    bool is_per_user_install =
+        InstallUtil::IsPerUserInstall(properties.target.value().c_str());
+    shortcut_properties.set_app_id(
+        ShellUtil::GetBrowserModelId(dist, is_per_user_install));
+  }
+
+  if (properties.options & ChromeShortcutProperties::PROPERTIES_DUAL_MODE)
+    shortcut_properties.set_dual_mode(properties.dual_mode);
+
+  return shortcut_properties;
+}
+
 }  // 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 35 matching lines @@
 const wchar_t* ShellUtil::kRegDelegateExecute = L"DelegateExecute";
 const wchar_t* ShellUtil::kRegOpenWithProgids = L"OpenWithProgids";
 
 bool ShellUtil::QuickIsChromeRegisteredInHKLM(BrowserDistribution* dist,
                                               const string16& chrome_exe,
                                               const string16& suffix) {
   return QuickIsChromeRegistered(dist, chrome_exe, suffix,
                                  CONFIRM_SHELL_REGISTRATION_IN_HKLM);
 }
 
-bool ShellUtil::CreateChromeDesktopShortcut(BrowserDistribution* dist,
-                                            const string16& chrome_exe,
-                                            const string16& description,
-                                            const string16& appended_name,
-                                            const string16& arguments,
-                                            const string16& icon_path,
-                                            int icon_index,
-                                            ShellChange shell_change,
-                                            uint32 options) {
-  string16 shortcut_name;
-  bool alternate = (options & ShellUtil::SHORTCUT_ALTERNATE) != 0;
-  if (!ShellUtil::GetChromeShortcutName(dist, alternate, appended_name,
-                                        &shortcut_name))
+bool ShellUtil::GetShortcutPath(ChromeShortcutLocation location,
+                                BrowserDistribution* dist,

[robertshield, 2012/10/02 13:08:04]

this doesn't use the dist parameter

[gab, 2012/10/03 15:14:59]

Oops, I added this with the intent of removing GetDistribution() on line 1120
below (done now).
I debated going either way, but figured that most callers already knew |dist|
and that GetDistribution() was a non-trivial getter so that it might as well get
passed in (on the other hand it's not always needed which would argue in favor
of keeping the GetDistribution() below and removing the |dist| parameter).

I'm fine either way... your thoughts sirs?!

+                                bool system_level,
+                                FilePath* path) {
+  int dir_key = -1;
+  bool add_folder_for_dist = false;
+  switch (location) {
+    case SHORTCUT_DESKTOP:
+      dir_key = system_level ? base::DIR_COMMON_DESKTOP :
+                               base::DIR_USER_DESKTOP;
+      break;
+    case SHORTCUT_QUICK_LAUNCH:
+      dir_key = system_level ? base::DIR_DEFAULT_USER_QUICK_LAUNCH :
+                               base::DIR_USER_QUICK_LAUNCH;
+      break;
+    case SHORTCUT_START_MENU:
+      dir_key = system_level ? base::DIR_COMMON_START_MENU :
+                               base::DIR_START_MENU;
+      add_folder_for_dist = true;
+      break;
+    default:
+      NOTREACHED();
+      return false;
+  }
+
+  if (dir_key == -1 || !PathService::Get(dir_key, path) || path->empty()) {
+    NOTREACHED() << dir_key;
     return false;
+  }
 
-  bool ret = false;
-  if (shell_change == ShellUtil::CURRENT_USER) {
-    FilePath shortcut_path;
-    // We do not want to create a desktop shortcut to Chrome in the current
-    // user's desktop folder if there is already one in the "All Users"
-    // desktop folder.
-    bool got_system_desktop = ShellUtil::GetDesktopPath(true, &shortcut_path);
-    FilePath shortcut = shortcut_path.Append(shortcut_name);
-    if (!got_system_desktop || !file_util::PathExists(shortcut)) {
-      // Either we couldn't query the "All Users" Desktop folder or there's
-      // nothing in it, so let's continue.
-      if (ShellUtil::GetDesktopPath(false, &shortcut_path)) {
-        shortcut = shortcut_path.Append(shortcut_name);
-        ret = ShellUtil::UpdateChromeShortcut(dist,
-                                              chrome_exe,
-                                              shortcut.value(),
-                                              arguments,
-                                              description,
-                                              icon_path,
-                                              icon_index,
-                                              options);
-      }
-    }
-  } else if (shell_change == ShellUtil::SYSTEM_LEVEL) {
-    FilePath shortcut_path;
-    if (ShellUtil::GetDesktopPath(true, &shortcut_path)) {
-      FilePath shortcut = shortcut_path.Append(shortcut_name);
-      ret = ShellUtil::UpdateChromeShortcut(dist,
-                                            chrome_exe,
-                                            shortcut.value(),
-                                            arguments,
-                                            description,
-                                            icon_path,
-                                            icon_index,
-                                            options);
-    }
-  } else {
-    NOTREACHED();
+  if (add_folder_for_dist) {
+    BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+    *path = path->Append(dist->GetAppShortCutName());
   }
-  return ret;
+
+  return true;
 }
 
-bool ShellUtil::CreateChromeQuickLaunchShortcut(BrowserDistribution* dist,
-                                                const string16& chrome_exe,
-                                                int shell_change,
-                                                uint32 options) {
-  string16 shortcut_name;
-  if (!ShellUtil::GetChromeShortcutName(dist, false, L"", &shortcut_name))
+bool ShellUtil::CreateOrUpdateChromeShortcut(
+    ShellUtil::ChromeShortcutLocation location,
+    BrowserDistribution* dist,
+    const ChromeShortcutProperties& properties,
+    ChromeShortcutOperation operation) {
+  DCHECK(dist);
+  // |pin_to_taskbar| is only acknowledged when first creating the shortcut.
+  DCHECK(!properties.pin_to_taskbar ||
+         operation == base::win::SHORTCUT_CREATE_ALWAYS);
+
+  FilePath user_shortcut_path;
+  FilePath system_shortcut_path;
+  if (!GetShortcutPath(location, dist, false, &user_shortcut_path) ||
+      !GetShortcutPath(location, dist, true, &system_shortcut_path) ||
+      user_shortcut_path.empty() ||
+      system_shortcut_path.empty()) {
+    NOTREACHED();
     return false;
+  }
 
-  bool ret = true;
-  // First create shortcut for the current user.
-  if (shell_change & ShellUtil::CURRENT_USER) {
-    FilePath user_ql_path;
-    if (ShellUtil::GetQuickLaunchPath(false, &user_ql_path)) {
-      user_ql_path = user_ql_path.Append(shortcut_name);
-      ret = ShellUtil::UpdateChromeShortcut(dist, chrome_exe,
-                                            user_ql_path.value(),
-                                            L"", L"", chrome_exe,
-                                            dist->GetIconIndex(),
-                                            options);
-    } else {
-      ret = false;
+  string16 shortcut_name(ExtractShortcutNameFromProperties(dist, properties));
+  user_shortcut_path = user_shortcut_path.Append(shortcut_name);
+  system_shortcut_path = system_shortcut_path.Append(shortcut_name);
+
+  FilePath *chosen_path;
+  if (properties.system_level) {
+    // Install the system-level shortcut if requested.
+    chosen_path = &system_shortcut_path;
+  } else if (operation != 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.
+    chosen_path = &user_shortcut_path;
+  } else {
+    // Do not install anything if we are told to install a user-level shortcut,
+    // but the system-level variant of that shortcut is present.
+    chosen_path = &FilePath();
+  }
+
+  // No shortcut needs to be created/updated.
+  if (chosen_path->empty())
+    return true;
+
+  // Make sure the parent directories exist when creating the shortcut.
+  if (operation == SHORTCUT_CREATE_ALWAYS &&
+      !file_util::CreateDirectory(chosen_path->DirName())) {
+    NOTREACHED();
+    return false;
+  }
+
+  base::win::ShortcutOperation shortcut_operation =
+      (operation == SHORTCUT_UPDATE_EXISTING ?
+           base::win::SHORTCUT_UPDATE_EXISTING :
+           (operation == SHORTCUT_REPLACE_EXISTING ?
+                base::win::SHORTCUT_REPLACE_EXISTING :
+                base::win::SHORTCUT_CREATE_ALWAYS));
+  base::win::ShortcutProperties shortcut_properties(
+      GetShortcutPropertiesFromChromeShortcutProperties(dist, properties,
+                                                        shortcut_operation));
+  bool ret = base::win::CreateOrUpdateShortcutLink(
+      *chosen_path, shortcut_properties, shortcut_operation);
+
+  if (ret && operation == SHORTCUT_CREATE_ALWAYS && properties.pin_to_taskbar &&
+      base::win::GetVersion() >= base::win::VERSION_WIN7) {
+    ret = base::win::TaskbarPinShortcutLink(chosen_path->value().c_str());
+    if (!ret) {
+      LOG(ERROR) << "The shorcut at " << chosen_path->value()
+                 << " was created, but the taskbar pin failed.";
     }
   }
 
-  // Add a shortcut to Default User's profile so that all new user profiles
-  // get it.
-  if (shell_change & ShellUtil::SYSTEM_LEVEL) {
-    FilePath default_ql_path;
-    if (ShellUtil::GetQuickLaunchPath(true, &default_ql_path)) {
-      default_ql_path = default_ql_path.Append(shortcut_name);
-      ret = ShellUtil::UpdateChromeShortcut(dist, chrome_exe,
-                                            default_ql_path.value(),
-                                            L"", L"", chrome_exe,
-                                            dist->GetIconIndex(),
-                                            options) && ret;
-    } else {
-      ret = false;
-    }
-  }
-
   return ret;
 }
 
 string16 ShellUtil::GetChromeIcon(BrowserDistribution* dist,
                                   const string16& chrome_exe) {
   string16 chrome_icon(chrome_exe);
   chrome_icon.append(L",");
   chrome_icon.append(base::IntToString16(dist->GetIconIndex()));
   return chrome_icon;
 }
 
 string16 ShellUtil::GetChromeShellOpenCmd(const string16& chrome_exe) {
   return L"\"" + chrome_exe + L"\" -- \"%1\"";
 }
 
 string16 ShellUtil::GetChromeDelegateCommand(const string16& chrome_exe) {
   return L"\"" + chrome_exe + L"\" -- %*";
 }
 
-bool ShellUtil::GetChromeShortcutName(BrowserDistribution* dist,
-                                      bool alternate,
-                                      const string16& appended_name,
-                                      string16* shortcut) {
-  shortcut->assign(alternate ? dist->GetAlternateApplicationName() :
-                               dist->GetAppShortCutName());
-  if (!appended_name.empty()) {
-    shortcut->append(L" (");
-    shortcut->append(appended_name);
-    shortcut->append(L")");
-  }
-  shortcut->append(L".lnk");
-  return true;
-}
-
-bool ShellUtil::GetDesktopPath(bool system_level, FilePath* path) {
-  int dir_key = system_level ? base::DIR_COMMON_DESKTOP :
-                               base::DIR_USER_DESKTOP;
-  return PathService::Get(dir_key, path);
-}
-
-bool ShellUtil::GetQuickLaunchPath(bool system_level, FilePath* path) {
-  int dir_key = system_level ? base::DIR_DEFAULT_USER_QUICK_LAUNCH :
-                               base::DIR_USER_QUICK_LAUNCH;
-  return PathService::Get(dir_key, path);
-}
-
 void ShellUtil::GetRegisteredBrowsers(
     BrowserDistribution* dist,
     std::map<string16, string16>* browsers) {
   DCHECK(dist);
   DCHECK(browsers);
 
   const string16 base_key(ShellUtil::kRegStartMenuInternet);
   string16 client_path;
   RegKey key;
   string16 name;
@@ skipping 396 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::RemoveChromeDesktopShortcut(BrowserDistribution* dist,
-                                            int shell_change, uint32 options) {
-  // Only SHORTCUT_ALTERNATE is a valid option for this function.
-  DCHECK(!options || options == ShellUtil::SHORTCUT_ALTERNATE);
+bool ShellUtil::RemoveChromeShortcut(
+    ChromeShortcutLocation location,
+    BrowserDistribution* dist,
+    const ChromeShortcutProperties& properties) {
+  bool delete_folder = (location == SHORTCUT_START_MENU);
 
-  string16 shortcut_name;
-  bool alternate = (options & ShellUtil::SHORTCUT_ALTERNATE) != 0;
-  if (!ShellUtil::GetChromeShortcutName(dist, alternate, L"",
-                                        &shortcut_name))
+  FilePath shortcut_folder;
+  if (!GetShortcutPath(location, dist, properties.system_level,
+                       &shortcut_folder) ||
+      shortcut_folder.empty()) {
+    NOTREACHED();
     return false;
-
-  bool ret = true;
-  if (shell_change & ShellUtil::CURRENT_USER) {
-    FilePath shortcut_path;
-    if (ShellUtil::GetDesktopPath(false, &shortcut_path)) {
-      FilePath shortcut = shortcut_path.Append(shortcut_name);
-      ret = file_util::Delete(shortcut, false);
-    } else {
-      ret = false;
-    }
   }
 
-  if (shell_change & ShellUtil::SYSTEM_LEVEL) {
-    FilePath shortcut_path;
-    if (ShellUtil::GetDesktopPath(true, &shortcut_path)) {
-      FilePath shortcut = shortcut_path.Append(shortcut_name);
-      ret = file_util::Delete(shortcut, false) && ret;
-    } else {
-      ret = false;
-    }
-  }
-  return ret;
-}
+  FilePath shortcut_path(shortcut_folder.Append(
+      ExtractShortcutNameFromProperties(dist, properties)));
 
-bool ShellUtil::RemoveChromeDesktopShortcutsWithAppendedNames(
-    const std::vector<string16>& appended_names) {
-  FilePath shortcut_path;
-  bool ret = true;
-  if (ShellUtil::GetDesktopPath(false, &shortcut_path)) {
-    for (std::vector<string16>::const_iterator it =
-             appended_names.begin();
-         it != appended_names.end();
-         ++it) {
-      FilePath delete_shortcut = shortcut_path.Append(*it);
-      ret = ret && file_util::Delete(delete_shortcut, false);
-    }
-  } else {
-    ret = false;
-  }
-  return ret;
-}
+  // 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).";
 
-bool ShellUtil::RemoveChromeQuickLaunchShortcut(BrowserDistribution* dist,
-                                                int shell_change) {
-  string16 shortcut_name;
-  if (!ShellUtil::GetChromeShortcutName(dist, false, L"", &shortcut_name))
-    return false;
-
-  bool ret = true;
-  // First remove shortcut for the current user.
-  if (shell_change & ShellUtil::CURRENT_USER) {
-    FilePath user_ql_path;
-    if (ShellUtil::GetQuickLaunchPath(false, &user_ql_path)) {
-      user_ql_path = user_ql_path.Append(shortcut_name);
-      ret = file_util::Delete(user_ql_path, false);
-    } else {
-      ret = false;
-    }
-  }
-
-  // Delete shortcut in Default User's profile
-  if (shell_change & ShellUtil::SYSTEM_LEVEL) {
-    FilePath default_ql_path;
-    if (ShellUtil::GetQuickLaunchPath(true, &default_ql_path)) {
-      default_ql_path = default_ql_path.Append(shortcut_name);
-      ret = file_util::Delete(default_ql_path, false) && ret;
-    } else {
-      ret = false;
-    }
-  }
-
-  return ret;
+  if (delete_folder)
+    return file_util::Delete(shortcut_folder, true);
+  else
+    return file_util::Delete(shortcut_path, false);
 }
 
 void ShellUtil::RemoveChromeStartScreenShortcuts(BrowserDistribution* dist,
                                                  const string16& chrome_exe) {
   if (base::win::GetVersion() < base::win::VERSION_WIN8)
     return;
 
   FilePath app_shortcuts_path;
   if (!PathService::Get(base::DIR_APP_SHORTCUTS, &app_shortcuts_path)) {
     LOG(ERROR) << "Could not get application shortcuts location to delete"
@@ skipping 10 matching lines @@
   }
 
   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();
   }
 }
 
-bool ShellUtil::UpdateChromeShortcut(BrowserDistribution* dist,
-                                     const string16& chrome_exe,
-                                     const string16& shortcut,
-                                     const string16& arguments,
-                                     const string16& description,
-                                     const string16& icon_path,
-                                     int icon_index,
-                                     uint32 options) {
-  const FilePath chrome_path(FilePath(chrome_exe).DirName());
-
-  installer::MasterPreferences prefs(
-      chrome_path.AppendASCII(installer::kDefaultMasterPrefs));
-  if (FilePath::CompareEqualIgnoreCase(icon_path, chrome_exe)) {
-    prefs.GetInt(installer::master_preferences::kChromeShortcutIconIndex,
-                 &icon_index);
-  }
-
-  const string16 app_id(
-      GetBrowserModelId(dist,
-                        InstallUtil::IsPerUserInstall(chrome_exe.c_str())));
-  const bool is_dual_mode = ((options & ShellUtil::SHORTCUT_DUAL_MODE) != 0);
-  const base::win::ShortcutOperation operation =
-      (options & ShellUtil::SHORTCUT_CREATE_ALWAYS) != 0 ?
-          base::win::SHORTCUT_CREATE_ALWAYS :
-          base::win::SHORTCUT_UPDATE_EXISTING;
-
-  // TODO(gab): The shell_util interface will also be refactored in an upcoming
-  // CL to use a ShortcutProperties like interface for its shortcut methods.
-  base::win::ShortcutProperties shortcut_properties;
-  shortcut_properties.set_target(FilePath(chrome_exe));
-  shortcut_properties.set_working_dir(chrome_path);
-  shortcut_properties.set_arguments(arguments);
-  shortcut_properties.set_description(description);
-  shortcut_properties.set_icon(FilePath(icon_path), icon_index);
-  shortcut_properties.set_app_id(app_id);
-  shortcut_properties.set_dual_mode(is_dual_mode);
-  return base::win::CreateOrUpdateShortcutLink(
-      FilePath(shortcut), shortcut_properties, operation);
-}
-
 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) {
   wchar_t user_name[256];
   DWORD size = arraysize(user_name);
@@ skipping 47 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;
 }