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

base/metrics/field_trial.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.
 
 // FieldTrial is a class for handling details of statistical experiments
 // performed by actual users in the field (i.e., in a shipped or beta product).
 // All code is called exclusively on the UI thread currently.
 //
 // The simplest example is an experiment to see whether one of two options
 // produces "better" results across our user population.  In that scenario, UMA
@@ skipping 76 matching lines @@
 #include "base/time.h"
 
 namespace base {
 
 class FieldTrialList;
 
 class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> {
  public:
   typedef int Probability;  // Probability type for being selected in a trial.
 
+  // EntropyProvider is an interface for providing entropy for one-time
+  // randomized (persistent) field trials.
+  class BASE_EXPORT EntropyProvider {
+   public:
+    virtual ~EntropyProvider();
+
+    // Returns a double in the range of [0, 1) based on |trial_name| that will
+    // be used for the dice roll for the specified field trial. A given instance
+    // should always return the same value given the same input |trial_name|.
+    virtual double GetEntropyForTrial(const std::string& trial_name) const = 0;
+  };

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

Please add DISALLOW_COPY_AND_ASSIGN(..)

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

We don't usually do this for pure interface classes. Here's an example search
for "Delegate", which brings up a bunch of these (that don't use the macro):

http://code.google.com/p/chromium/source/search?q=Delegate

Using the macro creates the following problem for subclasses that implement the
interface (i.e. forcing them to declare an empty constructor):

error: constructor for 'SHA1EntropyProvider' must explicitly initialize the base
class 'base::FieldTrial::EntropyProvider' which does not have a default
constructor

+
   // A pair representing a Field Trial and its selected group.
   struct SelectedGroup {
     std::string trial;
     std::string group;
   };
 
   typedef std::vector<SelectedGroup> SelectedGroups;
 
   // A return value to indicate that a given instance has not yet had a group
   // assignment (and hence is not yet participating in the trial).
@@ skipping 74 matching lines @@
 
   friend class base::FieldTrialList;
 
   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,
+  FieldTrial(const EntropyProvider* entropy_provider,

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

nit: add comment that |entropy_provider_| is null when one time randomization is
not supported.

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

No longer needed since the param is removed.

+             const std::string& name,
              Probability total_probability,
              const std::string& default_group_name);
   virtual ~FieldTrial();
 
   // Return the default group name of the FieldTrial.
   std::string default_group_name() const { return default_group_name_; }
 
   // Sets the group_name as well as group_name_hash to make sure they are sync.
   void SetGroupChoice(const std::string& name, int number);
 
   // Returns the group_name. A winner need not have been chosen.
   std::string group_name_internal() const { return group_name_; }
 
-  // Calculates a uniformly-distributed double between [0.0, 1.0) given
-  // a |client_id| and a |trial_name| (the latter is used as salt to avoid
-  // separate one-time randomized trials from all having the same results).
-  static double HashClientId(const std::string& client_id,
-                             const std::string& trial_name);
+  // Entropy provider that will be used if the field trial uses one-time
+  // randomization. Weak pointer, owned by FieldTrialList.
+  const EntropyProvider* const entropy_provider_;

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

Why is this needed on a per-FieldTrial basis?  I would have expected the entropy
to be calculated exactly once. Why do we need this hanging around (resulting in
having to talk about weak pointers)?

Note the original code was a static method... which could equally well have
existed in the TrialList class.

Perhaps this method could even be passed in when needed, closer to
one-time-randomization time.

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

The entropy provider is responsible for providing a random number dice roll
based on the entropy and the trial name. So calculating it exactly once doesn't
work.

We also can't just calculate the double in FieldTrialList and pass to that value
to the constructor because we only want to do this for one time randomized field
trials - and it is not known whether a trial will be such until
|UseOneTimeRandomization()| is called.

 
   // The name of the field trial, as can be found via the FieldTrialList.
   const std::string name_;
 
   // The maximum sum of all probabilities supplied, which corresponds to 100%.
   // This is the scaling factor used to adjust supplied probabilities.
   const Probability divisor_;
 
   // The name of the default group.
   const std::string default_group_name_;
@@ skipping 46 matching lines @@
   static int kExpirationYearInFuture;
 
   // Observer is notified when a FieldTrial's group is selected.
   class BASE_EXPORT Observer {
    public:
     // Notify observers when FieldTrials's group is selected.
     virtual void OnFieldTrialGroupFinalized(const std::string& trial_name,
                                             const std::string& group_name) = 0;
 
    protected:
-    virtual ~Observer() {}
+    virtual ~Observer();
   };
 
   // This singleton holds the global list of registered FieldTrials.
   //
-  // |client_id| should be an opaque, diverse ID for this client that does not
-  // change between sessions, to enable one-time randomized trials. The empty
-  // string may be provided, in which case one-time randomized trials will
-  // not be available.
-  explicit FieldTrialList(const std::string& client_id);
+  // To support one-time randomized field trials, specify a non-NULL
+  // |entropy_provider| which should be a source of uniformly distributed
+  // entropy values. Takes ownership of |entropy_provider|.

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

nit: Comment should be explicit that a NULL value is legal.

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

Done.

+  explicit FieldTrialList(FieldTrial::EntropyProvider* entropy_provider);

[Ilya Sherman, 2012/08/17 17:53:20]

Optional nit: I would pass this in as a scoped_ptr<> as well, and transfer
ownership internally.  The motivation for this pattern is that it's then harder
for the implementation to e.g. return early and fail to take ownership.  That's
obviously not a problem for the current implementation, but explicit transfers
of ownership make it less likely for this sort of error to creep in in the
future.

[Alexei Svitkine (slow), 2012/08/17 17:58:58]

I tried that, but then FieldTrialList(NULL) doesn't compile.

[Ilya Sherman, 2012/08/17 18:16:55]

How about FieldTrialList<scoped_ptr<FieldTrial::EntropyProvider>()>?  If that
doesn't work either, no worries, this is ok as it is.

+
   // Destructor Release()'s references to all registered FieldTrial instances.
   ~FieldTrialList();
 
   // Get a FieldTrial instance from the factory.
   //
   // |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
@@ skipping 90 matching lines @@
   }
 
   // Return the number of active field trials.
   static size_t GetFieldTrialCount();
 
   // Returns true if you can call |FieldTrial::UseOneTimeRandomization()|
   // without error, i.e. if a non-empty string was provided as the client_id
   // when constructing the FieldTrialList singleton.
   static bool IsOneTimeRandomizationEnabled();
 
-  // Returns an opaque, diverse ID for this client that does not change
-  // between sessions.
-  //
-  // Returns the empty string if one-time randomization is not enabled.
-  static const std::string& client_id();
-
  private:
   // A map from FieldTrial names to the actual instances.
   typedef std::map<std::string, FieldTrial*> RegistrationList;
 
   // 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
   // trial or check if one-time randomization is enabled without
   // creating the FieldTrialList. This is not an error, unless a
   // FieldTrialList is created after that.
   static bool used_without_global_;
 
   // A helper value made available to users, that shows when the FieldTrialList
   // was initialized.  Note that this is a singleton instance, and hence is a
   // good approximation to the start of the process.
   TimeTicks application_start_time_;
 
   // Lock for access to registered_.
   base::Lock lock_;
   RegistrationList registered_;
 
-  // An opaque, diverse ID for this client that does not change
-  // between sessions, or the empty string if not initialized.
-  std::string client_id_;
+  // Entropy provider to be used for one-time randomized field trials. If NULL,
+  // one-time randomization is not supported.
+  scoped_ptr<FieldTrial::EntropyProvider> entropy_provider_;
 
   // List of observers to be notified when a group is selected for a FieldTrial.
   scoped_refptr<ObserverListThreadSafe<Observer> > observer_list_;
 
   DISALLOW_COPY_AND_ASSIGN(FieldTrialList);
 };
 
 }  // namespace base
 
 #endif  // BASE_METRICS_FIELD_TRIAL_H_