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

chrome/installer/setup/install_worker.cc

Index: chrome/installer/setup/install_worker.cc
diff --git a/chrome/installer/setup/install_worker.cc b/chrome/installer/setup/install_worker.cc
index c92219032b35bd77ca230dade2043cdb6d4d4476..a54db69709501b2a02a247d27f129e337b12b446 100644
--- a/chrome/installer/setup/install_worker.cc
+++ b/chrome/installer/setup/install_worker.cc
@@ -29,6 +29,7 @@
 #include "base/win/windows_version.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/installer/launcher_support/chrome_launcher_support.h"
 #include "chrome/installer/setup/install.h"
 #include "chrome/installer/setup/setup_constants.h"
 #include "chrome/installer/setup/setup_util.h"
@@ -179,34 +180,28 @@ void AddInstallerCopyTasks(const InstallerState& installer_state,
         WorkItem::ALWAYS);
   }
 
-  // If only the App Host (not even the Chrome Binaries) is being installed,
-  // this must be a user-level App Host piggybacking on system-level Chrome
-  // Binaries. Only setup.exe is required, and only for uninstall.
-  if (installer_state.products().size() != 1 ||
-      !installer_state.FindProduct(BrowserDistribution::CHROME_APP_HOST)) {
-    base::FilePath archive_dst(installer_dir.Append(archive_path.BaseName()));
-    if (archive_path != archive_dst) {
-      // In the past, we copied rather than moved for system level installs so
-      // that the permissions of %ProgramFiles% would be picked up.  Now that
-      // |temp_path| is in %ProgramFiles% for system level installs (and in
-      // %LOCALAPPDATA% otherwise), there is no need to do this for the archive.
-      // Setup.exe, on the other hand, is created elsewhere so it must always be
-      // copied.
-      if (temp_path.IsParent(archive_path)) {
-        install_list->AddMoveTreeWorkItem(archive_path.value(),
-                                          archive_dst.value(),
-                                          temp_path.value(),
-                                          WorkItem::ALWAYS_MOVE);
-      } else {
-        // This may occur when setup is run out of an existing installation
-        // directory. For example, when quick-enabling user-level App Launcher
-        // from system-level Binaries. We can't (and don't want to) remove the
-        // system-level archive.
-        install_list->AddCopyTreeWorkItem(archive_path.value(),
-                                          archive_dst.value(),
-                                          temp_path.value(),
-                                          WorkItem::ALWAYS);
-      }
+  base::FilePath archive_dst(installer_dir.Append(archive_path.BaseName()));
+  if (archive_path != archive_dst) {
+    // In the past, we copied rather than moved for system level installs so
+    // that the permissions of %ProgramFiles% would be picked up.  Now that
+    // |temp_path| is in %ProgramFiles% for system level installs (and in
+    // %LOCALAPPDATA% otherwise), there is no need to do this for the archive.
+    // Setup.exe, on the other hand, is created elsewhere so it must always be
+    // copied.
+    if (temp_path.IsParent(archive_path)) {
+      install_list->AddMoveTreeWorkItem(archive_path.value(),
+                                        archive_dst.value(),
+                                        temp_path.value(),
+                                        WorkItem::ALWAYS_MOVE);
+    } else {
+      // This may occur when setup is run out of an existing installation
+      // directory. For example, when quick-enabling user-level App Launcher
+      // from system-level Binaries. We can't (and don't want to) remove the
+      // system-level archive.
+      install_list->AddCopyTreeWorkItem(archive_path.value(),
+                                        archive_dst.value(),
+                                        temp_path.value(),
+                                        WorkItem::ALWAYS);
     }
   }
 }
@@ -266,7 +261,7 @@ void AddInstallAppCommandWorkItems(const InstallerState& installer_state,
   DCHECK(product.is_chrome_app_host());
   AddCommandWithParameterWorkItems(installer_state, machine_state, new_version,
                                    product, kCmdInstallApp,
-                                   installer::kChromeAppHostExe,
+                                   installer::kChromeExe,
                                    ::switches::kInstallFromWebstore,
                                    work_item_list);
 }
@@ -362,7 +357,6 @@ void AddQuickEnableApplicationLauncherWorkItems(
                                                       new_version));
     // kMultiInstall and kVerboseLogging were processed above.
     cmd_line.AppendSwitch(switches::kChromeAppLauncher);
-    cmd_line.AppendSwitch(switches::kEnsureGoogleUpdatePresent);

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

what did this do?

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

Installs Google Update at user-level (even if Chrome is at system-level)
whenever quick-enable-application-host occurs.

     AppCommand cmd(cmd_line.GetCommandLineString());
     cmd.set_sends_pings(true);
     cmd.set_is_web_accessible(true);
@@ -661,6 +655,34 @@ void CleanupBadCanaryDelegateExecuteRegistration(
   }
 }
 
+// As of M29, App Launcher is unified with Chrome. Here we delete a number of
+// legacy install artifacts for the product, before new artifacts are installed.
+void MigrateLegacyAppLauncher(const InstallerState& installer_state,
+                              const base::FilePath& temp_path,
+                              WorkItemList* install_list) {
+  VLOG(1) << "Migrating legacy App Launcher that used app_host.exe.";
+  HKEY reg_root = HKEY_CURRENT_USER;

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

inline below

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

Done.

+
+  if (!installer_state.system_install()) {
+    // Delete Add/Remove entry. Skip this for system-level install, since in
+    // this case it's up to the self-destruct flow to call uninstall.

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

App launcher was never installed at system-level, right?

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

This is |installer_state|. The new App Launcher may be in system level.  The
purpose of this check is avoid system-level install meddling with user-level.

+    string16 legacy_app_host_uninstall_reg_path(
+        L"Software\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\"
+        L"Google Chrome App Launcher");
+    install_list->AddDeleteRegKeyWorkItem(reg_root,
+                                          legacy_app_host_uninstall_reg_path);
+  }
+
+  // Delete app_host.exe at the install level.
+  base::FilePath app_host_exe(

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

Did app_host.exe live beside chrome.exe or in the version folder? If it lived in
the version folder than you don't need to do anything.

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

app_host.exe lives besides chrome.exe (or by itself if user-level Chrome
self-destructed; there was code to explicitly leave app_host.exe alone).

+      chrome_launcher_support::GetAppHostPathForInstallationLevel(
+          installer_state.system_install() ?
+          chrome_launcher_support::SYSTEM_LEVEL_INSTALLATION :
+          chrome_launcher_support::USER_LEVEL_INSTALLATION));
+  if (!app_host_exe.empty())
+    install_list->AddDeleteTreeWorkItem(app_host_exe, temp_path);
+}
+
 }  // namespace
 
 // This method adds work items to create (or update) Chrome uninstall entry in
@@ -1173,6 +1195,14 @@ void AddInstallWorkItems(const InstallationState& original_state,
   install_list->AddCreateDirWorkItem(temp_path);
   install_list->AddCreateDirWorkItem(target_path);
 
+  // User-level App Launcher may be an old version that uses app_host.exe,
+  // which needs to be migrated.
+  if (installer_state.FindProduct(BrowserDistribution::CHROME_BINARIES)) {
+    if (installer_state.need_to_migrate_legacy_app_launcher()) {

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

Use && rather than 2 ifs

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

Done.

+      MigrateLegacyAppLauncher(installer_state, temp_path, install_list);
+    }
+  }
+
   if (installer_state.FindProduct(BrowserDistribution::CHROME_BROWSER) ||
       installer_state.FindProduct(BrowserDistribution::CHROME_FRAME) ||
       installer_state.FindProduct(BrowserDistribution::CHROME_BINARIES)) {
@@ -1187,15 +1217,6 @@ void AddInstallWorkItems(const InstallationState& original_state,
                        install_list);
   }
 
-  if (installer_state.FindProduct(BrowserDistribution::CHROME_APP_HOST)) {
-    install_list->AddCopyTreeWorkItem(
-        src_path.Append(installer::kChromeAppHostExe).value(),
-        target_path.Append(installer::kChromeAppHostExe).value(),
-        temp_path.value(),
-        WorkItem::ALWAYS,
-        L"");
-  }
-
   // Copy installer in install directory
   AddInstallerCopyTasks(installer_state, setup_path, archive_path, temp_path,
                         new_version, install_list);
@@ -1223,21 +1244,6 @@ void AddInstallWorkItems(const InstallationState& original_state,
                             install_list);
   }
 
-  // TODO(huangs): Implement actual migration code and remove the hack below.
-  // If installing Chrome without the legacy stand-alone App Launcher (to be
-  // handled later), add "shadow" App Launcher registry keys so Google Update
-  // would recognize the "dr" value in the App Launcher ClientState key.
-  // Checking .is_multi_install() excludes Chrome Canary and stand-alone Chrome.
-  if (installer_state.is_multi_install() &&
-      installer_state.FindProduct(BrowserDistribution::CHROME_BROWSER) &&
-      !installer_state.FindProduct(BrowserDistribution::CHROME_APP_HOST)) {
-    BrowserDistribution* shadow_app_launcher_dist =
-        BrowserDistribution::GetSpecificDistribution(
-            BrowserDistribution::CHROME_APP_HOST);
-    AddVersionKeyWorkItems(root, shadow_app_launcher_dist, new_version,
-                           add_language_identifier, install_list);
-  }
-
   // Add any remaining work items that involve special settings for
   // each product.
   AddProductSpecificWorkItems(original_state, installer_state, setup_path,