diff --git a/browser/containers/containers_browsertest.cc b/browser/containers/containers_browsertest.cc index 0bbd5e1e431d..e6cf37954925 100644 --- a/browser/containers/containers_browsertest.cc +++ b/browser/containers/containers_browsertest.cc @@ -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(¶ms); + + 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(¶ms); + + 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(¶ms); 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(¶ms); + + 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(¶ms); + + 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 diff --git a/components/BUILD.gn b/components/BUILD.gn index d36c327c846d..bfdb5013a078 100644 --- a/components/BUILD.gn +++ b/components/BUILD.gn @@ -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") diff --git a/components/containers/content/browser/storage_partition_utils.cc b/components/containers/content/browser/storage_partition_utils.cc index 8a3db63d13dc..43c5d725918b 100644 --- a/components/containers/content/browser/storage_partition_utils.cc +++ b/components/containers/content/browser/storage_partition_utils.cc @@ -5,6 +5,9 @@ #include "brave/components/containers/content/browser/storage_partition_utils.h" +#include + +#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 MaybeInheritStoragePartition( @@ -33,6 +49,7 @@ std::optional MaybeInheritStoragePartition( } std::string GetContainerIdForWebContents(content::WebContents* web_contents) { + CHECK(base::FeatureList::IsEnabled(features::kContainers)); if (!web_contents) { return std::string(); } diff --git a/components/containers/content/browser/storage_partition_utils.h b/components/containers/content/browser/storage_partition_utils.h index e16bc5b9ab8e..bd15b6379e7f 100644 --- a/components/containers/content/browser/storage_partition_utils.h +++ b/components/containers/content/browser/storage_partition_utils.h @@ -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. diff --git a/components/containers/content/browser/storage_partition_utils_unittest.cc b/components/containers/content/browser/storage_partition_utils_unittest.cc index 8fce8e3ea299..b6cc0de45476 100644 --- a/components/containers/content/browser/storage_partition_utils_unittest.cc +++ b/components/containers/content/browser/storage_partition_utils_unittest.cc @@ -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) { - 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(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(StoragePartitionUtilsTest, IsContainersStoragePartition_WrongDomain) { - auto config = content::CreateStoragePartitionConfigForTesting( - /*in_memory=*/false, - /*partition_domain=*/"wrong_domain", - /*partition_name=*/"test_container"); - - EXPECT_FALSE(IsContainersStoragePartition(config)); +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(StoragePartitionUtilsTest, - IsContainersStoragePartition_EmptyPartitionName) { - auto config = content::CreateStoragePartitionConfigForTesting( - /*in_memory=*/false, - /*partition_domain=*/kContainersStoragePartitionDomain, - /*partition_name=*/""); - - EXPECT_FALSE(IsContainersStoragePartition(config)); +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(StoragePartitionUtilsTest, MaybeInheritStoragePartition_ValidConfig) { +TEST_F(ContainersStoragePartitionUtilsTest, + 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 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);