Skip to content

Commit

Permalink
Merge pull request #8470 from brave/mpilgrim_domain_block_aggressive
Browse files Browse the repository at this point in the history
Only apply top level domain blocking in aggressive mode
  • Loading branch information
pilgrim-brave committed Apr 8, 2021
2 parents f7ec47e + 7ea5455 commit 869537b
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 4 deletions.
5 changes: 2 additions & 3 deletions browser/brave_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -640,11 +640,8 @@ BraveContentBrowserClient::CreateThrottlesForNavigation(
std::make_unique<extensions::BraveWebTorrentNavigationThrottle>(handle));
#endif

#if BUILDFLAG(ENABLE_TOR) || BUILDFLAG(IPFS_ENABLED) || \
BUILDFLAG(DECENTRALIZED_DNS_ENABLED)
content::BrowserContext* context =
handle->GetWebContents()->GetBrowserContext();
#endif

#if BUILDFLAG(ENABLE_TOR)
std::unique_ptr<content::NavigationThrottle> tor_navigation_throttle =
Expand Down Expand Up @@ -689,6 +686,8 @@ BraveContentBrowserClient::CreateThrottlesForNavigation(
brave_shields::DomainBlockNavigationThrottle::MaybeCreateThrottleFor(
handle, g_brave_browser_process->ad_block_service(),
g_brave_browser_process->ad_block_custom_filters_service(),
HostContentSettingsMapFactory::GetForProfile(
Profile::FromBrowserContext(context)),
g_brave_browser_process->GetApplicationLocale()))
throttles.push_back(std::move(domain_block_navigation_throttle));

Expand Down
49 changes: 49 additions & 0 deletions browser/brave_shields/domain_block_page_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "base/test/bind.h"
#include "brave/browser/brave_browser_process_impl.h"
#include "brave/components/brave_shields/browser/ad_block_custom_filters_service.h"
#include "brave/components/brave_shields/browser/brave_shields_util.h"
#include "brave/components/brave_shields/common/features.h"
#include "chrome/browser/extensions/extension_browsertest.h"
#include "chrome/browser/interstitials/security_interstitial_page_test_utils.h"
Expand All @@ -23,6 +24,10 @@
#include "content/public/test/test_navigation_observer.h"
#include "url/gurl.h"

using brave_shields::ControlType;
using brave_shields::ResetBraveShieldsEnabled;
using brave_shields::SetAdControlType;
using brave_shields::SetBraveShieldsEnabled;
using brave_shields::features::kBraveDomainBlock;

class DomainBlockTestBase : public AdBlockServiceTest {
Expand Down Expand Up @@ -100,6 +105,7 @@ class DomainBlockDisabledTest : public DomainBlockTestBase {
IN_PROC_BROWSER_TEST_F(DomainBlockTest, ShowInterstitial) {
ASSERT_TRUE(InstallDefaultAdBlockExtension());
GURL url = embedded_test_server()->GetURL("a.com", "/simple.html");
SetAdControlType(content_settings(), ControlType::BLOCK, url);

// Navigate to a page on a.com. This should work normally.
NavigateTo(url);
Expand All @@ -115,6 +121,7 @@ IN_PROC_BROWSER_TEST_F(DomainBlockTest, ShowInterstitial) {
IN_PROC_BROWSER_TEST_F(DomainBlockTest, ShowInterstitialAndProceed) {
ASSERT_TRUE(InstallDefaultAdBlockExtension());
GURL url = embedded_test_server()->GetURL("a.com", "/simple.html");
SetAdControlType(content_settings(), ControlType::BLOCK, url);

// Navigate to a page on a.com. This should work normally.
NavigateTo(url);
Expand All @@ -138,6 +145,7 @@ IN_PROC_BROWSER_TEST_F(DomainBlockTest, ShowInterstitialAndProceed) {
IN_PROC_BROWSER_TEST_F(DomainBlockTest, ShowInterstitialAndReload) {
ASSERT_TRUE(InstallDefaultAdBlockExtension());
GURL url = embedded_test_server()->GetURL("a.com", "/simple.html");
SetAdControlType(content_settings(), ControlType::BLOCK, url);

// Navigate to a page on a.com. This should work normally.
NavigateTo(url);
Expand All @@ -158,6 +166,7 @@ IN_PROC_BROWSER_TEST_F(DomainBlockTest, ShowInterstitialAndReload) {
IN_PROC_BROWSER_TEST_F(DomainBlockTest, ProceedAndReload) {
ASSERT_TRUE(InstallDefaultAdBlockExtension());
GURL url = embedded_test_server()->GetURL("a.com", "/simple.html");
SetAdControlType(content_settings(), ControlType::BLOCK, url);

// Navigate to a page on a.com. This should work normally.
NavigateTo(url);
Expand Down Expand Up @@ -190,6 +199,7 @@ IN_PROC_BROWSER_TEST_F(DomainBlockTest, ProceedAndReload) {
IN_PROC_BROWSER_TEST_F(DomainBlockTest, ProceedDoesNotAffectNewTabs) {
ASSERT_TRUE(InstallDefaultAdBlockExtension());
GURL url = embedded_test_server()->GetURL("a.com", "/simple.html");
SetAdControlType(content_settings(), ControlType::BLOCK, url);

// Navigate to a page on a.com. This should work normally.
NavigateTo(url);
Expand Down Expand Up @@ -223,6 +233,7 @@ IN_PROC_BROWSER_TEST_F(DomainBlockTest, ProceedDoesNotAffectNewTabs) {
IN_PROC_BROWSER_TEST_F(DomainBlockTest, DontWarnAgainAndProceed) {
ASSERT_TRUE(InstallDefaultAdBlockExtension());
GURL url = embedded_test_server()->GetURL("a.com", "/simple.html");
SetAdControlType(content_settings(), ControlType::BLOCK, url);

// Navigate to a page on a.com. This should work normally.
NavigateTo(url);
Expand Down Expand Up @@ -263,7 +274,9 @@ IN_PROC_BROWSER_TEST_F(DomainBlockTest, DontWarnAgainAndProceed) {
IN_PROC_BROWSER_TEST_F(DomainBlockTest, ShowInterstitialAndGoBack) {
ASSERT_TRUE(InstallDefaultAdBlockExtension());
GURL url_a = embedded_test_server()->GetURL("a.com", "/simple.html");
SetAdControlType(content_settings(), ControlType::BLOCK, url_a);
GURL url_b = embedded_test_server()->GetURL("b.com", "/simple.html");
SetAdControlType(content_settings(), ControlType::BLOCK, url_b);

// Navigate to a page on a.com. This should work normally.
NavigateTo(url_a);
Expand All @@ -286,6 +299,7 @@ IN_PROC_BROWSER_TEST_F(DomainBlockTest, NoFetch) {
ASSERT_TRUE(InstallDefaultAdBlockExtension());
ASSERT_EQ(0, request_count_);
GURL url = embedded_test_server()->GetURL("a.com", "/simple.html");
SetAdControlType(content_settings(), ControlType::BLOCK, url);
BlockDomainByURL(url);
ui_test_utils::AllBrowserTabAddedWaiter new_tab;
ui_test_utils::NavigateToURLWithDisposition(
Expand All @@ -306,8 +320,10 @@ IN_PROC_BROWSER_TEST_F(DomainBlockTest, NoThirdPartyInterstitial) {
->UpdateCustomFilters("||b.com^$third-party"));

GURL url = embedded_test_server()->GetURL("a.com", "/simple_link.html");
SetAdControlType(content_settings(), ControlType::BLOCK, url);
GURL cross_url =
embedded_test_server()->GetURL("a.com", "/cross-site/b.com/simple.html");
SetAdControlType(content_settings(), ControlType::BLOCK, cross_url);

// Navigate to a page on a.com. This should work normally.
NavigateTo(url);
Expand All @@ -331,9 +347,42 @@ IN_PROC_BROWSER_TEST_F(DomainBlockTest, NoThirdPartyInterstitial) {
ASSERT_STRNE("chrome-error://chromewebdata/", location.c_str());
}

IN_PROC_BROWSER_TEST_F(DomainBlockTest, NoInterstitialUnlessAggressive) {
ASSERT_TRUE(InstallDefaultAdBlockExtension());
GURL url = embedded_test_server()->GetURL("a.com", "/simple.html");
SetAdControlType(content_settings(), ControlType::BLOCK, url);

// Navigate to a page on a.com. This should work normally.
NavigateTo(url);
ASSERT_FALSE(IsShowingInterstitial());

// Block a.com in rules but allow a.com via shields, then attempt to navigate
// to a page on a.com. This should not show an interstitial.
BlockDomainByURL(url);
SetAdControlType(content_settings(), ControlType::ALLOW, url);
NavigateTo(url);
ASSERT_FALSE(IsShowingInterstitial());

// Block a.com in rules but set a.com to default shield settings, then attempt
// to navigate to a page on a.com. This should not show an interstitial.
BlockDomainByURL(url);
ResetBraveShieldsEnabled(content_settings(), url);
NavigateTo(url);
ASSERT_FALSE(IsShowingInterstitial());

// Block a.com in rules but drop shields, then attempt to navigate
// to a page on a.com. This should not show an interstitial.
BlockDomainByURL(url);
SetAdControlType(content_settings(), ControlType::BLOCK, url);
SetBraveShieldsEnabled(content_settings(), false /* enable */, url);
NavigateTo(url);
ASSERT_FALSE(IsShowingInterstitial());
}

IN_PROC_BROWSER_TEST_F(DomainBlockDisabledTest, NoInterstitial) {
ASSERT_TRUE(InstallDefaultAdBlockExtension());
GURL url = embedded_test_server()->GetURL("a.com", "/simple.html");
SetAdControlType(content_settings(), ControlType::BLOCK, url);

// Navigate to a page on a.com. This should work normally.
NavigateTo(url);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@
#include "base/threading/thread_task_runner_handle.h"
#include "brave/components/brave_shields/browser/ad_block_custom_filters_service.h"
#include "brave/components/brave_shields/browser/ad_block_service.h"
#include "brave/components/brave_shields/browser/brave_shields_util.h"
#include "brave/components/brave_shields/browser/domain_block_controller_client.h"
#include "brave/components/brave_shields/browser/domain_block_page.h"
#include "brave/components/brave_shields/browser/domain_block_tab_storage.h"
#include "brave/components/brave_shields/common/features.h"
#include "components/content_settings/core/browser/host_content_settings_map.h"
#include "components/prefs/pref_service.h"
#include "components/security_interstitials/content/security_interstitial_tab_helper.h"
#include "components/user_prefs/user_prefs.h"
Expand Down Expand Up @@ -54,24 +56,27 @@ DomainBlockNavigationThrottle::MaybeCreateThrottleFor(
content::NavigationHandle* navigation_handle,
AdBlockService* ad_block_service,
AdBlockCustomFiltersService* ad_block_custom_filters_service,
HostContentSettingsMap* content_settings,
const std::string& locale) {
if (!ad_block_service || !ad_block_custom_filters_service)
return nullptr;
if (!base::FeatureList::IsEnabled(brave_shields::features::kBraveDomainBlock))
return nullptr;
return std::make_unique<DomainBlockNavigationThrottle>(
navigation_handle, ad_block_service, ad_block_custom_filters_service,
locale);
content_settings, locale);
}

DomainBlockNavigationThrottle::DomainBlockNavigationThrottle(
content::NavigationHandle* navigation_handle,
AdBlockService* ad_block_service,
AdBlockCustomFiltersService* ad_block_custom_filters_service,
HostContentSettingsMap* content_settings,
const std::string& locale)
: content::NavigationThrottle(navigation_handle),
ad_block_service_(ad_block_service),
ad_block_custom_filters_service_(ad_block_custom_filters_service),
content_settings_(content_settings),
locale_(locale) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
}
Expand All @@ -97,6 +102,14 @@ DomainBlockNavigationThrottle::WillStartRequest() {
if (!request_url.SchemeIsHTTPOrHTTPS())
return content::NavigationThrottle::PROCEED;

// Only block if Brave Shields is enabled and tracker/ad blocking is set to
// aggressive.
if (!brave_shields::GetBraveShieldsEnabled(content_settings_, request_url))
return content::NavigationThrottle::PROCEED;
if (brave_shields::GetAdControlType(content_settings_, request_url) !=
ControlType::BLOCK)
return content::NavigationThrottle::PROCEED;

// If user has just chosen to proceed on our interstitial, don't show
// another one.
content::WebContents* web_contents = handle->GetWebContents();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include "content/public/browser/navigation_throttle.h"
#include "url/gurl.h"

class HostContentSettingsMap;

namespace content {
class NavigationHandle;
class WebContents;
Expand All @@ -30,6 +32,7 @@ class DomainBlockNavigationThrottle : public content::NavigationThrottle {
content::NavigationHandle* navigation_handle,
AdBlockService* ad_block_service,
AdBlockCustomFiltersService* ad_block_custom_filters_service,
HostContentSettingsMap* content_settings,
const std::string& locale);
~DomainBlockNavigationThrottle() override;

Expand All @@ -41,6 +44,7 @@ class DomainBlockNavigationThrottle : public content::NavigationThrottle {
content::NavigationHandle* navigation_handle,
AdBlockService* ad_block_service,
AdBlockCustomFiltersService* ad_block_custom_filters_service,
HostContentSettingsMap* content_settings,
const std::string& locale);

// content::NavigationThrottle implementation:
Expand All @@ -55,6 +59,7 @@ class DomainBlockNavigationThrottle : public content::NavigationThrottle {

AdBlockService* ad_block_service_ = nullptr;
AdBlockCustomFiltersService* ad_block_custom_filters_service_ = nullptr;
HostContentSettingsMap* content_settings_ = nullptr;
std::string locale_;
base::WeakPtrFactory<DomainBlockNavigationThrottle> weak_ptr_factory_{this};
};
Expand Down

0 comments on commit 869537b

Please sign in to comment.