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:
+  // 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"),
   // 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'.');
     return string16(ShellUtil::kRegStartMenuInternet)
         .append(1, L'\\')
         .append(dist->GetApplicationName())
         .append(suffix);
   }
 
   // Returns the Windows Default Programs capabilities key for Chrome.  For
   // example:
   // "Software\Clients\StartMenuInternet\Chromium[.user]\Capabilities".
   static string16 GetCapabilitiesKey(BrowserDistribution* dist,
                                      const string16& suffix) {
     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
   // HKLM for system-level installs.
   // TODO (gab): Extract the Windows 8 only registrations out of this function.
   static bool GetProgIdEntries(BrowserDistribution* dist,
                                const string16& chrome_exe,
                                const string16& suffix,
                                std::list<RegistryEntry*>* entries) {
     string16 icon_path(ShellUtil::GetChromeIcon(dist, chrome_exe));
@@ skipping 132 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->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);
     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 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:
   // 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 62 matching lines @@
     (*itr)->AddToWorkItemList(root, items.get());
 
   // Apply all the registry changes and if there is a problem, rollback
   if (!items->Do()) {
     items->Rollback();
     return false;
   }
   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;
 }
 
 // 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 | RegistryEntry::LOOK_IN_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 | RegistryEntry::LOOK_IN_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 =

[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>
     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) << "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";
 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)
+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,
                                   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, 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
     // 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();
 
       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());
+  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);
+  // 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);
 }
 
 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));
 }