Refactor VariationsSeedProcessor to add ProcessedStudy struct.

This paves way for more changes to follow that will re-use the
FilterAndValidateStudies logic, but will do different things with the
resulting ProcessedStudy list (simulate the resulting field trials).

BUG=315807
TEST=New unit test.

Review URL: https://codereview.chromium.org/59103009

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

6 files changed, 165 insertions, 21 deletions

diff --git a/components/variations.gypi b/components/variations.gypi
index a5b9c6e..4a69d65 100644
--- a/components/variations.gypi
+++ b/components/variations.gypi
@@ -17,6 +17,8 @@
       'sources': [
         'variations/entropy_provider.cc',
         'variations/entropy_provider.h',
+        'variations/processed_study.cc',
+        'variations/processed_study.h',
         'variations/proto/variations_seed.proto',
         'variations/proto/study.proto',
         'variations/metrics_util.cc',
diff --git a/components/variations/processed_study.cc b/components/variations/processed_study.cc
new file mode 100644
index 0000000..b13e138
--- /dev/null
+++ b/components/variations/processed_study.cc
@@ -0,0 +1,22 @@
+// Copyright 2013 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 "components/variations/processed_study.h"
+
+#include "components/variations/proto/study.pb.h"
+
+namespace chrome_variations {
+
+ProcessedStudy::ProcessedStudy(const Study* study,
+                               base::FieldTrial::Probability total_probability,
+                               bool is_expired)
+    : study(study),
+      total_probability(total_probability),
+      is_expired(is_expired) {
+}
+
+ProcessedStudy::~ProcessedStudy() {
+}
+
+}  // namespace chrome_variations
diff --git a/components/variations/processed_study.h b/components/variations/processed_study.h
new file mode 100644
index 0000000..64b290c
--- /dev/null
+++ b/components/variations/processed_study.h
@@ -0,0 +1,34 @@
+// Copyright 2013 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.
+
+#ifndef COMPONENTS_VARIATIONS_PROCESSED_STUDY_H_
+#define COMPONENTS_VARIATIONS_PROCESSED_STUDY_H_
+
+#include "base/metrics/field_trial.h"
+
+namespace chrome_variations {
+
+class Study;
+
+// Wrapper over Study with extra information computed during pre-processing,
+// such as whether the study is expired and its total probability.
+struct ProcessedStudy {
+  ProcessedStudy(const Study* study,
+                 base::FieldTrial::Probability total_probability,
+                 bool is_expired);
+  ~ProcessedStudy();
+
+  // Corresponding Study object. Weak reference.
+  const Study* study;
+
+  // Computed total group probability for the study.
+  base::FieldTrial::Probability total_probability;
+
+  // Whether the study is expired.
+  bool is_expired;
+};
+
+}  // namespace chrome_variations
+
+#endif  // COMPONENTS_VARIATIONS_PROCESSED_STUDY_H_
diff --git a/components/variations/variations_seed_processor.cc b/components/variations/variations_seed_processor.cc
index e268272..b26cae8 100644
--- a/components/variations/variations_seed_processor.cc
+++ b/components/variations/variations_seed_processor.cc
@@ -12,6 +12,7 @@
 #include "base/metrics/field_trial.h"
 #include "base/stl_util.h"
 #include "base/version.h"
+#include "components/variations/processed_study.h"
 #include "components/variations/variations_associated_data.h"
 
 namespace chrome_variations {
@@ -69,6 +70,21 @@ void VariationsSeedProcessor::CreateTrialsFromSeed(
     const base::Time& reference_date,
     const base::Version& version,
     Study_Channel channel) {
+  std::vector<ProcessedStudy> filtered_studies;
+  FilterAndValidateStudies(seed, locale, reference_date, version, channel,
+                           &filtered_studies);
+
+  for (size_t i = 0; i < filtered_studies.size(); ++i)
+    CreateTrialFromStudy(filtered_studies[i]);
+}
+
+void VariationsSeedProcessor::FilterAndValidateStudies(
+    const VariationsSeed& seed,
+    const std::string& locale,
+    const base::Time& reference_date,
+    const base::Version& version,
+    Study_Channel channel,
+    std::vector<ProcessedStudy>* filtered_studies) {
   DCHECK(version.IsValid());
 
   // Add expired studies (in a disabled state) only after all the non-expired
@@ -85,18 +101,29 @@ void VariationsSeedProcessor::CreateTrialsFromSeed(
 
     if (IsStudyExpired(study, reference_date)) {
       expired_studies.push_back(&study);
-    } else {
-      CreateTrialFromStudy(study, false);
+    } else if (!ContainsKey(created_studies, study.name())) {
+      ValidateAndAddStudy(study, false, filtered_studies);
       created_studies.insert(study.name());
     }
   }
 
   for (size_t i = 0; i < expired_studies.size(); ++i) {
     if (!ContainsKey(created_studies, expired_studies[i]->name()))
-      CreateTrialFromStudy(*expired_studies[i], true);
+      ValidateAndAddStudy(*expired_studies[i], true, filtered_studies);
   }
 }
 
+void VariationsSeedProcessor::ValidateAndAddStudy(
+    const Study& study,
+    bool is_expired,
+    std::vector<ProcessedStudy>* filtered_studies) {
+  base::FieldTrial::Probability total_probability = 0;
+  if (!ValidateStudyAndComputeTotalProbability(study, &total_probability))
+    return;
+  filtered_studies->push_back(ProcessedStudy(&study, total_probability,
+                                              is_expired));
+}
+
 bool VariationsSeedProcessor::CheckStudyChannel(const Study_Filter& filter,
                                                 Study_Channel channel) {
   // An empty channel list matches all channels.
@@ -166,11 +193,9 @@ bool VariationsSeedProcessor::CheckStudyVersion(
   return true;
 }
 
-void VariationsSeedProcessor::CreateTrialFromStudy(const Study& study,
-                                                   bool is_expired) {
-  base::FieldTrial::Probability total_probability = 0;
-  if (!ValidateStudyAndComputeTotalProbability(study, &total_probability))
-    return;
+void VariationsSeedProcessor::CreateTrialFromStudy(
+    const ProcessedStudy& processed_study) {
+  const Study& study = *processed_study.study;
 
   // Check if any experiments need to be forced due to a command line
   // flag. Force the first experiment with an existing flag.
@@ -202,7 +227,8 @@ void VariationsSeedProcessor::CreateTrialFromStudy(const Study& study,
   // the expiration check using |reference_date| is done explicitly below.
   scoped_refptr<base::FieldTrial> trial(
       base::FieldTrialList::FactoryGetFieldTrialWithRandomizationSeed(
-          study.name(), total_probability, study.default_experiment_name(),
+          study.name(), processed_study.total_probability,
+          study.default_experiment_name(),
           base::FieldTrialList::kNoExpirationYear, 1, 1, randomization_type,
           randomization_seed, NULL));
 
@@ -237,7 +263,7 @@ void VariationsSeedProcessor::CreateTrialFromStudy(const Study& study,
   }
 
   trial->SetForced();
-  if (is_expired)
+  if (processed_study.is_expired)
     trial->Disable();
   else if (study.activation_type() == Study_ActivationType_ACTIVATION_AUTO)
     trial->group();
diff --git a/components/variations/variations_seed_processor.h b/components/variations/variations_seed_processor.h
index 90effff..45a2393 100644
--- a/components/variations/variations_seed_processor.h
+++ b/components/variations/variations_seed_processor.h
@@ -6,6 +6,7 @@
 #define COMPONENTS_VARIATIONS_VARIATIONS_SEED_PROCESSOR_H_
 
 #include <string>
+#include <vector>
 
 #include "base/compiler_specific.h"
 #include "base/gtest_prod_util.h"
@@ -17,6 +18,8 @@
 
 namespace chrome_variations {
 
+struct ProcessedStudy;
+
 // Helper class to instantiate field trials from a variations seed.
 class VariationsSeedProcessor {
  public:
@@ -38,7 +41,7 @@ class VariationsSeedProcessor {
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest, CheckStudyStartDate);
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest, CheckStudyVersion);
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest,
-                           CheckStudyVersionWildcards);
+                           FilterAndValidateStudies);
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest, ForceGroupWithFlag1);
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest, ForceGroupWithFlag2);
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest,
@@ -51,6 +54,22 @@ class VariationsSeedProcessor {
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest,
                            VariationParamsWithForcingFlag);
 
+  // Filters the list of studies in |seed| and validates and pre-processes them,
+  // adding any kept studies to |filtered_studies| list. Ensures that the
+  // resulting list will not have more than one study with the same name.
+  void FilterAndValidateStudies(const VariationsSeed& seed,
+                                const std::string& locale,
+                                const base::Time& reference_date,
+                                const base::Version& version,
+                                Study_Channel channel,
+                                std::vector<ProcessedStudy>* filtered_studies);
+
+  // Validates |study| and if valid, adds it to |filtered_studies| as a
+  // ProcessedStudy object.
+  void ValidateAndAddStudy(const Study& study,
+                           bool is_expired,
+                           std::vector<ProcessedStudy>* filtered_studies);
+
   // Checks whether a study is applicable for the given |channel| per |filter|.
   bool CheckStudyChannel(const Study_Filter& filter, Study_Channel channel);
 
@@ -68,9 +87,9 @@ class VariationsSeedProcessor {
   bool CheckStudyVersion(const Study_Filter& filter,
                          const base::Version& version);
 
-  // Creates and registers a field trial from the |study| data. Disables the
-  // trial if |is_expired| is true.
-  void CreateTrialFromStudy(const Study& study, bool is_expired);
+  // Creates and registers a field trial from the |processed_study| data.
+  // Disables the trial if |processed_study.is_expired| is true.
+  void CreateTrialFromStudy(const ProcessedStudy& processed_study);
 
   // Checks whether |study| is expired using the given date/time.
   bool IsStudyExpired(const Study& study, const base::Time& date_time);
diff --git a/components/variations/variations_seed_processor_unittest.cc b/components/variations/variations_seed_processor_unittest.cc
index d3e7c5ee..f68bbe6 100644
--- a/components/variations/variations_seed_processor_unittest.cc
+++ b/components/variations/variations_seed_processor_unittest.cc
@@ -8,6 +8,7 @@
 
 #include "base/command_line.h"
 #include "base/strings/string_split.h"
+#include "components/variations/processed_study.h"
 #include "components/variations/variations_associated_data.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -338,6 +339,41 @@ TEST_F(VariationsSeedProcessorTest, CheckStudyVersion) {
   }
 }
 
+TEST_F(VariationsSeedProcessorTest, FilterAndValidateStudies) {
+  const std::string kTrial1Name = "A";
+  const std::string kGroup1Name = "Group1";
+  const std::string kTrial3Name = "B";
+
+  VariationsSeed seed;
+  Study* study1 = seed.add_study();
+  study1->set_name(kTrial1Name);
+  study1->set_default_experiment_name("Default");
+  AddExperiment(kGroup1Name, 100, study1);
+  AddExperiment("Default", 0, study1);
+
+  Study* study2 = seed.add_study();
+  *study2 = *study1;
+  study2->mutable_experiment(0)->set_name("Bam");
+  ASSERT_EQ(seed.study(0).name(), seed.study(1).name());
+
+  Study* study3 = seed.add_study();
+  study3->set_name(kTrial3Name);
+  study3->set_default_experiment_name("Default");
+  AddExperiment("A", 10, study3);
+  AddExperiment("Default", 25, study3);
+
+  std::vector<ProcessedStudy> processed_studies;
+  VariationsSeedProcessor().FilterAndValidateStudies(
+      seed, "en-CA", base::Time::Now(), base::Version("20.0.0.0"),
+      Study_Channel_STABLE, &processed_studies);
+
+  // Check that only the first kTrial1Name study was kept.
+  ASSERT_EQ(2U, processed_studies.size());
+  EXPECT_EQ(kTrial1Name, processed_studies[0].study->name());
+  EXPECT_EQ(kGroup1Name, processed_studies[0].study->experiment(0).name());
+  EXPECT_EQ(kTrial3Name, processed_studies[1].study->name());
+}
+
 // Test that the group for kForcingFlag1 is forced.
 TEST_F(VariationsSeedProcessorTest, ForceGroupWithFlag1) {
   CommandLine::ForCurrentProcess()->AppendSwitch(kForcingFlag1);
@@ -345,7 +381,8 @@ TEST_F(VariationsSeedProcessorTest, ForceGroupWithFlag1) {
   base::FieldTrialList field_trial_list(NULL);
 
   Study study = CreateStudyWithFlagGroups(100, 0, 0);
-  VariationsSeedProcessor().CreateTrialFromStudy(study, false);
+  VariationsSeedProcessor().CreateTrialFromStudy(
+      ProcessedStudy(&study, 100, false));
 
   EXPECT_EQ(kFlagGroup1Name,
             base::FieldTrialList::FindFullName(kFlagStudyName));
@@ -358,7 +395,8 @@ TEST_F(VariationsSeedProcessorTest, ForceGroupWithFlag2) {
   base::FieldTrialList field_trial_list(NULL);
 
   Study study = CreateStudyWithFlagGroups(100, 0, 0);
-  VariationsSeedProcessor().CreateTrialFromStudy(study, false);
+  VariationsSeedProcessor().CreateTrialFromStudy(
+      ProcessedStudy(&study, 100, false));
 
   EXPECT_EQ(kFlagGroup2Name,
             base::FieldTrialList::FindFullName(kFlagStudyName));
@@ -372,7 +410,8 @@ TEST_F(VariationsSeedProcessorTest, ForceGroup_ChooseFirstGroupWithFlag) {
   base::FieldTrialList field_trial_list(NULL);
 
   Study study = CreateStudyWithFlagGroups(100, 0, 0);
-  VariationsSeedProcessor().CreateTrialFromStudy(study, false);
+  VariationsSeedProcessor().CreateTrialFromStudy(
+      ProcessedStudy(&study, 100, false));
 
   EXPECT_EQ(kFlagGroup1Name,
             base::FieldTrialList::FindFullName(kFlagStudyName));
@@ -385,7 +424,8 @@ TEST_F(VariationsSeedProcessorTest, ForceGroup_DontChooseGroupWithFlag) {
   // them very likely to be chosen. They won't be chosen since flag groups are
   // never chosen when their flag isn't present.
   Study study = CreateStudyWithFlagGroups(1, 999, 999);
-  VariationsSeedProcessor().CreateTrialFromStudy(study, false);
+  VariationsSeedProcessor().CreateTrialFromStudy(
+      ProcessedStudy(&study, 1 + 999 + 999, false));
   EXPECT_EQ(kNonFlagGroupName,
             base::FieldTrialList::FindFullName(kFlagStudyName));
 }
@@ -546,13 +586,13 @@ TEST_F(VariationsSeedProcessorTest, VariationParams) {
 
   Study_Experiment* experiment2 = AddExperiment("B", 0, &study);
 
-  seed_processor.CreateTrialFromStudy(study, false);
+  seed_processor.CreateTrialFromStudy(ProcessedStudy(&study, 1, false));
   EXPECT_EQ("y", GetVariationParamValue("Study1", "x"));
 
   study.set_name("Study2");
   experiment1->set_probability_weight(0);
   experiment2->set_probability_weight(1);
-  seed_processor.CreateTrialFromStudy(study, false);
+  seed_processor.CreateTrialFromStudy(ProcessedStudy(&study, 1, false));
   EXPECT_EQ(std::string(), GetVariationParamValue("Study2", "x"));
 }
 
@@ -565,7 +605,8 @@ TEST_F(VariationsSeedProcessorTest, VariationParamsWithForcingFlag) {
 
   CommandLine::ForCurrentProcess()->AppendSwitch(kForcingFlag1);
   base::FieldTrialList field_trial_list(NULL);
-  VariationsSeedProcessor().CreateTrialFromStudy(study, false);
+  VariationsSeedProcessor().CreateTrialFromStudy(
+      ProcessedStudy(&study, 100, false));
   EXPECT_EQ(kFlagGroup1Name, base::FieldTrialList::FindFullName(study.name()));
   EXPECT_EQ("y", GetVariationParamValue(study.name(), "x"));
 }