Clarify function names for registry magic in shell_util.cc

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 95c6b5a8159c46eb88fa23feab68c086cc682e95..cb2bd277c90ad19a9c773650bbbdf9ca59a5f46e 100644
--- a/chrome/installer/util/shell_util.cc
+++ b/chrome/installer/util/shell_util.cc
@@ -425,11 +425,12 @@ class RegistryEntry {
   }
 
   // This method returns a list of all the user level registry entries that
-  // are needed to make Chromium the default handler for a protocol.
-  static void GetUserProtocolEntries(const string16& protocol,
-                                     const string16& chrome_icon,
-                                     const string16& chrome_open,
-                                     ScopedVector<RegistryEntry>* entries) {
+  // are needed to make Chromium the default handler for a protocol on XP.
+  static void GetUserProtocolEntriesForXP(

[grt (UTC plus 2), 2012/10/06 01:16:24]

in general, i really like this change.  the name gives me pause, though, since
at first glance it's easy to interpret "...ForXP" as meaning that this should
only be used on XP (which isn't the case).  what do you think of
GetXPStyleUserProtocolEntries or GetXPEraUserProtocolEntries or something like
that?

[gab, 2012/10/09 14:14:52]

Right, I was going to go with "XPStyle" instead of "ForXP" when I first
introduced this method. Robert didn't like "XPStyle" (I don't recall why), going
back to it now as I agree with you "ForXP" implies this should only be done on
XP. As for "XPEra" it sounds good in the getter ("GetXPEra..."), but not in the
methods ("RegisterChromeAsDefaultXPEra"...), so I'd rather keep it consistent
with "XPStyle".

+      const string16& protocol,
+      const string16& chrome_icon,
+      const string16& chrome_open,
+      ScopedVector<RegistryEntry>* entries) {
     // Protocols associations.
     string16 url_key(ShellUtil::kRegClasses);
     url_key.push_back(FilePath::kSeparators[0]);
@@ -459,11 +460,11 @@ class RegistryEntry {
   }
 
   // This method returns a list of all the user level registry entries that
-  // are needed to make Chromium default browser.
+  // are needed to make Chromium default browser on XP.
   // Some of these entries are irrelevant in recent versions of Windows, but
   // we register them anyways as some legacy apps are hardcoded to lookup those
   // values.
-  static void GetDefaultBrowserUserEntries(
+  static void GetDefaultBrowserUserEntriesForXP(
       BrowserDistribution* dist,
       const string16& chrome_exe,
       const string16& suffix,
@@ -481,8 +482,8 @@ class RegistryEntry {
     string16 chrome_open = ShellUtil::GetChromeShellOpenCmd(chrome_exe);
     string16 chrome_icon = ShellUtil::GetChromeIcon(dist, chrome_exe);
     for (int i = 0; ShellUtil::kBrowserProtocolAssociations[i] != NULL; i++) {
-      GetUserProtocolEntries(ShellUtil::kBrowserProtocolAssociations[i],
-                             chrome_icon, chrome_open, entries);
+      GetUserProtocolEntriesForXP(ShellUtil::kBrowserProtocolAssociations[i],
+                                  chrome_icon, chrome_open, entries);
     }
 
     // start->Internet shortcut.
@@ -881,10 +882,10 @@ bool GetInstallationSpecificSuffix(BrowserDistribution* dist,
   return ShellUtil::GetUserSpecificRegistrySuffix(suffix);
 }
 
-// Returns the root registry key (HKLM or HKCU) into which shell integration
-// registration for default protocols must be placed. As of Windows 8 everything
-// can go in HKCU for per-user installs.
-HKEY DetermineShellIntegrationRoot(bool is_per_user) {
+// Returns the root registry key (HKLM or HKCU) under which registrations must
+// be placed for this install. As of Windows 8 everything can go in HKCU for
+// per-user installs.
+HKEY DetermineRegistrationRoot(bool is_per_user) {
   return is_per_user && base::win::GetVersion() >= base::win::VERSION_WIN8 ?
       HKEY_CURRENT_USER : HKEY_LOCAL_MACHINE;
 }
@@ -897,7 +898,7 @@ bool RegisterChromeAsDefaultForXP(BrowserDistribution* dist,
                                   const string16& chrome_exe) {
   bool ret = true;
   ScopedVector<RegistryEntry> entries;
-  RegistryEntry::GetDefaultBrowserUserEntries(
+  RegistryEntry::GetDefaultBrowserUserEntriesForXP(
       dist, chrome_exe,
       ShellUtil::GetCurrentInstallationSuffix(dist, chrome_exe), &entries);
 
@@ -928,8 +929,8 @@ bool RegisterChromeAsDefaultProtocolClientForXP(BrowserDistribution* dist,
   ScopedVector<RegistryEntry> entries;
   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);
+  RegistryEntry::GetUserProtocolEntriesForXP(protocol, chrome_icon, chrome_open,
+                                             &entries);
   // Change the default protocol handler for current user.
   if (!AddRegistryEntries(HKEY_CURRENT_USER, entries)) {
     LOG(ERROR) << "Could not make Chrome default protocol client (XP).";
@@ -1461,8 +1462,8 @@ bool ShellUtil::MakeChromeDefaultProtocolClient(BrowserDistribution* dist,
   }
 
   // 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.
+  // should not be required on Vista+, but since some applications still read
+  // Software\Classes\<protocol> key directly, do this on Vista+ also.
   if (!RegisterChromeAsDefaultProtocolClientForXP(dist, chrome_exe, protocol))
     ret = false;
 
@@ -1517,7 +1518,7 @@ bool ShellUtil::RegisterChromeBrowser(BrowserDistribution* dist,
     return true;
 
   bool user_level = InstallUtil::IsPerUserInstall(chrome_exe.c_str());
-  HKEY root = DetermineShellIntegrationRoot(user_level);
+  HKEY root = DetermineRegistrationRoot(user_level);
 
   // Do the full registration if we can do it at user-level or if the user is an
   // admin.
@@ -1589,7 +1590,7 @@ bool ShellUtil::RegisterChromeForProtocol(BrowserDistribution* dist,
   if (IsChromeRegisteredForProtocol(dist, suffix, protocol))
     return true;
 
-  HKEY root = DetermineShellIntegrationRoot(
+  HKEY root = DetermineRegistrationRoot(
       InstallUtil::IsPerUserInstall(chrome_exe.c_str()));
 
   if (root == HKEY_CURRENT_USER || IsUserAnAdmin()) {