Update Containers storage partition utils before session/sync changes. (#34208)

* Update Containers storage partition utils before session/sync changes. * Use char directly. * Add missing buildflag import.
2026-02-27 13:09:00 +00:00
parent 3f35655f1d
commit 78fdbde9f1
5 changed files with 270 additions and 56 deletions
@@ -33,6 +33,8 @@

 namespace containers {

+constexpr char kTestContainerId[] = "test-container-id";
+
 class ContainersBrowserTest : public InProcessBrowserTest {
 public:
  ContainersBrowserTest() : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {
@@ -48,6 +50,11 @@ class ContainersBrowserTest : public InProcessBrowserTest {

  ~ContainersBrowserTest() override = default;

+  void SetUp() override {
+    set_open_about_blank_on_browser_launch(false);
+    InProcessBrowserTest::SetUp();
+  }
+
  void SetUpCommandLine(base::CommandLine* command_line) override {
    InProcessBrowserTest::SetUpCommandLine(command_line);
    command_line->AppendSwitchASCII(
@@ -332,6 +339,73 @@ IN_PROC_BROWSER_TEST_F(ContainersBrowserTest, IsolateCookiesAndStorage) {
                                           GetIndexedDBJS("test_key")));
 }

+IN_PROC_BROWSER_TEST_F(ContainersBrowserTest,
+                       PRE_StoragePersistenceAcrossSessions) {
+  const GURL url("https://a.test/simple.html");
+
+  // Navigate to the page
+  NavigateParams params(browser(), url, ui::PAGE_TRANSITION_LINK);
+  params.disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;
+  params.storage_partition_config = content::StoragePartitionConfig::Create(
+      browser()->profile(), kContainersStoragePartitionDomain, kTestContainerId,
+      browser()->profile()->IsOffTheRecord());
+  ui_test_utils::NavigateToURL(&params);
+
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(web_contents);
+
+  // Set persistent storage data
+  EXPECT_TRUE(content::ExecJs(
+      web_contents, SetCookieJS("persistent_cookie", "persistent_value")));
+  EXPECT_TRUE(content::ExecJs(
+      web_contents, SetLocalStorageJS("persistent_key", "persistent_value")));
+
+  EXPECT_TRUE(content::ExecJs(
+      web_contents, SetIndexedDBJS("persistent_key", "persistent_value")));
+
+  // Verify data is set
+  content::EvalJsResult cookie_result =
+      content::EvalJs(web_contents, GetCookiesJS());
+  EXPECT_TRUE(cookie_result.ExtractString().find(
+                  "persistent_cookie=persistent_value") != std::string::npos);
+
+  EXPECT_EQ("persistent_value",
+            content::EvalJs(web_contents, GetLocalStorageJS("persistent_key")));
+  EXPECT_EQ("persistent_value",
+            content::EvalJs(web_contents, GetIndexedDBJS("persistent_key")));
+}
+
+IN_PROC_BROWSER_TEST_F(ContainersBrowserTest,
+                       StoragePersistenceAcrossSessions) {
+  const GURL url("https://a.test/simple.html");
+
+  // Navigate to the page
+  NavigateParams params(browser(), url, ui::PAGE_TRANSITION_LINK);
+  params.disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;
+  params.storage_partition_config = content::StoragePartitionConfig::Create(
+      browser()->profile(), kContainersStoragePartitionDomain, kTestContainerId,
+      browser()->profile()->IsOffTheRecord());
+  ui_test_utils::NavigateToURL(&params);
+
+  content::WebContents* web_contents_reloaded =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(web_contents_reloaded);
+
+  // Verify persistent data is still available after reload
+  content::EvalJsResult cookie_result_reloaded =
+      content::EvalJs(web_contents_reloaded, GetCookiesJS());
+  EXPECT_TRUE(cookie_result_reloaded.ExtractString().find(
+                  "persistent_cookie=persistent_value") != std::string::npos);
+
+  EXPECT_EQ("persistent_value",
+            content::EvalJs(web_contents_reloaded,
+                            GetLocalStorageJS("persistent_key")));
+  EXPECT_EQ(
+      "persistent_value",
+      content::EvalJs(web_contents_reloaded, GetIndexedDBJS("persistent_key")));
+}
+
 IN_PROC_BROWSER_TEST_F(ContainersBrowserTest,
                       LinkNavigationInheritsContainerStoragePartition) {
  const GURL url("https://a.test/simple.html");
@@ -340,8 +414,8 @@ IN_PROC_BROWSER_TEST_F(ContainersBrowserTest,
  NavigateParams params(browser(), url, ui::PAGE_TRANSITION_LINK);
  params.disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;
  params.storage_partition_config = content::StoragePartitionConfig::Create(
-      browser()->profile(), kContainersStoragePartitionDomain,
-      "container-for-links", browser()->profile()->IsOffTheRecord());
+      browser()->profile(), kContainersStoragePartitionDomain, kTestContainerId,
+      browser()->profile()->IsOffTheRecord());
  ui_test_utils::NavigateToURL(&params);

  content::WebContents* container_web_contents =
@@ -385,6 +459,13 @@ IN_PROC_BROWSER_TEST_F(ContainersBrowserTest,
  // Verify the new tab is on the correct URL
  EXPECT_EQ(url, new_tab_contents->GetLastCommittedURL());

+  content::StoragePartition* storage_partition =
+      new_tab_contents->GetPrimaryMainFrame()->GetStoragePartition();
+  ASSERT_TRUE(storage_partition);
+  EXPECT_EQ(kContainersStoragePartitionDomain,
+            storage_partition->GetConfig().partition_domain());
+  EXPECT_EQ(kTestContainerId, storage_partition->GetConfig().partition_name());
+
  // Verify the new tab has access to the same container storage partition
  content::EvalJsResult cookie_result =
      content::EvalJs(new_tab_contents, GetCookiesJS());
@@ -899,4 +980,70 @@ IN_PROC_BROWSER_TEST_F(ContainersBrowserTest, ShouldShowTabAccent) {
  EXPECT_FALSE(tab_in_container->ShouldShowLargeAccentIcon());
 }

+IN_PROC_BROWSER_TEST_F(ContainersBrowserTest,
+                       PRE_ServiceWorkerPersistenceAcrossSessions) {
+  const GURL url("https://a.test/containers/container_test.html");
+  const GURL worker_url("https://a.test/containers/container_worker.js");
+  const std::string scope = "https://a.test/containers/";
+
+  // Navigate to the page with a container
+  NavigateParams params(browser(), url, ui::PAGE_TRANSITION_LINK);
+  params.disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;
+  params.storage_partition_config = content::StoragePartitionConfig::Create(
+      browser()->profile(), kContainersStoragePartitionDomain, kTestContainerId,
+      browser()->profile()->IsOffTheRecord());
+  ui_test_utils::NavigateToURL(&params);
+
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(web_contents);
+
+  // Register service worker
+  EXPECT_TRUE(content::ExecJs(
+      web_contents, RegisterServiceWorkerJS(worker_url.spec(), scope)));
+
+  // Verify service worker is registered
+  EXPECT_EQ(
+      "registered",
+      content::EvalJs(web_contents, CheckServiceWorkerRegisteredJS(scope)));
+
+  // Set some persistent storage data that the service worker might use
+  EXPECT_TRUE(content::ExecJs(
+      web_contents, SetLocalStorageJS("sw_data", "persistent_value")));
+  EXPECT_TRUE(content::ExecJs(web_contents,
+                              SetCookieJS("sw_cookie", "persistent_cookie")));
+}
+
+IN_PROC_BROWSER_TEST_F(ContainersBrowserTest,
+                       ServiceWorkerPersistenceAcrossSessions) {
+  const GURL url("https://a.test/containers/container_test.html");
+  const std::string scope = "https://a.test/containers/";
+
+  // Navigate to the page with the same container
+  NavigateParams params(browser(), url, ui::PAGE_TRANSITION_LINK);
+  params.disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;
+  params.storage_partition_config = content::StoragePartitionConfig::Create(
+      browser()->profile(), kContainersStoragePartitionDomain, kTestContainerId,
+      browser()->profile()->IsOffTheRecord());
+  ui_test_utils::NavigateToURL(&params);
+
+  content::WebContents* web_contents_reloaded =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(web_contents_reloaded);
+
+  // Verify service worker is still registered after browser restart
+  EXPECT_EQ("registered",
+            content::EvalJs(web_contents_reloaded,
+                            CheckServiceWorkerRegisteredJS(scope)));
+
+  // Verify persistent storage data is still available
+  EXPECT_EQ("persistent_value", content::EvalJs(web_contents_reloaded,
+                                                GetLocalStorageJS("sw_data")));
+
+  content::EvalJsResult cookie_result =
+      content::EvalJs(web_contents_reloaded, GetCookiesJS());
+  EXPECT_TRUE(cookie_result.ExtractString().find(
+                  "sw_cookie=persistent_cookie") != std::string::npos);
+}
+
 }  // namespace containers
@@ -6,6 +6,7 @@
 import("//brave/components/ai_chat/core/common/buildflags/buildflags.gni")
 import("//brave/components/brave_wallet/common/buildflags/buildflags.gni")
 import("//brave/components/brave_wayback_machine/buildflags/buildflags.gni")
+import("//brave/components/containers/buildflags/buildflags.gni")
 import("//brave/components/psst/buildflags/buildflags.gni")
 import("//brave/components/speedreader/common/buildflags/buildflags.gni")
 import("//build/config/features.gni")
@@ -5,6 +5,9 @@

 #include "brave/components/containers/content/browser/storage_partition_utils.h"

+#include <algorithm>
+
+#include "base/strings/string_util.h"
 #include "brave/components/containers/core/common/features.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/browser/storage_partition_config.h"
@@ -15,9 +18,22 @@ namespace containers {
 bool IsContainersStoragePartition(
    const content::StoragePartitionConfig& partition_config) {
  CHECK(base::FeatureList::IsEnabled(features::kContainers));
-  return partition_config.partition_domain() ==
-             kContainersStoragePartitionDomain &&
-         !partition_config.partition_name().empty();
+  return IsContainersStoragePartitionKey(partition_config.partition_domain(),
+                                         partition_config.partition_name());
+}
+
+bool IsContainersStoragePartitionKey(std::string_view partition_domain,
+                                     std::string_view partition_name) {
+  CHECK(base::FeatureList::IsEnabled(features::kContainers));
+  return partition_domain == kContainersStoragePartitionDomain &&
+         IsValidStoragePartitionKeyComponent(partition_name);
+}
+
+bool IsValidStoragePartitionKeyComponent(std::string_view component) {
+  CHECK(base::FeatureList::IsEnabled(features::kContainers));
+  return !component.empty() && std::ranges::all_of(component, [](char c) {
+    return base::IsAsciiAlphaNumeric(c) || c == '-';
+  });
 }

 std::optional<content::StoragePartitionConfig> MaybeInheritStoragePartition(
@@ -33,6 +49,7 @@ std::optional<content::StoragePartitionConfig> MaybeInheritStoragePartition(
 }

 std::string GetContainerIdForWebContents(content::WebContents* web_contents) {
+  CHECK(base::FeatureList::IsEnabled(features::kContainers));
  if (!web_contents) {
    return std::string();
  }
@@ -20,16 +20,25 @@ class WebContents;
 namespace containers {

 // The partition domain identifier used for all containers storage partitions.
-inline constexpr char kContainersStoragePartitionDomain[] =
-    "containers-default";
+inline constexpr char kContainersStoragePartitionDomain[] = "containers";

 // Checks whether a given StoragePartitionConfig belongs to Containers.
-// Partition domain should match kContainersStoragePartitionDomain and partition
-// name should be non-empty.
 COMPONENT_EXPORT(CONTAINERS_CONTENT_BROWSER)
 bool IsContainersStoragePartition(
    const content::StoragePartitionConfig& partition_config);

+// Checks whether a given StoragePartitionConfig partition domain and name
+// belongs to Containers.
+COMPONENT_EXPORT(CONTAINERS_CONTENT_BROWSER)
+bool IsContainersStoragePartitionKey(std::string_view partition_domain,
+                                     std::string_view partition_name);
+
+// Checks whether a storage partition key component is not empty and contains
+// only valid characters for Containers storage partitions. Valid characters are
+// ASCII alphanumeric characters and '-'.
+COMPONENT_EXPORT(CONTAINERS_CONTENT_BROWSER)
+bool IsValidStoragePartitionKeyComponent(std::string_view component);
+
 // Returns the StoragePartitionConfig if it is a Containers storage partition,
 // otherwise returns std::nullopt. Used to conditionally inherit
 // StoragePartitionConfig when creating a new SiteInstance.
@@ -14,60 +14,98 @@

 namespace containers {

-class StoragePartitionUtilsTest : public testing::Test {
+class ContainersStoragePartitionUtilsTest : public testing::Test {
 public:
-  StoragePartitionUtilsTest() {
+  ContainersStoragePartitionUtilsTest() {
    scoped_feature_list_.InitAndEnableFeature(features::kContainers);
  }
-  ~StoragePartitionUtilsTest() override = default;
+  ~ContainersStoragePartitionUtilsTest() override = default;

 protected:
  base::test::ScopedFeatureList scoped_feature_list_;
 };

-TEST_F(StoragePartitionUtilsTest, IsContainersStoragePartition_ValidConfig) {
+TEST_F(ContainersStoragePartitionUtilsTest, IsContainersStoragePartition) {
+  struct {
+    std::string partition_domain;
+    std::string partition_name;
+    bool expected_result;
+  } const test_cases[] = {
+      {kContainersStoragePartitionDomain, "test-container", true},
+      {kContainersStoragePartitionDomain, "123-test-container", true},
+      {"", "", false},
+      {kContainersStoragePartitionDomain, "", false},
+      {"wrong-domain", "test-container", false},
+      {kContainersStoragePartitionDomain, "test_container", false},
+      {kContainersStoragePartitionDomain, "test container", false},
+      {kContainersStoragePartitionDomain, "test_container-", false},
+      {kContainersStoragePartitionDomain, "-test_container", false},
+      {kContainersStoragePartitionDomain, "123-test_container", false},
+  };
+
+  for (const bool in_memory : {false, true}) {
+    for (const auto& test_case : test_cases) {
+      SCOPED_TRACE(test_case.partition_domain + "+" + test_case.partition_name);
+      auto config = content::CreateStoragePartitionConfigForTesting(
+          in_memory, test_case.partition_domain, test_case.partition_name);
+      EXPECT_EQ(IsContainersStoragePartition(config),
+                test_case.expected_result);
+    }
+  }
+}
+
+TEST_F(ContainersStoragePartitionUtilsTest, IsContainersStoragePartitionKey) {
+  struct {
+    std::string partition_domain;
+    std::string partition_name;
+    bool expected_result;
+  } const test_cases[] = {
+      {kContainersStoragePartitionDomain, "test-container", true},
+      {kContainersStoragePartitionDomain, "123-test-container", true},
+      {"", "", false},
+      {kContainersStoragePartitionDomain, "", false},
+      {"wrong-domain", "test-container", false},
+      {kContainersStoragePartitionDomain, "test_container", false},
+      {kContainersStoragePartitionDomain, "test container", false},
+      {kContainersStoragePartitionDomain, "test_container-", false},
+      {kContainersStoragePartitionDomain, "-test_container", false},
+      {kContainersStoragePartitionDomain, "123-test_container", false},
+  };
+
+  for (const auto& test_case : test_cases) {
+    SCOPED_TRACE(test_case.partition_domain + "+" + test_case.partition_name);
+    EXPECT_EQ(IsContainersStoragePartitionKey(test_case.partition_domain,
+                                              test_case.partition_name),
+              test_case.expected_result);
+  }
+}
+
+TEST_F(ContainersStoragePartitionUtilsTest,
+       IsValidStoragePartitionKeyComponent) {
+  struct {
+    std::string partition_name;
+    bool expected_result;
+  } const test_cases[] = {
+      {"container", true},           {"test-container", true},
+      {"123-test-container", true},  {"", false},
+      {"test_container", false},     {"test container", false},
+      {"test_container-", false},    {"-test_container", false},
+      {"123-test_container", false},
+  };
+
+  for (const auto& test_case : test_cases) {
+    SCOPED_TRACE(test_case.partition_name);
+    EXPECT_EQ(IsValidStoragePartitionKeyComponent(test_case.partition_name),
+              test_case.expected_result);
+  }
+}
+
+TEST_F(ContainersStoragePartitionUtilsTest,
+       MaybeInheritStoragePartition_ValidConfig) {
  auto config = content::CreateStoragePartitionConfigForTesting(
      /*in_memory=*/false,
      /*partition_domain=*/kContainersStoragePartitionDomain,
-      /*partition_name=*/"test_container");
-
-  EXPECT_TRUE(IsContainersStoragePartition(config));
-}
-
-TEST_F(StoragePartitionUtilsTest,
-       IsContainersStoragePartition_ValidInMemoryConfig) {
-  auto config = content::CreateStoragePartitionConfigForTesting(
-      /*in_memory=*/true,
-      /*partition_domain=*/kContainersStoragePartitionDomain,
-      /*partition_name=*/"test_container");
-
-  EXPECT_TRUE(IsContainersStoragePartition(config));
-}
-
-TEST_F(StoragePartitionUtilsTest, IsContainersStoragePartition_WrongDomain) {
-  auto config = content::CreateStoragePartitionConfigForTesting(
-      /*in_memory=*/false,
-      /*partition_domain=*/"wrong_domain",
-      /*partition_name=*/"test_container");
-
-  EXPECT_FALSE(IsContainersStoragePartition(config));
-}
-
-TEST_F(StoragePartitionUtilsTest,
-       IsContainersStoragePartition_EmptyPartitionName) {
-  auto config = content::CreateStoragePartitionConfigForTesting(
-      /*in_memory=*/false,
-      /*partition_domain=*/kContainersStoragePartitionDomain,
-      /*partition_name=*/"");
-
-  EXPECT_FALSE(IsContainersStoragePartition(config));
-}
-
-TEST_F(StoragePartitionUtilsTest, MaybeInheritStoragePartition_ValidConfig) {
-  auto config = content::CreateStoragePartitionConfigForTesting(
-      /*in_memory=*/false,
-      /*partition_domain=*/kContainersStoragePartitionDomain,
-      /*partition_name=*/"test_container");
+      /*partition_name=*/"test-container");

  auto result = MaybeInheritStoragePartition(config);

@@ -75,7 +113,8 @@ TEST_F(StoragePartitionUtilsTest, MaybeInheritStoragePartition_ValidConfig) {
  EXPECT_EQ(result.value(), config);
 }

-TEST_F(StoragePartitionUtilsTest, MaybeInheritStoragePartition_NullOpt) {
+TEST_F(ContainersStoragePartitionUtilsTest,
+       MaybeInheritStoragePartition_NullOpt) {
  base::optional_ref<const content::StoragePartitionConfig> null_ref;

  auto result = MaybeInheritStoragePartition(null_ref);
@@ -83,11 +122,12 @@ TEST_F(StoragePartitionUtilsTest, MaybeInheritStoragePartition_NullOpt) {
  EXPECT_FALSE(result.has_value());
 }

-TEST_F(StoragePartitionUtilsTest, MaybeInheritStoragePartition_InvalidConfig) {
+TEST_F(ContainersStoragePartitionUtilsTest,
+       MaybeInheritStoragePartition_InvalidConfig) {
  auto config = content::CreateStoragePartitionConfigForTesting(
      /*in_memory=*/false,
-      /*partition_domain=*/"wrong_domain",
-      /*partition_name=*/"test_container");
+      /*partition_domain=*/"wrong-domain",
+      /*partition_name=*/"test-container");

  auto result = MaybeInheritStoragePartition(config);