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

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

Issue 2844063002: Add support for multiple simultaneous subresource_filter::Configurations. (Closed)
Patch Set: Fix compile error. Created 3 years, 7 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 4ade5b930005f6f2513fc8a72e62c3a4bffa1faa..88957b6bec7a56f2a1578bd3083625f8d0b79a47 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
@@ -84,10 +84,7 @@ ContentSubresourceFilterDriverFactory::ContentSubresourceFilterDriverFactory(
base::MakeUnique<ContentSubresourceFilterThrottleManager>(
this,
client_->GetRulesetDealer(),
- web_contents)),
- activation_level_(ActivationLevel::DISABLED),
- activation_decision_(ActivationDecision::UNKNOWN),
- measure_performance_(false) {}
+ web_contents)) {}
ContentSubresourceFilterDriverFactory::
~ContentSubresourceFilterDriverFactory() {}
@@ -102,30 +99,52 @@ void ContentSubresourceFilterDriverFactory::
url, GetListForThreatTypeAndMetadata(threat_type, threat_type_metadata));
}
+void ContentSubresourceFilterDriverFactory::
+ ComputeActivationForMainFrameNavigation(
+ content::NavigationHandle* navigation_handle) {
+ activation_decision_ = ActivationDecision::ACTIVATION_DISABLED;
+ activation_options_ = Configuration::ActivationOptions();
+ const auto config_list = GetEnabledConfigurations();
+ for (const auto& config : config_list->configs_by_decreasing_priority()) {
+ ActivationDecision decision =
+ ComputePerConfigActivationDecisionForMainFrameNavigation(
+ navigation_handle, config);
+ if (decision == ActivationDecision::ACTIVATED) {
Charlie Harrison 2017/05/05 13:12:52 As Pavel pointed out I think the main problem here
engedy 2017/05/05 13:21:23 Ah, I slightly misunderstood the concern previousl
+ activation_decision_ = ActivationDecision::ACTIVATED;
+ activation_options_ = config.activation_options;
+ return;
+ }
+
+ // Remember a |decision| more specific than `ACTIVATION_DISABLED`, if any.
+ if (decision != ActivationDecision::ACTIVATION_DISABLED)
+ activation_decision_ = decision;
+ }
+}
+
ContentSubresourceFilterDriverFactory::ActivationDecision
ContentSubresourceFilterDriverFactory::
- ComputeActivationDecisionForMainFrameNavigation(
- content::NavigationHandle* navigation_handle) const {
+ ComputePerConfigActivationDecisionForMainFrameNavigation(
+ content::NavigationHandle* navigation_handle,
+ const Configuration& configuration) const {
const GURL& url(navigation_handle->GetURL());
-
- const auto configurations = GetActiveConfigurations();
- if (configurations->the_one_and_only().activation_level ==
+ if (configuration.activation_options.activation_level ==
ActivationLevel::DISABLED)
return ActivationDecision::ACTIVATION_DISABLED;
- if (configurations->the_one_and_only().activation_scope ==
+ if (configuration.activation_conditions.activation_scope ==
ActivationScope::NO_SITES)
return ActivationDecision::ACTIVATION_DISABLED;
if (!url.SchemeIsHTTPOrHTTPS())
return ActivationDecision::UNSUPPORTED_SCHEME;
+
// TODO(csharrison): The throttle manager also performs this check. Remove
// this one when the activation decision is sent directly to the throttle
// manager.
if (client_->ShouldSuppressActivation(navigation_handle))
return ActivationDecision::URL_WHITELISTED;
- switch (configurations->the_one_and_only().activation_scope) {
+ switch (configuration.activation_conditions.activation_scope) {
case ActivationScope::ALL_SITES:
return ActivationDecision::ACTIVATED;
case ActivationScope::ACTIVATION_LIST: {
@@ -134,10 +153,10 @@ ContentSubresourceFilterDriverFactory::
// entries.
DCHECK(url.SchemeIsHTTPOrHTTPS() ||
!DidURLMatchActivationList(
- url, configurations->the_one_and_only().activation_list));
+ url, configuration.activation_conditions.activation_list));
bool should_activate = DidURLMatchActivationList(
- url, configurations->the_one_and_only().activation_list);
- if (configurations->the_one_and_only().activation_list ==
+ url, configuration.activation_conditions.activation_list);
+ if (configuration.activation_conditions.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.
@@ -182,37 +201,30 @@ void ContentSubresourceFilterDriverFactory::WillProcessResponse(
RecordRedirectChainMatchPattern();
- const auto configurations = GetActiveConfigurations();
- if (configurations->the_one_and_only().should_whitelist_site_on_reload &&
+ if (activation_options_.should_whitelist_site_on_reload &&
NavigationIsPageReload(url, referrer, transition)) {
// Whitelist this host for the current as well as subsequent navigations.
client_->WhitelistInCurrentWebContents(url);
}
- activation_decision_ =
- ComputeActivationDecisionForMainFrameNavigation(navigation_handle);
- DCHECK(activation_decision_ != ActivationDecision::UNKNOWN);
+ ComputeActivationForMainFrameNavigation(navigation_handle);
+ DCHECK_NE(activation_decision_, ActivationDecision::UNKNOWN);
if (activation_decision_ != ActivationDecision::ACTIVATED) {
- ResetActivationState();
+ DCHECK_EQ(activation_options_.activation_level, ActivationLevel::DISABLED);
return;
}
- activation_level_ = configurations->the_one_and_only().activation_level;
- measure_performance_ =
- activation_level_ != ActivationLevel::DISABLED &&
- ShouldMeasurePerformanceForPageLoad(
- configurations->the_one_and_only().performance_measurement_rate);
- ActivationState state = ActivationState(activation_level_);
- state.measure_performance = measure_performance_;
+ DCHECK_NE(activation_options_.activation_level, ActivationLevel::DISABLED);
+ ActivationState state = ActivationState(activation_options_.activation_level);
+ state.measure_performance = ShouldMeasurePerformanceForPageLoad(
+ activation_options_.performance_measurement_rate);
throttle_manager_->NotifyPageActivationComputed(navigation_handle, state);
}
void ContentSubresourceFilterDriverFactory::OnFirstSubresourceLoadDisallowed() {
- const auto configurations = GetActiveConfigurations();
- if (configurations->the_one_and_only().should_suppress_notifications)
+ if (activation_options_.should_suppress_notifications)
return;
-
- client_->ToggleNotificationVisibility(activation_level_ ==
+ client_->ToggleNotificationVisibility(activation_options_.activation_level ==
ActivationLevel::ENABLED);
}
@@ -221,20 +233,13 @@ bool ContentSubresourceFilterDriverFactory::ShouldSuppressActivation(
return client_->ShouldSuppressActivation(navigation_handle);
}
-void ContentSubresourceFilterDriverFactory::ResetActivationState() {
- navigation_chain_.clear();
- activation_list_matches_.clear();
- activation_level_ = ActivationLevel::DISABLED;
- measure_performance_ = false;
-}
-
void ContentSubresourceFilterDriverFactory::DidStartNavigation(
content::NavigationHandle* navigation_handle) {
if (navigation_handle->IsInMainFrame() &&
!navigation_handle->IsSameDocument()) {
activation_decision_ = ActivationDecision::UNKNOWN;
- ResetActivationState();
- navigation_chain_.push_back(navigation_handle->GetURL());
+ activation_list_matches_.clear();
+ navigation_chain_ = {navigation_handle->GetURL()};
client_->ToggleNotificationVisibility(false);
}
}
@@ -246,6 +251,17 @@ void ContentSubresourceFilterDriverFactory::DidRedirectNavigation(
navigation_chain_.push_back(navigation_handle->GetURL());
}
+void ContentSubresourceFilterDriverFactory::DidFinishNavigation(
+ content::NavigationHandle* navigation_handle) {
+ if (navigation_handle->IsInMainFrame() &&
+ !navigation_handle->IsSameDocument() &&
+ activation_decision_ == ActivationDecision::UNKNOWN &&
+ navigation_handle->HasCommitted()) {
+ activation_decision_ = ActivationDecision::ACTIVATION_DISABLED;
+ activation_options_ = Configuration::ActivationOptions();
+ }
+}
+
bool ContentSubresourceFilterDriverFactory::DidURLMatchActivationList(
const GURL& url,
ActivationList activation_list) const {

Powered by Google App Engine
This is Rietveld 408576698