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

chrome/installer/util/shell_util.cc

Index: chrome/installer/util/shell_util.cc
diff --git a/chrome/installer/util/shell_util.cc b/chrome/installer/util/shell_util.cc
index ef7177ec96fb95b042d23b8defb0c757c4323347..c9c805e17af2465445b40f580e2a6c5443316874 100644
--- a/chrome/installer/util/shell_util.cc
+++ b/chrome/installer/util/shell_util.cc
@@ -74,6 +74,12 @@ bool IsChromeMetroSupported() {
 // class.
 class RegistryEntry {
  public:
+  // A bit-field enum of places to look for this key in the Windows registry.
+  enum LookForIn {
+    LOOK_IN_HKCU = 1 << 0,
+    LOOK_IN_HKLM = 1 << 1,

[grt (UTC plus 2), 2012/06/04 02:18:26]

consider adding LOOK_IN_HKCU_THEN_HKLM or something like that so that consumers
don't need to OR the two values together when they want both.

[gab, 2012/06/05 19:50:02]

Done.

+  };
+
   // Returns the Windows browser client registration key for Chrome.  For
   // example: "Software\Clients\StartMenuInternet\Chromium[.user]".  Strictly
   // speaking, we should use the name of the executable (e.g., "chrome.exe"),
@@ -98,6 +104,22 @@ class RegistryEntry {
     return GetBrowserClientKey(dist, suffix).append(L"\\Capabilities");
   }
 
+  // Returns true if Chrome is registered with the given |suffix| either in HKCU
+  // or HKLM.
+  // This function only checks for the existence of
+  // Software\Classes\ChromeHTML|suffix| and doesn't otherwise validate the
+  // registration of Chrome.

[grt (UTC plus 2), 2012/06/04 02:18:26]

Please explain in the comment why checking only the ProgID is sufficient.

[gab, 2012/06/05 19:50:02]

Done in the new enum RegistrationConfirmationLevel in shell_util.h

+  static bool IsChromeRegisteredWithSuffix(const string16& suffix) {
+    DCHECK(!suffix.empty());
+    string16 chrome_html_prog_id(ShellUtil::kRegClasses);
+    chrome_html_prog_id.push_back(FilePath::kSeparators[0]);
+    chrome_html_prog_id.append(ShellUtil::kChromeHTMLProgId);
+    chrome_html_prog_id.append(suffix);
+    return RegistryEntry(chrome_html_prog_id,
+                         ShellUtil::kChromeHTMLProgIdDesc).ExistsInRegistry(

[grt (UTC plus 2), 2012/06/04 02:18:26]

Checking that the description matches seems fragile.  How about either just
checking for existence of the ProgID (simple), or checking that it points to the
chrome.exe in question (more complex, but gives you the real right answer)?

[gab, 2012/06/05 19:50:02]

Done and merged in QuickChromeIsRegistered().

+                             LOOK_IN_HKCU | LOOK_IN_HKLM);
+  }
+
   // This method returns a list of all the registry entries that
   // are needed to register Chromium ProgIds.
   // These entries should be registered in HKCU for user-level installs and in
@@ -250,11 +272,11 @@ class RegistryEntry {
     entries->push_front(new RegistryEntry(install_info, L"IconsVisible", 1));
 
     // Register with Default Programs.
-    string16 app_name(dist->GetApplicationName().append(suffix));
+    string16 reg_app_name(dist->GetApplicationName().append(suffix));
     // Tell Windows where to find Chrome's Default Programs info.
     string16 capabilities(GetCapabilitiesKey(dist, suffix));
     entries->push_front(new RegistryEntry(ShellUtil::kRegRegisteredApplications,
-        app_name, capabilities));
+        reg_app_name, capabilities));
     // Write out Chrome's Default Programs info.
     // TODO(grt): http://crbug.com/75152 Write a reference to a localized
     // resource rather than this.
@@ -264,10 +286,11 @@ class RegistryEntry {
     entries->push_front(new RegistryEntry(
         capabilities, ShellUtil::kRegApplicationIcon, icon_path));
     entries->push_front(new RegistryEntry(
-        capabilities, ShellUtil::kRegApplicationName, app_name));
+        capabilities, ShellUtil::kRegApplicationName,
+        dist->GetApplicationName()));
 
     entries->push_front(new RegistryEntry(capabilities + L"\\Startmenu",
-        L"StartMenuInternet", app_name));
+        L"StartMenuInternet", reg_app_name));
 
     string16 html_prog_id(ShellUtil::kChromeHTMLProgId);
     html_prog_id.append(suffix);
@@ -377,17 +400,28 @@ class RegistryEntry {
   // Checks if the current registry entry exists in HKCU\|_key_path|\|_name|
   // and value is |_value|. If the key does NOT exist in HKCU, checks for
   // the correct name and value in HKLM.
-  // This mimics Windows' behavior when searching in HKCR (HKCU takes precedence
-  // over HKLM). For registrations outside of HKCR on versions of Windows up
-  // to Win7, Chrome's values go in HKLM. This function will make unnecessary
-  // (but harmless) queries into HKCU in that case. Starting with Windows 8,
-  // Chrome's values go in HKCU for user-level installs, which takes precedence
-  // over HKLM.
-  bool ExistsInRegistry() {
-    RegistryStatus hkcu_status = StatusInRegistryUnderRoot(HKEY_CURRENT_USER);
+  // |look_for_in| specifies roots (HKCU and/or HKLM) in which to look for the
+  // key, unspecified roots are not looked in and the key is assumed not to
+  // exist in them.
+  // |look_for_in| must at least specify one root to look into.
+  // If |look_for_in| specifies both HKCU and HKLM, this method mimics Windows'
+  // behavior when searching in HKCR (HKCU takes precedence over HKLM). For
+  // registrations outside of HKCR on versions of Windows up to Win7, Chrome's
+  // values go in HKLM. This function will make unnecessary (but harmless)
+  // queries into HKCU in that case.
+  bool ExistsInRegistry(uint32 look_for_in) const {
+    DCHECK(look_for_in);
+
+    RegistryStatus hkcu_status = DOES_NOT_EXIST;
+    if (look_for_in & LOOK_IN_HKCU)
+      hkcu_status = StatusInRegistryUnderRoot(HKEY_CURRENT_USER);
+
+    RegistryStatus hklm_status = DOES_NOT_EXIST;
+    if (hkcu_status == DOES_NOT_EXIST && (look_for_in & LOOK_IN_HKLM))
+      hklm_status = StatusInRegistryUnderRoot(HKEY_LOCAL_MACHINE);
+
     return (hkcu_status == SAME_VALUE ||
-            (hkcu_status == DOES_NOT_EXIST &&
-             StatusInRegistryUnderRoot(HKEY_LOCAL_MACHINE) == SAME_VALUE));
+            (hkcu_status == DOES_NOT_EXIST && hklm_status == SAME_VALUE));
   }
 
  private:
@@ -470,14 +504,32 @@ bool AddRegistryEntries(HKEY root, const std::list<RegistryEntry*>& entries) {
   return true;
 }
 
+// Returns true if any of the entries in |entries| are present on this computer.
+// |look_for_in| is passed to RegistryEntry::ExistsInRegistry(). Documentation
+// for it can be found there.
+bool AreAnyEntriesRegistered(const std::list<RegistryEntry*>& entries,
+                          uint32 look_for_in) {

[grt (UTC plus 2), 2012/06/04 02:18:26]

nit: indentation

[gab, 2012/06/05 19:50:02]

Done.

+  bool any = false;
+  for (std::list<RegistryEntry*>::const_iterator itr = entries.begin();
+       !any && itr != entries.end(); ++itr) {
+    // We do not need any = any && ... since the loop condition is set to exit
+    // early.
+    any = (*itr)->ExistsInRegistry(look_for_in);
+  }
+  return any;
+}
+
 // Checks that all |entries| are present on this computer.
-bool AreEntriesRegistered(const std::list<RegistryEntry*>& entries) {
+// |look_for_in| is passed to RegistryEntry::ExistsInRegistry(). Documentation
+// for it can be found there.
+bool AreEntriesRegistered(const std::list<RegistryEntry*>& entries,
+                          uint32 look_for_in) {
   bool registered = true;
   for (std::list<RegistryEntry*>::const_iterator itr = entries.begin();
        registered && itr != entries.end(); ++itr) {
     // We do not need registered = registered && ... since the loop condition
     // is set to exit early.
-    registered = (*itr)->ExistsInRegistry();
+    registered = (*itr)->ExistsInRegistry(look_for_in);
   }
   return registered;
 }
@@ -491,7 +543,8 @@ bool IsChromeRegistered(BrowserDistribution* dist,
   STLElementDeleter<std::list<RegistryEntry*> > entries_deleter(&entries);
   RegistryEntry::GetProgIdEntries(dist, chrome_exe, suffix, &entries);
   RegistryEntry::GetSystemEntries(dist, chrome_exe, suffix, &entries);
-  return AreEntriesRegistered(entries);
+  return AreEntriesRegistered(
+      entries, RegistryEntry::LOOK_IN_HKCU | RegistryEntry::LOOK_IN_HKLM);
 }
 
 // This method checks if Chrome is already registered on the local machine
@@ -502,7 +555,8 @@ bool IsChromeRegisteredForProtocol(BrowserDistribution* dist,
   std::list<RegistryEntry*> entries;
   STLElementDeleter<std::list<RegistryEntry*> > entries_deleter(&entries);
   RegistryEntry::GetProtocolCapabilityEntries(dist, suffix, protocol, &entries);
-  return AreEntriesRegistered(entries);
+  return AreEntriesRegistered(
+      entries, RegistryEntry::LOOK_IN_HKCU | RegistryEntry::LOOK_IN_HKLM);
 }
 
 // This method registers Chrome on Vista by launching an elevated setup.exe.
@@ -555,62 +609,6 @@ bool ElevateAndRegisterChrome(BrowserDistribution* dist,
   return false;
 }
 
-// This method tries to figure out if another user has already registered her
-// own copy of Chrome so that we can avoid overwriting it and append current
-// user's login name to default browser registry entries. This function is
-// not meant to detect all cases. It just tries to handle the most common case.
-// All the conditions below have to be true for it to return true:
-// - Software\Clients\StartMenuInternet\Chromium\"" key should have a valid
-//   value.
-// - The value should not be same as given value in |chrome_exe|
-// - Finally to handle the default install path (C:\Document and Settings\
-//   <user>\Local Settings\Application Data\Chromium\Application) the value
-//   of the above key should differ from |chrome_exe| only in user name.
-bool AnotherUserHasDefaultBrowser(BrowserDistribution* dist,
-                                  const string16& chrome_exe) {
-  const string16 reg_key(
-      RegistryEntry::GetBrowserClientKey(dist, string16())
-      .append(ShellUtil::kRegShellOpen));
-  RegKey key(HKEY_LOCAL_MACHINE, reg_key.c_str(), KEY_READ);
-  string16 registry_chrome_exe;
-  if ((key.ReadValue(L"", &registry_chrome_exe) != ERROR_SUCCESS) ||
-      registry_chrome_exe.length() < 2)
-    return false;
-
-  registry_chrome_exe = registry_chrome_exe.substr(1,
-      registry_chrome_exe.length() - 2);
-  if ((registry_chrome_exe.size() == chrome_exe.size()) &&
-      (std::equal(chrome_exe.begin(), chrome_exe.end(),
-                  registry_chrome_exe.begin(),
-                  base::CaseInsensitiveCompare<wchar_t>()))) {
-    return false;
-  }
-
-  std::vector<string16> v1, v2;
-  base::SplitString(registry_chrome_exe, L'\\', &v1);
-  base::SplitString(chrome_exe, L'\\', &v2);
-  if (v1.empty() || v2.empty() || v1.size() != v2.size())
-    return false;
-
-  // Now check that only one of the values within two '\' chars differ.
-  std::vector<string16>::iterator itr1 = v1.begin();
-  std::vector<string16>::iterator itr2 = v2.begin();
-  bool one_mismatch = false;
-  for ( ; itr1 < v1.end() && itr2 < v2.end(); ++itr1, ++itr2) {
-    string16 s1 = *itr1;
-    string16 s2 = *itr2;
-    if ((s1.size() != s2.size()) ||
-        (!std::equal(s1.begin(), s1.end(),
-                     s2.begin(), base::CaseInsensitiveCompare<wchar_t>()))) {
-      if (one_mismatch)
-        return false;
-      else
-        one_mismatch = true;
-    }
-  }
-  return true;
-}
-
 // Launches the Windows 7 and Windows 8 application association dialog, which
 // is the only documented way to make a browser the default browser on
 // Windows 8.
@@ -647,8 +645,7 @@ uint32 ConvertShellUtilShortcutOptionsToFileUtil(uint32 options) {
 // removal date remains correct).
 void RemoveBadWindows8RegistrationIfNeeded(
     BrowserDistribution* dist,
-    const string16& chrome_exe,
-    const string16& suffix) {
+    const string16& chrome_exe) {
   string16 handler_guid;
 
   if (dist->GetDelegateExecuteHandlerData(&handler_guid, NULL, NULL, NULL) &&
@@ -658,6 +655,10 @@ void RemoveBadWindows8RegistrationIfNeeded(
     // remove the values from the registry.
     const HKEY root_key = InstallUtil::IsPerUserInstall(chrome_exe.c_str()) ?
         HKEY_CURRENT_USER : HKEY_LOCAL_MACHINE;
+    // Use the current installation's suffix, not the about-to-be-installed
+    // suffix.
+    const string16 installation_suffix =

[grt (UTC plus 2), 2012/06/04 02:18:26]

nit: use () rather than =

[gab, 2012/06/05 19:50:02]

Done.

+        ShellUtil::GetCurrentInstallationSuffix();
     const string16 app_id(dist->GetBrowserAppId());
 
     // <root hkey>\Software\Classes\<app_id>
@@ -670,13 +671,52 @@ void RemoveBadWindows8RegistrationIfNeeded(
     key = ShellUtil::kRegClasses;
     key.push_back(FilePath::kSeparators[0]);
     key.append(ShellUtil::kChromeHTMLProgId);
-    key.append(suffix);
+    key.append(installation_suffix);
     key.append(ShellUtil::kRegShellOpen);
     InstallUtil::DeleteRegistryValue(root_key, key,
                                      ShellUtil::kRegDelegateExecute);
   }
 }
 
+// Sets |suffix| to this user's username preceded by a dot. This suffix is then
+// meant to be added to all registration that may conflict with another
+// user-level Chrome install.
+// Returns true unless the OS call to retrieve the username fails.
+bool GetUserSpecificRegistrySuffix(string16* suffix) {
+  wchar_t user_name[256];
+  DWORD size = arraysize(user_name);
+  if (::GetUserName(user_name, &size) == 0 || size < 1) {
+    PLOG(DFATAL) << "Failed to get username from Windows.";

[grt (UTC plus 2), 2012/06/04 02:18:26]

nit: since this log string ends up in the binary, consider just "GetUserName
failed".  PLOG will add ": " + system error message, so don't end the message
with a '.' in this case.

[gab, 2012/06/05 19:50:02]

Done.

+    return false;
+  }
+  suffix->reserve(size);
+  suffix->assign(1, L'.');
+  suffix->append(user_name, size - 1);
+  return true;
+}
+
+// Sets |suffix| to the current user's username, preceded by a dot, on
+// user-level installs.
+// To support old-style user-level installs however, |suffix| is cleared if
+// the user currently owns the non-suffixed HKLM registrations.
+// |suffix| is also cleared on system-level installs.
+// |suffix| should then be appended to all Chrome properties that may conflict
+// with other Chrome user-level installs.
+// Returns true unless one of the underlying calls fails.
+bool GetInstallationSpecificSuffix(BrowserDistribution* dist,
+                                   const string16& chrome_exe,
+                                   string16* suffix) {
+  if (!InstallUtil::IsPerUserInstall(chrome_exe.c_str()) ||
+      ShellUtil::CurrentUserIsRegisteredWithNoSuffix(dist, chrome_exe)) {
+    // No suffix on system-level installs and user-level installs already
+    // registered with no suffix.
+    *suffix = L"";

[grt (UTC plus 2), 2012/06/04 02:18:26]

suffix->clear();

[gab, 2012/06/05 19:50:02]

Done.

+    return true;
+  } else {
+    return GetUserSpecificRegistrySuffix(suffix);
+  }
+}
+
 }  // namespace
 
 const wchar_t* ShellUtil::kRegDefaultIcon = L"\\DefaultIcon";
@@ -923,19 +963,59 @@ void ShellUtil::GetRegisteredBrowsers(
   }
 }
 
-bool ShellUtil::GetUserSpecificDefaultBrowserSuffix(BrowserDistribution* dist,
-                                                    string16* entry) {
-  wchar_t user_name[256];
-  DWORD size = arraysize(user_name);
-  if (::GetUserName(user_name, &size) == 0 || size < 1)
+string16 ShellUtil::GetCurrentInstallationSuffix() {
+  // Make sure we return an empty suffix if this is a system-level install.
+  // This check is needed because it is technically possible for a system-level
+  // install to run when a suffixed user-level is present (if the user never
+  // runs his user-level install after a system-level Chrome is installed and
+  // instead runs the system-level chrome himself from Program Files...). In the
+  // unlikely event that we cannot get the path to chrome exe however, skip
+  // this check as it is already checking for a corner case...
+  FilePath chrome_exe;
+  bool skip_system_install_check = false;
+  if (!PathService::Get(base::FILE_EXE, &chrome_exe)) {
+    LOG(DFATAL) << "Failed to get the path to chrome exe in " << __FUNCTION__;
+    skip_system_install_check = true;
+  }
+
+  string16 tested_suffix;
+  if ((!skip_system_install_check &&
+       !InstallUtil::IsPerUserInstall(chrome_exe.value().c_str())) ||
+      !GetUserSpecificRegistrySuffix(&tested_suffix) ||
+      !RegistryEntry::IsChromeRegisteredWithSuffix(tested_suffix)) {
+    return L"";

[grt (UTC plus 2), 2012/06/04 02:18:26]

L"" -> string16()

[gab, 2012/06/05 19:50:02]

Done.

+  }
+  return tested_suffix;
+}
+
+bool ShellUtil::CurrentUserIsRegisteredWithNoSuffix(
+    BrowserDistribution* dist, const string16& chrome_exe) {
+  // Assert that a Chrome is registered under
+  // HKLM\Software\Clients\StartMenuInternet\Google Chrome
+  const string16 reg_key(
+      RegistryEntry::GetBrowserClientKey(dist, string16())
+      .append(ShellUtil::kRegShellOpen));
+  RegKey key(HKEY_LOCAL_MACHINE, reg_key.c_str(), KEY_READ);
+  string16 registry_chrome_exe;
+  if ((key.ReadValue(L"", &registry_chrome_exe) != ERROR_SUCCESS) ||
+      registry_chrome_exe.length() < 2) {
     return false;
-  entry->reserve(size);
-  entry->assign(1, L'.');
-  entry->append(user_name, size - 1);
+  }
 
-  return RegKey(HKEY_LOCAL_MACHINE,
-                RegistryEntry::GetBrowserClientKey(dist, *entry).c_str(),
-                KEY_READ).Valid();
+  // Assert that the Chrome registered there is |chrome_exe| (i.e. this user's
+  // Chrome).
+  FilePath registry_chrome_exe_path(registry_chrome_exe);
+  InstallUtil::ProgramCompare comparator(registry_chrome_exe_path);

[grt (UTC plus 2), 2012/06/04 02:18:26]

PC's ctor takes chrome_exe and Evaluate takes a registry value containing a
command line and pulls out the path to the exe.  This function can be simplified
to something along the lines of:

bool ShellUtil::CurrentUserIsRegisteredWithNoSuffix(
    BrowserDistribution* dist,
    const string16& chrome_exe) {
  // Assert that HKLM\Software\Clients\StartMenuInternet\Google Chrome
  // points to |chrome_exe|.
  const string16 reg_key(
      RegistryEntry::GetBrowserClientKey(dist, string16())
      .append(ShellUtil::kRegShellOpen));
  RegKey key(HKEY_LOCAL_MACHINE, reg_key.c_str(), KEY_QUERY_VALUE);
  string16 value;
  if (key.ReadValue(L"", &value) == ERROR_SUCCESS)
    return InstallUtil::ProgramCompare(FilePath(chrome_exe)).Evaluate(value);
  return false;
}

[gab, 2012/06/05 19:50:02]

Changed to QuickIsChromeRegistered() and added logic to merge the prior
RegistryEntry::IsChromeRegisteredWithSuffix().

I could also get rid of this and simply use an augmented IsChromeRegistered()
here, but looking at all possible registry entries seems like overkill when a
quick runtime check is needed (or a best effort uninstall check is needed)...
let me know if you feel otherwise.

+  return comparator.Evaluate(chrome_exe);
+}
+
+bool ShellUtil::IsInstallationPresentInHKLM(BrowserDistribution* dist,
+                                            const string16& chrome_exe,
+                                            const string16& suffix) {
+  std::list<RegistryEntry*> entries;
+  STLElementDeleter<std::list<RegistryEntry*> > entries_deleter(&entries);
+  RegistryEntry::GetSystemEntries(dist, chrome_exe, suffix, &entries);
+  return AreAnyEntriesRegistered(entries, RegistryEntry::LOOK_IN_HKLM);

[grt (UTC plus 2), 2012/06/04 02:18:26]

This will return true for an unsuffixed user-level install when a system-level
install is present.  I think it's safer (and simpler) to use ProgramCompare to
see if the open command under the StartMenuInternet key matches |chrome_exe|.

One fairly simple and safe thing would be to resurrect most of what was
CurrentUserHasDefaultBrowser in uninstall.cc with the following modifications:
- have it pass over any keys that aren't equal to the browser distribution's
ApplicationName or don't start with the bd's ApplicationName + '.'.
- when it finds a match via its ProgramCompare instance, have it extract the
suffix from the key name and return it via an out param.
The suffix returned is the one that should then be used to delete the other HKLM
registrations (but only if |remove_all| is set).

[gab, 2012/06/05 19:50:02]

Removed.

 }
 
 bool ShellUtil::MakeChromeDefault(BrowserDistribution* dist,
@@ -945,15 +1025,17 @@ bool ShellUtil::MakeChromeDefault(BrowserDistribution* dist,
   if (!dist->CanSetAsDefault())
     return false;
 
-  ShellUtil::RegisterChromeBrowser(dist, chrome_exe, L"", elevate_if_not_admin);
+  if (!ShellUtil::RegisterChromeBrowser(
+          dist, chrome_exe, L"", elevate_if_not_admin)) {

[grt (UTC plus 2), 2012/06/04 02:18:26]

L"" -> string16()

[gab, 2012/06/05 19:50:02]

Done.

+    return false;
+  }
 
   bool ret = true;
   // First use the new "recommended" way on Vista to make Chrome default
   // browser.
   string16 app_name = dist->GetApplicationName();
-  string16 app_suffix;
-  if (ShellUtil::GetUserSpecificDefaultBrowserSuffix(dist, &app_suffix))
-    app_name += app_suffix;
+  const string16 app_suffix(ShellUtil::GetCurrentInstallationSuffix());
+  app_name += app_suffix;
 
   if (base::win::GetVersion() >= base::win::VERSION_WIN8) {
     // On Windows 8, you can't set yourself as the default handler
@@ -1003,10 +1085,7 @@ bool ShellUtil::MakeChromeDefault(BrowserDistribution* dist,
 
   std::list<RegistryEntry*> entries;
   STLElementDeleter<std::list<RegistryEntry*> > entries_deleter(&entries);
-  string16 suffix;
-  if (!GetUserSpecificDefaultBrowserSuffix(dist, &suffix))
-    suffix = L"";
-  RegistryEntry::GetUserEntries(dist, chrome_exe, suffix, &entries);
+  RegistryEntry::GetUserEntries(dist, chrome_exe, app_suffix, &entries);
   // Change the default browser for current user.
   if ((shell_change & ShellUtil::CURRENT_USER) &&
       !AddRegistryEntries(HKEY_CURRENT_USER, entries)) {
@@ -1046,9 +1125,7 @@ bool ShellUtil::MakeChromeDefaultProtocolClient(BrowserDistribution* dist,
       NULL, CLSCTX_INPROC);
     if (SUCCEEDED(hr)) {
       string16 app_name = dist->GetApplicationName();
-      string16 suffix;
-      if (ShellUtil::GetUserSpecificDefaultBrowserSuffix(dist, &suffix))
-        app_name += suffix;
+      app_name += ShellUtil::GetCurrentInstallationSuffix();
 
       hr = pAAR->SetAppAsDefault(app_name.c_str(), protocol.c_str(),
                                  AT_URLPROTOCOL);
@@ -1066,11 +1143,9 @@ bool ShellUtil::MakeChromeDefaultProtocolClient(BrowserDistribution* dist,
 
   std::list<RegistryEntry*> entries;
   STLElementDeleter<std::list<RegistryEntry*> > entries_deleter(&entries);
-  string16 suffix;
-  if (!GetUserSpecificDefaultBrowserSuffix(dist, &suffix))
-    suffix = L"";
-  string16 chrome_open = ShellUtil::GetChromeShellOpenCmd(chrome_exe);
-  string16 chrome_icon = ShellUtil::GetChromeIcon(dist, chrome_exe);
+  const string16 suffix(GetCurrentInstallationSuffix());
+  const string16 chrome_open(ShellUtil::GetChromeShellOpenCmd(chrome_exe));
+  const string16 chrome_icon(ShellUtil::GetChromeIcon(dist, chrome_exe));
   RegistryEntry::GetUserProtocolEntries(protocol, chrome_icon, chrome_open,
                                         &entries);
   // Change the default protocol handler for current user.
@@ -1089,19 +1164,15 @@ bool ShellUtil::RegisterChromeBrowser(BrowserDistribution* dist,
   if (!dist->CanSetAsDefault())
     return false;
 
-  // First figure out we need to append a suffix to the registry entries to
-  // make them unique.
   string16 suffix;
   if (!unique_suffix.empty()) {
     suffix = unique_suffix;
-  } else if (InstallUtil::IsPerUserInstall(chrome_exe.c_str()) &&
-             !GetUserSpecificDefaultBrowserSuffix(dist, &suffix) &&
-             !AnotherUserHasDefaultBrowser(dist, chrome_exe)) {
-    suffix = L"";
+  } else if (!GetInstallationSpecificSuffix(dist, chrome_exe, &suffix)) {
+    return false;
   }
 
   // TODO(grt): remove this on or after 2012-08-01; see impl for details.
-  RemoveBadWindows8RegistrationIfNeeded(dist, chrome_exe, suffix);
+  RemoveBadWindows8RegistrationIfNeeded(dist, chrome_exe);
 
   // Check if Chromium is already registered with this suffix.
   if (IsChromeRegistered(dist, chrome_exe, suffix))
@@ -1128,11 +1199,19 @@ bool ShellUtil::RegisterChromeBrowser(BrowserDistribution* dist,
       ElevateAndRegisterChrome(dist, chrome_exe, suffix, L""))
     return true;
 
-  // If we got to this point then all we can do is create ProgIds under HKCU
-  // on XP as well as Vista.
+  // If we got to this point then all we can do is create ProgIds under HKCU.
   std::list<RegistryEntry*> entries;
   STLElementDeleter<std::list<RegistryEntry*> > entries_deleter(&entries);
   RegistryEntry::GetProgIdEntries(dist, chrome_exe, L"", &entries);
+  // Prefer to use |suffix|; unless Chrome's ProgIds are already registered with
+  // no suffix (as per the old registration style): in which case some other
+  // registry entries could refer to them and since we were not able to set our
+  // HKLM entries above, we are better off not altering these here.
+  if (!suffix.empty() &&
+      !AreEntriesRegistered(entries, RegistryEntry::LOOK_IN_HKCU)) {

[grt (UTC plus 2), 2012/06/04 02:18:26]

there's no need to AddRegistryEntries if AreEntriesRegistered returns true.  is
this equivalent (with appropriate comments added)?

if (!AreEntriesRegistered(entries, RegistryEntry::LOOK_IN_HKCU)) {
  if (!suffix.empty()) {
    STLDeleteElements(entries);
    RegistryEntry::GetProgIdEntries(dist, chrome_exe, suffix, &entries);
  }
  return AddRegistryEntries(HKEY_CURRENT_USER, entries);
}
return true;
}

[gab, 2012/06/05 19:50:02]

Indeed :), the one nice thing with the previous logic is that !suffix.empty() is
a faster check (if it were false, we're done), but given this will no longer
really ever be true this logic is strictly better.

Done.

+    entries.clear();

[grt (UTC plus 2), 2012/06/04 02:18:26]

entries.clear() -> STLDeleteElements(entries)

[gab, 2012/06/05 19:50:02]

Done.

+    RegistryEntry::GetProgIdEntries(dist, chrome_exe, suffix, &entries);
+  }
   return AddRegistryEntries(HKEY_CURRENT_USER, entries);
 }
 
@@ -1144,15 +1223,11 @@ bool ShellUtil::RegisterChromeForProtocol(BrowserDistribution* dist,
   if (!dist->CanSetAsDefault())
     return false;
 
-  // Figure out we need to append a suffix to the registry entries to
-  // make them unique.
   string16 suffix;
   if (!unique_suffix.empty()) {
     suffix = unique_suffix;
-  } else if (InstallUtil::IsPerUserInstall(chrome_exe.c_str()) &&
-             !GetUserSpecificDefaultBrowserSuffix(dist, &suffix) &&
-             !AnotherUserHasDefaultBrowser(dist, chrome_exe)) {
-    suffix = L"";
+  } else if (!GetInstallationSpecificSuffix(dist, chrome_exe, &suffix)) {
+    return false;
   }
 
   // Check if Chromium is already registered with this suffix.