RegistryEntry: Made constructors public.

chrome/installer/util/shell_util.cc

 // Copyright (c) 2012 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.
 //
 // This file defines functions that integrate Chrome in Windows shell. These
 // functions can be used by Chrome as well as Chrome installer. All of the
 // work is done by the local functions defined in anonymous namespace in
 // this class.
 
 #include "chrome/installer/util/shell_util.h"
@@ skipping 136 matching lines @@
 
 bool UserSpecificRegistrySuffix::GetSuffix(base::string16* suffix) {
   if (suffix_.empty()) {
     NOTREACHED();
     return false;
   }
   suffix->assign(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.
   enum LookForIn {
     LOOK_IN_HKCU = 1 << 0,
     LOOK_IN_HKLM = 1 << 1,
     LOOK_IN_HKCU_THEN_HKLM = LOOK_IN_HKCU | LOOK_IN_HKLM,
   };
 
   // Identifies the type of removal this RegistryEntry is flagged for, if any.
@@ skipping 34 matching lines @@
     base::string16 application_name;
     base::FilePath application_icon_path;
     int application_icon_index;
     base::string16 application_description;
     base::string16 publisher_name;
 
     // The CLSID for the application's DelegateExecute handler. May be empty.
     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"),
   // but that ship has sailed.  The cost of switching now is re-prompting users
   // to make Chrome their default browser, which isn't polite.  |suffix| is the
   // user-specific registration suffix; see GetUserSpecificDefaultBrowserSuffix
   // in shell_util.h for details.
   static base::string16 GetBrowserClientKey(BrowserDistribution* dist,
                                             const base::string16& suffix);
 
@@ skipping 142 matching lines @@
   // 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,
   };
 
-  // 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
   base::string16 value_;     // string value (useful if is_string_ = true)
   DWORD int_value_;          // integer value (useful if is_string_ = false)
 
   // Identifies whether this RegistryEntry is flagged for removal (i.e. no
   // longer relevant on the configuration it was created under).
   RemovalFlag removal_flag_;
 
   // 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);
 };  // 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,
     const base::string16& suffix) {
   DCHECK(suffix.empty() || suffix[0] == L'.');
   return base::string16(ShellUtil::kRegStartMenuInternet)
       .append(1, L'\\')
       .append(dist->GetBaseAppName())
       .append(suffix);
 }
@@ skipping 415 matching lines @@
   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::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);
   bool found = false;
   bool correct_value = false;
   if (is_string_) {
     base::string16 read_value;
     found = key.ReadValue(name_.c_str(), &read_value) == ERROR_SUCCESS;
     if (found) {
       correct_value =
@@ skipping 1717 matching lines @@
   base::string16 key_path(ShellUtil::kRegClasses);
   key_path.push_back(base::FilePath::kSeparators[0]);
   key_path.append(prog_id);
   return InstallUtil::DeleteRegistryKey(
       HKEY_CURRENT_USER, key_path, WorkItem::kWow64Default);
 
   // TODO(mgiuca): Remove the extension association entries. This requires that
   // the extensions associated with a particular prog_id are stored in that
   // prog_id's key.
 }