From ad53d05df5dfc8048867cc7dbd645bf0399f60b8 Mon Sep 17 00:00:00 2001 From: Anton Lazarev Date: Mon, 11 Apr 2022 16:02:07 -0700 Subject: [PATCH] Fix cosmetic_filters_js_handler.cc after revert --- .../ad_block_service_browsertest.cc | 15 +++++-------- .../renderer/cosmetic_filters_js_handler.cc | 21 +++++++++++++------ .../renderer/cosmetic_filters_js_handler.h | 8 ------- test/data/cosmetic_filtering.html | 7 ++++--- 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/browser/brave_shields/ad_block_service_browsertest.cc b/browser/brave_shields/ad_block_service_browsertest.cc index b05e47e59cbd..04c799e55904 100644 --- a/browser/brave_shields/ad_block_service_browsertest.cc +++ b/browser/brave_shields/ad_block_service_browsertest.cc @@ -1904,12 +1904,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringHide1pContent) { } // Test cosmetic filtering on elements added dynamically -<<<<<<< HEAD IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringDynamic) { - ASSERT_TRUE(InstallDefaultAdBlockExtension()); -======= -IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, DISABLED_CosmeticFilteringDynamic) { ->>>>>>> parent of bc8e4c25e6 (Merge pull request #12038 from brave/cosmetic-filtering-fixes) UpdateAdBlockInstanceWithRules("##.blockme"); GURL tab_url = @@ -1963,7 +1958,8 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringDynamicCustom) { browser()->tab_strip_model()->GetActiveWebContents(); auto result_first = EvalJs(contents, - R"(async function waitCSSSelector() { + R"(addElementsDynamically(); + async function waitCSSSelector() { if (await checkSelector('.blockme', 'display', 'none')) { window.domAutomationController.send(true); } else { @@ -2158,11 +2154,12 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringIframeScriptlet) { ASSERT_EQ(true, EvalJs(contents, "show_ad")); } -<<<<<<< HEAD // Test cosmetic filtering on an element that already has an `!important` // marker on its `display` style. +// Temporarily disabled by https://github.com/brave/brave-core/pull/12950 due +// to performance impact of newer injection method. IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, - CosmeticFilteringOverridesImportant) { + DISABLED_CosmeticFilteringOverridesImportant) { ASSERT_TRUE(InstallDefaultAdBlockExtension()); UpdateAdBlockInstanceWithRules("###inline-block-important"); @@ -2187,8 +2184,6 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, EXPECT_EQ(base::Value(true), result_first.value); } -======= ->>>>>>> parent of bc8e4c25e6 (Merge pull request #12038 from brave/cosmetic-filtering-fixes) class DefaultCookieListFlagEnabledTest : public AdBlockServiceTest { public: DefaultCookieListFlagEnabledTest() { diff --git a/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.cc b/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.cc index 3858a5178283..d8a9c473957d 100644 --- a/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.cc +++ b/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.cc @@ -479,6 +479,8 @@ void CosmeticFiltersJSHandler::OnHiddenClassIdSelectors(base::Value result) { return; } + blink::WebLocalFrame* web_frame = render_frame_->GetWebFrame(); + DCHECK(result.is_dict()); base::Value* hide_selectors = result.FindListKey("hide_selectors"); @@ -489,12 +491,20 @@ void CosmeticFiltersJSHandler::OnHiddenClassIdSelectors(base::Value result) { DCHECK(force_hide_selectors); if (force_hide_selectors->GetList().size() != 0) { - std::string stylesheet = ""; - for (auto& selector : force_hide_selectors->GetList()) { - DCHECK(selector.is_string()); - stylesheet += selector.GetString() + "{display:none !important}"; + std::string json_selectors; + if (!base::JSONWriter::Write(force_hide_selectors->GetList(), + &json_selectors) || + json_selectors.empty()) { + json_selectors = "[]"; } - InjectStylesheet(stylesheet, 0); + // Building a script for stylesheet modifications + std::string new_selectors_script = base::StringPrintf( + kForceHideSelectorsInjectScript, json_selectors.c_str()); + web_frame->ExecuteScriptInIsolatedWorld( + isolated_world_id_, + blink::WebScriptSource( + blink::WebString::FromUTF8(new_selectors_script)), + blink::BackForwardCacheAware::kAllow); } // If its a vetted engine AND we're not in aggressive @@ -502,7 +512,6 @@ void CosmeticFiltersJSHandler::OnHiddenClassIdSelectors(base::Value result) { if (!enabled_1st_party_cf_ && IsVettedSearchEngine(url_)) return; - blink::WebLocalFrame* web_frame = render_frame_->GetWebFrame(); std::string json_selectors; if (!base::JSONWriter::Write(*hide_selectors, &json_selectors) || json_selectors.empty()) { diff --git a/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.h b/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.h index b4ff6b532b73..8c0ac7c12f12 100644 --- a/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.h +++ b/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.h @@ -68,15 +68,7 @@ class CosmeticFiltersJSHandler { void OnHiddenClassIdSelectors(base::Value result); bool OnIsFirstParty(const std::string& url_string); -<<<<<<< HEAD - void InjectStylesheet(const std::string& stylesheet, int id); - void UninjectStylesheet(int id); - bool generichide_ = false; - std::map> inserted_stylesheet_ids; - -======= ->>>>>>> parent of bc8e4c25e6 (Merge pull request #12038 from brave/cosmetic-filtering-fixes) raw_ptr render_frame_ = nullptr; mojo::Remote cosmetic_filters_resources_; diff --git a/test/data/cosmetic_filtering.html b/test/data/cosmetic_filtering.html index 78218b5e0abf..9c4a10199306 100644 --- a/test/data/cosmetic_filtering.html +++ b/test/data/cosmetic_filtering.html @@ -51,10 +51,10 @@
-
+
-
-
+
+
@@ -77,5 +77,6 @@ A text of 30 chars and 5 words is needed here to consider element significant.
+