Refactoring and tests for the highly undertested file_util::CreateOrUpdateShortcutLink() method.

base/file_util.h

Index: base/file_util.h
diff --git a/base/file_util.h b/base/file_util.h
index 66fe51c38dc17dbc013142ad44b307e76a3cbea0..2915f06950bc7860ee20663a1b82347b18490a02 100644
--- a/base/file_util.h
+++ b/base/file_util.h
@@ -226,12 +226,91 @@ BASE_EXPORT bool SetPosixFilePermissions(const FilePath& path,
 #endif  // defined(OS_POSIX)
 
 #if defined(OS_WIN)
-enum ShortcutOptions {
-  SHORTCUT_NO_OPTIONS = 0,
-  // Set DualMode property for Windows 8 Metro-enabled shortcuts.
-  SHORTCUT_DUAL_MODE = 1 << 0,
+enum ShortcutOperation {
   // Create a new shortcut (overwriting if necessary).
-  SHORTCUT_CREATE_ALWAYS = 1 << 1,
+  SHORTCUT_CREATE_ALWAYS = 0,
+  // Update specified (non-null) properties only on an existing shortcut.
+  SHORTCUT_UPDATE_EXISTING,
+};
+
+// Propoerties for shortcuts. Properties set will be applied to the shortcut

[robertshield, 2012/09/06 02:37:34]

nit: *Properties

[gab, 2012/09/06 04:20:02]

Done.

+// on creation/update, others will be ignored.
+// Callers are encouraged to use the setters provided which take care of
+// setting |bitfield| as desired.
+struct ShortcutProperties {
+  enum IndividualProperties {
+    PROPERTIES_TARGET = 1 << 0,
+    PROPERTIES_WORKING_DIR = 1 << 1,
+    PROPERTIES_ARGUMENTS = 1 << 2,
+    PROPERTIES_DESCRIPTION = 1 << 3,
+    PROPERTIES_ICON = 1 << 4,
+    PROPERTIES_APP_ID = 1 << 5,
+    PROPERTIES_DUAL_MODE = 1 << 6,
+  };
+
+  ShortcutProperties() : bitfield(0U) {}
+
+  void set_target(const string16& target_in) {
+    target  = target_in;
+    bitfield |= PROPERTIES_TARGET;
+  }
+
+  void set_working_dir(const string16& working_dir_in) {
+    working_dir  = working_dir_in;
+    bitfield |= PROPERTIES_WORKING_DIR;
+  }
+
+  void set_arguments(const string16& arguments_in) {
+    arguments  = arguments_in;
+    bitfield |= PROPERTIES_ARGUMENTS;
+  }
+
+  void set_description(const string16& description_in) {
+    description  = description_in;
+    bitfield |= PROPERTIES_DESCRIPTION;
+  }
+
+  void set_icon(const string16& icon_in, int icon_index_in) {
+    icon  = icon_in;
+    icon_index = icon_index_in;
+    bitfield |= PROPERTIES_ICON;
+  }
+
+  void set_app_id(const string16& app_id_in) {
+    app_id  = app_id_in;
+    bitfield |= PROPERTIES_APP_ID;
+  }
+
+  void set_dual_mode(bool dual_mode_in) {
+    dual_mode  = dual_mode_in;
+    bitfield |= PROPERTIES_DUAL_MODE;
+  }
+
+  // The target to launch from this shortcut. This is mandatory when creating
+  // a shortcut.
+  string16 target;
+  // The name of the working directory when launching the shortcut.
+  string16 working_dir;
+  // The arguments to be applied to |target| when launching from this shortcut.
+  // The length of this string must be less than MAX_PATH.
+  string16 arguments;
+  // The localized description of the shortcut.
+  // The length of this string must be less than MAX_PATH.
+  string16 description;
+  // The path to the icon (can be a dll or exe, in which case |icon_index| is
+  // the resource id).
+  string16 icon;
+  int icon_index;
+  // The app model id for the shortcut (Win7+).
+  string16 app_id;
+  // Whether this is a dual mode shortcut (Win8+).
+  // For now this property can only be set to true (i.e. once set it cannot be
+  // unset).
+  // TODO (gab): Make it possible to set this property to false.
+  bool dual_mode;
+  // Bitfield made of IndividualProperties. Properties set in |bitfield| will
+  // be set on the shortcut, others will be ignored.
+  uint32 bitfield;

[robertshield, 2012/09/06 02:37:34]

bitfield is a bad name for a member. Consider 'options' or similar. 

[gab, 2012/09/06 04:20:02]

Done.

 };
 
 // Resolve Windows shortcut (.LNK file)
@@ -246,28 +325,16 @@ BASE_EXPORT bool ResolveShortcut(const FilePath& shortcut_path,
                                  FilePath* target_path,
                                  string16* args);
 
-// Creates (or updates) a Windows shortcut (.LNK file)
-// This method creates (or updates) a shortcut link using the information given.
+// This method creates (or updates) a shortcut link at |shortcut_path| using the
+// information given through |properties|.
 // Ensure you have initialized COM before calling into this function.
-// |destination| is required. |source| is required when SHORTCUT_CREATE_ALWAYS
-// is specified in |options|. All other parameters are optional and may be NULL.
-// |source| is the existing file, |destination| is the new link file to be
-// created; for best results pass the filename with the .lnk extension.
-// The |icon| can specify a dll or exe in which case the icon index is the
-// resource id. |app_id| is the app model id for the shortcut on Win7.
-// |options|: bitfield for which the options come from ShortcutOptions.
-// 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
+// |operation|: a choice from the ShortcutOperation enum.
+// If |operation| is SHORTCUT_UPDATE_EXISTING| and |shortcut_path| does not

[robertshield, 2012/09/06 02:37:34]

nit: SHORTCUT_UPDATE_EXISTING (no |)

[gab, 2012/09/06 04:20:02]

Whoopsy!

 // exist, this method is a no-op and returns false.
-BASE_EXPORT bool CreateOrUpdateShortcutLink(const wchar_t *source,
-                                            const wchar_t *destination,
-                                            const wchar_t *working_dir,
-                                            const wchar_t *arguments,
-                                            const wchar_t *description,
-                                            const wchar_t *icon,
-                                            int icon_index,
-                                            const wchar_t* app_id,
-                                            uint32 options);
+BASE_EXPORT bool CreateOrUpdateShortcutLink(
+    const string16& shortcut_path,
+    const ShortcutProperties& properties,
+    ShortcutOperation operation);
 
 // Pins a shortcut to the Windows 7 taskbar. The shortcut file must already
 // exist and be a shortcut that points to an executable.