From 611881dba06c0a8cef2fdca805c736873e462fc8 Mon Sep 17 00:00:00 2001 From: Anton Lazarev Date: Thu, 20 Feb 2020 16:08:33 -0500 Subject: [PATCH 1/3] support merging regional and custom hostname cosmetic adblock resources --- browser/extensions/api/brave_shields_api.cc | 21 +++ .../ad_block_regional_service_manager.cc | 25 ++++ .../ad_block_regional_service_manager.h | 5 + .../browser/ad_block_service_helper.cc | 56 ++++++++ .../browser/ad_block_service_helper.h | 3 + .../browser/cosmetic_merge_unittest.cc | 132 ++++++++++++++++++ test/BUILD.gn | 1 + 7 files changed, 243 insertions(+) create mode 100644 components/brave_shields/browser/cosmetic_merge_unittest.cc diff --git a/browser/extensions/api/brave_shields_api.cc b/browser/extensions/api/brave_shields_api.cc index 4d2b6cfcf78a..fd76dd679f8d 100644 --- a/browser/extensions/api/brave_shields_api.cc +++ b/browser/extensions/api/brave_shields_api.cc @@ -15,7 +15,11 @@ #include "brave/browser/webcompat_reporter/webcompat_reporter_dialog.h" #include "brave/common/extensions/api/brave_shields.h" #include "brave/common/extensions/extension_constants.h" +#include "brave/components/brave_shields/browser/ad_block_base_service.h" +#include "brave/components/brave_shields/browser/ad_block_custom_filters_service.h" +#include "brave/components/brave_shields/browser/ad_block_regional_service_manager.h" #include "brave/components/brave_shields/browser/ad_block_service.h" +#include "brave/components/brave_shields/browser/ad_block_service_helper.h" #include "brave/components/brave_shields/browser/brave_shields_p3a.h" #include "brave/components/brave_shields/browser/brave_shields_util.h" #include "brave/components/brave_shields/browser/brave_shields_web_contents_observer.h" @@ -59,6 +63,23 @@ BraveShieldsHostnameCosmeticResourcesFunction::Run() { return RespondNow(Error( "Hostname-specific cosmetic resources could not be returned")); } + + base::Optional regional_resources = g_brave_browser_process-> + ad_block_regional_service_manager()-> + HostnameCosmeticResources(params->hostname); + + if (regional_resources && regional_resources->is_dict()) { + ::brave_shields::MergeResourcesInto(&*resources, &*regional_resources); + } + + base::Optional custom_resources = g_brave_browser_process-> + ad_block_custom_filters_service()-> + HostnameCosmeticResources(params->hostname); + + if (custom_resources && custom_resources->is_dict()) { + ::brave_shields::MergeResourcesInto(&*resources, &*custom_resources); + } + auto result_list = std::make_unique(); result_list->GetList().push_back(std::move(*resources)); 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 689df46b54b3..31d1be6051c7 100644 --- a/components/brave_shields/browser/ad_block_regional_service_manager.cc +++ b/components/brave_shields/browser/ad_block_regional_service_manager.cc @@ -191,6 +191,31 @@ void AdBlockRegionalServiceManager::EnableFilterList(const std::string& uuid, base::Unretained(this), uuid, enabled)); } +base::Optional +AdBlockRegionalServiceManager::HostnameCosmeticResources( + const std::string& hostname) { + auto it = this->regional_services_.begin(); + if (it == this->regional_services_.end()) { + return base::Optional(); + } + base::Optional first_value = + it->second->HostnameCosmeticResources(hostname); + + for ( ; it != this->regional_services_.end(); it++) { + base::Optional next_value = + it->second->HostnameCosmeticResources(hostname); + if (first_value) { + if (next_value) { + MergeResourcesInto(&*first_value, &*next_value); + } + } else { + first_value = std::move(next_value); + } + } + + return first_value; +} + // static bool AdBlockRegionalServiceManager::IsSupportedLocale( const std::string& locale) { 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 c60f2e22a17f..0fd22650bf5c 100644 --- a/components/brave_shields/browser/ad_block_regional_service_manager.h +++ b/components/brave_shields/browser/ad_block_regional_service_manager.h @@ -12,7 +12,9 @@ #include "base/macros.h" #include "base/memory/scoped_refptr.h" +#include "base/optional.h" #include "base/synchronization/lock.h" +#include "base/values.h" #include "brave/components/brave_component_updater/browser/brave_component.h" #include "content/public/common/resource_type.h" #include "url/gurl.h" @@ -52,6 +54,9 @@ class AdBlockRegionalServiceManager { void AddResources(const std::string& resources); void EnableFilterList(const std::string& uuid, bool enabled); + base::Optional HostnameCosmeticResources( + const std::string& hostname); + private: friend class ::AdBlockServiceTest; bool Init(); diff --git a/components/brave_shields/browser/ad_block_service_helper.cc b/components/brave_shields/browser/ad_block_service_helper.cc index 6a89e9f46f52..f51cece837a7 100644 --- a/components/brave_shields/browser/ad_block_service_helper.cc +++ b/components/brave_shields/browser/ad_block_service_helper.cc @@ -6,8 +6,10 @@ #include "brave/components/brave_shields/browser/ad_block_service_helper.h" #include +#include #include "base/strings/string_util.h" +#include "base/values.h" using adblock::FilterList; @@ -44,4 +46,58 @@ std::vector::const_iterator FindAdBlockFilterListByLocale( }); } +// Merges the contents of the second HostnameCosmeticResources Value into the +// first one provided. +void MergeResourcesInto(base::Value* into, base::Value* from) { + base::Value* resources_hide_selectors = into->FindKey("hide_selectors"); + base::Value* from_resources_hide_selectors = + from->FindKey("hide_selectors"); + if (resources_hide_selectors && from_resources_hide_selectors) { + for (auto i = from_resources_hide_selectors->GetList().begin(); + i < from_resources_hide_selectors->GetList().end(); + i++) { + resources_hide_selectors->Append(std::move(*i)); + } + } + + base::Value* resources_style_selectors = into->FindKey("style_selectors"); + base::Value* from_resources_style_selectors = + from->FindKey("style_selectors"); + if (resources_style_selectors && from_resources_style_selectors) { + for (auto i : from_resources_style_selectors->DictItems()) { + base::Value* resources_entry = + resources_style_selectors->FindKey(i.first); + if (resources_entry) { + for (auto j = i.second.GetList().begin(); + j < i.second.GetList().end(); + j++) { + resources_entry->Append(std::move(*j)); + } + } else { + resources_style_selectors->SetPath(i.first, std::move(i.second)); + } + } + } + + base::Value* resources_exceptions = into->FindKey("exceptions"); + base::Value* from_resources_exceptions = from->FindKey("exceptions"); + if (resources_exceptions && from_resources_exceptions) { + for (auto i = from_resources_exceptions->GetList().begin(); + i < from_resources_exceptions->GetList().end(); + i++) { + resources_exceptions->Append(std::move(*i)); + } + } + + base::Value* resources_injected_script = into->FindKey("injected_script"); + base::Value* from_resources_injected_script = + from->FindKey("injected_script"); + if (resources_injected_script && from_resources_injected_script) { + *resources_injected_script = base::Value( + resources_injected_script->GetString() + + '\n' + + from_resources_injected_script->GetString()); + } +} + } // namespace brave_shields diff --git a/components/brave_shields/browser/ad_block_service_helper.h b/components/brave_shields/browser/ad_block_service_helper.h index 906bdcdbbb86..e7471d547528 100644 --- a/components/brave_shields/browser/ad_block_service_helper.h +++ b/components/brave_shields/browser/ad_block_service_helper.h @@ -9,6 +9,7 @@ #include #include +#include "base/values.h" #include "brave/vendor/adblock_rust_ffi/src/wrapper.hpp" namespace brave_shields { @@ -20,6 +21,8 @@ std::vector::const_iterator FindAdBlockFilterListByLocale( const std::vector& region_lists, const std::string& locale); +void MergeResourcesInto(base::Value* into, base::Value* from); + } // namespace brave_shields #endif // BRAVE_COMPONENTS_BRAVE_SHIELDS_BROWSER_AD_BLOCK_SERVICE_HELPER_H_ diff --git a/components/brave_shields/browser/cosmetic_merge_unittest.cc b/components/brave_shields/browser/cosmetic_merge_unittest.cc new file mode 100644 index 000000000000..82dcec44b294 --- /dev/null +++ b/components/brave_shields/browser/cosmetic_merge_unittest.cc @@ -0,0 +1,132 @@ +/* Copyright (c) 2020 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "base/json/json_reader.h" +#include "brave/components/brave_shields/browser/ad_block_service_helper.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace brave_shields { + +using ::testing::_; + +class CosmeticResourceMergeTest : public testing::Test { + public: + CosmeticResourceMergeTest() {} + ~CosmeticResourceMergeTest() override {} + + void CompareMergeFromStrings( + const std::string& a, + const std::string& b, + const std::string& expected) { + base::Optional a_val = base::JSONReader::Read(a); + ASSERT_TRUE(a_val); + + base::Optional b_val = base::JSONReader::Read(b); + ASSERT_TRUE(b_val); + + const base::Optional expected_val = + base::JSONReader::Read(expected); + ASSERT_TRUE(expected_val); + + MergeResourcesInto(&*a_val, &*b_val); + + ASSERT_EQ(*a_val, *expected_val); + } + + protected: + void SetUp() override {} + + void TearDown() override {} +}; + +const char EMPTY_RESOURCES[] = "{" + "\"hide_selectors\": [], " + "\"style_selectors\": {}, " + "\"exceptions\": [], " + "\"injected_script\": \"\"" +"}"; + +const char NONEMPTY_RESOURCES[] = "{" + "\"hide_selectors\": [\"a\", \"b\"], " + "\"style_selectors\": {\"c\": \"color: #fff\", \"d\": \"color: #000\"}, " + "\"exceptions\": [\"e\", \"f\"], " + "\"injected_script\": \"console.log('g')\"" +"}"; + +TEST_F(CosmeticResourceMergeTest, MergeTwoEmptyResources) { + const std::string a = EMPTY_RESOURCES; + const std::string b = EMPTY_RESOURCES; + + // Same as EMPTY_RESOURCES, but with an additional newline in the + // injected_script + const std::string expected = "{" + "\"hide_selectors\": [], " + "\"style_selectors\": {}, " + "\"exceptions\": [], " + "\"injected_script\": \"\n\"" + "}"; + + CompareMergeFromStrings(a, b, expected); +} + +TEST_F(CosmeticResourceMergeTest, MergeEmptyIntoNonEmpty) { + const std::string a = NONEMPTY_RESOURCES; + const std::string b = EMPTY_RESOURCES; + + // Same as a, but with an additional newline at the end of the + // injected_script + const std::string expected = "{" + "\"hide_selectors\": [\"a\", \"b\"], " + "\"style_selectors\": {\"c\": \"color: #fff\", \"d\": \"color: #000\"}, " + "\"exceptions\": [\"e\", \"f\"], " + "\"injected_script\": \"console.log('g')\n\"" + "}"; + + CompareMergeFromStrings(a, b, expected); +} + +TEST_F(CosmeticResourceMergeTest, MergeNonEmptyIntoEmpty) { + const std::string a = EMPTY_RESOURCES; + const std::string b = NONEMPTY_RESOURCES; + + // Same as b, but with an additional newline at the beginning of the + // injected_script + const std::string expected = "{" + "\"hide_selectors\": [\"a\", \"b\"]," + "\"style_selectors\": {\"c\": \"color: #fff\", \"d\": \"color: #000\"}, " + "\"exceptions\": [\"e\", \"f\"], " + "\"injected_script\": \"\nconsole.log('g')\"" + "}"; + + CompareMergeFromStrings(a, b, expected); +} + +TEST_F(CosmeticResourceMergeTest, MergeNonEmptyIntoNonEmpty) { + const std::string a = NONEMPTY_RESOURCES; + const std::string b = "{" + "\"hide_selectors\": [\"h\", \"i\"], " + "\"style_selectors\": {\"j\": \"color: #eee\", \"k\": \"color: #111\"}, " + "\"exceptions\": [\"l\", \"m\"], " + "\"injected_script\": \"console.log('n')\"" + "}"; + + const std::string expected = "{" + "\"hide_selectors\": [\"a\", \"b\", \"h\", \"i\"], " + "\"style_selectors\": {" + "\"c\": \"color: #fff\", " + "\"d\": \"color: #000\", " + "\"j\": \"color: #eee\", " + "\"k\": \"color: #111\"" + "}, " + "\"exceptions\": [\"e\", \"f\", \"l\", \"m\"], " + "\"injected_script\": \"console.log('g')\nconsole.log('n')\"" + "}"; + + CompareMergeFromStrings(a, b, expected); +} + + +} // namespace brave_shields diff --git a/test/BUILD.gn b/test/BUILD.gn index 75bf06fdc0a8..eb20e38eeb11 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -95,6 +95,7 @@ test("brave_unit_tests") { "//brave/components/assist_ranker/ranker_model_loader_impl_unittest.cc", "//brave/components/brave_shields/browser/ad_block_regional_service_unittest.cc", "//brave/components/brave_shields/browser/adblock_stub_response_unittest.cc", + "//brave/components/brave_shields/browser/cosmetic_merge_unittest.cc", "//brave/components/brave_shields/browser/https_everywhere_recently_used_cache_unittest.cpp", "//brave/components/ntp_sponsored_images/browser/view_counter_model_unittest.cc", "//brave/components/ntp_sponsored_images/browser/view_counter_service_unittest.cc", From b4de1ce4beab975324c4f5331d734fc9f51cba16 Mon Sep 17 00:00:00 2001 From: Anton Lazarev Date: Thu, 20 Feb 2020 17:46:29 -0500 Subject: [PATCH 2/3] support merging regional and custom cosmetic selectors --- browser/extensions/api/brave_shields_api.cc | 42 ++++++++++++++++++- common/extensions/api/brave_shields.json | 3 +- .../background/api/cosmeticFilterAPI.ts | 3 +- .../browser/ad_block_base_service.cc | 9 ++-- .../browser/ad_block_base_service.h | 2 +- .../ad_block_regional_service_manager.cc | 31 ++++++++++++++ .../ad_block_regional_service_manager.h | 5 +++ components/definitions/chromel.d.ts | 2 +- 8 files changed, 86 insertions(+), 11 deletions(-) diff --git a/browser/extensions/api/brave_shields_api.cc b/browser/extensions/api/brave_shields_api.cc index fd76dd679f8d..20b96f1da6d8 100644 --- a/browser/extensions/api/brave_shields_api.cc +++ b/browser/extensions/api/brave_shields_api.cc @@ -93,11 +93,49 @@ BraveShieldsHiddenClassIdSelectorsFunction::Run() { brave_shields::HiddenClassIdSelectors::Params::Create(*args_)); EXTENSION_FUNCTION_VALIDATE(params.get()); - std::string stylesheet = g_brave_browser_process-> + base::Optional selectors = g_brave_browser_process-> ad_block_service()->HiddenClassIdSelectors(params->classes, params->ids, params->exceptions); - return RespondNow(OneArgument(std::make_unique(stylesheet))); + + base::Optional regional_selectors = g_brave_browser_process-> + ad_block_regional_service_manager()-> + HiddenClassIdSelectors(params->classes, + params->ids, + params->exceptions); + + if (selectors && selectors->is_list()) { + if (regional_selectors && regional_selectors->is_list()) { + for (auto i = regional_selectors->GetList().begin(); + i < regional_selectors->GetList().end(); + i++) { + selectors->Append(std::move(*i)); + } + } + } else { + selectors = std::move(regional_selectors); + } + + base::Optional custom_selectors = g_brave_browser_process-> + ad_block_custom_filters_service()-> + HiddenClassIdSelectors(params->classes, + params->ids, + params->exceptions); + + if (selectors && custom_selectors->is_list()) { + if (custom_selectors && custom_selectors->is_list()) { + for (auto i = custom_selectors->GetList().begin(); + i < custom_selectors->GetList().end(); + i++) { + selectors->Append(std::move(*i)); + } + } + } else { + selectors = std::move(custom_selectors); + } + + return RespondNow(OneArgument(std::make_unique( + std::move(*selectors)))); } diff --git a/common/extensions/api/brave_shields.json b/common/extensions/api/brave_shields.json index 832f924945f1..f3e859b65ffa 100644 --- a/common/extensions/api/brave_shields.json +++ b/common/extensions/api/brave_shields.json @@ -126,7 +126,8 @@ "parameters": [ { "name": "selectorsJson", - "type": "string" + "type": "array", + "items": {"type": "string"} } ] } diff --git a/components/brave_extension/extension/brave_extension/background/api/cosmeticFilterAPI.ts b/components/brave_extension/extension/brave_extension/background/api/cosmeticFilterAPI.ts index 580ea2c6efc7..bec58c3d30fe 100644 --- a/components/brave_extension/extension/brave_extension/background/api/cosmeticFilterAPI.ts +++ b/components/brave_extension/extension/brave_extension/background/api/cosmeticFilterAPI.ts @@ -19,8 +19,7 @@ const informTabOfCosmeticRulesToConsider = (tabId: number, selectors: string[]) // Fires when content-script calls hiddenClassIdSelectors export const injectClassIdStylesheet = (tabId: number, classes: string[], ids: string[], exceptions: string[]) => { - chrome.braveShields.hiddenClassIdSelectors(classes, ids, exceptions, (jsonSelectors) => { - const selectors = JSON.parse(jsonSelectors) + chrome.braveShields.hiddenClassIdSelectors(classes, ids, exceptions, (selectors) => { informTabOfCosmeticRulesToConsider(tabId, selectors) }) } diff --git a/components/brave_shields/browser/ad_block_base_service.cc b/components/brave_shields/browser/ad_block_base_service.cc index 80faa281d22e..11c412f23cc4 100644 --- a/components/brave_shields/browser/ad_block_base_service.cc +++ b/components/brave_shields/browser/ad_block_base_service.cc @@ -202,13 +202,14 @@ base::Optional AdBlockBaseService::HostnameCosmeticResources( this->ad_block_client_->hostnameCosmeticResources(hostname)); } -std::string AdBlockBaseService::HiddenClassIdSelectors( +base::Optional AdBlockBaseService::HiddenClassIdSelectors( const std::vector& classes, const std::vector& ids, const std::vector& exceptions) { - return this->ad_block_client_->hiddenClassIdSelectors(classes, - ids, - exceptions); + return base::JSONReader::Read( + this->ad_block_client_->hiddenClassIdSelectors(classes, + ids, + exceptions)); } void AdBlockBaseService::GetDATFileData(const base::FilePath& dat_file_path) { diff --git a/components/brave_shields/browser/ad_block_base_service.h b/components/brave_shields/browser/ad_block_base_service.h index 37a8a4fb3ae7..b555c5e8d055 100644 --- a/components/brave_shields/browser/ad_block_base_service.h +++ b/components/brave_shields/browser/ad_block_base_service.h @@ -49,7 +49,7 @@ class AdBlockBaseService : public BaseBraveShieldsService { base::Optional HostnameCosmeticResources( const std::string& hostname); - std::string HiddenClassIdSelectors( + base::Optional HiddenClassIdSelectors( const std::vector& classes, const std::vector& ids, const std::vector& exceptions); 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 31d1be6051c7..0500cf2638cf 100644 --- a/components/brave_shields/browser/ad_block_regional_service_manager.cc +++ b/components/brave_shields/browser/ad_block_regional_service_manager.cc @@ -216,6 +216,37 @@ AdBlockRegionalServiceManager::HostnameCosmeticResources( return first_value; } +base::Optional +AdBlockRegionalServiceManager::HiddenClassIdSelectors( + const std::vector& classes, + const std::vector& ids, + const std::vector& exceptions) { + auto it = this->regional_services_.begin(); + if (it == this->regional_services_.end()) { + return base::Optional(); + } + base::Optional first_value = + it->second->HiddenClassIdSelectors(classes, ids, exceptions); + + for ( ; it != this->regional_services_.end(); it++) { + base::Optional next_value = + it->second->HiddenClassIdSelectors(classes, ids, exceptions); + if (first_value && first_value->is_list()) { + if (next_value && next_value->is_list()) { + for (auto i = next_value->GetList().begin(); + i < next_value->GetList().end(); + i++) { + first_value->Append(std::move(*i)); + } + } + } else { + first_value = std::move(next_value); + } + } + + return first_value; +} + // static bool AdBlockRegionalServiceManager::IsSupportedLocale( const std::string& locale) { 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 0fd22650bf5c..d4b188017cde 100644 --- a/components/brave_shields/browser/ad_block_regional_service_manager.h +++ b/components/brave_shields/browser/ad_block_regional_service_manager.h @@ -9,6 +9,7 @@ #include #include #include +#include #include "base/macros.h" #include "base/memory/scoped_refptr.h" @@ -56,6 +57,10 @@ class AdBlockRegionalServiceManager { base::Optional HostnameCosmeticResources( const std::string& hostname); + base::Optional HiddenClassIdSelectors( + const std::vector& classes, + const std::vector& ids, + const std::vector& exceptions); private: friend class ::AdBlockServiceTest; diff --git a/components/definitions/chromel.d.ts b/components/definitions/chromel.d.ts index 4185f4cf3429..88122ad6b3eb 100644 --- a/components/definitions/chromel.d.ts +++ b/components/definitions/chromel.d.ts @@ -220,7 +220,7 @@ declare namespace chrome.braveShields { injected_script: string } const hostnameCosmeticResources: (hostname: string, callback: (resources: HostnameSpecificResources) => void) => void - const hiddenClassIdSelectors: (classes: string[], ids: string[], exceptions: string[], callback: (selectorsJson: string) => void) => void + const hiddenClassIdSelectors: (classes: string[], ids: string[], exceptions: string[], callback: (selectors: string[]) => void) => void type BraveShieldsViewPreferences = { showAdvancedView: boolean From e61911581c4a5d879dada6a3f1e8ba15bc1c0b3d Mon Sep 17 00:00:00 2001 From: Anton Lazarev Date: Thu, 20 Feb 2020 23:08:29 -0500 Subject: [PATCH 3/3] force custom rules to be used regardless of 1p content --- browser/extensions/api/brave_shields_api.cc | 36 ++++++------- common/extensions/api/brave_shields.json | 12 +++-- .../background/api/cosmeticFilterAPI.ts | 16 +++++- .../ad_block_regional_service_manager.cc | 2 +- .../browser/ad_block_service_helper.cc | 20 ++++++- .../browser/ad_block_service_helper.h | 2 +- .../browser/cosmetic_merge_unittest.cc | 53 +++++++++++++++++-- components/definitions/chromel.d.ts | 3 +- 8 files changed, 111 insertions(+), 33 deletions(-) diff --git a/browser/extensions/api/brave_shields_api.cc b/browser/extensions/api/brave_shields_api.cc index 20b96f1da6d8..fbd4517d3f4d 100644 --- a/browser/extensions/api/brave_shields_api.cc +++ b/browser/extensions/api/brave_shields_api.cc @@ -69,7 +69,10 @@ BraveShieldsHostnameCosmeticResourcesFunction::Run() { HostnameCosmeticResources(params->hostname); if (regional_resources && regional_resources->is_dict()) { - ::brave_shields::MergeResourcesInto(&*resources, &*regional_resources); + ::brave_shields::MergeResourcesInto( + &*resources, + &*regional_resources, + false); } base::Optional custom_resources = g_brave_browser_process-> @@ -77,7 +80,10 @@ BraveShieldsHostnameCosmeticResourcesFunction::Run() { HostnameCosmeticResources(params->hostname); if (custom_resources && custom_resources->is_dict()) { - ::brave_shields::MergeResourcesInto(&*resources, &*custom_resources); + ::brave_shields::MergeResourcesInto( + &*resources, + &*custom_resources, + true); } auto result_list = std::make_unique(); @@ -93,7 +99,7 @@ BraveShieldsHiddenClassIdSelectorsFunction::Run() { brave_shields::HiddenClassIdSelectors::Params::Create(*args_)); EXTENSION_FUNCTION_VALIDATE(params.get()); - base::Optional selectors = g_brave_browser_process-> + base::Optional hide_selectors = g_brave_browser_process-> ad_block_service()->HiddenClassIdSelectors(params->classes, params->ids, params->exceptions); @@ -104,16 +110,16 @@ BraveShieldsHiddenClassIdSelectorsFunction::Run() { params->ids, params->exceptions); - if (selectors && selectors->is_list()) { + if (hide_selectors && hide_selectors->is_list()) { if (regional_selectors && regional_selectors->is_list()) { for (auto i = regional_selectors->GetList().begin(); i < regional_selectors->GetList().end(); i++) { - selectors->Append(std::move(*i)); + hide_selectors->Append(std::move(*i)); } } } else { - selectors = std::move(regional_selectors); + hide_selectors = std::move(regional_selectors); } base::Optional custom_selectors = g_brave_browser_process-> @@ -122,20 +128,12 @@ BraveShieldsHiddenClassIdSelectorsFunction::Run() { params->ids, params->exceptions); - if (selectors && custom_selectors->is_list()) { - if (custom_selectors && custom_selectors->is_list()) { - for (auto i = custom_selectors->GetList().begin(); - i < custom_selectors->GetList().end(); - i++) { - selectors->Append(std::move(*i)); - } - } - } else { - selectors = std::move(custom_selectors); - } + auto result_list = std::make_unique(); - return RespondNow(OneArgument(std::make_unique( - std::move(*selectors)))); + result_list->GetList().push_back(std::move(*hide_selectors)); + result_list->GetList().push_back(std::move(*custom_selectors)); + + return RespondNow(ArgumentList(std::move(result_list))); } diff --git a/common/extensions/api/brave_shields.json b/common/extensions/api/brave_shields.json index f3e859b65ffa..f28c0eb14275 100644 --- a/common/extensions/api/brave_shields.json +++ b/common/extensions/api/brave_shields.json @@ -90,10 +90,11 @@ "name": "hostnameSpecificResources", "type": "object", "properties": { - "hide_selectors": {"type": "array", "items": {"type": "string"}, "description": "Hostname-specific CSS selectors that should be hidden from the page"}, + "hide_selectors": {"type": "array", "items": {"type": "string"}, "description": "Hostname-specific CSS selectors that should be hidden from the page if they are determined to not include 1st party content"}, "style_selectors": {"type": "object", "additionalProperties": {"type": "array", "items": {"type": "string"}}, "description": "Hostname-specific CSS selectors that should be restyled, with their associated CSS style rules"}, "exceptions": {"type": "array", "items": {"type": "string"}, "description": "Hostname-specific overrides for generic cosmetic blocking selectors"}, - "injected_script": {"type": "string", "description": "A script to inject as the page is loading"} + "injected_script": {"type": "string", "description": "A script to inject as the page is loading"}, + "force_hide_selectors": {"type": "array", "items": {"type": "string"}, "description": "Hostname-specific CSS selectors that should be hidden from the page"} } } ] @@ -125,7 +126,12 @@ "name": "callback", "parameters": [ { - "name": "selectorsJson", + "name": "selectors", + "type": "array", + "items": {"type": "string"} + }, + { + "name": "forceHideSelectors", "type": "array", "items": {"type": "string"} } diff --git a/components/brave_extension/extension/brave_extension/background/api/cosmeticFilterAPI.ts b/components/brave_extension/extension/brave_extension/background/api/cosmeticFilterAPI.ts index bec58c3d30fe..ec13a2894ee4 100644 --- a/components/brave_extension/extension/brave_extension/background/api/cosmeticFilterAPI.ts +++ b/components/brave_extension/extension/brave_extension/background/api/cosmeticFilterAPI.ts @@ -19,8 +19,18 @@ const informTabOfCosmeticRulesToConsider = (tabId: number, selectors: string[]) // Fires when content-script calls hiddenClassIdSelectors export const injectClassIdStylesheet = (tabId: number, classes: string[], ids: string[], exceptions: string[]) => { - chrome.braveShields.hiddenClassIdSelectors(classes, ids, exceptions, (selectors) => { + chrome.braveShields.hiddenClassIdSelectors(classes, ids, exceptions, (selectors, forceHideSelectors) => { informTabOfCosmeticRulesToConsider(tabId, selectors) + + if (forceHideSelectors.length > 0) { + const forceHideStylesheet = forceHideSelectors.join(',') + '{display:none!important;}\n' + + chrome.tabs.insertCSS(tabId, { + code: forceHideStylesheet, + cssOrigin: 'user', + runAt: 'document_start' + }) + } }) } @@ -33,7 +43,11 @@ export const applyAdblockCosmeticFilters = (tabId: number, hostname: string) => } informTabOfCosmeticRulesToConsider(tabId, resources.hide_selectors) + let styledStylesheet = '' + if (resources.force_hide_selectors.length > 0) { + styledStylesheet += resources.force_hide_selectors.join(',') + '{display:none!important;}\n' + } for (const selector in resources.style_selectors) { styledStylesheet += selector + '{' + resources.style_selectors[selector].join(';') + ';}\n' } 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 0500cf2638cf..859abea27ca1 100644 --- a/components/brave_shields/browser/ad_block_regional_service_manager.cc +++ b/components/brave_shields/browser/ad_block_regional_service_manager.cc @@ -206,7 +206,7 @@ AdBlockRegionalServiceManager::HostnameCosmeticResources( it->second->HostnameCosmeticResources(hostname); if (first_value) { if (next_value) { - MergeResourcesInto(&*first_value, &*next_value); + MergeResourcesInto(&*first_value, &*next_value, false); } } else { first_value = std::move(next_value); diff --git a/components/brave_shields/browser/ad_block_service_helper.cc b/components/brave_shields/browser/ad_block_service_helper.cc index f51cece837a7..92f9355df32b 100644 --- a/components/brave_shields/browser/ad_block_service_helper.cc +++ b/components/brave_shields/browser/ad_block_service_helper.cc @@ -48,8 +48,24 @@ std::vector::const_iterator FindAdBlockFilterListByLocale( // Merges the contents of the second HostnameCosmeticResources Value into the // first one provided. -void MergeResourcesInto(base::Value* into, base::Value* from) { - base::Value* resources_hide_selectors = into->FindKey("hide_selectors"); +// +// If `force_hide` is true, the contents of `from`'s `hide_selectors` field +// will be moved into a possibly new field of `into` called +// `force_hide_selectors`. +void MergeResourcesInto( + base::Value* into, + base::Value* from, + bool force_hide) { + base::Value* resources_hide_selectors = nullptr; + if (force_hide) { + resources_hide_selectors = into->FindKey("force_hide_selectors"); + if (!resources_hide_selectors || !resources_hide_selectors->is_list()) { + into->SetPath("force_hide_selectors", base::ListValue()); + resources_hide_selectors = into->FindKey("force_hide_selectors"); + } + } else { + resources_hide_selectors = into->FindKey("hide_selectors"); + } base::Value* from_resources_hide_selectors = from->FindKey("hide_selectors"); if (resources_hide_selectors && from_resources_hide_selectors) { diff --git a/components/brave_shields/browser/ad_block_service_helper.h b/components/brave_shields/browser/ad_block_service_helper.h index e7471d547528..7ce4e8b20122 100644 --- a/components/brave_shields/browser/ad_block_service_helper.h +++ b/components/brave_shields/browser/ad_block_service_helper.h @@ -21,7 +21,7 @@ std::vector::const_iterator FindAdBlockFilterListByLocale( const std::vector& region_lists, const std::string& locale); -void MergeResourcesInto(base::Value* into, base::Value* from); +void MergeResourcesInto(base::Value* into, base::Value* from, bool force_hide); } // namespace brave_shields diff --git a/components/brave_shields/browser/cosmetic_merge_unittest.cc b/components/brave_shields/browser/cosmetic_merge_unittest.cc index 82dcec44b294..893c5ccaf261 100644 --- a/components/brave_shields/browser/cosmetic_merge_unittest.cc +++ b/components/brave_shields/browser/cosmetic_merge_unittest.cc @@ -20,6 +20,7 @@ class CosmeticResourceMergeTest : public testing::Test { void CompareMergeFromStrings( const std::string& a, const std::string& b, + bool force_hide, const std::string& expected) { base::Optional a_val = base::JSONReader::Read(a); ASSERT_TRUE(a_val); @@ -31,7 +32,7 @@ class CosmeticResourceMergeTest : public testing::Test { base::JSONReader::Read(expected); ASSERT_TRUE(expected_val); - MergeResourcesInto(&*a_val, &*b_val); + MergeResourcesInto(&*a_val, &*b_val, force_hide); ASSERT_EQ(*a_val, *expected_val); } @@ -69,7 +70,7 @@ TEST_F(CosmeticResourceMergeTest, MergeTwoEmptyResources) { "\"injected_script\": \"\n\"" "}"; - CompareMergeFromStrings(a, b, expected); + CompareMergeFromStrings(a, b, false, expected); } TEST_F(CosmeticResourceMergeTest, MergeEmptyIntoNonEmpty) { @@ -85,7 +86,7 @@ TEST_F(CosmeticResourceMergeTest, MergeEmptyIntoNonEmpty) { "\"injected_script\": \"console.log('g')\n\"" "}"; - CompareMergeFromStrings(a, b, expected); + CompareMergeFromStrings(a, b, false, expected); } TEST_F(CosmeticResourceMergeTest, MergeNonEmptyIntoEmpty) { @@ -101,7 +102,7 @@ TEST_F(CosmeticResourceMergeTest, MergeNonEmptyIntoEmpty) { "\"injected_script\": \"\nconsole.log('g')\"" "}"; - CompareMergeFromStrings(a, b, expected); + CompareMergeFromStrings(a, b, false, expected); } TEST_F(CosmeticResourceMergeTest, MergeNonEmptyIntoNonEmpty) { @@ -125,7 +126,49 @@ TEST_F(CosmeticResourceMergeTest, MergeNonEmptyIntoNonEmpty) { "\"injected_script\": \"console.log('g')\nconsole.log('n')\"" "}"; - CompareMergeFromStrings(a, b, expected); + CompareMergeFromStrings(a, b, false, expected); +} + +TEST_F(CosmeticResourceMergeTest, MergeEmptyForceHide) { + const std::string a = EMPTY_RESOURCES; + const std::string b = EMPTY_RESOURCES; + + // Same as EMPTY_RESOURCES, but with an additional newline in the + // injected_script and a new empty `force_hide_selectors` array + const std::string expected = "{" + "\"hide_selectors\": [], " + "\"style_selectors\": {}, " + "\"exceptions\": [], " + "\"injected_script\": \"\n\"," + "\"force_hide_selectors\": []" + "}"; + + CompareMergeFromStrings(a, b, true, expected); +} + +TEST_F(CosmeticResourceMergeTest, MergeNonEmptyForceHide) { + const std::string a = NONEMPTY_RESOURCES; + const std::string b = "{" + "\"hide_selectors\": [\"h\", \"i\"], " + "\"style_selectors\": {\"j\": \"color: #eee\", \"k\": \"color: #111\"}, " + "\"exceptions\": [\"l\", \"m\"], " + "\"injected_script\": \"console.log('n')\"" + "}"; + + const std::string expected = "{" + "\"hide_selectors\": [\"a\", \"b\"], " + "\"style_selectors\": {" + "\"c\": \"color: #fff\", " + "\"d\": \"color: #000\", " + "\"j\": \"color: #eee\", " + "\"k\": \"color: #111\"" + "}, " + "\"exceptions\": [\"e\", \"f\", \"l\", \"m\"], " + "\"injected_script\": \"console.log('g')\nconsole.log('n')\"," + "\"force_hide_selectors\": [\"h\", \"i\"]" + "}"; + + CompareMergeFromStrings(a, b, true, expected); } diff --git a/components/definitions/chromel.d.ts b/components/definitions/chromel.d.ts index 88122ad6b3eb..d1081c5e2647 100644 --- a/components/definitions/chromel.d.ts +++ b/components/definitions/chromel.d.ts @@ -218,9 +218,10 @@ declare namespace chrome.braveShields { style_selectors: any exceptions: string[] injected_script: string + force_hide_selectors: string[] } const hostnameCosmeticResources: (hostname: string, callback: (resources: HostnameSpecificResources) => void) => void - const hiddenClassIdSelectors: (classes: string[], ids: string[], exceptions: string[], callback: (selectors: string[]) => void) => void + const hiddenClassIdSelectors: (classes: string[], ids: string[], exceptions: string[], callback: (selectors: string[], forceHideSelectors: string[]) => void) => void type BraveShieldsViewPreferences = { showAdvancedView: boolean