Adding App Launcher shortcuts on install.

chrome/installer/setup/uninstall.cc

Index: chrome/installer/setup/uninstall.cc
diff --git a/chrome/installer/setup/uninstall.cc b/chrome/installer/setup/uninstall.cc
index 2fb193b801d739f67c339aa1ae25dc8ecc1d9c93..62e2549e74a85172d3448ddb9744fccd8a12b33f 100644
--- a/chrome/installer/setup/uninstall.cc
+++ b/chrome/installer/setup/uninstall.cc
@@ -256,59 +256,78 @@ void CloseChromeFrameHelperProcess() {
   }
 }
 
-// This method deletes Chrome shortcut folder from Windows Start menu. It
-// checks system_uninstall to see if the shortcut is in all users start menu
-// or current user start menu.
-// We try to remove the standard desktop shortcut but if that fails we try
-// to remove the alternate desktop shortcut. Only one of them should be
-// present in a given install but at this point we don't know which one.
-// We remove all start screen secondary tiles by removing the folder Windows
-// uses to store this installation's tiles.
-void DeleteChromeShortcuts(const InstallerState& installer_state,
-                           const Product& product,
-                           const string16& chrome_exe) {
-  if (!product.is_chrome()) {
-    VLOG(1) << __FUNCTION__ " called for non-CHROME distribution";
-    return;
-  }
-
-  BrowserDistribution* dist = product.distribution();
-
-  // The per-user shortcut for this user, if present on a system-level install,
-  // has already been deleted in chrome_browser_main_win.cc::DoUninstallTasks().
-  ShellUtil::ShellChange install_level = installer_state.system_install() ?
-      ShellUtil::SYSTEM_LEVEL : ShellUtil::CURRENT_USER;
-
+// Deletes shortcuts at |install_level| from Start menu, Desktop,
+// Quick Launch, and secondary tiles on the Start Screen (Win8+).
+// Only shortcuts pointing to |target| will be removed.
+void DeleteShortcutsCommon(ShellUtil::ShellChange install_level,
+                           BrowserDistribution* dist,
+                           const string16& target) {
   VLOG(1) << "Deleting Desktop shortcut.";
   if (!ShellUtil::RemoveChromeShortcut(
-          ShellUtil::SHORTCUT_DESKTOP, dist, chrome_exe, install_level, NULL)) {
+          ShellUtil::SHORTCUT_DESKTOP, dist, target, install_level, NULL)) {
     LOG(WARNING) << "Failed to delete Desktop shortcut.";
   }
   // Also try to delete the alternate desktop shortcut. It is not sufficient
   // to do so upon failure of the above call as ERROR_FILE_NOT_FOUND on
   // delete is considered success.
   if (!ShellUtil::RemoveChromeShortcut(
-          ShellUtil::SHORTCUT_DESKTOP, dist, chrome_exe, install_level,
+          ShellUtil::SHORTCUT_DESKTOP, dist, target, install_level,
           &dist->GetAlternateApplicationName())) {
     LOG(WARNING) << "Failed to delete alternate Desktop shortcut.";
   }
 
   VLOG(1) << "Deleting Quick Launch shortcut.";
   if (!ShellUtil::RemoveChromeShortcut(
-          ShellUtil::SHORTCUT_QUICK_LAUNCH, dist, chrome_exe, install_level,
+          ShellUtil::SHORTCUT_QUICK_LAUNCH, dist, target, install_level,
           NULL)) {
     LOG(WARNING) << "Failed to delete Quick Launch shortcut.";
   }
 
   VLOG(1) << "Deleting Start Menu shortcuts.";
   if (!ShellUtil::RemoveChromeShortcut(
-          ShellUtil::SHORTCUT_START_MENU, dist, chrome_exe, install_level,
+          ShellUtil::SHORTCUT_START_MENU, dist, target, install_level,
           NULL)) {
     LOG(WARNING) << "Failed to delete Start Menu shortcuts.";
   }
 
-  ShellUtil::RemoveChromeStartScreenShortcuts(product.distribution(),
-                                              chrome_exe);
+  ShellUtil::RemoveChromeStartScreenShortcuts(dist, target);
+}
+
+// Deletes Chrome shortcuts as per DeleteShortcutsCommon().
+void DeleteChromeShortcuts(const InstallerState& installer_state,
+                           const Product& product,
+                           const string16& chrome_exe) {
+  if (!product.is_chrome()) {
+    VLOG(1) << __FUNCTION__ " called for non-Chrome distribution";
+    return;
+  }
+
+  VLOG(1) << "Deleting Chrome shortcuts.";
+
+  BrowserDistribution* dist = product.distribution();
+  // The per-user shortcut for this user, if present on a system-level install,
+  // has already been deleted in chrome_browser_main_win.cc::DoUninstallTasks().
+  ShellUtil::ShellChange install_level = installer_state.system_install() ?
+      ShellUtil::SYSTEM_LEVEL : ShellUtil::CURRENT_USER;
+  DeleteShortcutsCommon(install_level, dist, chrome_exe);
+}
+
+// Deletes App Launcher shortcuts as per DeleteShortcutsCommon().
+void DeleteAppLauncherShortcuts(const InstallerState& installer_state,
+                                const Product& product,
+                                const string16& app_host_exe) {
+  if (!product.is_chrome_app_host()) {
+    VLOG(1) << __FUNCTION__ " called for non-App Launcher distribution";
+    return;
+  }
+
+  VLOG(1) << "Deleting App Launcher shortcuts.";
+  ShellUtil::ShellChange install_level = installer_state.system_install() ?
+      ShellUtil::SYSTEM_LEVEL : ShellUtil::CURRENT_USER;
+
+  // Remove this check once we have system-level App Host.
+  DCHECK(install_level == ShellUtil::CURRENT_USER);

[gab, 2012/11/02 04:19:58]

I find this weird... if you want to keep the above logic for |install_level|,
remove this DCHECK() -- otherwise you "support it", but the DCHECK prevents it
from actually working in debug builds... (i.e. you can "implement" it by
removing the DCHECK...).

Otherwise make it:
DCHECK(!installer_state.system_install());
ShellUtil::ShellChange install_level = ShellUtil::CURRENT_USER;

[erikwright (departed), 2012/11/02 04:31:01]

There's presumably more code that would be needed if we wanted to support system
level installs.

For now we don't, and code elsewhere will bail if --system-level --app-launcher.

I asked Sam to go ahead and pass through the install level (don't hard code it
to user-level) but to DCHECK or NOTREACHED anywhere that additional code would
be required so that we will run into these areas if/when we do implement
system-level launcher installs.

[gab, 2012/11/02 04:39:54]

Well, no additional code would be required here that's the thing...

Maybe a DCHECK(!installer_state.system_install) at the callsite would make more
sense?

[erikwright (departed), 2012/11/02 04:48:39]

If that's the case (I hadn't looked, assumed it wasn't, but now I see that it
seems to be) then yes, this DCHECK should go.

[gab, 2012/11/02 12:43:35]

Well, thinking more about it, when you do implement system-level installs you
will likely want per-user shortcut as Chrome is now doing so that upon uninstall
you'll still only need to remove per-user shortcuts. We need to also check for
all-users shortcuts for Chrome as they used to exist, but if you never had them
you don't need to check for that, so this here should always be CURRENT_USER
(i.e. remove for uninstalling user, other users will eventually get a message
from Windows telling them their shortcut now point to an invalid target with a
YES/NO remove dialog).

[grt (UTC plus 2), 2012/11/02 19:55:30]

DCHECK_EQ

[huangs, 2012/11/02 21:05:10]

Moved the check to the caller.

[huangs, 2012/11/02 21:05:10]

Thanks (not applicable for this case any more though)!

+  DeleteShortcutsCommon(install_level, product.distribution(), app_host_exe);
 }
 
 bool ScheduleParentAndGrandparentForDeletion(const FilePath& path) {
@@ -1053,6 +1072,13 @@ InstallStatus UninstallProduct(const InstallationState& original_state,
     }
   }
 
+  if (product.is_chrome_app_host()) {
+    const string16 app_host_exe(installer_state
+        .target_path().Append(installer::kChromeAppHostExe).value());

[gab, 2012/11/02 04:19:58]

nit: style dictates that either you wrap at the paranthesis (which doesn't fit
here, or you wrap everything 4 spaces.

So in this case I think you'll have to do something like:
const string16 app_host_exe(
    installer_state.target_path().Append(installer::kChromeAppHostExe)
        .value());

[huangs, 2012/11/02 21:05:10]

Done.

+    // Delete shortcuts from Start menu, Desktop, and Quick Launch.
+    DeleteAppLauncherShortcuts(installer_state, product, app_host_exe);
+  }
+
   // Chrome is not in use so lets uninstall Chrome by deleting various files
   // and registry entries. Here we will just make best effort and keep going
   // in case of errors.
@@ -1067,7 +1093,7 @@ InstallStatus UninstallProduct(const InstallationState& original_state,
     auto_launch_util::DisableAllAutoStartFeatures(
         ASCIIToUTF16(chrome::kInitialProfile));
 
-    // First delete shortcuts from Start->Programs, Desktop & Quick Launch.
+    // First delete shortcuts from Start menu, Desktop, and Quick Launch.

[gab, 2012/11/02 04:19:58]

Again, remove "First" as this is the only thing you do here :).

[huangs, 2012/11/02 21:05:10]

Done.

     DeleteChromeShortcuts(installer_state, product, chrome_exe);
   }