Removing deprecated DCHECK's that require App Launcher to be user-level only.

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 518 matching lines @@
 
     if (result == FIRST_INSTALL_SUCCESS && !prefs_path.empty())
       CopyPreferenceFileForFirstRun(installer_state, prefs_path);
 
     installer_state.UpdateStage(installer::CREATING_SHORTCUTS);
 
     const Product* app_launcher_product =
         installer_state.FindProduct(BrowserDistribution::CHROME_APP_HOST);
     // Creates shortcuts for App Launcher.
     if (app_launcher_product) {
-      // TODO(huangs): Remove this check once we have system-level App Host.
-      DCHECK(!installer_state.system_install());
       const base::FilePath app_host_exe(
           installer_state.target_path().Append(kChromeAppHostExe));
       InstallShortcutOperation app_launcher_shortcut_operation =
           GetAppLauncherShortcutOperation(original_state, installer_state);
 
       // Always install per-user shortcuts for App Launcher.
       CreateOrUpdateShortcuts(app_host_exe, *app_launcher_product, prefs,
                               CURRENT_USER, app_launcher_shortcut_operation);

[grt (UTC plus 2), 2013/05/24 01:18:07]

will this ever be hit? it's very bad for this to try to create per-user
shortcuts for a system-level install.

[gab, 2013/05/24 12:32:33]

I don't think that's true. We already do this for Chrome (i.e. taskbar shortcuts
are per-user) and if it was up to me I'd make all shortcuts per-user, it avoids
problems with people deleting other users' shortcuts (which is a problem with
all-users shortcuts) -- we tried to do this, but failed because old versions of
Chrome unconditionally delete the per-user Chrome shortcuts on self-destruct
which would result in no shortcuts for that user (http://crbug.com/164655)...

Per-user shortcuts are a problem only on uninstall since we can't clean them,
but Windows takes care of prompting users using invalid shortcuts post-uninstall
so I think that's ok (even if we were to fix it for some Chrome shortcuts, we
can't fix it for user created shortcuts (profile/app shortcuts) unless doing a
full sweep of every user's shortcut folders on uninstall (which we don't want to
do)).

Per-user shortcuts also have a nice feature that an admin can uninstall and
re-install Chrome without messing up users' shortcuts (provided he re-installs
before anyone uses the shortcuts...).

[grt (UTC plus 2), 2013/05/24 15:41:09]

The installer may run as SYSTEM, where there is no such thing as a user. If
Sam's previous change (that this is supposed to fix) results in the installer
trying to create a per-user shortcut for a system-level install, then that's
what needs to be fixed. By itself, removing the DCHECK here is wrong.

[gab, 2013/05/24 15:42:17]

My bad, I didn't get Greg's point correctly at first.

Indeed the installer can't create per-user shortcuts on system-level installs
since it runs as SYSTEM in most cases.

This is not the DCHECK we are currently hitting though (the ones in
installation_validator.cc and uninstall.cc) are the problems.

[huangs, 2013/05/24 15:54:30]

We're not hitting this DCHECK, but I need to do more tracing to be sure. The
current priority is to get things to work, so I'd say, we should remove this
check, wrap shortcut creation inside "if (user_level)", and log warning if
system-level is detected (plus TODO). This will be properly fixed once we figure
out Chrome / App Launcher unification stuff.

I'll be in a position (better internet) to implement this in an hour or two;
please let me know in the meantime if you're okay with the plan.

[gab, 2013/05/24 15:57:55]

Aren't you on vacation?!

There is no rush really since the few of us with this issue can comment out the
DCHECK for now.

Take a break and we can talk about it next week ;)!

[grt (UTC plus 2), 2013/05/24 16:04:25]

If CheckAppHostPreconditions is to be believed, the app host can't be installed
at system-level. Will AddExistingMultiInstalls cause this code to be hit when a
system-level install that has the app host "pv" (for user tracking) is updated?
If so, that's possibly a bigger problem than this. What are the implications
there?

[huangs, 2013/05/24 16:21:57]

Okay... I'm back on Tuesday though. A new concern now is that with the
"quick-fix" IIRC this code is not hit at first install. But on update I'll have
to test! Just want to test this and put it to rest so I can sleep better. :)

[huangs, 2013/05/24 18:39:46]

Did some testing and investigation, verified that App Launcher shortcut will NOT
get added during update.

Reason: The "shadow" Products key makes App Launcher get added to
InstallationState; this triggers calls in validator and uninstall, which is
where our DCHECK() get called. But during update, since the "shadow" key has no
uninstall command, so product->is_multi_install() returns false, and App
Launcher is NOT added to InstallerState => hence no shortcut update (another
layer of protection is that no new shortcuts get created on update).

In short, my feared scenario (Chrome user magically gets App Launcher after
update) will not happen; I can relax for now. :)

     }
 
     const Product* chrome_product =
         installer_state.FindProduct(BrowserDistribution::CHROME_BROWSER);
     // Creates shortcuts for Chrome.
     if (chrome_product) {
       BrowserDistribution* chrome_dist = chrome_product->distribution();
       const base::FilePath chrome_exe(
           installer_state.target_path().Append(kChromeExe));
       CleanupLegacyShortcuts(installer_state, chrome_dist, chrome_exe);
@@ skipping 130 matching lines @@
   if (app_host_path.empty())
     return false;
 
   CommandLine cmd(app_host_path);
   cmd.AppendSwitchASCII(::switches::kInstallFromWebstore, app_code);
   VLOG(1) << "App install command: " << cmd.GetCommandLineString();
   return base::LaunchProcess(cmd, base::LaunchOptions(), NULL);
 }
 
 }  // namespace installer