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

base/win/registry.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.
 
 #include "base/win/registry.h"
 
 #include <shlwapi.h>
 #include <algorithm>
 
 #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 {
 
 namespace {
 
 // RegEnumValue() reports the number of characters from the name that were
 // written to the buffer, not how many there are. This constant is the maximum
 // name size, such that a buffer with this size should read any name.
 const DWORD MAX_REGISTRY_NAME_SIZE = 16384;
 
 // Registry values are read as BYTE* but can have wchar_t* data whose last
 // wchar_t is truncated. This function converts the reported |byte_size| to
 // a size in wchar_t that can store a truncated wchar_t if necessary.
 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);
     else
       Open(rootkey, subkey, access);
   } else {
     DCHECK(!subkey);
+    wow64access_ = access & kWow64AccessMask;
   }
 }
 
 RegKey::~RegKey() {
   Close();
 }
 
 LONG RegKey::Create(HKEY rootkey, const wchar_t* subkey, REGSAM access) {
   DWORD disposition_value;
   return CreateWithDisposition(rootkey, subkey, &disposition_value, access);
 }
 
 LONG RegKey::CreateWithDisposition(HKEY rootkey, const wchar_t* subkey,
                                    DWORD* disposition, REGSAM access) {
   DCHECK(rootkey && subkey && access && disposition);
   HKEY subhkey = NULL;
   LONG result = RegCreateKeyEx(rootkey, subkey, 0, NULL,
                                REG_OPTION_NON_VOLATILE, access, NULL, &subhkey,
                                disposition);
   if (result == ERROR_SUCCESS) {
     Close();
     key_ = subhkey;
+    wow64access_ = access & kWow64AccessMask;
   }
 
   return result;
 }
 
 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;
 }
 
 LONG RegKey::Open(HKEY rootkey, const wchar_t* subkey, REGSAM access) {
   DCHECK(rootkey && subkey && access);
   HKEY subhkey = NULL;
 
   LONG result = RegOpenKeyEx(rootkey, subkey, 0, access, &subhkey);
   if (result == ERROR_SUCCESS) {
     Close();
     key_ = subhkey;
+    wow64access_ = access & kWow64AccessMask;
   }
 
   return result;
 }
 
 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);
 
   // We have to close the current opened key before replacing it with the new
   // one.
   if (result == ERROR_SUCCESS) {
     Close();
     key_ = subkey;
   }
   return result;
 }
 
 void RegKey::Close() {
   StopWatching();
   if (key_) {
     ::RegCloseKey(key_);
     key_ = NULL;
+    wow64access_ = 0;
   }
 }
 
 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;
   return key;
 }
 
 bool RegKey::HasValue(const wchar_t* name) const {
   return RegQueryValueEx(key_, name, 0, NULL, NULL, NULL) == ERROR_SUCCESS;
 }
 
@@ skipping 10 matching lines @@
   LONG r = ::RegEnumValue(key_, index, buf, &bufsize, NULL, NULL, NULL, NULL);
   if (r == ERROR_SUCCESS)
     *name = buf;
 
   return r;
 }
 
 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) {
   DCHECK(key_);
   LONG result = RegDeleteValue(key_, value_name);
   return result;
 }
 
 LONG RegKey::ReadValueDW(const wchar_t* name, DWORD* out_value) const {
   DCHECK(out_value);
@@ skipping 152 matching lines @@
 LONG RegKey::StopWatching() {
   LONG result = ERROR_INVALID_HANDLE;
   if (watch_event_) {
     CloseHandle(watch_event_);
     watch_event_ = 0;
     result = ERROR_SUCCESS;
   }
   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,
                                              const wchar_t* folder_key)
     : name_(MAX_PATH, L'\0'),
       value_(MAX_PATH, L'\0') {
   LONG result = RegOpenKeyEx(root_key, folder_key, 0, KEY_READ, &key_);
   if (result != ERROR_SUCCESS) {
     key_ = NULL;
   } else {
@@ skipping 132 matching lines @@
     if (ERROR_SUCCESS == r)
       return true;
   }
 
   name_[0] = '\0';
   return false;
 }
 
 }  // namespace win
 }  // namespace base