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

base/file_util.h

 // 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 contains utility functions for dealing with the local
 // filesystem.
 
 #ifndef BASE_FILE_UTIL_H_
 #define BASE_FILE_UTIL_H_
 
@@ skipping 208 matching lines @@
 // a file which the symlink points to.
 BASE_EXPORT bool GetPosixFilePermissions(const FilePath& path,
                                          int* mode);
 // Sets the permission of the given |path|. If |path| is symbolic link, sets
 // the permission of a file which the symlink points to.
 BASE_EXPORT bool SetPosixFilePermissions(const FilePath& path,
                                          int mode);
 #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)
 // This methods tries to resolve a shortcut .LNK file. The path of the shortcut
 // to resolve is in |shortcut_path|. If |target_path| is not NULL, the target
 // will be resolved and placed in |target_path|. If |args| is not NULL, the
 // arguments will be retrieved and placed in |args|. The function returns true
 // if all requested fields are are found successfully.
 // Callers can safely use the same variable for both |shortcut_path| and
 // |target_path|.
 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.
 BASE_EXPORT bool TaskbarPinShortcutLink(const wchar_t* shortcut);
 
 // Unpins a shortcut from the Windows 7 taskbar. The shortcut must exist and
 // already be pinned to the taskbar.
 BASE_EXPORT bool TaskbarUnpinShortcutLink(const wchar_t* shortcut);
 
 // Copy from_path to to_path recursively and then delete from_path recursively.
@@ skipping 390 matching lines @@
 };
 
 // Attempts determine the FileSystemType for |path|.
 // Returns false if |path| doesn't exist.
 BASE_EXPORT bool GetFileSystemType(const FilePath& path, FileSystemType* type);
 #endif
 
 }  // namespace file_util
 
 #endif  // BASE_FILE_UTIL_H_