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

Unified Diff: components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc

Issue 2798983002: Introduce subresource_filter::Configuration. (Closed)
Patch Set: Address comments from csharrison@. 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/content/browser/content_subresource_filter_driver_factory.cc
diff --git a/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc b/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc
index 97f23d1ab8fb7b4476f417638f42d31d412578d5..0f737006d5e0bc0ac1db6d2f5153615e119c5f18 100644
--- a/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc
+++ b/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc
@@ -34,14 +34,14 @@ std::string DistillURLToHostAndPath(const GURL& url) {
return url.host() + url.path();
}
-// Returns true with a probability of GetPerformanceMeasurementRate() if
-// ThreadTicks is supported, otherwise returns false.
-bool ShouldMeasurePerformanceForPageLoad() {
+// Returns true with a probability given by |performance_measurement_rate| in
+// |configuration| if ThreadTicks is supported, otherwise returns false.
+bool ShouldMeasurePerformanceForPageLoad(const Configuration& configuration) {
if (!base::ThreadTicks::IsSupported())
return false;
// TODO(pkalinnikov): Cache |rate| and other variation params in
pkalinnikov 2017/04/07 09:22:06 nit: You can now remove this TODO because this CL
engedy 2017/04/07 09:28:29 Done.
// ContentSubresourceFilterDriverFactory.
- const double rate = GetPerformanceMeasurementRate();
+ const double rate = configuration.performance_measurement_rate;
return rate == 1 || (rate > 0 && base::RandDouble() < rate);
}
@@ -91,6 +91,7 @@ ContentSubresourceFilterDriverFactory::ContentSubresourceFilterDriverFactory(
content::WebContents* web_contents,
std::unique_ptr<SubresourceFilterClient> client)
: content::WebContentsObserver(web_contents),
+ configuration_(GetActiveConfiguration()),
client_(std::move(client)),
throttle_manager_(
base::MakeUnique<ContentSubresourceFilterThrottleManager>(
@@ -146,11 +147,10 @@ void ContentSubresourceFilterDriverFactory::AddHostOfURLToWhitelistSet(
ContentSubresourceFilterDriverFactory::ActivationDecision
ContentSubresourceFilterDriverFactory::ComputeActivationDecisionForMainFrameURL(
const GURL& url) const {
- if (GetMaximumActivationLevel() == ActivationLevel::DISABLED)
+ if (configuration_.activation_level == ActivationLevel::DISABLED)
return ActivationDecision::ACTIVATION_DISABLED;
- ActivationScope scope = GetCurrentActivationScope();
- if (scope == ActivationScope::NO_SITES)
+ if (configuration_.activation_scope == ActivationScope::NO_SITES)
return ActivationDecision::ACTIVATION_DISABLED;
if (!url.SchemeIsHTTPOrHTTPS())
@@ -158,7 +158,7 @@ ContentSubresourceFilterDriverFactory::ComputeActivationDecisionForMainFrameURL(
if (IsWhitelisted(url))
return ActivationDecision::URL_WHITELISTED;
- switch (scope) {
+ switch (configuration_.activation_scope) {
case ActivationScope::ALL_SITES:
return ActivationDecision::ACTIVATED;
case ActivationScope::ACTIVATION_LIST: {
@@ -166,10 +166,11 @@ ContentSubresourceFilterDriverFactory::ComputeActivationDecisionForMainFrameURL(
// AddActivationListMatch to ensure the activation list only has relevant
// entries.
DCHECK(url.SchemeIsHTTPOrHTTPS() ||
- !DidURLMatchActivationList(url, GetCurrentActivationList()));
+ !DidURLMatchActivationList(url, configuration_.activation_list));
bool should_activate =
- DidURLMatchActivationList(url, GetCurrentActivationList());
- if (GetCurrentActivationList() == ActivationList::PHISHING_INTERSTITIAL) {
+ DidURLMatchActivationList(url, configuration_.activation_list);
+ if (configuration_.activation_list ==
+ ActivationList::PHISHING_INTERSTITIAL) {
// Handling special case, where activation on the phishing sites also
// mean the activation on the sites with social engineering metadata.
should_activate |= DidURLMatchActivationList(
@@ -213,7 +214,7 @@ void ContentSubresourceFilterDriverFactory::WillProcessResponse(
RecordRedirectChainMatchPattern();
- if (ShouldWhitelistSiteOnReload() &&
+ if (configuration_.should_whitelist_site_on_reload &&
NavigationIsPageReload(url, referrer, transition)) {
// Whitelist this host for the current as well as subsequent navigations.
AddHostOfURLToWhitelistSet(url);
@@ -226,16 +227,16 @@ void ContentSubresourceFilterDriverFactory::WillProcessResponse(
return;
}
- activation_level_ = GetMaximumActivationLevel();
+ activation_level_ = configuration_.activation_level;
measure_performance_ = activation_level_ != ActivationLevel::DISABLED &&
- ShouldMeasurePerformanceForPageLoad();
+ ShouldMeasurePerformanceForPageLoad(configuration_);
ActivationState state = ActivationState(activation_level_);
state.measure_performance = measure_performance_;
throttle_manager_->NotifyPageActivationComputed(navigation_handle, state);
}
void ContentSubresourceFilterDriverFactory::OnFirstSubresourceLoadDisallowed() {
- if (ShouldSuppressNotifications())
+ if (configuration_.should_suppress_notifications)
return;
client_->ToggleNotificationVisibility(activation_level_ ==

Powered by Google App Engine
This is Rietveld 408576698