Review changes

brave · Jul 29, 2024 · 92ccc3f · 92ccc3f
1 parent ecff2ff
commit 92ccc3f
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 15 deletions.
diff --git a/browser/ephemeral_storage/brave_ephemeral_storage_service_delegate.cc b/browser/ephemeral_storage/brave_ephemeral_storage_service_delegate.cc
@@ -115,16 +115,15 @@ void BraveEphemeralStorageServiceDelegate::CleanupFirstPartyStorageArea(
                             origin_type, std::move(filter_builder));
 }
 
-void BraveEphemeralStorageServiceDelegate::DoesProfileHaveAnyBrowserWindow(
-    std::optional<bool>& is_window_visible) {
+bool BraveEphemeralStorageServiceDelegate::DoesProfileHaveAnyBrowserWindow()
+    const {
   Profile* profile = Profile::FromBrowserContext(context_);
   for (Browser* browser : chrome::FindAllBrowsersWithProfile(profile)) {
     if (browser->window()) {
-      is_window_visible = true;
-      return;
+      return true;
     }
   }
-  is_window_visible = false;
+  return false;
 }
 
 }  // namespace ephemeral_storage
diff --git a/browser/ephemeral_storage/brave_ephemeral_storage_service_delegate.h b/browser/ephemeral_storage/brave_ephemeral_storage_service_delegate.h
@@ -33,8 +33,7 @@ class BraveEphemeralStorageServiceDelegate
   void CleanupTLDEphemeralArea(const TLDEphemeralAreaKey& key) override;
   void CleanupFirstPartyStorageArea(
       const std::string& registerable_domain) override;
-  void DoesProfileHaveAnyBrowserWindow(
-      std::optional<bool>& is_window_visible) override;
+  bool DoesProfileHaveAnyBrowserWindow() const override;
 
  private:
   raw_ptr<content::BrowserContext> context_ = nullptr;

diff --git a/browser/ephemeral_storage/ephemeral_storage_forget_by_default_browsertest.cc b/browser/ephemeral_storage/ephemeral_storage_forget_by_default_browsertest.cc
@@ -581,12 +581,12 @@ class EphemeralStorageForgetByDefaultIncognitoBrowserTest
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
     EphemeralStorageForgetByDefaultBrowserTest::SetUpCommandLine(command_line);
-    if (IsPreTest()) {
+    if (IsPreTestToEnableIncognito()) {
       command_line->AppendSwitch(switches::kIncognito);
     }
   }
 
-  static bool IsPreTest() {
+  static bool IsPreTestToEnableIncognito() {
     const testing::TestInfo* const test_info =
         testing::UnitTest::GetInstance()->current_test_info();
     return base::StartsWith(test_info->name(), "PRE_DontForgetFirstParty");

diff --git a/components/ephemeral_storage/BUILD.gn b/components/ephemeral_storage/BUILD.gn
@@ -8,6 +8,7 @@ static_library("ephemeral_storage") {
     "ephemeral_storage_pref_names.h",
     "ephemeral_storage_service.cc",
     "ephemeral_storage_service.h",
+    "ephemeral_storage_service_delegate.cc",
     "ephemeral_storage_service_delegate.h",
     "ephemeral_storage_service_observer.h",
     "ephemeral_storage_types.h",

diff --git a/components/ephemeral_storage/ephemeral_storage_service.cc b/components/ephemeral_storage/ephemeral_storage_service.cc
@@ -328,9 +328,7 @@ void EphemeralStorageService::ScheduleFirstPartyStorageAreasCleanupOnStartup() {
 void EphemeralStorageService::CleanupFirstPartyStorageAreasOnStartup() {
   DCHECK(!context_->IsOffTheRecord());
 
-  std::optional<bool> is_window_visible = std::nullopt;
-  delegate_->DoesProfileHaveAnyBrowserWindow(is_window_visible);
-  if (is_window_visible.has_value() && !is_window_visible.value()) {
+  if (!delegate_->DoesProfileHaveAnyBrowserWindow()) {
     first_party_storage_areas_to_cleanup_on_startup_.clear();
     return;
   }

diff --git a/components/ephemeral_storage/ephemeral_storage_service_delegate.cc b/components/ephemeral_storage/ephemeral_storage_service_delegate.cc
@@ -0,0 +1,14 @@
+/* Copyright (c) 2024 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 https://mozilla.org/MPL/2.0/. */
+
+#include "brave/components/ephemeral_storage/ephemeral_storage_service_delegate.h"
+
+namespace ephemeral_storage {
+
+bool EphemeralStorageServiceDelegate::DoesProfileHaveAnyBrowserWindow() const {
+  return false;
+}
+
+}  // namespace ephemeral_storage
diff --git a/components/ephemeral_storage/ephemeral_storage_service_delegate.h b/components/ephemeral_storage/ephemeral_storage_service_delegate.h
@@ -6,7 +6,6 @@
 #ifndef BRAVE_COMPONENTS_EPHEMERAL_STORAGE_EPHEMERAL_STORAGE_SERVICE_DELEGATE_H_
 #define BRAVE_COMPONENTS_EPHEMERAL_STORAGE_EPHEMERAL_STORAGE_SERVICE_DELEGATE_H_
 
-#include <optional>
 #include <string>
 #include <utility>
 
@@ -25,8 +24,7 @@ class EphemeralStorageServiceDelegate {
   // Cleanups non-ephemeral first party storage areas (cache, dom storage).
   virtual void CleanupFirstPartyStorageArea(
       const std::string& registerable_domain) {}
-  virtual void DoesProfileHaveAnyBrowserWindow(
-      std::optional<bool>& is_window_visible) {}
+  virtual bool DoesProfileHaveAnyBrowserWindow() const;
 };
 
 }  // namespace ephemeral_storage