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 86b374fa710dcfb48b072e5f3ab8fa08161bb737..05b4e8ac92749fdcff9eb235e9c44d37253e55df 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;
-  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.
@@ -675,13 +648,10 @@ const wchar_t kChromeExtProgId[] = L"ChromeExt";
 const wchar_t kChromeExtProgId[] = L"ChromiumExt";
 #endif
 
+  const string16 suffix(ShellUtil::GetCurrentInstallationSuffix());
+
   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 &&

[gab, 2012/05/30 00:06:24]

This was introduced in 2009 in
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/setup/uninst...

however the corresponding codereview (http://codereview.chromium.org/210007)
does not have this line.

It's not clear to me why the suffix is only applied for HKLM... can you comment
on this section of the change please.

We can actually probably remove this function entirely given it is doing some
cleanup needed 2.5 years ago...

[gab, 2012/05/30 02:45:31]

Ok, thanks aa@, I'll remove it: https://chromiumcodereview.appspot.com/10454067/

[gab, 2012/05/30 03:16:42]

Actually, as discussed with grt@, we won't remove this (ping me offline for
details).

I modified this CL to reflect exactly the current behavior for this function
with my new set of calls.

-        !ShellUtil::GetUserSpecificDefaultBrowserSuffix(dist, &suffix))
-      suffix = L"";
-
     // Delete Software\Classes\ChromeExt,
     string16 ext_prog_id(ShellUtil::kRegClasses);
     ext_prog_id.push_back(FilePath::kSeparators[0]);
@@ -730,12 +700,12 @@ 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());
+
   bool is_chrome = product.is_chrome();
 
   VLOG(1) << "UninstallProduct: " << browser_dist->GetApplicationName();
@@ -756,8 +726,9 @@ InstallStatus UninstallProduct(const InstallationState& original_state,
     // 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.
     // And continue uninstalling in the current process also to do HKCU cleanup.
-    if (remove_all &&
-        (!suffix.empty() || CurrentUserHasDefaultBrowser(installer_state)) &&
+    if (ShellUtil::IsInstallationPresentInHKLM(browser_dist, chrome_exe,
+                                               suffix) &&
+        (!suffix.empty() || remove_all) &&
         !::IsUserAnAdmin() &&
         base::win::GetVersion() >= base::win::VERSION_VISTA &&
         !cmd_line.HasSwitch(installer::switches::kRunAsAdmin)) {
@@ -811,21 +782,29 @@ 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) and later
+  // had to be suffixed when fully registered in HKLM).
+  // Remove remaining HKCU entries with no suffix if any.
+  if (!suffix.empty()) {
+    DeleteChromeRegistrationKeys(browser_dist, HKEY_CURRENT_USER, L"",
+                                 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.
-  if (installer_state.system_install() ||
-      (remove_all &&
-       (!suffix.empty() || CurrentUserHasDefaultBrowser(installer_state)))) {
-    DeleteChromeRegistrationKeys(product.distribution(), HKEY_LOCAL_MACHINE,
-                                 suffix, installer_state.target_path(), &ret);
+  // user-level installs, only remove it if either: 1) The registration has a
+  // suffix, or 2) |remove_all| is true which means this is not a self-destruct
+  // following a system-level Chrome install (which would otherwise need the
+  // same HKLM registrations).
+  if (installer_state.system_install() || !suffix.empty() || remove_all) {
+    DeleteChromeRegistrationKeys(browser_dist, HKEY_LOCAL_MACHINE, suffix,
+                                 installer_state.target_path(), &ret);
   }
 
   ProcessDelegateExecuteWorkItems(installer_state, product);