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

chrome/installer/util/shell_util.h

Index: chrome/installer/util/shell_util.h
diff --git a/chrome/installer/util/shell_util.h b/chrome/installer/util/shell_util.h
index 538a42e2373cd2de891c3dc74b05f17c2a8073c1..bf2d40ae62fec206e01119f85a843d19d4a0ac38 100644
--- a/chrome/installer/util/shell_util.h
+++ b/chrome/installer/util/shell_util.h
@@ -15,15 +15,12 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/file_path.h"
+#include "base/logging.h"
 #include "base/string16.h"
 #include "chrome/installer/util/work_item_list.h"
 
 class BrowserDistribution;
-class FilePath;
-
-namespace base {
-class DictionaryValue;
-}
 
 // This is a utility class that provides common shell integration methods
 // that can be used by installer as well as Chrome.
@@ -35,12 +32,128 @@ class ShellUtil {
     SYSTEM_LEVEL = 0x2   // Make any shell changes only at the system level
   };
 
-  enum VerifyShortcutStatus {
-    VERIFY_SHORTCUT_SUCCESS = 0,
-    VERIFY_SHORTCUT_FAILURE_UNEXPECTED,
-    VERIFY_SHORTCUT_FAILURE_PATH,
-    VERIFY_SHORTCUT_FAILURE_DESCRIPTION,
-    VERIFY_SHORTCUT_FAILURE_ICON_INDEX,
+  // Typical shortcut directories. Resolved in GetShortcutPath().
+  enum ChromeShortcutLocation {
+    SHORTCUT_DESKTOP,
+    SHORTCUT_QUICK_LAUNCH,
+    SHORTCUT_START_MENU,
+  };
+
+  enum ChromeShortcutOperation {
+    // Create a new shortcut (overwriting if necessary).
+    SHORTCUT_CREATE_ALWAYS,
+    // Create the per-user shortcut only if its system-level equivalent is not
+    // present.
+    SHORTCUT_CREATE_IF_NO_SYSTEM_LEVEL,
+    // Overwrite an existing shortcut (fail if the shortcut doesn't exist).
+    SHORTCUT_REPLACE_EXISTING,
+    // Update specified properties only on an existing shortcut.
+    SHORTCUT_UPDATE_EXISTING,
+  };
+
+  // Properties for shortcuts to chrome.exe. Properties set will be applied to
+  // the shortcut on creation/update. On update, unset properties are ignored;
+  // on create (and replaced) unset properties might have a default value (see
+  // individual property setters below for details).
+  // Callers are encouraged to use the setters provided which take care of
+  // setting |options| as desired.
+  struct ChromeShortcutProperties {
+    enum ChromeIndividualProperties {
+      PROPERTIES_CHROME_EXE = 1 << 0,

[grt (UTC plus 2), 2012/10/04 14:31:22]

you could add inline has_foo methods to make checking for these easier for the
callers.  for example:
  bool has_chrome_exe() const {
    return options & PROPERTIES_CHROME_EXE;
  }
i think this can reduce wrapping (and need for braces) and increase clarity in a
number of places.

[gab, 2012/10/04 16:39:23]

Done.

+      PROPERTIES_ARGUMENTS = 1 << 1,
+      PROPERTIES_DESCRIPTION = 1 << 2,
+      PROPERTIES_ICON = 1 << 3,
+      PROPERTIES_APP_ID = 1 << 4,
+      PROPERTIES_SHORTCUT_NAME = 1 << 5,
+      PROPERTIES_DUAL_MODE = 1 << 6,
+    };
+
+    explicit ChromeShortcutProperties(ShellChange level_in)
+        : level(level_in), dual_mode(false), pin_to_taskbar(false),
+          options(0U) {}
+
+    // Sets the chrome.exe to launch from this shortcut. This is mandatory when
+    // creating a shortcut.
+    void set_chrome_exe(const FilePath& chrome_exe_in) {
+      chrome_exe = chrome_exe_in;
+      options |= PROPERTIES_CHROME_EXE;
+    }
+
+    // Sets the arguments to be passed to |chrome_exe| when launching from this
+    // shortcut.
+    // The length of this string must be less than MAX_PATH.
+    void set_arguments(const string16& arguments_in) {
+      // Size restriction as per MSDN at
+      // http://msdn.microsoft.com/library/windows/desktop/bb774954.aspx.
+      DCHECK(arguments_in.length() < MAX_PATH);
+      arguments = arguments_in;
+      options |= PROPERTIES_ARGUMENTS;
+    }
+
+    // Sets the localized description of the shortcut.
+    // Default: the current distribution's description.
+    // The length of this string must be less than MAX_PATH.
+    void set_description(const string16& description_in) {
+      // Size restriction as per MSDN at
+      // http://msdn.microsoft.com/library/windows/desktop/bb774955.aspx.
+      DCHECK(description_in.length() < MAX_PATH);
+      description = description_in;
+      options |= PROPERTIES_DESCRIPTION;
+    }
+
+    // Sets the path to the icon (icon_index set to 0).
+    // Default: chrome.exe (icon_index defaults to the current distribution's
+    // icon index unless otherwise specified in master_preferences).
+    void set_icon(const FilePath& icon_in) {
+      icon = icon_in;
+      options |= PROPERTIES_ICON;
+    }
+
+    // Sets the app model id for the shortcut (Win7+).
+    // Default: the browser model id for the current install.
+    void set_app_id(const string16& app_id_in) {
+      app_id = app_id_in;
+      options |= PROPERTIES_APP_ID;
+    }
+
+    // Forces the shortcut's name to |shortcut_name_in|.
+    // Default: the current distribution's GetAppShortcutName().
+    // The ".lnk" extension will automatically be added to this name.
+    void set_shortcut_name(const string16& shortcut_name_in) {
+      shortcut_name = shortcut_name_in;
+      options |= PROPERTIES_SHORTCUT_NAME;
+    }
+
+    // Sets whether this is a dual mode shortcut (Win8+).

[grt (UTC plus 2), 2012/10/04 14:31:22]

as discussed: please add to this comment some text explaining that this should
only be set for start menu shortcuts to Chrome the browser when it has the
default app id and no arguments.

[gab, 2012/10/04 16:39:23]

Done, also made a logic change to now only set dual mode on the Start menu
shortcut (as we discussed it doesn't make sense for any other shortcut than the
default Chrome shortcut to be dual mode; furthermore the hopes that setting it
on the Desktop shortcut would help make it a tile if pinned to Start turned out
to be wrong as the appid gets clobbered by Windows when pinned to Start so that
it doesn't become a tile anyways).

+    void set_dual_mode(bool dual_mode_in) {
+      dual_mode = dual_mode_in;
+      options |= PROPERTIES_DUAL_MODE;
+    }
+
+    // Sets whether to pin this shortcut to the taskbar after creating it
+    // (ignored if the shortcut is only being updated).
+    // Note: This property doesn't have a mask in |options|.
+    void set_pin_to_taskbar(bool pin_to_taskbar_in) {
+      pin_to_taskbar = pin_to_taskbar_in;
+    }
+
+    // The level to install this shortcut at (CURRENT_USER for a per-user
+    // shortcut and SYSTEM_LEVEL for an all-users shortcut).
+    ShellChange level;
+
+    FilePath chrome_exe;
+    string16 arguments;
+    string16 description;
+    FilePath icon;
+    string16 app_id;
+    string16 shortcut_name;
+    bool dual_mode;
+    bool pin_to_taskbar;
+    // Bitfield made of IndividualProperties. Properties set in |options| will
+    // be used to create/update the shortcut, others will be ignored on update
+    // and possibly replaced by default values on create (see individual
+    // property setters above for details on default values).
+    uint32 options;
   };
 
   // Relative path of the URL Protocol registry entry (prefixed with '\').
@@ -142,43 +255,25 @@ class ShellUtil {
                                             const string16& chrome_exe,
                                             const string16& suffix);
 
-  // Creates Chrome shortcut on the Desktop.
+  // Sets |path| to the path for a shortcut at the |location| desired for the
+  // given |level| (CURRENT_USER for per-user path and SYSTEM_LEVEL for
+  // all-users path).
+  // Returns false on failure.
+  static bool GetShortcutPath(ChromeShortcutLocation location,
+                              BrowserDistribution* dist,
+                              ShellChange level,
+                              FilePath* path);
+
+  // Updates Chrome shortcut in |location| (or creates it if |options| specify
+  // SHORTCUT_CREATE_ALWAYS).
   // |dist| gives the type of browser distribution currently in use.
-  // |chrome_exe| provides the target path information.
-  // |description| provides the shortcut's "comment" property.
-  // |appended_name| provides a string to be appended to the distribution name,
-  //     and can be the empty string.
-  // |arguments| gives a set of arguments to be passed to the executable.
-  // |icon_path| provides the path to the icon file to use.
-  // |icon_index| provides the index of the icon within the provided icon file.
-  // If |shell_change| is CURRENT_USER, the shortcut is created in the
-  //     Desktop folder of current user's profile.
-  // If |shell_change| is SYSTEM_LEVEL, the shortcut is created in the
-  //     Desktop folder of the "All Users" profile.
-  // |options|: bitfield for which the options come from ChromeShortcutOptions.
-  // Returns true iff the method causes a shortcut to be created / updated.
-  static bool 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);
-
-  // Create Chrome shortcut on Quick Launch Bar.
-  // If shell_change is CURRENT_USER, the shortcut is created in the
-  // Quick Launch folder of current user's profile.
-  // If shell_change is SYSTEM_LEVEL, the shortcut is created in the
-  // Quick Launch folder of "Default User" profile. This will make sure
-  // that this shortcut will be seen by all the new users logging into the
-  // system.
-  // |options|: bitfield for which the options come from ChromeShortcutOptions.
-  static bool CreateChromeQuickLaunchShortcut(BrowserDistribution* dist,
-                                              const string16& chrome_exe,
-                                              int shell_change,
-                                              uint32 options);
+  // |properties| and |operation| affect this method as described on their
+  // invidividual definitions above.
+  static bool CreateOrUpdateChromeShortcut(
+      ChromeShortcutLocation location,
+      BrowserDistribution* dist,
+      const ChromeShortcutProperties& properties,
+      ChromeShortcutOperation operation);
 
   // This method appends the Chrome icon index inside chrome.exe to the
   // chrome.exe path passed in as input, to generate the full path for
@@ -198,27 +293,6 @@ class ShellUtil {
   // |chrome_exe|: the full path to chrome.exe
   static string16 GetChromeDelegateCommand(const string16& chrome_exe);
 
-  // Returns the localized name of Chrome shortcut in |shortcut|. If
-  // |appended_name| is not empty, it is included in the shortcut name. If
-  // |alternate| is true, a second localized text that is better suited for
-  // certain scenarios is used.
-  static bool GetChromeShortcutName(BrowserDistribution* dist,
-                                    bool alternate,
-                                    const string16& appended_name,
-                                    string16* shortcut);
-
-  // Gets the desktop path for the current user or all users (if system_level
-  // is true) and returns it in 'path' argument. Return true if successful,
-  // otherwise returns false.
-  static bool GetDesktopPath(bool system_level, FilePath* path);
-
-  // Gets the Quick Launch shortcuts path for the current user and
-  // returns it in 'path' argument. Return true if successful, otherwise
-  // returns false. If system_level is true this function returns the path
-  // to Default Users Quick Launch shortcuts path. Adding a shortcut to Default
-  // User's profile only affects any new user profiles (not existing ones).
-  static bool GetQuickLaunchPath(bool system_level, FilePath* path);
-
   // Gets a mapping of all registered browser names (excluding browsers in the
   // |dist| distribution) and their reinstall command (which usually sets
   // browser as default).
@@ -378,64 +452,24 @@ class ShellUtil {
                                         const string16& protocol,
                                         bool elevate_if_not_admin);
 
-  // Remove Chrome shortcut from Desktop.
-  // If |shell_change| is CURRENT_USER, the shortcut is removed from the
-  // Desktop folder of current user's profile.
-  // If |shell_change| is SYSTEM_LEVEL, the shortcut is removed from the
-  // Desktop folder of "All Users" profile.
-  // |options|: bitfield for which the options come from ChromeShortcutOptions.
-  // Only SHORTCUT_ALTERNATE is a valid option for this function.
-  static bool RemoveChromeDesktopShortcut(BrowserDistribution* dist,
-                                          int shell_change,
-                                          uint32 options);
-
-  // Removes a set of existing Chrome desktop shortcuts. |appended_names| is a
-  // list of shortcut file names as obtained from
-  // ShellUtil::GetChromeShortcutName.
-  static bool RemoveChromeDesktopShortcutsWithAppendedNames(
-      const std::vector<string16>& appended_names);
-
-  // Remove Chrome shortcut from Quick Launch Bar.
-  // If shell_change is CURRENT_USER, the shortcut is removed from
-  // the Quick Launch folder of current user's profile.
-  // If shell_change is SYSTEM_LEVEL, the shortcut is removed from
-  // the Quick Launch folder of "Default User" profile.
-  static bool RemoveChromeQuickLaunchShortcut(BrowserDistribution* dist,
-                                              int shell_change);
+  // Removes installed Chrome shortcut at |location|.
+  // |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_START_MENU the shortcut folder specific to |dist|
+  // is deleted.
+  // Also attempts to unpin the removed shortcut from the taskbar.
+  // Returns true on success or if no shortcut is found at |location|.
+  static bool RemoveChromeShortcut(ChromeShortcutLocation location,
+                                   BrowserDistribution* dist,
+                                   ShellChange level,
+                                   const string16* shortcut_name);
 
   // This will remove all secondary tiles from the start screen for |dist|.
   static void RemoveChromeStartScreenShortcuts(BrowserDistribution* dist,
                                                const string16& chrome_exe);
 
-  enum ChromeShortcutOptions {
-    SHORTCUT_NO_OPTIONS = 0,
-    // Set DualMode property for Windows 8 Metro-enabled shortcuts.
-    SHORTCUT_DUAL_MODE = 1 << 0,
-    // Create a new shortcut (overwriting if necessary).
-    SHORTCUT_CREATE_ALWAYS = 1 << 1,
-    // Use an alternate application name for the shortcut (e.g. "The Internet").
-    // This option is only applied to the Desktop shortcut.
-    SHORTCUT_ALTERNATE = 1 << 2,
-  };
-
-  // Updates shortcut (or creates a new shortcut) at destination given by
-  // shortcut to a target given by chrome_exe. The arguments are given by
-  // |arguments| for the target and icon is set based on |icon_path| and
-  // |icon_index|. If create_new is set to true, the function will create a new
-  // shortcut if it doesn't exist.
-  // |options|: bitfield for which the options come from ChromeShortcutOptions.
-  // If SHORTCUT_CREATE_ALWAYS is not set in |options|, only specified (non-
-  // null) properties on an existing shortcut will be modified. If the shortcut
-  // does not exist, this method is a no-op and returns false.
-  static bool 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);
-
   // Sets |suffix| to the base 32 encoding of the md5 hash of this user's sid
   // preceded by a dot.
   // This is guaranteed to be unique on the machine and 27 characters long