Move shell_util's RegistryEntry class into a separate file.

chrome/installer/util/registry_entry.cc

Index: chrome/installer/util/registry_entry.cc
diff --git a/chrome/installer/util/registry_entry.cc b/chrome/installer/util/registry_entry.cc
new file mode 100644
index 0000000000000000000000000000000000000000..4df95df04fa1a93b97d19a98f3e0165463338511
--- /dev/null
+++ b/chrome/installer/util/registry_entry.cc
@@ -0,0 +1,97 @@
+// Copyright 2014 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 "chrome/installer/util/registry_entry.h"
+
+#include "base/strings/string_util.h"
+#include "base/win/registry.h"
+#include "chrome/installer/util/work_item.h"
+#include "chrome/installer/util/work_item_list.h"
+
+RegistryEntry::RegistryEntry(const base::string16& key_path,
+                             const base::string16& value)
+    : key_path_(key_path),
+      name_(),
+      is_string_(true),

[grt (UTC plus 2), 2014/10/03 14:31:07]

      type_(REG_SZ),
      value_(reinterpret_cast<const uint8_t*>(value.c_str()),
             reinterpret_cast<const uint8_t*>(value.c_str()) +
                 (value.size() + 1) * sizeof(base::string16::value_type)))

[Matt Giuca, 2015/01/20 02:41:17]

Acknowledged.

+      value_(value),
+      int_value_(0) {
+}
+
+RegistryEntry::RegistryEntry(const base::string16& key_path,
+                             const base::string16& name,
+                             const base::string16& value)
+    : key_path_(key_path),
+      name_(name),
+      is_string_(true),

[grt (UTC plus 2), 2014/10/03 14:31:07]

as above

[Matt Giuca, 2015/01/20 02:41:17]

Acknowledged.

+      value_(value),
+      int_value_(0) {
+}
+
+RegistryEntry::RegistryEntry(const base::string16& key_path,
+                             const base::string16& name,
+                             DWORD value)
+    : key_path_(key_path),
+      name_(name),
+      is_string_(false),

[grt (UTC plus 2), 2014/10/03 14:31:07]

      type_(REG_DWORD),
      value_(reinterpret_cast<const uint8_t*>(&value),
             reinterpret_cast<const uint8_t*>(&value) + sizeof(value))

[Matt Giuca, 2015/01/20 02:41:17]

Acknowledged.

+      value_(),
+      int_value_(value) {
+}
+
+void RegistryEntry::AddToWorkItemList(HKEY root, WorkItemList* items) const {
+  items->AddCreateRegKeyWorkItem(root, key_path_, WorkItem::kWow64Default);

[Will Harris, 2014/10/03 05:13:45]

Will this class ever be used to access 64-bit or 32-bit hives - using
KEY_WOW64_64KEY or KEY_WOW64_32KEY?  If so, this should be a parameter to the
constructor?

[Matt Giuca, 2014/10/03 10:09:39]

No idea --- note that I'm just moving this code here (not writing it) and I
don't think it's a good idea to modify said code in the same CL.

If you think it should be altered or made more general in some way, please
submit a follow-up or precursor patch. (If you don't want to wait for this CL,
just make a patch against the existing RegistryEntry class in shell_util.cc and
I will merge your change in.)

[grt (UTC plus 2), 2015/01/20 20:05:31]

This CL takes a private class and makes it public. This may require small
changes to make it appropriate for general consumption. For RegistryEntry to be
generally useful, it must take a "REGSAM wow64_access" param in its ctors that
it passes down to the work items in place of WorkItem::kWow64Default and that it
uses in StatusInRegistryUnderRoot to probe the registry. kWow64Default was
correct for all uses in shell_util.cc, but it isn't necessarily correct for all
potential consumers of RegistryEntry once it's publicly available. The point of
that extra parameter is to force consumers to make a conscious choice. Leaving
it for later opens up the possibility of RegistryEntry consumers doing the wrong
thing because they thought the API was somehow safe and protected them from the
reality of the registry.

[Matt Giuca, 2015/01/20 23:16:35]

OK, if you really feel strongly about this, then I will add this parameter in a
precursor CL. I feel equally strongly about not changing the interface in the
same CL as moving it.

[grt (UTC plus 2), 2015/01/20 23:30:55]

I think it's a waste of both of our time to do it in a separate CL. The
parameter wasn't needed when it was a private class since no callers needed it.
By making the class public, it really needs to be correct for any caller who
tries to use it.

If it helps you sleep at night, feel free to tell yourself that an OWNER asked
you to defy your own sense of what is right. :-)

+  if (is_string_) {

[grt (UTC plus 2), 2014/10/03 14:31:07]

  items->AddSetRegValueWorkItem(root_, key_path_, WorkItem::kWow64Default,
name_, type_, value_, true);

looks like this method doesn't exist in the WorkItem family. adding it is a
bunch of copy-n-paste.

[Matt Giuca, 2015/01/20 02:41:17]

Again, should not be changed here.

+    items->AddSetRegValueWorkItem(
+        root, key_path_, WorkItem::kWow64Default, name_, value_, true);
+  } else {
+    items->AddSetRegValueWorkItem(
+        root, key_path_, WorkItem::kWow64Default, name_, int_value_, true);
+  }
+}
+
+bool RegistryEntry::ExistsInRegistry(uint32 look_for_in) const {
+  DCHECK(look_for_in);
+
+  RegistryStatus status = DOES_NOT_EXIST;
+  if (look_for_in & LOOK_IN_HKCU)
+    status = StatusInRegistryUnderRoot(HKEY_CURRENT_USER);
+  if (status == DOES_NOT_EXIST && (look_for_in & LOOK_IN_HKLM))
+    status = StatusInRegistryUnderRoot(HKEY_LOCAL_MACHINE);
+  return status == SAME_VALUE;
+}
+
+bool RegistryEntry::KeyExistsInRegistry(uint32 look_for_in) const {
+  DCHECK(look_for_in);
+
+  RegistryStatus status = DOES_NOT_EXIST;
+  if (look_for_in & LOOK_IN_HKCU)
+    status = StatusInRegistryUnderRoot(HKEY_CURRENT_USER);
+  if (status == DOES_NOT_EXIST && (look_for_in & LOOK_IN_HKLM))
+    status = StatusInRegistryUnderRoot(HKEY_LOCAL_MACHINE);
+  return status != DOES_NOT_EXIST;
+}
+
+RegistryEntry::RegistryStatus RegistryEntry::StatusInRegistryUnderRoot(
+    HKEY root) const {
+  base::win::RegKey key(root, key_path_.c_str(), KEY_QUERY_VALUE);
+  bool found = false;
+  bool correct_value = false;
+  if (is_string_) {

[grt (UTC plus 2), 2014/10/03 14:31:07]

since this has string-specific handling, you could do something like:
  if (type_ == REG_SZ) {
    ...same as before...
  } else {
    std::vector<uint8_t> buffer(value_.size());
    DWORD data_size = buffer.size();
    DWORD data_type = REG_NONE;
    LONG result = key.ReadValue(name_.c_str(), &buffer[0], &data_size,
                                &data_type);
    found = (result == ERROR_SUCCESS || result == ERROR_MORE_DATA);
    if (found && data_size == value_.size() && data_type == type_)
      correct_value = buffer == value_;
  }

[Matt Giuca, 2015/01/20 02:41:17]

Again, not here.

+    base::string16 read_value;
+    found = key.ReadValue(name_.c_str(), &read_value) == ERROR_SUCCESS;
+    if (found) {
+      correct_value = read_value.size() == value_.size() &&
+                      std::equal(value_.begin(),
+                                 value_.end(),
+                                 read_value.begin(),
+                                 base::CaseInsensitiveCompare<wchar_t>());
+    }
+  } else {
+    DWORD read_value;
+    found = key.ReadValueDW(name_.c_str(), &read_value) == ERROR_SUCCESS;
+    if (found)
+      correct_value = read_value == int_value_;
+  }
+  return found ? (correct_value ? SAME_VALUE : DIFFERENT_VALUE)
+               : DOES_NOT_EXIST;
+}