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

chrome/installer/util/shell_util.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 functions that integrate Chrome in Windows shell. These
 // functions can be used by Chrome as well as Chrome installer. All of the
 // work is done by the local functions defined in anonymous namespace in
 // this class.
 
 #include "chrome/installer/util/shell_util.h"
@@ skipping 56 matching lines @@
   return VerifyVersionInfo(&min_version_info, type_mask, condition_mask) != 0;
 }
 
 // This class represents a single registry entry. The objective is to
 // encapsulate all the registry entries required for registering Chrome at one
 // place. This class can not be instantiated outside the class and the objects
 // of this class type can be obtained only by calling a static method of this
 // 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"),
   // but that ship has sailed.  The cost of switching now is re-prompting users
   // to make Chrome their default browser, which isn't polite.  |suffix| is the
   // user-specific registration suffix; see GetUserSpecificDefaultBrowserSuffix
   // in shell_util.h for details.
   static string16 GetBrowserClientKey(BrowserDistribution* dist,
                                       const string16& suffix) {
     DCHECK(suffix.empty() || suffix[0] == L'.');
@@ skipping 156 matching lines @@
     // strings.
     entries->push_front(new RegistryEntry(install_info, kReinstallCommand,
         quoted_exe_path + L" --" + ASCIIToWide(switches::kMakeDefaultBrowser)));
     entries->push_front(new RegistryEntry(install_info, L"HideIconsCommand",
         quoted_exe_path + L" --" + ASCIIToWide(switches::kHideIcons)));
     entries->push_front(new RegistryEntry(install_info, L"ShowIconsCommand",
         quoted_exe_path + L" --" + ASCIIToWide(switches::kShowIcons)));
     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.
     entries->push_front(new RegistryEntry(
         capabilities, ShellUtil::kRegApplicationDescription,
         dist->GetLongAppDescription()));
     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);
     for (int i = 0; ShellUtil::kFileAssociations[i] != NULL; i++) {
       entries->push_front(new RegistryEntry(
           capabilities + L"\\FileAssociations",
           ShellUtil::kFileAssociations[i], html_prog_id));
     }
     for (int i = 0; ShellUtil::kPotentialProtocolAssociations[i] != NULL;
         i++) {
@@ skipping 89 matching lines @@
     if (_is_string) {
       items->AddSetRegValueWorkItem(root, _key_path, _name, _value, true);
     } else {
       items->AddSetRegValueWorkItem(root, _key_path, _name, _int_value, true);
     }
   }
 
   // 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:
   // States this RegistryKey can be in compared to the registry.
   enum RegistryStatus {
     // |_name| does not exist in the registry
     DOES_NOT_EXIST,
     // |_name| exists, but its value != |_value|
     DIFFERENT_VALUE,
     // |_name| exists and its value is |_value|
@@ skipping 63 matching lines @@
 
   // Apply all the registry changes and if there is a problem, rollback
   if (!items->Do()) {
     items->Rollback();
     return false;
   }
   return true;
 }
 
 // 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;
 }
 
 // Checks that all required registry entries for Chrome are already present
 // on this computer.
 bool IsChromeRegistered(BrowserDistribution* dist,
                         const string16& chrome_exe,
                         const string16& suffix) {
   std::list<RegistryEntry*> entries;
   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
 // for the requested protocol. It just checks the one value required for this.
 bool IsChromeRegisteredForProtocol(BrowserDistribution* dist,
                                    const string16& suffix,
                                    const string16& protocol) {
   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.
 // That will show the user the standard Vista elevation prompt. If the user
 // accepts it the new process will make the necessary changes and return SUCCESS
 // that we capture and return.
 // If protocol is non-empty we will also register Chrome as being capable of
 // handling the protocol.
 bool ElevateAndRegisterChrome(BrowserDistribution* dist,
                               const string16& chrome_exe,
@@ skipping 32 matching lines @@
     }
 
     DWORD ret_val = 0;
     InstallUtil::ExecuteExeAsAdmin(cmd, &ret_val);
     if (ret_val == 0)
       return true;
   }
   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.
 bool LaunchApplicationAssociationDialog(const string16& app_id) {
   base::win::ScopedComPtr<IApplicationAssociationRegistrationUI> aarui;
   HRESULT hr = aarui.CreateInstance(CLSID_ApplicationAssociationRegistrationUI);
   if (FAILED(hr))
     return false;
   hr = aarui->LaunchAdvancedAssociationUI(app_id.c_str());
   return SUCCEEDED(hr);
@@ skipping 16 matching lines @@
 // should be removed after a reasonable time, say 2012-08-01.  Anyone on Win8
 // dev channel who hasn't been autoupdated or manually updated by then will have
 // to uninstall and reinstall Chrome to repair.  See http://crbug.com/124666 and
 // http://crbug.com/123994 for gory details.
 // This function is also used to remove DelegateExecute verb handler
 // registrations on builds for which Metro is no longer supported. This will
 // also become irrelevant sometime after Windows 8 RC (thus the aforementioned
 // 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) &&
       (!InstallUtil::HasDelegateExecuteHandler(dist, chrome_exe) ||
        !IsChromeMetroSupported())) {
     // There's no need to rollback, so forgo the usual work item lists and just
     // 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>
     string16 key(ShellUtil::kRegClasses);
     key.push_back(FilePath::kSeparators[0]);
     key.append(app_id);
     InstallUtil::DeleteRegistryKey(root_key, key);
 
     // <root hkey>\Software\Classes\ChromiumHTML[.user]\shell\open\command
     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";
 const wchar_t* ShellUtil::kRegShellPath = L"\\shell";
 const wchar_t* ShellUtil::kRegShellOpen = L"\\shell\\open\\command";
 const wchar_t* ShellUtil::kRegStartMenuInternet =
     L"Software\\Clients\\StartMenuInternet";
 const wchar_t* ShellUtil::kRegClasses = L"Software\\Classes";
 const wchar_t* ShellUtil::kRegRegisteredApplications =
     L"Software\\RegisteredApplications";
@@ skipping 226 matching lines @@
                  KEY_QUERY_VALUE) != ERROR_SUCCESS ||
         key.ReadValue(kReinstallCommand, &command) != ERROR_SUCCESS) {
       continue;
     }
     if (!name.empty() && !command.empty() &&
         name.find(dist->GetApplicationName()) == string16::npos)
       (*browsers)[name] = command;
   }
 }
 
-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;
+}
 
-  return RegKey(HKEY_LOCAL_MACHINE,
-                RegistryEntry::GetBrowserClientKey(dist, *entry).c_str(),
-                KEY_READ).Valid();
+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);
+    }
+  }
+
+  // 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,
                                   int shell_change,
                                   const string16& chrome_exe,
                                   bool elevate_if_not_admin) {
   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
     // programatically. In other words IApplicationAssociationRegistration
     // has been rendered useless. What you can do is to launch
     // "Set Program Associations" section of the "Default Programs"
     // control panel. This action does not require elevation and we
     // don't get to control window activation. More info at:
     // http://msdn.microsoft.com/en-us/library/cc144154(VS.85).aspx
     return LaunchApplicationAssociationDialog(app_name.c_str());
@@ skipping 29 matching lines @@
     }
   }
 
   // Now use the old way to associate Chrome with supported protocols and file
   // associations. This should not be required on Vista but since some
   // applications still read Software\Classes\http key directly, we have to do
   // this on Vista also.
 
   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)) {
     ret = false;
     LOG(ERROR) << "Could not make Chrome default browser (XP/current user).";
   }
 
   // Chrome as default browser at system level.
   if ((shell_change & ShellUtil::SYSTEM_LEVEL) &&
       !AddRegistryEntries(HKEY_LOCAL_MACHINE, entries)) {
@@ skipping 19 matching lines @@
   // First use the new "recommended" way on Vista to make Chrome default
   // protocol handler.
   if (base::win::GetVersion() >= base::win::VERSION_VISTA) {
     VLOG(1) << "Registering Chrome as default handler for " << protocol
             << " on Vista.";
     base::win::ScopedComPtr<IApplicationAssociationRegistration> pAAR;
     HRESULT hr = pAAR.CreateInstance(CLSID_ApplicationAssociationRegistration,
       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);
     }
     if (!SUCCEEDED(hr)) {
       ret = false;
       LOG(ERROR) << "Could not make Chrome default protocol client (Vista):"
                  << " HRESULT=" << hr << ".";
     }
   }
 
   // Now use the old way to associate Chrome with the desired protocol. This
   // should not be required on Vista but since some applications still read
   // Software\Classes\http key directly, we have to do this on Vista also.
 
   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.
   if (!AddRegistryEntries(HKEY_CURRENT_USER, entries)) {
       ret = false;
       LOG(ERROR) << "Could not make Chrome default protocol client (XP).";
   }
 
   return ret;
 }
 
 bool ShellUtil::RegisterChromeBrowser(BrowserDistribution* dist,
                                       const string16& chrome_exe,
                                       const string16& unique_suffix,
                                       bool elevate_if_not_admin) {
   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))
     return true;
 
   // If user is an admin try to register and return the status.
   if (IsUserAnAdmin()) {
     std::list<RegistryEntry*> progids;
     STLElementDeleter<std::list<RegistryEntry*> > progids_deleter(&progids);
     std::list<RegistryEntry*> sys_entries;
     STLElementDeleter<std::list<RegistryEntry*> > sys_deleter(&sys_entries);
     RegistryEntry::GetProgIdEntries(dist, chrome_exe, suffix, &progids);
     RegistryEntry::GetSystemEntries(dist, chrome_exe, suffix, &sys_entries);
     return AddRegistryEntries(
                InstallUtil::IsPerUserInstall(chrome_exe.c_str()) ?
                    HKEY_CURRENT_USER : HKEY_LOCAL_MACHINE,
                progids) &&
            AddRegistryEntries(HKEY_LOCAL_MACHINE, sys_entries);
   }
 
   // If user is not an admin and OS is Vista, try to elevate and register.
   if (elevate_if_not_admin &&
       base::win::GetVersion() >= base::win::VERSION_VISTA &&
       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,
                                           const string16& chrome_exe,
                                           const string16& unique_suffix,
                                           const string16& protocol,
                                           bool elevate_if_not_admin) {
   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.
   if (IsChromeRegisteredForProtocol(dist, suffix, protocol))
     return true;
 
   if (IsUserAnAdmin()) {
     // We can do this operation directly.
     // If we're not registered at all, try to register. If that fails
     // we should give up.
@@ skipping 125 matching lines @@
       chrome_exe.c_str(),
       shortcut.c_str(),
       chrome_path.c_str(),
       arguments.c_str(),
       description.c_str(),
       icon_path.c_str(),
       icon_index,
       dist->GetBrowserAppId().c_str(),
       ConvertShellUtilShortcutOptionsToFileUtil(options));
 }