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

base/win/registry_unittest.cc

 // Copyright (c) 2011 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 <cstring>
 #include <vector>
 
 #include "base/compiler_specific.h"
 #include "base/stl_util.h"
+#include "base/win/windows_version.h"

[grt (UTC plus 2), 2014/05/16 15:44:43]

unused

[Will Harris, 2014/05/16 22:55:45]

used for OSInfo below

 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
 namespace win {
 
 namespace {
 
 const wchar_t kRootKey[] = L"Base_Registry_Unittest";
 
 class RegistryTest : public testing::Test {
  public:
   RegistryTest() {}
 
  protected:
   virtual void SetUp() OVERRIDE {
     // Create a temporary key.
     RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS);
     key.DeleteKey(kRootKey);
     ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_CURRENT_USER, kRootKey, KEY_READ));
     ASSERT_EQ(ERROR_SUCCESS, key.Create(HKEY_CURRENT_USER, kRootKey, KEY_READ));
   }
 
   virtual void TearDown() OVERRIDE {
     // 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));

[grt (UTC plus 2), 2014/05/16 15:44:43]

indentation. please try "git cl format". it wraps the conditional operator
wrong, though. :-(

[Will Harris, 2014/05/16 22:55:45]

Done.

+    key.DeleteKey(kRootKey);
+
+    ASSERT_EQ(ERROR_SUCCESS, key.Open(HKEY_LOCAL_MACHINE, L"Software",
+      KEY_SET_VALUE | KEY_WOW64_64KEY));
+    key.DeleteKey(kRootKey);
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(RegistryTest);
 };
 
 TEST_F(RegistryTest, ValueTest) {
   RegKey key;
 
   std::wstring foo_key(kRootKey);
@@ skipping 103 matching lines @@
   ASSERT_EQ(arraysize(kData), iterator.ValueSize());
   // Value() is NUL terminated.
   int end = (iterator.ValueSize() + sizeof(wchar_t) - 1) / sizeof(wchar_t);
   EXPECT_NE(L'\0', iterator.Value()[end-1]);
   EXPECT_EQ(L'\0', iterator.Value()[end]);
   EXPECT_EQ(0, std::memcmp(kData, iterator.Value(), arraysize(kData)));
   ++iterator;
   EXPECT_FALSE(iterator.Valid());
 }
 
+TEST_F(RegistryTest, RecursiveDelete) {

[grt (UTC plus 2), 2014/05/16 15:44:43]

DeleteKey was untested before?!??!?

[Will Harris, 2014/05/16 22:55:45]

Yes - scary.  I tested the old SHDeleteKey code with this new test to verify the
new behavior was identical.

+  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

[grt (UTC plus 2), 2014/05/16 15:44:43]

can the Administrator requirement be removed by making the test operate in
HKEY_CURRENT_USER\Software\Classes or one of its subkeys that is
redirected/reflected?

[Will Harris, 2014/05/16 22:55:45]

HKLM\Software is the only key that is universally redirected on all versions of
Windows.  HKCU\Software\Classes is reflected below Win7, and so would have
required a load of windows version specific test code.

+// http://msdn.microsoft.com/en-us/library/windows/desktop/aa384253.aspx
+TEST_F(RegistryTest, Wow64) {
+  RegKey key;
+  std::wstring foo_key(L"Software\\");
+  foo_key += kRootKey;
+  foo_key += L"\\Foo";
+  std::wstring foo_wow64_key(L"Software\\Wow6432Node\\");
+  foo_wow64_key += kRootKey;
+  foo_wow64_key += L"\\Foo";
+
+#if defined(_WIN64)
+  // Test 32-bit key access from 64-bit.

[grt (UTC plus 2), 2014/05/16 15:44:43]

it looks like a lot of the _WIN64 part and the non-_WIN64 part are the same
except for some constants. could you split this into a few small TEST functions
that test individual aspects of this? for example, you could do something like:

class RegistryTest {
 protected:
  // Returns true if the current process is influenced by the registry
redirector.
  static bool IsRedirectorPresent() {
#if defined(_WIN64)
    return true;
#else
    return os_info->wow64_status() == OSInfo::WOW64_ENABLED;
#endif
  }
  static REGASM GetOppositeViewMask() {
    DCHECK(IsRedirectoryPresent());
#if defined(_WIN64)
    return KEY_WOW64_32KEY;
#else
    return KEY_WOW64_64KEY;
#endif
  }
};

TEST_F(RegistryTest, AccessOpposite) {
  // early exit
  if (!IsRedirectorPresent())
    return;
  // test accessing either 32-bit from 64-bit, or 64-bit from 32-bit
  // using GetOppositeViewMask()
}

TEST_F(RegistryTest, DeleteOpposite) {
  // early exit
  if (!IsRedirectorPresent())
    return;
  // test deleting either 32-bit from 64-bit, or 64-bit from 32-bit
  // using GetOppositeViewMask()
}

...

[Will Harris, 2014/05/16 22:55:45]

I went ahead and split this test up a bit.

+  ASSERT_EQ(ERROR_SUCCESS, key.Create(HKEY_LOCAL_MACHINE, foo_key.c_str(),
+      KEY_WRITE | KEY_WOW64_32KEY));
+  ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_LOCAL_MACHINE, foo_key.c_str(),
+      KEY_READ));
+  ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_LOCAL_MACHINE, foo_key.c_str(),
+      KEY_READ | KEY_WOW64_64KEY));
+
+  // Open the 64-bit 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 | KEY_WOW64_64KEY));
+  ASSERT_NE(ERROR_SUCCESS, key.DeleteKey(kRootKey));
+
+  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 64-bit key access from 64-bit.
+  ASSERT_EQ(ERROR_SUCCESS, key.Create(HKEY_LOCAL_MACHINE, foo_key.c_str(),
+      KEY_WRITE | KEY_WOW64_64KEY));
+  ASSERT_EQ(ERROR_SUCCESS, key.Open(HKEY_LOCAL_MACHINE, foo_key.c_str(),
+      KEY_READ));
+  ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_LOCAL_MACHINE, foo_key.c_str(),
+      KEY_READ | KEY_WOW64_32KEY));
+
+  // Open the 32-bit 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 | KEY_WOW64_32KEY));
+  ASSERT_NE(ERROR_SUCCESS, key.DeleteKey(kRootKey));
+
+  ASSERT_EQ(ERROR_SUCCESS, key.Open(HKEY_LOCAL_MACHINE, L"Software",
+      KEY_SET_VALUE | KEY_WOW64_64KEY));
+  ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(kRootKey));
+#else
+  OSInfo* os_info = OSInfo::GetInstance();
+
+  if (os_info->wow64_status() == OSInfo::WOW64_ENABLED) {
+    // Test 64-bit key access from 32-bit.
+    ASSERT_EQ(ERROR_SUCCESS, key.Create(HKEY_LOCAL_MACHINE, foo_key.c_str(),
+        KEY_WRITE | KEY_WOW64_64KEY));
+    ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_LOCAL_MACHINE, foo_key.c_str(),
+        KEY_READ));
+    ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_LOCAL_MACHINE, foo_key.c_str(),
+        KEY_READ | KEY_WOW64_32KEY));
+
+    // Open the 32-bit 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 | KEY_WOW64_32KEY));
+    ASSERT_NE(ERROR_SUCCESS, key.DeleteKey(kRootKey));
+
+    ASSERT_EQ(ERROR_SUCCESS, key.Open(HKEY_LOCAL_MACHINE, L"Software",
+        KEY_SET_VALUE | KEY_WOW64_64KEY));
+    ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(kRootKey));
+
+    // Test 32-bit key access from 32-bit.
+    ASSERT_EQ(ERROR_SUCCESS, key.Create(HKEY_LOCAL_MACHINE, foo_key.c_str(),
+        KEY_WRITE | KEY_WOW64_32KEY));
+    ASSERT_EQ(ERROR_SUCCESS, key.Open(HKEY_LOCAL_MACHINE, foo_key.c_str(),
+        KEY_READ));
+    ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_LOCAL_MACHINE, foo_key.c_str(),
+        KEY_READ | KEY_WOW64_64KEY));
+
+    // Test accessing the 32-bit values via Wow6432Node.
+    ASSERT_EQ(ERROR_SUCCESS, key.Open(HKEY_LOCAL_MACHINE,
+        foo_wow64_key.c_str(), KEY_READ));
+    ASSERT_EQ(ERROR_SUCCESS, key.Open(HKEY_LOCAL_MACHINE,
+        foo_wow64_key.c_str(), KEY_READ | KEY_WOW64_64KEY));
+    ASSERT_EQ(ERROR_SUCCESS, key.Open(HKEY_LOCAL_MACHINE,
+        foo_wow64_key.c_str(), KEY_READ | KEY_WOW64_32KEY));
+
+    // Open the 64-bit 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 | KEY_WOW64_64KEY));
+    ASSERT_NE(ERROR_SUCCESS, key.DeleteKey(kRootKey));
+
+    ASSERT_EQ(ERROR_SUCCESS, key.Open(HKEY_LOCAL_MACHINE, L"Software",
+        KEY_SET_VALUE | KEY_WOW64_32KEY));
+    ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(kRootKey));
+  } else {
+    ASSERT_EQ(ERROR_SUCCESS, key.Create(HKEY_LOCAL_MACHINE, foo_key.c_str(),
+        KEY_READ));
+    ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_LOCAL_MACHINE, foo_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));
+  }
+#endif // _WIN64
+}
+
+
 }  // namespace
 
 }  // namespace win
 }  // namespace base