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

base/file_util_win.cc

Index: base/file_util_win.cc
diff --git a/base/file_util_win.cc b/base/file_util_win.cc
index b88e13169e8993db2ba2ebb48bd1090cbf355c3f..d8fdd28897bbb5f5664a1ffbef71e1793f28d05d 100644
--- a/base/file_util_win.cc
+++ b/base/file_util_win.cc
@@ -381,73 +381,84 @@ bool ResolveShortcut(const FilePath& shortcut_path,
   return true;
 }
 
-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) {
+bool CreateOrUpdateShortcutLink(const string16& shortcut_path,
+                                const ShortcutProperties& properties,
+                                ShortcutOperation operation) {
   base::ThreadRestrictions::AssertIOAllowed();
 
-  bool create = (options & SHORTCUT_CREATE_ALWAYS) != 0;
-
-  // |source| is required when SHORTCUT_CREATE_ALWAYS is specified.
-  DCHECK(source || !create);
-
-  // Length of arguments and description must be less than MAX_PATH.
-  DCHECK(lstrlen(arguments) < MAX_PATH);
-  DCHECK(lstrlen(description) < MAX_PATH);
+  // A target is required when |operation| is SHORTCUT_CREATE_ALWAYS.
+  if (operation == SHORTCUT_CREATE_ALWAYS &&
+      !(properties.options & ShortcutProperties::PROPERTIES_TARGET)) {
+    NOTREACHED();
+    return false;
+  }
 
   base::win::ScopedComPtr<IShellLink> i_shell_link;
   base::win::ScopedComPtr<IPersistFile> i_persist_file;
-
-  // Get pointer to the IShellLink interface.
   if (FAILED(i_shell_link.CreateInstance(CLSID_ShellLink, NULL,
                                          CLSCTX_INPROC_SERVER)) ||
       FAILED(i_persist_file.QueryFrom(i_shell_link))) {
     return false;
   }
 
-  if (!create && FAILED(i_persist_file->Load(destination, STGM_READWRITE)))
+  if (operation == SHORTCUT_UPDATE_EXISTING &&
+      FAILED(i_persist_file->Load(shortcut_path.c_str(), STGM_READWRITE))) {
     return false;
+  }
 
-  if ((source || create) && FAILED(i_shell_link->SetPath(source)))
+  if ((properties.options & ShortcutProperties::PROPERTIES_TARGET) &&
+      FAILED(i_shell_link->SetPath(properties.target.c_str()))) {
     return false;
+  }
 
-  if (working_dir && FAILED(i_shell_link->SetWorkingDirectory(working_dir)))
+  if ((properties.options & ShortcutProperties::PROPERTIES_WORKING_DIR) &&
+      FAILED(i_shell_link->SetWorkingDirectory(
+          properties.working_dir.c_str()))) {
     return false;
+  }
 
-  if (arguments && FAILED(i_shell_link->SetArguments(arguments)))
+  if ((properties.options & ShortcutProperties::PROPERTIES_ARGUMENTS) &&
+      FAILED(i_shell_link->SetArguments(properties.arguments.c_str()))) {
     return false;
+  }
 
-  if (description && FAILED(i_shell_link->SetDescription(description)))
+  if ((properties.options & ShortcutProperties::PROPERTIES_DESCRIPTION) &&
+      FAILED(i_shell_link->SetDescription(properties.description.c_str()))) {
     return false;
+  }
 
-  if (icon && FAILED(i_shell_link->SetIconLocation(icon, icon_index)))
+  if ((properties.options & ShortcutProperties::PROPERTIES_ICON) &&
+      FAILED(i_shell_link->SetIconLocation(properties.icon.c_str(),
+                                           properties.icon_index))) {
     return false;
+  }
 
-  bool is_dual_mode = (options & SHORTCUT_DUAL_MODE) != 0;
-  if ((app_id || is_dual_mode) &&
+  const bool has_app_id =
+      (properties.options & ShortcutProperties::PROPERTIES_APP_ID) != 0;
+  const bool has_dual_mode =
+      (properties.options & ShortcutProperties::PROPERTIES_DUAL_MODE) != 0;
+  if ((has_app_id || has_dual_mode) &&
       base::win::GetVersion() >= base::win::VERSION_WIN7) {
     base::win::ScopedComPtr<IPropertyStore> property_store;
     if (FAILED(property_store.QueryFrom(i_shell_link)) || !property_store.get())
       return false;
 
-    if (app_id && !base::win::SetAppIdForPropertyStore(property_store, app_id))
+    if (has_app_id &&
+        !base::win::SetAppIdForPropertyStore(property_store,
+                                             properties.app_id.c_str())) {
       return false;
-    if (is_dual_mode &&
-        !base::win::SetDualModeForPropertyStore(property_store)) {
+    }
+    if (has_dual_mode &&
+        !base::win::SetDualModeForPropertyStore(property_store,
+                                                properties.dual_mode)) {
       return false;
     }
   }
 
-  HRESULT result = i_persist_file->Save(destination, TRUE);
+  HRESULT result = i_persist_file->Save(shortcut_path.c_str(), TRUE);
 
   // If we successfully updated the icon, notify the shell that we have done so.
-  if (!create && SUCCEEDED(result)) {
+  if (operation == SHORTCUT_UPDATE_EXISTING && SUCCEEDED(result)) {
     // Release the interfaces in case the SHChangeNotify call below depends on
     // the operations above being fully completed.
     i_persist_file.Release();