Allow variation id with forcing flag for special setup & unit tests.

BUG=317879

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

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

3 files changed, 123 insertions, 18 deletions

diff --git a/components/variations/variations_seed_processor.cc b/components/variations/variations_seed_processor.cc
index e7a33ca..6174558 100644
--- a/components/variations/variations_seed_processor.cc
+++ b/components/variations/variations_seed_processor.cc
@@ -56,6 +56,28 @@ void RegisterExperimentParams(const Study& study,
     AssociateVariationParams(study.name(), experiment.name(), params);
 }
 
+// If there are variation ids associated with |experiment|, register the
+// variation ids.
+void RegisterVariationIds(const Study_Experiment& experiment,
+                          const std::string& trial_name) {
+  if (experiment.has_google_web_experiment_id()) {
+    const VariationID variation_id =
+        static_cast<VariationID>(experiment.google_web_experiment_id());
+    AssociateGoogleVariationIDForce(GOOGLE_WEB_PROPERTIES,
+                                    trial_name,
+                                    experiment.name(),
+                                    variation_id);
+  }
+  if (experiment.has_google_update_experiment_id()) {
+    const VariationID variation_id =
+        static_cast<VariationID>(experiment.google_update_experiment_id());
+    AssociateGoogleVariationIDForce(GOOGLE_UPDATE_SERVICE,
+                                    trial_name,
+                                    experiment.name(),
+                                    variation_id);
+  }
+}
+
 }  // namespace
 
 VariationsSeedProcessor::VariationsSeedProcessor() {
@@ -64,6 +86,28 @@ VariationsSeedProcessor::VariationsSeedProcessor() {
 VariationsSeedProcessor::~VariationsSeedProcessor() {
 }
 
+bool VariationsSeedProcessor::AllowVariationIdWithForcingFlag(
+    const Study& study) {
+  if (!study.has_filter())
+    return false;
+  const Study_Filter& filter = study.filter();
+  if (filter.platform_size() == 0 || filter.channel_size() == 0)
+    return false;
+  for (int i = 0; i < filter.platform_size(); ++i) {
+    if (filter.platform(i) != Study_Platform_PLATFORM_ANDROID &&
+        filter.platform(i) != Study_Platform_PLATFORM_IOS) {
+      return false;
+    }
+  }
+  for (int i = 0; i < filter.channel_size(); ++i) {
+    if (filter.channel(i) != Study_Channel_CANARY &&
+        filter.channel(i) != Study_Channel_DEV) {
+      return false;
+    }
+  }
+  return true;
+}
+
 void VariationsSeedProcessor::CreateTrialsFromSeed(
     const VariationsSeed& seed,
     const std::string& locale,
@@ -225,6 +269,8 @@ void VariationsSeedProcessor::CreateTrialFromStudy(
       RegisterExperimentParams(study, experiment);
       DVLOG(1) << "Trial " << study.name() << " forced by flag: "
                << experiment.forcing_flag();
+      if (AllowVariationIdWithForcingFlag(study))
+        RegisterVariationIds(experiment, study.name());
       return;
     }
   }
@@ -253,30 +299,15 @@ void VariationsSeedProcessor::CreateTrialFromStudy(
     const Study_Experiment& experiment = study.experiment(i);
     RegisterExperimentParams(study, experiment);
 
-    // Groups with flags can't be selected randomly, so we don't add them to
-    // the field trial.
+    // Groups with forcing flags have probability 0 and will never be selected.
+    // Therefore, there's no need to add them to the field trial.
     if (experiment.has_forcing_flag())
       continue;
 
     if (experiment.name() != study.default_experiment_name())
       trial->AppendGroup(experiment.name(), experiment.probability_weight());
 
-    if (experiment.has_google_web_experiment_id()) {
-      const VariationID variation_id =
-          static_cast<VariationID>(experiment.google_web_experiment_id());
-      AssociateGoogleVariationIDForce(GOOGLE_WEB_PROPERTIES,
-                                      study.name(),
-                                      experiment.name(),
-                                      variation_id);
-    }
-    if (experiment.has_google_update_experiment_id()) {
-      const VariationID variation_id =
-          static_cast<VariationID>(experiment.google_update_experiment_id());
-      AssociateGoogleVariationIDForce(GOOGLE_UPDATE_SERVICE,
-                                      study.name(),
-                                      experiment.name(),
-                                      variation_id);
-    }
+    RegisterVariationIds(experiment, study.name());
   }
 
   trial->SetForced();
diff --git a/components/variations/variations_seed_processor.h b/components/variations/variations_seed_processor.h
index d714943..dbf8cd2 100644
--- a/components/variations/variations_seed_processor.h
+++ b/components/variations/variations_seed_processor.h
@@ -36,6 +36,10 @@ class VariationsSeedProcessor {
                             Study_FormFactor form_factor);
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest,
+                           AllowForceGroupAndVariationId);
+  FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest,
+                           AllowVariationIdWithForcingFlag);
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest, CheckStudyChannel);
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest, CheckStudyFormFactor);
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest, CheckStudyLocale);
@@ -44,6 +48,8 @@ class VariationsSeedProcessor {
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest, CheckStudyVersion);
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest,
                            FilterAndValidateStudies);
+  FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest,
+                           ForbidForceGroupWithVariationId);
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest, ForceGroupWithFlag1);
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest, ForceGroupWithFlag2);
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest,
@@ -56,6 +62,11 @@ class VariationsSeedProcessor {
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest,
                            VariationParamsWithForcingFlag);
 
+  // Check if the |study| is only associated with platform Android/iOS and
+  // channel dev/canary. If so, forcing flag and variation id can both be set.
+  // (Otherwise, forcing_flag and variation_id are mutually exclusive.)
+  bool AllowVariationIdWithForcingFlag(const Study& study);
+
   // 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.
diff --git a/components/variations/variations_seed_processor_unittest.cc b/components/variations/variations_seed_processor_unittest.cc
index 7b86def..35a21ca 100644
--- a/components/variations/variations_seed_processor_unittest.cc
+++ b/components/variations/variations_seed_processor_unittest.cc
@@ -29,6 +29,8 @@ const char kNonFlagGroupName[] = "non_flag_group";
 const char kForcingFlag1[] = "flag_test1";
 const char kForcingFlag2[] = "flag_test2";
 
+const VariationID kExperimentId = 123;
+
 // Adds an experiment to |study| with the specified |name| and |probability|.
 Study_Experiment* AddExperiment(const std::string& name, int probability,
                                 Study* study) {
@@ -90,6 +92,47 @@ class VariationsSeedProcessorTest : public ::testing::Test {
   DISALLOW_COPY_AND_ASSIGN(VariationsSeedProcessorTest);
 };
 
+TEST_F(VariationsSeedProcessorTest, AllowForceGroupAndVariationId) {
+  CommandLine::ForCurrentProcess()->AppendSwitch(kForcingFlag1);
+
+  base::FieldTrialList field_trial_list(NULL);
+
+  Study study = CreateStudyWithFlagGroups(100, 0, 0);
+  study.mutable_experiment(1)->set_google_web_experiment_id(kExperimentId);
+  study.mutable_filter()->add_channel(Study_Channel_DEV);
+  study.mutable_filter()->add_channel(Study_Channel_CANARY);
+  study.mutable_filter()->add_platform(Study_Platform_PLATFORM_ANDROID);
+
+  VariationsSeedProcessor().CreateTrialFromStudy(
+      ProcessedStudy(&study, 100, false));
+
+  EXPECT_EQ(kFlagGroup1Name,
+            base::FieldTrialList::FindFullName(kFlagStudyName));
+
+  VariationID id = GetGoogleVariationID(GOOGLE_WEB_PROPERTIES, kFlagStudyName,
+                                        kFlagGroup1Name);
+  EXPECT_EQ(kExperimentId, id);
+}
+
+TEST_F(VariationsSeedProcessorTest, AllowVariationIdWithForcingFlag) {
+  VariationsSeedProcessor seed_processor;
+  Study study = CreateStudyWithFlagGroups(100, 0, 0);
+  EXPECT_FALSE(seed_processor.AllowVariationIdWithForcingFlag(study));
+
+  study.mutable_filter()->add_channel(Study_Channel_DEV);
+  EXPECT_FALSE(seed_processor.AllowVariationIdWithForcingFlag(study));
+
+  study.mutable_filter()->add_platform(Study_Platform_PLATFORM_ANDROID);
+  EXPECT_TRUE(seed_processor.AllowVariationIdWithForcingFlag(study));
+
+  study.mutable_filter()->add_channel(Study_Channel_CANARY);
+  study.mutable_filter()->add_platform(Study_Platform_PLATFORM_IOS);
+  EXPECT_TRUE(seed_processor.AllowVariationIdWithForcingFlag(study));
+
+  study.mutable_filter()->add_platform(Study_Platform_PLATFORM_WINDOWS);
+  EXPECT_FALSE(seed_processor.AllowVariationIdWithForcingFlag(study));
+}
+
 TEST_F(VariationsSeedProcessorTest, CheckStudyChannel) {
   VariationsSeedProcessor seed_processor;
 
@@ -424,6 +467,26 @@ TEST_F(VariationsSeedProcessorTest, FilterAndValidateStudies) {
   EXPECT_EQ(kTrial3Name, processed_studies[1].study->name());
 }
 
+TEST_F(VariationsSeedProcessorTest, ForbidForceGroupWithVariationId) {
+  CommandLine::ForCurrentProcess()->AppendSwitch(kForcingFlag1);
+
+  base::FieldTrialList field_trial_list(NULL);
+
+  Study study = CreateStudyWithFlagGroups(100, 0, 0);
+  study.mutable_experiment(1)->set_google_web_experiment_id(kExperimentId);
+  // Adding windows platform makes forcing_flag and variation Id incompatible.
+  study.mutable_filter()->add_platform(Study_Platform_PLATFORM_WINDOWS);
+
+  VariationsSeedProcessor().CreateTrialFromStudy(
+      ProcessedStudy(&study, 100, false));
+
+  EXPECT_EQ(kFlagGroup1Name,
+            base::FieldTrialList::FindFullName(kFlagStudyName));
+  VariationID id = GetGoogleVariationID(GOOGLE_WEB_PROPERTIES, kFlagStudyName,
+                                        kFlagGroup1Name);
+  EXPECT_EQ(EMPTY_ID, id);
+}
+
 // Test that the group for kForcingFlag1 is forced.
 TEST_F(VariationsSeedProcessorTest, ForceGroupWithFlag1) {
   CommandLine::ForCurrentProcess()->AppendSwitch(kForcingFlag1);