From f997dbf43bc9a2da0a7457a0acac8d26a127f1f4 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Mon, 2 Nov 2020 21:42:12 -0700 Subject: [PATCH 1/4] Revert "Merge pull request #7028 from brave/bsc-revert-6632" This reverts commit 12ea1cd1cf9ebdc63c8701b0c22ec50d0d09114b, reversing changes made to fbce153e8b8a527845ce58727237878b02c60bba. --- .../ad_block_service_browsertest.cc | 45 +++++-------------- ...ave_ad_block_tp_network_delegate_helper.cc | 4 +- .../net/brave_proxying_url_loader_factory.cc | 3 +- browser/net/brave_request_handler.cc | 2 +- browser/net/url_context.h | 5 ++- .../browser/ad_block_base_service.cc | 5 +-- .../browser/ad_block_base_service.h | 1 - .../ad_block_regional_service_manager.cc | 3 +- .../ad_block_regional_service_manager.h | 1 - .../brave_shields/browser/ad_block_service.cc | 10 ++--- .../brave_shields/browser/ad_block_service.h | 1 - .../browser/base_brave_shields_service.cc | 2 - .../browser/base_brave_shields_service.h | 1 - .../browser/tracking_protection_service.cc | 3 +- .../browser/tracking_protection_service.h | 3 +- 15 files changed, 28 insertions(+), 61 deletions(-) diff --git a/browser/brave_shields/ad_block_service_browsertest.cc b/browser/brave_shields/ad_block_service_browsertest.cc index bca56bc6a8bc..634de60c9ba4 100644 --- a/browser/brave_shields/ad_block_service_browsertest.cc +++ b/browser/brave_shields/ad_block_service_browsertest.cc @@ -273,7 +273,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, AdsGetBlockedByDefaultBlocker) { bool as_expected = false; ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(0, 1, 0, 0, 0, 0);" + "setExpectations(0, 0, 1, 0, 0, 0);" "addImage('ad_banner.png')", &as_expected)); EXPECT_TRUE(as_expected); @@ -317,7 +317,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, AdsGetBlockedByCustomBlocker) { bool as_expected = false; ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(0, 1, 0, 0, 0, 0);" + "setExpectations(0, 0, 1, 0, 0, 0);" "addImage('ad_banner.png')", &as_expected)); EXPECT_TRUE(as_expected); @@ -369,7 +369,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, AdsGetBlockedByRegionalBlocker) { bool as_expected = false; ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(0, 1, 0, 0, 0, 0);" + "setExpectations(0, 0, 1, 0, 0, 0);" "addImage('ad_fr.png')", &as_expected)); EXPECT_TRUE(as_expected); @@ -430,7 +430,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, bool as_expected = false; ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(0, 1, 0, 0, 0, 0);" + "setExpectations(0, 0, 1, 0, 0, 0);" "addImage('v4_specific_banner.png')", &as_expected)); EXPECT_TRUE(as_expected); @@ -453,7 +453,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, TwoSameAdsGetCountedAsOne) { bool as_expected = false; ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(0, 0, 0, 1, 2, 0);" + "setExpectations(0, 0, 0, 1, 0, 2);" "xhr('adbanner.js');" "xhr('normal.js');" "xhr('adbanner.js')", @@ -477,7 +477,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, TwoDiffAdsGetCountedAsTwo) { bool as_expected = false; ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(0, 0, 0, 1, 2, 0);" + "setExpectations(0, 0, 0, 1, 0, 2);" "xhr('adbanner.js?1');" "xhr('normal.js');" "xhr('adbanner.js?2')", @@ -501,7 +501,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, NewTabContinuesToBlock) { bool as_expected = false; ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(0, 0, 0, 0, 1, 0);" + "setExpectations(0, 0, 0, 0, 0, 1);" "xhr('adbanner.js');", &as_expected)); EXPECT_TRUE(as_expected); @@ -512,7 +512,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, NewTabContinuesToBlock) { as_expected = false; ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(0, 0, 0, 0, 1, 0);" + "setExpectations(0, 0, 0, 0, 0, 1);" "xhr('adbanner.js');", &as_expected)); EXPECT_TRUE(as_expected); @@ -536,7 +536,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, SubFrame) { bool as_expected = false; ASSERT_TRUE(ExecuteScriptAndExtractBool(contents->GetAllFrames()[1], - "setExpectations(0, 0, 0, 0, 1, 0);" + "setExpectations(0, 0, 0, 0, 0, 1);" "xhr('adbanner.js?1');", &as_expected)); EXPECT_TRUE(as_expected); @@ -618,7 +618,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, bool as_expected = false; ASSERT_TRUE(ExecuteScriptAndExtractBool( contents, - base::StringPrintf("setExpectations(0, 1, 0, 0, 0, 0);" + base::StringPrintf("setExpectations(0, 0, 1, 0, 0, 0);" "addImage('%s')", resource_url.spec().c_str()), &as_expected)); @@ -639,7 +639,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, BlockNYP) { bool as_expected = false; ASSERT_TRUE(ExecuteScriptAndExtractBool( contents, - base::StringPrintf("setExpectations(0, 1, 0, 0, 0, 0);" + base::StringPrintf("setExpectations(0, 0, 1, 0, 0, 0);" "addImage('%s')", resource_url.spec().c_str()), &as_expected)); @@ -666,7 +666,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, SocialButttonAdBLockTagTest) { bool as_expected = false; ASSERT_TRUE(ExecuteScriptAndExtractBool( contents, - base::StringPrintf("setExpectations(0, 1, 0, 0, 0, 0);" + base::StringPrintf("setExpectations(0, 0, 1, 0, 0, 0);" "addImage('%s')", resource_url.spec().c_str()), &as_expected)); @@ -755,27 +755,6 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, TagPrefsControlTags) { AssertTagExists(brave_shields::kLinkedInEmbeds, false); } -// Make sure that cancelrequest actually blocks -IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CancelRequestOptionTest) { - UpdateAdBlockInstanceWithRules("logo.png$explicitcancel"); - EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL); - GURL tab_url = embedded_test_server()->GetURL("b.com", kAdBlockTestPage); - GURL resource_url = - embedded_test_server()->GetURL("example.com", "/logo.png"); - ui_test_utils::NavigateToURL(browser(), tab_url); - content::WebContents* contents = - browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - base::StringPrintf("setExpectations(0, 0, 1, 0, 0, 0);" - "addImage('%s')", - resource_url.spec().c_str()), - &as_expected)); - EXPECT_TRUE(as_expected); - EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL); -} - // Load a page with a script which uses a redirect data URL. IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, RedirectRulesAreRespected) { UpdateAdBlockInstanceWithRules( diff --git a/browser/net/brave_ad_block_tp_network_delegate_helper.cc b/browser/net/brave_ad_block_tp_network_delegate_helper.cc index 51663ac3bd14..68a4d34a050d 100644 --- a/browser/net/brave_ad_block_tp_network_delegate_helper.cc +++ b/browser/net/brave_ad_block_tp_network_delegate_helper.cc @@ -60,7 +60,7 @@ void ShouldBlockAdOnTaskRunner(std::shared_ptr ctx, std::string tab_host = ctx->tab_origin.host(); if (!g_brave_browser_process->ad_block_service()->ShouldStartRequest( ctx->request_url, ctx->resource_type, tab_host, &did_match_exception, - &ctx->cancel_request_explicitly, &ctx->mock_data_url)) { + &ctx->mock_data_url)) { ctx->blocked_by = kAdBlocked; } else if (!did_match_exception && canonical_name.has_value() && ctx->request_url.host() != *canonical_name && @@ -73,7 +73,7 @@ void ShouldBlockAdOnTaskRunner(std::shared_ptr ctx, if (!g_brave_browser_process->ad_block_service()->ShouldStartRequest( canonical_url, ctx->resource_type, tab_host, &did_match_exception, - &ctx->cancel_request_explicitly, &ctx->mock_data_url)) { + &ctx->mock_data_url)) { ctx->blocked_by = kAdBlocked; } } diff --git a/browser/net/brave_proxying_url_loader_factory.cc b/browser/net/brave_proxying_url_loader_factory.cc index f368590e2c64..641f2fa0da08 100644 --- a/browser/net/brave_proxying_url_loader_factory.cc +++ b/browser/net/brave_proxying_url_loader_factory.cc @@ -348,10 +348,11 @@ void BraveProxyingURLLoaderFactory::InProgressRequest:: // TODO(iefremov): Shorten if (ctx_->blocked_by != brave::kNotBlocked) { - if (ctx_->cancel_request_explicitly) { + if (!ctx_->ShouldMockRequest()) { OnRequestError(network::URLLoaderCompletionStatus(net::ERR_ABORTED)); return; } + auto response = network::mojom::URLResponseHead::New(); std::string response_data; brave_shields::MakeStubResponse(ctx_->mock_data_url, request_, &response, diff --git a/browser/net/brave_request_handler.cc b/browser/net/brave_request_handler.cc index 587d795a57da..f2b0c48ca933 100644 --- a/browser/net/brave_request_handler.cc +++ b/browser/net/brave_request_handler.cc @@ -297,7 +297,7 @@ void BraveRequestHandler::RunNextCallback( *ctx->new_url = GURL(ctx->new_url_spec); } if (ctx->blocked_by == brave::kAdBlocked) { - if (ctx->cancel_request_explicitly) { + if (!ctx->ShouldMockRequest()) { RunCallbackForRequestIdentifier(ctx->request_identifier, net::ERR_ABORTED); return; diff --git a/browser/net/url_context.h b/browser/net/url_context.h index 1ee6e54f1967..ad90046be9af 100644 --- a/browser/net/url_context.h +++ b/browser/net/url_context.h @@ -96,10 +96,13 @@ struct BraveRequestInfo { BraveNetworkDelegateEventType event_type = kUnknownEventType; const base::ListValue* referral_headers_list = nullptr; BlockedBy blocked_by = kNotBlocked; - bool cancel_request_explicitly = false; std::string mock_data_url; bool ipfs_local = true; + bool ShouldMockRequest() const { + return !mock_data_url.empty(); + } + net::NetworkIsolationKey network_isolation_key = net::NetworkIsolationKey(); // Default to invalid type for resource_type, so delegate helpers diff --git a/components/brave_shields/browser/ad_block_base_service.cc b/components/brave_shields/browser/ad_block_base_service.cc index ff59558fd123..86f8ec652637 100644 --- a/components/brave_shields/browser/ad_block_base_service.cc +++ b/components/brave_shields/browser/ad_block_base_service.cc @@ -118,7 +118,6 @@ bool AdBlockBaseService::ShouldStartRequest( blink::mojom::ResourceType resource_type, const std::string& tab_host, bool* did_match_exception, - bool* cancel_request_explicitly, std::string* mock_data_url) { DCHECK(GetTaskRunner()->RunsTasksInCurrentSequence()); @@ -129,15 +128,13 @@ bool AdBlockBaseService::ShouldStartRequest( url, url::Origin::CreateFromNormalizedTuple("https", tab_host.c_str(), 80), INCLUDE_PRIVATE_REGISTRIES); + // TODO(spinda): Remove explicit_cancel here when removed from adblock-rust. bool explicit_cancel; bool saved_from_exception; if (ad_block_client_->matches( url.spec(), url.host(), tab_host, is_third_party, ResourceTypeToString(resource_type), &explicit_cancel, &saved_from_exception, mock_data_url)) { - if (cancel_request_explicitly) { - *cancel_request_explicitly = explicit_cancel; - } // We'd only possibly match an exception filter if we're returning true. if (did_match_exception) { *did_match_exception = false; diff --git a/components/brave_shields/browser/ad_block_base_service.h b/components/brave_shields/browser/ad_block_base_service.h index 3c4a57a50197..bcb632726e3e 100644 --- a/components/brave_shields/browser/ad_block_base_service.h +++ b/components/brave_shields/browser/ad_block_base_service.h @@ -44,7 +44,6 @@ class AdBlockBaseService : public BaseBraveShieldsService { blink::mojom::ResourceType resource_type, const std::string& tab_host, bool* did_match_exception, - bool* cancel_request_explicitly, std::string* mock_data_url) override; void AddResources(const std::string& resources); void EnableTag(const std::string& tag, bool enabled); diff --git a/components/brave_shields/browser/ad_block_regional_service_manager.cc b/components/brave_shields/browser/ad_block_regional_service_manager.cc index a9ad4e6de547..d2bb45190f98 100644 --- a/components/brave_shields/browser/ad_block_regional_service_manager.cc +++ b/components/brave_shields/browser/ad_block_regional_service_manager.cc @@ -128,13 +128,12 @@ bool AdBlockRegionalServiceManager::ShouldStartRequest( blink::mojom::ResourceType resource_type, const std::string& tab_host, bool* matching_exception_filter, - bool* cancel_request_explicitly, std::string* mock_data_url) { base::AutoLock lock(regional_services_lock_); for (const auto& regional_service : regional_services_) { if (!regional_service.second->ShouldStartRequest( url, resource_type, tab_host, matching_exception_filter, - cancel_request_explicitly, mock_data_url)) { + mock_data_url)) { return false; } if (matching_exception_filter && *matching_exception_filter) { diff --git a/components/brave_shields/browser/ad_block_regional_service_manager.h b/components/brave_shields/browser/ad_block_regional_service_manager.h index b4ba4c616616..d4ac68bc3278 100644 --- a/components/brave_shields/browser/ad_block_regional_service_manager.h +++ b/components/brave_shields/browser/ad_block_regional_service_manager.h @@ -51,7 +51,6 @@ class AdBlockRegionalServiceManager { blink::mojom::ResourceType resource_type, const std::string& tab_host, bool* matching_exception_filter, - bool* cancel_request_explicitly, std::string* mock_data_url); void EnableTag(const std::string& tag, bool enabled); void AddResources(const std::string& resources); diff --git a/components/brave_shields/browser/ad_block_service.cc b/components/brave_shields/browser/ad_block_service.cc index a8f30d4b1371..42ed5cc0f96a 100644 --- a/components/brave_shields/browser/ad_block_service.cc +++ b/components/brave_shields/browser/ad_block_service.cc @@ -77,12 +77,10 @@ bool AdBlockService::ShouldStartRequest( blink::mojom::ResourceType resource_type, const std::string& tab_host, bool* did_match_exception, - bool* cancel_request_explicitly, std::string* mock_data_url) { if (!AdBlockBaseService::ShouldStartRequest( - url, resource_type, tab_host, did_match_exception, - cancel_request_explicitly, mock_data_url)) { + url, resource_type, tab_host, did_match_exception, mock_data_url)) { return false; } if (did_match_exception && *did_match_exception) { @@ -90,8 +88,7 @@ bool AdBlockService::ShouldStartRequest( } if (!regional_service_manager()->ShouldStartRequest( - url, resource_type, tab_host, did_match_exception, - cancel_request_explicitly, mock_data_url)) { + url, resource_type, tab_host, did_match_exception, mock_data_url)) { return false; } if (did_match_exception && *did_match_exception) { @@ -99,8 +96,7 @@ bool AdBlockService::ShouldStartRequest( } if (!custom_filters_service()->ShouldStartRequest( - url, resource_type, tab_host, did_match_exception, - cancel_request_explicitly, mock_data_url)) { + url, resource_type, tab_host, did_match_exception, mock_data_url)) { return false; } if (did_match_exception && *did_match_exception) { diff --git a/components/brave_shields/browser/ad_block_service.h b/components/brave_shields/browser/ad_block_service.h index c3adbdde5f7e..d00572ab7caa 100644 --- a/components/brave_shields/browser/ad_block_service.h +++ b/components/brave_shields/browser/ad_block_service.h @@ -50,7 +50,6 @@ class AdBlockService : public AdBlockBaseService { blink::mojom::ResourceType resource_type, const std::string& tab_host, bool* did_match_exception, - bool* cancel_request_explicitly, std::string* mock_data_url) override; AdBlockRegionalServiceManager* regional_service_manager(); diff --git a/components/brave_shields/browser/base_brave_shields_service.cc b/components/brave_shields/browser/base_brave_shields_service.cc index b82a3695fcb7..1b900a74951a 100644 --- a/components/brave_shields/browser/base_brave_shields_service.cc +++ b/components/brave_shields/browser/base_brave_shields_service.cc @@ -54,12 +54,10 @@ bool BaseBraveShieldsService::ShouldStartRequest( blink::mojom::ResourceType resource_type, const std::string& tab_host, bool* did_match_exception, - bool* cancel_request_explicitly, std::string* mock_data_url) { if (did_match_exception) { *did_match_exception = false; } - // Intentionally don't set cancel_request_explicitly return true; } diff --git a/components/brave_shields/browser/base_brave_shields_service.h b/components/brave_shields/browser/base_brave_shields_service.h index ad6f9a44d6e3..f87f0c19e76d 100644 --- a/components/brave_shields/browser/base_brave_shields_service.h +++ b/components/brave_shields/browser/base_brave_shields_service.h @@ -35,7 +35,6 @@ class BaseBraveShieldsService : public BraveComponent { blink::mojom::ResourceType resource_type, const std::string& tab_host, bool* did_match_exception, - bool* cancel_request_explicitly, std::string* mock_data_url); protected: diff --git a/components/brave_shields/browser/tracking_protection_service.cc b/components/brave_shields/browser/tracking_protection_service.cc index 59a640ad7376..6ff5633d3ff7 100644 --- a/components/brave_shields/browser/tracking_protection_service.cc +++ b/components/brave_shields/browser/tracking_protection_service.cc @@ -197,8 +197,7 @@ bool TrackingProtectionService::ShouldStartRequest( const GURL& url, blink::mojom::ResourceType resource_type, const std::string& tab_host, - bool* matching_exception_filter, - bool* cancel_request_explicitly) { + bool* matching_exception_filter) { DCHECK_CURRENTLY_ON(BrowserThread::IO); // There are no exceptions in the TP service, but exceptions are // combined with brave/ad-block. diff --git a/components/brave_shields/browser/tracking_protection_service.h b/components/brave_shields/browser/tracking_protection_service.h index 9d505dd2631b..27d85d0414b3 100644 --- a/components/brave_shields/browser/tracking_protection_service.h +++ b/components/brave_shields/browser/tracking_protection_service.h @@ -43,8 +43,7 @@ class TrackingProtectionService : public LocalDataFilesObserver { bool ShouldStartRequest(const GURL& spec, blink::mojom::ResourceType resource_type, const std::string& tab_host, - bool* matching_exception_filter, - bool* cancel_request_explicitly); + bool* matching_exception_filter); // implementation of LocalDataFilesObserver void OnComponentReady(const std::string& component_id, From 99b0cf8bd4f8779c1ac16d9e64531f320e14eed3 Mon Sep 17 00:00:00 2001 From: Anton Lazarev Date: Tue, 3 Nov 2020 13:41:02 -0500 Subject: [PATCH 2/4] fix failing tests The XHRs from the 4 failing tests were triggering `onabort`, not `onerror`. --- browser/net/brave_proxying_url_loader_factory.cc | 3 ++- browser/net/brave_request_handler.cc | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/browser/net/brave_proxying_url_loader_factory.cc b/browser/net/brave_proxying_url_loader_factory.cc index 641f2fa0da08..427a697dd7f5 100644 --- a/browser/net/brave_proxying_url_loader_factory.cc +++ b/browser/net/brave_proxying_url_loader_factory.cc @@ -349,7 +349,8 @@ void BraveProxyingURLLoaderFactory::InProgressRequest:: // TODO(iefremov): Shorten if (ctx_->blocked_by != brave::kNotBlocked) { if (!ctx_->ShouldMockRequest()) { - OnRequestError(network::URLLoaderCompletionStatus(net::ERR_ABORTED)); + OnRequestError( + network::URLLoaderCompletionStatus(net::ERR_BLOCKED_BY_CLIENT)); return; } diff --git a/browser/net/brave_request_handler.cc b/browser/net/brave_request_handler.cc index f2b0c48ca933..2a8a2f20bcc9 100644 --- a/browser/net/brave_request_handler.cc +++ b/browser/net/brave_request_handler.cc @@ -299,7 +299,7 @@ void BraveRequestHandler::RunNextCallback( if (ctx->blocked_by == brave::kAdBlocked) { if (!ctx->ShouldMockRequest()) { RunCallbackForRequestIdentifier(ctx->request_identifier, - net::ERR_ABORTED); + net::ERR_BLOCKED_BY_CLIENT); return; } } From f15718107e3d6e7fd6919f4dd3ca371c9c52c1a2 Mon Sep 17 00:00:00 2001 From: Anton Lazarev Date: Tue, 3 Nov 2020 13:51:00 -0500 Subject: [PATCH 3/4] modernize adblock tests --- .../ad_block_service_browsertest.cc | 464 ++++++------------ test/data/blocking.html | 100 ++-- test/data/cosmetic_filtering.html | 21 +- 3 files changed, 229 insertions(+), 356 deletions(-) diff --git a/browser/brave_shields/ad_block_service_browsertest.cc b/browser/brave_shields/ad_block_service_browsertest.cc index 634de60c9ba4..979f2b925aa7 100644 --- a/browser/brave_shields/ad_block_service_browsertest.cc +++ b/browser/brave_shields/ad_block_service_browsertest.cc @@ -271,12 +271,9 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, AdsGetBlockedByDefaultBlocker) { content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(0, 0, 1, 0, 0, 0);" - "addImage('ad_banner.png')", - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + "setExpectations(0, 0, 1, 0, 0, 0);" + "addImage('ad_banner.png')")); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL); } @@ -294,12 +291,9 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(1, 0, 0, 0, 0, 0);" - "addImage('logo.png')", - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + "setExpectations(1, 0, 0, 0, 0, 0);" + "addImage('logo.png')")); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL); } @@ -315,12 +309,9 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, AdsGetBlockedByCustomBlocker) { content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(0, 0, 1, 0, 0, 0);" - "addImage('ad_banner.png')", - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + "setExpectations(0, 0, 1, 0, 0, 0);" + "addImage('ad_banner.png')")); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL); } @@ -339,12 +330,9 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(1, 0, 0, 0, 0, 0);" - "addImage('logo.png')", - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + "setExpectations(1, 0, 0, 0, 0, 0);" + "addImage('logo.png')")); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL); } @@ -367,12 +355,9 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, AdsGetBlockedByRegionalBlocker) { content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(0, 0, 1, 0, 0, 0);" - "addImage('ad_fr.png')", - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + "setExpectations(0, 0, 1, 0, 0, 0);" + "addImage('ad_fr.png')")); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL); } @@ -396,12 +381,9 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(1, 0, 0, 0, 0, 0);" - "addImage('logo.png')", - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + "setExpectations(1, 0, 0, 0, 0, 0);" + "addImage('logo.png')")); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL); } @@ -428,12 +410,9 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(0, 0, 1, 0, 0, 0);" - "addImage('v4_specific_banner.png')", - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + "setExpectations(0, 0, 1, 0, 0, 0);" + "addImage('v4_specific_banner.png')")); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL); } @@ -451,14 +430,15 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, TwoSameAdsGetCountedAsOne) { content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(0, 0, 0, 1, 0, 2);" - "xhr('adbanner.js');" - "xhr('normal.js');" - "xhr('adbanner.js')", - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + "setExpectations(0, 0, 0, 0, 0, 1);" + "xhr('adbanner.js')")); + ASSERT_EQ(true, EvalJs(contents, + "setExpectations(0, 0, 0, 1, 0, 1);" + "xhr('normal.js')")); + ASSERT_EQ(true, EvalJs(contents, + "setExpectations(0, 0, 0, 1, 0, 2);" + "xhr('adbanner.js')")); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL); } @@ -475,14 +455,15 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, TwoDiffAdsGetCountedAsTwo) { content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(0, 0, 0, 1, 0, 2);" - "xhr('adbanner.js?1');" - "xhr('normal.js');" - "xhr('adbanner.js?2')", - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + "setExpectations(0, 0, 0, 0, 0, 1);" + "xhr('adbanner.js?1')")); + ASSERT_EQ(true, EvalJs(contents, + "setExpectations(0, 0, 0, 1, 0, 1);" + "xhr('normal.js')")); + ASSERT_EQ(true, EvalJs(contents, + "setExpectations(0, 0, 0, 1, 0, 2);" + "xhr('adbanner.js?2')")); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 2ULL); } @@ -499,23 +480,17 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, NewTabContinuesToBlock) { content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(0, 0, 0, 0, 0, 1);" - "xhr('adbanner.js');", - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + "setExpectations(0, 0, 0, 0, 0, 1);" + "xhr('adbanner.js')")); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL); ui_test_utils::NavigateToURL(browser(), url); contents = browser()->tab_strip_model()->GetActiveWebContents(); - as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(0, 0, 0, 0, 0, 1);" - "xhr('adbanner.js');", - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + "setExpectations(0, 0, 0, 0, 0, 1);" + "xhr('adbanner.js')")); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 2ULL); ui_test_utils::NavigateToURL(browser(), url); @@ -534,12 +509,9 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, SubFrame) { content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool(contents->GetAllFrames()[1], - "setExpectations(0, 0, 0, 0, 0, 1);" - "xhr('adbanner.js?1');", - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents->GetAllFrames()[1], + "setExpectations(0, 0, 0, 0, 0, 1);" + "xhr('adbanner.js?1')")); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL); // Check also an explicit request for a script since it is a common real-world @@ -574,12 +546,9 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, ui_test_utils::NavigateToURL(browser(), url); content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(1, 0, 0, 0, 0, 0);" - "addImage('ad_fr.png')", - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + "setExpectations(1, 0, 0, 0, 0, 0);" + "addImage('ad_fr.png')")); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL); } @@ -594,14 +563,10 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, AdBlockThirdPartyWorksByETLDP1) { ui_test_utils::NavigateToURL(browser(), tab_url); content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - base::StringPrintf("setExpectations(1, 0, 0, 0, 0, 0);" - "addImage('%s')", - resource_url.spec().c_str()), - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + base::StringPrintf("setExpectations(1, 0, 0, 0, 0, 0);" + "addImage('%s')", + resource_url.spec().c_str()))); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL); } @@ -615,14 +580,10 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, ui_test_utils::NavigateToURL(browser(), tab_url); content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - base::StringPrintf("setExpectations(0, 0, 1, 0, 0, 0);" - "addImage('%s')", - resource_url.spec().c_str()), - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + base::StringPrintf("setExpectations(0, 0, 1, 0, 0, 0);" + "addImage('%s')", + resource_url.spec().c_str()))); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL); } @@ -636,19 +597,15 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, BlockNYP) { ui_test_utils::NavigateToURL(browser(), tab_url); content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - base::StringPrintf("setExpectations(0, 0, 1, 0, 0, 0);" - "addImage('%s')", - resource_url.spec().c_str()), - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + base::StringPrintf("setExpectations(0, 0, 1, 0, 0, 0);" + "addImage('%s')", + resource_url.spec().c_str()))); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL); } // Tags for social buttons work -IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, SocialButttonAdBLockTagTest) { +IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, SocialButttonAdBlockTagTest) { UpdateAdBlockInstanceWithRules( base::StringPrintf("||example.com^$tag=%s", brave_shields::kFacebookEmbeds) @@ -663,14 +620,10 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, SocialButttonAdBLockTagTest) { ui_test_utils::NavigateToURL(browser(), tab_url); content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - base::StringPrintf("setExpectations(0, 0, 1, 0, 0, 0);" - "addImage('%s')", - resource_url.spec().c_str()), - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + base::StringPrintf("setExpectations(0, 0, 1, 0, 0, 0);" + "addImage('%s')", + resource_url.spec().c_str()))); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL); } @@ -687,14 +640,10 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, SocialButttonAdBlockDiffTagTest) { ui_test_utils::NavigateToURL(browser(), tab_url); content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - base::StringPrintf("setExpectations(1, 0, 0, 0, 0, 0);" - "addImage('%s')", - resource_url.spec().c_str()), - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + base::StringPrintf("setExpectations(1, 0, 0, 0, 0, 0);" + "addImage('%s')", + resource_url.spec().c_str()))); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL); } @@ -772,23 +721,20 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, RedirectRulesAreRespected) { ])"); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL); - const GURL url = embedded_test_server()->GetURL("example.com", - kAdBlockTestPage); + const GURL url = + embedded_test_server()->GetURL("example.com", kAdBlockTestPage); ui_test_utils::NavigateToURL(browser(), url); content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); const std::string noopjs = "(function() {\\n \\'use strict\\';\\n})();\\n"; - bool as_expected = false; const GURL resource_url = embedded_test_server()->GetURL("example.com", "/js_mock_me.js"); - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - base::StringPrintf("setExpectations(0, 0, 0, 1, 0, 0);" - "xhr_expect_content('%s', '%s');", - resource_url.spec().c_str(), noopjs.c_str()), - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, + EvalJs(contents, base::StringPrintf( + "setExpectations(0, 0, 0, 1, 0, 0);" + "xhr_expect_content('%s', '%s');", + resource_url.spec().c_str(), noopjs.c_str()))); EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL); } @@ -804,81 +750,53 @@ class CosmeticFilteringFlagDisabledTest : public AdBlockServiceTest { // Ensure no cosmetic filtering occurs when the feature flag is disabled IN_PROC_BROWSER_TEST_F(CosmeticFilteringFlagDisabledTest, - CosmeticFilteringSimple) { + CosmeticFilteringSimple) { UpdateAdBlockInstanceWithRules( "b.com###ad-banner\n" "##.ad"); WaitForBraveExtensionShieldsDataReady(); - GURL tab_url = embedded_test_server()->GetURL("b.com", - "/cosmetic_filtering.html"); + GURL tab_url = + embedded_test_server()->GetURL("b.com", "/cosmetic_filtering.html"); ui_test_utils::NavigateToURL(browser(), tab_url); content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "checkSelector('#ad-banner', 'display', 'block')", - &as_expected)); - EXPECT_TRUE(as_expected); - - as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "checkSelector('.ad-banner', 'display', 'block')", - &as_expected)); - EXPECT_TRUE(as_expected); - - as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "checkSelector('.ad', 'display', 'block')", - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + "checkSelector('#ad-banner', 'display', 'block')")); + + ASSERT_EQ(true, EvalJs(contents, + "checkSelector('.ad-banner', 'display', 'block')")); + + ASSERT_EQ(true, EvalJs(contents, "checkSelector('.ad', 'display', 'block')")); } // Ensure no cosmetic filtering occurs when the shields setting is disabled IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringDisabled) { brave_shields::SetCosmeticFilteringControlType( - content_settings(), - brave_shields::ControlType::ALLOW, - GURL()); + content_settings(), brave_shields::ControlType::ALLOW, GURL()); UpdateAdBlockInstanceWithRules( "b.com###ad-banner\n" "##.ad"); WaitForBraveExtensionShieldsDataReady(); - GURL tab_url = embedded_test_server()->GetURL("b.com", - "/cosmetic_filtering.html"); + GURL tab_url = + embedded_test_server()->GetURL("b.com", "/cosmetic_filtering.html"); ui_test_utils::NavigateToURL(browser(), tab_url); content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "checkSelector('#ad-banner', 'display', 'block')", - &as_expected)); - EXPECT_TRUE(as_expected); - - as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "checkSelector('.ad-banner', 'display', 'block')", - &as_expected)); - EXPECT_TRUE(as_expected); - - as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "checkSelector('.ad', 'display', 'block')", - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + "checkSelector('#ad-banner', 'display', 'block')")); + + ASSERT_EQ(true, EvalJs(contents, + "checkSelector('.ad-banner', 'display', 'block')")); + + ASSERT_EQ(true, EvalJs(contents, "checkSelector('.ad', 'display', 'block')")); } // Test simple cosmetic filtering @@ -889,81 +807,56 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringSimple) { WaitForBraveExtensionShieldsDataReady(); - GURL tab_url = embedded_test_server()->GetURL("b.com", - "/cosmetic_filtering.html"); + GURL tab_url = + embedded_test_server()->GetURL("b.com", "/cosmetic_filtering.html"); ui_test_utils::NavigateToURL(browser(), tab_url); content::WebContents* contents = - browser()->tab_strip_model()->GetActiveWebContents(); - - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "checkSelector('#ad-banner', 'display', 'none')", - &as_expected)); - EXPECT_TRUE(as_expected); - - as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "checkSelector('.ad-banner', 'display', 'block')", - &as_expected)); - EXPECT_TRUE(as_expected); - - as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "checkSelector('.ad', 'display', 'none')", - &as_expected)); - EXPECT_TRUE(as_expected); + browser()->tab_strip_model()->GetActiveWebContents(); + + ASSERT_EQ(true, + EvalJs(contents, "checkSelector('#ad-banner', 'display', 'none')")); + + ASSERT_EQ(true, EvalJs(contents, + "checkSelector('.ad-banner', 'display', 'block')")); + + ASSERT_EQ(true, EvalJs(contents, "checkSelector('.ad', 'display', 'none')")); } // Test cosmetic filtering ignores content determined to be 1st party -IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, - CosmeticFilteringProtect1p) { +IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringProtect1p) { UpdateAdBlockInstanceWithRules("b.com##.fpsponsored\n"); WaitForBraveExtensionShieldsDataReady(); - GURL tab_url = embedded_test_server()->GetURL("b.com", - "/cosmetic_filtering.html"); + GURL tab_url = + embedded_test_server()->GetURL("b.com", "/cosmetic_filtering.html"); ui_test_utils::NavigateToURL(browser(), tab_url); content::WebContents* contents = - browser()->tab_strip_model()->GetActiveWebContents(); - - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "checkSelector('.fpsponsored', 'display', 'block')", - &as_expected)); - EXPECT_TRUE(as_expected); + browser()->tab_strip_model()->GetActiveWebContents(); + + ASSERT_EQ(true, EvalJs(contents, + "checkSelector('.fpsponsored', 'display', 'block')")); } // Test cosmetic filtering bypasses 1st party checks when toggled -IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, - CosmeticFilteringHide1pContent) { +IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringHide1pContent) { brave_shields::SetCosmeticFilteringControlType( - content_settings(), - brave_shields::ControlType::BLOCK, - GURL()); + content_settings(), brave_shields::ControlType::BLOCK, GURL()); UpdateAdBlockInstanceWithRules("b.com##.fpsponsored\n"); WaitForBraveExtensionShieldsDataReady(); - GURL tab_url = embedded_test_server()->GetURL("b.com", - "/cosmetic_filtering.html"); + GURL tab_url = + embedded_test_server()->GetURL("b.com", "/cosmetic_filtering.html"); ui_test_utils::NavigateToURL(browser(), tab_url); content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "checkSelector('.fpsponsored', 'display', 'none')", - &as_expected)); - EXPECT_TRUE(as_expected); + ASSERT_EQ(true, EvalJs(contents, + "checkSelector('.fpsponsored', 'display', 'none')")); } // Test cosmetic filtering on elements added dynamically @@ -972,27 +865,19 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringDynamic) { WaitForBraveExtensionShieldsDataReady(); - GURL tab_url = embedded_test_server()->GetURL("b.com", - "/cosmetic_filtering.html"); + GURL tab_url = + embedded_test_server()->GetURL("b.com", "/cosmetic_filtering.html"); ui_test_utils::NavigateToURL(browser(), tab_url); content::WebContents* contents = - browser()->tab_strip_model()->GetActiveWebContents(); - - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "addElementsDynamically();\n" - "checkSelector('.blockme', 'display', 'none')", - &as_expected)); - EXPECT_TRUE(as_expected); - - as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "checkSelector('.dontblockme', 'display', 'block')", - &as_expected)); - EXPECT_TRUE(as_expected); + browser()->tab_strip_model()->GetActiveWebContents(); + + ASSERT_EQ(true, EvalJs(contents, + "addElementsDynamically();\n" + "checkSelector('.blockme', 'display', 'none')")); + + ASSERT_EQ(true, EvalJs(contents, + "checkSelector('.dontblockme', 'display', 'block')")); } // Test cosmetic filtering ignores generic cosmetic rules in the presence of a @@ -1011,23 +896,16 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringGenerichide) { ui_test_utils::NavigateToURL(browser(), tab_url); content::WebContents* contents = - browser()->tab_strip_model()->GetActiveWebContents(); - - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "addElementsDynamically();\n" - "checkSelector('.blockme', 'display', 'inline')", - &as_expected)); - EXPECT_TRUE(as_expected); - - as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "checkSelector('img[src=\"https://example.com/logo.png\"]', " - "'display', 'inline')", - &as_expected)); - EXPECT_TRUE(as_expected); + browser()->tab_strip_model()->GetActiveWebContents(); + + ASSERT_EQ(true, EvalJs(contents, + "addElementsDynamically();\n" + "checkSelector('.blockme', 'display', 'inline')")); + + ASSERT_EQ(true, + EvalJs(contents, + "checkSelector('img[src=\"https://example.com/logo.png\"]', " + "'display', 'inline')")); } // Test custom style rules @@ -1042,14 +920,10 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, ui_test_utils::NavigateToURL(browser(), tab_url); content::WebContents* contents = - browser()->tab_strip_model()->GetActiveWebContents(); - - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "checkSelector('.ad', 'padding-bottom', '10px')", - &as_expected)); - EXPECT_TRUE(as_expected); + browser()->tab_strip_model()->GetActiveWebContents(); + + ASSERT_EQ(true, + EvalJs(contents, "checkSelector('.ad', 'padding-bottom', '10px')")); } // Test rules overridden by hostname-specific exception rules @@ -1067,21 +941,12 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringUnhide) { ui_test_utils::NavigateToURL(browser(), tab_url); content::WebContents* contents = - browser()->tab_strip_model()->GetActiveWebContents(); - - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "checkSelector('.ad', 'display', 'block')", - &as_expected)); - EXPECT_TRUE(as_expected); - - as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "checkSelector('#ad-banner', 'display', 'none')", - &as_expected)); - EXPECT_TRUE(as_expected); + browser()->tab_strip_model()->GetActiveWebContents(); + + ASSERT_EQ(true, EvalJs(contents, "checkSelector('.ad', 'display', 'block')")); + + ASSERT_EQ(true, + EvalJs(contents, "checkSelector('#ad-banner', 'display', 'none')")); } // Test scriptlet injection that modifies window attributes @@ -1113,14 +978,10 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, ui_test_utils::NavigateToURL(browser(), tab_url); content::WebContents* contents = - browser()->tab_strip_model()->GetActiveWebContents(); - - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "checkSelector('.ad', 'color', 'Impossible value')", - &as_expected)); - EXPECT_TRUE(as_expected); + browser()->tab_strip_model()->GetActiveWebContents(); + + ASSERT_EQ(true, EvalJs(contents, + "checkSelector('.ad', 'color', 'Impossible value')")); } // Test scriptlet injection that modifies window attributes @@ -1144,12 +1005,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, ui_test_utils::NavigateToURL(browser(), tab_url); content::WebContents* contents = - browser()->tab_strip_model()->GetActiveWebContents(); - - bool as_expected = true; - ASSERT_TRUE(ExecuteScriptAndExtractBool( - contents, - "window.domAutomationController.send(show_ad)", - &as_expected)); - EXPECT_TRUE(as_expected); + browser()->tab_strip_model()->GetActiveWebContents(); + + ASSERT_EQ(true, EvalJs(contents, "show_ad")); } diff --git a/test/data/blocking.html b/test/data/blocking.html index 1cf7fcb0f0ae..693c2b9f342a 100644 --- a/test/data/blocking.html +++ b/test/data/blocking.html @@ -16,66 +16,68 @@ // Performs an XHR for the specified src function xhr(src) { - const xhr = new XMLHttpRequest(); - xhr.open('GET', src, true); - xhr.onload = function(e) { - if (xhr.response.length !== 0) { - numXHRLoaded++ - } else { - numXHRLoadedBlank++ + return new Promise(resolve => { + const xhr = new XMLHttpRequest(); + xhr.open('GET', src, true); + xhr.onload = function(e) { + if (xhr.response.length !== 0) { + numXHRLoaded++ + } else { + numXHRLoadedBlank++ + } + resolve(onLoad()) } - onLoad() - } - xhr.onerror = function(e) { - numXHRBlocked++ - onLoad() - } - xhr.send(); + xhr.onerror = function(e) { + numXHRBlocked++ + resolve(onLoad()) + } + xhr.send(); + }); } // Performs an XHR for the specified src and reports successful, // only if the content matches expected_content. function xhr_expect_content(src, expected_content) { - const xhr = new XMLHttpRequest(); - xhr.open('GET', src, true); - xhr.onload = function(e) { - if (xhr.response === expected_content) { - numXHRLoaded++ + return new Promise(resolve => { + const xhr = new XMLHttpRequest(); + xhr.open('GET', src, true); + xhr.onload = function(e) { + if (xhr.response === expected_content) { + numXHRLoaded++ + } + resolve(onLoad()) } - onLoad() - } - xhr.onerror = function(e) { - console.log(e) - numXHRBlocked++ - onLoad() - } - xhr.send(); + xhr.onerror = function(e) { + console.log(e) + numXHRBlocked++ + resolve(onLoad()) + } + xhr.send(); + }); } // Adds an image to the DOM with the specified src function addImage(src) { - const img = document.createElement('img') - img.src = src - img.className = 'adImage' - img.addEventListener('load', onLoad) - img.addEventListener('error', () => { - onLoad() - }) + return new Promise(resolve => { + const img = document.createElement('img') + img.src = src + img.className = 'adImage' - img.addEventListener('load', () => { - if (img.complete && - (img.naturalHeight !== 1 || img.naturalWidth !== 1)) { - numImgLoaded++ - } else { - numImgLoadedBlank++ - } - onLoad() - }) - img.addEventListener('error', () => { - numImgBlocked++ - onLoad() - }) - document.body.appendChild(img) + img.addEventListener('load', () => { + if (img.complete && + (img.naturalHeight !== 1 || img.naturalWidth !== 1)) { + numImgLoaded++ + } else { + numImgLoadedBlank++ + } + resolve(onLoad()) + }) + img.addEventListener('error', () => { + numImgBlocked++ + resolve(onLoad()) + }) + document.body.appendChild(img) + }); } // Sets the expectation for the number of images and XHR loaded and blocked @@ -102,7 +104,7 @@ // For debugging: // console.log('sending: ' + JSON.stringify({result, expectedImgLoaded, expectedImgLoadedBlank, expectedImgBlocked, numImgLoadedBlank, numImgBlocked, // numXHRLoaded, expectedXHRLoaded, numXHRLoadedBlank, numXHRBlocked, expectedXHRLoadedBlank, expectedXHRBlocked})) - window.domAutomationController.send(result) + return result; } diff --git a/test/data/cosmetic_filtering.html b/test/data/cosmetic_filtering.html index ea7aa32bbb4a..89c24d038054 100644 --- a/test/data/cosmetic_filtering.html +++ b/test/data/cosmetic_filtering.html @@ -18,15 +18,30 @@ } } +let didWait = false; + function checkSelector(selector, property, expected) { - setTimeout(() => { + const checkSelectorInner = () => { let elements = [].slice.call(document.querySelectorAll(selector)); let result = elements.every(e => { let style = window.getComputedStyle(e); return style[property] === expected; }) - window.domAutomationController.send(result) - }, 3000) + return result; + }; + + // The first selector check must occur after the page has had time to load, + // but subsequent checks can be done instantly. + if (didWait) { + return checkSelectorInner(); + } else { + return new Promise(resolve => { + setTimeout(() => { + didWait = true; + resolve(checkSelectorInner()) + }, 3000) + }) + } } From d708578f6cc1b3b8fdb4ffcafffaccfc2145cd5a Mon Sep 17 00:00:00 2001 From: Anton Lazarev Date: Tue, 3 Nov 2020 16:15:50 -0500 Subject: [PATCH 4/4] fix PerfPredictorTabHelperTest as well --- .../perf_predictor_tab_helper_browsertest.cc | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/components/brave_perf_predictor/browser/perf_predictor_tab_helper_browsertest.cc b/components/brave_perf_predictor/browser/perf_predictor_tab_helper_browsertest.cc index 5a7b3691f6bb..975c75be50b6 100644 --- a/components/brave_perf_predictor/browser/perf_predictor_tab_helper_browsertest.cc +++ b/components/brave_perf_predictor/browser/perf_predictor_tab_helper_browsertest.cc @@ -79,10 +79,10 @@ IN_PROC_BROWSER_TEST_F(PerfPredictorTabHelperTest, NoBlockNoSavings) { content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - EXPECT_EQ(true, EvalJsWithManualReply(contents, - "addImage('logo.png');" - "setExpectations(0, 0, 0, 0, 1, 0);" - "xhr('analytics.js')")); + EXPECT_EQ(true, EvalJs(contents, + "addImage('logo.png');" + "setExpectations(0, 0, 0, 0, 1, 0);" + "xhr('analytics.js')")); // Prediction triggered when web contents are closed contents->Close(); EXPECT_EQ(getProfileBandwidthSaved(browser()), 0ULL); @@ -98,10 +98,10 @@ IN_PROC_BROWSER_TEST_F(PerfPredictorTabHelperTest, ScriptBlockHasSavings) { content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - EXPECT_EQ(true, EvalJsWithManualReply(contents, - "addImage('logo.png');" - "setExpectations(0, 0, 0, 0, 1, 0);" - "xhr('analytics.js')")); + EXPECT_EQ(true, EvalJs(contents, + "addImage('logo.png');" + "setExpectations(0, 0, 0, 0, 0, 1);" + "xhr('analytics.js')")); EXPECT_EQ(getProfileAdsBlocked(browser()), 1ULL); // Prediction triggered when web contents are closed @@ -119,19 +119,19 @@ IN_PROC_BROWSER_TEST_F(PerfPredictorTabHelperTest, NewNavigationStoresSavings) { content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - EXPECT_EQ(true, EvalJsWithManualReply(contents, - "addImage('logo.png');" - "setExpectations(0, 0, 0, 0, 1, 0);" - "xhr('analytics.js')")); + EXPECT_EQ(true, EvalJs(contents, + "addImage('logo.png');" + "setExpectations(0, 0, 0, 0, 0, 1);" + "xhr('analytics.js')")); EXPECT_EQ(getProfileAdsBlocked(browser()), 1ULL); // Prediction triggered when web contents are closed GURL second_url = embedded_test_server()->GetURL("example.com", "/blocking.html"); ui_test_utils::NavigateToURL(browser(), second_url); - EXPECT_EQ(true, EvalJsWithManualReply(contents, - "addImage('logo.png');" - "setExpectations(0, 0, 0, 0, 1, 0);" - "xhr('analytics.js')")); + EXPECT_EQ(true, EvalJs(contents, + "addImage('logo.png');" + "setExpectations(0, 0, 0, 0, 0, 1);" + "xhr('analytics.js')")); auto previous_nav_savings = getProfileBandwidthSaved(browser()); EXPECT_NE(previous_nav_savings, 0ULL);