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

Unified Diff: components/subresource_filter/core/browser/subresource_filter_features.cc

Issue 2798983002: Introduce subresource_filter::Configuration. (Closed)
Patch Set: Workaround 2. Created 3 years, 8 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
Index: components/subresource_filter/core/browser/subresource_filter_features.cc
diff --git a/components/subresource_filter/core/browser/subresource_filter_features.cc b/components/subresource_filter/core/browser/subresource_filter_features.cc
index acef5adaf65199cd4c78f7f3ddad96d3c9b3ed53..73608745e5ec2213fc0a1c709af895ea093e9d87 100644
--- a/components/subresource_filter/core/browser/subresource_filter_features.cc
+++ b/components/subresource_filter/core/browser/subresource_filter_features.cc
@@ -8,51 +8,23 @@
#include "base/metrics/field_trial_params.h"
#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_piece.h"
#include "base/strings/string_split.h"
#include "base/strings/string_util.h"
#include "components/variations/variations_associated_data.h"
namespace subresource_filter {
-const base::Feature kSafeBrowsingSubresourceFilter{
- "SubresourceFilter", base::FEATURE_DISABLED_BY_DEFAULT};
-
-const base::Feature kSafeBrowsingSubresourceFilterExperimentalUI{
- "SubresourceFilterExperimentalUI", base::FEATURE_DISABLED_BY_DEFAULT};
+namespace {
-const base::Feature kSubresourceFilterSafeBrowsingActivationThrottle{
- "SubresourceFilterSafeBrowsingActivationThrottle",
- base::FEATURE_DISABLED_BY_DEFAULT};
-
-// Legacy name `activation_state` is used in variation parameters.
-const char kActivationLevelParameterName[] = "activation_state";
-const char kActivationLevelDryRun[] = "dryrun";
-const char kActivationLevelEnabled[] = "enabled";
-const char kActivationLevelDisabled[] = "disabled";
-
-const char kActivationScopeParameterName[] = "activation_scope";
-const char kActivationScopeAllSites[] = "all_sites";
-const char kActivationScopeActivationList[] = "activation_list";
-const char kActivationScopeNoSites[] = "no_sites";
-
-const char kActivationListsParameterName[] = "activation_lists";
-const char kActivationListSocialEngineeringAdsInterstitial[] =
- "social_engineering_ads_interstitial";
-const char kActivationListPhishingInterstitial[] = "phishing_interstitial";
-const char kActivationListSubresourceFilter[] = "subresource_filter";
-
-const char kRulesetFlavorParameterName[] = "ruleset_flavor";
-
-const char kPerformanceMeasurementRateParameterName[] =
- "performance_measurement_rate";
-
-const char kSuppressNotificationsParameterName[] = "suppress_notifications";
-
-const char kWhitelistSiteOnReloadParameterName[] = "whitelist_site_on_reload";
+base::StringPiece GetVariationParamOrEmpty(
Charlie Harrison 2017/04/05 20:43:04 A little odd to have this return a non-owning Stri
engedy 2017/04/07 08:40:58 Indeed. I reworked this slightly, what do you thin
+ const std::map<std::string, std::string>& params,
+ const std::string& key) {
+ auto it = params.find(key);
+ return it != params.end() ? it->second : base::StringPiece();
+}
-ActivationLevel GetMaximumActivationLevel() {
- std::string activation_level = variations::GetVariationParamValueByFeature(
- kSafeBrowsingSubresourceFilter, kActivationLevelParameterName);
+ActivationLevel ParseActivationLevel(const base::StringPiece activation_level) {
if (base::LowerCaseEqualsASCII(activation_level, kActivationLevelEnabled))
return ActivationLevel::ENABLED;
else if (base::LowerCaseEqualsASCII(activation_level, kActivationLevelDryRun))
@@ -60,9 +32,7 @@ ActivationLevel GetMaximumActivationLevel() {
return ActivationLevel::DISABLED;
}
-ActivationScope GetCurrentActivationScope() {
- std::string activation_scope = variations::GetVariationParamValueByFeature(
- kSafeBrowsingSubresourceFilter, kActivationScopeParameterName);
+ActivationScope ParseActivationScope(const base::StringPiece activation_scope) {
if (base::LowerCaseEqualsASCII(activation_scope, kActivationScopeAllSites))
return ActivationScope::ALL_SITES;
else if (base::LowerCaseEqualsASCII(activation_scope,
@@ -71,9 +41,7 @@ ActivationScope GetCurrentActivationScope() {
return ActivationScope::NO_SITES;
}
-ActivationList GetCurrentActivationList() {
- std::string activation_lists = variations::GetVariationParamValueByFeature(
- kSafeBrowsingSubresourceFilter, kActivationListsParameterName);
+ActivationList ParseActivationList(const base::StringPiece activation_lists) {
ActivationList activation_list_type = ActivationList::NONE;
for (const base::StringPiece& activation_list :
base::SplitStringPiece(activation_lists, ",", base::TRIM_WHITESPACE,
@@ -93,31 +61,88 @@ ActivationList GetCurrentActivationList() {
return activation_list_type;
}
-double GetPerformanceMeasurementRate() {
- const std::string rate = variations::GetVariationParamValueByFeature(
- kSafeBrowsingSubresourceFilter, kPerformanceMeasurementRateParameterName);
+double ParsePerformanceMeasurementRate(const base::StringPiece rate) {
double value = 0;
- if (!base::StringToDouble(rate, &value) || value < 0)
+ // TODO(engedy): Refactor StringToDouble to take a StringPiece.
+ if (!base::StringToDouble(rate.as_string(), &value) || value < 0)
return 0;
return value < 1 ? value : 1;
}
-bool ShouldSuppressNotifications() {
- return base::GetFieldTrialParamByFeatureAsBool(
- kSafeBrowsingSubresourceFilter, kSuppressNotificationsParameterName,
- false /* default value */);
+bool ParseBool(const base::StringPiece value) {
+ return base::LowerCaseEqualsASCII(value, "true");
}
-std::string GetRulesetFlavor() {
- return variations::GetVariationParamValueByFeature(
- subresource_filter::kSafeBrowsingSubresourceFilter,
- subresource_filter::kRulesetFlavorParameterName);
-}
+} // namespace
+
+const base::Feature kSafeBrowsingSubresourceFilter{
+ "SubresourceFilter", base::FEATURE_DISABLED_BY_DEFAULT};
+
+const base::Feature kSafeBrowsingSubresourceFilterExperimentalUI{
+ "SubresourceFilterExperimentalUI", base::FEATURE_DISABLED_BY_DEFAULT};
+
+const base::Feature kSubresourceFilterSafeBrowsingActivationThrottle{
+ "SubresourceFilterSafeBrowsingActivationThrottle",
+ base::FEATURE_DISABLED_BY_DEFAULT};
+
+// Legacy name `activation_state` is used in variation parameters.
+const char kActivationLevelParameterName[] = "activation_state";
+const char kActivationLevelDryRun[] = "dryrun";
+const char kActivationLevelEnabled[] = "enabled";
+const char kActivationLevelDisabled[] = "disabled";
+
+const char kActivationScopeParameterName[] = "activation_scope";
+const char kActivationScopeAllSites[] = "all_sites";
+const char kActivationScopeActivationList[] = "activation_list";
+const char kActivationScopeNoSites[] = "no_sites";
+
+const char kActivationListsParameterName[] = "activation_lists";
+const char kActivationListSocialEngineeringAdsInterstitial[] =
+ "social_engineering_ads_interstitial";
+const char kActivationListPhishingInterstitial[] = "phishing_interstitial";
+const char kActivationListSubresourceFilter[] = "subresource_filter";
+
+const char kRulesetFlavorParameterName[] = "ruleset_flavor";
+
+const char kPerformanceMeasurementRateParameterName[] =
+ "performance_measurement_rate";
+
+const char kSuppressNotificationsParameterName[] = "suppress_notifications";
+
+const char kWhitelistSiteOnReloadParameterName[] = "whitelist_site_on_reload";
+
+Configuration::Configuration() = default;
+Configuration::~Configuration() = default;
+
+Configuration GetActiveConfiguration() {
+ Configuration active_configuration;
+
+ std::map<std::string, std::string> params;
+ base::GetFieldTrialParamsByFeature(kSafeBrowsingSubresourceFilter, &params);
+
+ active_configuration.activation_level = ParseActivationLevel(
+ GetVariationParamOrEmpty(params, kActivationLevelParameterName));
+
+ active_configuration.activation_scope = ParseActivationScope(
+ GetVariationParamOrEmpty(params, kActivationScopeParameterName));
+
+ active_configuration.activation_list = ParseActivationList(
+ GetVariationParamOrEmpty(params, kActivationListsParameterName));
+
+ active_configuration.performance_measurement_rate =
+ ParsePerformanceMeasurementRate(GetVariationParamOrEmpty(
+ params, kPerformanceMeasurementRateParameterName));
+
+ active_configuration.should_suppress_notifications = ParseBool(
+ GetVariationParamOrEmpty(params, kSuppressNotificationsParameterName));
+
+ active_configuration.ruleset_flavor =
+ GetVariationParamOrEmpty(params, kRulesetFlavorParameterName).as_string();
+
+ active_configuration.should_whitelist_site_on_reload = ParseBool(
+ GetVariationParamOrEmpty(params, kWhitelistSiteOnReloadParameterName));
-bool ShouldWhitelistSiteOnReload() {
- return base::GetFieldTrialParamByFeatureAsBool(
- kSafeBrowsingSubresourceFilter, kWhitelistSiteOnReloadParameterName,
- false /* default value */);
+ return active_configuration;
}
} // namespace subresource_filter

Powered by Google App Engine
This is Rietveld 408576698