Add WOW64 support and DeleteEmptyKey to base::win::registry.

base/win/registry.cc

Index: base/win/registry.cc
diff --git a/base/win/registry.cc b/base/win/registry.cc
index 7330d08aa277e0ab5a67d84492176556bdd9dde0..2a903e2d212486128ef3945cb35181dd7bbfe113 100644
--- a/base/win/registry.cc
+++ b/base/win/registry.cc
@@ -10,8 +10,7 @@
 #include "base/logging.h"
 #include "base/strings/string_util.h"
 #include "base/threading/thread_restrictions.h"
-
-#pragma comment(lib, "shlwapi.lib")  // for SHDeleteKey
+#include "base/win/windows_version.h"
 
 namespace base {
 namespace win {
@@ -30,23 +29,29 @@ inline DWORD to_wchar_size(DWORD byte_size) {
   return (byte_size + sizeof(wchar_t) - 1) / sizeof(wchar_t);
 }
 
+// Mask to pull WOW64 access flags out of REGSAM access.
+const REGSAM kWow64AccessMask = KEY_WOW64_32KEY | KEY_WOW64_64KEY;
+
 }  // namespace
 
 // RegKey ----------------------------------------------------------------------
 
 RegKey::RegKey()
     : key_(NULL),
-      watch_event_(0) {
+      watch_event_(0),
+      wow64access_(0) {
 }
 
 RegKey::RegKey(HKEY key)
     : key_(key),
-      watch_event_(0) {
+      watch_event_(0),
+      wow64access_(0) {
 }
 
 RegKey::RegKey(HKEY rootkey, const wchar_t* subkey, REGSAM access)
     : key_(NULL),
-      watch_event_(0) {
+      watch_event_(0),
+      wow64access_(0) {
   if (rootkey) {
     if (access & (KEY_SET_VALUE | KEY_CREATE_SUB_KEY | KEY_CREATE_LINK))
       Create(rootkey, subkey, access);
@@ -54,6 +59,7 @@ RegKey::RegKey(HKEY rootkey, const wchar_t* subkey, REGSAM access)
       Open(rootkey, subkey, access);
   } else {
     DCHECK(!subkey);
+    wow64access_ = access & kWow64AccessMask;
   }
 }
 
@@ -76,6 +82,7 @@ LONG RegKey::CreateWithDisposition(HKEY rootkey, const wchar_t* subkey,
   if (result == ERROR_SUCCESS) {
     Close();
     key_ = subhkey;
+    wow64access_ = access & kWow64AccessMask;
   }
 
   return result;
@@ -83,13 +90,19 @@ LONG RegKey::CreateWithDisposition(HKEY rootkey, const wchar_t* subkey,
 
 LONG RegKey::CreateKey(const wchar_t* name, REGSAM access) {
   DCHECK(name && access);
+  // After the application has accessed an alternate registry view using one of
+  // the [KEY_WOW64_32KEY / KEY_WOW64_64KEY] flags, all subsequent operations
+  // (create, delete, or open) on child registry keys must explicitly use the
+  // same flag. Otherwise, there can be unexpected behavior.
+  // http://msdn.microsoft.com/en-us/library/windows/desktop/aa384129.aspx.
+  if ((access & kWow64AccessMask) != wow64access_)
+    return ERROR_INVALID_PARAMETER;

[grt (UTC plus 2), 2014/05/20 15:28:52]

i had a thought about this yesterday after sending the review comment. hitting
this statement means that the RegKey API is being mis-used. whaddya think about
NOTREACHED(); just before the return so that debug builds blow up fast? this
will make the mis-use much easier to diagnose.

[Will Harris, 2014/05/20 17:03:34]

Done.

   HKEY subkey = NULL;
   LONG result = RegCreateKeyEx(key_, name, 0, NULL, REG_OPTION_NON_VOLATILE,
                                access, NULL, &subkey, NULL);
- if (result == ERROR_SUCCESS) {
-   Close();
-
-   key_ = subkey;
+  if (result == ERROR_SUCCESS) {
+    Close();
+    key_ = subkey;
   }
 
   return result;
@@ -103,6 +116,7 @@ LONG RegKey::Open(HKEY rootkey, const wchar_t* subkey, REGSAM access) {
   if (result == ERROR_SUCCESS) {
     Close();
     key_ = subhkey;
+    wow64access_ = access & kWow64AccessMask;
   }
 
   return result;
@@ -110,6 +124,13 @@ LONG RegKey::Open(HKEY rootkey, const wchar_t* subkey, REGSAM access) {
 
 LONG RegKey::OpenKey(const wchar_t* relative_key_name, REGSAM access) {
   DCHECK(relative_key_name && access);
+  // After the application has accessed an alternate registry view using one of
+  // the [KEY_WOW64_32KEY / KEY_WOW64_64KEY] flags, all subsequent operations
+  // (create, delete, or open) on child registry keys must explicitly use the
+  // same flag. Otherwise, there can be unexpected behavior.
+  // http://msdn.microsoft.com/en-us/library/windows/desktop/aa384129.aspx.
+  if ((access & kWow64AccessMask) != wow64access_)
+    return ERROR_INVALID_PARAMETER;

[grt (UTC plus 2), 2014/05/20 15:28:52]

NOTREACHED(); here as well?

[Will Harris, 2014/05/20 17:03:34]

Done.

   HKEY subkey = NULL;
   LONG result = RegOpenKeyEx(key_, relative_key_name, 0, access, &subkey);
 
@@ -127,6 +148,7 @@ void RegKey::Close() {
   if (key_) {
     ::RegCloseKey(key_);
     key_ = NULL;
+    wow64access_ = 0;
   }
 }
 
@@ -134,10 +156,12 @@ void RegKey::Set(HKEY key) {
   if (key_ != key) {
     Close();
     key_ = key;
+    wow64access_ = 0;
   }
 }
 
 HKEY RegKey::Take() {
+  DCHECK(wow64access_ == 0);
   StopWatching();
   HKEY key = key_;
   key_ = NULL;
@@ -168,8 +192,24 @@ LONG RegKey::GetValueNameAt(int index, std::wstring* name) const {
 LONG RegKey::DeleteKey(const wchar_t* name) {
   DCHECK(key_);
   DCHECK(name);
-  LONG result = SHDeleteKey(key_, name);
-  return result;
+  HKEY subkey;

[grt (UTC plus 2), 2014/05/20 15:28:52]

nit: initialize this to NULL.

[Will Harris, 2014/05/20 17:03:34]

Done.

+
+  // Verify the key exists before attempting delete to replicate previous
+  // behavior.
+  LONG result =
+      RegOpenKeyEx(key_, name, 0, READ_CONTROL | wow64access_, &subkey);
+  if (result != ERROR_SUCCESS)
+    return result;
+  RegCloseKey(subkey);
+
+  return RegDelRecurse(key_, std::wstring(name), wow64access_);
+}
+
+LONG RegKey::DeleteEmptyKey(const wchar_t* name) {

[Will Harris, 2014/05/19 23:47:25]

added DeleteEmptyKey as a WOW64 compliant replacement for SHDeleteEmptyKey,
which is used in the installer code.

[grt (UTC plus 2), 2014/05/20 15:28:52]

According to MSDN, SHDeleteEmptyKey won't delete the key if it has values. If
RegKey::DeleteEmptyKey is a replacement for it, shouldn't this function behave
the same?

[Will Harris, 2014/05/20 17:03:34]

Done.

+  DCHECK(key_);
+  DCHECK(name);
+
+  return RegDeleteKeyExWrapper(key_, name, wow64access_, 0);
 }
 
 LONG RegKey::DeleteValue(const wchar_t* value_name) {
@@ -342,6 +382,92 @@ LONG RegKey::StopWatching() {
   return result;
 }
 
+// static
+LONG RegKey::RegDeleteKeyExWrapper(HKEY hKey,
+                                   const wchar_t* lpSubKey,
+                                   REGSAM samDesired,
+                                   DWORD Reserved) {
+  typedef LSTATUS(WINAPI* RegDeleteKeyExPtr)(HKEY, LPCWSTR, REGSAM, DWORD);
+
+  RegDeleteKeyExPtr reg_delete_key_ex_func =
+      reinterpret_cast<RegDeleteKeyExPtr>(
+          GetProcAddress(GetModuleHandleA("advapi32.dll"), "RegDeleteKeyExW"));
+
+  if (reg_delete_key_ex_func)
+    return reg_delete_key_ex_func(hKey, lpSubKey, samDesired, Reserved);
+
+  // Windows XP does not support RegDeleteKeyEx, so fallback to RegDeleteKey.
+  return RegDeleteKey(hKey, lpSubKey);
+}
+
+// static
+LONG RegKey::RegDelRecurse(HKEY root_key,
+                           const std::wstring& name,
+                           REGSAM access) {
+  if (name.empty()) {
+    NOTREACHED();
+    return ERROR_INVALID_PARAMETER;
+  }
+
+  // First, see if the key can be deleted without having to recurse.
+  LONG result = RegDeleteKeyExWrapper(root_key, name.c_str(), access, 0);
+

[grt (UTC plus 2), 2014/05/20 15:28:52]

nit: remove empty line

[Will Harris, 2014/05/20 17:03:34]

Done.

+  if (result == ERROR_SUCCESS)
+    return result;
+
+  HKEY target_key;

[grt (UTC plus 2), 2014/05/20 15:28:52]

nit: initialize to NULL

[Will Harris, 2014/05/20 17:03:34]

Done.

+  result = RegOpenKeyEx(
+      root_key, name.c_str(), 0, KEY_ENUMERATE_SUB_KEYS | access, &target_key);
+
+  if (result == ERROR_FILE_NOT_FOUND)
+    return ERROR_SUCCESS;
+  if (result != ERROR_SUCCESS)
+    return result;
+
+  // Copy string before modifying it.

[grt (UTC plus 2), 2014/05/20 15:28:52]

remove unneeded comment (name is const, so it can't be modified).

[Will Harris, 2014/05/20 17:03:34]

Done.

+  std::wstring subkey_name(name);
+
+  // Check for an ending slash and add one if it is missing.
+  if (subkey_name[name.length() - 1] != L'\\') {

[grt (UTC plus 2), 2014/05/20 15:28:52]

nit: no braces for single-liner (did "git cl format" add these?)

[Will Harris, 2014/05/20 17:03:34]

I left those in by accident and git cl format didn't catch it

+    subkey_name += L"\\";
+  }
+
+  // Enumerate the keys
+  result = ERROR_SUCCESS;
+  const DWORD kMaxKeyNameLength = MAX_PATH;
+  const size_t base_key_length = subkey_name.length();
+

[grt (UTC plus 2), 2014/05/20 15:28:52]

nit: remove empty line

[Will Harris, 2014/05/20 17:03:34]

Done.

+  std::wstring key_name;
+  while (result == ERROR_SUCCESS) {
+    DWORD key_size = kMaxKeyNameLength;
+    result = RegEnumKeyEx(target_key,
+                          0,
+                          WriteInto(&key_name, kMaxKeyNameLength),
+                          &key_size,
+                          NULL,
+                          NULL,
+                          NULL,
+                          NULL);
+
+    if (result != ERROR_SUCCESS)
+      break;
+
+    key_name.resize(key_size);
+    subkey_name.resize(base_key_length);
+    subkey_name += key_name;
+
+    if (RegDelRecurse(root_key, subkey_name, access) != ERROR_SUCCESS)
+      break;
+  }
+
+  RegCloseKey(target_key);
+
+  // Try again to delete the key.
+  result = RegDeleteKeyExWrapper(root_key, name.c_str(), access, 0);
+
+  return result;
+}
+
 // RegistryValueIterator ------------------------------------------------------
 
 RegistryValueIterator::RegistryValueIterator(HKEY root_key,