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..269f7b46765096955032070ddc3386d22e5df320 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_) {

[Alexei Svitkine (slow), 2012/03/29 19:19:07]

Hmm. If group_name_ == default_group_name_, then we don't know whether it's at
the default value or not. :/

So this effectively makes Disable() not work for forced trials. I am okay with
this as long as its clear to callers. Document this in the header.

[MAD, 2012/04/02 18:31:15]

Filed a bug for this since it's already broken without this change (although
just for the group name). And added a TODO comment in the header file.

+      group_ = kDefaultGroupNumber;
+      group_name_ = default_group_name_;
+      group_name_hash_ = HashName(group_name_);
+    }
   }
 }
 
 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 {
+      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,8 @@ 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.

[Alexei Svitkine (slow), 2012/03/29 19:19:07]

Add a DCHECK(!forced_);

[MAD, 2012/04/02 18:31:15]

Done.

     group_ = kDefaultGroupNumber;
     group_name_ = default_group_name_;
     group_name_hash_ = HashName(group_name_);
@@ -234,15 +258,31 @@ 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,
+    const std::string& default_group_name, int* default_group_number,
+    const int year, const int month, const int day_of_month) {
+  // First 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_);
+    if (default_group_number) {
+      // If the default group was forced, it won't have kDefaultGroupNumber

[Alexei Svitkine (slow), 2012/03/29 19:19:07]

I don't think this comment is clear. How about:

"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."

[MAD, 2012/04/02 18:31:15]

Done.

+      // as a group number, so we must return the chosen group number.

[Alexei Svitkine (slow), 2012/03/29 19:19:07]

Add a test for this logic.

[MAD, 2012/04/02 18:31:15]

I already added one, but it was missing one expectation to check we return the
static value... I added it now...

+      if (default_group_name == existing_trial->default_group_name())
+        *default_group_number = existing_trial->group();
+      else
+        *default_group_number = FieldTrial::kDefaultGroupNumber;
+    }
+    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);
+  if (default_group_number)
+    *default_group_number = FieldTrial::kDefaultGroupNumber;

[Alexei Svitkine (slow), 2012/03/29 19:19:07]

Nit: Move this to the top of the function. Then, in the "existing trial" block,
you can just do:

if (default_group_number &&
    default_group_name == existing_trial->default_group_name()) {
  *default_group_number = existing_trial->group();
}

[MAD, 2012/04/02 18:31:15]

Done.

+  return field_trial;
 }
 
 // static
@@ -283,7 +323,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,24 +353,23 @@ 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,
-                                               name_end + 1);
-    if (group_name_end == parent_trials.npos || name_end + 1 == group_name_end)
+    size_t group_name_end = trials_string.find(kPersistentStringSeparator,
+      name_end + 1);

[Alexei Svitkine (slow), 2012/03/29 19:19:07]

Nit: Align this with kPersistentStringSeparator, so that there's no diff with
the original.

[MAD, 2012/04/02 18:31:15]

Done.

+    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,
-                           group_name_end - 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);

[Alexei Svitkine (slow), 2012/03/29 19:19:07]

Nit: Align this with trials_string, so that there's no diff with the original.

[MAD, 2012/04/02 18:31:15]

Done.

     next_item = group_name_end + 1;
 
     if (!CreateFieldTrial(name, group_name))
@@ -349,9 +388,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 +399,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 +469,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