brave-core/docs/best-practices/coding-standards-apis.md

C++ API Usage, Containers & Type Safety

✅ Use Existing Utilities Instead of Custom Code

Always check for existing well-tested utilities before writing custom code. Chromium and base have extensive libraries for common operations.

// ❌ WRONG - custom query string parsing
std::string ParseQueryParam(const std::string& url, const std::string& key) {
  // custom parsing code...
}

// ✅ CORRECT - use existing utility
net::QueryIterator it(url);
while (!it.IsAtEnd()) {
  if (it.GetKey() == key) return it.GetValue();
  it.Advance();
}

---

✅ Use base::OnceCallback and base::BindOnce

base::Callback and base::Bind are deprecated. Use base::OnceCallback/base::RepeatingCallback and base::BindOnce/base::BindRepeating. Use std::move when passing or calling a base::OnceCallback.

---

✅ Never Use std::time - Use base::Time

Always use base::Time and related classes instead of C-style std::time, ctime, or time_t. The base library provides cross-platform, type-safe time utilities.

---

✅ Use JSONValueConverter for JSON/Type Conversion

When parsing JSON into C++ types, prefer base::JSONValueConverter over manual key-by-key parsing. Manual parsing is verbose, error-prone, and results in duplicated boilerplate.

// ❌ WRONG - manual JSON parsing
const auto* name = dict->FindStringKey("name");
const auto age = dict->FindIntKey("age");
if (name) result.name = *name;
if (age) result.age = *age;

// ✅ CORRECT - use JSONValueConverter
static void RegisterJSONConverter(
    base::JSONValueConverter<MyType>* converter) {
  converter->RegisterStringField("name", &MyType::name);
  converter->RegisterIntField("age", &MyType::age);
}

---

✅ Use the Right Associative Container

Chromium's container guidelines (base/containers/README.md) recommend specific containers for each use case. std::unordered_map/std::unordered_set are banned.

Default (unordered): Use absl::flat_hash_map and absl::flat_hash_set for general-purpose needs. They provide the best all-around performance for both small and large datasets.

Sorted, write-once or small: Use base::flat_map/base::flat_set. Good cache locality; O(n) mutations are fine for small collections or write-once containers.

Sorted, large, frequently-mutated: Use std::map/std::set. O(log n) mutations matter at scale.

Compile-time lookup tables: Use base::MakeFixedFlatMap/base::MakeFixedFlatSet (see CSA-045).

// ❌ WRONG - banned
std::unordered_map<std::string, double> feature_map_;

// ✅ CORRECT - default unordered container
absl::flat_hash_map<std::string, double> feature_map_;

// ✅ CORRECT - sorted, small/write-once
base::flat_map<std::string, int> lookup_;

// ✅ CORRECT - sorted, large, frequently mutated
std::map<std::string, int> large_mutable_lookup_;

Pointer stability: If you need stable pointers to values, wrap them in std::unique_ptr inside an absl::flat_hash_map. If you need stable pointers to keys, use absl::node_hash_map/absl::node_hash_set (separate node allocation).

// ✅ CORRECT - value pointer stability via unique_ptr wrapping
absl::flat_hash_map<Key, std::unique_ptr<Value>> stable_values;

// ✅ CORRECT - key pointer stability via node hash
absl::node_hash_map<std::string, int> stable_keys;

Also banned: absl::btree_map/absl::btree_set (significant code size penalties in Chromium). See Chromium container guidelines.

---

❌ Don't Use Deprecated GetAs* Methods on base::Value

The GetAsString(), GetAsInteger(), etc. methods on base::Value are deprecated. Use the newer direct access methods like GetString(), GetInt(), GetDouble().

// ❌ WRONG
std::string str;
value->GetAsString(&str);

// ✅ CORRECT
const std::string& str = value->GetString();

---

✅ Use GetIfBool/GetIfInt/GetIfString for Safe base::Value Access

When extracting values from a base::Value where the type may not match, use GetIf* accessors instead of Get* which CHECK-fails on type mismatch.

// ❌ WRONG - crashes if value is not a bool
if (value.GetBool()) { ... }

// ✅ CORRECT - safe accessor with value_or
if (value.GetIfBool().value_or(false)) { ... }

---

❌ Don't Use std::to_string - Use base::NumberToString

std::to_string is on Chromium's deprecated list. Use base::NumberToString instead.

// ❌ WRONG
std::string port_str = std::to_string(port);

// ✅ CORRECT
std::string port_str = base::NumberToString(port);

---

❌ Don't Use C-Style Casts

Chromium prohibits C-style casts. Use C++ casts (static_cast, reinterpret_cast, etc.) which are safer and more explicit.

// ❌ WRONG
double result = (double)integer_value / total;

// ✅ CORRECT
double result = static_cast<double>(integer_value) / total;

---

✅ Prefer std::move Over Clone

Use std::move instead of cloning when you don't need the original value anymore. This avoids unnecessary copies. This is especially important when passing std::vector or other large objects to callback .Run() calls — forgetting std::move silently copies the entire buffer.

// ❌ WRONG - copies the entire vector into the callback
std::vector<unsigned char> buffer = BuildData();
std::move(cb).Run(buffer, other_arg);

// ✅ CORRECT - moves the vector, no copy
std::vector<unsigned char> buffer = BuildData();
std::move(cb).Run(std::move(buffer), other_arg);

---

❌ Don't Create Unnecessary Wrapper Types

Don't create plural/container types when you can use arrays of the singular type. Extra wrapper types add complexity without value.

// ❌ WRONG - unnecessary plural type
struct MonthlyStatements {
  std::vector<MonthlyStatement> statements;
};

// ✅ CORRECT - just use the vector directly
std::vector<MonthlyStatement> GetMonthlyStatements();

---

✅ Use Pref Dict/List Values Directly

Don't serialize to JSON strings when storing structured data in prefs. Use SetDict/SetList directly instead of JSONWriter::Write + SetString.

// ❌ WRONG - serializing to JSON string unnecessarily
std::string result;
base::JSONWriter::Write(root, &result);
prefs->SetString(prefs::kMyPref, result);

// ✅ CORRECT - use native pref value types
prefs->SetDict(prefs::kMyPref, std::move(dict_value));
prefs->SetList(prefs::kMyPref, std::move(list_value));

---

✅ Use extern const char[] Over #define for Strings

Use extern const char[] instead of #define for string constants to keep them namespaced.

// ❌ WRONG - pollutes preprocessor namespace
#define MY_URL "https://example.com"

// ✅ CORRECT - properly namespaced
extern const char kMyUrl[];
// In .cc:
const char kMyUrl[] = "https://example.com";

Exception: use #define when you need to pass the value in from GN.

---

✅ Prefer Enum Types Over String Constants for Typed Values

When a value has a fixed set of valid options, use an enum with string conversion rather than passing raw strings. This enables compiler-checked switch statements and prevents invalid values.

// ❌ WRONG - raw strings
void SetWalletType(const std::string& type);

// ✅ CORRECT - enum with conversion
enum class WalletType { kUphold, kGemini };
void SetWalletType(WalletType type);

---

❌ No C++ Exceptions in Third-Party Libraries

C++ exceptions are disallowed in Chromium. When integrating third-party libraries, verify they build with exception support disabled.

---

✅ Use base::EraseIf / std::erase_if Instead of Manual Erase Loops

Prefer base::EraseIf (for base::flat_* containers) or std::erase_if (for standard containers) over manual iterator-based erase loops. Cleaner and less error-prone.

// ❌ WRONG - manual erase loop
for (auto it = items.begin(); it != items.end();) {
  if (it->IsExpired()) {
    it = items.erase(it);
  } else {
    ++it;
  }
}

// ✅ CORRECT
base::EraseIf(items, [](const auto& item) { return item.IsExpired(); });
// or for std containers:
std::erase_if(items, [](const auto& item) { return item.IsExpired(); });

---

✅ Use base::span at API Boundaries Instead of const std::vector&

Prefer base::span<const T> over const std::vector<T>& for function parameters that only read data. Spans are lightweight, non-owning views that accept any contiguous container (std::vector, base::HeapArray, C arrays, base::FixedArray), making APIs more flexible.

// ❌ WRONG - forces callers to use std::vector
void ProcessBuffer(const std::vector<uint8_t>& data);

// ✅ CORRECT - accepts any contiguous container
void ProcessBuffer(base::span<const uint8_t> data);

This is especially important for byte buffer APIs where the data source may be a std::vector, base::HeapArray, or a static array.

---

✅ Use base::FixedArray Over std::vector for Known-Size Runtime Allocations

When the size is known at creation but not at compile time, use base::FixedArray. It avoids heap allocation for small sizes and communicates immutable size.

// ❌ WRONG - vector suggests dynamic resizing
std::vector<uint8_t> out(size);

// ✅ CORRECT - size is fixed after construction
base::FixedArray<uint8_t> out(size);

---

✅ Use base::HeapArray<uint8_t> for Fixed-Size Byte Buffers

When you need an owned byte buffer that won't be resized after creation, use base::HeapArray<uint8_t> instead of std::vector<unsigned char> or std::vector<uint8_t>. HeapArray communicates that the size is fixed, provides bounds-checked indexing, and converts easily to base::span.

// ❌ WRONG - vector implies the buffer may grow
std::vector<unsigned char> dat_buffer(size);
ProcessBuffer(dat_buffer.data(), dat_buffer.size());

// ✅ CORRECT - HeapArray communicates fixed-size semantics
auto dat_buffer = base::HeapArray<uint8_t>::WithSize(size);
ProcessBuffer(dat_buffer.as_span());

Use HeapArray::Uninit(size) for performance-sensitive paths where zero-initialization is unnecessary.

Note: When interfaces (e.g., Mojo, Rust FFI) require std::vector, you may need to keep using std::vector at those boundaries, but prefer HeapArray for internal buffer management.

---

✅ Use base::ToVector for Range-to-Vector Conversions

Use base::ToVector(range) instead of manual copy patterns when converting a range to a std::vector. It handles reserve() and iteration automatically, and supports projections.

// ❌ WRONG - manual reserve + copy + back_inserter
std::vector<unsigned char> buffer;
buffer.reserve(sizeof(kStaticData) - 1);
std::copy_n(kStaticData, sizeof(kStaticData) - 1,
            std::back_inserter(buffer));

// ✅ CORRECT - base::ToVector
auto buffer = base::ToVector(base::span(kStaticData).first<sizeof(kStaticData) - 1>());

// ✅ CORRECT - with projection
auto names = base::ToVector(items, &Item::name);

---

✅ Prefer Contiguous Containers Over Linked Lists

Never use std::list for pure traversal — poor cache locality. Use std::list only when stable iterators or frequent mid-container insert/remove is required. Prefer std::vector with reserve() for known sizes.

---

✅ Use std::optional Instead of Sentinel Values

Never use empty string "", -1, or other magic values as sentinels for "no value". Use std::optional<T>.

// ❌ WRONG - "" as sentinel for "no custom title"
void SetCustomTitle(const std::string& title);  // "" means "unset"

// ✅ CORRECT - explicit optionality
void SetCustomTitle(std::optional<std::string> title);  // nullopt means "unset"

---

✅ Use .emplace() for std::optional Initialization Clarity

When engaging a std::optional member, prefer .emplace() for clarity about the intent.

// Less clear
elapsed_timer_ = base::ElapsedTimer();

// ✅ CORRECT - explicit engagement intent
elapsed_timer_.emplace();

---

✅ Return std::optional Instead of bool + Out Parameter

When a function needs to return a value that may or may not exist, use std::optional<T> instead of returning bool with an out parameter.

// ❌ WRONG
bool GetHistorySize(int* out_size);

// ✅ CORRECT
std::optional<int> GetHistorySize();

---

✅ Use constexpr for Compile-Time Constants

Constants defined in anonymous namespaces should use constexpr instead of const when the value is known at compile time. Place constants inside the component's namespace.

// ❌ WRONG
namespace {
const int kMaxRetries = 3;
}

// ✅ CORRECT
namespace brave_stats {
namespace {
constexpr int kMaxRetries = 3;
}  // namespace
}  // namespace brave_stats

---

✅ Use Raw String Literals for Multiline Strings

When embedding multiline strings (JavaScript, SQL, etc.), use raw string literals (R"()") instead of escaping each line.

// ❌ WRONG
const char kScript[] =
    "(function() {\n"
    "  let x = 1;\n"
    "})();";

// ✅ CORRECT
const char kScript[] = R"(
  (function() {
    let x = 1;
  })();
)";

---

❌ Don't Pass Primitive Types by const Reference

Primitive types (int, bool, float, pointers) should be passed by value, not by const reference. Passing by reference adds unnecessary indirection.

// ❌ WRONG
void ProcessItem(const int& id, const bool& enabled);

// ✅ CORRECT
void ProcessItem(int id, bool enabled);

---

❌ Don't Add DISALLOW_COPY_AND_ASSIGN in New Code

The DISALLOW_COPY_AND_ASSIGN macro is deprecated. Explicitly delete the copy constructor and copy assignment operator instead.

// ❌ WRONG
class MyClass {
 private:
  DISALLOW_COPY_AND_ASSIGN(MyClass);
};

// ✅ CORRECT
class MyClass {
 public:
  MyClass(const MyClass&) = delete;
  MyClass& operator=(const MyClass&) = delete;
};

---

✅ Declare Move Operations as noexcept

When defining custom move constructors/assignment operators for structs used in std::vector, declare them noexcept. Without noexcept, std::vector falls back to copying during reallocations.

// ❌ WRONG
Topic(Topic&&) = default;

// ✅ CORRECT
Topic(Topic&&) noexcept = default;
Topic& operator=(Topic&&) noexcept = default;

---

❌ Avoid std::optional<T>& References

Never pass std::optional<T>& as a function parameter. It's confusing and can cause hidden copies. Take by value if storing, or use base::optional_ref<T> for non-owning optional references.

// ❌ WRONG - confusing, hidden copies
void Process(const std::optional<std::string>& value);

// ✅ CORRECT - take by value if storing
void Process(std::optional<std::string> value);

// ✅ CORRECT - use base::optional_ref for non-owning optional references
void Process(base::optional_ref<const std::string> value);

---

✅ Short-Circuit on Non-HTTP(S) URLs

In URL processing code (shields, debouncing, content settings), add an early return for non-HTTP/HTTPS URLs. This prevents wasting time on irrelevant schemes and avoids edge cases.

// ✅ CORRECT - early exit
bool ShouldDebounce(const GURL& url) {
  if (!url.SchemeIsHTTPOrHTTPS())
    return false;
  // ...
}

---

❌ Don't Narrow Integer Types in Setters or Parameters

Setter and function parameter types must match the underlying field type. Accepting a narrower type (e.g., uint32_t when the field is uint64_t) silently truncates values. This is especially dangerous in security-sensitive code like wallet/crypto transactions.

// ❌ WRONG - parameter narrower than field, silent truncation
class Transaction {
  uint64_t invalid_after_ = 0;
  void set_invalid_after(uint32_t value) { invalid_after_ = value; }
};

// ✅ CORRECT - types match
class Transaction {
  uint64_t invalid_after_ = 0;
  void set_invalid_after(uint64_t value) { invalid_after_ = value; }
};

---

✅ Deprecate Prefs Before Removing Them

When removing a preference that was previously stored in user profiles, first deprecate the pref (register it for clearing) in one release before fully removing it. This ensures the old value is cleared from existing profiles.

---

❌ Don't Modify Production Code Solely to Accommodate Tests

Test-specific workarounds should not affect production behavior. Use test infrastructure like kHostResolverRules command line switches in SetUpCommandLine instead of adding production code paths only needed for tests.

Only flag this rule when you are certain the code exists solely for tests. Clear signals include CHECK_IS_TEST(), #if defined(UNIT_TEST), _for_testing suffixes, or comments explicitly mentioning test support. Do NOT flag legitimate production logic such as handling empty/null/default values, reset paths, or cleanup behavior — these are normal defensive coding patterns, not test accommodations. When uncertain, do not flag.

Exception: Thin ForTesting() accessors that expose internalized features (e.g., base::Feature) are acceptable. These keep the feature internalized while providing a clean way for tests to reference it, and do not affect production behavior.

---

✅ Use url::kStandardSchemeSeparator Instead of Hardcoded "://"

When constructing URLs, use url::kStandardSchemeSeparator instead of the hardcoded string "://". This is more maintainable and consistent with Chromium conventions.

// ❌ WRONG
std::string url = scheme + "://" + host + path;

// ✅ CORRECT
std::string url = base::StrCat({url::kHttpsScheme,
                                url::kStandardSchemeSeparator,
                                host, path});

---

✅ Use base::DoNothing() for No-Op Callbacks

Use base::DoNothing() instead of empty lambdas when a no-op callback is needed. It is the Chromium-idiomatic way and is more readable.

// ❌ WRONG - empty lambda
service->DoAsync([](const std::string&) {});

// ✅ CORRECT
service->DoAsync(base::DoNothing());

---

✅ Use base::StrAppend Over += base::StrCat

When appending to an existing string, use base::StrAppend(&str, {...}) instead of str += base::StrCat({...}). StrCat creates a temporary string that is then copied; StrAppend appends directly to the target, avoiding unnecessary allocation.

// ❌ WRONG - temporary string then copy
result += base::StrCat({kOpenTag, "\n", "=== METADATA ===\n"});

// ✅ CORRECT - append directly
base::StrAppend(&result, {kOpenTag, "\n", "=== METADATA ===\n"});

---

✅ Use base::Reversed() for Reverse Iteration

Prefer base::Reversed() with range-based for loops over explicit reverse iterators. Always add a comment explaining why reverse order is needed.

// ❌ WRONG - explicit reverse iterators
for (auto it = history.crbegin(); it != history.crend(); ++it) {
  ProcessEntry(*it);
}

// ✅ CORRECT - base::Reversed with comment
// Process newest entries first to prioritize recent content.
for (const auto& entry : base::Reversed(history)) {
  ProcessEntry(entry);
}

---

✅ Use absl::StrFormat Over base::StringPrintf

Prefer absl::StrFormat for formatted string construction. base::StringPrintf is being deprecated in favor of absl::StrFormat.

// ❌ WRONG - deprecated
std::string msg = base::StringPrintf("Error %d: %s", code, desc.c_str());

// ✅ CORRECT
std::string msg = absl::StrFormat("Error %d: %s", code, desc);

---

✅ Use base::saturated_cast for Safe Numeric Conversions

When converting between integer types, use base::saturated_cast<TargetType>() combined with .value_or(default) for safe, concise conversion of optional numeric values.

// ❌ WRONG - manual null-check and static_cast
if (value.has_value()) {
  result = static_cast<uint64_t>(*value);
}

// ✅ CORRECT - safe saturated cast with value_or
result = base::saturated_cast<uint64_t>(value.value_or(0));

---

✅ Use std::ranges Algorithms Over Manual Loops

Prefer C++20 std::ranges::any_of, std::ranges::all_of, std::ranges::find_if over manual for-loops with break conditions. The ranges versions are more concise and readable.

// ❌ WRONG - manual loop
bool found = false;
for (const auto& item : items) {
  if (item.IsExpired()) {
    found = true;
    break;
  }
}

// ✅ CORRECT - ranges algorithm
bool found = std::ranges::any_of(items,
    [](const auto& item) { return item.IsExpired(); });

---

✅ Guard substr() with Size Check

Only call substr() when the content actually exceeds the limit. For content within the limit, use the original string to avoid unnecessary memory allocation and copying.

// ❌ WRONG - always creates a substring
std::string truncated = content.substr(0, max_length);

// ✅ CORRECT - only substr when needed
const std::string& truncated = (content.size() > max_length)
    ? content.substr(0, max_length)
    : content;

---

✅ Use base::expected<T, E> Over Optional + Error Out-Parameter

When a function can fail and needs to communicate error details, use base::expected<T, E> instead of std::optional<T> with a separate error out-parameter. This bundles success and error into a single return value.

// ❌ WRONG - separate error out-parameter
std::optional<Result> Parse(const std::string& input, std::string* error);

// ✅ CORRECT - base::expected bundles both
base::expected<Result, std::string> Parse(const std::string& input);

---

✅ Use base::MakeFixedFlatMap for Static Enum-to-String Mappings

For compile-time constant mappings between enums and strings, use base::MakeFixedFlatMap. It provides compile-time verification and is more maintainable than switch statements or runtime-built maps.

// ❌ WRONG - runtime map
const std::map<ActionType, std::string> kActionNames = {
    {ActionType::kSummarize, "summarize"},
    {ActionType::kRewrite, "rewrite"},
};

// ✅ CORRECT - compile-time fixed flat map
constexpr auto kActionNames = base::MakeFixedFlatMap<ActionType, std::string_view>({
    {ActionType::kSummarize, "summarize"},
    {ActionType::kRewrite, "rewrite"},
});

---

✅ Use base::JSONReader::ReadDict for JSON Dictionary Parsing

When parsing a JSON string expected to be a dictionary, use base::JSONReader::ReadDict() which returns std::optional<base::Value::Dict> directly, instead of base::JSONReader::Read() followed by manual GetIfDict() extraction.

// ❌ WRONG - manual extraction
auto value = base::JSONReader::Read(json_str);
if (!value || !value->is_dict()) return;
auto& dict = value->GetDict();

// ✅ CORRECT - direct dict parsing
auto dict = base::JSONReader::ReadDict(json_str);
if (!dict) return;

---

✅ Pass-by-Value for Sink Parameters (Google Style)

Per Google C++ Style Guide, use pass-by-value for parameters that will be moved into the callee (sink parameters) instead of T&&. The caller uses std::move() either way, and pass-by-value is simpler.

// ❌ WRONG - rvalue reference parameter
void SetName(std::string&& name) { name_ = std::move(name); }

// ✅ CORRECT - pass by value
void SetName(std::string name) { name_ = std::move(name); }

---

✅ Annotate Obsolete Pref Migration Entries with Dates

When adding preference migration code that removes deprecated prefs, annotate the entry with the date it was added. This makes it easy to identify and clean up old migration code later.

// ❌ WRONG - no context for when this was added
profile_prefs->ClearPref(kOldFeaturePref);

// ✅ CORRECT - annotated with date
profile_prefs->ClearPref(kOldFeaturePref);  // Added 2025-01 (safe to remove after ~3 releases)

---

✅ Use base::FindOrNull() for Map Lookups

Use base::FindOrNull() instead of the manual find-and-check-end pattern for map lookups. It's more concise and less error-prone.

// ❌ WRONG - verbose find + check
auto it = metric_configs_.find(metric_name);
if (it == metric_configs_.end()) {
  return nullptr;
}
return &it->second;

// ✅ CORRECT
return base::FindOrNull(metric_configs_, metric_name);

---

✅ Use base::Extend for Appending Ranges to Vectors

Use base::Extend(target, source) instead of manual insert(end, begin, end) for appending one collection to another.

// ❌ WRONG - verbose
accelerator_list.insert(accelerator_list.end(),
    brave_accelerators.begin(), brave_accelerators.end());

// ✅ CORRECT
base::Extend(accelerator_list, base::span(kBraveAcceleratorMap));

---

✅ Use base::test::ParseJson and base::ExpectDict* in Tests

Use base::test::ParseJson() for parsing JSON in tests, and base::test::* utilities from base/test/values_test_util.h for asserting dict contents. These are more readable and produce better error messages than manual JSON parsing.

// ❌ WRONG - manual JSON parsing in tests
auto value = base::JSONReader::Read(json_str);
ASSERT_TRUE(value);
ASSERT_TRUE(value->is_dict());
auto* name = value->GetDict().FindString("name");
ASSERT_TRUE(name);
EXPECT_EQ(*name, "test");

// ✅ CORRECT - test utilities
auto dict = base::test::ParseJsonDict(json_str);
EXPECT_THAT(dict, base::test::DictHasValue("name", "test"));

---

✅ Use kOsAll for Cross-Platform Feature Flags

When registering feature flags in about_flags.cc that should be available on all platforms, use kOsAll instead of listing individual platform constants.

// ❌ WRONG - listing platforms individually
{"brave-my-feature", ..., kOsDesktop | kOsAndroid}

// ✅ CORRECT - use kOsAll
{"brave-my-feature", ..., kOsAll}

---

✅ Workaround Code Must Have Tracking Issues

Any temporary workaround or hack code must reference a tracking issue with a TODO(https://github.com/brave/brave-browser/issues/<id>) comment explaining when and why it can be removed. Workarounds without tracking issues become permanent technical debt.

This rule does NOT apply to permanent design decisions. If a comment explains why an alternative API or approach was not used due to a known limitation, and the current code is the intended long-term solution (not something to revisit later), it is not a workaround -- it is a design rationale comment and does not need a tracking issue.

// ❌ WRONG - unexplained workaround
// HACK: skip validation for now
if (ShouldSkipValidation()) return;

// ✅ CORRECT - tracked workaround with TODO
// TODO(https://github.com/brave/brave-browser/issues/12345): Remove
// this workaround once upstream fixes the validation race condition.
if (ShouldSkipValidation()) return;

// ✅ ALSO CORRECT - permanent design decision, no TODO needed
// FooApi::Connect() relies on the receiver sending Ack messages,
// but the JS bindings don't implement this protocol. Use an explicit
// timer instead.
base::OneShotTimer idle_timer_;

---

✅ Use Named Constants for JSON Property Keys

When accessing JSON object properties in C++, define named constants for the key strings rather than using inline string literals. This prevents typos and makes refactoring easier.

// ❌ WRONG - inline string literals
auto* name = dict.FindString("display_name");
auto* url = dict.FindString("endpoint_url");

// ✅ CORRECT - named constants
constexpr char kDisplayName[] = "display_name";
constexpr char kEndpointUrl[] = "endpoint_url";
auto* name = dict.FindString(kDisplayName);
auto* url = dict.FindString(kEndpointUrl);

---

❌ Never Return std::string_view from Functions That Build Strings

Do not return std::string_view from a function that constructs or concatenates a string internally. The view would point into a temporary string's buffer and become a dangling reference after the function returns. Return std::string or std::optional<std::string> instead.

// ❌ WRONG - dangling reference to temporary
std::string_view BuildUrl(std::string_view host) {
  std::string url = base::StrCat({"https://", host, "/api"});
  return url;  // url destroyed, view dangles!
}

// ✅ CORRECT - return by value
std::string BuildUrl(std::string_view host) {
  return base::StrCat({"https://", host, "/api"});
}

---

✅ Prefer constexpr int Over Single-Value Enums

When a constant is just a single numeric value, use constexpr int rather than creating a single-value enum. Enums are for sets of related values.

// ❌ WRONG - enum for a single value
enum { kBravePolicySource = 10 };

// ✅ CORRECT - constexpr int
constexpr int kBravePolicySource = 10;

---

✅ Use base::FilePath for File Path Parameters

Parameters representing file system paths should use base::FilePath instead of std::string. This provides type safety, simplifies call sites, and makes APIs self-documenting.

// ❌ WRONG - generic string for a path
std::string GetProfileId(const std::string& profile_path);

// ✅ CORRECT - domain-specific type
std::string GetProfileId(const base::FilePath& profile_path);

---

✅ Explicitly Assign Enum Values When Conditionally Compiling Out Members

When conditionally compiling out enum values behind a build flag, explicitly assign numeric values to remaining members. This prevents value shifts that break serialization, persistence, or IPC.

// ❌ WRONG - values shift when kTalk is compiled out
enum class SidebarItem {
  kBookmarks,
#if BUILDFLAG(ENABLE_BRAVE_TALK)
  kTalk,
#endif
  kHistory,  // value changes depending on build flag!
};

// ✅ CORRECT - explicit values prevent shifts
enum class SidebarItem {
  kBookmarks = 0,
#if BUILDFLAG(ENABLE_BRAVE_TALK)
  kTalk = 1,
#endif
  kHistory = 2,
};

---

✅ Name All Function Parameters in Header Declarations

Always name function parameters in header declarations, especially when types alone are ambiguous. Match the parameter names used in the .cc file.

// ❌ WRONG - ambiguous parameters
void OnSubmitSignedExtrinsic(std::optional<std::string>,
                             std::optional<std::string>);

// ✅ CORRECT - named parameters
void OnSubmitSignedExtrinsic(std::optional<std::string> transaction_hash,
                             std::optional<std::string> error_str);

---

✅ Struct Members: No Trailing Underscores

Plain struct members should not have trailing underscores. The trailing underscore convention is for class member variables, not struct fields.

// ❌ WRONG
struct ContentSite {
  std::string name_;
  int percentage_;
};

// ✅ CORRECT
struct ContentSite {
  std::string name;
  int percentage;
};

---

---

❌ Don't Introduce New Uses of Deprecated APIs

When an API is marked deprecated, never introduce new uses. Check headers for deprecation notices before using unfamiliar APIs.

Reviewer note: To flag a violation of this rule, you MUST read the actual header file that declares the API and confirm a deprecation notice exists in the current source tree. Do not rely on memory or training data — APIs change across chromium upgrades and assumptions about what is or isn't deprecated are frequently wrong.

// ❌ WRONG - base::Hash deprecated for 6+ years
uint32_t hash = base::Hash(str);

// ✅ CORRECT - use the recommended replacement
uint32_t hash = base::FastHash(base::as_byte_span(str));

---

✅ Default-Initialize POD-Type Members in Headers

Plain old data (POD) type members in structs and classes declared in headers must have explicit default initialization. Uninitialized POD members lead to undefined behavior when read before being written.

// ❌ WRONG
struct TopicArticle {
  int id;
  double score;
};

// ✅ CORRECT
struct TopicArticle {
  int id = 0;
  double score = 0.0;
};

---

✅ Prefer std::string_view Over const char* for Parameters

Use std::string_view instead of const char* for function parameters that accept string data. std::string_view is more flexible (accepts std::string, const char*, string literals) and carries size information.

// ❌ WRONG
std::string_view GetDomain(const char* env_from_switch);

// ✅ CORRECT
std::string_view GetDomain(std::string_view env_from_switch);

---

⚠️ std::string_view Class Members — Know When They're Safe

Storing std::string_view as a class member is dangerous in general because the referenced data must outlive the object. However, there are well-established safe patterns:

Safe — no need to flag:

• Members initialized from inline constexpr char[] constants (e.g., pref keys like kMyPrefName), kFeatureName string literals, or other compile-time string constants with static storage duration
• Members initialized from string literals directly ("some_string")
• Members in short-lived stack objects where the caller's string clearly outlives the object

Dangerous — flag these:

• Members initialized from std::string temporaries or function return values
• Members in long-lived objects (singletons, services) initialized from non-static strings
• Members where the constructor accepts std::string_view and the caller might pass a temporary

// ✅ SAFE - pref keys are inline constexpr with static lifetime
class MyMetrics {
  std::string_view histogram_name_;  // initialized from kHistogramName
  std::string_view pref_path_;       // initialized from prefs::kMyPref
};

// ❌ DANGEROUS - caller could pass a temporary
class Config {
  std::string_view name_;  // who owns the underlying string?
  Config(std::string_view name) : name_(name) {}  // dangles if temp passed
};

When reviewing: Check what the string_view member is actually initialized from before flagging. Pref key constants and string literals have static lifetime and are safe.

---

✅ Use base::circular_deque Over std::deque

Always use base::circular_deque (or base::queue/base::stack) instead of std::deque (or std::queue/std::stack). The base versions have consistent memory usage across platforms and save code size. See Chromium container guidelines.

// ❌ WRONG - platform-dependent behavior
std::deque<int> items;
std::queue<int> pending;
std::stack<int> history;

// ✅ CORRECT
base::circular_deque<int> items;
base::queue<int> pending;
base::stack<int> history;

Note: base::circular_deque does not maintain stable iterators during mutations. Use std::list if stable iterators with constant-time insert/remove are required.

---

✅ Use size_t for Sizes, Counts, and Indices

Use size_t for object sizes, allocation sizes, element counts, array offsets, and vector indices. This prevents unnecessary casts with STL APIs. Public APIs should use size_t even if internals optimize with uint32_t. See Chromium C++ style guide.

// ❌ WRONG - unnecessary casts
int count = static_cast<int>(vec.size());
for (int i = 0; i < vec.size(); ++i) { ... }

// ✅ CORRECT
size_t count = vec.size();
for (size_t i = 0; i < vec.size(); ++i) { ... }

---

✅ Use Transparent Comparisons for base::flat_map String Lookups

base::flat_map and base::flat_set support transparent comparisons, enabling lookups with std::string_view or const char* without constructing temporary std::string objects. This avoids unnecessary heap allocations on lookups.

// ❌ WRONG - operator[] constructs a temporary std::string
base::flat_map<std::string, int> map;
int val = map["key"];  // temporary std::string("key") created

// ✅ CORRECT - find() uses transparent comparison, no temporary
base::flat_map<std::string, int> map;
auto it = map.find("key");  // no temporary string created

For base::flat_set of std::unique_ptr, use base::UniquePtrComparator for transparent lookups by raw pointer. See Chromium container guidelines.

---

❌ Don't Rely on Implicit const char* → string_view for Non-Null-Terminated Data

When constructing std::string_view from a byte buffer or reinterpret_cast'd pointer, always pass the size explicitly. Implicit string_view(const char*) calls strlen(), which causes undefined behavior if the data is not null-terminated.

This commonly occurs when migrating from (const char* src, size_t src_len) APIs to string_view APIs — dropping the size parameter silently changes the semantics.

std::vector<uint8_t> data = GetBytes();

// ❌ WRONG - implicit strlen() on potentially non-null-terminated data
base::ReadUnicodeCharacter(
    reinterpret_cast<const char*>(data.data()), &i, &code_point);

// ✅ CORRECT - base::as_string_view handles the conversion safely
base::ReadUnicodeCharacter(
    base::as_string_view(data), &i, &code_point);

---

✅ String Usage in Rust/C++ FFI

Pick FFI string types by data flow (borrow vs. own) and payload size (standard vs. large). Rust's String/Vec and C++'s std::string/std::vector use different allocators and memory layouts, so the wrong choice causes hidden copies, lifetime bugs, or container overhead. Sections below cover each case with examples.

Rust → C++ read-only text — &str (maps to rust::Str)

Zero-copy. Rust retains ownership; C++ reads as a view. Use for logging, printing, parsing.

// ❌ WRONG - forces C++ container overhead for a borrow
unsafe extern "C++" {
    fn log_event(name: &CxxString);
}

// ✅ CORRECT - zero-copy borrow
unsafe extern "C++" {
    fn log_event(name: &str);
}

void LogEvent(rust::Str name) {
  // Treat rust::Str as std::string_view — read only, do not store.
  VLOG(1) << base::RustStrToStringView(name);
}

C++ → Rust read-only text — &str (maps to rust::Str)

Zero-copy. Rust receives a native &str it can search, slice, or print immediately. Use for validation, key lookup.

// ❌ WRONG - bridge type leaks into idiomatic Rust code
fn is_valid_key(key: &CxxString) -> bool;

// ✅ CORRECT - native &str on the Rust side
fn is_valid_key(key: &str) -> bool {
    KNOWN_KEYS.contains(&key)
}

bool valid = ffi::is_valid_key(key_string_view);  // implicit conversion

Rust → C++ ownership, standard text — String (maps to rust::String)

Container move; no text copy. C++ owns the rust::String for the scope of the call. Deep-copy into std::string to preserve past the function's return — never store rust::String as a class member.

// ✅ Return owned text by value
fn build_report() -> String {
    format!("report: {}", compute())
}

// ❌ WRONG - bridge type held past FFI call; ties C++ class to Rust
// allocator and bridge layout.
class Reporter {
  rust::String last_report_;
};
void Reporter::Refresh() {
  last_report_ = ffi::build_report();
}

// ✅ CORRECT - deep-copy into std::string at the boundary
class Reporter {
  std::string last_report_;
};
void Reporter::Refresh() {
  rust::String r = ffi::build_report();
  last_report_.assign(r.data(), r.length());
}

Rust → C++ ownership, MASSIVE payload — Box<Vec<u8>> (maps to rust::Box<rust::Vec<uint8_t>>)

Zero-copy. Heap pointer moves to C++; C++ wraps bytes in std::string_view or base::span. Rust's allocator cleans up when Box drops. Use for file buffers, JSON blobs, streaming payloads.

// ❌ WRONG - container copy through rust::String layout
fn produce_payload() -> String;

// ✅ CORRECT - pointer move, zero-copy
fn produce_payload() -> Box<Vec<u8>>;

void Consume(rust::Box<rust::Vec<uint8_t>> buf) {
  base::span<const uint8_t> bytes(buf);
  // ...consume bytes. Rust deallocator runs when buf drops.
}

C++ → Rust ownership, standard text — String (maps to rust::String)

Deep copy into Rust heap. C++ builds text, copies into rust::String, drops its local copy. Rust gains un-aliased control of an idiomatic container.

// ❌ WRONG - borrowing CxxString into long-lived Rust state ties Rust's
// lifetime to a C++ container.
fn store_name(name: &CxxString);

// ✅ CORRECT - C++ deep-copies at the call site
fn store_name(name: String);

std::string computed = base::StrCat({prefix, suffix});
ffi::store_name(rust::String(computed));  // deep copy into Rust heap

C++ → Rust ownership, MASSIVE payload — UniquePtr<CxxString>

Zero-copy pointer move. Allocation context stays C++-side; cxx invokes the C++ destructor when Rust drops the UniquePtr. Use for ingesting large blobs.

// ❌ WRONG - forces deep copy of large buffer
fn ingest(blob: String);

// ✅ CORRECT - zero-copy pointer move
fn ingest(blob: UniquePtr<CxxString>);

auto blob = std::make_unique<std::string>(LoadLargeFile());
ffi::ingest(std::move(blob));  // ownership transfers to Rust

Collections of strings

Same borrow-vs-own split. &CxxVector<CxxString> is almost never the right answer.

Rust → C++ read-only array — &[&str]

Zero-copy, zero-allocation. No C++ container overhead.

// ❌ WRONG - C++ container overhead for a borrow
unsafe extern "C++" {
    fn process(keys: &CxxVector<CxxString>);
}

// ✅ CORRECT
unsafe extern "C++" {
    fn process(keys: &[&str]);
}

C++ → Rust ownership, large — UniquePtr<CxxVector<CxxString>>

Container lifetime owned by Rust; strings readable without per-element conversion.

fn ingest(items: UniquePtr<CxxVector<CxxString>>);

auto items = std::make_unique<std::vector<std::string>>(BuildItems());
ffi::ingest(std::move(items));

C++ → Rust ownership, small / ergonomic — Vec<String> (deep copy)

Highest safety. Fully decouples from C++ memory on receipt.

fn ingest_small(items: Vec<String>);

---