Remove kCmdInstallExtension and kCmdInstallApp from Installer code

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 d3366303404614eae086cd17f8ba6f37dcd9bd50..b3a6a1aa95bf86aff052bd2977e2ad756b7eedf0 100644
--- a/chrome/installer/setup/install_worker.cc
+++ b/chrome/installer/setup/install_worker.cc
@@ -71,6 +71,11 @@ const wchar_t kIELowRightsPolicyOldGuid[] =
 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);
@@ -230,31 +235,16 @@ void AddCommandWithParameterWorkItems(const InstallerState& installer_state,
   }
 }
 
-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 AddLegacyRemovalItem(const InstallerState& installer_state,

[grt (UTC plus 2), 2014/09/30 18:30:32]

since this specifically removes an app command, how about
AddLegacyAppCommandRemovalItem?

[Devlin, 2014/09/30 18:51:01]

It actually removes anything that's passed in via |name| (currently App or
Extension) so saying AppCommand is a bit misleading, no?

[grt (UTC plus 2), 2014/09/30 18:59:26]

name collision! "AppCommand" is the generic term for these types of commands
that are registered with Google Update, and is orthogonal to Chrome Apps or
Chrome Extensions. it is a coincidence that the two AppCommands you're removing
are called "install-extension" and "install-application".

[Devlin, 2014/09/30 19:10:12]

Ahh, gotcha. Done.

+                          const Product& product,
+                          const wchar_t* name,
+                          WorkItemList* work_item_list) {
+  base::string16 full_cmd_key(

[grt (UTC plus 2), 2014/09/30 18:30:32]

nit: get rid of this local var and call GetRegCommandKey in-line below

[Devlin, 2014/09/30 18:51:01]

Done.

+      GetRegCommandKey(product.distribution(), name));
+  work_item_list->AddDeleteRegKeyWorkItem(

[grt (UTC plus 2), 2014/09/30 18:30:32]

please add a comment stating that failures are ignored since this is a clean-up
operation that shouldn't block an install/update on failure.

[Devlin, 2014/09/30 18:51:02]

Done.

+      installer_state.root_key(),
+      full_cmd_key,
+      KEY_WOW64_32KEY)->set_ignore_failure(true);
 }
 
 // A callback invoked by |work_item| that adds firewall rules for Chrome. Rules
@@ -401,17 +391,15 @@ void AddProductSpecificWorkItems(const InstallationState& original_state,
   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_app_host())
+      AddLegacyRemovalItem(installer_state, p, kLegacyCmdInstallApp, list);

[Devlin, 2014/09/30 18:03:01]

I think in your email you recommended adding this removal item for all products,
but it was only added for the chrome_app_host product, and the cmd key is
generated based on the product distribution - should we still add it for any/all
products?

[grt (UTC plus 2), 2014/09/30 18:30:32]

To be safe, I think so. I can't say for sure that it's possible for the
kCmdInstallApp command to be registered without the app host product being in
the set of products. It's safe to remove it regardless.

     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);
+      AddLegacyRemovalItem(
+          installer_state, p, kLegacyCmdInstallExtension, list);
     }
     if (p.is_chrome_binaries()) {
       AddQueryEULAAcceptanceWorkItems(