Change DeleteRegistryKey to not require opening root keys with RegKey.

BUG=none

Review URL: http://codereview.chromium.org/6679007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@77979 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 42 insertions, 51 deletions

diff --git a/base/file_util_win.cc b/base/file_util_win.cc
index af989c8..fbca30b 100644
--- a/base/file_util_win.cc
+++ b/base/file_util_win.cc
@@ -55,7 +55,7 @@ bool DevicePathToDriveLetterPath(const FilePath& device_path,
   // For each string in the drive mapping, get the junction that links
   // to it.  If that junction is a prefix of |device_path|, then we
   // know that |drive| is the real path prefix.
-  while(*drive_map_ptr) {
+  while (*drive_map_ptr) {
     drive[0] = drive_map_ptr[0];  // Copy the drive letter.
 
     if (QueryDosDevice(drive, device_name, MAX_PATH) &&
@@ -66,7 +66,7 @@ bool DevicePathToDriveLetterPath(const FilePath& device_path,
     }
     // Move to the next drive letter string, which starts one
     // increment after the '\0' that terminates the current string.
-    while(*drive_map_ptr++);
+    while (*drive_map_ptr++);
   }
 
   // No drive matched.  The path does not start with a device junction
@@ -104,7 +104,7 @@ int CountFilesCreatedAfter(const FilePath& path,
           (wcscmp(find_file_data.cFileName, L".") == 0))
         continue;
 
-      long result = CompareFileTime(&find_file_data.ftCreationTime,
+      long result = CompareFileTime(&find_file_data.ftCreationTime,  // NOLINT
                                     &comparison_filetime);
       // File was created after or on comparison time
       if ((result == 1) || (result == 0))
@@ -187,13 +187,26 @@ bool Move(const FilePath& from_path, const FilePath& to_path) {
   if (MoveFileEx(from_path.value().c_str(), to_path.value().c_str(),
                  MOVEFILE_COPY_ALLOWED | MOVEFILE_REPLACE_EXISTING) != 0)
     return true;
+
+  // Keep the last error value from MoveFileEx around in case the below
+  // fails.
+  bool ret = false;
+  DWORD last_error = ::GetLastError();
+
   if (DirectoryExists(from_path)) {
     // MoveFileEx fails if moving directory across volumes. We will simulate
     // the move by using Copy and Delete. Ideally we could check whether
     // from_path and to_path are indeed in different volumes.
-    return CopyAndDeleteDirectory(from_path, to_path);
+    ret = CopyAndDeleteDirectory(from_path, to_path);
   }
-  return false;
+
+  if (!ret) {
+    // Leave a clue about what went wrong so that it can be (at least) picked
+    // up by a PLOG entry.
+    ::SetLastError(last_error);
+  }
+
+  return ret;
 }
 
 bool ReplaceFile(const FilePath& from_path, const FilePath& to_path) {
@@ -442,7 +455,6 @@ bool CreateShortcutLink(const wchar_t *source, const wchar_t *destination,
   return SUCCEEDED(result);
 }
 
-
 bool UpdateShortcutLink(const wchar_t *source, const wchar_t *destination,
                         const wchar_t *working_dir, const wchar_t *arguments,
                         const wchar_t *description, const wchar_t *icon,
@@ -921,8 +933,9 @@ FilePath FileEnumerator::Next() {
       }
       if (file_type_ & FileEnumerator::DIRECTORIES)
         return cur_file;
-    } else if (file_type_ & FileEnumerator::FILES)
+    } else if (file_type_ & FileEnumerator::FILES) {
       return cur_file;
+    }
   }
 
   return FilePath();
@@ -1007,7 +1020,7 @@ void MemoryMappedFile::CloseHandles() {
 bool HasFileBeenModifiedSince(const FileEnumerator::FindInfo& find_info,
                               const base::Time& cutoff_time) {
   base::ThreadRestrictions::AssertIOAllowed();
-  long result = CompareFileTime(&find_info.ftLastWriteTime,
+  long result = CompareFileTime(&find_info.ftLastWriteTime,  // NOLINT
                                 &cutoff_time.ToFileTime());
   return result == 1 || result == 0;
 }
@@ -1050,7 +1063,7 @@ bool NormalizeToNativeFilePath(const FilePath& path, FilePath* nt_path) {
                           NULL,
                           PAGE_READONLY,
                           0,
-                          1, // Just one byte.  No need to look at the data.
+                          1,  // Just one byte.  No need to look at the data.
                           NULL));
   if (!file_map_handle)
     return false;
diff --git a/chrome/installer/setup/uninstall.cc b/chrome/installer/setup/uninstall.cc
index a4d596f..0b98e2c 100644
--- a/chrome/installer/setup/uninstall.cc
+++ b/chrome/installer/setup/uninstall.cc
@@ -366,7 +366,7 @@ bool MoveSetupOutOfInstallFolder(const InstallerState& installer_state,
   } else {
     VLOG(1) << "Attempting to move setup to: " << temp_file.value();
     ret = file_util::Move(setup_exe, temp_file);
-    LOG_IF(ERROR, !ret) << "Failed to move setup to " << temp_file.value();
+    PLOG_IF(ERROR, !ret) << "Failed to move setup to " << temp_file.value();
   }
   return ret;
 }
@@ -491,31 +491,24 @@ bool DeleteChromeRegistrationKeys(BrowserDistribution* dist, HKEY root,
     return true;
   }
 
-  RegKey key;
-  LONG result = key.Open(root, L"", KEY_ALL_ACCESS);
-  if (result != ERROR_SUCCESS) {
-    LOG(ERROR) << "DeleteChromeRegistrationKeys: failed to open root key, "
-                  "result: " << result;
-  }
-
   // Delete Software\Classes\ChromeHTML,
   std::wstring html_prog_id(ShellUtil::kRegClasses);
   file_util::AppendToPath(&html_prog_id, ShellUtil::kChromeHTMLProgId);
   html_prog_id.append(browser_entry_suffix);
-  InstallUtil::DeleteRegistryKey(key, html_prog_id);
+  InstallUtil::DeleteRegistryKey(root, html_prog_id);
 
   // Delete Software\Clients\StartMenuInternet\Chromium
   std::wstring set_access_key(ShellUtil::kRegStartMenuInternet);
   file_util::AppendToPath(&set_access_key, dist->GetApplicationName());
   set_access_key.append(browser_entry_suffix);
-  InstallUtil::DeleteRegistryKey(key, set_access_key);
+  InstallUtil::DeleteRegistryKey(root, set_access_key);
 
   // We have renamed the StartMenuInternet\chrome.exe to
   // StartMenuInternet\Chromium so for old users we still need to delete
   // the old key.
   std::wstring old_set_access_key(ShellUtil::kRegStartMenuInternet);
   file_util::AppendToPath(&old_set_access_key, installer::kChromeExe);
-  InstallUtil::DeleteRegistryKey(key, old_set_access_key);
+  InstallUtil::DeleteRegistryKey(root, old_set_access_key);
 
   // Delete Software\RegisteredApplications\Chromium
   InstallUtil::DeleteRegistryValue(root, ShellUtil::kRegRegisteredApplications,
@@ -525,12 +518,12 @@ bool DeleteChromeRegistrationKeys(BrowserDistribution* dist, HKEY root,
   std::wstring app_key(ShellUtil::kRegClasses);
   file_util::AppendToPath(&app_key, L"Applications");
   file_util::AppendToPath(&app_key, installer::kChromeExe);
-  InstallUtil::DeleteRegistryKey(key, app_key);
+  InstallUtil::DeleteRegistryKey(root, app_key);
 
   // Delete the App Paths key that lets explorer find Chrome.
   std::wstring app_path_key(ShellUtil::kAppPathsRegistryKey);
   file_util::AppendToPath(&app_path_key, installer::kChromeExe);
-  InstallUtil::DeleteRegistryKey(key, app_path_key);
+  InstallUtil::DeleteRegistryKey(root, app_path_key);
 
   // Cleanup OpenWithList
   for (int i = 0; ShellUtil::kFileAssociations[i] != NULL; i++) {
@@ -538,10 +531,9 @@ bool DeleteChromeRegistrationKeys(BrowserDistribution* dist, HKEY root,
     file_util::AppendToPath(&open_with_key, ShellUtil::kFileAssociations[i]);
     file_util::AppendToPath(&open_with_key, L"OpenWithList");
     file_util::AppendToPath(&open_with_key, installer::kChromeExe);
-    InstallUtil::DeleteRegistryKey(key, open_with_key);
+    InstallUtil::DeleteRegistryKey(root, open_with_key);
   }
 
-  key.Close();
   exit_code = installer::UNINSTALL_SUCCESSFUL;
   return true;
 }
@@ -557,10 +549,8 @@ const wchar_t kChromeExtProgId[] = L"ChromeExt";
 const wchar_t kChromeExtProgId[] = L"ChromiumExt";
 #endif
 
-  HKEY roots[] = {HKEY_LOCAL_MACHINE, HKEY_CURRENT_USER};
+  HKEY roots[] = { HKEY_LOCAL_MACHINE, HKEY_CURRENT_USER };
   for (size_t i = 0; i < arraysize(roots); ++i) {
-    RegKey key(roots[i], L"", KEY_ALL_ACCESS);
-
     std::wstring suffix;
     if (roots[i] == HKEY_LOCAL_MACHINE &&
         !ShellUtil::GetUserSpecificDefaultBrowserSuffix(dist, &suffix))
@@ -570,13 +560,13 @@ const wchar_t kChromeExtProgId[] = L"ChromiumExt";
     std::wstring ext_prog_id(ShellUtil::kRegClasses);
     file_util::AppendToPath(&ext_prog_id, kChromeExtProgId);
     ext_prog_id.append(suffix);
-    InstallUtil::DeleteRegistryKey(key, ext_prog_id);
+    InstallUtil::DeleteRegistryKey(roots[i], ext_prog_id);
 
     // Delete Software\Classes\.crx,
     std::wstring ext_association(ShellUtil::kRegClasses);
     ext_association.append(L"\\");
     ext_association.append(chrome::kExtensionFileExtension);
-    InstallUtil::DeleteRegistryKey(key, ext_association);
+    InstallUtil::DeleteRegistryKey(roots[i], ext_association);
   }
 }
 
@@ -664,15 +654,14 @@ InstallStatus UninstallProduct(const InstallationState& original_state,
 
   // Delete the registry keys (Uninstall key and Version key).
   HKEY reg_root = installer_state.root_key();
-  RegKey key(reg_root, L"", KEY_ALL_ACCESS);
 
   // Note that we must retrieve the distribution-specific data before deleting
   // product.GetVersionKey().
   std::wstring distribution_data(browser_dist->GetDistributionData(reg_root));
 
   // Remove Control Panel uninstall link and Omaha product key.
-  InstallUtil::DeleteRegistryKey(key, browser_dist->GetUninstallRegPath());
-  InstallUtil::DeleteRegistryKey(key, browser_dist->GetVersionKey());
+  InstallUtil::DeleteRegistryKey(reg_root, browser_dist->GetUninstallRegPath());
+  InstallUtil::DeleteRegistryKey(reg_root, browser_dist->GetVersionKey());
 
   // Also try to delete the MSI value in the ClientState key (it might not be
   // there). This is due to a Google Update behaviour where an uninstall and a
@@ -715,14 +704,9 @@ InstallStatus UninstallProduct(const InstallationState& original_state,
       // Delete media player registry key that exists only in HKLM.
       // We don't delete this key in SxS uninstall or Chrome Frame uninstall
       // as we never set the key for those products.
-      RegKey hklm_key(HKEY_LOCAL_MACHINE, L"", KEY_QUERY_VALUE);
-      if (hklm_key.Valid()) {
-        std::wstring reg_path(installer::kMediaPlayerRegPath);
-        file_util::AppendToPath(&reg_path, installer::kChromeExe);
-        InstallUtil::DeleteRegistryKey(hklm_key, reg_path);
-      } else {
-        LOG(ERROR) << "Failed to open HKLM to remove media player registration";
-      }
+      std::wstring reg_path(installer::kMediaPlayerRegPath);
+      file_util::AppendToPath(&reg_path, installer::kChromeExe);
+      InstallUtil::DeleteRegistryKey(HKEY_LOCAL_MACHINE, reg_path);
     }
 
     // Unregister any dll servers that we may have registered for this
@@ -792,7 +776,7 @@ InstallStatus UninstallProduct(const InstallationState& original_state,
 
       BrowserDistribution* multi_dist =
           installer_state.multi_package_binaries_distribution();
-      InstallUtil::DeleteRegistryKey(key, multi_dist->GetVersionKey());
+      InstallUtil::DeleteRegistryKey(reg_root, multi_dist->GetVersionKey());
     }
   }
 
diff --git a/chrome/installer/util/install_util.cc b/chrome/installer/util/install_util.cc
index ea5d9ce..043061b 100644
--- a/chrome/installer/util/install_util.cc
+++ b/chrome/installer/util/install_util.cc
@@ -9,6 +9,7 @@
 
 #include <shellapi.h>
 #include <shlobj.h>
+#include <shlwapi.h>
 
 #include <algorithm>
 
@@ -253,10 +254,10 @@ bool InstallUtil::BuildDLLRegistrationList(const std::wstring& install_path,
 // This method tries to delete a registry key and logs an error message
 // in case of failure. It returns true if deletion is successful,
 // otherwise false.
-bool InstallUtil::DeleteRegistryKey(RegKey& root_key,
+bool InstallUtil::DeleteRegistryKey(HKEY root_key,
                                     const std::wstring& key_path) {
   VLOG(1) << "Deleting registry key " << key_path;
-  LONG result = root_key.DeleteKey(key_path.c_str());
+  LONG result = ::SHDeleteKey(root_key, key_path.c_str());
   if (result != ERROR_SUCCESS && result != ERROR_FILE_NOT_FOUND) {
     LOG(ERROR) << "Failed to delete registry key: " << key_path
                << " error: " << result;
diff --git a/chrome/installer/util/install_util.h b/chrome/installer/util/install_util.h
index e03a4f9..0106f4c 100644
--- a/chrome/installer/util/install_util.h
+++ b/chrome/installer/util/install_util.h
@@ -22,12 +22,6 @@
 class Version;
 class WorkItemList;
 
-namespace base {
-namespace win {
-class RegKey;
-}  // namespace win
-}  // namespace base
-
 // This is a utility class that provides common installation related
 // utility methods that can be used by installer and also unit tested
 // independently.
@@ -103,8 +97,7 @@ class InstallUtil {
                                        WorkItemList* registration_list);
 
   // Deletes the registry key at path key_path under the key given by root_key.
-  static bool DeleteRegistryKey(base::win::RegKey& root_key,
-                                const std::wstring& key_path);
+  static bool DeleteRegistryKey(HKEY root_key, const std::wstring& key_path);
 
   // Deletes the registry value named value_name at path key_path under the key
   // given by reg_root.