Install a user-level Start Menu shortcut for every user on system-installs through Active Setup

chrome/installer/setup/setup_main.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 <windows.h>
 #include <msi.h>
 #include <shellapi.h>
 #include <shlobj.h>
 
 #include "base/at_exit.h"
@@ skipping 1072 matching lines @@
     // then the dialog is shown and regardless of the outcome setup exits here.
     string16 inner_frame =
         cmd_line.GetSwitchValueNative(installer::switches::kShowEula);
     *exit_code = ShowEULADialog(inner_frame);
     if (installer::EULA_REJECTED != *exit_code) {
       GoogleUpdateSettings::SetEULAConsent(
           original_state, BrowserDistribution::GetDistribution(), true);
     }
   } else if (cmd_line.HasSwitch(
       installer::switches::kConfigureUserSettings)) {
+    // NOTE: Should the work done here, on kConfigureUserSettings, change:
+    // kActiveSetupVersion in install_worker.cc needs to be increased for Active
+    // Setup to invoke this again for all users of this install.
     DCHECK(installer_state->system_install());

[grt (UTC plus 2), 2012/09/02 14:18:19]

since it's legitimate for someone to inadvertently run this without
--system-level, i think it's better to make this do the right thing in release
builds.  please change the logic so that ForceCreateUserLevelStartMenuShortcut
isn't called in that case.  one way would be to add " &&
installer_state->system_install()" to the condition on line 1106 (and maybe move
the assignment of *exit_code up so it isn't set in that case).

[gab, 2012/09/02 16:15:12]

Done and made InstallStatus better for this type of error.

Imo this whole method is wrong in the way it works with |exit_code| and
InstallStatuses. There should a local |status| at the top which everyone sets
(and which is otherwise initialized to UNKNOWN_STATUS) at the end we could then
DCHECK(!handled || status != UNKNOWN_STATUS) and only set the exit code then.
It doesn't make sense that everyone has to set |exit_code| in the same way in
each block; plus this would prevent someone implementing a new block from
forgetting to set |exit_code| (as I originally did).

If you agree with this plan I'll follow up with another CL.

[grt (UTC plus 2), 2012/09/04 13:29:59]

I don't think this new status code provides much of a signal on its own.  If
it's meant to be a general-purpose "something was called wrong" code, then
seeing it on a dashboard may not tell us what was invoked (except for specific
dashboards) or what the invalid state was.  Do you think UNKNOWN_STATUS is
sufficient for this?

Also, I think it makes sense to LOG(DFATAL) any time this would be returned with
enough info to know what was wrong so that release builds produce actionable
(albeit local) info.
This certainly could be improved.  Feel free to TODO it to yourself to take care
of when it's time to pay down some debt.

[gab, 2012/09/04 15:40:36]

As discussed, we would rather never see UNKNOWN_STATUS in logs, on the flip side
we also don't a new failure flag for a bunch of things that are never supposed
to happen... so we introduce this new flag which should never happen, but at
least it's not UNKNOWN_STATUS (which usually means we messed in the code and
didn't handle a possible failure). Accompanied by DFATAL logs in Release builds
we can trace back where things went unexpectingly should we ever get this error.
Done.
Done.

     const Product* chrome_install =
         installer_state->FindProduct(BrowserDistribution::CHROME_BROWSER);
-    DCHECK(chrome_install);
-    // TODO(gab): Implement the new shortcut functionality here.
-    LOG(ERROR) << "--configure-user-settings is not implemented.";
+
+    // Create per-user Start Menu shortcuts.
+    // TODO(gab): This should really be more involved, but for now creating
+    // the shortcut and leaving it non-updated in later updates is fine.
+    // This is all coming in an upcoming refactoring of the shortcuts, but for
+    // now this will simply create the user-level shortcut once on the first
+    // (and only) invocation of Active Setup for Chrome for this user.
+    if (chrome_install)
+      ForceCreateUserLevelStartMenuShortcut(*installer_state, *chrome_install);
+    else
+      NOTREACHED();
+
+    *exit_code = InstallUtil::GetInstallReturnCode(installer::INSTALL_REPAIRED);
+
   } else if (cmd_line.HasSwitch(
       installer::switches::kRegisterChromeBrowser)) {
     installer::InstallStatus status = installer::UNKNOWN_STATUS;
     const Product* chrome_install =
         installer_state->FindProduct(BrowserDistribution::CHROME_BROWSER);
     if (chrome_install) {
       // If --register-chrome-browser option is specified, register all
       // Chrome protocol/file associations, as well as register it as a valid
       // browser for Start Menu->Internet shortcut. This switch will also
       // register Chrome as a valid handler for a set of URL protocols that
@@ skipping 390 matching lines @@
   if (!(installer_state.is_msi() && is_uninstall))
     // Note that we allow the status installer::UNINSTALL_REQUIRES_REBOOT
     // to pass through, since this is only returned on uninstall which is
     // never invoked directly by Google Update.
     return_code = InstallUtil::GetInstallReturnCode(install_status);
 
   VLOG(1) << "Installation complete, returning: " << return_code;
 
   return return_code;
 }