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..bc3d86d46e19520edbb9ea6e8f0db1a4a48b46f4 100644
--- a/chrome/installer/util/shell_util.cc
+++ b/chrome/installer/util/shell_util.cc
@@ -74,6 +74,13 @@ bool IsChromeMetroSupported() {
 // class.
 class RegistryEntry {
  public:
+  // An enum of places to look for this key in the Windows registry.
+  enum LookForIn {
+    LOOK_IN_HKCU = 0,
+    LOOK_IN_HKLM,
+    LOOK_IN_HKCU_THEN_HKLM,

[grt (UTC plus 2), 2012/06/08 16:24:13]

this makes the logic worse in the func.  i meant leaving the first two as they
were and adding:
LOOK_IN_HKCU_THEN_HKLM = LOOK_IN_HKCU | LOOK_IN_HKLM,

[gab, 2012/06/08 20:15:51]

Ah ok, that's what I did originally, but felt it was slightly less
consumer-friendly as it's a bitfield in which some flags have more than 1 bit
and wasn't sure that was ok with Chromium-style.

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"),
@@ -250,11 +257,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 +271,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->GetAppShortCutName()));

[grt (UTC plus 2), 2012/06/08 16:24:13]

this seemingly trivial change will make IsChromeRegistered return false for all
existing installs, will it not?  does the test plan in your design doc include
how to test for such breaking changes?

[gab, 2012/06/08 20:15:51]

IsChromeRegistered() returning false simply means registering again (for
non-HKLM entries).

My test matrix includes installing a new-style install over an old-style install
so I think it's covered.

[grt (UTC plus 2), 2012/06/11 17:28:57]

Isn't it worse than that?  What happens when a user tries to make Chrome default
for a system-level install?  Chrome is registered in HKLM by the installer, so a
user should be able to make Chrome their default without UAC.  I think a change
that causes IsChromeRegistered() to return false will break this.
To catch this, the test plan would have to include:
1) Install system-level old-style Chrome
2) Update to new-style Chrome
3) Visit chrome://chrome/settings/ and ensure that Chrome doesn't think it's the
default browser
4) Click to make Chrome the default browser and ensure that Chrome is made the
default without UAC

[grt (UTC plus 2), 2012/06/11 17:49:07]

Ah, but the subtlety continues: RegisterChromeBrowser() is called on each
update, so the new value will be written for an existing install.  Is this
correct?  If so, I think this is safe.

[gab, 2012/06/11 19:29:39]

Ack.

[gab, 2012/06/11 19:29:39]

As discussed this will be fine, the worst case is a UAC for user-level installs
when the user makes Chrome default a second time (i.e. having done it the first
time with the other registration). We decided it is OK to have this one-time
UAC. We agreed that IsChromeRegistered() is essentially wrong, but fixing this
is beyond the scope of this patched.

 
     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 +385,30 @@ 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 into (i.e. the the key is assumed not
+  // to exist in them.
+  // If |look_for_in| is LOOK_IN_HKCU_THEN_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

[grt (UTC plus 2), 2012/06/08 16:24:13]

"up to Win7" -> "prior to Win8" to reduce the ambiguity for Server 2008 R2,
which could be considered past Win7 but is clearly before Win8.

[gab, 2012/06/08 20:15:51]

Done.

+  // values go in HKLM. This function will make unnecessary (but harmless)
+  // queries into HKCU in that case.
+  bool ExistsInRegistry(LookForIn look_for_in) const {
+    const bool look_in_hkcu = (look_for_in == LOOK_IN_HKCU ||
+                               look_for_in == LOOK_IN_HKCU_THEN_HKLM);
+    const bool look_in_hklm = (look_for_in == LOOK_IN_HKLM ||
+                               look_for_in == LOOK_IN_HKCU_THEN_HKLM);
+
+    RegistryStatus hkcu_status = DOES_NOT_EXIST;

[grt (UTC plus 2), 2012/06/08 16:24:13]

i think this shorter form is easier to read:

RegistryStatus status = DOES_NOT_EXIST;
if (look_in_hkcu)
  status = ...;
if (status == DOES_NOT_EXIST && look_in_hklm)
  status = ...;
return status == SAME_VALUE;

i think it's also clean to do away with the look_in bools as well if you make
look_for_in a bitfield again.

[gab, 2012/06/08 20:15:51]

Done.

+    if (look_in_hkcu)
+      hkcu_status = StatusInRegistryUnderRoot(HKEY_CURRENT_USER);
+
+    RegistryStatus hklm_status = DOES_NOT_EXIST;
+    if (hkcu_status == DOES_NOT_EXIST && 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:
@@ -471,13 +492,16 @@ bool AddRegistryEntries(HKEY root, const std::list<RegistryEntry*>& entries) {
 }
 
 // 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,
+                          RegistryEntry::LookForIn 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 +515,7 @@ 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_THEN_HKLM);
 }
 
 // This method checks if Chrome is already registered on the local machine
@@ -502,7 +526,7 @@ 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_THEN_HKLM);
 }
 
 // This method registers Chrome on Vista by launching an elevated setup.exe.
@@ -555,62 +579,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 +615,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 +625,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(
+        ShellUtil::GetCurrentInstallationSuffix(dist, chrome_exe));
     const string16 app_id(dist->GetBrowserAppId());
 
     // <root hkey>\Software\Classes\<app_id>
@@ -670,13 +641,54 @@ 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) << "GetUserName failed";
+    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::QuickIsChromeRegistered(
+          dist, chrome_exe, string16(),
+          ShellUtil::CONFIRM_SYSTEM_REGISTRATION)) {
+    // No suffix on system-level installs and user-level installs already
+    // registered with no suffix.
+    suffix->clear();
+    return true;
+  } else {
+    return GetUserSpecificRegistrySuffix(suffix);
+  }
+}
+
 }  // namespace
 
 const wchar_t* ShellUtil::kRegDefaultIcon = L"\\DefaultIcon";
@@ -923,19 +935,66 @@ 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)
-    return false;
-  entry->reserve(size);
-  entry->assign(1, L'.');
-  entry->append(user_name, size - 1);
+string16 ShellUtil::GetCurrentInstallationSuffix(BrowserDistribution* dist,
+                                                 const string16& chrome_exe) {
+  string16 tested_suffix;
+  if (!InstallUtil::IsPerUserInstall(chrome_exe.c_str()) ||
+      !GetUserSpecificRegistrySuffix(&tested_suffix) ||
+      !QuickIsChromeRegistered(dist, chrome_exe, tested_suffix,
+                               CONFIRM_PROGID_REGISTRATION)) {
+    return string16();
+  }
+  return tested_suffix;
+}
+
+bool ShellUtil::QuickIsChromeRegistered(
+    BrowserDistribution* dist,
+    const string16& chrome_exe,
+    const string16& suffix,
+    RegistrationConfirmationLevel confirmation_level) {
+  // Get the appropriate key to look for based on the level desired.
+  string16 reg_key;
+  switch (confirmation_level) {
+    case CONFIRM_PROGID_REGISTRATION:
+      // Software\Classes\ChromeHTML|suffix|
+      reg_key = ShellUtil::kRegClasses;
+      reg_key.push_back(FilePath::kSeparators[0]);
+      reg_key.append(ShellUtil::kChromeHTMLProgId);
+      reg_key.append(suffix);
+      break;
+    case CONFIRM_SYSTEM_REGISTRATION:
+      // Software\Clients\StartMenuInternet\Google Chrome|suffix|
+      reg_key = RegistryEntry::GetBrowserClientKey(dist, suffix);
+      break;
+    default:
+      NOTREACHED();
+      break;
+  }
+  reg_key.append(ShellUtil::kRegShellOpen);
+
+  // User-level installs ProgId registrations can be found in HKCU (and values

[grt (UTC plus 2), 2012/06/08 16:24:13]

Note: this is only true for Chrome 20 onward.  Any user-level install that was
done prior to then has ProgId registrations in HKLM.  Please make sure the
design doc includes the history of what used to be where, as such configurations
will need to be maintained (e.g., upgrades from pre-20 to post-20).

[gab, 2012/06/08 20:15:51]

Right, which is why I said "can be found" and not "are found". The old-style
install section of the design doc also mentions this part for ProgIds.

[grt (UTC plus 2), 2012/06/11 17:28:57]

Ah, I see (note that "can be" is often used as a polite "are").  Would you mind
making the comment more explicit?  I know that three months from now I won't
remember the subtlety here.

[gab, 2012/06/11 19:29:39]

Done.

+  // there have priority over HKLM). The same is true for system entries as

[grt (UTC plus 2), 2012/06/08 16:24:13]

As of this CL, the system entries are always in HKLM.  Please move this portion
of the comment to the CL that changes that so that matters aren't confused here.
 In that other CL, I urge you to rename the "system entries" to something else
to avoid confusion, as "system" implies system-wide, which means HKLM.

[gab, 2012/06/08 20:15:51]

I added it in this CL because it is still true that the values are *allowed* to
be there and if they were would have priority over HKLM. This does not entail
that they *are* there and simply falls through to the HKLM lookup if they
aren't.

I will make sure to rename SystemEntries in the HKCU CL.

+  // of Windows 8.

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

This has yet to be implemented for Chrome, but it is still true that these
values are valid in Win8 in HKCU, so it is correct to look there first anyways.

+  if (confirmation_level == CONFIRM_PROGID_REGISTRATION ||
+      base::win::GetVersion() >= base::win::VERSION_WIN8) {
+    const RegKey key_hkcu(HKEY_CURRENT_USER, reg_key.c_str(), KEY_QUERY_VALUE);
+    string16 hkcu_value;
+    // If |reg_key| is present in HKCU, assert that it points to |chrome_exe|.
+    // Otherwise, fall back on an HKLM lookup below.
+    if (key_hkcu.ReadValue(L"", &hkcu_value) == ERROR_SUCCESS) {
+      return InstallUtil::ProgramCompare(
+          FilePath(chrome_exe)).Evaluate(hkcu_value);
+    }
+  }
 
-  return RegKey(HKEY_LOCAL_MACHINE,
-                RegistryEntry::GetBrowserClientKey(dist, *entry).c_str(),
-                KEY_READ).Valid();
+  // Assert that |reg_key| points to |chrome_exe| in HKLM.
+  const RegKey key_hklm(HKEY_LOCAL_MACHINE, reg_key.c_str(), KEY_QUERY_VALUE);
+  string16 hklm_value;
+  if (key_hklm.ReadValue(L"", &hklm_value) == ERROR_SUCCESS) {
+    return InstallUtil::ProgramCompare(
+        FilePath(chrome_exe)).Evaluate(hklm_value);
+  }
+  return false;
 }
 
 bool ShellUtil::MakeChromeDefault(BrowserDistribution* dist,
@@ -945,15 +1004,18 @@ 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, string16(), elevate_if_not_admin)) {
+    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(dist, chrome_exe));
+  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 +1065,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 +1105,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(dist, chrome_exe);
 
       hr = pAAR->SetAppAsDefault(app_name.c_str(), protocol.c_str(),
                                  AT_URLPROTOCOL);
@@ -1066,11 +1123,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(dist, chrome_exe));
+  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 +1144,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,12 +1179,22 @@ 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);
-  return AddRegistryEntries(HKEY_CURRENT_USER, 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 (!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;
 }
 
 bool ShellUtil::RegisterChromeForProtocol(BrowserDistribution* dist,
@@ -1144,15 +1205,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.