Remove unsafe registry APIs from base::win::RegKey

base/win/registry.cc

Index: base/win/registry.cc
diff --git a/base/win/registry.cc b/base/win/registry.cc
index 2fe53cf8b8c4308dcdebc4caca5afbb826999eac..671baa9934c3ccc019322c95280a98a340baf8e3 100644
--- a/base/win/registry.cc
+++ b/base/win/registry.cc
@@ -34,6 +34,16 @@ inline DWORD to_wchar_size(DWORD byte_size) {
 // Mask to pull WOW64 access flags out of REGSAM access.
 const REGSAM kWow64AccessMask = KEY_WOW64_32KEY | KEY_WOW64_64KEY;
 
+#if DCHECK_IS_ON()
+inline bool isValidRootKey(HKEY key) {
+  return key == HKEY_CLASSES_ROOT || key == HKEY_CURRENT_USER ||
+         key == HKEY_LOCAL_MACHINE || key == HKEY_USERS ||
+         key == HKEY_PERFORMANCE_DATA || key == HKEY_PERFORMANCE_TEXT ||
+         key == HKEY_PERFORMANCE_NLSTEXT || key == HKEY_CURRENT_CONFIG ||
+         key == HKEY_DYN_DATA || key == HKEY_CURRENT_USER_LOCAL_SETTINGS;
+}
+#endif  // DCHECK_IS_ON()
+
 }  // namespace
 
 // Watches for modifications to a key.
@@ -42,6 +52,9 @@ class RegKey::Watcher : public ObjectWatcher::Delegate {
   Watcher() {}
   ~Watcher() override {}
 
+  Watcher(Watcher&&) = default;
+  Watcher& operator=(Watcher&&) = default;
+
   bool StartWatching(HKEY key, const ChangeCallback& callback);
 
   // Implementation of ObjectWatcher::Delegate.
@@ -91,7 +104,8 @@ bool RegKey::Watcher::StartWatching(HKEY key, const ChangeCallback& callback) {
 RegKey::RegKey() : key_(NULL), wow64access_(0) {
 }
 
-RegKey::RegKey(HKEY key) : key_(key), wow64access_(0) {
+RegKey::RegKey(HKEY rootkey) : key_(rootkey), wow64access_(0) {
+  DCHECK(isValidRootKey(rootkey));
 }
 
 RegKey::RegKey(HKEY rootkey, const wchar_t* subkey, REGSAM access)
@@ -112,7 +126,12 @@ RegKey::~RegKey() {
   Close();
 }
 
+RegKey::RegKey(RegKey&&) = default;
+
+RegKey& RegKey::operator=(RegKey&&) = default;
+
 LONG RegKey::Create(HKEY rootkey, const wchar_t* subkey, REGSAM access) {
+  DCHECK(isValidRootKey(rootkey));
   DWORD disposition_value;
   return CreateWithDisposition(rootkey, subkey, &disposition_value, access);
 }
@@ -120,6 +139,7 @@ LONG RegKey::Create(HKEY rootkey, const wchar_t* subkey, REGSAM access) {
 LONG RegKey::CreateWithDisposition(HKEY rootkey, const wchar_t* subkey,
                                    DWORD* disposition, REGSAM access) {
   DCHECK(rootkey && subkey && access && disposition);
+  DCHECK(isValidRootKey(rootkey));
   HKEY subhkey = NULL;
   LONG result = RegCreateKeyEx(rootkey, subkey, 0, NULL,
                                REG_OPTION_NON_VOLATILE, access, NULL, &subhkey,
@@ -135,6 +155,7 @@ LONG RegKey::CreateWithDisposition(HKEY rootkey, const wchar_t* subkey,
 
 LONG RegKey::CreateKey(const wchar_t* name, REGSAM access) {
   DCHECK(name && access);
+  DCHECK(key_);

[grt (UTC plus 2), 2017/06/29 08:49:47]

i'm not sure about this. when you don't care about error handling, it's nice to
be able to open a key and try to operate on it. if the open fails, so does
everything else. does RegCreateKeyEx return a reasonable result if key_ is 0?

   // 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
@@ -158,6 +179,7 @@ LONG RegKey::CreateKey(const wchar_t* name, REGSAM access) {
 
 LONG RegKey::Open(HKEY rootkey, const wchar_t* subkey, REGSAM access) {
   DCHECK(rootkey && subkey && access);
+  DCHECK(isValidRootKey(rootkey));
   HKEY subhkey = NULL;
 
   LONG result = RegOpenKeyEx(rootkey, subkey, 0, access, &subhkey);
@@ -172,6 +194,7 @@ 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);
+  DCHECK(key_);
   // 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
@@ -202,26 +225,13 @@ void RegKey::Close() {
   }
 }
 
-// TODO(wfh): Remove this and other unsafe methods. See http://crbug.com/375400
-void RegKey::Set(HKEY key) {
-  if (key_ != key) {
-    Close();
-    key_ = key;
-  }
-}
-
-HKEY RegKey::Take() {
-  DCHECK_EQ(wow64access_, 0u);
-  HKEY key = key_;
-  key_ = NULL;
-  return key;
-}
-
 bool RegKey::HasValue(const wchar_t* name) const {
+  DCHECK(key_);
   return RegQueryValueEx(key_, name, 0, NULL, NULL, NULL) == ERROR_SUCCESS;
 }
 
 DWORD RegKey::GetValueCount() const {
+  DCHECK(key_);
   DWORD count = 0;
   LONG result = RegQueryInfoKey(key_, NULL, 0, NULL, NULL, NULL, NULL, &count,
                                 NULL, NULL, NULL, NULL);
@@ -229,6 +239,7 @@ DWORD RegKey::GetValueCount() const {
 }
 
 LONG RegKey::GetValueNameAt(int index, std::wstring* name) const {
+  DCHECK(key_);
   wchar_t buf[256];
   DWORD bufsize = arraysize(buf);
   LONG r = ::RegEnumValue(key_, index, buf, &bufsize, NULL, NULL, NULL, NULL);
@@ -353,6 +364,7 @@ LONG RegKey::ReadValue(const wchar_t* name,
                        void* data,
                        DWORD* dsize,
                        DWORD* dtype) const {
+  DCHECK(key_);
   LONG result = RegQueryValueEx(key_, name, 0, dtype,
                                 reinterpret_cast<LPBYTE>(data), dsize);
   return result;
@@ -404,6 +416,7 @@ LONG RegKey::WriteValue(const wchar_t* name,
                         DWORD dsize,
                         DWORD dtype) {
   DCHECK(data || !dsize);
+  DCHECK(key_);
 
   LONG result = RegSetValueEx(key_, name, 0, dtype,
       reinterpret_cast<LPBYTE>(const_cast<void*>(data)), dsize);
@@ -621,6 +634,7 @@ RegistryKeyIterator::~RegistryKeyIterator() {
 }
 
 DWORD RegistryKeyIterator::SubkeyCount() const {
+  DCHECK(key_);
   DWORD count = 0;
   LONG result = ::RegQueryInfoKey(key_, NULL, 0, NULL, &count, NULL, NULL,
                                   NULL, NULL, NULL, NULL, NULL);