Add Containers Settings section.

[goodov]

This PR adds basic Containers settings UI to be able to add and remove containers into prefs. The PR intentionally does not add color and icon UI for containers to keep things simple and land more features iteratively.

Resolves brave/brave-browser#46351

r3IyNIIKt2.mp4

[brave-builds]

A Storybook has been deployed to preview UI for the latest push

[goodov]

PR is ready for review. PTAL.

[bridiver]

nothing happens here? Should this be NOTREACHED or something for now with a TODO issue link?

[bridiver]

maybe these should all be left as empty for now and split this into two PRs where this one just has a stub implementation on the cpp side? This feels like it's getting kind of big and while the PR does say it handles add and remove only, it's a bit confusing that modify is left partially implemented. I think it would be better to just move all of the implementations to a separate PR(s)?

[goodov]

maybe these should all be left as empty for now and split this into two PRs where this one just has a stub implementation on the cpp side? This feels like it's getting kind of big and while the PR does say it handles add and remove only, it's a bit confusing that modify is left partially implemented. I think it would be better to just move all of the implementations to a separate PR(s)?

Update is implemented correctly. If a container is not found then it should not be updated/added, nor an error should be displayed. This is an edge case that should just not crash and not modify anything in case a container is removed at the same time via sync.

We can't allow Update call to add a new container, because id should always be generated on the C++ side.

[bridiver]

Shouldn't this be something like ContainersObserver?

[petemill]

If it's behaving purely like an observer then perhaps and sure, why not. We usually call it SettingsUI because it's not necessarily or always a one-to-many blind observer pattern, sometimes the browser wants to do an explicit action on a single UI client, but it's rare.

[goodov]

Team, I just followed an existing approach in all Chromium WebUIs. They usually use Page/PageHandler.

upd: renamed to ContainersSettingsHandler, ContainersSettingsObserver.

[petemill]

We probably don't need this since the main interface doesn't need any parameters relevant to the page to be created

[goodov]

I did this exactly how webui_explainer.md recommends to wire both ends in a single mojo call. Should I go against the recommended approach?

I agree a BraveSettingsUI class pollution is a bit of a downside here, but this is how Chromium currently prefers this webui/c++/mojo integration to work.

[github-actions]

[puLL-Merge] - brave/brave-core@29230

Description

This PR adds a new "Containers" feature to Brave browser that allows users to organize their browsing with separate containers. Each container maintains isolated cookies, storage, and site data while sharing the same browser profile. The implementation includes:

• Core browser components for container management (preferences, settings handler)
• Settings UI integration with dialogs for creating, editing, and deleting containers
• Mojo interfaces for browser-renderer communication
• Preference synchronization support
• Build system integration with feature flags

Possible Issues

1. Resource Management: The RemoveContainer operation includes a hardcoded 5-second delay in OnContainerDataRemoved() which appears to be a temporary workaround for simulating async data removal. This could lead to poor user experience or potential race conditions.

2. Error Handling: Limited error handling in the settings page handler - operations like container creation/update don't validate for duplicate names or handle edge cases gracefully.

3. UI State Consistency: The containers list updates through preference changes, but there's potential for UI inconsistencies if multiple settings windows are open simultaneously during container operations.

4. Build Dependencies: The feature adds multiple new build dependencies and cross-component references which could increase build complexity and coupling.

Security Hotspots

1. Container Data Isolation: The current implementation has a placeholder NoOpDelegate::RemoveContainerData() that doesn't actually remove container data. This could lead to data leakage between containers if not properly implemented.

2. UUID Generation: Container IDs are generated using base::Uuid::GenerateRandomV4() which should be cryptographically secure, but the implementation should ensure these IDs aren't predictable or enumerable.

3. Preference Synchronization: Container data is marked as syncable in preferences, which could potentially sync sensitive container configurations across devices without proper encryption or user consent verification.

Privacy Hotspots

1. Data Synchronization: The kContainersList preference is marked as SYNCABLE_PREF, meaning container names and configurations will be synchronized across user devices. This could expose user's organizational patterns and browsing habits.

2. Container Metadata: Container names chosen by users might contain personally identifiable information that gets stored in preferences and potentially synced.

3. Learn More Link: The containers description includes a link to GitHub documentation (kContainersLearnMoreURL) which could be used for tracking user engagement with the feature.

Changes

Changes

app/BUILD.gn & app/brave_settings_strings.grdp

• Added containers feature flag and localized strings for the containers UI

browser/DEPS & browser/brave_content_browser_client.cc

• Added dependencies and Mojo interface bindings for containers feature

browser/brave_profile_prefs.cc

• Registered container preferences for profile-level storage

browser/resources/settings/

• Added complete settings UI implementation with TypeScript components for container management
• Includes CSS styling, HTML templates, and browser proxy for Mojo communication
• Integrated containers section into main settings navigation

browser/ui/webui/brave_settings_ui.cc & .h

• Implemented Mojo interface bindings and settings page handler factory

components/containers/core/

• Created complete browser-side implementation including preferences, settings handler, and Mojo interfaces
• Added comprehensive unit tests for core functionality

chromium_src/components/sync_preferences/

• Extended Chromium's syncable preferences system to include Brave containers data

sequenceDiagram
    participant UI as Settings UI
    participant Proxy as Browser Proxy  
    participant Handler as Settings Handler
    participant Prefs as Preferences
    participant Delegate as Container Delegate

    UI->>Proxy: User adds container
    Proxy->>Handler: AddOrUpdateContainer(container)
    Handler->>Handler: Generate UUID for new container
    Handler->>Prefs: SetContainersToPrefs(containers)
    Prefs->>Handler: OnContainersChanged()
    Handler->>UI: OnContainersChanged(containers)

    UI->>Proxy: User deletes container  
    Proxy->>Handler: RemoveContainer(id)
    Handler->>Delegate: RemoveContainerData(id, callback)
    Delegate->>Handler: OnContainerDataRemoved(id, callback)
    Handler->>Prefs: SetContainersToPrefs(filtered containers)
    Prefs->>Handler: OnContainersChanged()
    Handler->>UI: OnContainersChanged(containers)
    Handler->>UI: callback() [after 5s delay]

Loading

[brave-builds]

Warning

You have got a presubmit warning. Please address it if possible.

Found 4 lines longer than 80 characters (first 5 shown).

Items:

browser/resources/settings/brave_content_page/containers.ts, line 17, 94 chars
browser/resources/settings/brave_content_page/containers.ts, line 19, 102 chars
browser/resources/settings/brave_content_page/containers_browser_proxy.ts, line 30, 83 chars
browser/resources/settings/brave_overrides/basic_page.ts, line 354, 86 chars