Refactor SetOmahaExperimentLabel out of gcpai and into install_util.

chrome/installer/util/browser_distribution.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 a class that contains various method related to branding.
 
 #ifndef CHROME_INSTALLER_UTIL_BROWSER_DISTRIBUTION_H_
 #define CHROME_INSTALLER_UTIL_BROWSER_DISTRIBUTION_H_
 
 #include <string>
@@ skipping 146 matching lines @@
                                     const installer::Product& product,
                                     bool system_level);
 
   // The user has qualified for the inactive user toast experiment and this
   // function just performs it.
   virtual void InactiveUserToastExperiment(int flavor,
       const string16& experiment_group,
       const installer::Product& installation,
       const FilePath& application_path);
 
+  // Returns true if this distribution should write to the Omaha
+  // experiment_labels registry value.
+  virtual bool ShouldWriteExperimentLabels() const;

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

I admire (and share) your desire to make this const. In the interest of staying
close to the style of the existing methods, I think it's best to make it
non-const. Otherwise readers may spend time trying to figure out why this one is
special.

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

While writing the experiment labels is the fine-grained operation that this bit
controls, I wonder if there's a broader name that is even more applicable. Is it
really only the writing of the value that's special to Google Chrome, or is
there anything else like the way field trials are set up, the way they're named,
etc? Is it possible that other code in Chrome might query this bit to decide how
to set up, participate in, or report on field trials?

If you stick with this name, I suggest "ShouldSetExperimentLabels" for
consistency with the label-setting function.

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

I _was_ a bit confused that similar methods in this class (and subclasses) were
not const. But better readability/understandability sounds good to me.

However, I also think that future readers might come across the same initial
confusion that I did :\

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

I thought about your original suggestion "HasExperiments", and realized that
it's a bit inaccurate, in that almost all Chrome binaries _have_
experiments/field trials, but not all of them should be setting these values in
the registry.

For example, I chose, in this latest patch set, to exclude Chrome Frame as
distributions that writes experiment labels, because we currently do not care to
do that for CF.

I've renamed this to ShouldSetExperimentLabels. Thanks for pointing that out.

+
  protected:
   explicit BrowserDistribution(Type type);
 
   template<class DistributionClass>
   static BrowserDistribution* GetOrCreateBrowserDistribution(
       BrowserDistribution** dist);
 
   const Type type_;
 
  private:
   BrowserDistribution();
 
   DISALLOW_COPY_AND_ASSIGN(BrowserDistribution);
 };
 
 #endif  // CHROME_INSTALLER_UTIL_BROWSER_DISTRIBUTION_H_