Supporting command line argument to force field trials

base/metrics/field_trial.cc

Index: base/metrics/field_trial.cc
diff --git a/base/metrics/field_trial.cc b/base/metrics/field_trial.cc
index b211326ac37c61e7c8130107a669fdbfc36d2b22..2889016c6e100dfab0b6b68888affb359e2c16f4 100644
--- a/base/metrics/field_trial.cc
+++ b/base/metrics/field_trial.cc
@@ -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.
 
@@ -43,11 +43,11 @@ FieldTrial::FieldTrial(const std::string& name,
     next_group_number_(kDefaultGroupNumber + 1),
     group_(kNotFinalized),
     group_name_hash_(kReservedHashValue),
-    enable_field_trial_(true) {
+    enable_field_trial_(true),
+    forced_(false) {
   DCHECK_GT(total_probability, 0);
   DCHECK(!name_.empty());
   DCHECK(!default_group_name_.empty());
-  FieldTrialList::Register(this);
 
   DCHECK_GT(year, 1970);
   DCHECK_GT(month, 0);
@@ -71,6 +71,9 @@ FieldTrial::FieldTrial(const std::string& name,
 }
 
 void FieldTrial::UseOneTimeRandomization() {
+  // No need to specify randomization when the group choice was forced.
+  if (forced_)
+    return;
   DCHECK_EQ(group_, kNotFinalized);
   DCHECK_EQ(kDefaultGroupNumber + 1, next_group_number_);
   if (!FieldTrialList::IsOneTimeRandomizationEnabled()) {
@@ -89,14 +92,33 @@ void FieldTrial::Disable() {
   // In case we are disabled after initialization, we need to switch
   // the trial to the default group.
   if (group_ != kNotFinalized) {
-    group_ = kDefaultGroupNumber;
-    group_name_ = default_group_name_;
-    group_name_hash_ = HashName(group_name_);
+    // Only reset when not already the default group, because in case we were
+    // forced to the default group, the group number may not be
+    // kDefaultGroupNumber, so we should keep it as is.
+    if (group_name_ != default_group_name_) {
+      group_ = kDefaultGroupNumber;
+      group_name_ = default_group_name_;
+      group_name_hash_ = HashName(group_name_);

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

I'm starting to see this parallel initialization many times in this file (re:
both group_name_ and group_name_hash_).  I suspect you should have a accessor
for group_name_ that also sets the hash, so that you don't mess up and fail to
set it hear and there.

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

Done.

+    }
   }
 }
 
 int FieldTrial::AppendGroup(const std::string& name,
                             Probability group_probability) {
+  // When the group choice was previously forced, we only need to return the
+  // the id of the chosen group, and anything can be returned for the others.
+  if (forced_) {
+    DCHECK(!group_name_.empty());
+    if (name == group_name_) {
+      return group_;
+    } else {

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

nit: else not needed since we returned.

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

Done.

+      DCHECK_NE(next_group_number_, group_);
+      // We still return different numbers each time, in case some caller need
+      // them to be different.
+      return next_group_number_++;
+    }
+  }
+
   DCHECK_LE(group_probability, divisor_);
   DCHECK_GE(group_probability, 0);
 
@@ -122,6 +144,9 @@ int FieldTrial::AppendGroup(const std::string& name,
 int FieldTrial::group() {
   if (group_ == kNotFinalized) {
     accumulated_group_probability_ = divisor_;
+    // Here is't OK to use kDefaultGroupNumber
+    // since we can't be forced and not finalized.
+    DCHECK(!forced_);
     group_ = kDefaultGroupNumber;
     group_name_ = default_group_name_;
     group_name_hash_ = HashName(group_name_);
@@ -234,15 +259,29 @@ FieldTrialList::~FieldTrialList() {
 }
 
 // static
-void FieldTrialList::Register(FieldTrial* trial) {
-  if (!global_) {
-    used_without_global_ = true;
-    return;
+FieldTrial* FieldTrialList::GetFieldTrialInstance(
+    const std::string& name, FieldTrial::Probability total_probability,

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

nit: one parameter per line.

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

Done.

+    const std::string& default_group_name, int* default_group_number,
+    const int year, const int month, const int day_of_month) {
+  if (default_group_number)
+    *default_group_number = FieldTrial::kDefaultGroupNumber;
+  // Check if the field trial has already been created in some other way.
+  FieldTrial* existing_trial = Find(name);
+  if (existing_trial) {
+    DCHECK(existing_trial->forced_);

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

This is tempting to make a CHECK.
If a user tries to force a value too late... they'll never know that their
parameter did nothing :-/.

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

Done.
Though it doesn't change much currently since we only force trials from the
command line in a DEBUG build, but it will protect us in the future...

But this wouldn't catch a forcing done too late, this would happen in
CreateFieldTrial below... Not here... And in that case we return NULL (or false
if called indirectly from CreateTrialsFromString), so it shouldn't go unnoticed.

+    // If the field trial has already been forced, check whether it was forced
+    // to the default group. Return the chosen group number, in that case..
+    if (default_group_number &&
+        default_group_name == existing_trial->default_group_name()) {
+      *default_group_number = existing_trial->group();
+    }
+    return existing_trial;
   }
-  AutoLock auto_lock(global_->lock_);
-  DCHECK(!global_->PreLockedFind(trial->name()));
-  trial->AddRef();
-  global_->registered_[trial->name()] = trial;
+
+  FieldTrial* field_trial = new FieldTrial(
+      name, total_probability, default_group_name, year, month, day_of_month);
+  FieldTrialList::Register(field_trial);
+  return field_trial;
 }
 
 // static
@@ -283,7 +322,7 @@ void FieldTrialList::StatesToString(std::string* output) {
 
   for (RegistrationList::iterator it = global_->registered_.begin();
        it != global_->registered_.end(); ++it) {
-    const std::string name = it->first;
+    const std::string& name = it->first;
     std::string group_name = it->second->group_name_internal();
     if (group_name.empty())
       continue;  // Should not include uninitialized trials.
@@ -313,23 +352,22 @@ void FieldTrialList::GetFieldTrialNameGroupIds(
 }
 
 // static
-bool FieldTrialList::CreateTrialsInChildProcess(
-    const std::string& parent_trials) {
+bool FieldTrialList::CreateTrialsFromString(const std::string& trials_string) {
   DCHECK(global_);
-  if (parent_trials.empty() || !global_)
+  if (trials_string.empty() || !global_)
     return true;
 
   size_t next_item = 0;
-  while (next_item < parent_trials.length()) {
-    size_t name_end = parent_trials.find(kPersistentStringSeparator, next_item);
-    if (name_end == parent_trials.npos || next_item == name_end)
+  while (next_item < trials_string.length()) {
+    size_t name_end = trials_string.find(kPersistentStringSeparator, next_item);
+    if (name_end == trials_string.npos || next_item == name_end)
       return false;
-    size_t group_name_end = parent_trials.find(kPersistentStringSeparator,
+    size_t group_name_end = trials_string.find(kPersistentStringSeparator,
                                                name_end + 1);
-    if (group_name_end == parent_trials.npos || name_end + 1 == group_name_end)
+    if (group_name_end == trials_string.npos || name_end + 1 == group_name_end)
       return false;
-    std::string name(parent_trials, next_item, name_end - next_item);
-    std::string group_name(parent_trials, name_end + 1,
+    std::string name(trials_string, next_item, name_end - next_item);
+    std::string group_name(trials_string, name_end + 1,
                            group_name_end - name_end - 1);
     next_item = group_name_end + 1;
 
@@ -349,9 +387,10 @@ FieldTrial* FieldTrialList::CreateFieldTrial(
   if (name.empty() || group_name.empty() || !global_)
     return NULL;
 
-  FieldTrial *field_trial(FieldTrialList::Find(name));
+  FieldTrial* field_trial = FieldTrialList::Find(name);
   if (field_trial) {
-    // In single process mode, we may have already created the field trial.
+    // In single process mode, or when we force them from the command line,
+    // we may have already created the field trial.
     if (field_trial->group_name_internal() != group_name)
       return NULL;
     return field_trial;
@@ -359,7 +398,11 @@ FieldTrial* FieldTrialList::CreateFieldTrial(
   const int kTotalProbability = 100;
   field_trial = new FieldTrial(name, kTotalProbability, group_name,
                                kExpirationYearInFuture, 1, 1);
+  // This is where we may assign a group number different from
+  // kDefaultGroupNumber to the default group.
   field_trial->AppendGroup(group_name, kTotalProbability);
+  field_trial->forced_ = true;
+  FieldTrialList::Register(field_trial);
   return field_trial;
 }
 
@@ -425,4 +468,16 @@ FieldTrial* FieldTrialList::PreLockedFind(const std::string& name) {
   return it->second;
 }
 
+// static
+void FieldTrialList::Register(FieldTrial* trial) {
+  if (!global_) {
+    used_without_global_ = true;
+    return;
+  }
+  AutoLock auto_lock(global_->lock_);
+  DCHECK(!global_->PreLockedFind(trial->name()));
+  trial->AddRef();
+  global_->registered_[trial->name()] = trial;
+}
+
 }  // namespace base