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

components/variations/variations_seed_processor.cc

Index: components/variations/variations_seed_processor.cc
diff --git a/components/variations/variations_seed_processor.cc b/components/variations/variations_seed_processor.cc
index e7a33cad035433b67603eabfccb819868d54ca38..aacef718dde771e23e96e9977812a003e6229eb8 100644
--- a/components/variations/variations_seed_processor.cc
+++ b/components/variations/variations_seed_processor.cc
@@ -64,6 +64,27 @@ VariationsSeedProcessor::VariationsSeedProcessor() {
 VariationsSeedProcessor::~VariationsSeedProcessor() {
 }
 
+bool VariationsSeedProcessor::AllowVariationIdWithForcingFlag(

[Alexei Svitkine (slow), 2013/11/14 16:54:15]

This and the other function can just be free-standing functions in the anon
namespace at the top of the file.

[yao, 2013/11/14 20:51:12]

Done.

+    const Study& study) {
+  if (!study.has_filter())
+    return false;
+  Study_Filter filter = study.filter();
+  if (filter.platform_size() == 0 || filter.channel_size() == 0) {

[Alexei Svitkine (slow), 2013/11/14 16:54:15]

This doesn't need {}'s. (single line if and single line body)

[yao, 2013/11/14 20:51:12]

Done.

+    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)

[Alexei Svitkine (slow), 2013/11/14 16:54:15]

This does need {}'s - multi-line if. (Same below).

[yao, 2013/11/14 20:51:12]

Done.

+      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,
@@ -210,9 +231,31 @@ bool VariationsSeedProcessor::CheckStudyVersion(
   return true;
 }
 
+void VariationsSeedProcessor::CheckVariationId(

[Alexei Svitkine (slow), 2013/11/14 16:54:15]

This should be called RegisterVariationIds().

[yao, 2013/11/14 20:51:12]

Done.

+    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);
+    }
+}
+
 void VariationsSeedProcessor::CreateTrialFromStudy(
     const ProcessedStudy& processed_study) {
   const Study& study = *processed_study.study;
+  bool allowBoth = AllowVariationIdWithForcingFlag(study);

[Alexei Svitkine (slow), 2013/11/14 16:54:15]

Can we do this check lazily? (e.g. block below when we create the trial for the
forced flag?

That way, if there's 100 studies, we only do the check when necessary.

[yao, 2013/11/14 20:51:12]

Done.

 
   // Check if any experiments need to be forced due to a command line
   // flag. Force the first experiment with an existing flag.
@@ -225,6 +268,9 @@ void VariationsSeedProcessor::CreateTrialFromStudy(
       RegisterExperimentParams(study, experiment);
       DVLOG(1) << "Trial " << study.name() << " forced by flag: "
                << experiment.forcing_flag();
+      if (allowBoth) {
+        CheckVariationId(experiment, study.name());
+      }
       return;
     }
   }
@@ -255,28 +301,13 @@ void VariationsSeedProcessor::CreateTrialFromStudy(
 
     // Groups with flags can't be selected randomly, so we don't add them to
     // the field trial.
-    if (experiment.has_forcing_flag())
+    if (experiment.has_forcing_flag() && !allowBoth)

[Alexei Svitkine (slow), 2013/11/14 16:54:15]

I think we don't need this logic in this case. It's fine not registering the ids
except for the selected group.

[yao, 2013/11/14 20:51:12]

Done.

       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);
-    }
+    CheckVariationId(experiment, study.name());
   }
 
   trial->SetForced();