Move shell_util's RegistryEntry class into a separate file.

chrome/installer/util/registry_entry.h

+// 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.
+
+#ifndef CHROME_INSTALLER_UTIL_REGISTRY_ENTRY_H_
+#define CHROME_INSTALLER_UTIL_REGISTRY_ENTRY_H_
+
+#include <windows.h>
+
+#include "base/basictypes.h"
+#include "base/macros.h"
+#include "base/strings/string16.h"
+
+class WorkItemList;
+
+// This class represents a single registry entry (a key and its value). A
+// collection of registry entries should be collected into a list and written
+// atomically using a WorkItemList. This is preferred to writing to the registry
+// directly, because if anything goes wrong, they can be rolled back atomically.
+class RegistryEntry {
+ public:
+  // A bit-field enum of places to look for this key in the Windows registry.
+  enum LookForIn {
+    LOOK_IN_HKCU = 1 << 0,
+    LOOK_IN_HKLM = 1 << 1,
+    LOOK_IN_HKCU_THEN_HKLM = LOOK_IN_HKCU | LOOK_IN_HKLM,
+  };
+
+  // Create a object that represent default value of a key
+  RegistryEntry(const base::string16& key_path, const base::string16& value);
+
+  // Create a object that represent a key of type REG_SZ
+  RegistryEntry(const base::string16& key_path,
+                const base::string16& name,
+                const base::string16& value);
+
+  // Create a object that represent a key of integer type
+  RegistryEntry(const base::string16& key_path,
+                const base::string16& name,
+                DWORD value);
+
+  // Generate work_item tasks required to create current registry entry and
+  // add them to the given work item list.
+  void AddToWorkItemList(HKEY root, WorkItemList* items) const;
+
+  // Checks if the current registry entry exists in HKCU\|key_path_|\|name_|
+  // and value is |value_|. If the key does NOT exist in HKCU, checks for
+  // the correct name and value in HKLM.
+  // |look_for_in| specifies roots (HKCU and/or HKLM) in which to look for the
+  // key, unspecified roots are not looked into (i.e. the the key is assumed not
+  // to exist in them).
+  // |look_for_in| must at least specify one root to look into.
+  // If |look_for_in| is LOOK_IN_HKCU_THEN_HKLM, this method mimics Windows'
+  // behavior when searching in HKCR (HKCU takes precedence over HKLM). For
+  // registrations outside of HKCR on versions of Windows prior to Win8,
+  // Chrome's values go in HKLM. This function will make unnecessary (but
+  // harmless) queries into HKCU in that case.
+  bool ExistsInRegistry(uint32 look_for_in) const;
+
+  // Checks if the current registry entry exists in \|key_path_|\|name_|,
+  // regardless of value. Same lookup rules as ExistsInRegistry.
+  // Unlike ExistsInRegistry, this returns true if some other value is present
+  // with the same key.
+  bool KeyExistsInRegistry(uint32 look_for_in) const;
+
+ private:
+  // States this RegistryKey can be in compared to the registry.
+  enum RegistryStatus {
+    // |name_| does not exist in the registry
+    DOES_NOT_EXIST,
+    // |name_| exists, but its value != |value_|
+    DIFFERENT_VALUE,
+    // |name_| exists and its value is |value_|
+    SAME_VALUE,
+  };
+
+  base::string16 key_path_;  // key path for the registry entry
+  base::string16 name_;      // name of the registry entry
+  bool is_string_;           // true if current registry entry is of type REG_SZ
+  base::string16 value_;     // string value (useful if is_string_ = true)

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

this class would be more flexible if is_string_, value_, and int_value_ were
replaced with:
  DWORD type_;
  std::vector<uint8_t> value_;
see my comments in the implementation for how these would be used.

i would also give the class a single public ctor along these lines:
  RegistryEntry(const base::string16& key_path,
                const base::string16& name,
                DWORD type,
                const uint8_t* data,
                size_t data_size);
and then a set of builders like:
  static scoped_ptr<RegistryEntry> CreateString(const base::string16& key_path,
                                                const base::string16& name,
                                                const base::string16& value);
that simply new up an instance, calling the ctor as appropriate,

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

OK, I totally agree that this is better, but it is not appropriate in this CL.
It is never a good idea to mix semantic changes with large amounts of code being
moved around. I will take this into consideration after landing the big one (or
you can make that change if you like).

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

Not to nitpick, but I proposed an implementation change rather than a semantic
change. That aside, tests should prove that such changes are safe. It'd be nice
to have at least rudimentary unit test for this class now that it's public.
Would you add one?

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

semantics != interface. I meant a semantic change as opposed to a syntactic
change (i.e., anything which changes the meaning of the program, as opposed to
the formatting and arrangement).
OK I will add a test case.

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

I don't think my proposal changed the meaning or behavior of the class from the
caller's point of view. It change the implementation. Callers wouldn't care.
This is why regression tests are awesome. Such changes are easy to make and
validate.
Thanks!

+  DWORD int_value_;          // integer value (useful if is_string_ = false)
+
+  // Helper function for ExistsInRegistry().
+  // Returns the RegistryStatus of the current registry entry in
+  // |root|\|key_path_|\|name_|.
+  RegistryStatus StatusInRegistryUnderRoot(HKEY root) const;
+
+  DISALLOW_COPY_AND_ASSIGN(RegistryEntry);
+};
+
+#endif  // CHROME_INSTALLER_UTIL_REGISTRY_ENTRY_H_