Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(57)

Unified Diff: components/variations/study_filtering.cc

Issue 2924983003: [Variations] Refactor all state used for study filtering into a container struct. (Closed)
Patch Set: A bit more cleanup Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « components/variations/study_filtering.h ('k') | components/variations/study_filtering_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/variations/study_filtering.cc
diff --git a/components/variations/study_filtering.cc b/components/variations/study_filtering.cc
index 0712c2068fcd9566cffad13d8d65848b634eeea8..e7ccd25888f272b6ad9d4946c33943e3b2c243ff 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,27 @@ bool CheckStudyCountry(const Study_Filter& filter, const std::string& country) {
return !base::ContainsValue(filter.exclude_country(), country);
}
+const std::string& GetClientCountryForStudy(
+ const Study& study,
+ 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 +199,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 = GetClientCountryForStudy(study, client_state);
if (!CheckStudyCountry(study.filter(), country)) {
DVLOG(1) << "Filtered out study " << study.name() << " due to country.";
return false;
@@ -263,16 +259,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 +272,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);
« no previous file with comments | « components/variations/study_filtering.h ('k') | components/variations/study_filtering_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698