Refactor SetOmahaExperimentLabel out of gcpai and into install_util.

chrome/installer/util/install_util.h

 // 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 declares utility functions for the installer. The original reason
 // for putting these functions in installer\util library is so that we can
 // separate out the critical logic and write unit tests for it.
 
 #ifndef CHROME_INSTALLER_UTIL_INSTALL_UTIL_H_
 #define CHROME_INSTALLER_UTIL_INSTALL_UTIL_H_
@@ skipping 157 matching lines @@
   static int GetInstallReturnCode(installer::InstallStatus install_status);
 
   // Composes |program| and |arguments| into |command_line|.
   static void MakeUninstallCommand(const string16& program,
                                    const string16& arguments,
                                    CommandLine* command_line);
 
   // Returns a string in the form YYYYMMDD of the current date.
   static string16 GetCurrentDate();
 
+  // Sets |experiment_label| as the Omaha experiment_labels value in the
+  // ClientState key for this Chrome product. If |system_install| is true, this

[grt (UTC plus 2), 2012/11/24 02:58:37]

The HKLM/HKCU part of the comment is a bit superfluous, since that's how
everything in here works.

[SteveT, 2012/11/26 20:45:06]

K, removed.

+  // will write to HKLM, otherwise it will write to HKCU. This will return true
+  // if the label was successfully written, false otherwise (even if the label

[grt (UTC plus 2), 2012/11/24 02:58:37]

I think it's less ambiguous from the standpoint of callers for false to
represent only an error condition rather than either an error or noop. Unless,
of course, the thing a caller would care about is whether or not the registry
was modified. Do you have an idea of what's more important (e.g., being able to
log that a value was written vs. being able to log that there was an error
trying to write a value)?

[SteveT, 2012/11/26 20:45:06]

GCAPI doesn't seem to care at all what the result is - they don't use it at all
(so I don't know the answer for that use case). I suspect that my code is going
to be a best-effort attempt, too.

What's the best practice for cases where you fail to write a registry key? Are
there cases where this retries later on a timer? Or are such errors ever logged?

[grt (UTC plus 2), 2012/11/26 21:19:05]

In the installer, we make heavy use of LOG(foo) and VLOG(1). For regular Chrome,
DLOG(ERROR) is appropriate.  Since this is regular Chrome code, you could DLOG
in the implementation if you fail to set/delete the value. In that case, it'd
nice to stream out the return value from the RegKey operation so that it appears
in the log.

+  // does not need to be written for this particular distribution type).
+  static bool SetOmahaExperimentLabel(const string16& experiment_label,

[grt (UTC plus 2), 2012/11/24 02:58:37]

I think it's more consistent with the surrounding code for system_install to
precede experiment_labels.

[SteveT, 2012/11/26 20:45:06]

Done.

+                                      bool system_install);
+
   // A predicate that compares the program portion of a command line with a
   // given file path.  First, the file paths are compared directly.  If they do
   // not match, the filesystem is consulted to determine if the paths reference
   // the same file.
   class ProgramCompare : public RegistryValuePredicate {
    public:
     explicit ProgramCompare(const FilePath& path_to_match);
     virtual ~ProgramCompare();
     virtual bool Evaluate(const string16& value) const OVERRIDE;
 
@@ skipping 10 matching lines @@
    private:
     DISALLOW_COPY_AND_ASSIGN(ProgramCompare);
   };  // class ProgramCompare
 
  private:
   DISALLOW_COPY_AND_ASSIGN(InstallUtil);
 };
 
 
 #endif  // CHROME_INSTALLER_UTIL_INSTALL_UTIL_H_