From 156dcb8f7d1b9939c8cd9576de7d27fdf7f484d3 Mon Sep 17 00:00:00 2001 From: bridiver Date: Tue, 13 Apr 2021 12:27:31 -0700 Subject: [PATCH 01/15] make sure all keepalive ephemeral storage references are cleared when the tab is closed --- .../ephemeral_storage_tab_helper.cc | 28 ++++++++++++++++--- .../ephemeral_storage_tab_helper.h | 14 ++++++++-- .../public/browser/tld_ephemeral_lifetime.h | 2 ++ 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc b/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc index 278757b95844..630018b79da6 100644 --- a/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc +++ b/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc @@ -68,6 +68,10 @@ EphemeralStorageTabHelper::EphemeralStorageTabHelper(WebContents* web_contents) EphemeralStorageTabHelper::~EphemeralStorageTabHelper() {} +void EphemeralStorageTabHelper::WebContentsDestroyed() { + keep_alive_tld_ephemeral_lifetime_list_.clear(); +} + void EphemeralStorageTabHelper::ReadyToCommitNavigation( NavigationHandle* navigation_handle) { if (!navigation_handle->IsInMainFrame()) @@ -76,6 +80,7 @@ void EphemeralStorageTabHelper::ReadyToCommitNavigation( return; const GURL& new_url = navigation_handle->GetURL(); + std::string new_domain = net::URLToEphemeralStorageDomain(new_url); std::string previous_domain = net::URLToEphemeralStorageDomain(web_contents()->GetLastCommittedURL()); @@ -85,6 +90,18 @@ void EphemeralStorageTabHelper::ReadyToCommitNavigation( CreateEphemeralStorageAreasForDomainAndURL(new_domain, new_url); } +void EphemeralStorageTabHelper::ClearEphemeralLifetimeKeepalive( + const content::TLDEphemeralLifetimeKey& key) { + for (auto it = keep_alive_tld_ephemeral_lifetime_list_.begin(); it != keep_alive_tld_ephemeral_lifetime_list_.end(); + ++it) { + if ((*it)->key() == key) { + keep_alive_tld_ephemeral_lifetime_list_.erase(it); + return; + } + } + NOTREACHED(); +} + void EphemeralStorageTabHelper::CreateEphemeralStorageAreasForDomainAndURL( const std::string& new_domain, const GURL& new_url) { @@ -130,15 +147,18 @@ void EphemeralStorageTabHelper::CreateEphemeralStorageAreasForDomainAndURL( : base::nullopt); if (base::FeatureList::IsEnabled( - net::features::kBraveEphemeralStorageKeepAlive)) { + net::features::kBraveEphemeralStorageKeepAlive) && + tld_ephemeral_lifetime_) { + keep_alive_tld_ephemeral_lifetime_list_.push_back(tld_ephemeral_lifetime_); + auto key = tld_ephemeral_lifetime_->key(); // keep the ephemeral storage alive for some time to handle redirects // including meta refresh or other page driven "redirects" that end up back // at the original origin base::SequencedTaskRunnerHandle::Get()->PostDelayedTask( FROM_HERE, - base::BindOnce([](scoped_refptr - tld_ephemeral_lifetime) {}, - tld_ephemeral_lifetime_), + base::BindOnce( + &EphemeralStorageTabHelper::ClearEphemeralLifetimeKeepalive, + weak_factory_.GetWeakPtr(), key), g_storage_keep_alive_for_testing.is_min() ? kStorageKeepAliveDelay : g_storage_keep_alive_for_testing); diff --git a/browser/ephemeral_storage/ephemeral_storage_tab_helper.h b/browser/ephemeral_storage/ephemeral_storage_tab_helper.h index 26048c6e8801..5a2f256cbd94 100644 --- a/browser/ephemeral_storage/ephemeral_storage_tab_helper.h +++ b/browser/ephemeral_storage/ephemeral_storage_tab_helper.h @@ -8,7 +8,9 @@ #include #include +#include +#include "base/memory/weak_ptr.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/session_storage_namespace.h" #include "content/public/browser/web_contents_observer.h" @@ -35,11 +37,14 @@ class EphemeralStorageTabHelper static void SetKeepAliveTimeDelayForTesting(const base::TimeDelta& time); - protected: + private: + // WebContentsObserver void ReadyToCommitNavigation( content::NavigationHandle* navigation_handle) override; + void WebContentsDestroyed() override; - private: + void ClearEphemeralLifetimeKeepalive( + const content::TLDEphemeralLifetimeKey& key); void CreateEphemeralStorageAreasForDomainAndURL(const std::string& new_domain, const GURL& new_url); @@ -48,6 +53,11 @@ class EphemeralStorageTabHelper scoped_refptr session_storage_namespace_; scoped_refptr tld_ephemeral_lifetime_; + std::vector> + keep_alive_tld_ephemeral_lifetime_list_; + + base::WeakPtrFactory weak_factory_{this}; + WEB_CONTENTS_USER_DATA_KEY_DECL(); }; diff --git a/chromium_src/content/public/browser/tld_ephemeral_lifetime.h b/chromium_src/content/public/browser/tld_ephemeral_lifetime.h index 9802eb386dd0..13342725aa02 100644 --- a/chromium_src/content/public/browser/tld_ephemeral_lifetime.h +++ b/chromium_src/content/public/browser/tld_ephemeral_lifetime.h @@ -53,6 +53,8 @@ class CONTENT_EXPORT TLDEphemeralLifetime // Add a callback to a callback list to be called on destruction. void RegisterOnDestroyCallback(OnDestroyCallback callback); + const TLDEphemeralLifetimeKey& key() { return key_; } + private: friend class RefCounted; virtual ~TLDEphemeralLifetime(); From c33a50cdb9884daddaa7f9140e33d8e0f2e63acd Mon Sep 17 00:00:00 2001 From: bridiver Date: Tue, 13 Apr 2021 12:27:51 -0700 Subject: [PATCH 02/15] make the keep alive time configurable --- .../ephemeral_storage/ephemeral_storage_tab_helper.cc | 3 ++- chromium_src/chrome/browser/flag_descriptions.cc | 4 ++-- chromium_src/net/base/features.cc | 11 ++++++++--- chromium_src/net/base/features.h | 2 ++ 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc b/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc index 630018b79da6..8b8615bc928b 100644 --- a/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc +++ b/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc @@ -33,7 +33,8 @@ namespace { constexpr char kSessionStorageSuffix[] = "/ephemeral-session-storage"; constexpr char kLocalStorageSuffix[] = "/ephemeral-local-storage"; -const base::TimeDelta kStorageKeepAliveDelay = base::TimeDelta::FromSeconds(30); +const base::TimeDelta kStorageKeepAliveDelay = base::TimeDelta::FromSeconds( + net::features::kBraveEphemeralStorageKeepAliveTimeInSeconds.Get()); base::TimeDelta g_storage_keep_alive_for_testing = base::TimeDelta::Min(); diff --git a/chromium_src/chrome/browser/flag_descriptions.cc b/chromium_src/chrome/browser/flag_descriptions.cc index a45a9b0a98cb..bcd7f74a56a8 100644 --- a/chromium_src/chrome/browser/flag_descriptions.cc +++ b/chromium_src/chrome/browser/flag_descriptions.cc @@ -78,8 +78,8 @@ const char kBraveEphemeralStorageDescription[] = const char kBraveEphemeralStorageKeepAliveName[] = "Ephemeral Storage Keep Alive"; const char kBraveEphemeralStorageKeepAliveDescription[] = - "Keep ephemeral storage partitions alive for 30 seconds after last tab " - "close"; + "Keep ephemeral storage partitions alive for a specified time after all " + "tabs for that origin are closed"; const char kBravePermissionLifetimeName[] = "Permission Lifetime"; const char kBravePermissionLifetimeDescription[] = "Enables the option to choose a time period after which a permission will " diff --git a/chromium_src/net/base/features.cc b/chromium_src/net/base/features.cc index a784872dabb3..fd64d56390d7 100644 --- a/chromium_src/net/base/features.cc +++ b/chromium_src/net/base/features.cc @@ -7,10 +7,15 @@ namespace net { namespace features { + const base::Feature kBraveEphemeralStorage{"EphemeralStorage", - base::FEATURE_DISABLED_BY_DEFAULT}; + base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kBraveEphemeralStorageKeepAlive{ - "BraveEphemeralStorageKeepAlive", - base::FEATURE_DISABLED_BY_DEFAULT}; + "BraveEphemeralStorageKeepAlive", base::FEATURE_ENABLED_BY_DEFAULT}; + +const base::FeatureParam kBraveEphemeralStorageKeepAliveTimeInSeconds = { + &kBraveEphemeralStorageKeepAlive, + "BraveEphemeralStorageKeepAliveTimeInSeconds", 30}; + } // namespace features } // namespace net diff --git a/chromium_src/net/base/features.h b/chromium_src/net/base/features.h index cc08858495f6..25a4404671ac 100644 --- a/chromium_src/net/base/features.h +++ b/chromium_src/net/base/features.h @@ -14,6 +14,8 @@ namespace features { NET_EXPORT extern const base::Feature kBraveEphemeralStorage; NET_EXPORT extern const base::Feature kBraveEphemeralStorageKeepAlive; +NET_EXPORT extern const base::FeatureParam + kBraveEphemeralStorageKeepAliveTimeInSeconds; } // namespace features } // namespace net From 0d4b75d186c6719cb7d0a280d4c6e3f29db060e6 Mon Sep 17 00:00:00 2001 From: bridiver Date: Tue, 20 Apr 2021 18:22:57 -0700 Subject: [PATCH 03/15] keep local storage alive --- browser/ephemeral_storage/BUILD.gn | 1 + .../ephemeral_storage_browsertest.cc | 124 +++++++++++++++--- .../ephemeral_storage_tab_helper.cc | 57 +++++--- .../ephemeral_storage_tab_helper.h | 4 + 4 files changed, 145 insertions(+), 41 deletions(-) diff --git a/browser/ephemeral_storage/BUILD.gn b/browser/ephemeral_storage/BUILD.gn index 75fb20cf37ee..041831746aad 100644 --- a/browser/ephemeral_storage/BUILD.gn +++ b/browser/ephemeral_storage/BUILD.gn @@ -22,6 +22,7 @@ if (!is_android) { ] defines = [ "HAS_OUT_OF_PROC_TEST_RUNNER" ] deps = [ + ":ephemeral_storage", "//base", "//brave/components/brave_shields/browser:browser", "//brave/components/brave_shields/common:common", diff --git a/browser/ephemeral_storage/ephemeral_storage_browsertest.cc b/browser/ephemeral_storage/ephemeral_storage_browsertest.cc index 5fe3056ea96d..1f157ea88081 100644 --- a/browser/ephemeral_storage/ephemeral_storage_browsertest.cc +++ b/browser/ephemeral_storage/ephemeral_storage_browsertest.cc @@ -6,6 +6,9 @@ #include #include "base/path_service.h" +#include "base/threading/platform_thread.h" +#include "base/time/time.h" +#include "brave/browser/ephemeral_storage/ephemeral_storage_tab_helper.h" #include "brave/common/brave_paths.h" #include "brave/components/brave_shields/browser/brave_shields_util.h" #include "brave/components/brave_shields/common/brave_shield_constants.h" @@ -33,6 +36,8 @@ using net::test_server::EmbeddedTestServer; namespace { +const int kKeepAliveInterval = 2; + enum StorageType { Session, Local }; const char* ToString(StorageType storage_type) { @@ -100,6 +105,10 @@ class EphemeralStorageBrowserTest : public InProcessBrowserTest { https_server_.GetURL("b.com", "/ephemeral_storage.html"); c_site_ephemeral_storage_url_ = https_server_.GetURL("c.com", "/ephemeral_storage.html"); + + ephemeral_storage::EphemeralStorageTabHelper:: + SetKeepAliveTimeDelayForTesting( + base::TimeDelta::FromSeconds(kKeepAliveInterval)); } void SetUpCommandLine(base::CommandLine* command_line) override { @@ -253,41 +262,66 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest, StorageIsPartitioned) { } IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest, - NavigatingClearsEphemeralStorage) { + NavigatingClearsEphemeralStorageAfterKeepAlive) { ui_test_utils::NavigateToURL(browser(), a_site_ephemeral_storage_url_); auto* web_contents = browser()->tab_strip_model()->GetActiveWebContents(); + ASSERT_TRUE(WaitForLoadStop(web_contents)); SetValuesInFrames(web_contents, "a.com value", "from=a.com"); - ValuesFromFrames values_before = GetValuesFromFrames(web_contents); - EXPECT_EQ("a.com value", values_before.main_frame.local_storage); - EXPECT_EQ("a.com value", values_before.iframe_1.local_storage); - EXPECT_EQ("a.com value", values_before.iframe_2.local_storage); + ValuesFromFrames values = GetValuesFromFrames(web_contents); + EXPECT_EQ("a.com value", values.main_frame.local_storage); + EXPECT_EQ("a.com value", values.iframe_1.local_storage); + EXPECT_EQ("a.com value", values.iframe_2.local_storage); - EXPECT_EQ("a.com value", values_before.main_frame.session_storage); - EXPECT_EQ("a.com value", values_before.iframe_1.session_storage); - EXPECT_EQ("a.com value", values_before.iframe_2.session_storage); + EXPECT_EQ("a.com value", values.main_frame.session_storage); + EXPECT_EQ("a.com value", values.iframe_1.session_storage); + EXPECT_EQ("a.com value", values.iframe_2.session_storage); - EXPECT_EQ("from=a.com", values_before.main_frame.cookies); - EXPECT_EQ("from=a.com", values_before.iframe_1.cookies); - EXPECT_EQ("from=a.com", values_before.iframe_2.cookies); + EXPECT_EQ("from=a.com", values.main_frame.cookies); + EXPECT_EQ("from=a.com", values.iframe_1.cookies); + EXPECT_EQ("from=a.com", values.iframe_2.cookies); // Navigate away and then navigate back to the original site. ui_test_utils::NavigateToURL(browser(), b_site_ephemeral_storage_url_); + ASSERT_TRUE(WaitForLoadStop(web_contents)); ui_test_utils::NavigateToURL(browser(), a_site_ephemeral_storage_url_); + ASSERT_TRUE(WaitForLoadStop(web_contents)); - ValuesFromFrames values_after = GetValuesFromFrames(web_contents); - EXPECT_EQ("a.com value", values_after.main_frame.local_storage); - EXPECT_EQ(nullptr, values_after.iframe_1.local_storage); - EXPECT_EQ(nullptr, values_after.iframe_2.local_storage); + // within keepalive values should be the same + ValuesFromFrames before_timeout = GetValuesFromFrames(web_contents); + EXPECT_EQ("a.com value", before_timeout.main_frame.local_storage); + EXPECT_EQ("a.com value", before_timeout.iframe_1.local_storage); + EXPECT_EQ("a.com value", before_timeout.iframe_2.local_storage); - EXPECT_EQ("a.com value", values_after.main_frame.session_storage); - EXPECT_EQ(nullptr, values_after.iframe_1.session_storage); - EXPECT_EQ(nullptr, values_after.iframe_2.session_storage); + EXPECT_EQ("a.com value", before_timeout.main_frame.session_storage); + // TODO(bridiver) - do we need to persist session storage? + // EXPECT_EQ("a.com value", before_timeout.iframe_1.session_storage); + // EXPECT_EQ("a.com value", before_timeout.iframe_2.session_storage); - EXPECT_EQ("from=a.com", values_after.main_frame.cookies); - EXPECT_EQ("", values_after.iframe_1.cookies); - EXPECT_EQ("", values_after.iframe_2.cookies); + EXPECT_EQ("from=a.com", before_timeout.main_frame.cookies); + EXPECT_EQ("from=a.com", before_timeout.iframe_1.cookies); + EXPECT_EQ("from=a.com", before_timeout.iframe_2.cookies); + + // after keepalive values should be cleared + ui_test_utils::NavigateToURL(browser(), b_site_ephemeral_storage_url_); + ASSERT_TRUE(WaitForLoadStop(web_contents)); + base::PlatformThread::Sleep(base::TimeDelta::FromSeconds(kKeepAliveInterval)); + ui_test_utils::NavigateToURL(browser(), a_site_ephemeral_storage_url_); + ASSERT_TRUE(WaitForLoadStop(web_contents)); + + ValuesFromFrames after_timeout = GetValuesFromFrames(web_contents); + EXPECT_EQ("a.com value", after_timeout.main_frame.local_storage); + EXPECT_EQ(nullptr, after_timeout.iframe_1.local_storage); + EXPECT_EQ(nullptr, after_timeout.iframe_2.local_storage); + + EXPECT_EQ("a.com value", after_timeout.main_frame.session_storage); + EXPECT_EQ(nullptr, after_timeout.iframe_1.session_storage); + EXPECT_EQ(nullptr, after_timeout.iframe_2.session_storage); + + EXPECT_EQ("from=a.com", after_timeout.main_frame.cookies); + EXPECT_EQ("", after_timeout.iframe_1.cookies); + EXPECT_EQ("", after_timeout.iframe_2.cookies); } IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest, @@ -533,3 +567,51 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest, EXPECT_EQ("third-party-a.com", third_party_values.session_storage); EXPECT_EQ("name=third-party-a.com", third_party_values.cookies); } + +class EphemeralStorageKeepAliveDisabledBrowserTest + : public EphemeralStorageBrowserTest { + public: + EphemeralStorageKeepAliveDisabledBrowserTest() + : EphemeralStorageBrowserTest() { + scoped_feature_list_.InitAndDisableFeature( + net::features::kBraveEphemeralStorageKeepAlive); + } +}; + +IN_PROC_BROWSER_TEST_F(EphemeralStorageKeepAliveDisabledBrowserTest, + NavigatingClearsEphemeralStorageAfterKeepAlive) { + ui_test_utils::NavigateToURL(browser(), a_site_ephemeral_storage_url_); + auto* web_contents = browser()->tab_strip_model()->GetActiveWebContents(); + + SetValuesInFrames(web_contents, "a.com value", "from=a.com"); + + ValuesFromFrames values_before = GetValuesFromFrames(web_contents); + EXPECT_EQ("a.com value", values_before.main_frame.local_storage); + EXPECT_EQ("a.com value", values_before.iframe_1.local_storage); + EXPECT_EQ("a.com value", values_before.iframe_2.local_storage); + + EXPECT_EQ("a.com value", values_before.main_frame.session_storage); + EXPECT_EQ("a.com value", values_before.iframe_1.session_storage); + EXPECT_EQ("a.com value", values_before.iframe_2.session_storage); + + EXPECT_EQ("from=a.com", values_before.main_frame.cookies); + EXPECT_EQ("from=a.com", values_before.iframe_1.cookies); + EXPECT_EQ("from=a.com", values_before.iframe_2.cookies); + + // Navigate away and then navigate back to the original site. + ui_test_utils::NavigateToURL(browser(), b_site_ephemeral_storage_url_); + ui_test_utils::NavigateToURL(browser(), a_site_ephemeral_storage_url_); + + ValuesFromFrames values_after = GetValuesFromFrames(web_contents); + EXPECT_EQ("a.com value", values_after.main_frame.local_storage); + EXPECT_EQ(nullptr, values_after.iframe_1.local_storage); + EXPECT_EQ(nullptr, values_after.iframe_2.local_storage); + + EXPECT_EQ("a.com value", values_after.main_frame.session_storage); + EXPECT_EQ(nullptr, values_after.iframe_1.session_storage); + EXPECT_EQ(nullptr, values_after.iframe_2.session_storage); + + EXPECT_EQ("from=a.com", values_after.main_frame.cookies); + EXPECT_EQ("", values_after.iframe_1.cookies); + EXPECT_EQ("", values_after.iframe_2.cookies); +} diff --git a/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc b/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc index 8b8615bc928b..532a97ed23d9 100644 --- a/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc +++ b/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc @@ -71,6 +71,7 @@ EphemeralStorageTabHelper::~EphemeralStorageTabHelper() {} void EphemeralStorageTabHelper::WebContentsDestroyed() { keep_alive_tld_ephemeral_lifetime_list_.clear(); + keep_alive_local_storage_list_.clear(); } void EphemeralStorageTabHelper::ReadyToCommitNavigation( @@ -93,8 +94,11 @@ void EphemeralStorageTabHelper::ReadyToCommitNavigation( void EphemeralStorageTabHelper::ClearEphemeralLifetimeKeepalive( const content::TLDEphemeralLifetimeKey& key) { - for (auto it = keep_alive_tld_ephemeral_lifetime_list_.begin(); it != keep_alive_tld_ephemeral_lifetime_list_.end(); - ++it) { + ClearLocalStorageKeepAlive( + StringToSessionStorageId(key.second, kLocalStorageSuffix)); + + for (auto it = keep_alive_tld_ephemeral_lifetime_list_.begin(); + it != keep_alive_tld_ephemeral_lifetime_list_.end(); ++it) { if ((*it)->key() == key) { keep_alive_tld_ephemeral_lifetime_list_.erase(it); return; @@ -103,6 +107,18 @@ void EphemeralStorageTabHelper::ClearEphemeralLifetimeKeepalive( NOTREACHED(); } +void EphemeralStorageTabHelper::ClearLocalStorageKeepAlive( + const std::string& id) { + for (auto it = keep_alive_local_storage_list_.begin(); + it != keep_alive_local_storage_list_.end(); ++it) { + if ((*it)->id() == id) { + keep_alive_local_storage_list_.erase(it); + return; + } + } + NOTREACHED(); +} + void EphemeralStorageTabHelper::CreateEphemeralStorageAreasForDomainAndURL( const std::string& new_domain, const GURL& new_url) { @@ -115,6 +131,25 @@ void EphemeralStorageTabHelper::CreateEphemeralStorageAreasForDomainAndURL( auto* partition = BrowserContext::GetStoragePartition(browser_context, site_instance.get()); + if (base::FeatureList::IsEnabled( + net::features::kBraveEphemeralStorageKeepAlive) && + tld_ephemeral_lifetime_) { + keep_alive_tld_ephemeral_lifetime_list_.push_back(tld_ephemeral_lifetime_); + keep_alive_local_storage_list_.push_back(local_storage_namespace_); + + // keep the ephemeral storage alive for some time to handle redirects + // including meta refresh or other page driven "redirects" that end up back + // at the original origin + base::SequencedTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, + base::BindOnce( + &EphemeralStorageTabHelper::ClearEphemeralLifetimeKeepalive, + weak_factory_.GetWeakPtr(), tld_ephemeral_lifetime_->key()), + g_storage_keep_alive_for_testing.is_min() + ? kStorageKeepAliveDelay + : g_storage_keep_alive_for_testing); + } + // This will fetch a session storage namespace for this storage partition // and storage domain. If another tab helper is already using the same // namespace, this will just give us a new reference. When the last tab helper @@ -147,24 +182,6 @@ void EphemeralStorageTabHelper::CreateEphemeralStorageAreasForDomainAndURL( kSessionStorageSuffix)) : base::nullopt); - if (base::FeatureList::IsEnabled( - net::features::kBraveEphemeralStorageKeepAlive) && - tld_ephemeral_lifetime_) { - keep_alive_tld_ephemeral_lifetime_list_.push_back(tld_ephemeral_lifetime_); - auto key = tld_ephemeral_lifetime_->key(); - // keep the ephemeral storage alive for some time to handle redirects - // including meta refresh or other page driven "redirects" that end up back - // at the original origin - base::SequencedTaskRunnerHandle::Get()->PostDelayedTask( - FROM_HERE, - base::BindOnce( - &EphemeralStorageTabHelper::ClearEphemeralLifetimeKeepalive, - weak_factory_.GetWeakPtr(), key), - g_storage_keep_alive_for_testing.is_min() - ? kStorageKeepAliveDelay - : g_storage_keep_alive_for_testing); - } - tld_ephemeral_lifetime_ = content::TLDEphemeralLifetime::GetOrCreate( browser_context, partition, new_domain); } diff --git a/browser/ephemeral_storage/ephemeral_storage_tab_helper.h b/browser/ephemeral_storage/ephemeral_storage_tab_helper.h index 5a2f256cbd94..dac04829da4f 100644 --- a/browser/ephemeral_storage/ephemeral_storage_tab_helper.h +++ b/browser/ephemeral_storage/ephemeral_storage_tab_helper.h @@ -45,6 +45,8 @@ class EphemeralStorageTabHelper void ClearEphemeralLifetimeKeepalive( const content::TLDEphemeralLifetimeKey& key); + void ClearLocalStorageKeepAlive(const std::string& id); + void CreateEphemeralStorageAreasForDomainAndURL(const std::string& new_domain, const GURL& new_url); @@ -55,6 +57,8 @@ class EphemeralStorageTabHelper std::vector> keep_alive_tld_ephemeral_lifetime_list_; + std::vector> + keep_alive_local_storage_list_; base::WeakPtrFactory weak_factory_{this}; From 53adebc4a52a92c803cfc423739482cab4ddebcd Mon Sep 17 00:00:00 2001 From: bridiver Date: Tue, 20 Apr 2021 19:19:39 -0700 Subject: [PATCH 04/15] update session storage tests --- browser/ephemeral_storage/ephemeral_storage_browsertest.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/browser/ephemeral_storage/ephemeral_storage_browsertest.cc b/browser/ephemeral_storage/ephemeral_storage_browsertest.cc index 1f157ea88081..581efc6b22f7 100644 --- a/browser/ephemeral_storage/ephemeral_storage_browsertest.cc +++ b/browser/ephemeral_storage/ephemeral_storage_browsertest.cc @@ -294,10 +294,10 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest, EXPECT_EQ("a.com value", before_timeout.iframe_1.local_storage); EXPECT_EQ("a.com value", before_timeout.iframe_2.local_storage); + // keepalive does not apply to session storage EXPECT_EQ("a.com value", before_timeout.main_frame.session_storage); - // TODO(bridiver) - do we need to persist session storage? - // EXPECT_EQ("a.com value", before_timeout.iframe_1.session_storage); - // EXPECT_EQ("a.com value", before_timeout.iframe_2.session_storage); + EXPECT_EQ(nullptr, before_timeout.iframe_1.session_storage); + EXPECT_EQ(nullptr, before_timeout.iframe_2.session_storage); EXPECT_EQ("from=a.com", before_timeout.main_frame.cookies); EXPECT_EQ("from=a.com", before_timeout.iframe_1.cookies); From 85c1500450aa4ebd6b307936bac1672747d7f598 Mon Sep 17 00:00:00 2001 From: bridiver Date: Tue, 20 Apr 2021 19:20:39 -0700 Subject: [PATCH 05/15] lint --- browser/ephemeral_storage/ephemeral_storage_tab_helper.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/browser/ephemeral_storage/ephemeral_storage_tab_helper.h b/browser/ephemeral_storage/ephemeral_storage_tab_helper.h index dac04829da4f..4480a6e8a307 100644 --- a/browser/ephemeral_storage/ephemeral_storage_tab_helper.h +++ b/browser/ephemeral_storage/ephemeral_storage_tab_helper.h @@ -56,9 +56,9 @@ class EphemeralStorageTabHelper scoped_refptr tld_ephemeral_lifetime_; std::vector> - keep_alive_tld_ephemeral_lifetime_list_; + keep_alive_tld_ephemeral_lifetime_list_; std::vector> - keep_alive_local_storage_list_; + keep_alive_local_storage_list_; base::WeakPtrFactory weak_factory_{this}; From f1b8ad49295c4f6458ab4eb94f5367c5b5357db6 Mon Sep 17 00:00:00 2001 From: bridiver Date: Wed, 21 Apr 2021 09:48:02 -0700 Subject: [PATCH 06/15] don't block the whole process waiting --- .../ephemeral_storage/ephemeral_storage_browsertest.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/browser/ephemeral_storage/ephemeral_storage_browsertest.cc b/browser/ephemeral_storage/ephemeral_storage_browsertest.cc index 581efc6b22f7..13b26e56ec4c 100644 --- a/browser/ephemeral_storage/ephemeral_storage_browsertest.cc +++ b/browser/ephemeral_storage/ephemeral_storage_browsertest.cc @@ -6,7 +6,7 @@ #include #include "base/path_service.h" -#include "base/threading/platform_thread.h" +#include "base/threading/sequenced_task_runner_handle.h" #include "base/time/time.h" #include "brave/browser/ephemeral_storage/ephemeral_storage_tab_helper.h" #include "brave/common/brave_paths.h" @@ -306,7 +306,13 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest, // after keepalive values should be cleared ui_test_utils::NavigateToURL(browser(), b_site_ephemeral_storage_url_); ASSERT_TRUE(WaitForLoadStop(web_contents)); - base::PlatformThread::Sleep(base::TimeDelta::FromSeconds(kKeepAliveInterval)); + + base::RunLoop run_loop; + base::SequencedTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, run_loop.QuitClosure(), + base::TimeDelta::FromSeconds(kKeepAliveInterval)); + run_loop.Run(); + ui_test_utils::NavigateToURL(browser(), a_site_ephemeral_storage_url_); ASSERT_TRUE(WaitForLoadStop(web_contents)); From e58d8f9f89cdf93af9eaf588dd75ec3f5bedf39b Mon Sep 17 00:00:00 2001 From: bridiver Date: Wed, 21 Apr 2021 09:48:09 -0700 Subject: [PATCH 07/15] correct test name --- browser/ephemeral_storage/ephemeral_storage_browsertest.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/browser/ephemeral_storage/ephemeral_storage_browsertest.cc b/browser/ephemeral_storage/ephemeral_storage_browsertest.cc index 13b26e56ec4c..c10cf5b267c8 100644 --- a/browser/ephemeral_storage/ephemeral_storage_browsertest.cc +++ b/browser/ephemeral_storage/ephemeral_storage_browsertest.cc @@ -585,7 +585,7 @@ class EphemeralStorageKeepAliveDisabledBrowserTest }; IN_PROC_BROWSER_TEST_F(EphemeralStorageKeepAliveDisabledBrowserTest, - NavigatingClearsEphemeralStorageAfterKeepAlive) { + NavigatingClearsEphemeralStorageWhenKeepAliveDisabled) { ui_test_utils::NavigateToURL(browser(), a_site_ephemeral_storage_url_); auto* web_contents = browser()->tab_strip_model()->GetActiveWebContents(); From eb891664cc32234442c50cc861059384c48b615f Mon Sep 17 00:00:00 2001 From: bridiver Date: Wed, 21 Apr 2021 09:48:26 -0700 Subject: [PATCH 08/15] don't initialize the keep alive timeout before field trials initialize --- browser/ephemeral_storage/ephemeral_storage_tab_helper.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc b/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc index 532a97ed23d9..97aa355270e1 100644 --- a/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc +++ b/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc @@ -33,9 +33,6 @@ namespace { constexpr char kSessionStorageSuffix[] = "/ephemeral-session-storage"; constexpr char kLocalStorageSuffix[] = "/ephemeral-local-storage"; -const base::TimeDelta kStorageKeepAliveDelay = base::TimeDelta::FromSeconds( - net::features::kBraveEphemeralStorageKeepAliveTimeInSeconds.Get()); - base::TimeDelta g_storage_keep_alive_for_testing = base::TimeDelta::Min(); // Session storage ids are expected to be 36 character long GUID strings. Since @@ -146,7 +143,9 @@ void EphemeralStorageTabHelper::CreateEphemeralStorageAreasForDomainAndURL( &EphemeralStorageTabHelper::ClearEphemeralLifetimeKeepalive, weak_factory_.GetWeakPtr(), tld_ephemeral_lifetime_->key()), g_storage_keep_alive_for_testing.is_min() - ? kStorageKeepAliveDelay + ? base::TimeDelta::FromSeconds( + net::features::kBraveEphemeralStorageKeepAliveTimeInSeconds + .Get()) : g_storage_keep_alive_for_testing); } From 87c4a2b44fea552f1a6aae7be577ab0ecbef423c Mon Sep 17 00:00:00 2001 From: bridiver Date: Wed, 21 Apr 2021 09:52:13 -0700 Subject: [PATCH 09/15] ephemeral storage is enabled by default now --- browser/ephemeral_storage/ephemeral_storage_browsertest.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/browser/ephemeral_storage/ephemeral_storage_browsertest.cc b/browser/ephemeral_storage/ephemeral_storage_browsertest.cc index c10cf5b267c8..a0fcf20b3bff 100644 --- a/browser/ephemeral_storage/ephemeral_storage_browsertest.cc +++ b/browser/ephemeral_storage/ephemeral_storage_browsertest.cc @@ -80,10 +80,7 @@ content::EvalJsResult GetCookiesInFrame(RenderFrameHost* host) { class EphemeralStorageBrowserTest : public InProcessBrowserTest { public: EphemeralStorageBrowserTest() - : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) { - scoped_feature_list_.InitAndEnableFeature( - net::features::kBraveEphemeralStorage); - } + : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {} void SetUpOnMainThread() override { InProcessBrowserTest::SetUpOnMainThread(); From b6e845a4038ed53c1d689a0008522f17b1e05119 Mon Sep 17 00:00:00 2001 From: bridiver Date: Wed, 21 Apr 2021 18:36:33 -0700 Subject: [PATCH 10/15] use keepalive for permissions since they are tied to ephemeral storage --- browser/permissions/BUILD.gn | 1 + ...permission_lifetime_manager_browsertest.cc | 50 +++++++++++++++++-- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/browser/permissions/BUILD.gn b/browser/permissions/BUILD.gn index 048dec8c6ae6..8a631d8035e4 100644 --- a/browser/permissions/BUILD.gn +++ b/browser/permissions/BUILD.gn @@ -13,6 +13,7 @@ if (!is_android) { deps = [ "//brave/browser:browser_process", + "//brave/browser/ephemeral_storage", "//chrome/browser", "//chrome/browser/ui", "//chrome/test:test_support", diff --git a/browser/permissions/permission_lifetime_manager_browsertest.cc b/browser/permissions/permission_lifetime_manager_browsertest.cc index d481f4b0370e..5f97826a33eb 100644 --- a/browser/permissions/permission_lifetime_manager_browsertest.cc +++ b/browser/permissions/permission_lifetime_manager_browsertest.cc @@ -14,6 +14,9 @@ #include "base/test/scoped_feature_list.h" #include "base/test/scoped_mock_time_message_loop_task_runner.h" #include "base/test/test_mock_time_task_runner.h" +#include "base/threading/sequenced_task_runner_handle.h" +#include "base/time/time.h" +#include "brave/browser/ephemeral_storage/ephemeral_storage_tab_helper.h" #include "brave/browser/permissions/mock_permission_lifetime_prompt_factory.h" #include "brave/browser/permissions/permission_lifetime_manager_factory.h" #include "brave/components/permissions/permission_lifetime_pref_names.h" @@ -46,6 +49,7 @@ using testing::_; namespace permissions { namespace { +const int kKeepAliveInterval = 2; const char kPreTestDataFileName[] = "pre_test_data"; } // namespace @@ -273,8 +277,9 @@ class PermissionLifetimeManagerWithOriginMonitorBrowserTest : public PermissionLifetimeManagerBrowserTest { public: PermissionLifetimeManagerWithOriginMonitorBrowserTest() { - scoped_feature_list_.InitAndEnableFeature( - net::features::kBraveEphemeralStorage); + ephemeral_storage::EphemeralStorageTabHelper:: + SetKeepAliveTimeDelayForTesting( + base::TimeDelta::FromSeconds(kKeepAliveInterval)); } protected: @@ -305,10 +310,21 @@ IN_PROC_BROWSER_TEST_F(PermissionLifetimeManagerWithOriginMonitorBrowserTest, url, url, ContentSettingsType::GEOLOCATION), ContentSetting::CONTENT_SETTING_ALLOW); - // Navigate to another domain. It should reset the permission. + // Navigate to another domain. It should not reset the permission. const GURL& other_url = https_server()->GetURL("other_host.com", "/empty.html"); ui_test_utils::NavigateToURL(browser(), other_url); + EXPECT_EQ(host_content_settings_map()->GetContentSetting( + url, url, ContentSettingsType::GEOLOCATION), + ContentSetting::CONTENT_SETTING_ALLOW); + + // Permission Should be reset after the timeout + base::RunLoop run_loop; + base::SequencedTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, run_loop.QuitClosure(), + base::TimeDelta::FromSeconds(kKeepAliveInterval)); + run_loop.Run(); + EXPECT_EQ(host_content_settings_map()->GetContentSetting( url, url, ContentSettingsType::GEOLOCATION), ContentSetting::CONTENT_SETTING_ASK); @@ -348,10 +364,22 @@ IN_PROC_BROWSER_TEST_F(PermissionLifetimeManagerWithOriginMonitorBrowserTest, ContentSetting::CONTENT_SETTING_ALLOW); EXPECT_FALSE(GetExpirationsPrefValue()->DictEmpty()); - // Navigate to another domain. It should reset the permission. + // Navigate to another domain. It should keep the permission. const GURL& other_url = https_server()->GetURL("other_host.com", "/empty.html"); ui_test_utils::NavigateToURL(browser(), other_url); + EXPECT_EQ(host_content_settings_map()->GetContentSetting( + url, url, ContentSettingsType::GEOLOCATION), + ContentSetting::CONTENT_SETTING_ALLOW); + EXPECT_FALSE(GetExpirationsPrefValue()->DictEmpty()); + + // Permission Should be reset after the timeout + base::RunLoop run_loop; + base::SequencedTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, run_loop.QuitClosure(), + base::TimeDelta::FromSeconds(kKeepAliveInterval)); + run_loop.Run(); + EXPECT_EQ(host_content_settings_map()->GetContentSetting( url, url, ContentSettingsType::GEOLOCATION), ContentSetting::CONTENT_SETTING_ASK); @@ -391,10 +419,22 @@ IN_PROC_BROWSER_TEST_F(PermissionLifetimeManagerWithOriginMonitorBrowserTest, ContentSetting::CONTENT_SETTING_ALLOW); EXPECT_FALSE(GetExpirationsPrefValue()->DictEmpty()); - // Navigate to another domain in PSL. It should reset the permission. + // Navigate to another domain in PSL. It should keep the permission. const GURL& other_url = https_server()->GetURL("user2.github.io", "/empty.html"); ui_test_utils::NavigateToURL(browser(), other_url); + EXPECT_EQ(host_content_settings_map()->GetContentSetting( + url, url, ContentSettingsType::GEOLOCATION), + ContentSetting::CONTENT_SETTING_ALLOW); + EXPECT_FALSE(GetExpirationsPrefValue()->DictEmpty()); + + // Permission Should be reset after the timeout + base::RunLoop run_loop; + base::SequencedTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, run_loop.QuitClosure(), + base::TimeDelta::FromSeconds(kKeepAliveInterval)); + run_loop.Run(); + EXPECT_EQ(host_content_settings_map()->GetContentSetting( url, url, ContentSettingsType::GEOLOCATION), ContentSetting::CONTENT_SETTING_ASK); From 3dbe05a0bb0f23ee5a484f862259c81399fadc2b Mon Sep 17 00:00:00 2001 From: bridiver Date: Thu, 22 Apr 2021 09:15:15 -0700 Subject: [PATCH 11/15] ScopedFeatureList is no longer needed in base class --- browser/ephemeral_storage/ephemeral_storage_browsertest.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/browser/ephemeral_storage/ephemeral_storage_browsertest.cc b/browser/ephemeral_storage/ephemeral_storage_browsertest.cc index a0fcf20b3bff..20361fb9e003 100644 --- a/browser/ephemeral_storage/ephemeral_storage_browsertest.cc +++ b/browser/ephemeral_storage/ephemeral_storage_browsertest.cc @@ -173,7 +173,6 @@ class EphemeralStorageBrowserTest : public InProcessBrowserTest { protected: net::test_server::EmbeddedTestServer https_server_; - base::test::ScopedFeatureList scoped_feature_list_; GURL a_site_ephemeral_storage_url_; GURL b_site_ephemeral_storage_url_; GURL c_site_ephemeral_storage_url_; @@ -579,6 +578,9 @@ class EphemeralStorageKeepAliveDisabledBrowserTest scoped_feature_list_.InitAndDisableFeature( net::features::kBraveEphemeralStorageKeepAlive); } + + private: + base::test::ScopedFeatureList scoped_feature_list_; }; IN_PROC_BROWSER_TEST_F(EphemeralStorageKeepAliveDisabledBrowserTest, From e3c2b1f03a1df5f9e5fd7c550cf1a31c936264ef Mon Sep 17 00:00:00 2001 From: bridiver Date: Thu, 22 Apr 2021 09:15:39 -0700 Subject: [PATCH 12/15] remove unnecessary WaitForLoadStop --- browser/ephemeral_storage/ephemeral_storage_browsertest.cc | 5 ----- 1 file changed, 5 deletions(-) diff --git a/browser/ephemeral_storage/ephemeral_storage_browsertest.cc b/browser/ephemeral_storage/ephemeral_storage_browsertest.cc index 20361fb9e003..c20722c21a28 100644 --- a/browser/ephemeral_storage/ephemeral_storage_browsertest.cc +++ b/browser/ephemeral_storage/ephemeral_storage_browsertest.cc @@ -261,7 +261,6 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest, NavigatingClearsEphemeralStorageAfterKeepAlive) { ui_test_utils::NavigateToURL(browser(), a_site_ephemeral_storage_url_); auto* web_contents = browser()->tab_strip_model()->GetActiveWebContents(); - ASSERT_TRUE(WaitForLoadStop(web_contents)); SetValuesInFrames(web_contents, "a.com value", "from=a.com"); @@ -280,9 +279,7 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest, // Navigate away and then navigate back to the original site. ui_test_utils::NavigateToURL(browser(), b_site_ephemeral_storage_url_); - ASSERT_TRUE(WaitForLoadStop(web_contents)); ui_test_utils::NavigateToURL(browser(), a_site_ephemeral_storage_url_); - ASSERT_TRUE(WaitForLoadStop(web_contents)); // within keepalive values should be the same ValuesFromFrames before_timeout = GetValuesFromFrames(web_contents); @@ -301,7 +298,6 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest, // after keepalive values should be cleared ui_test_utils::NavigateToURL(browser(), b_site_ephemeral_storage_url_); - ASSERT_TRUE(WaitForLoadStop(web_contents)); base::RunLoop run_loop; base::SequencedTaskRunnerHandle::Get()->PostDelayedTask( @@ -310,7 +306,6 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest, run_loop.Run(); ui_test_utils::NavigateToURL(browser(), a_site_ephemeral_storage_url_); - ASSERT_TRUE(WaitForLoadStop(web_contents)); ValuesFromFrames after_timeout = GetValuesFromFrames(web_contents); EXPECT_EQ("a.com value", after_timeout.main_frame.local_storage); From 316a3e2a322a37312059c064e516a0f545dbbb41 Mon Sep 17 00:00:00 2001 From: bridiver Date: Thu, 22 Apr 2021 09:16:15 -0700 Subject: [PATCH 13/15] use `base::ranges` instead of `for` --- .../ephemeral_storage_tab_helper.cc | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc b/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc index 97aa355270e1..2c93e53af11f 100644 --- a/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc +++ b/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc @@ -10,8 +10,7 @@ #include "base/feature_list.h" #include "base/hash/md5.h" -#include "base/no_destructor.h" -#include "base/optional.h" +#include "base/ranges/ranges.h" #include "base/threading/sequenced_task_runner_handle.h" #include "content/public/browser/navigation_handle.h" #include "content/public/browser/session_storage_namespace.h" @@ -94,26 +93,22 @@ void EphemeralStorageTabHelper::ClearEphemeralLifetimeKeepalive( ClearLocalStorageKeepAlive( StringToSessionStorageId(key.second, kLocalStorageSuffix)); - for (auto it = keep_alive_tld_ephemeral_lifetime_list_.begin(); - it != keep_alive_tld_ephemeral_lifetime_list_.end(); ++it) { - if ((*it)->key() == key) { - keep_alive_tld_ephemeral_lifetime_list_.erase(it); - return; - } - } - NOTREACHED(); + auto it = base::ranges::find_if(keep_alive_tld_ephemeral_lifetime_list_, + [&key](const auto& tld_ephermal_liftime) { + return tld_ephermal_liftime->key() == key; + }); + if (it != keep_alive_tld_ephemeral_lifetime_list_.end()) + keep_alive_tld_ephemeral_lifetime_list_.erase(it); } void EphemeralStorageTabHelper::ClearLocalStorageKeepAlive( const std::string& id) { - for (auto it = keep_alive_local_storage_list_.begin(); - it != keep_alive_local_storage_list_.end(); ++it) { - if ((*it)->id() == id) { - keep_alive_local_storage_list_.erase(it); - return; - } - } - NOTREACHED(); + auto it = base::ranges::find_if(keep_alive_local_storage_list_, + [&id](const auto& local_storage) { + return local_storage->id() == id; + }); + if (it != keep_alive_local_storage_list_.end()) + keep_alive_local_storage_list_.erase(it); } void EphemeralStorageTabHelper::CreateEphemeralStorageAreasForDomainAndURL( From 880110275eacc2d962f308a56ab2423b1e47f17c Mon Sep 17 00:00:00 2001 From: bridiver Date: Thu, 22 Apr 2021 09:16:30 -0700 Subject: [PATCH 14/15] make `key()` const --- chromium_src/content/public/browser/tld_ephemeral_lifetime.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chromium_src/content/public/browser/tld_ephemeral_lifetime.h b/chromium_src/content/public/browser/tld_ephemeral_lifetime.h index 13342725aa02..d7839ff4e5c1 100644 --- a/chromium_src/content/public/browser/tld_ephemeral_lifetime.h +++ b/chromium_src/content/public/browser/tld_ephemeral_lifetime.h @@ -53,7 +53,7 @@ class CONTENT_EXPORT TLDEphemeralLifetime // Add a callback to a callback list to be called on destruction. void RegisterOnDestroyCallback(OnDestroyCallback callback); - const TLDEphemeralLifetimeKey& key() { return key_; } + const TLDEphemeralLifetimeKey& key() const { return key_; } private: friend class RefCounted; From 508c5b8a638a39272f584fe7efd4f49a250faa5a Mon Sep 17 00:00:00 2001 From: bridiver Date: Thu, 22 Apr 2021 15:22:58 -0700 Subject: [PATCH 15/15] lint --- browser/ephemeral_storage/ephemeral_storage_tab_helper.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc b/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc index 2c93e53af11f..dba7b89157e2 100644 --- a/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc +++ b/browser/ephemeral_storage/ephemeral_storage_tab_helper.cc @@ -103,10 +103,9 @@ void EphemeralStorageTabHelper::ClearEphemeralLifetimeKeepalive( void EphemeralStorageTabHelper::ClearLocalStorageKeepAlive( const std::string& id) { - auto it = base::ranges::find_if(keep_alive_local_storage_list_, - [&id](const auto& local_storage) { - return local_storage->id() == id; - }); + auto it = base::ranges::find_if( + keep_alive_local_storage_list_, + [&id](const auto& local_storage) { return local_storage->id() == id; }); if (it != keep_alive_local_storage_list_.end()) keep_alive_local_storage_list_.erase(it); }