Always suffix ChromeHTML entries on Windows for user-level installs.

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 8a56753df03ec07fe1b2f9ee2b29da593cfa85bb..f7ab163cb39a8aeeb7161cf145dd7c5ed5393810 100644
--- a/chrome/installer/setup/uninstall.cc
+++ b/chrome/installer/setup/uninstall.cc
@@ -220,33 +220,6 @@ void CloseChromeFrameHelperProcess() {
   }
 }
 
-// This method tries to figure out if current user has registered Chrome.
-// It returns true iff there is a registered browser that will launch the
-// same chrome.exe as the current installation.
-bool CurrentUserHasDefaultBrowser(const InstallerState& installer_state) {
-  using base::win::RegistryKeyIterator;
-  const HKEY root = HKEY_LOCAL_MACHINE;
-  InstallUtil::ProgramCompare open_command_pred(
-      installer_state.target_path().Append(kChromeExe));
-  string16 client_open_path;
-  RegKey client_open_key;
-  string16 reg_exe;
-  for (RegistryKeyIterator iter(root, ShellUtil::kRegStartMenuInternet);
-       iter.Valid(); ++iter) {
-    client_open_path.assign(ShellUtil::kRegStartMenuInternet)
-        .append(1, L'\\')
-        .append(iter.Name())
-        .append(ShellUtil::kRegShellOpen);
-    if (client_open_key.Open(root, client_open_path.c_str(),
-                             KEY_QUERY_VALUE) == ERROR_SUCCESS &&
-        client_open_key.ReadValue(L"", &reg_exe) == ERROR_SUCCESS &&
-        open_command_pred.Evaluate(reg_exe)) {
-      return true;
-    }
-  }
-  return false;
-}
-
 // 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.
@@ -664,7 +637,8 @@ bool DeleteChromeRegistrationKeys(BrowserDistribution* dist, HKEY root,
   return true;
 }
 
-void RemoveChromeLegacyRegistryKeys(BrowserDistribution* dist) {
+void RemoveChromeLegacyRegistryKeys(BrowserDistribution* dist,
+                                    const string16& chrome_exe) {
   // We used to register Chrome to handle crx files, but this turned out
   // to be not worth the hassle. Remove these old registry entries if
   // they exist. See: http://codereview.chromium.org/210007
@@ -678,9 +652,8 @@ const wchar_t kChromeExtProgId[] = L"ChromiumExt";
   HKEY roots[] = { HKEY_LOCAL_MACHINE, HKEY_CURRENT_USER };
   for (size_t i = 0; i < arraysize(roots); ++i) {
     string16 suffix;
-    if (roots[i] == HKEY_LOCAL_MACHINE &&
-        !ShellUtil::GetUserSpecificDefaultBrowserSuffix(dist, &suffix))
-      suffix = L"";
+    if (roots[i] == HKEY_LOCAL_MACHINE)
+      suffix = ShellUtil::GetCurrentInstallationSuffix(dist, chrome_exe);
 
     // Delete Software\Classes\ChromeExt,
     string16 ext_prog_id(ShellUtil::kRegClasses);
@@ -730,12 +703,13 @@ InstallStatus UninstallProduct(const InstallationState& original_state,
                                bool force_uninstall,
                                const CommandLine& cmd_line) {
   InstallStatus status = installer::UNINSTALL_CONFIRMED;
-  string16 suffix;
-  if (!ShellUtil::GetUserSpecificDefaultBrowserSuffix(product.distribution(),
-                                                      &suffix))
-    suffix = L"";
-
   BrowserDistribution* browser_dist = product.distribution();
+  const string16 chrome_exe(
+      installer_state.target_path().Append(installer::kChromeExe).value());
+
+  const string16 suffix(ShellUtil::GetCurrentInstallationSuffix(browser_dist,
+                                                                chrome_exe));
+
   bool is_chrome = product.is_chrome();
 
   VLOG(1) << "UninstallProduct: " << browser_dist->GetApplicationName();
@@ -753,11 +727,15 @@ InstallStatus UninstallProduct(const InstallationState& original_state,
         status != installer::UNINSTALL_DELETE_PROFILE)
       return status;
 
-    // Check if we need admin rights to cleanup HKLM. If we do, try to launch
-    // another uninstaller (silent) in elevated mode to do HKLM cleanup.
+    // Check if we need admin rights to cleanup HKLM (the conditions for
+    // requiring a cleanup are the same as the conditions to do the actual
+    // cleanup where DeleteChromeRegistrationKeys() is invoked for
+    // HKEY_LOCAL_MACHINE below). If we do, try to launch another uninstaller
+    // (silent) in elevated mode to do HKLM cleanup.
     // And continue uninstalling in the current process also to do HKCU cleanup.
     if (remove_all &&
-        (!suffix.empty() || CurrentUserHasDefaultBrowser(installer_state)) &&
+        ShellUtil::QuickIsChromeRegisteredInHKLM(
+            browser_dist, chrome_exe, suffix) &&
         !::IsUserAnAdmin() &&
         base::win::GetVersion() >= base::win::VERSION_VISTA &&
         !cmd_line.HasSwitch(installer::switches::kRunAsAdmin)) {
@@ -811,21 +789,40 @@ InstallStatus UninstallProduct(const InstallationState& original_state,
   // Registration data is put in HKCU for both system level and user level
   // installs.
   InstallStatus ret = installer::UNKNOWN_STATUS;
-  DeleteChromeRegistrationKeys(product.distribution(), HKEY_CURRENT_USER,
-                               suffix, installer_state.target_path(), &ret);
+  DeleteChromeRegistrationKeys(browser_dist, HKEY_CURRENT_USER, suffix,
+                               installer_state.target_path(), &ret);
+
+  // If the user's Chrome is registered with a suffix: it is possible that old
+  // unsuffixed registrations were left in HKCU (e.g. if this install was
+  // previously installed with no suffix in HKCU (old suffix rules if the user
+  // is not an admin (or declined UAC at first run)) and later had to be
+  // suffixed when fully registered in HKLM (e.g. when later making Chrome
+  // default through the UI)).
+  // Remove remaining HKCU entries with no suffix if any.
+  if (!suffix.empty()) {
+    DeleteChromeRegistrationKeys(browser_dist, HKEY_CURRENT_USER, string16(),
+                                 installer_state.target_path(), &ret);
+  }
 
   // Chrome is registered in HKLM for all system-level installs and for
   // user-level installs for which Chrome has been made the default browser.
   // Always remove the HKLM registration for system-level installs.  For
-  // user-level installs, only remove it if both: 1) this uninstall isn't a
-  // self-destruct following the installation of system-level Chrome (because
-  // the system-level Chrome owns the HKLM registration now), and 2) this user
-  // had made Chrome their default browser.
+  // user-level installs, only remove it if both: 1) this uninstall isn't a self
+  // destruct following the installation of a system-level Chrome (because the
+  // system-level Chrome owns the HKLM registration now), and 2) this user has
+  // made Chrome their default browser (i.e. has system entries registered with
+  // |suffix| (note: |suffix| will be the empty string if required as it is
+  // obtained by GetCurrentInstallationSuffix() above)).
+  // TODO(gab): This can still leave parts of a suffixed install behind. To be
+  // able to remove them we would need to be able to remove only suffixed
+  // entries (as it is now some of the system entries are unsuffixed; thus
+  // removing suffixed installs is prohibited in HKLM if !|remove_all| for now).
   if (installer_state.system_install() ||
       (remove_all &&
-       (!suffix.empty() || CurrentUserHasDefaultBrowser(installer_state)))) {
-    DeleteChromeRegistrationKeys(product.distribution(), HKEY_LOCAL_MACHINE,
-                                 suffix, installer_state.target_path(), &ret);
+       ShellUtil::QuickIsChromeRegisteredInHKLM(
+          browser_dist, chrome_exe, suffix))) {

[grt (UTC plus 2), 2012/06/13 17:31:47]

nit: looks like this needs one more space to be 4-space indented from the line
above

[gab, 2012/06/13 20:00:47]

Done.

+    DeleteChromeRegistrationKeys(browser_dist, HKEY_LOCAL_MACHINE, suffix,
+                                 installer_state.target_path(), &ret);
   }
 
   ProcessDelegateExecuteWorkItems(installer_state, product);