Add WOW64 support to the installer registry work items

chrome/installer/util/install_util.cc

Index: chrome/installer/util/install_util.cc
diff --git a/chrome/installer/util/install_util.cc b/chrome/installer/util/install_util.cc
index aaa5663ccff5a158501e3952c369a30e9930e617..6c96b335d3d27b466fcf575f88f10ab3ad82614f 100644
--- a/chrome/installer/util/install_util.cc
+++ b/chrome/installer/util/install_util.cc
@@ -286,21 +286,37 @@ void InstallUtil::AddInstallerResultItems(
   DCHECK(install_list);
   const HKEY root = system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
   DWORD installer_result = (GetInstallReturnCode(status) == 0) ? 0 : 1;
-  install_list->AddCreateRegKeyWorkItem(root, state_key);
-  install_list->AddSetRegValueWorkItem(root, state_key,
+  install_list->AddCreateRegKeyWorkItem(
+      root, state_key, WorkItem::kWow64Default);
+  install_list->AddSetRegValueWorkItem(root,
+                                       state_key,
                                        installer::kInstallerResult,
-                                       installer_result, true);
-  install_list->AddSetRegValueWorkItem(root, state_key,
+                                       installer_result,
+                                       true,
+                                       WorkItem::kWow64Default);
+  install_list->AddSetRegValueWorkItem(root,
+                                       state_key,
                                        installer::kInstallerError,
-                                       static_cast<DWORD>(status), true);
+                                       static_cast<DWORD>(status),
+                                       true,
+                                       WorkItem::kWow64Default);
   if (string_resource_id != 0) {
     base::string16 msg = installer::GetLocalizedString(string_resource_id);
-    install_list->AddSetRegValueWorkItem(root, state_key,
-        installer::kInstallerResultUIString, msg, true);
+    install_list->AddSetRegValueWorkItem(root,
+                                         state_key,
+                                         installer::kInstallerResultUIString,
+                                         msg,
+                                         true,
+                                         WorkItem::kWow64Default);
   }
   if (launch_cmd != NULL && !launch_cmd->empty()) {
-    install_list->AddSetRegValueWorkItem(root, state_key,
-        installer::kInstallerSuccessLaunchCmdLine, *launch_cmd, true);
+    install_list->AddSetRegValueWorkItem(
+        root,
+        state_key,
+        installer::kInstallerSuccessLaunchCmdLine,
+        *launch_cmd,
+        true,
+        WorkItem::kWow64Default);
   }
 }
 
@@ -417,19 +433,16 @@ bool InstallUtil::GetEULASentinelFilePath(base::FilePath* path) {
   return true;
 }
 
-// This method tries to delete a registry key and logs an error message
-// in case of failure. It returns true if deletion is successful (or the key did
-// not exist), otherwise false.
-bool InstallUtil::DeleteRegistryKey(HKEY root_key,
-                                    const base::string16& key_path) {
-  VLOG(1) << "Deleting registry key " << key_path;
-  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;
-    return false;
-  }
-  return true;
+// Deletes the registry key at path key_path under the key given by root_key.
+LONG InstallUtil::DeleteRegistryKey(HKEY root_key,

[grt (UTC plus 2), 2014/05/21 16:41:05]

previously, this would return true for both successful delete and for the case
where the key didn't exist in the first place. since this is a convenience
function, i think it makes sense for it to return a bool since that's what all
consumers care about. otherwise, consumers need to know details of how the
registry functions work.

[Will Harris, 2014/05/21 23:23:16]

I changed this since it was used by the new code in DeleteRegKeyWorkItem which
needs the full code, and other places which just needs bool, so I just changed
all the other callers to handle the LONG return type and share the code.

[grt (UTC plus 2), 2014/05/22 01:37:44]

Does it? I could be reading it wrong, but modulo the text in the log message it
looks like DeleteRegKeyWorkItem makes the same determination that the old
InstallUtil::DeleteRegistryKey made. Can you not simplify
DeleteRegKeyWorkItem::Do() so that it does:

  if (!InstallUtil::DeleteRegistryKey(...)) {
    return ignore_failure_;
  }

and DeleteRegKeyWorkItem::Rollback() becomes just:

  InstallUtil::DeleteRegistryKey(...);

I think the logs will still be good enough to tell what's going on.

[Will Harris, 2014/05/22 21:29:24]

Done.

+                                    const base::string16& key_path,
+                                    REGSAM wow64_access) {
+  RegKey root_regkey;
+  LONG result = root_regkey.Open(root_key, L"",
+                                 KEY_READ | KEY_WRITE | wow64_access);
+  if (result != ERROR_SUCCESS)
+    return result;
+  return root_regkey.DeleteKey(key_path.c_str());
 }
 
 // This method tries to delete a registry value and logs an error message
@@ -456,7 +469,8 @@ InstallUtil::ConditionalDeleteResult InstallUtil::DeleteRegistryKeyIf(
     const base::string16& key_to_delete_path,
     const base::string16& key_to_test_path,
     const wchar_t* value_name,
-    const RegistryValuePredicate& predicate) {
+    const RegistryValuePredicate& predicate,
+    const REGSAM wow64_access) {
   DCHECK(root_key);
   ConditionalDeleteResult delete_result = NOT_FOUND;
   RegKey key;
@@ -466,7 +480,8 @@ InstallUtil::ConditionalDeleteResult InstallUtil::DeleteRegistryKeyIf(
       key.ReadValue(value_name, &actual_value) == ERROR_SUCCESS &&
       predicate.Evaluate(actual_value)) {
     key.Close();
-    delete_result = DeleteRegistryKey(root_key, key_to_delete_path)
+    LONG result = DeleteRegistryKey(root_key, key_to_delete_path, wow64_access);
+    delete_result = (result == ERROR_SUCCESS || result == ERROR_FILE_NOT_FOUND)
         ? DELETED : DELETE_FAILED;
   }
   return delete_result;
@@ -477,14 +492,16 @@ InstallUtil::ConditionalDeleteResult InstallUtil::DeleteRegistryValueIf(
     HKEY root_key,
     const wchar_t* key_path,
     const wchar_t* value_name,
-    const RegistryValuePredicate& predicate) {
+    const RegistryValuePredicate& predicate,
+    REGSAM wow64_access) {
   DCHECK(root_key);
   DCHECK(key_path);
   ConditionalDeleteResult delete_result = NOT_FOUND;
   RegKey key;
   base::string16 actual_value;
   if (key.Open(root_key, key_path,
-               KEY_QUERY_VALUE | KEY_SET_VALUE) == ERROR_SUCCESS &&
+               KEY_QUERY_VALUE | KEY_SET_VALUE | wow64_access)
+          == ERROR_SUCCESS &&
       key.ReadValue(value_name, &actual_value) == ERROR_SUCCESS &&
       predicate.Evaluate(actual_value)) {
     LONG result = key.DeleteValue(value_name);