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 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
 // 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_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(
           installer_state, setup_path, new_version, p, list);
       AddQuickEnableChromeFrameWorkItems(installer_state, list);
       AddQuickEnableApplicationLauncherWorkItems(
           installer_state, original_state, setup_path, new_version, list);
     }
   }
 }
@@ skipping 1233 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