Commit 5b3ed853 authored by Rainhard Findling's avatar Rainhard Findling Committed by Chromium LUCI CQ

Revert "[Subresource Filter] Add content settings-based adjustment to WebLayer"

This reverts commit db11bed9.

Reason for revert: 1165728

Original change's description:
> [Subresource Filter] Add content settings-based adjustment to WebLayer
>
> This CL incorporates Chrome's adjustment of subresource filter page
> activation decisions based on content settings within WebLayer. To do
> so, we have the shared SubresourceFilterSafeBrowsingActivationThrottle
> directly invoke the logic in question rather than having
> ChromeSubresourceFilterClient do so. That change necessitates some
> others, also made in this CL:
> - WebLayer brings up ProfileInteractionManager.
> - SubresourceFilterClient provides an accessor for
>   ProfileInteractionManager.
> - SubresourceFilterSafeBrowsingActivationThrottle calls the adjustment
>   logic via a new Delegate interface rather than doing so via
>   SubresourceFilterClient. This interface is implemented by
>   ProfileInteractionManager in production while allowing for porting of
>   tests that provide custom (or nil) stub logic.
>
> We also add browsertests of this interaction in //weblayer.
>
> Note that this CL does not do any incorporation of the relevant *UI*
> for content/site settings of ad blocking in WebLayer; that will be
> followup work.
>
> This CL is a reland. The original CL was reverted as the introduced
> tests were flaky. I have since debugged the source of the flake; the
> fix is contained in the diff between PS1 and PS2. It turned out to be
> a pre-existing problem in //weblayer's
> subresource_filter_browsertest.cc that the new tests simply tickled:
> I was holding the TestRulesetCreator in a local variable and sending
> the temporary files that it creates to renderer processes, but it
> destroys those temporary files when it is destroyed. D'oh!
>
> Bug: 1116095
> Change-Id: I25f5dca647761f652c9d5a2dfc41db1a4d7712b5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2622215
> Auto-Submit: Colin Blundell <blundell@chromium.org>
> Reviewed-by: Charlie Harrison <csharrison@chromium.org>
> Commit-Queue: Colin Blundell <blundell@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#842429}

TBR=blundell@chromium.org,csharrison@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

Change-Id: Ic15653af6a972ec0f8e8e66d274bd99aeb8f6179
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1116095
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2621758Reviewed-by: default avatarRainhard Findling <rainhard@chromium.org>
Commit-Queue: Rainhard Findling <rainhard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842485}
parent 566f8e6e
...@@ -29,6 +29,7 @@ ...@@ -29,6 +29,7 @@
#include "components/subresource_filter/core/common/activation_decision.h" #include "components/subresource_filter/core/common/activation_decision.h"
#include "components/subresource_filter/core/common/activation_scope.h" #include "components/subresource_filter/core/common/activation_scope.h"
#include "components/subresource_filter/core/mojom/subresource_filter.mojom.h" #include "components/subresource_filter/core/mojom/subresource_filter.mojom.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "services/metrics/public/cpp/ukm_source_id.h" #include "services/metrics/public/cpp/ukm_source_id.h"
...@@ -97,6 +98,18 @@ void ChromeSubresourceFilterClient::ShowNotification() { ...@@ -97,6 +98,18 @@ void ChromeSubresourceFilterClient::ShowNotification() {
} }
} }
subresource_filter::mojom::ActivationLevel
ChromeSubresourceFilterClient::OnPageActivationComputed(
content::NavigationHandle* navigation_handle,
subresource_filter::mojom::ActivationLevel initial_activation_level,
subresource_filter::ActivationDecision* decision) {
// TODO(crbug.com/1116095): Once SafeBrowsingActivationThrottle knows about
// ProfileInteractionManager, it can invoke ProfileInteractionManager directly
// and SubresourceFilterClient::OnPageActivationComputed() can be eliminated.
return profile_interaction_manager_->OnPageActivationComputed(
navigation_handle, initial_activation_level, decision);
}
void ChromeSubresourceFilterClient::OnAdsViolationTriggered( void ChromeSubresourceFilterClient::OnAdsViolationTriggered(
content::RenderFrameHost* rfh, content::RenderFrameHost* rfh,
subresource_filter::mojom::AdsViolation triggered_violation) { subresource_filter::mojom::AdsViolation triggered_violation) {
...@@ -116,11 +129,6 @@ ChromeSubresourceFilterClient::GetSafeBrowsingDatabaseManager() { ...@@ -116,11 +129,6 @@ ChromeSubresourceFilterClient::GetSafeBrowsingDatabaseManager() {
: nullptr; : nullptr;
} }
subresource_filter::ProfileInteractionManager*
ChromeSubresourceFilterClient::GetProfileInteractionManager() {
return profile_interaction_manager_.get();
}
void ChromeSubresourceFilterClient::ShowUI(const GURL& url) { void ChromeSubresourceFilterClient::ShowUI(const GURL& url) {
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
InfoBarService* infobar_service = InfoBarService* infobar_service =
......
...@@ -15,11 +15,13 @@ ...@@ -15,11 +15,13 @@
class GURL; class GURL;
namespace content { namespace content {
class NavigationHandle;
class WebContents; class WebContents;
} // namespace content } // namespace content
namespace subresource_filter { namespace subresource_filter {
class ContentSubresourceFilterThrottleManager; class ContentSubresourceFilterThrottleManager;
class ProfileInteractionManager;
class SubresourceFilterProfileContext; class SubresourceFilterProfileContext;
} // namespace subresource_filter } // namespace subresource_filter
...@@ -45,13 +47,15 @@ class ChromeSubresourceFilterClient ...@@ -45,13 +47,15 @@ class ChromeSubresourceFilterClient
// SubresourceFilterClient: // SubresourceFilterClient:
void ShowNotification() override; void ShowNotification() override;
subresource_filter::mojom::ActivationLevel OnPageActivationComputed(
content::NavigationHandle* navigation_handle,
subresource_filter::mojom::ActivationLevel initial_activation_level,
subresource_filter::ActivationDecision* decision) override;
void OnAdsViolationTriggered( void OnAdsViolationTriggered(
content::RenderFrameHost* rfh, content::RenderFrameHost* rfh,
subresource_filter::mojom::AdsViolation triggered_violation) override; subresource_filter::mojom::AdsViolation triggered_violation) override;
const scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> const scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager>
GetSafeBrowsingDatabaseManager() override; GetSafeBrowsingDatabaseManager() override;
subresource_filter::ProfileInteractionManager* GetProfileInteractionManager()
override;
void OnReloadRequested() override; void OnReloadRequested() override;
private: private:
......
...@@ -22,6 +22,7 @@ ...@@ -22,6 +22,7 @@
#include "components/content_settings/browser/test_page_specific_content_settings_delegate.h" #include "components/content_settings/browser/test_page_specific_content_settings_delegate.h"
#include "components/content_settings/core/browser/host_content_settings_map.h" #include "components/content_settings/core/browser/host_content_settings_map.h"
#include "components/subresource_filter/content/browser/fake_safe_browsing_database_manager.h" #include "components/subresource_filter/content/browser/fake_safe_browsing_database_manager.h"
#include "components/subresource_filter/content/browser/subresource_filter_client.h"
#include "components/subresource_filter/content/browser/subresource_filter_observer_manager.h" #include "components/subresource_filter/content/browser/subresource_filter_observer_manager.h"
#include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.h" #include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.h"
#include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_client.h" #include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_client.h"
...@@ -47,13 +48,31 @@ const char kNumBlockedHistogram[] = ...@@ -47,13 +48,31 @@ const char kNumBlockedHistogram[] =
"ContentSettings.Popups.StrongBlocker.NumBlocked"; "ContentSettings.Popups.StrongBlocker.NumBlocked";
class SafeBrowsingTriggeredPopupBlockerTest class SafeBrowsingTriggeredPopupBlockerTest
: public content::RenderViewHostTestHarness { : public content::RenderViewHostTestHarness,
public subresource_filter::SubresourceFilterClient {
public: public:
SafeBrowsingTriggeredPopupBlockerTest() = default; SafeBrowsingTriggeredPopupBlockerTest() = default;
~SafeBrowsingTriggeredPopupBlockerTest() override { ~SafeBrowsingTriggeredPopupBlockerTest() override {
settings_map_->ShutdownOnUIThread(); settings_map_->ShutdownOnUIThread();
} }
// subresource_filter::SubresourceFilterClient:
void ShowNotification() override {}
subresource_filter::mojom::ActivationLevel OnPageActivationComputed(
content::NavigationHandle* navigation_handle,
subresource_filter::mojom::ActivationLevel initial_activation_level,
subresource_filter::ActivationDecision* decision) override {
return initial_activation_level;
}
void OnAdsViolationTriggered(
content::RenderFrameHost*,
subresource_filter::mojom::AdsViolation) override {}
const scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager>
GetSafeBrowsingDatabaseManager() override {
return nullptr;
}
void OnReloadRequested() override {}
// content::RenderViewHostTestHarness: // content::RenderViewHostTestHarness:
void SetUp() override { void SetUp() override {
content::RenderViewHostTestHarness::SetUp(); content::RenderViewHostTestHarness::SetUp();
...@@ -142,7 +161,7 @@ class SafeBrowsingTriggeredPopupBlockerTest ...@@ -142,7 +161,7 @@ class SafeBrowsingTriggeredPopupBlockerTest
content::NavigationHandle* handle) { content::NavigationHandle* handle) {
return std::make_unique< return std::make_unique<
subresource_filter::SubresourceFilterSafeBrowsingActivationThrottle>( subresource_filter::SubresourceFilterSafeBrowsingActivationThrottle>(
handle, /*delegate=*/nullptr, content::GetIOThreadTaskRunner({}), handle, this, content::GetIOThreadTaskRunner({}),
fake_safe_browsing_database_); fake_safe_browsing_database_);
} }
......
...@@ -18,7 +18,6 @@ ...@@ -18,7 +18,6 @@
#include "components/subresource_filter/content/browser/activation_state_computing_navigation_throttle.h" #include "components/subresource_filter/content/browser/activation_state_computing_navigation_throttle.h"
#include "components/subresource_filter/content/browser/async_document_subresource_filter.h" #include "components/subresource_filter/content/browser/async_document_subresource_filter.h"
#include "components/subresource_filter/content/browser/page_load_statistics.h" #include "components/subresource_filter/content/browser/page_load_statistics.h"
#include "components/subresource_filter/content/browser/profile_interaction_manager.h"
#include "components/subresource_filter/content/browser/subresource_filter_client.h" #include "components/subresource_filter/content/browser/subresource_filter_client.h"
#include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.h" #include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.h"
#include "components/subresource_filter/content/common/subresource_filter_messages.h" #include "components/subresource_filter/content/common/subresource_filter_messages.h"
...@@ -439,7 +438,7 @@ void ContentSubresourceFilterThrottleManager::MaybeAppendNavigationThrottles( ...@@ -439,7 +438,7 @@ void ContentSubresourceFilterThrottleManager::MaybeAppendNavigationThrottles(
client_->GetSafeBrowsingDatabaseManager()) { client_->GetSafeBrowsingDatabaseManager()) {
throttles->push_back( throttles->push_back(
std::make_unique<SubresourceFilterSafeBrowsingActivationThrottle>( std::make_unique<SubresourceFilterSafeBrowsingActivationThrottle>(
navigation_handle, client_->GetProfileInteractionManager(), navigation_handle, client_.get(),
content::GetIOThreadTaskRunner({}), content::GetIOThreadTaskRunner({}),
client_->GetSafeBrowsingDatabaseManager())); client_->GetSafeBrowsingDatabaseManager()));
} }
......
...@@ -183,6 +183,12 @@ class TestSubresourceFilterClient : public SubresourceFilterClient { ...@@ -183,6 +183,12 @@ class TestSubresourceFilterClient : public SubresourceFilterClient {
// SubresourceFilterClient: // SubresourceFilterClient:
void ShowNotification() override { ++disallowed_notification_count_; } void ShowNotification() override { ++disallowed_notification_count_; }
mojom::ActivationLevel OnPageActivationComputed(
content::NavigationHandle* navigation_handle,
mojom::ActivationLevel effective_activation_level,
ActivationDecision* decision) override {
return effective_activation_level;
}
void OnAdsViolationTriggered( void OnAdsViolationTriggered(
content::RenderFrameHost* rfh, content::RenderFrameHost* rfh,
mojom::AdsViolation triggered_violation) override {} mojom::AdsViolation triggered_violation) override {}
...@@ -190,10 +196,6 @@ class TestSubresourceFilterClient : public SubresourceFilterClient { ...@@ -190,10 +196,6 @@ class TestSubresourceFilterClient : public SubresourceFilterClient {
GetSafeBrowsingDatabaseManager() override { GetSafeBrowsingDatabaseManager() override {
return database_manager_; return database_manager_;
} }
subresource_filter::ProfileInteractionManager* GetProfileInteractionManager()
override {
return nullptr;
}
void OnReloadRequested() override {} void OnReloadRequested() override {}
void CreateSafeBrowsingDatabaseManager() { void CreateSafeBrowsingDatabaseManager() {
......
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
#ifndef COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_PROFILE_INTERACTION_MANAGER_H_ #ifndef COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_PROFILE_INTERACTION_MANAGER_H_
#define COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_PROFILE_INTERACTION_MANAGER_H_ #define COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_PROFILE_INTERACTION_MANAGER_H_
#include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.h"
#include "components/subresource_filter/core/common/activation_decision.h" #include "components/subresource_filter/core/common/activation_decision.h"
#include "components/subresource_filter/core/mojom/subresource_filter.mojom.h" #include "components/subresource_filter/core/mojom/subresource_filter.mojom.h"
#include "content/public/browser/web_contents_observer.h" #include "content/public/browser/web_contents_observer.h"
...@@ -22,9 +21,7 @@ class SubresourceFilterProfileContext; ...@@ -22,9 +21,7 @@ class SubresourceFilterProfileContext;
// Class that manages interaction between interaction between the // Class that manages interaction between interaction between the
// per-navigation/per-tab subresource filter objects (i.e., the throttles and // per-navigation/per-tab subresource filter objects (i.e., the throttles and
// throttle manager) and the per-profile objects (e.g., content settings). // throttle manager) and the per-profile objects (e.g., content settings).
class ProfileInteractionManager class ProfileInteractionManager : public content::WebContentsObserver {
: public content::WebContentsObserver,
public SubresourceFilterSafeBrowsingActivationThrottle::Delegate {
public: public:
ProfileInteractionManager(content::WebContents* web_contents, ProfileInteractionManager(content::WebContents* web_contents,
SubresourceFilterProfileContext* profile_context); SubresourceFilterProfileContext* profile_context);
...@@ -46,11 +43,19 @@ class ProfileInteractionManager ...@@ -46,11 +43,19 @@ class ProfileInteractionManager
void OnAdsViolationTriggered(content::RenderFrameHost* rfh, void OnAdsViolationTriggered(content::RenderFrameHost* rfh,
mojom::AdsViolation triggered_violation); mojom::AdsViolation triggered_violation);
// SubresourceFilterSafeBrowsingActivationThrottle::Delegate: // Called when the initial activation decision has been computed by the
// safe browsing activation throttle. This object then applies any adjustments
// based on relevant state of the Profile (e.g., content settings). Returns
// the effective activation for this navigation.
//
// Note: |decision| is guaranteed to be non-nullptr, and can be modified by
// this method if any decision changes.
//
// Precondition: The navigation must be a main frame navigation.
mojom::ActivationLevel OnPageActivationComputed( mojom::ActivationLevel OnPageActivationComputed(
content::NavigationHandle* navigation_handle, content::NavigationHandle* navigation_handle,
mojom::ActivationLevel initial_activation_level, mojom::ActivationLevel initial_activation_level,
ActivationDecision* decision) override; ActivationDecision* decision);
private: private:
// Unowned and must outlive this object. // Unowned and must outlive this object.
......
...@@ -6,8 +6,15 @@ ...@@ -6,8 +6,15 @@
#define COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_SUBRESOURCE_FILTER_CLIENT_H_ #define COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_SUBRESOURCE_FILTER_CLIENT_H_
#include "base/memory/scoped_refptr.h" #include "base/memory/scoped_refptr.h"
#include "components/subresource_filter/content/browser/verified_ruleset_dealer.h"
#include "components/subresource_filter/core/common/activation_decision.h"
#include "components/subresource_filter/core/mojom/subresource_filter.mojom.h" #include "components/subresource_filter/core/mojom/subresource_filter.mojom.h"
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h"
namespace content {
class NavigationHandle;
} // namespace content
namespace safe_browsing { namespace safe_browsing {
class SafeBrowsingDatabaseManager; class SafeBrowsingDatabaseManager;
...@@ -15,8 +22,6 @@ class SafeBrowsingDatabaseManager; ...@@ -15,8 +22,6 @@ class SafeBrowsingDatabaseManager;
namespace subresource_filter { namespace subresource_filter {
class ProfileInteractionManager;
class SubresourceFilterClient { class SubresourceFilterClient {
public: public:
virtual ~SubresourceFilterClient() = default; virtual ~SubresourceFilterClient() = default;
...@@ -25,6 +30,20 @@ class SubresourceFilterClient { ...@@ -25,6 +30,20 @@ class SubresourceFilterClient {
// blocked. This method will be called at most once per main-frame navigation. // blocked. This method will be called at most once per main-frame navigation.
virtual void ShowNotification() = 0; virtual void ShowNotification() = 0;
// Called when the activation decision is otherwise completely computed by the
// subresource filter. At this point, the embedder still has a chance to
// alter the effective activation. Returns the effective activation for this
// navigation.
//
// Note: |decision| is guaranteed to be non-nullptr, and can be modified by
// the embedder if any decision changes.
//
// Precondition: The navigation must be a main frame navigation.
virtual mojom::ActivationLevel OnPageActivationComputed(
content::NavigationHandle* navigation_handle,
mojom::ActivationLevel initial_activation_level,
subresource_filter::ActivationDecision* decision) = 0;
// Called on the subresource filter client when an ads violation is detected. // Called on the subresource filter client when an ads violation is detected.
virtual void OnAdsViolationTriggered( virtual void OnAdsViolationTriggered(
content::RenderFrameHost* rfh, content::RenderFrameHost* rfh,
...@@ -35,14 +54,6 @@ class SubresourceFilterClient { ...@@ -35,14 +54,6 @@ class SubresourceFilterClient {
virtual const scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> virtual const scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager>
GetSafeBrowsingDatabaseManager() = 0; GetSafeBrowsingDatabaseManager() = 0;
// Returns the ProfileInteractionManager instance associated with this
// client, or null if there is no such instance.
// TODO(crbug.com/1116095): Have ContentSubresourceFilterThrottleManager
// create and own this object internally once ChromeSubresourceFilterClient no
// longer calls into it, replacing this method with a getter for
// SubresourceFilterProfileContext.
virtual ProfileInteractionManager* GetProfileInteractionManager() = 0;
// Invoked when the user has requested a reload of a page with blocked ads // Invoked when the user has requested a reload of a page with blocked ads
// (e.g., via an infobar). // (e.g., via an infobar).
virtual void OnReloadRequested() = 0; virtual void OnReloadRequested() = 0;
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "components/subresource_filter/content/browser/content_activation_list_utils.h" #include "components/subresource_filter/content/browser/content_activation_list_utils.h"
#include "components/subresource_filter/content/browser/devtools_interaction_tracker.h" #include "components/subresource_filter/content/browser/devtools_interaction_tracker.h"
#include "components/subresource_filter/content/browser/navigation_console_logger.h" #include "components/subresource_filter/content/browser/navigation_console_logger.h"
#include "components/subresource_filter/content/browser/subresource_filter_client.h"
#include "components/subresource_filter/content/browser/subresource_filter_observer_manager.h" #include "components/subresource_filter/content/browser/subresource_filter_observer_manager.h"
#include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_client.h" #include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_client.h"
#include "components/subresource_filter/core/browser/subresource_filter_constants.h" #include "components/subresource_filter/core/browser/subresource_filter_constants.h"
...@@ -63,7 +64,7 @@ base::Optional<RedirectPosition> GetEnforcementRedirectPosition( ...@@ -63,7 +64,7 @@ base::Optional<RedirectPosition> GetEnforcementRedirectPosition(
SubresourceFilterSafeBrowsingActivationThrottle:: SubresourceFilterSafeBrowsingActivationThrottle::
SubresourceFilterSafeBrowsingActivationThrottle( SubresourceFilterSafeBrowsingActivationThrottle(
content::NavigationHandle* handle, content::NavigationHandle* handle,
Delegate* delegate, SubresourceFilterClient* client,
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner, scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager>
database_manager) database_manager)
...@@ -75,7 +76,7 @@ SubresourceFilterSafeBrowsingActivationThrottle:: ...@@ -75,7 +76,7 @@ SubresourceFilterSafeBrowsingActivationThrottle::
io_task_runner_, io_task_runner_,
base::ThreadTaskRunnerHandle::Get()), base::ThreadTaskRunnerHandle::Get()),
base::OnTaskRunnerDeleter(io_task_runner_)), base::OnTaskRunnerDeleter(io_task_runner_)),
delegate_(delegate) { client_(client) {
DCHECK(handle->IsInMainFrame()); DCHECK(handle->IsInMainFrame());
CheckCurrentUrl(); CheckCurrentUrl();
...@@ -204,11 +205,9 @@ void SubresourceFilterSafeBrowsingActivationThrottle::NotifyResult() { ...@@ -204,11 +205,9 @@ void SubresourceFilterSafeBrowsingActivationThrottle::NotifyResult() {
activation_decision = ActivationDecision::FORCED_ACTIVATION; activation_decision = ActivationDecision::FORCED_ACTIVATION;
} }
// Let the delegate adjust the activation decision if present. // Let the embedder get the last word when it comes to activation level.
if (delegate_) { activation_level = client_->OnPageActivationComputed(
activation_level = delegate_->OnPageActivationComputed( navigation_handle(), activation_level, &activation_decision);
navigation_handle(), activation_level, &activation_decision);
}
LogMetricsOnChecksComplete(selection.matched_list, activation_decision, LogMetricsOnChecksComplete(selection.matched_list, activation_decision,
activation_level); activation_level);
......
...@@ -26,6 +26,8 @@ ...@@ -26,6 +26,8 @@
namespace subresource_filter { namespace subresource_filter {
class SubresourceFilterClient;
// Enum representing a position in the redirect chain. These values are // Enum representing a position in the redirect chain. These values are
// persisted to logs. Entries should not be renumbered and numeric values should // persisted to logs. Entries should not be renumbered and numeric values should
// never be reused. // never be reused.
...@@ -44,32 +46,9 @@ class SubresourceFilterSafeBrowsingActivationThrottle ...@@ -44,32 +46,9 @@ class SubresourceFilterSafeBrowsingActivationThrottle
public base::SupportsWeakPtr< public base::SupportsWeakPtr<
SubresourceFilterSafeBrowsingActivationThrottle> { SubresourceFilterSafeBrowsingActivationThrottle> {
public: public:
// Interface that allows the client of this class to adjust activation
// decisions if/as desired.
class Delegate {
public:
virtual ~Delegate() = default;
// Called when the initial activation decision has been computed by the
// safe browsing activation throttle. Returns
// the effective activation for this navigation.
//
// Note: |decision| is guaranteed to be non-nullptr, and can be modified by
// this method if any decision changes.
//
// Precondition: The navigation must be a main frame navigation.
virtual mojom::ActivationLevel OnPageActivationComputed(
content::NavigationHandle* navigation_handle,
mojom::ActivationLevel initial_activation_level,
ActivationDecision* decision) = 0;
};
// |delegate| is allowed to be null, in which case the client creating this
// throttle will not be able to adjust activation decisions made by the
// throttle.
SubresourceFilterSafeBrowsingActivationThrottle( SubresourceFilterSafeBrowsingActivationThrottle(
content::NavigationHandle* handle, content::NavigationHandle* handle,
Delegate* delegate, SubresourceFilterClient* client,
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner, scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager>
database_manager); database_manager);
...@@ -131,8 +110,8 @@ class SubresourceFilterSafeBrowsingActivationThrottle ...@@ -131,8 +110,8 @@ class SubresourceFilterSafeBrowsingActivationThrottle
base::OnTaskRunnerDeleter> base::OnTaskRunnerDeleter>
database_client_; database_client_;
// May be null. If non-null, must outlive this class. // Must outlive this class.
Delegate* delegate_; SubresourceFilterClient* client_;
// Set to TimeTicks::Now() when the navigation is deferred in // Set to TimeTicks::Now() when the navigation is deferred in
// WillProcessResponse. If deferral was not necessary, will remain null. // WillProcessResponse. If deferral was not necessary, will remain null.
......
...@@ -24,7 +24,6 @@ ...@@ -24,7 +24,6 @@
#include "components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h" #include "components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h"
#include "components/subresource_filter/content/browser/devtools_interaction_tracker.h" #include "components/subresource_filter/content/browser/devtools_interaction_tracker.h"
#include "components/subresource_filter/content/browser/fake_safe_browsing_database_manager.h" #include "components/subresource_filter/content/browser/fake_safe_browsing_database_manager.h"
#include "components/subresource_filter/content/browser/profile_interaction_manager.h"
#include "components/subresource_filter/content/browser/subresource_filter_client.h" #include "components/subresource_filter/content/browser/subresource_filter_client.h"
#include "components/subresource_filter/content/browser/subresource_filter_observer_test_utils.h" #include "components/subresource_filter/content/browser/subresource_filter_observer_test_utils.h"
#include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_client.h" #include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_client.h"
...@@ -70,17 +69,11 @@ const char kActivationListHistogram[] = ...@@ -70,17 +69,11 @@ const char kActivationListHistogram[] =
"SubresourceFilter.PageLoad.ActivationList"; "SubresourceFilter.PageLoad.ActivationList";
const char kSubresourceFilterActionsHistogram[] = "SubresourceFilter.Actions2"; const char kSubresourceFilterActionsHistogram[] = "SubresourceFilter.Actions2";
class TestSafeBrowsingActivationThrottleDelegate class MockSubresourceFilterClient : public SubresourceFilterClient {
: public SubresourceFilterSafeBrowsingActivationThrottle::Delegate {
public: public:
TestSafeBrowsingActivationThrottleDelegate() = default; MockSubresourceFilterClient() = default;
~TestSafeBrowsingActivationThrottleDelegate() override = default; ~MockSubresourceFilterClient() override = default;
TestSafeBrowsingActivationThrottleDelegate(
const TestSafeBrowsingActivationThrottleDelegate&) = delete;
TestSafeBrowsingActivationThrottleDelegate& operator=(
const TestSafeBrowsingActivationThrottleDelegate&) = delete;
// SubresourceFilterSafeBrowsingActivationThrottle::Delegate:
mojom::ActivationLevel OnPageActivationComputed( mojom::ActivationLevel OnPageActivationComputed(
content::NavigationHandle* handle, content::NavigationHandle* handle,
mojom::ActivationLevel effective_level, mojom::ActivationLevel effective_level,
...@@ -95,22 +88,6 @@ class TestSafeBrowsingActivationThrottleDelegate ...@@ -95,22 +88,6 @@ class TestSafeBrowsingActivationThrottleDelegate
return effective_level; return effective_level;
} }
void AllowlistInCurrentWebContents(const GURL& url) {
ASSERT_TRUE(url.SchemeIsHTTPOrHTTPS());
allowlisted_hosts_.insert(url.host());
}
void ClearAllowlist() { allowlisted_hosts_.clear(); }
private:
std::set<std::string> allowlisted_hosts_;
};
class MockSubresourceFilterClient : public SubresourceFilterClient {
public:
MockSubresourceFilterClient() = default;
~MockSubresourceFilterClient() override = default;
MOCK_METHOD0(ShowNotification, void()); MOCK_METHOD0(ShowNotification, void());
MOCK_METHOD2(OnAdsViolationTriggered, MOCK_METHOD2(OnAdsViolationTriggered,
void(content::RenderFrameHost*, void(content::RenderFrameHost*,
...@@ -118,11 +95,18 @@ class MockSubresourceFilterClient : public SubresourceFilterClient { ...@@ -118,11 +95,18 @@ class MockSubresourceFilterClient : public SubresourceFilterClient {
MOCK_METHOD0( MOCK_METHOD0(
GetSafeBrowsingDatabaseManager, GetSafeBrowsingDatabaseManager,
const scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager>()); const scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager>());
MOCK_METHOD0(GetProfileInteractionManager,
subresource_filter::ProfileInteractionManager*());
MOCK_METHOD0(OnReloadRequested, void()); MOCK_METHOD0(OnReloadRequested, void());
void AllowlistInCurrentWebContents(const GURL& url) {
ASSERT_TRUE(url.SchemeIsHTTPOrHTTPS());
allowlisted_hosts_.insert(url.host());
}
void ClearAllowlist() { allowlisted_hosts_.clear(); }
private: private:
std::set<std::string> allowlisted_hosts_;
DISALLOW_COPY_AND_ASSIGN(MockSubresourceFilterClient); DISALLOW_COPY_AND_ASSIGN(MockSubresourceFilterClient);
}; };
...@@ -229,7 +213,7 @@ class SubresourceFilterSafeBrowsingActivationThrottleTest ...@@ -229,7 +213,7 @@ class SubresourceFilterSafeBrowsingActivationThrottleTest
if (navigation_handle->IsInMainFrame()) { if (navigation_handle->IsInMainFrame()) {
navigation_handle->RegisterThrottleForTesting( navigation_handle->RegisterThrottleForTesting(
std::make_unique<SubresourceFilterSafeBrowsingActivationThrottle>( std::make_unique<SubresourceFilterSafeBrowsingActivationThrottle>(
navigation_handle, delegate(), test_io_task_runner_, navigation_handle, client(), test_io_task_runner_,
fake_safe_browsing_database_)); fake_safe_browsing_database_));
} }
std::vector<std::unique_ptr<content::NavigationThrottle>> throttles; std::vector<std::unique_ptr<content::NavigationThrottle>> throttles;
...@@ -353,7 +337,6 @@ class SubresourceFilterSafeBrowsingActivationThrottleTest ...@@ -353,7 +337,6 @@ class SubresourceFilterSafeBrowsingActivationThrottleTest
MockSubresourceFilterClient* client() { return client_; } MockSubresourceFilterClient* client() { return client_; }
TestSafeBrowsingActivationThrottleDelegate* delegate() { return &delegate_; }
base::TestMockTimeTaskRunner* test_io_task_runner() const { base::TestMockTimeTaskRunner* test_io_task_runner() const {
return test_io_task_runner_.get(); return test_io_task_runner_.get();
} }
...@@ -369,7 +352,6 @@ class SubresourceFilterSafeBrowsingActivationThrottleTest ...@@ -369,7 +352,6 @@ class SubresourceFilterSafeBrowsingActivationThrottleTest
testing::TestRulesetCreator test_ruleset_creator_; testing::TestRulesetCreator test_ruleset_creator_;
testing::TestRulesetPair test_ruleset_pair_; testing::TestRulesetPair test_ruleset_pair_;
TestSafeBrowsingActivationThrottleDelegate delegate_;
std::unique_ptr<VerifiedRulesetDealer::Handle> ruleset_dealer_; std::unique_ptr<VerifiedRulesetDealer::Handle> ruleset_dealer_;
std::unique_ptr<ContentSubresourceFilterThrottleManager> throttle_manager_; std::unique_ptr<ContentSubresourceFilterThrottleManager> throttle_manager_;
...@@ -537,7 +519,7 @@ TEST_F(SubresourceFilterSafeBrowsingActivationThrottleTest, ...@@ -537,7 +519,7 @@ TEST_F(SubresourceFilterSafeBrowsingActivationThrottleTest,
*observer()->GetPageActivationForLastCommittedLoad()); *observer()->GetPageActivationForLastCommittedLoad());
// Allowlisting occurs last, so the decision should still be DISABLED. // Allowlisting occurs last, so the decision should still be DISABLED.
delegate()->AllowlistInCurrentWebContents(url); client()->AllowlistInCurrentWebContents(url);
SimulateNavigateAndCommit({url}, main_rfh()); SimulateNavigateAndCommit({url}, main_rfh());
EXPECT_EQ(mojom::ActivationLevel::kDisabled, EXPECT_EQ(mojom::ActivationLevel::kDisabled,
*observer()->GetPageActivationForLastCommittedLoad()); *observer()->GetPageActivationForLastCommittedLoad());
...@@ -810,7 +792,7 @@ TEST_P(SubresourceFilterSafeBrowsingActivationThrottleScopeTest, ...@@ -810,7 +792,7 @@ TEST_P(SubresourceFilterSafeBrowsingActivationThrottleScopeTest,
EXPECT_EQ(test_data.expected_activation_level, EXPECT_EQ(test_data.expected_activation_level,
*observer()->GetPageActivationForLastCommittedLoad()); *observer()->GetPageActivationForLastCommittedLoad());
if (test_data.url_matches_activation_list) { if (test_data.url_matches_activation_list) {
delegate()->AllowlistInCurrentWebContents(test_url); client()->AllowlistInCurrentWebContents(test_url);
SimulateNavigateAndCommit({test_url}, main_rfh()); SimulateNavigateAndCommit({test_url}, main_rfh());
EXPECT_EQ(mojom::ActivationLevel::kDisabled, EXPECT_EQ(mojom::ActivationLevel::kDisabled,
*observer()->GetPageActivationForLastCommittedLoad()); *observer()->GetPageActivationForLastCommittedLoad());
......
...@@ -4,7 +4,6 @@ ...@@ -4,7 +4,6 @@
#include "base/json/json_reader.h" #include "base/json/json_reader.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "components/content_settings/core/browser/host_content_settings_map.h"
#include "components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h" #include "components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h"
#include "components/subresource_filter/content/browser/fake_safe_browsing_database_manager.h" #include "components/subresource_filter/content/browser/fake_safe_browsing_database_manager.h"
#include "components/subresource_filter/content/browser/ruleset_service.h" #include "components/subresource_filter/content/browser/ruleset_service.h"
...@@ -16,7 +15,6 @@ ...@@ -16,7 +15,6 @@
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/resource/resource_bundle.h" #include "ui/base/resource/resource_bundle.h"
#include "weblayer/browser/browser_process.h" #include "weblayer/browser/browser_process.h"
#include "weblayer/browser/host_content_settings_map_factory.h"
#include "weblayer/browser/subresource_filter_client_impl.h" #include "weblayer/browser/subresource_filter_client_impl.h"
#include "weblayer/browser/tab_impl.h" #include "weblayer/browser/tab_impl.h"
#include "weblayer/grit/weblayer_resources.h" #include "weblayer/grit/weblayer_resources.h"
...@@ -91,7 +89,8 @@ class SubresourceFilterBrowserTest : public WebLayerBrowserTest { ...@@ -91,7 +89,8 @@ class SubresourceFilterBrowserTest : public WebLayerBrowserTest {
protected: protected:
void SetRulesetToDisallowURLsWithPathSuffix(const std::string& suffix) { void SetRulesetToDisallowURLsWithPathSuffix(const std::string& suffix) {
subresource_filter::testing::TestRulesetPair test_ruleset_pair; subresource_filter::testing::TestRulesetPair test_ruleset_pair;
test_ruleset_creator_.CreateRulesetToDisallowURLsWithPathSuffix( subresource_filter::testing::TestRulesetCreator test_ruleset_creator;
test_ruleset_creator.CreateRulesetToDisallowURLsWithPathSuffix(
suffix, &test_ruleset_pair); suffix, &test_ruleset_pair);
subresource_filter::testing::TestRulesetPublisher test_ruleset_publisher( subresource_filter::testing::TestRulesetPublisher test_ruleset_publisher(
...@@ -115,9 +114,6 @@ class SubresourceFilterBrowserTest : public WebLayerBrowserTest { ...@@ -115,9 +114,6 @@ class SubresourceFilterBrowserTest : public WebLayerBrowserTest {
->client()); ->client());
client_impl->set_database_manager_for_testing(std::move(database_manager)); client_impl->set_database_manager_for_testing(std::move(database_manager));
} }
private:
subresource_filter::testing::TestRulesetCreator test_ruleset_creator_;
}; };
// Tests that the ruleset service is available. // Tests that the ruleset service is available.
...@@ -298,68 +294,6 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, ...@@ -298,68 +294,6 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
EXPECT_TRUE(WasParsedScriptElementLoaded(web_contents->GetMainFrame())); EXPECT_TRUE(WasParsedScriptElementLoaded(web_contents->GetMainFrame()));
} }
IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
ContentSettingsAllowlist_DoNotActivate) {
auto* web_contents = static_cast<TabImpl*>(shell()->tab())->web_contents();
GURL test_url(
embedded_test_server()->GetURL("/frame_with_included_script.html"));
ASSERT_NO_FATAL_FAILURE(
SetRulesetToDisallowURLsWithPathSuffix("included_script.js"));
ActivateSubresourceFilterInWebContentsForURL(web_contents, test_url);
NavigateAndWaitForCompletion(test_url, shell());
EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents->GetMainFrame()));
content::WebContentsConsoleObserver console_observer(web_contents);
console_observer.SetPattern(subresource_filter::kActivationConsoleMessage);
// Simulate explicitly allowlisting via content settings.
HostContentSettingsMap* settings_map =
HostContentSettingsMapFactory::GetForBrowserContext(
web_contents->GetBrowserContext());
settings_map->SetContentSettingDefaultScope(
test_url, test_url, ContentSettingsType::ADS, CONTENT_SETTING_ALLOW);
NavigateAndWaitForCompletion(test_url, shell());
EXPECT_TRUE(WasParsedScriptElementLoaded(web_contents->GetMainFrame()));
// No message for allowlisted url.
EXPECT_TRUE(console_observer.messages().empty());
}
IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
ContentSettingsAllowlistGlobal_DoNotActivate) {
auto* web_contents = static_cast<TabImpl*>(shell()->tab())->web_contents();
GURL test_url(
embedded_test_server()->GetURL("/frame_with_included_script.html"));
ASSERT_NO_FATAL_FAILURE(
SetRulesetToDisallowURLsWithPathSuffix("included_script.js"));
ActivateSubresourceFilterInWebContentsForURL(web_contents, test_url);
NavigateAndWaitForCompletion(test_url, shell());
EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents->GetMainFrame()));
content::WebContentsConsoleObserver console_observer(web_contents);
console_observer.SetPattern(subresource_filter::kActivationConsoleMessage);
// Simulate globally allowing ads via content settings.
HostContentSettingsMap* settings_map =
HostContentSettingsMapFactory::GetForBrowserContext(
web_contents->GetBrowserContext());
settings_map->SetDefaultContentSetting(ContentSettingsType::ADS,
CONTENT_SETTING_ALLOW);
NavigateAndWaitForCompletion(test_url, shell());
EXPECT_TRUE(WasParsedScriptElementLoaded(web_contents->GetMainFrame()));
// No message for loads that are not activated.
EXPECT_TRUE(console_observer.messages().empty());
}
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
// Test that the ads blocked infobar is presented when visiting a page where the // Test that the ads blocked infobar is presented when visiting a page where the
// subresource filter blocks resources from being loaded and is removed when // subresource filter blocks resources from being loaded and is removed when
......
...@@ -9,7 +9,6 @@ ...@@ -9,7 +9,6 @@
#include "base/macros.h" #include "base/macros.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h" #include "components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h"
#include "components/subresource_filter/content/browser/profile_interaction_manager.h"
#include "components/subresource_filter/content/browser/ruleset_service.h" #include "components/subresource_filter/content/browser/ruleset_service.h"
#include "components/subresource_filter/core/browser/subresource_filter_features.h" #include "components/subresource_filter/core/browser/subresource_filter_features.h"
#include "components/subresource_filter/core/common/activation_decision.h" #include "components/subresource_filter/core/common/activation_decision.h"
...@@ -19,7 +18,6 @@ ...@@ -19,7 +18,6 @@
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
#include "weblayer/browser/browser_process.h" #include "weblayer/browser/browser_process.h"
#include "weblayer/browser/safe_browsing/safe_browsing_service.h" #include "weblayer/browser/safe_browsing/safe_browsing_service.h"
#include "weblayer/browser/subresource_filter_profile_context_factory.h"
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
#include "components/safe_browsing/android/remote_database_manager.h" #include "components/safe_browsing/android/remote_database_manager.h"
...@@ -55,12 +53,7 @@ SubresourceFilterClientImpl::SubresourceFilterClientImpl( ...@@ -55,12 +53,7 @@ SubresourceFilterClientImpl::SubresourceFilterClientImpl(
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
web_contents_(web_contents), web_contents_(web_contents),
#endif #endif
database_manager_(GetDatabaseManagerFromSafeBrowsingService()), database_manager_(GetDatabaseManagerFromSafeBrowsingService()) {
profile_interaction_manager_(
std::make_unique<subresource_filter::ProfileInteractionManager>(
web_contents,
SubresourceFilterProfileContextFactory::GetForBrowserContext(
web_contents->GetBrowserContext()))) {
} }
SubresourceFilterClientImpl::~SubresourceFilterClientImpl() = default; SubresourceFilterClientImpl::~SubresourceFilterClientImpl() = default;
...@@ -94,6 +87,16 @@ void SubresourceFilterClientImpl::ShowNotification() { ...@@ -94,6 +87,16 @@ void SubresourceFilterClientImpl::ShowNotification() {
#endif #endif
} }
subresource_filter::mojom::ActivationLevel
SubresourceFilterClientImpl::OnPageActivationComputed(
content::NavigationHandle* navigation_handle,
subresource_filter::mojom::ActivationLevel initial_activation_level,
subresource_filter::ActivationDecision* decision) {
DCHECK(navigation_handle->IsInMainFrame());
return initial_activation_level;
}
void SubresourceFilterClientImpl::OnAdsViolationTriggered( void SubresourceFilterClientImpl::OnAdsViolationTriggered(
content::RenderFrameHost* rfh, content::RenderFrameHost* rfh,
subresource_filter::mojom::AdsViolation triggered_violation) {} subresource_filter::mojom::AdsViolation triggered_violation) {}
...@@ -103,9 +106,4 @@ SubresourceFilterClientImpl::GetSafeBrowsingDatabaseManager() { ...@@ -103,9 +106,4 @@ SubresourceFilterClientImpl::GetSafeBrowsingDatabaseManager() {
return database_manager_; return database_manager_;
} }
subresource_filter::ProfileInteractionManager*
SubresourceFilterClientImpl::GetProfileInteractionManager() {
return profile_interaction_manager_.get();
}
} // namespace weblayer } // namespace weblayer
...@@ -43,13 +43,15 @@ class SubresourceFilterClientImpl ...@@ -43,13 +43,15 @@ class SubresourceFilterClientImpl
// SubresourceFilterClient: // SubresourceFilterClient:
void ShowNotification() override; void ShowNotification() override;
subresource_filter::mojom::ActivationLevel OnPageActivationComputed(
content::NavigationHandle* navigation_handle,
subresource_filter::mojom::ActivationLevel initial_activation_level,
subresource_filter::ActivationDecision* decision) override;
void OnAdsViolationTriggered( void OnAdsViolationTriggered(
content::RenderFrameHost* rfh, content::RenderFrameHost* rfh,
subresource_filter::mojom::AdsViolation triggered_violation) override; subresource_filter::mojom::AdsViolation triggered_violation) override;
const scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> const scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager>
GetSafeBrowsingDatabaseManager() override; GetSafeBrowsingDatabaseManager() override;
subresource_filter::ProfileInteractionManager* GetProfileInteractionManager()
override;
void OnReloadRequested() override; void OnReloadRequested() override;
// Sets the SafeBrowsingDatabaseManager instance used to |database_manager|. // Sets the SafeBrowsingDatabaseManager instance used to |database_manager|.
...@@ -68,8 +70,6 @@ class SubresourceFilterClientImpl ...@@ -68,8 +70,6 @@ class SubresourceFilterClientImpl
std::unique_ptr<subresource_filter::ContentSubresourceFilterThrottleManager> std::unique_ptr<subresource_filter::ContentSubresourceFilterThrottleManager>
throttle_manager_; throttle_manager_;
scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> database_manager_; scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> database_manager_;
std::unique_ptr<subresource_filter::ProfileInteractionManager>
profile_interaction_manager_;
}; };
} // namespace weblayer } // namespace weblayer
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment