Use a different algorithm with the low entropy source for field trials.

base/metrics/field_trial.cc

 // 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.
 
 #include "base/metrics/field_trial.h"
 
 #include "base/build_time.h"
 #include "base/logging.h"
 #include "base/rand_util.h"
 #include "base/sha1.h"
@@ skipping 35 matching lines @@
 const int FieldTrial::kNotFinalized = -1;
 const int FieldTrial::kDefaultGroupNumber = 0;
 bool FieldTrial::enable_benchmarking_ = false;
 
 const char FieldTrialList::kPersistentStringSeparator('/');
 int FieldTrialList::kExpirationYearInFuture = 0;
 
 //------------------------------------------------------------------------------
 // FieldTrial methods and members.
 
-FieldTrial::FieldTrial(const std::string& name,
+FieldTrial::FieldTrial(const EntropyProvider* entropy_provider,
+                       const std::string& name,
                        const Probability total_probability,
                        const std::string& default_group_name)
-  : name_(name),
+  : entropy_provider_(entropy_provider),

[jar (doing other things), 2012/08/17 19:06:19]

nit: Please indent the ":" by four spaces.

[Alexei Svitkine (slow), 2012/08/20 15:57:40]

Done.

+    name_(name),
     divisor_(total_probability),
     default_group_name_(default_group_name),
     random_(static_cast<Probability>(divisor_ * RandDouble())),
     accumulated_group_probability_(0),
     next_group_number_(kDefaultGroupNumber + 1),
     group_(kNotFinalized),
     enable_field_trial_(true),
     forced_(false) {
   DCHECK_GT(total_probability, 0);
   DCHECK(!name_.empty());
   DCHECK(!default_group_name_.empty());
 }
 
+FieldTrial::EntropyProvider::~EntropyProvider() {
+}
+
 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()) {

[jar (doing other things), 2012/08/17 19:06:19]

This function, could, for example, return an entropy_provider, and then there
would be no reason to maintain a slot in the FieldTrial instance.

[Alexei Svitkine (slow), 2012/08/20 15:57:40]

Done.

By having the entropy provider passed in explicitly to the FieldTrial
constructor, the idea was to reduce coupling between FieldTrial and
FieldTrialList (by explicitly passing in dependencies). But it seems that the
two are already deeply coupled so it seems like a lost cause.

In particular, you're right to point out that this method already uses a static
function of FieldTrialList anyway, so I did as you suggested and made that
function return the EntropyProvider, removing the need for FieldTrial to hold
on to one.

     NOTREACHED();
     Disable();
     return;
   }
 
   random_ = static_cast<Probability>(
-      divisor_ * HashClientId(FieldTrialList::client_id(), name_));
+      divisor_ * entropy_provider_->GetEntropyForTrial(name_));
 }
 
 void FieldTrial::Disable() {
   enable_field_trial_ = false;
 
   // In case we are disabled after initialization, we need to switch
   // the trial to the default group.
   if (group_ != kNotFinalized) {
     // Only reset when not already the default group, because in case we were
     // forced to the default group, the group number may not be
@@ skipping 94 matching lines @@
 
 void FieldTrial::SetGroupChoice(const std::string& name, int number) {
   group_ = number;
   if (name.empty())
     StringAppendF(&group_name_, "%d", group_);
   else
     group_name_ = name;
   DVLOG(1) << "Field trial: " << name_ << " Group choice:" << group_name_;
 }
 
-// static
-double FieldTrial::HashClientId(const std::string& client_id,
-                                const std::string& trial_name) {
-  // SHA-1 is designed to produce a uniformly random spread in its output space,
-  // even for nearly-identical inputs, so it helps massage whatever client_id
-  // and trial_name we get into something with a uniform distribution, which
-  // is desirable so that we don't skew any part of the 0-100% spectrum.
-  std::string input(client_id + trial_name);
-  unsigned char sha1_hash[kSHA1Length];
-  SHA1HashBytes(reinterpret_cast<const unsigned char*>(input.c_str()),
-                input.size(),
-                sha1_hash);
-
-  COMPILE_ASSERT(sizeof(uint64) < sizeof(sha1_hash), need_more_data);
-  uint64 bits;
-  memcpy(&bits, sha1_hash, sizeof(bits));
-  bits = base::ByteSwapToLE64(bits);
-
-  return BitsToOpenEndedUnitInterval(bits);
-}
-
 //------------------------------------------------------------------------------
 // FieldTrialList methods and members.
 
 // static
 FieldTrialList* FieldTrialList::global_ = NULL;
 
 // static
 bool FieldTrialList::used_without_global_ = false;
 
-FieldTrialList::FieldTrialList(const std::string& client_id)
+FieldTrialList::Observer::~Observer() {
+}
+
+FieldTrialList::FieldTrialList(FieldTrial::EntropyProvider* entropy_provider)
     : application_start_time_(TimeTicks::Now()),
-      client_id_(client_id),
+      entropy_provider_(entropy_provider),
       observer_list_(new ObserverListThreadSafe<FieldTrialList::Observer>(
           ObserverListBase<FieldTrialList::Observer>::NOTIFY_EXISTING_ONLY)) {
   DCHECK(!global_);
   DCHECK(!used_without_global_);
   global_ = this;
 
   Time::Exploded exploded;
   Time two_years_from_now =
       Time::NowFromSystemTime() + TimeDelta::FromDays(730);
   two_years_from_now.LocalExplode(&exploded);
@@ skipping 28 matching lines @@
     CHECK(existing_trial->forced_);
     // 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;
   }
 
-  FieldTrial* field_trial =
-      new FieldTrial(name, total_probability, default_group_name);
+  FieldTrial* field_trial = new FieldTrial(global_->entropy_provider_.get(),
+                                           name, total_probability,
+                                           default_group_name);
   if (GetBuildTime() > CreateTimeFromParams(year, month, day_of_month))
     field_trial->Disable();
   FieldTrialList::Register(field_trial);
   return field_trial;
 }
 
 // static
 FieldTrial* FieldTrialList::Find(const std::string& name) {
   if (!global_)
     return NULL;
@@ skipping 98 matching lines @@
 
   FieldTrial* field_trial = FieldTrialList::Find(name);
   if (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;
   }
   const int kTotalProbability = 100;
-  field_trial = new FieldTrial(name, kTotalProbability, group_name);
+  field_trial = new FieldTrial(global_->entropy_provider_.get(),
+                               name, kTotalProbability, group_name);
   // 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;
 }
 
 // static
 void FieldTrialList::AddObserver(Observer* observer) {
@@ skipping 26 matching lines @@
 
 // static
 size_t FieldTrialList::GetFieldTrialCount() {
   if (!global_)
     return 0;
   AutoLock auto_lock(global_->lock_);
   return global_->registered_.size();
 }
 
 // static
 bool FieldTrialList::IsOneTimeRandomizationEnabled() {

[jar (doing other things), 2012/08/17 19:06:19]

You might change this to:
GetEntropyProviderForOneTimeRandomization()

...or... if you don't want to bother passing around the functoid... you could
pass in the string, and get back the entropy (with a bool if there was no
provider):
bool GetOneTimeEntropy(const std::string name, double* entropy);

[Alexei Svitkine (slow), 2012/08/20 15:57:40]

Done.

   if (!global_) {
     used_without_global_ = true;
     return false;
   }
 
-  return !global_->client_id_.empty();
-}
-
-// static
-const std::string& FieldTrialList::client_id() {
-  DCHECK(global_);
-  if (!global_)
-    return EmptyString();
-
-  return global_->client_id_;
+  return global_->entropy_provider_.get() != NULL;
 }
 
 FieldTrial* FieldTrialList::PreLockedFind(const std::string& name) {
   RegistrationList::iterator it = registered_.find(name);
   if (registered_.end() == it)
     return NULL;
   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