RegistryEntry: Made constructors public.

chrome/installer/util/shell_util.cc

Index: chrome/installer/util/shell_util.cc
diff --git a/chrome/installer/util/shell_util.cc b/chrome/installer/util/shell_util.cc
index af9108282b1f6516357f75f1e69404c3fa07a483..307c705fbb989ce2cece899691cfa352b1ab9c8a 100644
--- a/chrome/installer/util/shell_util.cc
+++ b/chrome/installer/util/shell_util.cc
@@ -154,11 +154,10 @@ bool UserSpecificRegistrySuffix::GetSuffix(base::string16* suffix) {
   return true;
 }
 
-// This class represents a single registry entry. The objective is to
-// encapsulate all the registry entries required for registering Chrome at one
-// place. This class can not be instantiated outside the class and the objects
-// of this class type can be obtained only by calling a static method of this
-// class.
+// 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.

[gab, 2015/11/30 13:04:00]

I'd say remove the two "atomically" words in this comment.

It's not really "atomic", it's simply "tracked" and allows rollbacks which your
comment highlights without the word "atomically".

[Matt Giuca, 2015/12/01 00:46:35]

I think we want to highlight the transactional nature of this. You're right that
"atomically" isn't quite correct so replaced with "transactionally".

(Next CL.)

 class RegistryEntry {
  public:
   // A bit-field enum of places to look for this key in the Windows registry.
@@ -213,6 +212,19 @@ class RegistryEntry {
     base::string16 delegate_clsid;
   };
 
+  // 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

[gab, 2015/11/30 13:04:00]

End comments with '.', here and above.

[gab, 2015/11/30 13:04:00]

s/a object/an object/

here and above.

[Matt Giuca, 2015/12/01 00:46:35]

Done (next CL).

Note: I wouldn't want to do this in the same CL as moving code. The whole reason
we got stuck last time is because I was doing too much in the one CL. Code moves
should just be moves and not make other changes, otherwise it is too hard to
review.

+  RegistryEntry(const base::string16& key_path,
+                const base::string16& name,
+                DWORD value);
+
   // Returns the Windows browser client registration key for Chrome.  For
   // example: "Software\Clients\StartMenuInternet\Chromium[.user]".  Strictly
   // speaking, we should use the name of the executable (e.g., "chrome.exe"),
@@ -375,19 +387,6 @@ class RegistryEntry {
     SAME_VALUE,
   };
 
-  // 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);
-
   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
@@ -406,6 +405,35 @@ class RegistryEntry {
   DISALLOW_COPY_AND_ASSIGN(RegistryEntry);
 };  // class RegistryEntry
 
+RegistryEntry::RegistryEntry(const base::string16& key_path,
+                             const base::string16& value)
+    : key_path_(key_path),
+      name_(),
+      is_string_(true),
+      value_(value),
+      int_value_(0),
+      removal_flag_(RemovalFlag::NONE) {}
+
+RegistryEntry::RegistryEntry(const base::string16& key_path,
+                             const base::string16& name,
+                             const base::string16& value)
+    : key_path_(key_path),
+      name_(name),
+      is_string_(true),
+      value_(value),
+      int_value_(0),
+      removal_flag_(RemovalFlag::NONE) {}
+
+RegistryEntry::RegistryEntry(const base::string16& key_path,
+                             const base::string16& name,
+                             DWORD value)
+    : key_path_(key_path),
+      name_(name),
+      is_string_(false),
+      value_(),
+      int_value_(value),
+      removal_flag_(RemovalFlag::NONE) {}
+
 // static
 base::string16 RegistryEntry::GetBrowserClientKey(
     BrowserDistribution* dist,
@@ -841,35 +869,6 @@ bool RegistryEntry::KeyExistsInRegistry(uint32 look_for_in) const {
   return status != DOES_NOT_EXIST;
 }
 
-RegistryEntry::RegistryEntry(const base::string16& key_path,
-                             const base::string16& value)
-    : key_path_(key_path),
-      name_(),
-      is_string_(true),
-      value_(value),
-      int_value_(0),
-      removal_flag_(RemovalFlag::NONE) {}
-
-RegistryEntry::RegistryEntry(const base::string16& key_path,
-                             const base::string16& name,
-                             const base::string16& value)
-    : key_path_(key_path),
-      name_(name),
-      is_string_(true),
-      value_(value),
-      int_value_(0),
-      removal_flag_(RemovalFlag::NONE) {}
-
-RegistryEntry::RegistryEntry(const base::string16& key_path,
-                             const base::string16& name,
-                             DWORD value)
-    : key_path_(key_path),
-      name_(name),
-      is_string_(false),
-      value_(),
-      int_value_(value),
-      removal_flag_(RemovalFlag::NONE) {}
-
 RegistryEntry::RegistryStatus RegistryEntry::StatusInRegistryUnderRoot(
     HKEY root) const {
   RegKey key(root, key_path_.c_str(), KEY_QUERY_VALUE);