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

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

Issue 2889913002: [subresource_filter] Remove Forwarding NavigationThrottles (Closed)
Patch Set: shivanisha review 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 15368f6e513fc29968a5f68dacc0806648ee60e5..18112bd3937a69c650af2344ced5fc1346be57ce 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
@@ -17,7 +17,6 @@
#include "components/subresource_filter/core/common/activation_list.h"
#include "components/subresource_filter/core/common/activation_state.h"
#include "content/public/browser/navigation_handle.h"
-#include "content/public/browser/navigation_throttle.h"
#include "content/public/browser/web_contents.h"
#include "net/base/net_errors.h"
#include "url/gurl.h"
@@ -29,10 +28,6 @@ namespace subresource_filter {
namespace {
-std::string DistillURLToHostAndPath(const GURL& url) {
- return url.host() + url.path();
-}
-
// Returns true with a probability given by |performance_measurement_rate| if
// ThreadTicks is supported, otherwise returns false.
bool ShouldMeasurePerformanceForPageLoad(double performance_measurement_rate) {
@@ -80,18 +75,62 @@ ContentSubresourceFilterDriverFactory::ContentSubresourceFilterDriverFactory(
ContentSubresourceFilterDriverFactory::
~ContentSubresourceFilterDriverFactory() {}
-void ContentSubresourceFilterDriverFactory::
- OnMainResourceMatchedSafeBrowsingBlacklist(
- const GURL& url,
- safe_browsing::SBThreatType threat_type,
- safe_browsing::ThreatPatternType threat_type_metadata) {
- AddActivationListMatch(
- url, GetListForThreatTypeAndMetadata(threat_type, threat_type_metadata));
+void ContentSubresourceFilterDriverFactory::OnSafeBrowsingMatchComputed(
+ content::NavigationHandle* navigation_handle,
+ safe_browsing::SBThreatType threat_type,
+ safe_browsing::ThreatPatternType threat_type_metadata) {
+ DCHECK(navigation_handle->IsInMainFrame());
+ DCHECK(!navigation_handle->IsSameDocument());
+ if (navigation_handle->GetNetErrorCode() != net::OK)
+ return;
+
+ ActivationList activation_list =
+ GetListForThreatTypeAndMetadata(threat_type, threat_type_metadata);
+ const GURL& url = navigation_handle->GetURL();
+ const content::Referrer& referrer = navigation_handle->GetReferrer();
+ ui::PageTransition transition = navigation_handle->GetPageTransition();
+
+ 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);
+ }
+
+ ComputeActivationForMainFrameNavigation(navigation_handle, activation_list);
+ DCHECK_NE(activation_decision_, ActivationDecision::UNKNOWN);
+
+ // Check for whitelisted status last, so that the client gets an accurate
+ // indication of whether there would be activation otherwise.
+ bool whitelisted = client_->OnPageActivationComputed(
+ navigation_handle,
+ activation_options_.activation_level == ActivationLevel::ENABLED);
+
+ // Only reset the activation decision reason if we would have activated.
+ if (whitelisted && activation_decision_ == ActivationDecision::ACTIVATED) {
+ TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("loading"), "ActivationWhitelisted");
+ activation_decision_ = ActivationDecision::URL_WHITELISTED;
+ activation_options_ = Configuration::ActivationOptions();
+ }
+
+ if (activation_decision_ != ActivationDecision::ACTIVATED) {
+ DCHECK_EQ(activation_options_.activation_level, ActivationLevel::DISABLED);
+ return;
+ }
+
+ DCHECK_NE(activation_options_.activation_level, ActivationLevel::DISABLED);
+ ActivationState state = ActivationState(activation_options_.activation_level);
+ state.measure_performance = ShouldMeasurePerformanceForPageLoad(
+ activation_options_.performance_measurement_rate);
+ // TODO(csharrison): Set state.enable_logging based on metadata returns from
+ // the safe browsing filter, when it is available. Add tests for this
+ // behavior.
+ throttle_manager_->NotifyPageActivationComputed(navigation_handle, state);
}
void ContentSubresourceFilterDriverFactory::
ComputeActivationForMainFrameNavigation(
- content::NavigationHandle* navigation_handle) {
+ content::NavigationHandle* navigation_handle,
+ ActivationList matched_list) {
const GURL& url(navigation_handle->GetURL());
if (!url.SchemeIsHTTPOrHTTPS()) {
@@ -104,9 +143,9 @@ void ContentSubresourceFilterDriverFactory::
const auto highest_priority_activated_config =
std::find_if(config_list->configs_by_decreasing_priority().begin(),
config_list->configs_by_decreasing_priority().end(),
- [&url, this](const Configuration& config) {
+ [&url, matched_list, this](const Configuration& config) {
return DoesMainFrameURLSatisfyActivationConditions(
- url, config.activation_conditions);
+ url, config.activation_conditions, matched_list);
});
bool has_activated_config =
@@ -135,16 +174,16 @@ void ContentSubresourceFilterDriverFactory::
bool ContentSubresourceFilterDriverFactory::
DoesMainFrameURLSatisfyActivationConditions(
const GURL& url,
- const Configuration::ActivationConditions& conditions) const {
+ const Configuration::ActivationConditions& conditions,
+ ActivationList matched_list) const {
switch (conditions.activation_scope) {
case ActivationScope::ALL_SITES:
return true;
case ActivationScope::ACTIVATION_LIST:
- if (DidURLMatchActivationList(url, conditions.activation_list))
+ if (conditions.activation_list == matched_list)
return true;
if (conditions.activation_list == ActivationList::PHISHING_INTERSTITIAL &&
- DidURLMatchActivationList(
- url, ActivationList::SOCIAL_ENG_ADS_INTERSTITIAL)) {
+ matched_list == ActivationList::SOCIAL_ENG_ADS_INTERSTITIAL) {
// Handling special case, where activation on the phishing sites also
// mean the activation on the sites with social engineering metadata.
return true;
@@ -173,55 +212,6 @@ void ContentSubresourceFilterDriverFactory::OnReloadRequested() {
web_contents()->GetController().Reload(content::ReloadType::NORMAL, true);
}
-void ContentSubresourceFilterDriverFactory::WillProcessResponse(
- content::NavigationHandle* navigation_handle) {
- DCHECK(!navigation_handle->IsSameDocument());
- if (!navigation_handle->IsInMainFrame() ||
- navigation_handle->GetNetErrorCode() != net::OK) {
- return;
- }
-
- const GURL& url = navigation_handle->GetURL();
- const content::Referrer& referrer = navigation_handle->GetReferrer();
- ui::PageTransition transition = navigation_handle->GetPageTransition();
-
- 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);
- }
-
- ComputeActivationForMainFrameNavigation(navigation_handle);
- DCHECK_NE(activation_decision_, ActivationDecision::UNKNOWN);
-
- // Check for whitelisted status last, so that the client gets an accurate
- // indication of whether there would be activation otherwise.
- bool whitelisted = client_->OnPageActivationComputed(
- navigation_handle,
- activation_options_.activation_level == ActivationLevel::ENABLED);
-
- // Only reset the activation decision reason if we would have activated.
- if (whitelisted && activation_decision_ == ActivationDecision::ACTIVATED) {
- TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("loading"), "ActivationWhitelisted");
- activation_decision_ = ActivationDecision::URL_WHITELISTED;
- activation_options_ = Configuration::ActivationOptions();
- }
-
- if (activation_decision_ != ActivationDecision::ACTIVATED) {
- DCHECK_EQ(activation_options_.activation_level, ActivationLevel::DISABLED);
- return;
- }
-
- DCHECK_NE(activation_options_.activation_level, ActivationLevel::DISABLED);
- ActivationState state = ActivationState(activation_options_.activation_level);
- state.measure_performance = ShouldMeasurePerformanceForPageLoad(
- activation_options_.performance_measurement_rate);
- // TODO(csharrison): Set state.enable_logging based on metadata returns from
- // the safe browsing filter, when it is available. Add tests for this
- // behavior.
- throttle_manager_->NotifyPageActivationComputed(navigation_handle, state);
-}
-
void ContentSubresourceFilterDriverFactory::OnFirstSubresourceLoadDisallowed() {
if (activation_options_.should_suppress_notifications)
return;
@@ -234,7 +224,6 @@ void ContentSubresourceFilterDriverFactory::DidStartNavigation(
if (navigation_handle->IsInMainFrame() &&
!navigation_handle->IsSameDocument()) {
activation_decision_ = ActivationDecision::UNKNOWN;
- activation_list_matches_.clear();
client_->ToggleNotificationVisibility(false);
}
}
@@ -250,22 +239,4 @@ void ContentSubresourceFilterDriverFactory::DidFinishNavigation(
}
}
-bool ContentSubresourceFilterDriverFactory::DidURLMatchActivationList(
- const GURL& url,
- ActivationList activation_list) const {
- auto match_types =
- activation_list_matches_.find(DistillURLToHostAndPath(url));
- return match_types != activation_list_matches_.end() &&
- match_types->second.find(activation_list) != match_types->second.end();
-}
-
-void ContentSubresourceFilterDriverFactory::AddActivationListMatch(
- const GURL& url,
- ActivationList match_type) {
- if (match_type == ActivationList::NONE)
- return;
- if (url.has_host() && url.SchemeIsHTTPOrHTTPS())
- activation_list_matches_[DistillURLToHostAndPath(url)].insert(match_type);
-}
-
} // namespace subresource_filter

Powered by Google App Engine
This is Rietveld 408576698