Cleanup forgetful browsing storage only if a profile window appeared.

browser/ephemeral_storage/brave_ephemeral_storage_service_delegate.cc

@@ -9,6 +9,7 @@
 
 #include "brave/components/brave_shields/content/browser/brave_shields_util.h"
 #include "chrome/browser/browsing_data/chrome_browsing_data_remover_constants.h"
+#include "chrome/browser/profiles/profile.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browsing_data_filter_builder.h"
@@ -18,6 +19,11 @@
 #include "net/base/schemeful_site.h"
 #include "url/origin.h"
 
+#if !BUILDFLAG(IS_ANDROID)
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list.h"
+#endif
+
 namespace ephemeral_storage {
 
 BraveEphemeralStorageServiceDelegate::BraveEphemeralStorageServiceDelegate(
@@ -32,8 +38,11 @@ BraveEphemeralStorageServiceDelegate::BraveEphemeralStorageServiceDelegate(
   DCHECK(cookie_settings_);
 }
 
-BraveEphemeralStorageServiceDelegate::~BraveEphemeralStorageServiceDelegate() =
-    default;
+BraveEphemeralStorageServiceDelegate::~BraveEphemeralStorageServiceDelegate() {
+#if !BUILDFLAG(IS_ANDROID)
+  BrowserList::RemoveObserver(this);
+#endif
+}
 
 void BraveEphemeralStorageServiceDelegate::CleanupTLDEphemeralArea(
     const TLDEphemeralAreaKey& key) {
@@ -111,4 +120,30 @@ void BraveEphemeralStorageServiceDelegate::CleanupFirstPartyStorageArea(
                             origin_type, std::move(filter_builder));
 }
 
+void BraveEphemeralStorageServiceDelegate::RegisterFirstWindowOpenedCallback(
+    base::OnceClosure callback) {
+  DCHECK(callback);
+#if !BUILDFLAG(IS_ANDROID)
+  BrowserList::AddObserver(this);
+  first_window_opened_callback_ = std::move(callback);
+#else
+  std::move(callback).Run();
+#endif  // !BUILDFLAG(IS_ANDROID)
+}
+
+#if !BUILDFLAG(IS_ANDROID)
+void BraveEphemeralStorageServiceDelegate::OnBrowserAdded(Browser* browser) {
+  if (browser->profile() != Profile::FromBrowserContext(context_)) {
+    return;
+  }
+
+  if (first_window_opened_callback_) {
+    std::move(first_window_opened_callback_).Run();
+  }
+
+  // No need to observe anymore.
+  BrowserList::RemoveObserver(this);
+}
+#endif
+
 }  // namespace ephemeral_storage

browser/ephemeral_storage/brave_ephemeral_storage_service_delegate.h

@@ -13,6 +13,10 @@
 #include "brave/components/ephemeral_storage/ephemeral_storage_service_delegate.h"
 #include "components/content_settings/core/browser/cookie_settings.h"
 
+#if !BUILDFLAG(IS_ANDROID)
+#include "chrome/browser/ui/browser_list_observer.h"
+#endif
+
 namespace content {
 class BrowserContext;
 }
@@ -22,22 +26,34 @@ class HostContentSettingsMap;
 namespace ephemeral_storage {
 
 class BraveEphemeralStorageServiceDelegate
-    : public EphemeralStorageServiceDelegate {
+    : public EphemeralStorageServiceDelegate
+#if !BUILDFLAG(IS_ANDROID)
+    ,
+      public BrowserListObserver {
+#endif
  public:
   BraveEphemeralStorageServiceDelegate(
       content::BrowserContext* context,
       HostContentSettingsMap* host_content_settings_map,
       scoped_refptr<content_settings::CookieSettings> cookie_settings);
   ~BraveEphemeralStorageServiceDelegate() override;
 
+  // EphemeralStorageServiceDelegate:
   void CleanupTLDEphemeralArea(const TLDEphemeralAreaKey& key) override;
   void CleanupFirstPartyStorageArea(
       const std::string& registerable_domain) override;
+  void RegisterFirstWindowOpenedCallback(base::OnceClosure callback) override;
+
+#if !BUILDFLAG(IS_ANDROID)
+  // BrowserListObserver:
+  void OnBrowserAdded(Browser* browser) override;
+#endif
 
  private:
   raw_ptr<content::BrowserContext> context_ = nullptr;
   raw_ptr<HostContentSettingsMap> host_content_settings_map_ = nullptr;
   scoped_refptr<content_settings::CookieSettings> cookie_settings_;
+  base::OnceClosure first_window_opened_callback_;
 };
 
 }  // namespace ephemeral_storage

browser/ephemeral_storage/ephemeral_storage_browsertest.cc

@@ -308,21 +308,21 @@ content::EvalJsResult EphemeralStorageBrowserTest::GetCookiesInFrame(
   return content::EvalJs(host, "document.cookie");
 }
 
-size_t EphemeralStorageBrowserTest::WaitForCleanupAfterKeepAlive(Browser* b) {
-  if (!b) {
-    b = browser();
+size_t EphemeralStorageBrowserTest::WaitForCleanupAfterKeepAlive(
+    Profile* profile) {
+  if (!profile) {
+    profile = browser()->profile();
   }
   const size_t fired_cnt = EphemeralStorageServiceFactory::GetInstance()
-                               ->GetForContext(b->profile())
+                               ->GetForContext(profile)
                                ->FireCleanupTimersForTesting();
 
   // NetworkService closes existing connections when a data removal action
   // linked to these connections is performed. This leads to rare page open
   // failures when the timing is "just right". Do a no-op removal here to make
   // sure the queued Ephemeral Storage cleanup was complete.
   BrowsingDataRemoverObserver data_remover_observer;
-  content::BrowsingDataRemover* remover =
-      b->profile()->GetBrowsingDataRemover();
+  content::BrowsingDataRemover* remover = profile->GetBrowsingDataRemover();
   remover->AddObserver(&data_remover_observer);
   remover->RemoveAndReply(base::Time(), base::Time::Max(), 0, 0,
                           &data_remover_observer);

browser/ephemeral_storage/ephemeral_storage_browsertest.h

@@ -88,7 +88,7 @@ class EphemeralStorageBrowserTest : public InProcessBrowserTest {
                                                StorageType storage_type);
   void SetCookieInFrame(content::RenderFrameHost* host, std::string cookie);
   content::EvalJsResult GetCookiesInFrame(content::RenderFrameHost* host);
-  size_t WaitForCleanupAfterKeepAlive(Browser* browser = nullptr);
+  size_t WaitForCleanupAfterKeepAlive(Profile* profile = nullptr);
   void ExpectValuesFromFramesAreEmpty(const base::Location& location,
                                       const ValuesFromFrames& values);
   void ExpectValuesFromFrameAreEmpty(const base::Location& location,

browser/ephemeral_storage/ephemeral_storage_forget_by_default_browsertest.cc

@@ -4,12 +4,14 @@
  * You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 #include "brave/browser/ephemeral_storage/ephemeral_storage_browsertest.h"
-
 #include "brave/components/brave_shields/content/browser/brave_shields_util.h"
 #include "chrome/browser/content_settings/cookie_settings_factory.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/chrome_test_utils.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/content_settings/core/browser/cookie_settings.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
@@ -206,7 +208,7 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageForgetByDefaultBrowserTest,
   // After keepalive values should be cleared.
   ASSERT_TRUE(ui_test_utils::NavigateToURL(incognito_browser,
                                            b_site_ephemeral_storage_url_));
-  WaitForCleanupAfterKeepAlive(incognito_browser);
+  WaitForCleanupAfterKeepAlive(incognito_browser->profile());
   ASSERT_TRUE(ui_test_utils::NavigateToURL(incognito_browser,
                                            a_site_ephemeral_storage_url_));
 
@@ -570,4 +572,80 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageForgetByDefaultDisabledBrowserTest,
   EXPECT_EQ(1u, GetAllCookies().size());
 }
 
+class EphemeralStorageForgetByDefaultIncognitoBrowserTest
+    : public EphemeralStorageForgetByDefaultBrowserTest {
+ public:
+  EphemeralStorageForgetByDefaultIncognitoBrowserTest() {
+    scoped_feature_list_.InitAndEnableFeature(
+        net::features::kBraveForgetFirstPartyStorage);
+  }
+  ~EphemeralStorageForgetByDefaultIncognitoBrowserTest() override = default;
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    EphemeralStorageForgetByDefaultBrowserTest::SetUpCommandLine(command_line);
+    if (IsPreTestToEnableIncognito()) {
+      command_line->AppendSwitch(switches::kIncognito);
+    }
+  }
+
+  static bool IsPreTestToEnableIncognito() {
+    const testing::TestInfo* const test_info =
+        testing::UnitTest::GetInstance()->current_test_info();
+    return base::StartsWith(test_info->name(), "PRE_DontForgetFirstParty");
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(EphemeralStorageForgetByDefaultIncognitoBrowserTest,
+                       PRE_PRE_DontForgetFirstPartyIfNoBrowserWindowIsActive) {
+  // This PRE test runs in a normal profile and sets a single cookie.
+
+  const GURL a_site_set_cookie_url(
+      "https://a.com/set-cookie?name=acom;path=/"
+      ";SameSite=None;Secure;Max-Age=600");
+  brave_shields::SetForgetFirstPartyStorageEnabled(content_settings(), true,
+                                                   a_site_set_cookie_url);
+
+  // Cookies should NOT exist for a.com.
+  EXPECT_EQ(0u, GetAllCookies().size());
+
+  EXPECT_TRUE(LoadURLInNewTab(a_site_set_cookie_url));
+
+  // Cookies SHOULD exist for a.com.
+  EXPECT_EQ(1u, GetAllCookies().size());
+
+  // Navigate to b.com to activate a deferred cleanup for a.com.
+  ASSERT_TRUE(
+      ui_test_utils::NavigateToURL(browser(), b_site_ephemeral_storage_url_));
+}
+
+IN_PROC_BROWSER_TEST_F(EphemeralStorageForgetByDefaultIncognitoBrowserTest,
+                       PRE_DontForgetFirstPartyIfNoBrowserWindowIsActive) {
+  // This PRE test runs in incognito mode, meaning no normal browser window is
+  // active. This should prevent the cleanup in the normal profile.
+
+  // Ensure no normal browser window is active.
+  EXPECT_TRUE(browser()->profile()->IsOffTheRecord());
+  for (const auto& browser_instance : *BrowserList::GetInstance()) {
+    EXPECT_TRUE(browser_instance->profile()->IsOffTheRecord());
+    EXPECT_EQ(browser_instance->profile(), browser()->profile());
+  }
+
+  EXPECT_EQ(0u, WaitForCleanupAfterKeepAlive());
+  EXPECT_EQ(0u, WaitForCleanupAfterKeepAlive(
+                    browser()->profile()->GetOriginalProfile()));
+}
+
+IN_PROC_BROWSER_TEST_F(EphemeralStorageForgetByDefaultIncognitoBrowserTest,
+                       DontForgetFirstPartyIfNoBrowserWindowIsActive) {
+  // Expect the cleanup did not happen (yet).
+  EXPECT_EQ(1u, GetAllCookies().size());
+
+  // But it is queued and should happen eventually.
+  EXPECT_EQ(1u, WaitForCleanupAfterKeepAlive());
+  EXPECT_EQ(0u, GetAllCookies().size());
+}
+
 }  // namespace ephemeral_storage