Suffix Chrome's appid on user-level installs

chrome/installer/setup/install.cc

Index: chrome/installer/setup/install.cc
diff --git a/chrome/installer/setup/install.cc b/chrome/installer/setup/install.cc
index 375b2ec1aba977b63842f28352b0ffc27c4ab4ed..e6d8a1ff119d24ace5989b999a31a68e2922bca8 100644
--- a/chrome/installer/setup/install.cc
+++ b/chrome/installer/setup/install.cc
@@ -87,12 +87,13 @@ void CopyPreferenceFileForFirstRun(const InstallerState& installer_state,
 // If the shortcuts do not exist, the function does not recreate them during
 // update.
 // |options|: bitfield for which the options come from ChromeShortcutOptions.
-void CreateOrUpdateChromeShortcuts(const InstallerState& installer_state,
-                                   const FilePath& setup_path,
-                                   const Version& new_version,
-                                   installer::InstallStatus install_status,
-                                   const Product& product,
-                                   uint32 options) {
+void CreateOrUpdateChromeShortcuts(
+    const InstallerState& installer_state,
+    const FilePath& setup_path,
+    const Version& new_version,
+    const installer::InstallStatus& install_status,

[grt (UTC plus 2), 2012/06/15 03:03:03]

why pass a value of an enum by const ref?

[gab, 2012/06/15 19:01:04]

Good question, I somehow thought it was an Object and remember having a reason
wanting this call not to modify its state... my bad!

+    const Product& product,
+    uint32 options) {
   // TODO(tommi): Change this function to use WorkItemList.
   DCHECK(product.is_chrome());
 
@@ -468,31 +469,11 @@ InstallStatus InstallOrUpdateProduct(
     if (result == FIRST_INSTALL_SUCCESS && !prefs_path.empty())
       CopyPreferenceFileForFirstRun(installer_state, prefs_path);
 
-    bool do_not_create_shortcuts = false;
-    prefs.GetBool(master_preferences::kDoNotCreateShortcuts,

[grt (UTC plus 2), 2012/06/15 03:03:03]

robert added this in http://crrev.com/25870.  it explicitly prevented
registration.  i'm not familiar with how it's used in practice, but this CL is a
significant departure from existing behavior.  robert: can you shed some light
on why this preference was added and under what circumstances it's used?

[robertshield, 2012/06/15 03:27:59]

This was added long ago for Chrome Frame, so that installing CF would not create
shortcuts to desktop Chrome. At one point, this argument was implied by the
presence of --chrome-frame, rendering the explicit command line flag obsolete,
but it looks like it never got scrubbed from our Omaha configs and still today,
this argument is passed to CF installers that users run, both stub and MSIs.

Chrome MSI and stub installers never get this, but I don't know whether any OEM
deals pass this explicitly for Chrome.

Several refactorings later, this appears to be the sole remaining usage and I
believe that a standalone CF install would be unaffected by this change due to
the "if (chrome_install)" check. 

Barring any OEM deals passing this explicitly for Chrome installations, making
this change is fine, but please try running a CF install, as well as a multi
CF+Chrome install to verify. 

[grt (UTC plus 2), 2012/06/15 03:33:51]

Do you know who we should ask about that?
How about just removing all traces of kDoNotCreateShortcuts from the code?  It
wouldn't break any vestigial uses of the option unless there are deals that use
it for Chrome.

[gab, 2012/06/15 19:01:04]

This sounds like its wrong/unused, we should probably remove it in another
patch, but I don't need to do this here anymore and thus won't be changing this
in this CL.

-                  &do_not_create_shortcuts);
-
     // Currently this only creates shortcuts for Chrome, but for other products
     // we might want to create shortcuts.
     const Product* chrome_install =
         installer_state.FindProduct(BrowserDistribution::CHROME_BROWSER);
-    if (chrome_install && !do_not_create_shortcuts) {
-      installer_state.UpdateStage(installer::CREATING_SHORTCUTS);
-
-      bool create_all_shortcut = false;
-      prefs.GetBool(master_preferences::kCreateAllShortcuts,
-                    &create_all_shortcut);
-      bool alt_shortcut = false;
-      prefs.GetBool(master_preferences::kAltShortcutText, &alt_shortcut);
-      uint32 shortcut_options = ShellUtil::SHORTCUT_NO_OPTIONS;
-      if (create_all_shortcut)
-        shortcut_options |= ShellUtil::SHORTCUT_CREATE_ALWAYS;
-      if (alt_shortcut)
-        shortcut_options |= ShellUtil::SHORTCUT_ALTERNATE;
-      CreateOrUpdateChromeShortcuts(installer_state, setup_path,
-                                    new_version, result, *chrome_install,
-                                    shortcut_options);
-
+    if (chrome_install) {
       bool make_chrome_default = false;
       prefs.GetBool(master_preferences::kMakeChromeDefault,
                     &make_chrome_default);
@@ -510,9 +491,33 @@ InstallStatus InstallOrUpdateProduct(
 
       installer_state.UpdateStage(installer::REGISTERING_CHROME);
 
+      // Chrome must be registered before creating shortcuts as the registration
+      // determines the appid that will later be embedded in the shortcut.
       RegisterChromeOnMachine(installer_state, *chrome_install,
           make_chrome_default || force_chrome_default_for_user);
 
+      bool do_not_create_shortcuts = false;
+      prefs.GetBool(master_preferences::kDoNotCreateShortcuts,
+                    &do_not_create_shortcuts);
+
+      if (!do_not_create_shortcuts) {
+        installer_state.UpdateStage(installer::CREATING_SHORTCUTS);
+
+        bool create_all_shortcut = false;
+        prefs.GetBool(master_preferences::kCreateAllShortcuts,
+                      &create_all_shortcut);
+        bool alt_shortcut = false;
+        prefs.GetBool(master_preferences::kAltShortcutText, &alt_shortcut);
+        uint32 shortcut_options = ShellUtil::SHORTCUT_NO_OPTIONS;
+        if (create_all_shortcut)
+          shortcut_options |= ShellUtil::SHORTCUT_CREATE_ALWAYS;
+        if (alt_shortcut)
+          shortcut_options |= ShellUtil::SHORTCUT_ALTERNATE;
+        CreateOrUpdateChromeShortcuts(installer_state, setup_path,
+                                      new_version, result, *chrome_install,
+                                      shortcut_options);
+      }
+
       if (result == FIRST_INSTALL_SUCCESS) {
         installer_state.UpdateStage(installer::CONFIGURE_AUTO_LAUNCH);