Remove kCmdInstallExtension and kCmdInstallApp from Installer code

chrome/installer/setup/install_worker.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.
 //
 // This file contains the definitions of the installer functions that build
 // the WorkItemList used to install the application.
 
 #include "chrome/installer/setup/install_worker.h"
 
 #include <oaidl.h>
@@ skipping 53 matching lines @@
 // Although the UUID of the ChromeFrame class is used for the "current" value,
 // this is done only as a convenience; there is no need for the GUID of the Low
 // Rights policies to match the ChromeFrame class's GUID.  Hence, it is safe to
 // use this completely unrelated GUID for the "old" policies.
 const wchar_t kIELowRightsPolicyOldGuid[] =
     L"{6C288DD7-76FB-4721-B628-56FAC252E199}";
 
 const wchar_t kElevationPolicyKeyPath[] =
     L"SOFTWARE\\Microsoft\\Internet Explorer\\Low Rights\\ElevationPolicy\\";
 
+// The legacy command ids for installing an application or extension. These are
+// only here so they can be removed from the registry.
+const wchar_t kLegacyCmdInstallApp[] = L"install-application";
+const wchar_t kLegacyCmdInstallExtension[] = L"install-extension";
+
 void GetOldIELowRightsElevationPolicyKeyPath(base::string16* key_path) {
   key_path->assign(kElevationPolicyKeyPath,
                    arraysize(kElevationPolicyKeyPath) - 1);
   key_path->append(kIELowRightsPolicyOldGuid,
                    arraysize(kIELowRightsPolicyOldGuid)- 1);
 }
 
 // Local helper to call AddRegisterComDllWorkItems for all DLLs in a set of
 // products managed by a given package.
 // |old_version| can be NULL to indicate no Chrome is currently installed.
@@ skipping 139 matching lines @@
     cmd_line.AppendSwitchASCII(command_with_parameter, "%1");
 
     AppCommand cmd(cmd_line.GetCommandLineString());
     cmd.set_sends_pings(true);
     cmd.set_is_web_accessible(true);
     cmd.set_is_run_as_user(true);
     cmd.AddWorkItems(installer_state.root_key(), full_cmd_key, work_item_list);
   }
 }
 
-void AddInstallAppCommandWorkItems(const InstallerState& installer_state,
-                                   const InstallationState& machine_state,
-                                   const Version& new_version,
-                                   const Product& product,
-                                   WorkItemList* work_item_list) {
-  DCHECK(product.is_chrome_app_host());
-  AddCommandWithParameterWorkItems(installer_state, machine_state, new_version,
-                                   product, kCmdInstallApp,
-                                   installer::kChromeAppHostExe,
-                                   ::switches::kInstallFromWebstore,
-                                   work_item_list);
-}
-
-void AddInstallExtensionCommandWorkItem(const InstallerState& installer_state,
-                                        const InstallationState& machine_state,
-                                        const base::FilePath& setup_path,
-                                        const Version& new_version,
-                                        const Product& product,
-                                        WorkItemList* work_item_list) {
-  DCHECK(product.is_chrome());
-  AddCommandWithParameterWorkItems(installer_state, machine_state, new_version,
-                                   product, kCmdInstallExtension,
-                                   installer::kChromeExe,
-                                   ::switches::kLimitedInstallFromWebstore,
-                                   work_item_list);
+void AddLegacyAppCommandRemovalItem(const InstallerState& installer_state,
+                                    BrowserDistribution* distribution,
+                                    const wchar_t* name,
+                                    WorkItemList* work_item_list) {
+  // These failures are ignored because this is a clean-up operation that
+  // shouldn't block an install or update on failing.
+  work_item_list->AddDeleteRegKeyWorkItem(
+      installer_state.root_key(),
+      GetRegCommandKey(distribution, name),
+      KEY_WOW64_32KEY)->set_ignore_failure(true);
 }
 
 // A callback invoked by |work_item| that adds firewall rules for Chrome. Rules
 // are left in-place on rollback unless |remove_on_rollback| is true. This is
 // the case for new installs only. Updates and overinstalls leave the rule
 // in-place on rollback since a previous install of Chrome will be used in that
 // case.
 bool AddFirewallRulesCallback(bool system_level,
                               BrowserDistribution* dist,
                               const base::FilePath& chrome_path,
@@ skipping 126 matching lines @@
 void AddProductSpecificWorkItems(const InstallationState& original_state,
                                  const InstallerState& installer_state,
                                  const base::FilePath& setup_path,
                                  const Version& new_version,
                                  bool is_new_install,
                                  WorkItemList* list) {
   const Products& products = installer_state.products();
   for (Products::const_iterator it = products.begin(); it < products.end();
        ++it) {
     const Product& p = **it;
-    if (p.is_chrome_app_host()) {
-      AddInstallAppCommandWorkItems(installer_state, original_state,
-                                    new_version, p, list);
-    }
     if (p.is_chrome()) {
       AddOsUpgradeWorkItems(installer_state, setup_path, new_version, p,
                             list);
-      AddInstallExtensionCommandWorkItem(installer_state, original_state,
-                                         setup_path, new_version, p, list);
       AddFirewallRulesWorkItems(
           installer_state, p.distribution(), is_new_install, list);
+      AddLegacyAppCommandRemovalItem(
+          installer_state, p.distribution(), kLegacyCmdInstallExtension, list);
     }
     if (p.is_chrome_binaries()) {
       AddQueryEULAAcceptanceWorkItems(
           installer_state, setup_path, new_version, p, list);
       AddQuickEnableChromeFrameWorkItems(installer_state, list);
       AddQuickEnableApplicationLauncherWorkItems(
           installer_state, original_state, setup_path, new_version, list);
     }
   }
 }
@@ skipping 912 matching lines @@
   }
 
   // Add any remaining work items that involve special settings for
   // each product.
   AddProductSpecificWorkItems(original_state,
                               installer_state,
                               setup_path,
                               new_version,
                               current_version == NULL,
                               install_list);
+  // Unconditionally remove the "install-application" command from the app
+  // hosts's key.
+  AddLegacyAppCommandRemovalItem(

[grt (UTC plus 2), 2014/09/30 20:39:24]

oh dear. i just noticed that there isn't a Chromium-branded variant of the
CHROME_APP_HOST distribution, so this is actually the wrong thing for non-Google
Chrome builds and for canary. fortunately,
BrowserDistribution::AppHostIsSupported() will return true only for the Google
Chrome CHROME_BROWSER distribution (and false for Chromium and Google Chrome
Canary). given that, i think the shortest-path to getting this right is to put
it into the is_chrome() block of AddProductSpecificWorkItems like so:

    if (p.is_chrome()) {
      ...
      if (p.distribution()->AppHostIsSupported()) {
        // Unconditionally remove the "install-application" command from the
        // app hosts's key.
        ...
      }
    }

apologies for not noticing this sooner.

[Devlin, 2014/09/30 20:55:50]

Done.

To confirm, this does mean we will never remove the key if
product.is_chrome_app_host() (the only time the key was previously added).  Is
that okay?

[grt (UTC plus 2), 2014/10/01 13:56:22]

Yes. The app host was generally installed along with Google Chrome, so
installer_state.products() most likely contains Chrome. There were exceptions,
but I don't think we need to be concerned with them now.

+       installer_state,
+       BrowserDistribution::GetSpecificDistribution(
+           BrowserDistribution::CHROME_APP_HOST),
+       kLegacyCmdInstallApp,
+       install_list);
 
   // Copy over brand, usagestats, and other values.
   AddGoogleUpdateWorkItems(original_state, installer_state, install_list);
 
   // Append the tasks that run after the installation.
   AppendPostInstallTasks(installer_state,
                          setup_path,
                          current_version,
                          new_version,
                          temp_path,
@@ skipping 301 matching lines @@
   // Unconditionally remove the legacy Quick Enable command from the binaries.
   // Do this even if multi-install Chrome isn't installed to ensure that it is
   // not left behind in any case.
   work_item_list->AddDeleteRegKeyWorkItem(
                       installer_state.root_key(), cmd_key, KEY_WOW64_32KEY)
       ->set_log_message("removing " + base::UTF16ToASCII(kCmdQuickEnableCf) +
                         " command");
 }
 
 }  // namespace installer