From 11ae01f7ee8f4adaeb441e1e2964ef7b92e2bef4 Mon Sep 17 00:00:00 2001 From: Aleksey Khoroshilov Date: Tue, 1 Feb 2022 19:29:00 +0700 Subject: [PATCH 1/2] Auto disable 1PES if it was enabeld in the Domain Block flow. --- ...ral_storage_1p_domain_block_browsertest.cc | 44 +++++++--- components/brave_shields/browser/BUILD.gn | 2 + .../browser/blocked_domain_1pes_lifetime.cc | 81 +++++++++++++++++++ .../browser/blocked_domain_1pes_lifetime.h | 52 ++++++++++++ .../browser/domain_block_controller_client.cc | 8 +- .../domain_block_navigation_throttle.cc | 26 ++++-- .../browser/domain_block_tab_storage.cc | 25 ++++++ .../browser/domain_block_tab_storage.h | 15 +++- .../ephemeral_storage_service.cc | 28 ++++--- .../ephemeral_storage_service.h | 13 ++- 10 files changed, 257 insertions(+), 37 deletions(-) create mode 100644 components/brave_shields/browser/blocked_domain_1pes_lifetime.cc create mode 100644 components/brave_shields/browser/blocked_domain_1pes_lifetime.h diff --git a/browser/ephemeral_storage/ephemeral_storage_1p_domain_block_browsertest.cc b/browser/ephemeral_storage/ephemeral_storage_1p_domain_block_browsertest.cc index 36b9075b646..ea899c2f9b7 100644 --- a/browser/ephemeral_storage/ephemeral_storage_1p_domain_block_browsertest.cc +++ b/browser/ephemeral_storage/ephemeral_storage_1p_domain_block_browsertest.cc @@ -17,6 +17,7 @@ #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser.h" #include "chrome/test/base/ui_test_utils.h" +#include "components/content_settings/core/browser/host_content_settings_map.h" #include "content/public/test/browser_test.h" #include "content/public/test/test_navigation_observer.h" #include "net/base/features.h" @@ -34,7 +35,7 @@ class EphemeralStorage1pDomainBlockBrowserTest brave_shields::features::kBraveDomainBlock1PES}, {}); } - ~EphemeralStorage1pDomainBlockBrowserTest() override {} + ~EphemeralStorage1pDomainBlockBrowserTest() override = default; void SetUpOnMainThread() override { EphemeralStorageBrowserTest::SetUpOnMainThread(); @@ -116,6 +117,10 @@ class EphemeralStorage1pDomainBlockBrowserTest WebContents* first_party_tab = BlockAndNavigateToBlockedDomain(a_site_simple_url_, false, false); + EXPECT_TRUE(GetAllCookies().empty()); + EXPECT_EQ(GetCookieSetting(a_site_simple_url_), + ContentSetting::CONTENT_SETTING_SESSION_ONLY); + // After keepalive values should be cleared. ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), b_site_simple_url_)); WaitForCleanupAfterKeepAlive(); @@ -123,11 +128,15 @@ class EphemeralStorage1pDomainBlockBrowserTest ExpectValuesFromFrameAreEmpty( FROM_HERE, GetValuesFromFrame(first_party_tab->GetMainFrame())); + EXPECT_EQ(GetCookieSetting(a_site_simple_url_), + ContentSetting::CONTENT_SETTING_SESSION_ONLY); } void NavigateToBlockedDomainAndExpectNotEphemeral() { WebContents* first_party_tab = BlockAndNavigateToBlockedDomain(a_site_simple_url_, false, false); + EXPECT_EQ(GetCookieSetting(a_site_simple_url_), + ContentSetting::CONTENT_SETTING_ALLOW); // After keepalive main frame values should not be cleared. ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), b_site_simple_url_)); @@ -141,6 +150,13 @@ class EphemeralStorage1pDomainBlockBrowserTest EXPECT_EQ("a.com", first_party_values.session_storage); EXPECT_EQ("from=a.com", first_party_values.cookies); } + EXPECT_EQ(GetCookieSetting(a_site_simple_url_), + ContentSetting::CONTENT_SETTING_ALLOW); + } + + ContentSetting GetCookieSetting(const GURL& url) { + return content_settings()->GetContentSetting(url, url, + ContentSettingsType::COOKIES); } protected: @@ -151,18 +167,11 @@ class EphemeralStorage1pDomainBlockBrowserTest IN_PROC_BROWSER_TEST_F(EphemeralStorage1pDomainBlockBrowserTest, FirstPartyEphemeralIsAutoEnabledInNormalBlockingMode) { - WebContents* first_party_tab = - BlockAndNavigateToBlockedDomain(a_site_simple_url_, false, false); + NavigateToBlockedDomainAndExpectEphemeralEnabled(); - EXPECT_TRUE(GetAllCookies().empty()); - - // After keepalive values should be cleared. ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), b_site_simple_url_)); - WaitForCleanupAfterKeepAlive(); - ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), a_site_simple_url_)); - - ExpectValuesFromFrameAreEmpty( - FROM_HERE, GetValuesFromFrame(first_party_tab->GetMainFrame())); + EXPECT_EQ(GetCookieSetting(a_site_simple_url_), + ContentSetting::CONTENT_SETTING_ALLOW); } IN_PROC_BROWSER_TEST_F(EphemeralStorage1pDomainBlockBrowserTest, @@ -172,6 +181,8 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorage1pDomainBlockBrowserTest, NavigateToBlockedDomainAndExpectNotEphemeral(); EXPECT_EQ(1u, GetAllCookies().size()); + EXPECT_EQ(GetCookieSetting(a_site_simple_url_), + ContentSetting::CONTENT_SETTING_ALLOW); } IN_PROC_BROWSER_TEST_F( @@ -188,6 +199,8 @@ IN_PROC_BROWSER_TEST_F( NavigateToBlockedDomainAndExpectNotEphemeral(); EXPECT_EQ(1u, GetAllCookies().size()); + EXPECT_EQ(GetCookieSetting(a_site_simple_url_), + ContentSetting::CONTENT_SETTING_ALLOW); } IN_PROC_BROWSER_TEST_F( @@ -195,7 +208,8 @@ IN_PROC_BROWSER_TEST_F( FirstPartyEphemeralIsAutoEnabledInAggressiveBlockingMode) { WebContents* first_party_tab = BlockAndNavigateToBlockedDomain(a_site_simple_url_, true, false); - + EXPECT_EQ(GetCookieSetting(a_site_simple_url_), + ContentSetting::CONTENT_SETTING_SESSION_ONLY); EXPECT_EQ(0u, GetAllCookies().size()); // After keepalive values should be cleared. @@ -210,6 +224,12 @@ IN_PROC_BROWSER_TEST_F( ExpectValuesFromFrameAreEmpty( FROM_HERE, GetValuesFromFrame(first_party_tab->GetMainFrame())); EXPECT_EQ(0u, GetAllCookies().size()); + EXPECT_EQ(GetCookieSetting(a_site_simple_url_), + ContentSetting::CONTENT_SETTING_SESSION_ONLY); + + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), b_site_simple_url_)); + EXPECT_EQ(GetCookieSetting(a_site_simple_url_), + ContentSetting::CONTENT_SETTING_ALLOW); } IN_PROC_BROWSER_TEST_F(EphemeralStorage1pDomainBlockBrowserTest, diff --git a/components/brave_shields/browser/BUILD.gn b/components/brave_shields/browser/BUILD.gn index e853b3a141d..9b82030b27f 100644 --- a/components/brave_shields/browser/BUILD.gn +++ b/components/brave_shields/browser/BUILD.gn @@ -27,6 +27,8 @@ static_library("browser") { "adblock_stub_response.h", "base_brave_shields_service.cc", "base_brave_shields_service.h", + "blocked_domain_1pes_lifetime.cc", + "blocked_domain_1pes_lifetime.h", "brave_shields_p3a.cc", "brave_shields_p3a.h", "brave_shields_util.cc", diff --git a/components/brave_shields/browser/blocked_domain_1pes_lifetime.cc b/components/brave_shields/browser/blocked_domain_1pes_lifetime.cc new file mode 100644 index 00000000000..66cdbdbddb5 --- /dev/null +++ b/components/brave_shields/browser/blocked_domain_1pes_lifetime.cc @@ -0,0 +1,81 @@ +/* Copyright (c) 2021 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 "brave/components/brave_shields/browser/blocked_domain_1pes_lifetime.h" + +#include "base/containers/flat_map.h" +#include "base/memory/ptr_util.h" +#include "base/no_destructor.h" +#include "brave/components/ephemeral_storage/ephemeral_storage_service.h" +#include "content/public/browser/web_contents.h" +#include "url/url_constants.h" + +namespace brave_shields { + +namespace { + +using BlockedDomain1PESLifetimeMap = + base::flat_map>; + +BlockedDomain1PESLifetimeMap& blocked_domain_1pes_lifetime_map() { + static base::NoDestructor map; + return *map; +} + +} // namespace + +// static +scoped_refptr BlockedDomain1PESLifetime::GetOrCreate( + ephemeral_storage::EphemeralStorageService* ephemeral_storage_service, + const GURL& url) { + const Key key(ephemeral_storage_service, url::Origin::Create(url).GetURL()); + auto& map = blocked_domain_1pes_lifetime_map(); + auto it = map.find(key); + DCHECK(it == map.end() || it->second); + if (it != map.end()) { + return it->second.get(); + } + auto instance = base::MakeRefCounted(key); + map.emplace(key, instance->AsWeakPtr()); + instance->Start1PESEnableRequest(); + return instance; +} + +BlockedDomain1PESLifetime::BlockedDomain1PESLifetime(const Key& key) + : key_(key) {} + +BlockedDomain1PESLifetime::~BlockedDomain1PESLifetime() { + if (is_1pes_enabled_ == true) { + key_.first->Set1PESEnabledForUrl(key_.second, false); + } + blocked_domain_1pes_lifetime_map().erase(key_); +} + +void BlockedDomain1PESLifetime::AddOnReadyCallback( + base::OnceCallback on_ready) { + if (is_1pes_enabled_.has_value()) { + std::move(on_ready).Run(); + } else { + on_ready_.push_back(std::move(on_ready)); + } +} + +void BlockedDomain1PESLifetime::Start1PESEnableRequest() { + key_.first->Enable1PESForUrlIfPossible( + key_.second, + base::BindOnce(&BlockedDomain1PESLifetime::On1PESEnableRequestComplete, + this)); +} + +void BlockedDomain1PESLifetime::On1PESEnableRequestComplete(bool is_enabled) { + is_1pes_enabled_ = is_enabled; + auto on_ready = std::move(on_ready_); + for (auto& ready_cb : on_ready) { + std::move(ready_cb).Run(); + } +} + +} // namespace brave_shields diff --git a/components/brave_shields/browser/blocked_domain_1pes_lifetime.h b/components/brave_shields/browser/blocked_domain_1pes_lifetime.h new file mode 100644 index 00000000000..bbebcedf637 --- /dev/null +++ b/components/brave_shields/browser/blocked_domain_1pes_lifetime.h @@ -0,0 +1,52 @@ +/* Copyright (c) 2021 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/. */ + +#ifndef BRAVE_COMPONENTS_BRAVE_SHIELDS_BROWSER_BLOCKED_DOMAIN_1PES_LIFETIME_H_ +#define BRAVE_COMPONENTS_BRAVE_SHIELDS_BROWSER_BLOCKED_DOMAIN_1PES_LIFETIME_H_ + +#include +#include + +#include "base/callback.h" +#include "base/memory/ref_counted.h" +#include "base/memory/weak_ptr.h" +#include "third_party/abseil-cpp/absl/types/optional.h" +#include "url/gurl.h" + +namespace ephemeral_storage { +class EphemeralStorageService; +} // namespace ephemeral_storage + +namespace brave_shields { + +class BlockedDomain1PESLifetime + : public base::RefCounted, + public base::SupportsWeakPtr { + public: + using Key = std::pair; + + static scoped_refptr GetOrCreate( + ephemeral_storage::EphemeralStorageService* ephemeral_storage_service, + const GURL& url); + + explicit BlockedDomain1PESLifetime(const Key& Key); + + void AddOnReadyCallback(base::OnceCallback on_ready); + + private: + friend class RefCounted; + virtual ~BlockedDomain1PESLifetime(); + + void Start1PESEnableRequest(); + void On1PESEnableRequestComplete(bool is_enabled); + + const Key key_; + std::vector> on_ready_; + absl::optional is_1pes_enabled_; +}; + +} // namespace brave_shields + +#endif // BRAVE_COMPONENTS_BRAVE_SHIELDS_BROWSER_BLOCKED_DOMAIN_1PES_LIFETIME_H_ diff --git a/components/brave_shields/browser/domain_block_controller_client.cc b/components/brave_shields/browser/domain_block_controller_client.cc index e4c392fe0d2..d0c57461e2a 100644 --- a/components/brave_shields/browser/domain_block_controller_client.cc +++ b/components/brave_shields/browser/domain_block_controller_client.cc @@ -7,7 +7,6 @@ #include "brave/components/brave_shields/browser/ad_block_custom_filters_service.h" #include "brave/components/brave_shields/browser/domain_block_tab_storage.h" -#include "brave/components/ephemeral_storage/ephemeral_storage_service.h" #include "components/prefs/pref_service.h" #include "components/security_interstitials/content/settings_page_helper.h" #include "components/security_interstitials/core/metrics_helper.h" @@ -66,9 +65,10 @@ void DomainBlockControllerClient::Proceed() { } if (!dont_warn_again_ && ephemeral_storage_service_) { - ephemeral_storage_service_->Enable1PESForUrlIfPossible( - request_url_, base::BindOnce(&DomainBlockControllerClient::ReloadPage, - weak_ptr_factory_.GetWeakPtr())); + tab_storage->Enable1PESForUrlIfPossible( + ephemeral_storage_service_, request_url_, + base::BindOnce(&DomainBlockControllerClient::ReloadPage, + weak_ptr_factory_.GetWeakPtr())); } else { ReloadPage(); } diff --git a/components/brave_shields/browser/domain_block_navigation_throttle.cc b/components/brave_shields/browser/domain_block_navigation_throttle.cc index 92f93ecfd20..918da3732e9 100644 --- a/components/brave_shields/browser/domain_block_navigation_throttle.cc +++ b/components/brave_shields/browser/domain_block_navigation_throttle.cc @@ -19,7 +19,6 @@ #include "brave/components/brave_shields/browser/domain_block_page.h" #include "brave/components/brave_shields/browser/domain_block_tab_storage.h" #include "brave/components/brave_shields/common/features.h" -#include "brave/components/ephemeral_storage/ephemeral_storage_service.h" #include "components/content_settings/core/browser/host_content_settings_map.h" #include "components/prefs/pref_service.h" #include "components/security_interstitials/content/security_interstitial_tab_helper.h" @@ -110,13 +109,18 @@ DomainBlockNavigationThrottle::WillStartRequest() { domain_blocking_type_ = brave_shields::GetDomainBlockingType(content_settings_, request_url); + content::WebContents* web_contents = handle->GetWebContents(); // Maybe don't block based on Brave Shields settings - if (domain_blocking_type_ == DomainBlockingType::kNone) + if (domain_blocking_type_ == DomainBlockingType::kNone) { + DomainBlockTabStorage* tab_storage = + DomainBlockTabStorage::FromWebContents(web_contents); + if (tab_storage) + tab_storage->DropBlockedDomain1PESLifetime(); return content::NavigationThrottle::PROCEED; + } // If user has just chosen to proceed on our interstitial, don't show // another one. - content::WebContents* web_contents = handle->GetWebContents(); DomainBlockTabStorage* tab_storage = DomainBlockTabStorage::GetOrCreate(web_contents); if (tab_storage->IsProceeding()) @@ -156,6 +160,10 @@ DomainBlockNavigationThrottle::WillProcessResponse() { void DomainBlockNavigationThrottle::OnShouldBlockDomain( bool should_block_domain) { if (!should_block_domain) { + DomainBlockTabStorage* tab_storage = DomainBlockTabStorage::FromWebContents( + navigation_handle()->GetWebContents()); + if (tab_storage) + tab_storage->DropBlockedDomain1PESLifetime(); // Navigation was deferred while we called the ad block service on a task // runner, but now we know that we want to allow navigation to continue. Resume(); @@ -215,10 +223,14 @@ void DomainBlockNavigationThrottle::ShowInterstitial() { void DomainBlockNavigationThrottle::Enable1PESAndResume() { DCHECK(ephemeral_storage_service_); - ephemeral_storage_service_->Enable1PESForUrlIfPossible( - navigation_handle()->GetURL(), - base::BindOnce(&DomainBlockNavigationThrottle::Resume, - weak_ptr_factory_.GetWeakPtr())); + DomainBlockTabStorage* tab_storage = DomainBlockTabStorage::FromWebContents( + navigation_handle()->GetWebContents()); + if (tab_storage) { + tab_storage->Enable1PESForUrlIfPossible( + ephemeral_storage_service_, navigation_handle()->GetURL(), + base::BindOnce(&DomainBlockNavigationThrottle::Resume, + weak_ptr_factory_.GetWeakPtr())); + } } const char* DomainBlockNavigationThrottle::GetNameForLogging() { diff --git a/components/brave_shields/browser/domain_block_tab_storage.cc b/components/brave_shields/browser/domain_block_tab_storage.cc index 8d43564eed3..20ca6897be0 100644 --- a/components/brave_shields/browser/domain_block_tab_storage.cc +++ b/components/brave_shields/browser/domain_block_tab_storage.cc @@ -5,7 +5,11 @@ #include "brave/components/brave_shields/browser/domain_block_tab_storage.h" +#include + +#include "base/containers/flat_map.h" #include "base/memory/ptr_util.h" +#include "base/no_destructor.h" #include "content/public/browser/web_contents.h" namespace brave_shields { @@ -14,6 +18,8 @@ namespace brave_shields { // Upstream does this too. const void* const kDomainBlockTabStorageKey = &kDomainBlockTabStorageKey; +DomainBlockTabStorage::DomainBlockTabStorage(content::WebContents* contents) + : content::WebContentsUserData(*contents) {} DomainBlockTabStorage::~DomainBlockTabStorage() = default; // static @@ -27,6 +33,25 @@ DomainBlockTabStorage* DomainBlockTabStorage::GetOrCreate( return storage; } +void DomainBlockTabStorage::Enable1PESForUrlIfPossible( + ephemeral_storage::EphemeralStorageService* ephemeral_storage_service, + const GURL& url, + base::OnceCallback on_ready) { + if (url.HostIsIPAddress()) { + std::move(on_ready).Run(); + return; + } + + DCHECK(ephemeral_storage_service); + blocked_domain_1pes_lifetime_ = + BlockedDomain1PESLifetime::GetOrCreate(ephemeral_storage_service, url); + blocked_domain_1pes_lifetime_->AddOnReadyCallback(std::move(on_ready)); +} + +void DomainBlockTabStorage::DropBlockedDomain1PESLifetime() { + blocked_domain_1pes_lifetime_.reset(); +} + WEB_CONTENTS_USER_DATA_KEY_IMPL(DomainBlockTabStorage); } // namespace brave_shields diff --git a/components/brave_shields/browser/domain_block_tab_storage.h b/components/brave_shields/browser/domain_block_tab_storage.h index e4028f045eb..ffdc89e6139 100644 --- a/components/brave_shields/browser/domain_block_tab_storage.h +++ b/components/brave_shields/browser/domain_block_tab_storage.h @@ -6,12 +6,18 @@ #ifndef BRAVE_COMPONENTS_BRAVE_SHIELDS_BROWSER_DOMAIN_BLOCK_TAB_STORAGE_H_ #define BRAVE_COMPONENTS_BRAVE_SHIELDS_BROWSER_DOMAIN_BLOCK_TAB_STORAGE_H_ +#include "base/memory/ref_counted.h" +#include "brave/components/brave_shields/browser/blocked_domain_1pes_lifetime.h" #include "content/public/browser/web_contents_user_data.h" namespace content { class WebContents; } // namespace content +namespace ephemeral_storage { +class EphemeralStorageService; +} // namespace ephemeral_storage + namespace brave_shields { // A short-lived, per tab storage for mixed form interstitials, that stores a @@ -32,13 +38,20 @@ class DomainBlockTabStorage void SetIsProceeding(bool is_proceeding) { is_proceeding_ = is_proceeding; } bool IsProceeding() const { return is_proceeding_; } + void Enable1PESForUrlIfPossible( + ephemeral_storage::EphemeralStorageService* ephemeral_storage_service, + const GURL& url, + base::OnceCallback on_ready); + void DropBlockedDomain1PESLifetime(); + private: - explicit DomainBlockTabStorage(content::WebContents* contents) {} + explicit DomainBlockTabStorage(content::WebContents* contents); friend class content::WebContentsUserData; WEB_CONTENTS_USER_DATA_KEY_DECL(); // Flag stores whether we are in the middle of a proceed action. bool is_proceeding_ = false; + scoped_refptr blocked_domain_1pes_lifetime_; }; } // namespace brave_shields diff --git a/components/ephemeral_storage/ephemeral_storage_service.cc b/components/ephemeral_storage/ephemeral_storage_service.cc index fe6dcee9177..ef048a85788 100644 --- a/components/ephemeral_storage/ephemeral_storage_service.cc +++ b/components/ephemeral_storage/ephemeral_storage_service.cc @@ -7,11 +7,9 @@ #include -#include "base/strings/strcat.h" #include "base/threading/sequenced_task_runner_handle.h" #include "brave/components/ephemeral_storage/url_storage_checker.h" #include "components/content_settings/core/browser/host_content_settings_map.h" -#include "components/content_settings/core/common/content_settings.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/site_instance.h" #include "net/base/features.h" @@ -28,7 +26,15 @@ EphemeralStorageService::EphemeralStorageService( DCHECK(host_content_settings_map_); } -EphemeralStorageService::~EphemeralStorageService() {} +EphemeralStorageService::~EphemeralStorageService() = default; + +void EphemeralStorageService::Shutdown() { + for (const auto& pattern : patterns_to_cleanup_) { + host_content_settings_map_->SetContentSettingCustomScope( + pattern, ContentSettingsPattern::Wildcard(), + ContentSettingsType::COOKIES, CONTENT_SETTING_DEFAULT); + } +} void EphemeralStorageService::CanEnable1PESForUrl( const GURL& url, @@ -49,10 +55,14 @@ void EphemeralStorageService::CanEnable1PESForUrl( void EphemeralStorageService::Set1PESEnabledForUrl(const GURL& url, bool enable) { + auto pattern = ContentSettingsPattern::FromURLNoWildcard(url); + if (enable) { + patterns_to_cleanup_.insert(pattern); + } else { + patterns_to_cleanup_.erase(pattern); + } host_content_settings_map_->SetContentSettingCustomScope( - ContentSettingsPattern::FromString( - base::StrCat({"[*.]", url.host_piece(), ":*"})), - ContentSettingsPattern::Wildcard(), ContentSettingsType::COOKIES, + pattern, ContentSettingsPattern::Wildcard(), ContentSettingsType::COOKIES, enable ? CONTENT_SETTING_SESSION_ONLY : CONTENT_SETTING_DEFAULT); } @@ -64,7 +74,7 @@ bool EphemeralStorageService::Is1PESEnabledForUrl(const GURL& url) const { void EphemeralStorageService::Enable1PESForUrlIfPossible( const GURL& url, - base::OnceCallback on_ready) { + base::OnceCallback on_ready) { CanEnable1PESForUrl( url, base::BindOnce(&EphemeralStorageService::OnCanEnable1PESForUrl, @@ -73,12 +83,12 @@ void EphemeralStorageService::Enable1PESForUrlIfPossible( void EphemeralStorageService::OnCanEnable1PESForUrl( const GURL& url, - base::OnceCallback on_ready, + base::OnceCallback on_ready, bool can_enable_1pes) { if (can_enable_1pes) { Set1PESEnabledForUrl(url, true); } - std::move(on_ready).Run(); + std::move(on_ready).Run(can_enable_1pes); } bool EphemeralStorageService::IsDefaultCookieSetting(const GURL& url) const { diff --git a/components/ephemeral_storage/ephemeral_storage_service.h b/components/ephemeral_storage/ephemeral_storage_service.h index 5a6fb794495..1d3a59fda7b 100644 --- a/components/ephemeral_storage/ephemeral_storage_service.h +++ b/components/ephemeral_storage/ephemeral_storage_service.h @@ -7,7 +7,9 @@ #define BRAVE_COMPONENTS_EPHEMERAL_STORAGE_EPHEMERAL_STORAGE_SERVICE_H_ #include "base/callback.h" +#include "base/containers/flat_set.h" #include "base/memory/weak_ptr.h" +#include "components/content_settings/core/common/content_settings.h" #include "components/keyed_service/core/keyed_service.h" #include "url/gurl.h" @@ -30,6 +32,8 @@ class EphemeralStorageService HostContentSettingsMap* host_content_settings_map); ~EphemeralStorageService() override; + void Shutdown() override; + // Performs storage check (cookies, localStorage) and callbacks `true` if // nothing is stored in all of these storages. void CanEnable1PESForUrl( @@ -41,16 +45,17 @@ class EphemeralStorageService bool Is1PESEnabledForUrl(const GURL& url) const; // Enables 1PES for url if nothing is stored for |url|. void Enable1PESForUrlIfPossible(const GURL& url, - base::OnceCallback on_ready); + base::OnceCallback on_ready); private: void OnCanEnable1PESForUrl(const GURL& url, - base::OnceCallback on_ready, + base::OnceCallback on_ready, bool can_enable_1pes); bool IsDefaultCookieSetting(const GURL& url) const; - content::BrowserContext* context_ = nullptr; - HostContentSettingsMap* host_content_settings_map_ = nullptr; + raw_ptr context_ = nullptr; + raw_ptr host_content_settings_map_ = nullptr; + base::flat_set patterns_to_cleanup_; base::WeakPtrFactory weak_ptr_factory_{this}; }; From d37842740d5d2c975cb6ea5817cba6a5a30410a5 Mon Sep 17 00:00:00 2001 From: Aleksey Khoroshilov Date: Mon, 14 Feb 2022 14:30:58 +0700 Subject: [PATCH 2/2] Add comments, make cleanup on shutdown clearer. --- .../brave_shields/browser/blocked_domain_1pes_lifetime.cc | 2 +- .../brave_shields/browser/blocked_domain_1pes_lifetime.h | 7 ++++++- components/ephemeral_storage/ephemeral_storage_service.cc | 6 +++--- components/ephemeral_storage/ephemeral_storage_service.h | 3 ++- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/components/brave_shields/browser/blocked_domain_1pes_lifetime.cc b/components/brave_shields/browser/blocked_domain_1pes_lifetime.cc index 66cdbdbddb5..8b98b942b38 100644 --- a/components/brave_shields/browser/blocked_domain_1pes_lifetime.cc +++ b/components/brave_shields/browser/blocked_domain_1pes_lifetime.cc @@ -1,4 +1,4 @@ -/* Copyright (c) 2021 The Brave Authors. All rights reserved. +/* Copyright (c) 2022 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/. */ diff --git a/components/brave_shields/browser/blocked_domain_1pes_lifetime.h b/components/brave_shields/browser/blocked_domain_1pes_lifetime.h index bbebcedf637..c034e21e079 100644 --- a/components/brave_shields/browser/blocked_domain_1pes_lifetime.h +++ b/components/brave_shields/browser/blocked_domain_1pes_lifetime.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2021 The Brave Authors. All rights reserved. +/* Copyright (c) 2022 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/. */ @@ -21,6 +21,11 @@ class EphemeralStorageService; namespace brave_shields { +// Manages the lifetime of auto-enabled 1PES mode (by DomainBlock feature). +// Each instance is shared by each 1PES-enabled top-level frame with the same +// BlockedDomain1PESLifetime::Key. When the last top-level frame holding a +// reference is destroyed or navigates to a non-blocked domain, 1PES will be +// disabled. class BlockedDomain1PESLifetime : public base::RefCounted, public base::SupportsWeakPtr { diff --git a/components/ephemeral_storage/ephemeral_storage_service.cc b/components/ephemeral_storage/ephemeral_storage_service.cc index ef048a85788..ccb6e9b9c11 100644 --- a/components/ephemeral_storage/ephemeral_storage_service.cc +++ b/components/ephemeral_storage/ephemeral_storage_service.cc @@ -29,7 +29,7 @@ EphemeralStorageService::EphemeralStorageService( EphemeralStorageService::~EphemeralStorageService() = default; void EphemeralStorageService::Shutdown() { - for (const auto& pattern : patterns_to_cleanup_) { + for (const auto& pattern : patterns_to_cleanup_on_shutdown_) { host_content_settings_map_->SetContentSettingCustomScope( pattern, ContentSettingsPattern::Wildcard(), ContentSettingsType::COOKIES, CONTENT_SETTING_DEFAULT); @@ -57,9 +57,9 @@ void EphemeralStorageService::Set1PESEnabledForUrl(const GURL& url, bool enable) { auto pattern = ContentSettingsPattern::FromURLNoWildcard(url); if (enable) { - patterns_to_cleanup_.insert(pattern); + patterns_to_cleanup_on_shutdown_.insert(pattern); } else { - patterns_to_cleanup_.erase(pattern); + patterns_to_cleanup_on_shutdown_.erase(pattern); } host_content_settings_map_->SetContentSettingCustomScope( pattern, ContentSettingsPattern::Wildcard(), ContentSettingsType::COOKIES, diff --git a/components/ephemeral_storage/ephemeral_storage_service.h b/components/ephemeral_storage/ephemeral_storage_service.h index 1d3a59fda7b..e4af8a7a8a0 100644 --- a/components/ephemeral_storage/ephemeral_storage_service.h +++ b/components/ephemeral_storage/ephemeral_storage_service.h @@ -55,7 +55,8 @@ class EphemeralStorageService raw_ptr context_ = nullptr; raw_ptr host_content_settings_map_ = nullptr; - base::flat_set patterns_to_cleanup_; + // These patterns are removed on service Shutdown. + base::flat_set patterns_to_cleanup_on_shutdown_; base::WeakPtrFactory weak_ptr_factory_{this}; };