Refactor out study validation function in VariationsService and add a unit test.

BUG=none
TEST=VariationsServiceTest.ValidateStudy

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

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

3 files changed, 88 insertions, 29 deletions

diff --git a/chrome/browser/metrics/variations_service.cc b/chrome/browser/metrics/variations_service.cc
index e47916f..9b890ca 100644
--- a/chrome/browser/metrics/variations_service.cc
+++ b/chrome/browser/metrics/variations_service.cc
@@ -208,6 +208,42 @@ bool VariationsService::CheckStudyDate(const chrome_variations::Study& study,
   return true;
 }
 
+// static
+bool VariationsService::ValidateStudyAndComputeTotalProbability(
+    const chrome_variations::Study& study,
+    base::FieldTrial::Probability* total_probability) {
+  // At the moment, a missing default_experiment_name makes the study invalid.
+  if (study.default_experiment_name().empty()) {
+    DVLOG(1) << study.name() << " has no default experiment defined.";
+    return false;
+  }
+
+  const std::string& default_group_name = study.default_experiment_name();
+  base::FieldTrial::Probability divisor = 0;
+
+  bool found_default_group = false;
+  for (int i = 0; i < study.experiment_size(); ++i) {
+    if (study.experiment(i).name().empty()) {
+      DVLOG(1) << study.name() << " is missing experiment " << i << " name";
+      return false;
+    }
+    divisor += study.experiment(i).probability_weight();
+    if (study.experiment(i).name() == default_group_name)
+      found_default_group = true;
+  }
+
+  if (!found_default_group) {
+    DVLOG(1) << study.name() << " is missing default experiment in its "
+             << "experiment list";
+    // The default group was not found in the list of groups. This study is not
+    // valid.
+    return false;
+  }
+
+  *total_probability = divisor;
+  return true;
+}
+
 bool VariationsService::LoadTrialsSeedFromPref(
     PrefService* local_prefs,
     chrome_variations::TrialsSeed* seed) {
@@ -228,31 +264,12 @@ bool VariationsService::LoadTrialsSeedFromPref(
 
 void VariationsService::CreateTrialFromStudy(
     const chrome_variations::Study& study) {
-  if (!ShouldAddStudy(study))
+  base::FieldTrial::Probability total_probability = 0;
+  if (!ValidateStudyAndComputeTotalProbability(study, &total_probability))
     return;
 
-  // At the moment, a missing default_experiment_name makes the study invalid.
-  if (!study.has_default_experiment_name()) {
-    DVLOG(1) << study.name() << " has no default experiment defined.";
-    return;
-  }
-
-  const std::string& default_group_name = study.default_experiment_name();
-  base::FieldTrial::Probability divisor = 0;
-
-  bool found_default_group = false;
-  for (int i = 0; i < study.experiment_size(); ++i) {
-    divisor += study.experiment(i).probability_weight();
-    if (study.experiment(i).name() == default_group_name)
-      found_default_group = true;
-  }
-  if (!found_default_group) {
-    DVLOG(1) << study.name() << " is missing default experiment in it's "
-             << "experiment list";
-    // The default group was not found in the list of groups. This study is not
-    // valid.
+  if (!ShouldAddStudy(study))
     return;
-  }
 
   const base::Time expiry_date =
       ConvertStudyDateToBaseTime(study.expiry_date());
@@ -261,8 +278,9 @@ void VariationsService::CreateTrialFromStudy(
 
   scoped_refptr<base::FieldTrial> trial(
       base::FieldTrialList::FactoryGetFieldTrial(
-          study.name(), divisor, default_group_name, exploded_end_date.year,
-          exploded_end_date.month, exploded_end_date.day_of_month, NULL));
+          study.name(), total_probability, study.default_experiment_name(),
+          exploded_end_date.year, exploded_end_date.month,
+          exploded_end_date.day_of_month, NULL));
 
   if (study.has_consistency() &&
       study.consistency() == chrome_variations::Study_Consistency_PERMANENT) {
@@ -270,7 +288,7 @@ void VariationsService::CreateTrialFromStudy(
   }
 
   for (int i = 0; i < study.experiment_size(); ++i) {
-    if (study.experiment(i).name() != default_group_name) {
+    if (study.experiment(i).name() != study.default_experiment_name()) {
       trial->AppendGroup(study.experiment(i).name(),
                          study.experiment(i).probability_weight());
     }
diff --git a/chrome/browser/metrics/variations_service.h b/chrome/browser/metrics/variations_service.h
index b0009d2..f580494 100644
--- a/chrome/browser/metrics/variations_service.h
+++ b/chrome/browser/metrics/variations_service.h
@@ -11,6 +11,7 @@
 #include "base/compiler_specific.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/metrics/field_trial.h"
 #include "base/time.h"
 #include "chrome/browser/metrics/proto/study.pb.h"
 #include "chrome/browser/metrics/proto/trials_seed.pb.h"
@@ -50,6 +51,7 @@ class VariationsService : public net::URLFetcherDelegate {
   FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, CheckStudyVersion);
   FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, CheckStudyVersionWildcards);
   FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, CheckStudyDate);
+  FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, ValidateStudy);
 
   // Store the given seed data to the given local prefs. Note that |seed_data|
   // is assumed to be the raw serialized protobuf data stored in a string. It
@@ -73,6 +75,11 @@ class VariationsService : public net::URLFetcherDelegate {
   static bool CheckStudyDate(const chrome_variations::Study& study,
                              const base::Time& date_time);
 
+  // Validates the sanity of |study| and computes the total probability.
+  static bool ValidateStudyAndComputeTotalProbability(
+      const chrome_variations::Study& study,
+      base::FieldTrial::Probability* total_probability);
+
   // Loads the Variations seed data from the given local prefs into |seed|. If
   // there is a problem with loading, the pref value is cleared and false is
   // returned. If successful, |seed| will contain the loaded data and true is
diff --git a/chrome/browser/metrics/variations_service_unittest.cc b/chrome/browser/metrics/variations_service_unittest.cc
index 67f2c19f..0f66ffe 100644
--- a/chrome/browser/metrics/variations_service_unittest.cc
+++ b/chrome/browser/metrics/variations_service_unittest.cc
@@ -44,8 +44,7 @@ TEST(VariationsServiceTest, CheckStudyChannel) {
       EXPECT_EQ(expected, result) << "Case " << i << "," << j << " failed!";
     }
 
-    if (i < arraysize(study_channels))
-    {
+    if (i < arraysize(study_channels)) {
       study.add_channel(study_channels[i]);
       channel_added[i] = true;
     }
@@ -62,8 +61,7 @@ TEST(VariationsServiceTest, CheckStudyChannel) {
       EXPECT_EQ(expected, result) << "Case " << i << "," << j << " failed!";
     }
 
-    if (i < arraysize(study_channels))
-    {
+    if (i < arraysize(study_channels)) {
       const int index = arraysize(study_channels) - i - 1;
       study.add_channel(study_channels[index]);
       channel_added[index] = true;
@@ -208,3 +206,39 @@ TEST(VariationsServiceTest, CheckStudyDate) {
     }
   }
 }
+
+TEST(VariationsServiceTest, ValidateStudy) {
+  chrome_variations::Study study;
+  study.set_default_experiment_name("def");
+
+  chrome_variations::Study_Experiment* experiment = study.add_experiment();
+  experiment->set_name("abc");
+  experiment->set_probability_weight(100);
+
+  chrome_variations::Study_Experiment* default_group = study.add_experiment();
+  default_group->set_name("def");
+  default_group->set_probability_weight(200);
+
+  base::FieldTrial::Probability total_probability = 0;
+  bool valid = VariationsService::ValidateStudyAndComputeTotalProbability(
+      study, &total_probability);
+  EXPECT_TRUE(valid);
+  EXPECT_EQ(300, total_probability);
+
+  study.clear_default_experiment_name();
+  valid = VariationsService::ValidateStudyAndComputeTotalProbability(study,
+      &total_probability);
+  EXPECT_FALSE(valid);
+
+  study.set_default_experiment_name("xyz");
+  valid = VariationsService::ValidateStudyAndComputeTotalProbability(study,
+      &total_probability);
+  EXPECT_FALSE(valid);
+
+  study.set_default_experiment_name("def");
+  default_group->clear_name();
+  valid = VariationsService::ValidateStudyAndComputeTotalProbability(study,
+      &total_probability);
+  EXPECT_FALSE(valid);
+}
+