This PR introduces the YAML frontend for plaster that is meant to
eventually become the only frontend, once all `.toml` Plaster files are
migrated.
This change covers all places where the assumption about `.toml` files
was being used, inclusing `brockit`, and `git-cr` tools.
Most of the code that should be deleted in the future is well guarded
with comments leading back to the issue tracking, so the TOML parser can
be dropped eventually.
With this change, a dependency to `pyyaml` has been introduced. This
dependency has wheels provided by `vpython`, which is already the
expected python runtime for Plaster.
- Rationale for this change
We have experimented at length with `.toml` files, in order to
understand some of the shortcomings they have that would be addressed
with `.yaml` files.
* `prettier` offers YAML formatting out-of-the-box. On the other hand,
formatters for TOML files are not easy to find, as both `prettier`
and `vpython` have their own challanges with the current options
* YAML's sytanx works better with codeblocks, as it doesn't require
quoting. This makes the content seen less noisy. Looking on some of
the migrated plasters, the YAML substitutions look more readable.
* YAML has better sytanx highlight support in some editors.
Bug: https://github.com/brave/brave-browser/issues/55738
Add best practice: Use *.mojom-forward.h in headers
CS-070: prefer the auto-generated *.mojom-forward.h over the full
*.mojom.h bindings when a header only references mojom types as
pointers, references, or function parameters. Reduces compile times
and transitive dependencies.
Source: PR #35622 review comment by netzenbot.
Adds ARCH-072 to docs/best-practices/architecture.md. A KeyedServiceFactory
may only return nullptr based on session-stable attributes (profile type,
buildflags, base::Feature). Gating creation on user-toggleable prefs forces
restarts and was repeatedly flagged in PR reviews.
* Update best practices from upstream Chromium docs
Add three new rules sourced from upstream Chromium documentation:
- CS-070: Pointer/reference symbol positioning (T* not T *)
- CS-071: No Yoda conditions (foo == 0, not 0 == foo)
- TI-041: Feature flag combination testing with bitmask parameterization
Source URLs checked: Chromium C++ style guide, Chromium C++ testing
best practices, smart pointer guidelines, container guidelines,
componentization cookbook.
* Remove best practices covered by linting and formatting tools
CS-070 (pointer/reference positioning) is enforced by clang-format.
CS-071 (no Yoda conditions) is caught by clang-tidy.
Neither belongs in a human-facing best practices doc.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
* Docs updates
- New test suites doc (how to run tests)
- New WebUI Frontend doc (basic introduction to Brave's different stack)
- Patches points to best practices
- bug fix in claude code skills doc
- Adjust docs/README.md to prioritize the index
- Preflight skill addition of missing test suites
* [Rewards] Make gate3 URL environment-aware via CLI switches
* Address review: network audit list, public_deps, CHECK
- Add gate3 prod and dev endpoints to network audit allowed list
- Move //base to public_deps in :constants (BS-024)
- Replace DCHECK with CHECK in GetGate3URL (CS-025)
* Fix broken tests
* Remove gate3 endpoints from network audit list
* Add gate3 endpoints to TLS pin list
* Replace auto with explicit type
* Refactor GetGate3URL tests into table-driven loop
* Move kEnvGate3Switch to header and use named constants in tests
* Add explicit default for gate3_url in RewardsEngineOptions
* Fix MaybeWarnSwitchValue comment to describe both log paths
* Remove gate3 entries from brave_network_audit_allowed_lists.h
* Simplify GetGate3URL to delegate to GetServicesDomain
* Use std::optional<std::string> for gate3_url instead of empty sentinel
* Rename brave_domains/constants.h/cc to urls.h/cc
* Switch to GURL in GetGate3URL
* Clean up review nits from gate3 env switch
* Add comment in brave_domains/urls.h
ARCH-062 combined two distinct concerns under one heading, causing
the bot to demand CUJ InteractiveBrowserTest for non-UI code like
tab helpers and services. Split into:
- ARCH-062: CUJ tests required only for features with UI changes
- ARCH-062b: Avoid change detector tests (general testing guidance)
Clarify that non-UI components should use unit tests or browser tests
appropriate to the code rather than CUJ tests.
* Add --update-crate flag to update a single vendored tool crate
Adds a --update-crate=<name>@<version> option to tools/crates/update.py
that updates one crate in-place without running cargo vendor. It downloads
the tarball directly from crates.io, replaces the vendor directory, patches
Cargo.lock, and regenerates .cargo-checksum.json. This avoids collateral
version bumps from a full re-vendor and is useful for targeted security
advisory fixes.
Exposes the flag via `npm run update_brave_tools_crates -- --update-crate=`.
* Clarify best practice applicability and reduce upstream hallucinations
- ARCH-020: Clarify rule applies to bypassing access control, not
normal inheritance patterns (calling protected methods from subclasses)
- ARCH-062: Clarify CUJ test requirement applies to UI features, not
backend services or infrastructure
- Review rules: Add explicit instruction to verify upstream code claims
by reading actual files rather than relying on training data
* Update ARCH-020 friend class example to modern #define pattern
Replace outdated BRAVE_CLASS_NAME_H guidance with the current
chromium_src #define approach for adding friend declarations.
Self-contained tab features that don't need external retrieval can use
simple unique_ptr ownership in BraveTabFeatures. The Unowned User Data
pattern should only be used when external code needs to look up the
feature via a static From() method.
Clarify that CSA-053 only applies to temporary workarounds, not
permanent design decisions. Comments explaining why an alternative
API was not used due to a known limitation do not need TODO tracking
issues when the current code is the intended long-term solution.
The wiki content was moved into the docs folder, so the external
link to brave-browser/wiki/Patching-Chromium#making-methods-virtual
now just redirects. Use a local anchor instead.
Clarify that network auditor entries are only needed for
endpoints that do not require user opt-in. Add new best
practice (BS-056) requiring all new Brave endpoints be
added to the HSTS pin list.
* Support tab groups in tree tab strip
Integrate tab groups with the tree tab strip so that groups are represented
as a single tree node wrapping the group (not one tree node per tab). Tabs
inside a group remain direct children of the group collection.
Collection and delegate:
- BraveTreeTabStripCollectionDelegate: When adding a tab with new_group_id,
add to collection without wrapping in a tree node; the group wraps it.
MoveTabsRecursive: handle empty tab_indices (no-op when moving within same
group), and route move-out-of-group and move-into-group to new helpers.
- MoveTabsIntoGroup: unwrap tabs from tree nodes (or detach from other
groups), add to target group; when group is detached (new group), wrap
group in a TreeTabNode and attach at the correct tree position.
- MoveTabsOutOfGroup: move tabs from TabGroupTabCollection back into the
tree by wrapping each in a tree node at the destination index.
- BraveTabStripCollection: add PopDetachedGroupCollectionForDelegate and
GetTreeTabNodeIdForGroup; chromium_src TabStripCollection gains virtual
GetTreeTabNodeIdForGroup. BraveTabStripCollectionDelegate implements
GetTreeTabNodeIdForGroup.
TreeTabNodeTabCollection:
- BuildTreeTabs: wrap entire groups in one tree node (processed_groups set);
grouped tabs stay as direct children of the group.
- New constructor that wraps a TabGroupTabCollection (for creating a group
in tree mode). Single-tab constructor allows null for GetEmptyTreeTabNode
when a tab was moved into a group and the view still holds the old node id.
Model, controller, and UI:
- BraveTabStripModel::GetTreeTabNodeIdForGroup returns the tree node id for
a group (or nullptr if tree tabs off). Wired through controller and
BraveTabStripCollection.
- BraveBrowserTabStripController::GetTreeTabNode returns GetEmptyTreeTabNode()
when the node is null (e.g. tab just moved into group, before
TabGroupedStateChanged/AddTabToGroup updates the view).
- OnTreeTabChanged: handle tab index kNoTab and detached state during group
creation; avoid double-clearing when AddTabToGroup will clear the node id.
- BraveTabStrip::AddTabToGroup override sets the tab's tree_tab_node from
GetTreeTabNodeIdForGroup when adding to a group in tree mode.
Add upstream test failures best practices doc
Consolidates upstream-flake filter guidance from testing-isolation.md
(TI-031, TI-032, TI-041, TI-042, TI-043) and patches.md (PATCH-011)
into a dedicated testing-upstream-failures.md. Expands the flake-check
section with full script usage and the LUCI verdict table.
* Add split tab support for tree tabs
In order to support split tabs in tree tabs, mainly we need to reimplment
two methods - CreateSplit and Unsplit.
For CreateSplit, we need to wrap the split tabs in a tree node, and insert it
at the position.
For Unsplit, we need to extract the tabs back to the tree nodes.
- TabStripCollection: virtual CreateSplit, Unsplit, AddCollectionMapping,
RemoveCollectionMapping; AddTabCollectionAtPosition for delegate.
- BraveTabStripCollection(delegate): override CreateSplit/Unsplit and
AddCollectionMapping/RemoveCollectionMapping; PassKey overloads for
AddTabCollectionAtPosition and collection mapping.
- BraveTreeTabStripCollectionDelegate: implement CreateSplit (wrap split in
tree node, insert at position), Unsplit (extract tabs back to tree nodes),
AddCollectionMapping/RemoveCollectionMapping for TREE_NODE holding split/group;
MoveTabsRecursive same-position handling for split/group creation.
- Tab strip API: handle TREE_NODE in tab_converters (unpinned container).
- Docs: tree tabs + split tabs flow and MoveTabsRecursive behavior.
* Support contained tabs session restore and sync.
* Review fixes.
* Use struct as a return value instead of optional with out args.
* Add a unit test for view-source: scheme.
* Move TabRestore helper to components.
* Add containers-specific SerializedNavigationBuilder* tests.
* Guard PageState.top.url_string use.
* Add plaster files for few patches.
* Replace browser_tabrestore.cc #define patch with plaster rewrite.
* Replace content_serialized_navigation_builder.cc change with rewrite.
* Fix gn check.
* Add prefix_length check.
* Improve rewrite rules.
* Guard PageState changes with buildflag.
* Rename StoragePartitionKeyToUrlPrefix.
* Add buildflag check into SerializedNavigationEntry.
* Fix build issues with containers buildflag disabled.
* Fix iOS build.
cdesouza-chromium