Fix the path of Chrome shortcuts from the installer.

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>
 
 #include <string>
 
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/win/shortcut.h"
 #include "base/win/windows_version.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/installer/setup/install_worker.h"
 #include "chrome/installer/setup/installer_crash_reporting.h"
 #include "chrome/installer/setup/setup_constants.h"
 #include "chrome/installer/setup/setup_util.h"
 #include "chrome/installer/setup/update_active_setup_version_work_item.h"
+#include "chrome/installer/setup/update_shortcuts.h"
 #include "chrome/installer/util/beacons.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/create_reg_key_work_item.h"
 #include "chrome/installer/util/delete_after_reboot_helper.h"
 #include "chrome/installer/util/google_update_constants.h"
 #include "chrome/installer/util/helper.h"
 #include "chrome/installer/util/install_util.h"
 #include "chrome/installer/util/master_preferences.h"
 #include "chrome/installer/util/master_preferences_constants.h"
 #include "chrome/installer/util/set_reg_value_work_item.h"
+#include "chrome/installer/util/shell_util.h"
 #include "chrome/installer/util/util_constants.h"
 #include "chrome/installer/util/work_item.h"
 #include "chrome/installer/util/work_item_list.h"
 
 
 namespace {
 
 void LogShortcutOperation(ShellUtil::ShortcutLocation location,
                           BrowserDistribution* dist,
                           const ShellUtil::ShortcutProperties& properties,
@@ skipping 319 matching lines @@
   // Shortcuts are always installed per-user unless specified.
   ShellUtil::ShellChange shortcut_level = (install_level == ALL_USERS ?
       ShellUtil::SYSTEM_LEVEL : ShellUtil::CURRENT_USER);
 
   // |base_properties|: The basic properties to set on every shortcut installed
   // (to be refined on a per-shortcut basis).
   ShellUtil::ShortcutProperties base_properties(shortcut_level);
   product.AddDefaultShortcutProperties(target, &base_properties);
 
   if (!do_not_create_desktop_shortcut ||
       shortcut_operation == ShellUtil::SHELL_SHORTCUT_REPLACE_EXISTING) {

[gab, 2016/03/09 20:10:44]

I was initially going to say that these are redundant when touching the per-user
shortcut, but I guess they're not quite redundant as they "REPLACE" instead of
"UPDATE" -- so it's the update task that's redundant, but since the update spans
more than just one shortcut we can't cut it either even when both operations are
done at ShellUtil::CURRENT_USER level...

So I guess having both (as you have it) is the simplest way to write this
despite the minor redundancy.

WDYT?

[fdoray, 2016/03/09 22:13:03]

They are redundant. This code should be refactored, but I wanted to get a fix
that works as quickly as possible.

     ExecuteAndLogShortcutOperation(
         ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist, base_properties,
         shortcut_operation);
   }
 
   if (!do_not_create_quick_launch_shortcut ||
       shortcut_operation == ShellUtil::SHELL_SHORTCUT_REPLACE_EXISTING) {
     // There is no such thing as an all-users Quick Launch shortcut, always
     // install the per-user shortcut.
     ShellUtil::ShortcutProperties quick_launch_properties(base_properties);
@@ skipping 21 matching lines @@
   if (base::PathExists(old_shortcut_path)) {
     ShellUtil::MoveExistingShortcut(
         ShellUtil::SHORTCUT_LOCATION_START_MENU_CHROME_DIR_DEPRECATED,
         ShellUtil::SHORTCUT_LOCATION_START_MENU_ROOT,
         dist, start_menu_properties);
   }
 
   ExecuteAndLogShortcutOperation(
       ShellUtil::SHORTCUT_LOCATION_START_MENU_ROOT, dist,
       start_menu_properties, shortcut_operation);
+
+  // Update the target path of existing shortcuts.

[gab, 2016/03/09 20:10:44]

s/shortcuts/per-user shortcuts/

[fdoray, 2016/03/09 22:13:03]

Done.

+  if (install_operation == INSTALL_SHORTCUT_REPLACE_EXISTING) {
+    const base::FilePath target_path_prefix = target.DirName();

[gab, 2016/03/09 20:10:44]

If I'm not mistaken, |target_path| here will be something like
C:\Users\<user>\AppData\Local\Google\Chrome SxS\Application\chrome.exe
so I think for the prefix you want .DirName().DirName() to be one-level below
Application/

[gab, 2016/03/09 20:10:44]

How about:

s/target_path_prefix/updated_prefix/
s/target_path_suffix/updated_suffix/

?

[fdoray, 2016/03/09 22:13:03]

Done.

[fdoray, 2016/03/09 22:13:03]

Done. I didn't use the path from the bug report to test this code initially... I
did test the updated CL with the path from the bug report.

+    const base::FilePath target_path_suffix = target.BaseName();
+
+    UpdateShortcuts(ShellUtil::SHORTCUT_LOCATION_DESKTOP, shortcut_level, dist,

[gab, 2016/03/09 20:10:44]

s/shortcut_level/ShellUtil::CURRENT_USER/

here and below (but don't add "&& shortcut_level == ShellUtil::CURRENT_USER" to
initial condition as fixing up per-user shortcuts on system-level installs has
the advantage to also work for any user running the installer as admin for
himself (instead of say an update running as SYSTEM which isn't a "user" in the
shortcut world).

[fdoray, 2016/03/09 22:13:02]

CURRENT_USER now hard-coded in UpdatePerUserShortcutsInLocation

+                    target_path_prefix, target_path_suffix, target);
+    UpdateShortcuts(ShellUtil::SHORTCUT_LOCATION_START_MENU_ROOT,

[gab, 2016/03/09 20:10:43]

Don't think we need to touch the Start Menu shortcut -- it's always at install
level and thus already updated above. Plus recursing through all shortcuts in
Start Menu seems overkill (I guess it could fixup some Chrome Apps shortcut, but
if that's a goal we should explicit list the Apps Shortcut folder here instead
of SHORTCUT_LOCATION_START_MENU_ROOT).

[fdoray, 2016/03/09 22:13:03]

Done.

+                    shortcut_level, dist, target_path_prefix,
+                    target_path_suffix, target);
+
+    // Check that the quick launch directory is a parent of the taskbar pins

[gab, 2016/03/09 20:10:44]

// The next update assumes that the quick launch directory is a parent of the
taskbar pins directory, confirm that this is true.

[fdoray, 2016/03/09 22:13:03]

I now have separate calls to UpdatePerUserShortcutsInLocation to update quick
launch and taskbar pins.

+    // directory.
+    if (DCHECK_IS_ON()) {
+      base::FilePath taskbar_pins_path;
+      ShellUtil::GetShortcutPath(ShellUtil::SHORTCUT_LOCATION_TASKBAR_PINS,
+                                 dist, shortcut_level, &taskbar_pins_path);
+      base::FilePath quick_launch_path;
+      ShellUtil::GetShortcutPath(ShellUtil::SHORTCUT_LOCATION_QUICK_LAUNCH,
+                                 dist, shortcut_level, &quick_launch_path);
+      DCHECK(quick_launch_path.IsParent(taskbar_pins_path));
+    }
+    // This updates both quick launch and taskbar pins.

[gab, 2016/03/09 20:10:43]

// Update quick launch and taskbar pins.

[fdoray, 2016/03/09 22:13:02]

This comment is no longer applicable.

+    UpdateShortcuts(ShellUtil::SHORTCUT_LOCATION_QUICK_LAUNCH,
+                    ShellUtil::CURRENT_USER, dist, target_path_prefix,
+                    target_path_suffix, target);
+  }
 }
 
 void RegisterChromeOnMachine(const installer::InstallerState& installer_state,
                              const installer::Product& product,
                              bool make_chrome_default) {
   DCHECK(product.is_chrome());
 
   // Try to add Chrome to Media Player shim inclusion list. We don't do any
   // error checking here because this operation will fail if user doesn't
   // have admin rights and we want to ignore the error.
@@ skipping 222 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