Clarify function names for registry magic in shell_util.cc

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 407 matching lines @@
       string16 key(ShellUtil::kRegClasses);
       key.push_back(FilePath::kSeparators[0]);
       key.append(ShellUtil::kFileAssociations[i]);
       key.push_back(FilePath::kSeparators[0]);
       key.append(ShellUtil::kRegOpenWithProgids);
       entries->push_back(new RegistryEntry(key, html_prog_id, string16()));
     }
   }
 
   // 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]);
     url_key.append(protocol);
 
     // This registry value tells Windows that this 'class' is a URL scheme
     // so IE, explorer and other apps will route it to our handler.
     // <root hkey>\Software\Classes\<protocol>\URL Protocol
     entries->push_back(new RegistryEntry(url_key,
         ShellUtil::kRegUrlProtocol, L""));
 
     // <root hkey>\Software\Classes\<protocol>\DefaultIcon
     string16 icon_key = url_key + ShellUtil::kRegDefaultIcon;
     entries->push_back(new RegistryEntry(icon_key, chrome_icon));
 
     // <root hkey>\Software\Classes\<protocol>\shell\open\command
     string16 shell_key = url_key + ShellUtil::kRegShellOpen;
     entries->push_back(new RegistryEntry(shell_key, chrome_open));
 
     // <root hkey>\Software\Classes\<protocol>\shell\open\ddeexec
     string16 dde_key = url_key + L"\\shell\\open\\ddeexec";
     entries->push_back(new RegistryEntry(dde_key, L""));
 
     // <root hkey>\Software\Classes\<protocol>\shell\@
     string16 protocol_shell_key = url_key + ShellUtil::kRegShellPath;
     entries->push_back(new RegistryEntry(protocol_shell_key, L"open"));
   }
 
   // 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,
       ScopedVector<RegistryEntry>* entries) {
     // File extension associations.
     string16 html_prog_id(GetBrowserProgId(suffix));
     for (int i = 0; ShellUtil::kFileAssociations[i] != NULL; i++) {
       string16 ext_key(ShellUtil::kRegClasses);
       ext_key.push_back(FilePath::kSeparators[0]);
       ext_key.append(ShellUtil::kFileAssociations[i]);
       entries->push_back(new RegistryEntry(ext_key, html_prog_id));
     }
 
     // Protocols associations.
     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.
     string16 start_menu(ShellUtil::kRegStartMenuInternet);
     string16 app_name = dist->GetBaseAppName() + suffix;
     entries->push_back(new RegistryEntry(start_menu, app_name));
   }
 
   // Generate work_item tasks required to create current registry entry and
   // add them to the given work item list.
@@ skipping 378 matching lines @@
   }
   if (QuickIsChromeRegistered(dist, chrome_exe, *suffix,
                               CONFIRM_SHELL_REGISTRATION)) {
     // Username suffix for installs that are suffixed as per the old-style.
     return true;
   }
 
   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;
 }
 
 // Associates 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+ as well.
 bool RegisterChromeAsDefaultForXP(BrowserDistribution* dist,
                                   int shell_change,
                                   const string16& chrome_exe) {
   bool ret = true;
   ScopedVector<RegistryEntry> entries;
-  RegistryEntry::GetDefaultBrowserUserEntries(
+  RegistryEntry::GetDefaultBrowserUserEntriesForXP(
       dist, chrome_exe,
       ShellUtil::GetCurrentInstallationSuffix(dist, chrome_exe), &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).";
   }
 
@@ skipping 10 matching lines @@
 // Associates Chrome with |protocol| in the registry. This should not be
 // required on Vista+ but since some applications still read these registry
 // keys directly, we have to do this on Vista+ as well.
 // See http://msdn.microsoft.com/library/aa767914.aspx for more details.
 bool RegisterChromeAsDefaultProtocolClientForXP(BrowserDistribution* dist,
                                                 const string16& chrome_exe,
                                                 const string16& protocol) {
   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).";
     return false;
   }
 
   return true;
 }
 
 // Returns |properties.shortcut_name| if the property is set, otherwise it
@@ skipping 511 matching lines @@
                                  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.
+  // 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;
 
   return ret;
 }
 
 bool ShellUtil::ShowMakeChromeDefaultProtocolClientSystemUI(
     BrowserDistribution* dist,
     const string16& chrome_exe,
     const string16& protocol) {
@@ skipping 34 matching lines @@
   }
 
   // TODO(grt): remove this on or after 2012-08-01; see impl for details.
   RemoveBadWindows8RegistrationIfNeeded(dist, chrome_exe);
 
   // Check if Chromium is already registered with this suffix.
   if (IsChromeRegistered(dist, chrome_exe, suffix))
     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.
   if (root == HKEY_CURRENT_USER || IsUserAnAdmin()) {
     ScopedVector<RegistryEntry> progid_and_appreg_entries;
     ScopedVector<RegistryEntry> shell_entries;
     RegistryEntry::GetProgIdEntries(dist, chrome_exe, suffix,
                                     &progid_and_appreg_entries);
     RegistryEntry::GetAppRegistrationEntries(chrome_exe, suffix,
                                              &progid_and_appreg_entries);
@@ skipping 51 matching lines @@
   if (!unique_suffix.empty()) {
     suffix = unique_suffix;
   } 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;
 
-  HKEY root = DetermineShellIntegrationRoot(
+  HKEY root = DetermineRegistrationRoot(
       InstallUtil::IsPerUserInstall(chrome_exe.c_str()));
 
   if (root == HKEY_CURRENT_USER || IsUserAnAdmin()) {
     // We can do this operation directly.
     // First, make sure Chrome is fully registered on this machine.
     if (!RegisterChromeBrowser(dist, chrome_exe, suffix, false))
       return false;
 
     // Write in the capabillity for the protocol.
     ScopedVector<RegistryEntry> entries;
@@ skipping 128 matching lines @@
     // are any left...).
     if (free_bits >= 8 && next_byte_index < size) {
       free_bits -= 8;
       bit_stream += bytes[next_byte_index++] << free_bits;
     }
   }
 
   DCHECK_EQ(ret.length(), encoded_length);
   return ret;
 }