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

base/win/registry_unittest.cc

Index: base/win/registry_unittest.cc
diff --git a/base/win/registry_unittest.cc b/base/win/registry_unittest.cc
index 155402a351fc4593d57f6f8a479c09969d041400..1e7b7fc1264768df30709bc071c6410f1560e09e 100644
--- a/base/win/registry_unittest.cc
+++ b/base/win/registry_unittest.cc
@@ -9,6 +9,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/stl_util.h"
+#include "base/win/windows_version.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
@@ -18,9 +19,24 @@ namespace {
 
 const wchar_t kRootKey[] = L"Base_Registry_Unittest";
 
+#if defined(_WIN64)
+const REGSAM kNativeViewMask = KEY_WOW64_64KEY;

[grt (UTC plus 2), 2014/05/17 19:31:53]

make these static const protected members of the test fixture.
may as well do the same to kRootKey. i can't think of any advantage to having it
hanging out in namespace scope like that.

[Will Harris, 2014/05/19 21:49:03]

Done.

+const REGSAM kRedirectedViewMask = KEY_WOW64_32KEY;
+#else
+const REGSAM kNativeViewMask = KEY_WOW64_32KEY;
+const REGSAM kRedirectedViewMask = KEY_WOW64_64KEY;
+#endif  //  _WIN64
+
 class RegistryTest : public testing::Test {
  public:
-  RegistryTest() {}
+  RegistryTest() {

[grt (UTC plus 2), 2014/05/17 19:31:53]

while you're here, please move this ctor into the protected: section.

[Will Harris, 2014/05/19 21:49:03]

Done.

+    foo_software_key_ = L"Software\\";

[grt (UTC plus 2), 2014/05/17 19:31:53]

may as well do this work in SetUp so that the fixture initialization isn't
divided between the ctor and SetUp methods.

[Will Harris, 2014/05/19 21:49:03]

Done.

+    foo_software_key_ += kRootKey;
+    foo_software_key_ += L"\\Foo";
+    foo_software_wow64_key_ = L"Software\\Wow6432Node\\";
+    foo_software_wow64_key_ += kRootKey;
+    foo_software_wow64_key_ += L"\\Foo";
+  }
 
  protected:
   virtual void SetUp() OVERRIDE {
@@ -35,6 +51,33 @@ class RegistryTest : public testing::Test {
     // Clean up the temporary key.
     RegKey key(HKEY_CURRENT_USER, L"", KEY_SET_VALUE);
     ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(kRootKey));
+    ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_CURRENT_USER, kRootKey, KEY_READ));
+
+    // Delete all variations of the Software key we created.
+    ASSERT_EQ(
+        ERROR_SUCCESS,
+        key.Open(
+            HKEY_LOCAL_MACHINE, L"Software", KEY_SET_VALUE | KEY_WOW64_32KEY));
+    key.DeleteKey(kRootKey);

[grt (UTC plus 2), 2014/05/17 19:31:53]

?    EXPECT_EQ(ERROR_SUCCESS, key.DeleteKey(kRootKey));?
below, too?

+
+    ASSERT_EQ(
+        ERROR_SUCCESS,
+        key.Open(
+            HKEY_LOCAL_MACHINE, L"Software", KEY_SET_VALUE | KEY_WOW64_64KEY));
+    key.DeleteKey(kRootKey);
+  }
+
+  std::wstring foo_software_key_;

[grt (UTC plus 2), 2014/05/17 19:31:53]

move these down to the end of the protected: section (non-const data members
come last;
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Declaration_Order).

[Will Harris, 2014/05/19 21:49:03]

Done.

+  std::wstring foo_software_wow64_key_;
+
+  static bool IsRedirectorPresent() {
+#if defined(_WIN64)
+    return true;
+#else
+    OSInfo* os_info = OSInfo::GetInstance();
+
+    return os_info->wow64_status() == OSInfo::WOW64_ENABLED;

[grt (UTC plus 2), 2014/05/17 19:31:53]

    return OSInfo::GetInstance()->wow64_status() == OSInfo::WOW64_ENABLED;

[Will Harris, 2014/05/19 21:49:03]

Done.

+#endif
   }
 
  private:
@@ -158,6 +201,162 @@ TEST_F(RegistryTest, TruncatedCharTest) {
   EXPECT_FALSE(iterator.Valid());
 }
 
+TEST_F(RegistryTest, RecursiveDelete) {
+  RegKey key;
+  // Create kRootKey->Foo
+  //                  \->Bar
+  //                     \->Foo
+  //                  \->Moo
+  //                  \->Foo
+  // and delete kRootKey->Foo
+  std::wstring foo_key(kRootKey);
+  foo_key += L"\\Foo";
+  ASSERT_EQ(ERROR_SUCCESS,
+            key.Create(HKEY_CURRENT_USER, foo_key.c_str(), KEY_WRITE));
+  foo_key += L"\\Bar";
+
+  ASSERT_NE(ERROR_SUCCESS,
+            key.Open(HKEY_CURRENT_USER, foo_key.c_str(), KEY_READ));
+  ASSERT_EQ(ERROR_SUCCESS, key.CreateKey(L"Bar", KEY_WRITE));
+  ASSERT_EQ(ERROR_SUCCESS, key.CreateKey(L"Moo", KEY_WRITE));
+  ASSERT_EQ(ERROR_SUCCESS, key.CreateKey(L"Foo", KEY_WRITE));
+  ASSERT_EQ(ERROR_SUCCESS,
+            key.Open(HKEY_CURRENT_USER, foo_key.c_str(), KEY_WRITE));
+  foo_key += L"\\Foo";
+  ASSERT_EQ(ERROR_SUCCESS, key.CreateKey(L"Foo", KEY_WRITE));
+  ASSERT_EQ(ERROR_SUCCESS,
+            key.Open(HKEY_CURRENT_USER, foo_key.c_str(), KEY_READ));
+
+  ASSERT_EQ(ERROR_SUCCESS, key.Open(HKEY_CURRENT_USER, kRootKey, KEY_WRITE));
+  ASSERT_NE(ERROR_SUCCESS, key.DeleteKey(L"Bar"));
+  ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(L"Foo"));
+  ASSERT_NE(ERROR_SUCCESS, key.DeleteKey(L"Foo"));
+  ASSERT_NE(ERROR_SUCCESS,
+            key.Open(HKEY_CURRENT_USER, foo_key.c_str(), KEY_READ));
+}
+
+// This test requires running as an Administrator as it tests redirected
+// registry writes to HKLM\Software
+// http://msdn.microsoft.com/en-us/library/windows/desktop/aa384253.aspx
+TEST_F(RegistryTest, Wow64RedirectedFromNative) {
+  if (IsRedirectorPresent()) {

[grt (UTC plus 2), 2014/05/17 19:31:53]

please have these early-exit in the case where the test doesn't apply. it avoids
having the whole body of the test indented an extra two spaces and makes it more
clear that the whole test is skipped when there's no redirection.

[Will Harris, 2014/05/19 21:49:03]

Done.

+    RegKey key;
+
+    // Test redirected key access from non-redirected.
+    ASSERT_EQ(ERROR_SUCCESS,
+              key.Create(HKEY_LOCAL_MACHINE,
+                         foo_software_key_.c_str(),
+                         KEY_WRITE | kRedirectedViewMask));
+    ASSERT_NE(ERROR_SUCCESS,
+              key.Open(HKEY_LOCAL_MACHINE,
+                       foo_software_key_.c_str(),
+                       KEY_READ));
+    ASSERT_NE(ERROR_SUCCESS,
+              key.Open(HKEY_LOCAL_MACHINE,
+                       foo_software_key_.c_str(),
+                       KEY_READ | kNativeViewMask));
+
+    // Open the non-redirected view of the parent and try to delete the test
+    // key.
+    ASSERT_EQ(ERROR_SUCCESS,
+              key.Open(HKEY_LOCAL_MACHINE,
+                       L"Software",
+                       KEY_SET_VALUE));
+    ASSERT_NE(ERROR_SUCCESS, key.DeleteKey(kRootKey));
+    ASSERT_EQ(ERROR_SUCCESS,
+              key.Open(HKEY_LOCAL_MACHINE,
+                       L"Software",
+                       KEY_SET_VALUE | kNativeViewMask));
+    ASSERT_NE(ERROR_SUCCESS, key.DeleteKey(kRootKey));
+

[grt (UTC plus 2), 2014/05/17 19:31:53]

    // Open the redirected view and delete the key created above.

[Will Harris, 2014/05/19 21:49:03]

Done.

+    ASSERT_EQ(ERROR_SUCCESS,
+              key.Open(HKEY_LOCAL_MACHINE,
+                       L"Software",
+                       KEY_SET_VALUE | kRedirectedViewMask));
+    ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(kRootKey));
+  }
+}
+
+TEST_F(RegistryTest, Wow64NativeFromRedirected) {
+  if (IsRedirectorPresent()) {
+    RegKey key;
+
+    // Test non-redirected key access from redirected.
+    ASSERT_EQ(ERROR_SUCCESS,
+              key.Create(HKEY_LOCAL_MACHINE,
+                         foo_software_key_.c_str(),
+                         KEY_WRITE | kNativeViewMask));
+    ASSERT_EQ(ERROR_SUCCESS,
+              key.Open(HKEY_LOCAL_MACHINE,
+                       foo_software_key_.c_str(),
+                       KEY_READ));
+    ASSERT_NE(ERROR_SUCCESS,
+              key.Open(HKEY_LOCAL_MACHINE,
+                       foo_software_key_.c_str(),
+                       KEY_READ | kRedirectedViewMask));
+
+    // Open the redirected view of the parent and try to delete the test key
+    // from the non-redirected view.
+    ASSERT_EQ(ERROR_SUCCESS,
+              key.Open(HKEY_LOCAL_MACHINE,
+                       L"Software",
+                       KEY_SET_VALUE | kRedirectedViewMask));
+    ASSERT_NE(ERROR_SUCCESS, key.DeleteKey(kRootKey));
+
+    ASSERT_EQ(ERROR_SUCCESS,
+              key.Open(HKEY_LOCAL_MACHINE,
+                       L"Software",
+                       KEY_SET_VALUE | kNativeViewMask));
+    ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(kRootKey));
+  }
+}
+
+TEST_F(RegistryTest, Wow6432NodeFromRedirected) {
+  if (IsRedirectorPresent()) {
+    RegKey key;
+    // Test access to 32-bit values on 64-bit via the Wow6432Node key.

[grt (UTC plus 2), 2014/05/17 19:31:53]

is this worth testing? i thought Wow6432Node was an implementation detail and
that apps aren't supposed to try to use it directly.

[Will Harris, 2014/05/19 21:49:03]

earlier tests just confirm that when KEY_WOW64_32KEY is passed, the keys go into
a different place in the registry, but don't actually test that they go into the
right place in the registry...  I think this test is still worth having.

+    ASSERT_EQ(ERROR_SUCCESS,
+              key.Create(HKEY_LOCAL_MACHINE,
+                         foo_software_key_.c_str(),
+                         KEY_WRITE | KEY_WOW64_32KEY));
+    ASSERT_EQ(ERROR_SUCCESS,
+              key.Open(HKEY_LOCAL_MACHINE,
+                       foo_software_wow64_key_.c_str(),
+                       KEY_READ));
+    ASSERT_EQ(ERROR_SUCCESS,
+              key.Open(HKEY_LOCAL_MACHINE,
+                       foo_software_wow64_key_.c_str(),
+                       KEY_READ | KEY_WOW64_64KEY));
+    ASSERT_EQ(ERROR_SUCCESS,
+              key.Open(HKEY_LOCAL_MACHINE,
+                       foo_software_wow64_key_.c_str(),
+                       KEY_READ | KEY_WOW64_32KEY));
+    ASSERT_EQ(ERROR_SUCCESS,
+              key.Open(HKEY_LOCAL_MACHINE,
+                       L"Software",
+                       KEY_SET_VALUE | KEY_WOW64_32KEY));
+    ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(kRootKey));
+  }
+}
+
+TEST_F(RegistryTest, NativeNotRedirected) {
+  if (!IsRedirectorPresent()) {
+    RegKey key;
+    // 32-bit on 32-bit shouldn't even see a Wow6432Node key.

[grt (UTC plus 2), 2014/05/17 19:31:53]

this also seems to be testing an implementation detail if windows rather than
correct behavior of RegKey

[Will Harris, 2014/05/19 21:49:03]

Happy for this to be removed.

+    ASSERT_EQ(ERROR_SUCCESS,
+              key.Create(HKEY_LOCAL_MACHINE,
+                         foo_software_key_.c_str(),
+                         KEY_READ));
+    ASSERT_NE(ERROR_SUCCESS,
+              key.Open(HKEY_LOCAL_MACHINE,
+                       foo_software_wow64_key_.c_str(),
+                       KEY_READ));
+    ASSERT_EQ(ERROR_SUCCESS,
+              key.Open(HKEY_LOCAL_MACHINE, L"Software", KEY_SET_VALUE));
+    ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(kRootKey));
+  }
+}
+
 }  // namespace
 
 }  // namespace win