diff --git a/browser/brave_shields/ad_block_service_browsertest.cc b/browser/brave_shields/ad_block_service_browsertest.cc index bca56bc6a8bcc..634de60c9ba42 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 51663ac3bd14c..68a4d34a050dd 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 f368590e2c648..641f2fa0da084 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 587d795a57dae..f2b0c48ca933d 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 1ee6e54f1967b..ad90046be9af3 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 ff59558fd1237..86f8ec6526371 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 3c4a57a50197b..bcb632726e3e8 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 a9ad4e6de5472..d2bb45190f98f 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 b4ba4c6166169..d4ac68bc3278b 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 a8f30d4b13715..42ed5cc0f96a7 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 c3adbdde5f7e7..d00572ab7caa9 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 b82a3695fcb7e..1b900a74951a2 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 ad6f9a44d6e3f..f87f0c19e76d5 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 59a640ad7376a..6ff5633d3ff73 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 9d505dd2631b7..27d85d0414b3e 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,