Adding App Launcher shortcuts on install.

chrome/installer/setup/uninstall.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 defines the methods useful for uninstalling Chrome.
 
 #include "chrome/installer/setup/uninstall.h"
 
 #include <windows.h>
 
@@ skipping 238 matching lines @@
     }
   }
 
   if (kill) {
     VLOG(1) << installer::kChromeFrameHelperExe << " hung.  Killing.";
     base::CleanupProcesses(installer::kChromeFrameHelperExe, base::TimeDelta(),
                            content::RESULT_CODE_HUNG, NULL);
   }
 }
 
+// Deletes shortcuts at |install_level| from Start menu, Desktop,

[erikwright (departed), 2012/11/07 21:15:30]

I guess this is to be removed?

[huangs, 2012/11/07 21:41:59]

I was trying to see if there's anything that can be salvaged.  Removed.

[gab, 2012/11/07 21:44:40]

I'd say the opposite, add "taskbar" to this list and keep this comment.
Remove the old comment on DeleteShortcuts().

[huangs, 2012/11/07 22:14:46]

Done, replaced old comment with 3-line comment.

+// Quick Launch, and secondary tiles on the Start Screen (Win8+).
+// Only shortcuts pointing to |target| will be removed.
+
+
 // This method deletes the product's shortcut folder from the
 // 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 DeleteShortcuts(const InstallerState& installer_state,

[gab, 2012/11/07 21:44:40]

And see, that's why we asked you not to commit your "simple addition of
DeleteShortcutsCommon()" as part of your previous CL; to keep it in sync with
the motivating changes (this CL). Because now the motivating changes don't need
this anymore.

You never know and there is no point doing something before you actually need it
;)!

Cheers!

[huangs, 2012/11/07 22:14:46]

!

                      const Product& product,
                      const string16& target_exe) {
   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;
 
   VLOG(1) << "Deleting Desktop shortcut.";
@@ skipping 768 matching lines @@
       new_cmd.AppendSwitch(installer::switches::kRemoveChromeRegistration);
       if (!suffix.empty()) {
         new_cmd.AppendSwitchNative(
             installer::switches::kRegisterChromeBrowserSuffix, suffix);
       }
       DWORD exit_code = installer::UNKNOWN_STATUS;
       InstallUtil::ExecuteExeAsAdmin(new_cmd, &exit_code);
     }
   }
 
+  if (product.is_chrome_app_host()) {
+    // Remove this check once we have system-level App Host.

[gab, 2012/11/07 21:44:40]

Add TODO in front of comment.

[huangs, 2012/11/07 22:14:46]

Done.

+    DCHECK(!installer_state.system_install());
+    const string16 app_host_exe(
+        installer_state.target_path().Append(installer::kChromeAppHostExe)
+            .value());
+    // Delete shortcuts from Start menu, Desktop, and Quick Launch.
+    DeleteShortcuts(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.
   if (is_chrome) {

[gab, 2012/11/07 21:44:40]

else if {} from block above, the product can't both be app_host and chrome.

I think I actually prefer it to be flipped, i.e.:
if (is_chrome) {
  ...
} else if (is_app_host) {
  ...
}
but I'm fine either way.

[huangs, 2012/11/07 22:14:46]

Done (using else if; handling is_chrome first).

     ClearRlzProductState();
     // Delete the key that delegate_execute might make.
     if (base::win::GetVersion() >= base::win::VERSION_WIN8) {
       InstallUtil::DeleteRegistryKey(HKEY_CURRENT_USER,
                                      chrome::kMetroRegistryPath);
     }
 
     auto_launch_util::DisableAllAutoStartFeatures(
         ASCIIToUTF16(chrome::kInitialProfile));
 
-    // First delete shortcuts from Start->Programs, Desktop & Quick Launch.
+    // Delete shortcuts from Start menu, Desktop, and Quick Launch.

[gab, 2012/11/07 21:44:40]

This comment is now also missing taskbar and start screen... in general I think
it's overly verbose, the call itself is pretty implicit. I'd say just remove it.

[huangs, 2012/11/07 22:14:46]

Done (removed both comments from the 2 callers).

     DeleteShortcuts(installer_state, product, chrome_exe);
   }
 
   // Delete the registry keys (Uninstall key and Version key).
   HKEY reg_root = installer_state.root_key();
 
   // Note that we must retrieve the distribution-specific data before deleting
   // product.GetVersionKey().
   string16 distribution_data(browser_dist->GetDistributionData(reg_root));
 
@@ skipping 184 matching lines @@
 
   // Try and delete the preserved local state once the post-install
   // operations are complete.
   if (!backup_state_file.empty())
     file_util::Delete(backup_state_file, false);
 
   return ret;
 }
 
 }  // namespace installer