Fix inconsistent FieldTrial group assignment due to float errors.

This was discovered by the InstantExtended field trial that was
using very small group percentages and discovered several groups
of the same size were getting different user counts.

BUG=290438
TEST=New unit test that fails without the fix.

Review URL: https://chromiumcodereview.appspot.com/23710041

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@224011 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 52 insertions, 3 deletions

diff --git a/base/metrics/field_trial.cc b/base/metrics/field_trial.cc
index 9d4ec72..d500a29 100644
--- a/base/metrics/field_trial.cc
+++ b/base/metrics/field_trial.cc
@@ -4,6 +4,8 @@
 
 #include "base/metrics/field_trial.h"
 
+#include <algorithm>
+
 #include "base/build_time.h"
 #include "base/logging.h"
 #include "base/rand_util.h"
@@ -38,9 +40,26 @@ Time CreateTimeFromParams(int year, int month, int day_of_month) {
   return Time::FromLocalExploded(exploded);
 }
 
-}  // namespace
+// Returns the boundary value for comparing against the FieldTrial's added
+// groups for a given |divisor| (total probability) and |entropy_value|.
+FieldTrial::Probability GetGroupBoundaryValue(
+    FieldTrial::Probability divisor,
+    double entropy_value) {
+  // Add a tiny epsilon value to get consistent results when converting floating
+  // points to int. Without it, boundary values have inconsistent results, e.g.:
+  //
+  //   static_cast<FieldTrial::Probability>(100 * 0.56) == 56
+  //   static_cast<FieldTrial::Probability>(100 * 0.57) == 56
+  //   static_cast<FieldTrial::Probability>(100 * 0.58) == 57
+  //   static_cast<FieldTrial::Probability>(100 * 0.59) == 59
+  const double kEpsilon = 1e-8;
+  const FieldTrial::Probability result =
+      static_cast<FieldTrial::Probability>(divisor * entropy_value + kEpsilon);
+  // Ensure that adding the epsilon still results in a value < |divisor|.
+  return std::min(result, divisor - 1);
+}
 
-static const char kHistogramFieldTrialSeparator('_');
+}  // namespace
 
 // statics
 const int FieldTrial::kNotFinalized = -1;
@@ -60,7 +79,7 @@ FieldTrial::FieldTrial(const std::string& trial_name,
     : trial_name_(trial_name),
       divisor_(total_probability),
       default_group_name_(default_group_name),
-      random_(static_cast<Probability>(divisor_ * entropy_value)),
+      random_(GetGroupBoundaryValue(total_probability, entropy_value)),
       accumulated_group_probability_(0),
       next_group_number_(kDefaultGroupNumber + 1),
       group_(kNotFinalized),
diff --git a/base/metrics/field_trial.h b/base/metrics/field_trial.h
index 6dd2d9c..5060a74 100644
--- a/base/metrics/field_trial.h
+++ b/base/metrics/field_trial.h
@@ -173,6 +173,8 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> {
   FRIEND_TEST_ALL_PREFIXES(FieldTrialTest, SetForcedTurnFeatureOn);
   FRIEND_TEST_ALL_PREFIXES(FieldTrialTest, SetForcedChangeDefault_Default);
   FRIEND_TEST_ALL_PREFIXES(FieldTrialTest, SetForcedChangeDefault_NonDefault);
+  FRIEND_TEST_ALL_PREFIXES(FieldTrialTest, FloatBoundariesGiveEqualGroupSizes);
+  FRIEND_TEST_ALL_PREFIXES(FieldTrialTest, DoesNotSurpassTotalProbability);
 
   friend class base::FieldTrialList;
 
diff --git a/base/metrics/field_trial_unittest.cc b/base/metrics/field_trial_unittest.cc
index bfcba0f..10d3c3f 100644
--- a/base/metrics/field_trial_unittest.cc
+++ b/base/metrics/field_trial_unittest.cc
@@ -849,4 +849,32 @@ TEST_F(FieldTrialTest, ExpirationYearNotExpired) {
   EXPECT_EQ(kGroupName, trial->group_name());
 }
 
+TEST_F(FieldTrialTest, FloatBoundariesGiveEqualGroupSizes) {
+  const int kBucketCount = 100;
+
+  // Try each boundary value |i / 100.0| as the entropy value.
+  for (int i = 0; i < kBucketCount; ++i) {
+    const double entropy = i / static_cast<double>(kBucketCount);
+
+    scoped_refptr<base::FieldTrial> trial(
+        new base::FieldTrial("test", kBucketCount, "default", entropy));
+    for (int j = 0; j < kBucketCount; ++j)
+      trial->AppendGroup(base::StringPrintf("%d", j), 1);
+
+    EXPECT_EQ(base::StringPrintf("%d", i), trial->group_name());
+  }
+}
+
+TEST_F(FieldTrialTest, DoesNotSurpassTotalProbability) {
+  const double kEntropyValue = 1.0 - 1e-9;
+  ASSERT_LT(kEntropyValue, 1.0);
+
+  scoped_refptr<base::FieldTrial> trial(
+      new base::FieldTrial("test", 2, "default", kEntropyValue));
+  trial->AppendGroup("1", 1);
+  trial->AppendGroup("2", 1);
+
+  EXPECT_EQ("2", trial->group_name());
+}
+
 }  // namespace base