Fix the path of shortcuts with an icon in the current install dir.

chrome/installer/setup/install.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 "chrome/installer/setup/install.h"
 
 #include <windows.h>
 #include <shlobj.h>
 #include <time.h>
 
@@ skipping 264 matching lines @@
   // ...\Quick Launch\User Pinned\TaskBar for SHORTCUT_LOCATION_TASKBAR_PINS.
   if (shortcut_location == ShellUtil::SHORTCUT_LOCATION_TASKBAR_PINS) {
     shortcut_path = shortcut_path.DirName();
     recursive = true;
   }
 
   base::FileEnumerator shortcuts_enum(shortcut_path, recursive,
                                       base::FileEnumerator::FILES);
   for (base::FilePath shortcut = shortcuts_enum.Next(); !shortcut.empty();
        shortcut = shortcuts_enum.Next()) {
-    base::FilePath existing_target_path;
-    if (!base::win::ResolveShortcut(shortcut, &existing_target_path, nullptr) ||
-        !base::StartsWith(existing_target_path.value(),
-                          old_target_dir.AsEndingWithSeparator().value(),
-                          base::CompareCase::INSENSITIVE_ASCII) ||
-        !base::EndsWith(existing_target_path.value(),
-                        old_target_path_suffix.value(),
-                        base::CompareCase::INSENSITIVE_ASCII)) {
+    base::win::ShortcutProperties shortcut_properties;
+    if (!base::win::ResolveShortcutProperties(
+            shortcut, (base::win::ShortcutProperties::PROPERTIES_TARGET |
+                       base::win::ShortcutProperties::PROPERTIES_ICON),
+            &shortcut_properties)) {
       continue;
     }
 
+    // TODO(fdoray): Stop using the icon to identify shortcuts that need to be
+    // updated in May 2016. This is just intended to fix shortcuts broken by
+    // crbug.com/595374.
+    if (!(base::StartsWith(shortcut_properties.target.value(),
+                           old_target_dir.AsEndingWithSeparator().value(),
+                           base::CompareCase::INSENSITIVE_ASCII) &&
+          base::EndsWith(shortcut_properties.target.value(),
+                         old_target_path_suffix.value(),
+                         base::CompareCase::INSENSITIVE_ASCII)) &&
+        !old_target_dir.IsParent(shortcut_properties.icon)) {

[gab, 2016/03/17 20:14:23]

I had a hard time reading this logic, I'd say add a comment above that spells
out in English what this does, something like:

// Skip shortcuts whose target isn't rooted at |old_target_dir| and whose base
// doesn't end in |old_target_path_suffix|. Except for shortcuts whose icon is
// rooted at |old_target_dir|.
// TODO(fdoray): The second condition is only intended to fix Canary shortcuts
// broken by crbug.com/595374, remove it in May 2016.

[gab, 2016/03/17 20:14:23]

How about we do path comparison everywhere instead of string comparison to be
safer?

i.e. 
1) IsParent() instead of StartsWith() and 
2) EndsWith() on BaseName()

(and adjust API nomenclature as appropriate)

I guess this is a comment about the existing code, but might as well be pedantic
about making it extra correct given the unintended side-effects to date.

[fdoray, 2016/03/18 14:08:59]

Done.

[fdoray, 2016/03/18 14:08:59]

Done.

+      continue;
+    }
+
     base::win::ShortcutProperties updated_properties;
     updated_properties.set_target(new_target_path);
     base::win::CreateOrUpdateShortcutLink(shortcut, updated_properties,
                                           base::win::SHORTCUT_UPDATE_EXISTING);
   }
 }
 
 void EscapeXmlAttributeValueInSingleQuotes(base::string16* att_value) {
   base::ReplaceChars(*att_value, base::ASCIIToUTF16("&"),
                      base::ASCIIToUTF16("&"), att_value);
@@ skipping 407 matching lines @@
   // Read master_preferences copied beside chrome.exe at install.
   MasterPreferences prefs(installation_root.AppendASCII(kDefaultMasterPrefs));
   base::FilePath chrome_exe(installation_root.Append(kChromeExe));
   CreateOrUpdateShortcuts(
       chrome_exe, chrome, prefs, CURRENT_USER, install_operation);
 
   UpdateDefaultBrowserBeaconForPath(chrome_exe);
 }
 
 }  // namespace installer