diff --git a/browser/brave_content_browser_client.cc b/browser/brave_content_browser_client.cc index 293594fa212..790b360d8e5 100644 --- a/browser/brave_content_browser_client.cc +++ b/browser/brave_content_browser_client.cc @@ -212,7 +212,8 @@ using extensions::ChromeContentBrowserClientExtensionsPart; #endif #if BUILDFLAG(ENABLE_PLAYLIST) -#include "brave/components/playlist/playlist_service_helper.h" +#include "brave/browser/playlist/playlist_service_factory.h" +#include "brave/components/playlist/playlist_service.h" #endif #if BUILDFLAG(ENABLE_PLAYLIST_WEBUI) @@ -1053,8 +1054,11 @@ void BraveContentBrowserClient::OverrideWebkitPrefs(WebContents* web_contents, web_prefs->allow_non_empty_navigator_plugins = true; #if BUILDFLAG(ENABLE_PLAYLIST) - if (playlist::IsBackgroundWebContents(web_contents)) { - web_prefs->allow_cosmetic_filtering = true; + if (auto* playlist_service = + playlist::PlaylistServiceFactory::GetForBrowserContext( + web_contents->GetBrowserContext())) { + playlist_service->ConfigureWebPrefsforBackgroundWebContents(web_contents, + web_prefs); } #endif } diff --git a/browser/brave_shields/ad_block_service_browsertest.cc b/browser/brave_shields/ad_block_service_browsertest.cc index 66b01ec8bf7..4e1e61b6672 100644 --- a/browser/brave_shields/ad_block_service_browsertest.cc +++ b/browser/brave_shields/ad_block_service_browsertest.cc @@ -54,7 +54,6 @@ #if BUILDFLAG(ENABLE_PLAYLIST) #include "brave/browser/playlist/playlist_service_factory.h" #include "brave/components/playlist/features.h" -#include "brave/components/playlist/playlist_download_request_manager.h" #include "brave/components/playlist/playlist_service.h" #endif @@ -1800,20 +1799,26 @@ IN_PROC_BROWSER_TEST_F(CosmeticFilteringFlagDisabledTest, #if BUILDFLAG(ENABLE_PLAYLIST) -namespace playlist { - class CosmeticFilteringPlaylistFlagEnabledTest : public AdBlockServiceTest { public: CosmeticFilteringPlaylistFlagEnabledTest() { - feature_list_.InitAndEnableFeature(features::kPlaylist); + feature_list_.InitAndEnableFeature(playlist::features::kPlaylist); + } + + content::WebContents* GetBackgroundWebContents() { + auto* playlist_service = + playlist::PlaylistServiceFactory::GetForBrowserContext( + browser()->profile()); + + return playlist_service->GetBackgroundWebContentsForTesting(); } private: base::test::ScopedFeatureList feature_list_; }; -// Ensure cosmetic filtering occurs always when AllowCosmeticFiltering() is -// called. +// Check cosmetic filtering is applied to any loading from Playlist's +// background web contents. IN_PROC_BROWSER_TEST_F(CosmeticFilteringPlaylistFlagEnabledTest, AllowCosmeticFiltering) { ASSERT_TRUE(InstallDefaultAdBlockExtension()); @@ -1825,25 +1830,17 @@ IN_PROC_BROWSER_TEST_F(CosmeticFilteringPlaylistFlagEnabledTest, content_settings(), brave_shields::ControlType::ALLOW, url); UpdateAdBlockInstanceWithRules("b.com###ad-banner"); - auto* playlist_service_factory = PlaylistServiceFactory::GetInstance(); - auto* playlist_service = - playlist_service_factory->GetForBrowserContext(browser()->profile()); - auto* download_request_manager = - playlist_service->download_request_manager_.get(); - download_request_manager->CreateWebContents(); - auto* web_contents = download_request_manager->web_contents_.get(); + auto* web_contents = GetBackgroundWebContents(); web_contents->GetController().LoadURLWithParams( content::NavigationController::LoadURLParams(url)); - content::WaitForLoadStop(web_contents); + EXPECT_TRUE(content::WaitForLoadStop(web_contents)); // Check filter is applied properly. EXPECT_EQ(false, EvalJs(web_contents, "checkSelector('#ad-banner', 'display', 'block')")); } -} // namespace playlist - #endif // Ensure no cosmetic filtering occurs when the shields setting is disabled diff --git a/chromium_src/third_party/blink/common/web_preferences/web_preferences.cc b/chromium_src/third_party/blink/common/web_preferences/web_preferences.cc index 0588fd3e797..3757dcb6522 100644 --- a/chromium_src/third_party/blink/common/web_preferences/web_preferences.cc +++ b/chromium_src/third_party/blink/common/web_preferences/web_preferences.cc @@ -17,14 +17,10 @@ namespace blink { namespace web_pref { WebPreferences::WebPreferences(const WebPreferences& other) = default; - WebPreferences::WebPreferences(WebPreferences&& other) = default; - WebPreferences::~WebPreferences() = default; - WebPreferences& WebPreferences::operator=(const WebPreferences& other) = default; - WebPreferences& WebPreferences::operator=(WebPreferences&& other) = default; } // namespace web_pref diff --git a/chromium_src/third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc b/chromium_src/third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc index a35a83a4f7e..896691b885b 100644 --- a/chromium_src/third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc +++ b/chromium_src/third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc @@ -23,7 +23,7 @@ bool StructTraitsallow_cosmetic_filtering = data.allow_cosmetic_filtering(); + out->force_cosmetic_filtering = data.force_cosmetic_filtering(); return true; } diff --git a/chromium_src/third_party/blink/public/common/web_preferences/web_preferences.h b/chromium_src/third_party/blink/public/common/web_preferences/web_preferences.h index 916eaf3fc84..fa13d1a1ba9 100644 --- a/chromium_src/third_party/blink/public/common/web_preferences/web_preferences.h +++ b/chromium_src/third_party/blink/public/common/web_preferences/web_preferences.h @@ -25,8 +25,7 @@ struct BLINK_COMMON_EXPORT WebPreferences : public WebPreferences_ChromiumImpl { WebPreferences& operator=(const WebPreferences& other); WebPreferences& operator=(WebPreferences&& other); - // Force to allow cosmetic filtering. - bool allow_cosmetic_filtering = false; + bool force_cosmetic_filtering = false; }; } // namespace web_pref diff --git a/chromium_src/third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h b/chromium_src/third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h index 54c5a600886..6efc548d702 100644 --- a/chromium_src/third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h +++ b/chromium_src/third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h @@ -22,9 +22,9 @@ struct BLINK_COMMON_EXPORT StructTraits : public StructTraits { - static bool allow_cosmetic_filtering( + static bool force_cosmetic_filtering( const blink::web_pref::WebPreferences& r) { - return r.allow_cosmetic_filtering; + return r.force_cosmetic_filtering; } static bool Read(blink::mojom::WebPreferencesDataView r, diff --git a/chromium_src/third_party/blink/public/mojom/webpreferences/web_preferences.mojom b/chromium_src/third_party/blink/public/mojom/webpreferences/web_preferences.mojom index 5268b301124..93b133bde4f 100644 --- a/chromium_src/third_party/blink/public/mojom/webpreferences/web_preferences.mojom +++ b/chromium_src/third_party/blink/public/mojom/webpreferences/web_preferences.mojom @@ -7,5 +7,5 @@ module blink.mojom; [BraveExtend] struct WebPreferences { - bool allow_cosmetic_filtering; + bool force_cosmetic_filtering; }; diff --git a/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.cc b/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.cc index 8c8cf9270f9..7ed44a4bfe8 100644 --- a/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.cc +++ b/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.cc @@ -276,9 +276,9 @@ bool CosmeticFiltersJSHandler::ProcessURL( static_cast( content_settings::ContentSettingsAgentImpl::Get(render_frame_)); - const bool always_allow_cosmetic_filtering = - render_frame_->GetBlinkPreferences().allow_cosmetic_filtering; - if (!always_allow_cosmetic_filtering && + const bool force_cosmetic_filtering = + render_frame_->GetBlinkPreferences().force_cosmetic_filtering; + if (!force_cosmetic_filtering && !content_settings->IsCosmeticFilteringEnabled(url_)) { return false; } diff --git a/components/playlist/DEPS b/components/playlist/DEPS index a1138d5a89d..8436d7dc7f5 100644 --- a/components/playlist/DEPS +++ b/components/playlist/DEPS @@ -8,6 +8,6 @@ include_rules = [ "+content/public/common", "+services/network/public/cpp", "+services/preferences/public/cpp", - "+third_party/blink/public/common", + "+third_party/blink/public/common/web_preferences", "+third_party/re2", ] diff --git a/components/playlist/playlist_download_request_manager.cc b/components/playlist/playlist_download_request_manager.cc index a59c064364e..c9ec581c71a 100644 --- a/components/playlist/playlist_download_request_manager.cc +++ b/components/playlist/playlist_download_request_manager.cc @@ -12,13 +12,13 @@ #include "base/strings/utf_string_conversions.h" #include "base/time/time.h" #include "base/timer/timer.h" -#include "brave/components/playlist/playlist_service_helper.h" #include "content/public/browser/navigation_controller.h" #include "content/public/browser/navigation_handle.h" #include "content/public/browser/render_frame_host.h" #include "content/public/browser/web_contents.h" #include "content/public/common/isolated_world_ids.h" #include "net/base/registry_controlled_domains/registry_controlled_domain.h" +#include "third_party/blink/public/common/web_preferences/web_preferences.h" #include "third_party/re2/src/re2/re2.h" #include "ui/base/page_transition_types.h" @@ -72,7 +72,6 @@ void PlaylistDownloadRequestManager::CreateWebContents() { // |web_contents_| is created on demand. content::WebContents::CreateParams create_params(context_, nullptr); web_contents_ = content::WebContents::Create(create_params); - MarkAsBackgroundWebContents(web_contents_.get()); } Observe(web_contents_.get()); @@ -257,4 +256,18 @@ void PlaylistDownloadRequestManager::DestroyWebContents() { web_contents_.reset(); } +void PlaylistDownloadRequestManager::ConfigureWebPrefsforBackgroundWebContents( + content::WebContents* web_contents, + blink::web_pref::WebPreferences* web_prefs) { + if (web_contents_ && web_contents_.get() == web_contents) { + web_prefs->force_cosmetic_filtering = true; + } +} + +content::WebContents* +PlaylistDownloadRequestManager::GetBackgroundWebContentsForTesting() { + CreateWebContents(); + return web_contents_.get(); +} + } // namespace playlist diff --git a/components/playlist/playlist_download_request_manager.h b/components/playlist/playlist_download_request_manager.h index cf1f07fb5a7..8c2afdea2cd 100644 --- a/components/playlist/playlist_download_request_manager.h +++ b/components/playlist/playlist_download_request_manager.h @@ -25,6 +25,10 @@ class OneShotTimer; class Value; } // namespace base +namespace blink::web_pref { +struct WebPreferences; +} // namespace blink::web_pref + namespace content { class BrowserContext; } // namespace content @@ -66,10 +70,14 @@ class PlaylistDownloadRequestManager // Request::callback will be called with generated param. void GetMediaFilesFromPage(Request request); - private: - FRIEND_TEST_ALL_PREFIXES(CosmeticFilteringPlaylistFlagEnabledTest, - AllowCosmeticFiltering); + // Update |web_prefs| if we want for |web_contents|. + void ConfigureWebPrefsforBackgroundWebContents( + content::WebContents* web_contents, + blink::web_pref::WebPreferences* web_prefs); + content::WebContents* GetBackgroundWebContentsForTesting(); + + private: // Calling this will trigger loading |url| on a web contents, // and we'll inject javascript on the contents to get a list of // media files on the page. diff --git a/components/playlist/playlist_service.cc b/components/playlist/playlist_service.cc index ffcfcdebcda..fd4d6c944d5 100644 --- a/components/playlist/playlist_service.cc +++ b/components/playlist/playlist_service.cc @@ -219,6 +219,17 @@ base::FilePath PlaylistService::GetPlaylistItemDirPath( return base_dir_.AppendASCII(id); } +void PlaylistService::ConfigureWebPrefsforBackgroundWebContents( + content::WebContents* web_contents, + blink::web_pref::WebPreferences* web_prefs) { + download_request_manager_->ConfigureWebPrefsforBackgroundWebContents( + web_contents, web_prefs); +} + +content::WebContents* PlaylistService::GetBackgroundWebContentsForTesting() { + return download_request_manager_->GetBackgroundWebContentsForTesting(); +} + void PlaylistService::UpdatePlaylistItemValue(const std::string& id, base::Value value) { prefs::ScopedDictionaryPrefUpdate update(prefs_, kPlaylistItemsPref); diff --git a/components/playlist/playlist_service.h b/components/playlist/playlist_service.h index 14b9c98e054..e5b0b30f4dc 100644 --- a/components/playlist/playlist_service.h +++ b/components/playlist/playlist_service.h @@ -25,12 +25,17 @@ namespace base { class SequencedTaskRunner; } // namespace base +namespace blink::web_pref { +struct WebPreferences; +} // namespace blink::web_pref + namespace content { class BrowserContext; class WebContents; } // namespace content class PrefService; +class CosmeticFilteringPlaylistFlagEnabledTest; namespace playlist { @@ -119,15 +124,19 @@ class PlaylistService : public KeyedService, base::FilePath GetPlaylistItemDirPath(const std::string& id) const; + // Update |web_prefs| if we want for |web_contents|. + void ConfigureWebPrefsforBackgroundWebContents( + content::WebContents* web_contents, + blink::web_pref::WebPreferences* web_prefs); + private: + friend class ::CosmeticFilteringPlaylistFlagEnabledTest; FRIEND_TEST_ALL_PREFIXES(PlaylistBrowserTest, ApiFunctions); FRIEND_TEST_ALL_PREFIXES(PlaylistBrowserTest, CreatePlaylist); FRIEND_TEST_ALL_PREFIXES(PlaylistBrowserTest, CreatePlaylistItem); FRIEND_TEST_ALL_PREFIXES(PlaylistBrowserTest, MediaDownloadFailed); FRIEND_TEST_ALL_PREFIXES(PlaylistBrowserTest, ThumbnailFailed); FRIEND_TEST_ALL_PREFIXES(PlaylistBrowserTest, RemoveAndRestoreLocalData); - FRIEND_TEST_ALL_PREFIXES(CosmeticFilteringPlaylistFlagEnabledTest, - AllowCosmeticFiltering); // KeyedService overrides: void Shutdown() override; @@ -179,6 +188,8 @@ class PlaylistService : public KeyedService, void OnGetMetadata(base::Value value); + content::WebContents* GetBackgroundWebContentsForTesting(); + const base::FilePath base_dir_; base::ObserverList observers_; diff --git a/components/playlist/playlist_service_helper.cc b/components/playlist/playlist_service_helper.cc index 986a17c0412..38e42ba19f7 100644 --- a/components/playlist/playlist_service_helper.cc +++ b/components/playlist/playlist_service_helper.cc @@ -9,37 +9,9 @@ #include "brave/components/playlist/playlist_constants.h" #include "brave/components/playlist/playlist_types.h" -#include "content/public/browser/web_contents.h" -#include "content/public/browser/web_contents_user_data.h" namespace playlist { -namespace { - -class PlaylistBackgroundWebContentsTag - : public content::WebContentsUserData { - public: - PlaylistBackgroundWebContentsTag(const PlaylistBackgroundWebContentsTag&) = - delete; - PlaylistBackgroundWebContentsTag& operator=( - const PlaylistBackgroundWebContentsTag&) = delete; - - ~PlaylistBackgroundWebContentsTag() override = default; - - private: - friend class content::WebContentsUserData; - - explicit PlaylistBackgroundWebContentsTag(content::WebContents* web_contents) - : content::WebContentsUserData( - *web_contents) {} - - WEB_CONTENTS_USER_DATA_KEY_DECL(); -}; - -WEB_CONTENTS_USER_DATA_KEY_IMPL(PlaylistBackgroundWebContentsTag); - -} // namespace - base::Value::Dict GetValueFromPlaylistItemInfo(const PlaylistItemInfo& info) { base::Value::Dict playlist_value; playlist_value.Set(kPlaylistItemIDKey, info.id); @@ -53,12 +25,4 @@ base::Value::Dict GetValueFromPlaylistItemInfo(const PlaylistItemInfo& info) { return playlist_value; } -void MarkAsBackgroundWebContents(content::WebContents* web_contents) { - PlaylistBackgroundWebContentsTag::CreateForWebContents(web_contents); -} - -bool IsBackgroundWebContents(content::WebContents* web_contents) { - return !!PlaylistBackgroundWebContentsTag::FromWebContents(web_contents); -} - } // namespace playlist diff --git a/components/playlist/playlist_service_helper.h b/components/playlist/playlist_service_helper.h index 8d0039bbf30..1c68bf053e6 100644 --- a/components/playlist/playlist_service_helper.h +++ b/components/playlist/playlist_service_helper.h @@ -10,17 +10,11 @@ #include "base/values.h" -namespace content { -class WebContents; -} // namespace content - namespace playlist { struct PlaylistItemInfo; base::Value::Dict GetValueFromPlaylistItemInfo(const PlaylistItemInfo& info); -void MarkAsBackgroundWebContents(content::WebContents* web_contents); -bool IsBackgroundWebContents(content::WebContents* web_contents); } // namespace playlist