Supporting command line argument to force field trials

base/metrics/field_trial.h

Index: base/metrics/field_trial.h
diff --git a/base/metrics/field_trial.h b/base/metrics/field_trial.h
index 50508d241425e558f4f4d737cf352947a86f0bef..59aa8a8fbd26ae29b88aaa6ca6acbebb585c6a6c 100644
--- a/base/metrics/field_trial.h
+++ b/base/metrics/field_trial.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -103,26 +103,6 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> {
   // assignment (and hence is not yet participating in the trial).
   static const int kNotFinalized;
 
-  // This is the group number of the 'default' group. This provides an easy way
-  // to assign all the remaining probability to a group ('default').
-  static const int kDefaultGroupNumber;
-
-  // The name is used to register the instance with the FieldTrialList class,
-  // and can be used to find the trial (only one trial can be present for each
-  // name).  |name| and |default_group_name| may not be empty.
-  //
-  // Group probabilities that are later supplied must sum to less than or equal
-  // to the total_probability. Arguments year, month and day_of_month specify
-  // the expiration time. If the build time is after the expiration time then
-  // the field trial reverts to the 'default' group.
-  //
-  // Using this constructor creates a startup-randomized FieldTrial. If you
-  // want a one-time randomized trial, call UseOneTimeRandomization() right
-  // after construction.
-  FieldTrial(const std::string& name, Probability total_probability,
-             const std::string& default_group_name, const int year,
-             const int month, const int day_of_month);
-
   // Changes the field trial to use one-time randomization, i.e. produce the
   // same result for the current trial on every run of this client. Must be
   // called right after construction.
@@ -133,6 +113,8 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> {
   // at any time after initialization (should not be interleaved with
   // AppendGroup calls). Once disabled, there is no way to re-enable a
   // trial.
+  // TODO(mad): http://code.google.com/p/chromium/issues/detail?id=121446
+  // This doesn't properly reset to Default when a group was forced.
   void Disable();
 
   // Establish the name and probability of the next group in this trial.
@@ -145,24 +127,20 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> {
   std::string name() const { return name_; }
 
   // Return the randomly selected group number that was assigned.
-  // Return kDefaultGroupNumber if the instance is in the 'default' group.
   // Note that this will force an instance to participate, and make it illegal
   // to attempt to probabilistically add any other groups to the trial.
   int group();
 
-  // If the group's name is empty, a string version containing the group
-  // number is used as the group name.
+  // If the group's name is empty, a string version containing the group number
+  // is used as the group name. This causes a winner to be chosen if none was.
   std::string group_name();
 
   // Gets the unique identifier of the Field Trial, but only if a group was
   // officially chosen, otherwise name_group_id is left untouched and false
-  // is returned. When true is returned, the name and group ids were successfuly
-  // set in name_group_id.
+  // is returned. When true is returned, the name and group ids were
+  // successfully set in name_group_id.
   bool GetNameGroupId(NameGroupId* name_group_id);
 
-  // Return the default group name of the FieldTrial.
-  std::string default_group_name() const { return default_group_name_; }
-
   // Helper function for the most common use: as an argument to specify the
   // name of a HISTOGRAM.  Use the original histogram name as the name_prefix.
   static std::string MakeName(const std::string& name_prefix,
@@ -193,8 +171,19 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> {
 
   friend class RefCounted<FieldTrial>;
 
+  // This is the group number of the 'default' group when a choice wasn't forced
+  // by a call to FieldTrialList::CreateFieldTrial. It is kept private so that
+  // consumers don't use it by mistake in cases where the group was forced.
+  static const int kDefaultGroupNumber;
+
+  FieldTrial(const std::string& name, Probability total_probability,
+             const std::string& default_group_name, const int year,
+             const int month, const int day_of_month);
   virtual ~FieldTrial();
 
+  // Return the default group name of the FieldTrial.
+  std::string default_group_name() const { return default_group_name_; }
+
   // Returns the group_name. A winner need not have been chosen.
   std::string group_name_internal() const { return group_name_; }
 
@@ -247,6 +236,10 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> {
   // group.
   bool enable_field_trial_;
 
+  // When forced_ is true, we return the chosen group from AppendGroup when
+  // appropriate.
+  bool forced_;
+
   // When benchmarking is enabled, field trials all revert to the 'default'
   // group.
   static bool enable_benchmarking_;
@@ -263,7 +256,7 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> {
 // Only one instance of this class exists.
 class BASE_EXPORT FieldTrialList {
  public:
-  // Define a separator charactor to use when creating a persistent form of an
+  // Define a separator character to use when creating a persistent form of an
   // instance.  This is intended for use as a command line argument, passed to a
   // second process to mimic our state (i.e., provide the same group name).
   static const char kPersistentStringSeparator;  // Currently a slash.
@@ -292,9 +285,30 @@ class BASE_EXPORT FieldTrialList {
   // Destructor Release()'s references to all registered FieldTrial instances.
   ~FieldTrialList();
 
-  // Register() stores a pointer to the given trial in a global map.
-  // This method also AddRef's the indicated trial.
-  static void Register(FieldTrial* trial);
+  // Creates a new FieldTrial instance if none exists already. If one exists,
+  // it has to be because it was forced by CreateFieldTrial.

[jar (doing other things), 2012/04/06 01:41:29]

nit: I had to read the sentence a few times, and stumbled over "...it has to be
because it was..."

IMO, the subtle distinction between "finding an existing instance" and "creates"
should not be surfaced here, as that is an internal element of the
implementation.  For instance, a list of "forced" group-results could be stored
separately, and then the construction (which might take place now!) could have
adjusted to their planned/forced state.

IMO, this CL just changes from using a constructor, to using a Factory, and the
factory has more liberties about construction and reuse... but that is hidden
inside the implementation.  

I'd suggest you use that metaphor, and you can leave your implementation hidden
more cleanly.  I'd suggest this function should probably be called something
more like:
FactoryGetFieldTrial()

IF you don't like that picture, and want to stay with the current commentary
(exposing the hidden fact that the instance may already be created) I'd suggest
changing your wording to something like:
Generally, this function should only be called once by a caller.  If the field
trial exists prior to this call, then it was probably created by a call to
CreateFieldTrial, which forced a specific group result.  Such pre-existence
should be transparent to the caller.

[MAD, 2012/04/11 02:38:08]

OK, I agree, I like this picture... Thanks!
Done.

+  //
+  // |name| is used to register the instance with the FieldTrialList class,
+  // and can be used to find the trial (only one trial can be present for each
+  // name). |default_group_name| is the name of the default group which will
+  // be chosen if none of the subsequent appended groups get to be chosen.
+  // |default_group_number| can receive the group number of the default group as
+  // AppendGroup returns the number of the subsequence groups. |name| and
+  // |default_group_name| may not be empty but |default_group_number| can be
+  // NULL if the value is not needed.
+  //
+  // Group probabilities that are later supplied must sum to less than or equal
+  // to the |total_probability|. Arguments |year|, |month| and |day_of_month|
+  // specify the expiration time. If the build time is after the expiration time
+  // then the field trial reverts to the 'default' group.
+  //
+  // Use this static method to get a startup-randomized FieldTrial or a
+  // previously created forced FieldTrial. If you want a one-time randomized
+  // trial, call UseOneTimeRandomization() right after creation.
+  static FieldTrial* GetFieldTrialInstance(
+      const std::string& name, FieldTrial::Probability total_probability,

[jar (doing other things), 2012/04/06 01:41:29]

nit: I think it is more readable with one parameter per line once you had to
wrap, and that might follow from the style rule: "All parameters should be
aligned if possible" 

[MAD, 2012/04/11 02:38:08]

Done.

+      const std::string& default_group_name, int* default_group_number,

[jar (doing other things), 2012/04/06 01:41:29]

Since the return group number is a "return argument," it should be placed at the
end of the arg list, after all the "input" arguments.

[MAD, 2012/04/11 02:38:08]

Done.

+      const int year, const int month, const int day_of_month);
 
   // The Find() method can be used to test to see if a named Trial was already
   // registered, or to retrieve a pointer to it from the global map.
@@ -311,11 +325,12 @@ class BASE_EXPORT FieldTrialList {
   // Returns true if the named trial has been registered.
   static bool TrialExists(const std::string& name);
 
-  // Create a persistent representation of all FieldTrial instances and the
-  // |client_id()| state for resurrection in another process.  This allows
-  // randomization to be done in one process, and secondary processes can be
-  // synchronized on the result. The resulting string contains the
-  // |client_id()|, the names, the trial name, and a "/" separator.
+  // Creates a persistent representation of all FieldTrial instances for
+  // resurrection in another process. This allows randomization to be done in
+  // one process, and secondary processes can be synchronized on the result.
+  // The resulting string contains the name and group name pairs for all trials,
+  // with "/" used to separate all names and to terminate the string. This
+  // string is parsed by CreateTrialsFromString().
   static void StatesToString(std::string* output);
 
   // Returns an array of Unique IDs for each Field Trial that has a chosen
@@ -324,24 +339,20 @@ class BASE_EXPORT FieldTrialList {
   static void GetFieldTrialNameGroupIds(
       std::vector<FieldTrial::NameGroupId>* name_group_ids);
 
-  // Use a previously generated state string (re: StatesToString()) augment the
-  // current list of field tests to include the supplied tests, and using a 100%
-  // probability for each test, force them to have the same group string.  This
-  // is commonly used in a non-browser process, to carry randomly selected state
-  // in a browser process into this non-browser process. This method calls
-  // CreateFieldTrial to create the FieldTrial in the non-browser process.
-  // Currently only the group_name_ and name_ are restored, as well as the
-  // |client_id()| state, that could be used for one-time randomized trials
-  // set up only in child processes.
-  static bool CreateTrialsInChildProcess(const std::string& prior_trials);
+  // Use a state string (re: StatesToString()) to augment the current list of
+  // field tests to include the supplied tests, and using a 100% probability for
+  // each test, force them to have the same group string.  This is commonly used
+  // in a non-browser process, to carry randomly selected state in a browser
+  // process into this non-browser process, but could also be invoked through a
+  // command line argument to the browser process.
+  static bool CreateTrialsFromString(const std::string& prior_trials);
 
   // Create a FieldTrial with the given |name| and using 100% probability for
   // the FieldTrial, force FieldTrial to have the same group string as
   // |group_name|. This is commonly used in a non-browser process, to carry
   // randomly selected state in a browser process into this non-browser process.
-  // Currently only the group_name_ and name_ are set in the FieldTrial. It
-  // returns NULL if there is a FieldTrial that is already registered with the
-  // same |name| but has different finalized group string (|group_name|).
+  // It returns NULL if there is a FieldTrial that is already registered with
+  // the same |name| but has different finalized group string (|group_name|).
   static FieldTrial* CreateFieldTrial(const std::string& name,
                                       const std::string& group_name);
 
@@ -390,6 +401,11 @@ class BASE_EXPORT FieldTrialList {
   // Helper function should be called only while holding lock_.
   FieldTrial* PreLockedFind(const std::string& name);
 
+  // Register() stores a pointer to the given trial in a global map.
+  // This method also AddRef's the indicated trial.
+  // This should always be called after creating a new FieldTrial instance.
+  static void Register(FieldTrial* trial);
+
   static FieldTrialList* global_;  // The singleton of this class.
 
   // This will tell us if there is an attempt to register a field