[Variations] Refactor all state used for study filtering into a container struct.

components/variations/study_filtering.cc

Index: components/variations/study_filtering.cc
diff --git a/components/variations/study_filtering.cc b/components/variations/study_filtering.cc
index 0712c2068fcd9566cffad13d8d65848b634eeea8..5fd4b44f456809c63097617f11b6766faeffb49b 100644
--- a/components/variations/study_filtering.cc
+++ b/components/variations/study_filtering.cc
@@ -10,32 +10,12 @@
 #include <set>
 
 #include "base/stl_util.h"
-#include "build/build_config.h"
+#include "base/strings/string_util.h"
+#include "components/variations/client_filterable_state.h"
 
 namespace variations {
-
 namespace {
 
-Study_Platform GetCurrentPlatform() {
-#if defined(OS_WIN)
-  return Study_Platform_PLATFORM_WINDOWS;
-#elif defined(OS_IOS)
-  return Study_Platform_PLATFORM_IOS;
-#elif defined(OS_MACOSX)
-  return Study_Platform_PLATFORM_MAC;
-#elif defined(OS_CHROMEOS)
-  return Study_Platform_PLATFORM_CHROMEOS;
-#elif defined(OS_ANDROID)
-  return Study_Platform_PLATFORM_ANDROID;
-#elif defined(OS_LINUX) || defined(OS_BSD) || defined(OS_SOLARIS)
-  // Default BSD and SOLARIS to Linux to not break those builds, although these
-  // platforms are not officially supported by Chrome.
-  return Study_Platform_PLATFORM_LINUX;
-#else
-#error Unknown platform
-#endif
-}
-
 // Converts |date_time| in Study date format to base::Time.
 base::Time ConvertStudyDateToBaseTime(int64_t date_time) {
   return base::Time::UnixEpoch() + base::TimeDelta::FromSeconds(date_time);
@@ -45,7 +25,7 @@ base::Time ConvertStudyDateToBaseTime(int64_t date_time) {
 
 namespace internal {
 
-bool CheckStudyChannel(const Study_Filter& filter, Study_Channel channel) {
+bool CheckStudyChannel(const Study::Filter& filter, Study::Channel channel) {
   // An empty channel list matches all channels.
   if (filter.channel_size() == 0)
     return true;
@@ -57,8 +37,8 @@ bool CheckStudyChannel(const Study_Filter& filter, Study_Channel channel) {
   return false;
 }
 
-bool CheckStudyFormFactor(const Study_Filter& filter,
-                          Study_FormFactor form_factor) {
+bool CheckStudyFormFactor(const Study::Filter& filter,
+                          Study::FormFactor form_factor) {
   // Empty whitelist and blacklist signifies matching any form factor.
   if (filter.form_factor_size() == 0 && filter.exclude_form_factor_size() == 0)
     return true;
@@ -74,7 +54,7 @@ bool CheckStudyFormFactor(const Study_Filter& filter,
   return !base::ContainsValue(filter.exclude_form_factor(), form_factor);
 }
 
-bool CheckStudyHardwareClass(const Study_Filter& filter,
+bool CheckStudyHardwareClass(const Study::Filter& filter,
                              const std::string& hardware_class) {
   // Empty hardware_class and exclude_hardware_class matches all.
   if (filter.hardware_class_size() == 0 &&
@@ -109,7 +89,7 @@ bool CheckStudyHardwareClass(const Study_Filter& filter,
   return true;
 }
 
-bool CheckStudyLocale(const Study_Filter& filter, const std::string& locale) {
+bool CheckStudyLocale(const Study::Filter& filter, const std::string& locale) {
   // Empty locale and exclude_locale lists matches all locales.
   if (filter.locale_size() == 0 && filter.exclude_locale_size() == 0)
     return true;
@@ -124,7 +104,7 @@ bool CheckStudyLocale(const Study_Filter& filter, const std::string& locale) {
   return !base::ContainsValue(filter.exclude_locale(), locale);
 }
 
-bool CheckStudyPlatform(const Study_Filter& filter, Study_Platform platform) {
+bool CheckStudyPlatform(const Study::Filter& filter, Study::Platform platform) {
   // An empty platform list matches all platforms.
   if (filter.platform_size() == 0)
     return true;
@@ -136,7 +116,7 @@ bool CheckStudyPlatform(const Study_Filter& filter, Study_Platform platform) {
   return false;
 }
 
-bool CheckStudyStartDate(const Study_Filter& filter,
+bool CheckStudyStartDate(const Study::Filter& filter,
                          const base::Time& date_time) {
   if (filter.has_start_date()) {
     const base::Time start_date =
@@ -147,7 +127,7 @@ bool CheckStudyStartDate(const Study_Filter& filter,
   return true;
 }
 
-bool CheckStudyEndDate(const Study_Filter& filter,
+bool CheckStudyEndDate(const Study::Filter& filter,
                        const base::Time& date_time) {
   if (filter.has_end_date()) {
     const base::Time end_date = ConvertStudyDateToBaseTime(filter.end_date());
@@ -157,7 +137,7 @@ bool CheckStudyEndDate(const Study_Filter& filter,
   return true;
 }
 
-bool CheckStudyVersion(const Study_Filter& filter,
+bool CheckStudyVersion(const Study::Filter& filter,
                        const base::Version& version) {
   if (filter.has_min_version()) {
     if (version.CompareToWildcardString(filter.min_version()) < 0)
@@ -172,7 +152,8 @@ bool CheckStudyVersion(const Study_Filter& filter,
   return true;
 }
 
-bool CheckStudyCountry(const Study_Filter& filter, const std::string& country) {
+bool CheckStudyCountry(const Study::Filter& filter,
+                       const std::string& country) {
   // Empty country and exclude_country matches all.
   if (filter.country_size() == 0 && filter.exclude_country_size() == 0)
     return true;
@@ -187,6 +168,26 @@ bool CheckStudyCountry(const Study_Filter& filter, const std::string& country) {
   return !base::ContainsValue(filter.exclude_country(), country);
 }
 
+const std::string& GetStudyCountry(const Study& study,

[Alexei Svitkine (slow), 2017/06/07 15:18:08]

Nit: I think name is misleading - since this is getting the client state country
for filtering the study.

How about GetClientStateCountryForStudy()?

[Ilya Sherman, 2017/06/07 21:50:08]

Done.  (I dropped "State" though, i.e. GetClientCountryForStudy().)

+                                   const ClientFilterableState& client_state) {
+  switch (study.consistency()) {
+    case Study::SESSION:
+      return client_state.session_consistency_country;
+    case Study::PERMANENT:
+      // Use the saved country for permanent consistency studies. This allows
+      // Chrome to use the same country for filtering permanent consistency
+      // studies between Chrome upgrades. Since some studies have user-visible
+      // effects, this helps to avoid annoying users with experimental group
+      // churn while traveling.
+      return client_state.permanent_consistency_country;
+  }
+
+  // Unless otherwise specified, use an empty country that won't pass any
+  // filters that specifically include countries, but will pass any filters
+  // that specifically exclude countries.
+  return base::EmptyString();
+}
+
 bool IsStudyExpired(const Study& study, const base::Time& date_time) {
   if (study.has_expiry_date()) {
     const base::Time expiry_date =
@@ -197,59 +198,53 @@ bool IsStudyExpired(const Study& study, const base::Time& date_time) {
   return false;
 }
 
-bool ShouldAddStudy(
-    const Study& study,
-    const std::string& locale,
-    const base::Time& reference_date,
-    const base::Version& version,
-    Study_Channel channel,
-    Study_FormFactor form_factor,
-    const std::string& hardware_class,
-    const std::string& country) {
+bool ShouldAddStudy(const Study& study,
+                    const ClientFilterableState& client_state) {
   if (study.has_filter()) {
-    if (!CheckStudyChannel(study.filter(), channel)) {
+    if (!CheckStudyChannel(study.filter(), client_state.channel)) {
       DVLOG(1) << "Filtered out study " << study.name() << " due to channel.";
       return false;
     }
 
-    if (!CheckStudyFormFactor(study.filter(), form_factor)) {
+    if (!CheckStudyFormFactor(study.filter(), client_state.form_factor)) {
       DVLOG(1) << "Filtered out study " << study.name() <<
                   " due to form factor.";
       return false;
     }
 
-    if (!CheckStudyLocale(study.filter(), locale)) {
+    if (!CheckStudyLocale(study.filter(), client_state.locale)) {
       DVLOG(1) << "Filtered out study " << study.name() << " due to locale.";
       return false;
     }
 
-    if (!CheckStudyPlatform(study.filter(), GetCurrentPlatform())) {
+    if (!CheckStudyPlatform(study.filter(), client_state.platform)) {
       DVLOG(1) << "Filtered out study " << study.name() << " due to platform.";
       return false;
     }
 
-    if (!CheckStudyVersion(study.filter(), version)) {
+    if (!CheckStudyVersion(study.filter(), client_state.version)) {
       DVLOG(1) << "Filtered out study " << study.name() << " due to version.";
       return false;
     }
 
-    if (!CheckStudyStartDate(study.filter(), reference_date)) {
+    if (!CheckStudyStartDate(study.filter(), client_state.reference_date)) {
       DVLOG(1) << "Filtered out study " << study.name() <<
                   " due to start date.";
       return false;
     }
 
-    if (!CheckStudyEndDate(study.filter(), reference_date)) {
+    if (!CheckStudyEndDate(study.filter(), client_state.reference_date)) {
       DVLOG(1) << "Filtered out study " << study.name() << " due to end date.";
       return false;
     }
 
-    if (!CheckStudyHardwareClass(study.filter(), hardware_class)) {
+    if (!CheckStudyHardwareClass(study.filter(), client_state.hardware_class)) {
       DVLOG(1) << "Filtered out study " << study.name() <<
                   " due to hardware_class.";
       return false;
     }
 
+    const std::string& country = GetStudyCountry(study, client_state);
     if (!CheckStudyCountry(study.filter(), country)) {
       DVLOG(1) << "Filtered out study " << study.name() << " due to country.";
       return false;
@@ -263,16 +258,9 @@ bool ShouldAddStudy(
 }  // namespace internal
 
 void FilterAndValidateStudies(const VariationsSeed& seed,
-                              const std::string& locale,
-                              const base::Time& reference_date,
-                              const base::Version& version,
-                              Study_Channel channel,
-                              Study_FormFactor form_factor,
-                              const std::string& hardware_class,
-                              const std::string& session_consistency_country,
-                              const std::string& permanent_consistency_country,
+                              const ClientFilterableState& client_state,
                               std::vector<ProcessedStudy>* filtered_studies) {
-  DCHECK(version.IsValid());
+  DCHECK(client_state.version.IsValid());
 
   // Add expired studies (in a disabled state) only after all the non-expired
   // studies have been added (and do not add an expired study if a corresponding
@@ -283,32 +271,10 @@ void FilterAndValidateStudies(const VariationsSeed& seed,
 
   for (int i = 0; i < seed.study_size(); ++i) {
     const Study& study = seed.study(i);
-
-    // Unless otherwise specified, use an empty country that won't pass any
-    // filters that specifically include countries, but will pass any filters
-    // that specifically exclude countries.
-    std::string country;
-    switch (study.consistency()) {
-      case Study_Consistency_SESSION:
-        country = session_consistency_country;
-        break;
-      case Study_Consistency_PERMANENT:
-        // Use the saved |permanent_consistency_country| for permanent
-        // consistency studies. This allows Chrome to use the same country for
-        // filtering permanent consistency studies between Chrome upgrades.
-        // Since some studies have user-visible effects, this helps to avoid
-        // annoying users with experimental group churn while traveling.
-        country = permanent_consistency_country;
-        break;
-    }
-
-    if (!internal::ShouldAddStudy(study, locale, reference_date, version,
-                                  channel, form_factor, hardware_class,
-                                  country)) {
+    if (!internal::ShouldAddStudy(study, client_state))
       continue;
-    }
 
-    if (internal::IsStudyExpired(study, reference_date)) {
+    if (internal::IsStudyExpired(study, client_state.reference_date)) {
       expired_studies.push_back(&study);
     } else if (!base::ContainsKey(created_studies, study.name())) {
       ProcessedStudy::ValidateAndAppendStudy(&study, false, filtered_studies);