Commit 4233d1e7 authored by Colin Blundell's avatar Colin Blundell Committed by Commit Bot

[Subresource Filter] Eliminate infobar dependence on //chrome client

We are targeting the ads blocked infobar for componentization and reuse
by WebLayer. To facilitate that componentization this CL eliminates the
infobar's dependence on ChromeSubresourceFilterClient. To do so, we
make ChromeSubresourceFilterClient::OnReloadRequested() an API on the
SubresourceFilterClient interface. There is no behavioral change.

In the future we plan to move the *implementation* of
ChromeSubresourceFilterClient::OnReloadRequested() into the throttle
manager for sharing with //weblayer. However, that is for a later
stage, as it is dependent on first bringing awareness of content
settings into the component. We have put a TODO in the code to this
effect.

Bug: 1116095
Change-Id: Iff57d3a3fe382d0371fc983084e75d3eb8fb7909
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2461006
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: default avatarEric Robinson <ericrobinson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815532}
parent 47dba4a9
...@@ -86,6 +86,10 @@ void ChromeSubresourceFilterClient::DidStartNavigation( ...@@ -86,6 +86,10 @@ void ChromeSubresourceFilterClient::DidStartNavigation(
} }
void ChromeSubresourceFilterClient::OnReloadRequested() { void ChromeSubresourceFilterClient::OnReloadRequested() {
// TODO(crbug.com/1116095): Once ContentSubresourceFilterThrottleManager knows
// about content settings, this method can move entirely into
// ContentSubresourceFilterThrottleManager::OnReloadRequested() and
// SubresourceFilterClient::OnReloadRequested() can be eliminated.
subresource_filter::ContentSubresourceFilterThrottleManager::LogAction( subresource_filter::ContentSubresourceFilterThrottleManager::LogAction(
subresource_filter::SubresourceFilterAction::kAllowlistedSite); subresource_filter::SubresourceFilterAction::kAllowlistedSite);
AllowlistByContentSettings(web_contents()->GetLastCommittedURL()); AllowlistByContentSettings(web_contents()->GetLastCommittedURL());
......
...@@ -46,8 +46,6 @@ class ChromeSubresourceFilterClient ...@@ -46,8 +46,6 @@ class ChromeSubresourceFilterClient
static ChromeSubresourceFilterClient* FromWebContents( static ChromeSubresourceFilterClient* FromWebContents(
content::WebContents* web_contents); content::WebContents* web_contents);
void OnReloadRequested();
// content::WebContentsObserver: // content::WebContentsObserver:
void DidStartNavigation( void DidStartNavigation(
content::NavigationHandle* navigation_handle) override; content::NavigationHandle* navigation_handle) override;
...@@ -63,6 +61,7 @@ class ChromeSubresourceFilterClient ...@@ -63,6 +61,7 @@ class ChromeSubresourceFilterClient
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;
void OnReloadRequested() override;
// Should be called by devtools in response to a protocol command to enable ad // Should be called by devtools in response to a protocol command to enable ad
// blocking in this WebContents. Should only persist while devtools is // blocking in this WebContents. Should only persist while devtools is
......
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "chrome/browser/subresource_filter/chrome_subresource_filter_client.h"
#include "chrome/browser/ui/android/infobars/ads_blocked_infobar.h" #include "chrome/browser/ui/android/infobars/ads_blocked_infobar.h"
#include "components/infobars/content/content_infobar_manager.h" #include "components/infobars/content/content_infobar_manager.h"
#include "components/infobars/core/infobar.h" #include "components/infobars/core/infobar.h"
...@@ -78,9 +77,9 @@ base::string16 AdsBlockedInfobarDelegate::GetButtonLabel( ...@@ -78,9 +77,9 @@ base::string16 AdsBlockedInfobarDelegate::GetButtonLabel(
} }
bool AdsBlockedInfobarDelegate::Cancel() { bool AdsBlockedInfobarDelegate::Cancel() {
auto* filter_client = ChromeSubresourceFilterClient::FromWebContents( subresource_filter::ContentSubresourceFilterThrottleManager::FromWebContents(
infobars::ContentInfoBarManager::WebContentsFromInfoBar(infobar())); infobars::ContentInfoBarManager::WebContentsFromInfoBar(infobar()))
filter_client->OnReloadRequested(); ->OnReloadRequested();
return true; return true;
} }
......
...@@ -34,7 +34,6 @@ ...@@ -34,7 +34,6 @@
#include "chrome/browser/plugins/chrome_plugin_service_filter.h" #include "chrome/browser/plugins/chrome_plugin_service_filter.h"
#include "chrome/browser/plugins/plugin_utils.h" #include "chrome/browser/plugins/plugin_utils.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/subresource_filter/chrome_subresource_filter_client.h"
#include "chrome/browser/ui/collected_cookies_infobar_delegate.h" #include "chrome/browser/ui/collected_cookies_infobar_delegate.h"
#include "chrome/browser/ui/content_settings/content_setting_bubble_model_delegate.h" #include "chrome/browser/ui/content_settings/content_setting_bubble_model_delegate.h"
#include "chrome/common/chrome_features.h" #include "chrome/common/chrome_features.h"
...@@ -1423,8 +1422,9 @@ void ContentSettingSubresourceFilterBubbleModel::OnLearnMoreClicked() { ...@@ -1423,8 +1422,9 @@ void ContentSettingSubresourceFilterBubbleModel::OnLearnMoreClicked() {
void ContentSettingSubresourceFilterBubbleModel::CommitChanges() { void ContentSettingSubresourceFilterBubbleModel::CommitChanges() {
if (is_checked_) { if (is_checked_) {
ChromeSubresourceFilterClient::FromWebContents(web_contents()) subresource_filter::ContentSubresourceFilterThrottleManager::
->OnReloadRequested(); FromWebContents(web_contents())
->OnReloadRequested();
} }
} }
......
...@@ -71,6 +71,7 @@ class SafeBrowsingTriggeredPopupBlockerTest ...@@ -71,6 +71,7 @@ class SafeBrowsingTriggeredPopupBlockerTest
GetSafeBrowsingDatabaseManager() override { GetSafeBrowsingDatabaseManager() override {
return nullptr; return nullptr;
} }
void OnReloadRequested() override {}
// content::RenderViewHostTestHarness: // content::RenderViewHostTestHarness:
void SetUp() override { void SetUp() override {
......
...@@ -404,6 +404,10 @@ ContentSubresourceFilterThrottleManager::LoadPolicyForLastCommittedNavigation( ...@@ -404,6 +404,10 @@ ContentSubresourceFilterThrottleManager::LoadPolicyForLastCommittedNavigation(
return base::nullopt; return base::nullopt;
} }
void ContentSubresourceFilterThrottleManager::OnReloadRequested() {
client_->OnReloadRequested();
}
// static // static
void ContentSubresourceFilterThrottleManager::LogAction( void ContentSubresourceFilterThrottleManager::LogAction(
SubresourceFilterAction action) { SubresourceFilterAction action) {
......
...@@ -142,6 +142,10 @@ class ContentSubresourceFilterThrottleManager ...@@ -142,6 +142,10 @@ class ContentSubresourceFilterThrottleManager
base::Optional<LoadPolicy> LoadPolicyForLastCommittedNavigation( base::Optional<LoadPolicy> LoadPolicyForLastCommittedNavigation(
const content::RenderFrameHost* frame_host) const; const content::RenderFrameHost* frame_host) const;
// Notifies the client that the user has requested a reload of a page with
// blocked ads (e.g., via an infobar).
void OnReloadRequested();
static void LogAction(SubresourceFilterAction action); static void LogAction(SubresourceFilterAction action);
protected: protected:
......
...@@ -196,6 +196,7 @@ class TestSubresourceFilterClient : public SubresourceFilterClient { ...@@ -196,6 +196,7 @@ class TestSubresourceFilterClient : public SubresourceFilterClient {
GetSafeBrowsingDatabaseManager() override { GetSafeBrowsingDatabaseManager() override {
return database_manager_; return database_manager_;
} }
void OnReloadRequested() override {}
void CreateSafeBrowsingDatabaseManager() { void CreateSafeBrowsingDatabaseManager() {
database_manager_ = database_manager_ =
......
...@@ -53,6 +53,10 @@ class SubresourceFilterClient { ...@@ -53,6 +53,10 @@ class SubresourceFilterClient {
// client, or null if there is no such instance. // client, or null if there is no such instance.
virtual const scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> virtual const scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager>
GetSafeBrowsingDatabaseManager() = 0; GetSafeBrowsingDatabaseManager() = 0;
// Invoked when the user has requested a reload of a page with blocked ads
// (e.g., via an infobar).
virtual void OnReloadRequested() = 0;
}; };
} // namespace subresource_filter } // namespace subresource_filter
......
...@@ -94,6 +94,7 @@ class MockSubresourceFilterClient : public SubresourceFilterClient { ...@@ -94,6 +94,7 @@ class MockSubresourceFilterClient : public SubresourceFilterClient {
MOCK_METHOD0( MOCK_METHOD0(
GetSafeBrowsingDatabaseManager, GetSafeBrowsingDatabaseManager,
const scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager>()); const scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager>());
MOCK_METHOD0(OnReloadRequested, void());
void AllowlistInCurrentWebContents(const GURL& url) { void AllowlistInCurrentWebContents(const GURL& url) {
ASSERT_TRUE(url.SchemeIsHTTPOrHTTPS()); ASSERT_TRUE(url.SchemeIsHTTPOrHTTPS());
......
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