Add ability to filter by form factor in VariationsService.

The from factor API under /ui/base/form_factor.h is used to obtained the current device form factor. 


BUG=312920

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

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

4 files changed, 110 insertions, 12 deletions

diff --git a/components/variations/proto/study.proto b/components/variations/proto/study.proto
index d3a2cf7..265b274 100644
--- a/components/variations/proto/study.proto
+++ b/components/variations/proto/study.proto
@@ -78,7 +78,7 @@ message Study {
     optional string forcing_flag = 5;
 
     // Parameter values for this experiment.
-    repeated Param param = 6; 
+    repeated Param param = 6;
   }
 
   // List of experiments in this study. This list should include the default /
@@ -112,10 +112,17 @@ message Study {
     PLATFORM_IOS      = 5;
   }
 
+  // Possible form factors Chrome is running on.
+  enum FormFactor {
+    DESKTOP = 0;
+    PHONE   = 1;
+    TABLET  = 2;
+  }
+
   // Filtering criteria specifying whether this study is applicable to a given
   // Chrome instance.
   //
-  // Next tag: 7
+  // Next tag: 8
   message Filter {
     // The start date of the study in Unix time format. (Seconds since midnight
     // January 1, 1970 UTC). See: http://en.wikipedia.org/wiki/Unix_time
@@ -150,6 +157,11 @@ message Study {
     // applies to all locales.
     // Ex: ["en-US", "en-CA"]
     repeated string locale = 6;
+
+    // List of form factors that will receive this study. If omitted, the study
+    // applies to all form factors.
+    // Ex: [PHONE, TABLET]
+    repeated FormFactor form_factor = 7;
   }
 
   // Filtering criteria for this study. A study that is filtered out for a given
diff --git a/components/variations/variations_seed_processor.cc b/components/variations/variations_seed_processor.cc
index b26cae8..e7a33ca 100644
--- a/components/variations/variations_seed_processor.cc
+++ b/components/variations/variations_seed_processor.cc
@@ -69,10 +69,11 @@ void VariationsSeedProcessor::CreateTrialsFromSeed(
     const std::string& locale,
     const base::Time& reference_date,
     const base::Version& version,
-    Study_Channel channel) {
+    Study_Channel channel,
+    Study_FormFactor form_factor) {
   std::vector<ProcessedStudy> filtered_studies;
   FilterAndValidateStudies(seed, locale, reference_date, version, channel,
-                           &filtered_studies);
+                           form_factor, &filtered_studies);
 
   for (size_t i = 0; i < filtered_studies.size(); ++i)
     CreateTrialFromStudy(filtered_studies[i]);
@@ -84,6 +85,7 @@ void VariationsSeedProcessor::FilterAndValidateStudies(
     const base::Time& reference_date,
     const base::Version& version,
     Study_Channel channel,
+    Study_FormFactor form_factor,
     std::vector<ProcessedStudy>* filtered_studies) {
   DCHECK(version.IsValid());
 
@@ -96,7 +98,8 @@ void VariationsSeedProcessor::FilterAndValidateStudies(
 
   for (int i = 0; i < seed.study_size(); ++i) {
     const Study& study = seed.study(i);
-    if (!ShouldAddStudy(study, locale, reference_date, version, channel))
+    if (!ShouldAddStudy(study, locale, reference_date,
+                        version, channel, form_factor))
       continue;
 
     if (IsStudyExpired(study, reference_date)) {
@@ -137,6 +140,20 @@ bool VariationsSeedProcessor::CheckStudyChannel(const Study_Filter& filter,
   return false;
 }
 
+bool VariationsSeedProcessor::CheckStudyFormFactor(
+    const Study_Filter& filter,
+    Study_FormFactor form_factor) {
+  // An empty form factor list matches all form factors.
+  if (filter.form_factor_size() == 0)
+    return true;
+
+  for (int i = 0; i < filter.form_factor_size(); ++i) {
+    if (filter.form_factor(i) == form_factor)
+      return true;
+  }
+  return false;
+}
+
 bool VariationsSeedProcessor::CheckStudyLocale(
     const Study_Filter& filter,
     const std::string& locale) {
@@ -285,13 +302,20 @@ bool VariationsSeedProcessor::ShouldAddStudy(
     const std::string& locale,
     const base::Time& reference_date,
     const base::Version& version,
-    Study_Channel channel) {
+    Study_Channel channel,
+    Study_FormFactor form_factor) {
   if (study.has_filter()) {
     if (!CheckStudyChannel(study.filter(), channel)) {
       DVLOG(1) << "Filtered out study " << study.name() << " due to channel.";
       return false;
     }
 
+    if (!CheckStudyFormFactor(study.filter(), form_factor)) {
+      DVLOG(1) << "Filtered out study " << study.name() <<
+                  " due to form factor.";
+      return false;
+    }
+
     if (!CheckStudyLocale(study.filter(), locale)) {
       DVLOG(1) << "Filtered out study " << study.name() << " due to locale.";
       return false;
diff --git a/components/variations/variations_seed_processor.h b/components/variations/variations_seed_processor.h
index 45a2393..d714943 100644
--- a/components/variations/variations_seed_processor.h
+++ b/components/variations/variations_seed_processor.h
@@ -32,10 +32,12 @@ class VariationsSeedProcessor {
                             const std::string& locale,
                             const base::Time& reference_date,
                             const base::Version& version,
-                            Study_Channel channel);
+                            Study_Channel channel,
+                            Study_FormFactor form_factor);
 
  private:
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest, CheckStudyChannel);
+  FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest, CheckStudyFormFactor);
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest, CheckStudyLocale);
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest, CheckStudyPlatform);
   FRIEND_TEST_ALL_PREFIXES(VariationsSeedProcessorTest, CheckStudyStartDate);
@@ -62,6 +64,7 @@ class VariationsSeedProcessor {
                                 const base::Time& reference_date,
                                 const base::Version& version,
                                 Study_Channel channel,
+                                Study_FormFactor form_factor,
                                 std::vector<ProcessedStudy>* filtered_studies);
 
   // Validates |study| and if valid, adds it to |filtered_studies| as a
@@ -73,6 +76,11 @@ class VariationsSeedProcessor {
   // Checks whether a study is applicable for the given |channel| per |filter|.
   bool CheckStudyChannel(const Study_Filter& filter, Study_Channel channel);
 
+  // Checks whether a study is applicable for the given |form_factor| per
+  // |filter|.
+  bool CheckStudyFormFactor(const Study_Filter& filter,
+                            Study_FormFactor form_factor);
+
   // Checks whether a study is applicable for the given |locale| per |filter|.
   bool CheckStudyLocale(const Study_Filter& filter, const std::string& locale);
 
@@ -102,7 +110,8 @@ class VariationsSeedProcessor {
                       const std::string& locale,
                       const base::Time& reference_date,
                       const base::Version& version,
-                      Study_Channel channel);
+                      Study_Channel channel,
+                      Study_FormFactor form_factor);
 
   // Validates the sanity of |study| and computes the total probability.
   bool ValidateStudyAndComputeTotalProbability(
diff --git a/components/variations/variations_seed_processor_unittest.cc b/components/variations/variations_seed_processor_unittest.cc
index f68bbe6..7b86def 100644
--- a/components/variations/variations_seed_processor_unittest.cc
+++ b/components/variations/variations_seed_processor_unittest.cc
@@ -136,6 +136,56 @@ TEST_F(VariationsSeedProcessorTest, CheckStudyChannel) {
   }
 }
 
+TEST_F(VariationsSeedProcessorTest, CheckStudyFormFactor) {
+  VariationsSeedProcessor seed_processor;
+
+  const Study_FormFactor form_factors[] = {
+    Study_FormFactor_DESKTOP,
+    Study_FormFactor_PHONE,
+    Study_FormFactor_TABLET,
+  };
+
+  ASSERT_EQ(Study_FormFactor_FormFactor_ARRAYSIZE,
+            static_cast<int>(arraysize(form_factors)));
+
+  bool form_factor_added[arraysize(form_factors)] = { 0 };
+  Study_Filter filter;
+
+  for (size_t i = 0; i <= arraysize(form_factors); ++i) {
+    for (size_t j = 0; j < arraysize(form_factors); ++j) {
+      const bool expected = form_factor_added[j] ||
+                            filter.form_factor_size() == 0;
+      const bool result = seed_processor.CheckStudyFormFactor(filter,
+                                                              form_factors[j]);
+      EXPECT_EQ(expected, result) << "Case " << i << "," << j << " failed!";
+    }
+
+    if (i < arraysize(form_factors)) {
+      filter.add_form_factor(form_factors[i]);
+      form_factor_added[i] = true;
+    }
+  }
+
+  // Do the same check in the reverse order.
+  filter.clear_form_factor();
+  memset(&form_factor_added, 0, sizeof(form_factor_added));
+  for (size_t i = 0; i <= arraysize(form_factors); ++i) {
+    for (size_t j = 0; j < arraysize(form_factors); ++j) {
+      const bool expected = form_factor_added[j] ||
+                            filter.form_factor_size() == 0;
+      const bool result = seed_processor.CheckStudyFormFactor(filter,
+                                                              form_factors[j]);
+      EXPECT_EQ(expected, result) << "Case " << i << "," << j << " failed!";
+    }
+
+    if (i < arraysize(form_factors)) {
+      const int index = arraysize(form_factors) - i - 1;;
+      filter.add_form_factor(form_factors[index]);
+      form_factor_added[index] = true;
+    }
+  }
+}
+
 TEST_F(VariationsSeedProcessorTest, CheckStudyLocale) {
   VariationsSeedProcessor seed_processor;
 
@@ -365,7 +415,7 @@ TEST_F(VariationsSeedProcessorTest, FilterAndValidateStudies) {
   std::vector<ProcessedStudy> processed_studies;
   VariationsSeedProcessor().FilterAndValidateStudies(
       seed, "en-CA", base::Time::Now(), base::Version("20.0.0.0"),
-      Study_Channel_STABLE, &processed_studies);
+      Study_Channel_STABLE, Study_FormFactor_DESKTOP, &processed_studies);
 
   // Check that only the first kTrial1Name study was kept.
   ASSERT_EQ(2U, processed_studies.size());
@@ -485,7 +535,8 @@ TEST_F(VariationsSeedProcessorTest,
     base::FieldTrialList field_trial_list(NULL);
     study1->set_expiry_date(TimeToProtoTime(year_ago));
     seed_processor.CreateTrialsFromSeed(seed, "en-CA", base::Time::Now(),
-                                        version, Study_Channel_STABLE);
+                                        version, Study_Channel_STABLE,
+                                        Study_FormFactor_DESKTOP);
     EXPECT_EQ(kGroup1Name, base::FieldTrialList::FindFullName(kTrialName));
   }
 
@@ -496,7 +547,8 @@ TEST_F(VariationsSeedProcessorTest,
     study1->clear_expiry_date();
     study2->set_expiry_date(TimeToProtoTime(year_ago));
     seed_processor.CreateTrialsFromSeed(seed, "en-CA", base::Time::Now(),
-                                        version, Study_Channel_STABLE);
+                                        version, Study_Channel_STABLE,
+                                        Study_FormFactor_DESKTOP);
     EXPECT_EQ(kGroup1Name, base::FieldTrialList::FindFullName(kTrialName));
   }
 }
@@ -638,7 +690,8 @@ TEST_F(VariationsSeedProcessorTest, StartsActive) {
   VariationsSeedProcessor seed_processor;
   seed_processor.CreateTrialsFromSeed(seed, "en-CA", base::Time::Now(),
                                       base::Version("20.0.0.0"),
-                                      Study_Channel_STABLE);
+                                      Study_Channel_STABLE,
+                                      Study_FormFactor_DESKTOP);
 
   // Non-specified and ACTIVATION_EXPLICIT should not start active, but
   // ACTIVATION_AUTO should.