Implementing unified Chrome / App Launcher flow, and migrating old stand-alone App Launcher.

chrome/installer/util/installer_state.cc

Index: chrome/installer/util/installer_state.cc
diff --git a/chrome/installer/util/installer_state.cc b/chrome/installer/util/installer_state.cc
index 00906292c31514cf2d4333150d52fb9b5383555d..81802ca6cd627be9932e3dc694a865cf5b224ca7 100644
--- a/chrome/installer/util/installer_state.cc
+++ b/chrome/installer/util/installer_state.cc
@@ -24,6 +24,7 @@
 #include "chrome/installer/util/master_preferences.h"
 #include "chrome/installer/util/master_preferences_constants.h"
 #include "chrome/installer/util/product.h"
+#include "chrome/installer/util/util_constants.h"
 #include "chrome/installer/util/work_item.h"
 #include "chrome/installer/util/work_item_list.h"
 
@@ -78,7 +79,9 @@ InstallerState::InstallerState()
       root_key_(NULL),
       msi_(false),
       verbose_logging_(false),
-      ensure_google_update_present_(false) {
+      ensure_google_update_present_(false),
+      need_to_migrate_legacy_app_launcher_(false),
+      create_app_launcher_shortcuts_(false) {
 }
 
 InstallerState::InstallerState(Level level)
@@ -90,7 +93,9 @@ InstallerState::InstallerState(Level level)
       root_key_(NULL),
       msi_(false),
       verbose_logging_(false),
-      ensure_google_update_present_(false) {
+      ensure_google_update_present_(false),
+      need_to_migrate_legacy_app_launcher_(false),
+      create_app_launcher_shortcuts_(false) {
   // Use set_level() so that root_key_ is updated properly.
   set_level(level);
 }
@@ -118,12 +123,38 @@ void InstallerState::Initialize(const CommandLine& command_line,
 
   const bool is_uninstall = command_line.HasSwitch(switches::kUninstall);
 
-  if (prefs.install_chrome()) {
-    Product* p = AddProductFromPreferences(
+  // TODO(huangs): Remove by M30.
+  // Create App Launcher shortcuts if --app-launcher is specified.

[grt (UTC plus 2), 2013/05/16 14:55:52]

This comment also appears on line 154. Can one of them be removed (this one?)?

[huangs, 2013/05/17 20:59:24]

Done (deleted this one).

+  if (prefs.is_multi_install()) {

[grt (UTC plus 2), 2013/05/16 14:55:52]

  if (!is_uninstall && ....)?

[huangs, 2013/05/17 20:59:24]

Done.

+    // Check the version of App Launcher ClientState to determine if legacy.
+    const ProductState* app_launcher_state = machine_state.GetProductState(
+        system_install(), BrowserDistribution::CHROME_APP_HOST);

[grt (UTC plus 2), 2013/05/16 14:55:52]

I forget: is the legacy AL only ever installed at user-level? If so, does it
make sense to only perform this evaluation when !system_install()?

[huangs, 2013/05/17 20:59:24]

Yes, but I'm relying on app_launcher_state == NULL whenever system_install() ==
true to skip this.  But I'll spell this out explicitly.

+    if (app_launcher_state) {
+      need_to_migrate_legacy_app_launcher_ =
+          app_launcher_state->version().IsOlderThan("29.0.1506.0");

[gab, 2013/05/15 22:42:20]

Make sure this is the current version of trunk the day you commit.

[huangs, 2013/05/17 20:59:24]

Yes.  In the planned change, we'll have need_to_migrate_legacy_app_launcher_ =
false always, and in the final "capstone" CL, activate the code with the current
version.

+    }
+  }
+
+  // App Launcher and Chrome are unified.
+  if (prefs.install_chrome() || prefs.install_chrome_app_launcher() ||

[grt (UTC plus 2), 2013/05/16 14:55:52]

  if (!is_uninstall && ...)?

[huangs, 2013/05/17 20:59:24]

I'm "short-circuiting" the --chrome switch and the --app-launcher switch (only
difference being --app-launcher will also set create_app_launcher_shortcuts_). 
I'd rather not introduce more asymmetry to this.

+      need_to_migrate_legacy_app_launcher_) {

[grt (UTC plus 2), 2013/05/16 14:55:52]

Does this mean that a user-level AL installation (from which Chrome had been
uninstalled) will have user-level Chrome magically appear?

[huangs, 2013/05/17 20:59:24]

That is correct; it's the product decision.

+    Product* p1 = AddProductFromPreferences(
         BrowserDistribution::CHROME_BROWSER, prefs, machine_state);
     VLOG(1) << (is_uninstall ? "Uninstall" : "Install")
-            << " distribution: " << p->distribution()->GetAppShortCutName();
+            << " distribution: " << p1->distribution()->GetAppShortCutName();
+
+    if (is_multi_install()) {
+      Product* p2 = AddProductFromPreferences(
+          BrowserDistribution::CHROME_APP_HOST, prefs, machine_state);
+      VLOG(1) << (is_uninstall ? "Uninstall" : "Install")
+              << " distribution: " << p2->distribution()->GetAppShortCutName();
+    }
   }
+  // TODO(huangs): Remove by M30.
+  // Create App Launcher shortcuts if --app-launcher is specified.

[grt (UTC plus 2), 2013/05/16 14:55:52]

Who/what will run the installer with --app-launcher?

[huangs, 2013/05/17 20:59:24]

App command quick-enable-app-launcher, testing, and old tagged meta-installer
that would specify "--app-launcher".

+  create_app_launcher_shortcuts_ = need_to_migrate_legacy_app_launcher_ ||
+      prefs.install_chrome_app_launcher();
+
   if (prefs.install_chrome_frame()) {
     Product* p = AddProductFromPreferences(
         BrowserDistribution::CHROME_FRAME, prefs, machine_state);
@@ -131,38 +162,18 @@ void InstallerState::Initialize(const CommandLine& command_line,
             << " distribution: " << p->distribution()->GetAppShortCutName();
   }
 
-  if (prefs.install_chrome_app_launcher()) {
-    Product* p = AddProductFromPreferences(
-        BrowserDistribution::CHROME_APP_HOST, prefs, machine_state);
-    VLOG(1) << (is_uninstall ? "Uninstall" : "Install")
-            << " distribution: " << p->distribution()->GetAppShortCutName();
-  }
-
   if (!is_uninstall && is_multi_install()) {

[grt (UTC plus 2), 2013/05/16 14:55:52]

This whole block can be compacted:
if (!is_uninstall && is_multi_install() &&
    !FindProduct(BrowserDistribution::CHROME_BINARIES) &&
    (FindProduct(BrowserDistribution::CHROME_BROWSER) ||
     FindProduct(BrowserDistribution::CHROME_FRAME) ||
     FindProduct(BrowserDistribution::CHROME_APP_HOST))) {
  ...
}

[huangs, 2013/05/17 20:59:24]

Done, but I'll have to remember to exclude the CHROME_APP_HOST case until the
final "capstone" CL.

     bool need_binaries = false;
-    if (FindProduct(BrowserDistribution::CHROME_APP_HOST)) {
-      // App Host will happily use Chrome at system level, or binaries at system
-      // level, even if app host is user level.
-      const ProductState* chrome_state = machine_state.GetProductState(
-          true,  // system level
-          BrowserDistribution::CHROME_BROWSER);
-      // If Chrome is at system-level, multi- or otherwise. We'll use it.
-      if (!chrome_state) {
-        const ProductState* binaries_state = machine_state.GetProductState(
-            true,  // system level
-            BrowserDistribution::CHROME_BINARIES);
-        if (!binaries_state)
-          need_binaries = true;
-      }
-    }
-
-    // Chrome/Chrome Frame multi need Binaries at their own level.
+    // Chrome/Chrome Frame/App Launcher multi need Binaries at their own level.
     if (FindProduct(BrowserDistribution::CHROME_BROWSER))
       need_binaries = true;
 
     if (FindProduct(BrowserDistribution::CHROME_FRAME))
       need_binaries = true;
 
+    if (FindProduct(BrowserDistribution::CHROME_APP_HOST))
+      need_binaries = true;
+
     if (need_binaries && !FindProduct(BrowserDistribution::CHROME_BINARIES)) {
       // Force binaries to be installed/updated.
       Product* p = AddProductFromPreferences(