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

chrome/installer/util/shell_util.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.
 //
 // This file defines functions that integrate Chrome in Windows shell. These
 // functions can be used by Chrome as well as Chrome installer. All of the
 // work is done by the local functions defined in anonymous namespace in
 // this class.
 
 #include "chrome/installer/util/shell_util.h"
@@ skipping 1253 matching lines @@
 }
 
 bool ShortcutOpUpdate(const base::win::ShortcutProperties& shortcut_properties,
                       const base::FilePath& shortcut_path) {
   bool ret = base::win::CreateOrUpdateShortcutLink(
       shortcut_path, shortcut_properties, base::win::SHORTCUT_UPDATE_EXISTING);
   LOG_IF(ERROR, !ret) << "Failed to update " << shortcut_path.value();
   return ret;
 }
 
+bool ShortcutOpRetarget(const base::FilePath& old_target_path,
+                        const base::FilePath& new_target_path,
+                        const base::FilePath& shortcut_path) {
+  base::win::ShortcutProperties old_prop;
+  if (base::win::ResolveShortcutProperties(shortcut_path,

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

Wrap |shortcut_path| as well.

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

Unless you strongly oppose, I don't think we should stop retargeting if getting
the old icon fails; i.e. retarget the exe just not the icon in that case.

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

Done, but |old_prop| now gets wrapped, too, so now 1 param per line.

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

Makes sense. Restructured code a bit.

+          base::win::ShortcutProperties::PROPERTIES_ICON, &old_prop)) {

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

Both the ShortcutFilter and this action will end up resolving the same shortcut
back to back, one for the args another for the icon :(.

Just pointing this out as it's an unfortunate side-effect of the current design,
but I don't think there is a clean way to solve this (and it's not worth making
a hacky way to solve it unless we could prove that this is indeed way too slow
in practice -- which I doubt matters).

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

Yeah I felt a bit dirty about this. :)

I think we should restructure BatchShortcutAction() by separating it into 2
parts:
(1) Routine to resolve a list of shortcuts with filter, storing it into a vector
(with all info).
(2) Routine to take vector of shortcut, and apply batch operations. 

This probably should not be done in this CL. Adding TODO to
BatchShortcutAction().

+    base::win::ShortcutProperties new_prop;
+    new_prop.set_target(new_target_path);
+    if (old_prop.icon == old_target_path)

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

tl;dr; use InstallUtil::ProgramCompare here.

String comparison of paths isn't reliable (on Windows you at least want to
compare FilePaths to deal with case-insensitiveness); in this case however
you'll want to use InstallUtil::ProgramCompare which can deal with the strings
not matching yet this being the same path (i.e., imagine the IShellLink
interface returns an 8.3 path to the shortcut and |new_target_path| is the full
path).

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

Ah yes. Done.

+      new_prop.set_icon(new_target_path, old_prop.icon_index);
+    if (base::win::CreateOrUpdateShortcutLink(
+          shortcut_path, new_prop, base::win::SHORTCUT_UPDATE_EXISTING)) {
+      return true;
+    }
+  }
+  LOG(ERROR) << "Failed to retarget " << shortcut_path.value();

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

Rather than putting a generic ERROR at the bottom, prefer the following pattern:

if (!first_thing) {
  LOG(ERROR) << "first thing failed";
  return false;
}

....


if (!second_thing) {
  LOG(ERROR) << "second thing failed";
  return false;
}

return true;


It avoids nesting and provides a clear ERROR case for each failure case.

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

There is only 1 error now, though I'm adding warning for the case where we fail
to resolve the icon.

+  return false;
+}
+
 // {|location|, |dist|, |level|} determine |shortcut_folder|.
 // For each shortcut in |shortcut_folder| that match |shortcut_filter|, apply
 // |shortcut_operation|. Returns true if all operations are successful.
 // All intended operations are attempted, even if failures occur.
 bool BatchShortcutAction(const ShortcutFilterCallback& shortcut_filter,
                          const ShortcutOperationCallback& shortcut_operation,
                          ShellUtil::ShortcutLocation location,
                          BrowserDistribution* dist,
                          ShellUtil::ShellChange level) {
   DCHECK(!shortcut_operation.is_null());
@@ skipping 816 matching lines @@
   if (!ShellUtil::ShortcutLocationIsSupported(location))
     return true;  // Vacuous success.
 
   FilterTargetEq shortcut_filter(target_exe, true);
   ShortcutOperationCallback shortcut_operation(
       base::Bind(&ShortcutOpUpdate, TranslateShortcutProperties(properties)));
   return BatchShortcutAction(shortcut_filter.AsShortcutFilterCallback(),
                              shortcut_operation, location, dist, level);
 }
 
+// static
+bool ShellUtil::RetargetShortcutsWithArgs(
+    ShellUtil::ShortcutLocation location,
+    BrowserDistribution* dist,
+    ShellChange level,
+    const base::FilePath& old_target_exe,
+    const base::FilePath& new_target_exe) {
+  if (!ShellUtil::ShortcutLocationIsSupported(location))
+    return true;  // Vacuous success.
+
+  FilterTargetEq shortcut_filter(old_target_exe, true);
+  ShortcutOperationCallback shortcut_operation(
+      base::Bind(&ShortcutOpRetarget, old_target_exe, new_target_exe));
+  return BatchShortcutAction(shortcut_filter.AsShortcutFilterCallback(),
+                             shortcut_operation, location, dist, level);
+}
+
 bool ShellUtil::GetUserSpecificRegistrySuffix(string16* suffix) {
   // Use a thread-safe cache for the user's suffix.
   static base::LazyInstance<UserSpecificRegistrySuffix>::Leaky suffix_instance =
       LAZY_INSTANCE_INITIALIZER;
   return suffix_instance.Get().GetSuffix(suffix);
 }
 
 bool ShellUtil::GetOldUserSpecificRegistrySuffix(string16* suffix) {
   wchar_t user_name[256];
   DWORD size = arraysize(user_name);
@@ skipping 47 matching lines @@
     // are any left...).
     if (free_bits >= 8 && next_byte_index < size) {
       free_bits -= 8;
       bit_stream += bytes[next_byte_index++] << free_bits;
     }
   }
 
   DCHECK_EQ(ret.length(), encoded_length);
   return ret;
 }