Installer: adding ResolveShortcutProperties(); updating shortcut icons during shortcut migration.

base/win/shortcut.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.
 
 #include "base/win/shortcut.h"
 
 #include <shellapi.h>
 #include <shlobj.h>
 #include <propkey.h>
 
@@ skipping 154 matching lines @@
       SHChangeNotify(SHCNE_ASSOCCHANGED, SHCNF_IDLIST, NULL, NULL);
     } else {
       SHChangeNotify(SHCNE_CREATE, SHCNF_PATH, shortcut_path.value().c_str(),
                      NULL);
     }
   }
 
   return succeeded;
 }
 
-bool ResolveShortcut(const FilePath& shortcut_path,
-                     FilePath* target_path,
-                     string16* args) {
+bool ResolveShortcutProperties(const FilePath& shortcut_path,
+                               uint32 options,
+                               ShortcutProperties* p) {

[gab, 2013/12/23 14:25:01]

s/p/properties

Chromium doesn't encourage abbreviated variable names.

[huangs, 2013/12/30 20:15:10]

Done (also .h and comments).

+  DCHECK(p != NULL);

[gab, 2013/12/23 14:25:01]

DCHECK(properties);

(i.e. no need for "!= NULL")

Also DCHECK(options); otherwise it's kind of pointless to call this (and
expensive as it loads the shortcut anyways).

[huangs, 2013/12/30 20:15:10]

Done.

   base::ThreadRestrictions::AssertIOAllowed();
 
   HRESULT result;
   ScopedComPtr<IShellLink> i_shell_link;
 
   // Get pointer to the IShellLink interface.
   result = i_shell_link.CreateInstance(CLSID_ShellLink, NULL,
                                        CLSCTX_INPROC_SERVER);
   if (FAILED(result))
     return false;
 
   ScopedComPtr<IPersistFile> persist;
   // Query IShellLink for the IPersistFile interface.
   result = persist.QueryFrom(i_shell_link);
   if (FAILED(result))
     return false;
 
   // Load the shell link.
   result = persist->Load(shortcut_path.value().c_str(), STGM_READ);
   if (FAILED(result))
     return false;
 
+  p->options = 0;
   WCHAR temp[MAX_PATH];
-  if (target_path) {
+  if (options & ShortcutProperties::PROPERTIES_TARGET) {
     // Try to find the target of a shortcut.
     result = i_shell_link->Resolve(0, SLR_NO_UI | SLR_NOSEARCH);
     if (FAILED(result))
       return false;
 
     result = i_shell_link->GetPath(temp, MAX_PATH, NULL, SLGP_UNCPRIORITY);
     if (FAILED(result))
       return false;
 
-    *target_path = FilePath(temp);
+    p->set_target(FilePath(temp));
   }
 
-  if (args) {
+  if (options & ShortcutProperties::PROPERTIES_ARGUMENTS) {
     result = i_shell_link->GetArguments(temp, MAX_PATH);

[gab, 2013/12/23 14:25:01]

Looks like |result| is a write-only variable here and above.

Consider inlining all the calls down into the FAILED() blocks like you did in
your new code below, I find this cleaner.

[huangs, 2013/12/30 20:15:10]

Done.

     if (FAILED(result))
       return false;
 
-    *args = string16(temp);
+    p->set_arguments(string16(temp));
   }
+
+  if (options & ShortcutProperties::PROPERTIES_ICON) {
+    int temp_index;
+    if (FAILED(i_shell_link->GetIconLocation(temp, MAX_PATH, &temp_index)))
+      return false;
+
+    p->set_icon(FilePath(temp), temp_index);
+  }
+
+  // Implement other options if the need arise.
+
   return true;
 }
 
+bool ResolveShortcut(const FilePath& shortcut_path,
+                     FilePath* target_path,
+                     string16* args) {
+  uint32 options = 0;
+  options |= target_path ? ShortcutProperties::PROPERTIES_TARGET : 0;
+  options |= args ? ShortcutProperties::PROPERTIES_ARGUMENTS : 0;

[gab, 2013/12/23 14:25:01]

Change these two lines to:

if (target_path)
  options |= ShortcutProperties::PROPERTIES_TARGET;
if (args)
  options |= ShortcutProperties::PROPERTIES_ARGUMENT;

Although this results in the same thing I prefer this to OR'ing with 0.

[huangs, 2013/12/30 20:15:10]

Done. Also, being defensive: return true if options == 0, since the old
behaviour did not forbid target_path == NULL and args == NULL, but we
ResolveShortcutProperties() now has DCHECK.

+  ShortcutProperties p;
+  if (!ResolveShortcutProperties(shortcut_path, options, &p))
+    return false;
+
+  if (target_path)
+    *target_path = p.target;
+  if (args)
+    *args = p.arguments;
+  return true;
+}
+
 bool TaskbarPinShortcutLink(const wchar_t* shortcut) {
   base::ThreadRestrictions::AssertIOAllowed();
 
   // "Pin to taskbar" is only supported after Win7.
   if (GetVersion() < VERSION_WIN7)
     return false;
 
   int result = reinterpret_cast<int>(ShellExecute(NULL, L"taskbarpin", shortcut,
       NULL, NULL, 0));
   return result > 32;
 }
 
 bool TaskbarUnpinShortcutLink(const wchar_t* shortcut) {
   base::ThreadRestrictions::AssertIOAllowed();
 
   // "Unpin from taskbar" is only supported after Win7.
   if (base::win::GetVersion() < base::win::VERSION_WIN7)
     return false;
 
   int result = reinterpret_cast<int>(ShellExecute(NULL, L"taskbarunpin",
       shortcut, NULL, NULL, 0));
   return result > 32;
 }
 
 }  // namespace win
 }  // namespace base