PSA: The New Two-Argument BASE_FEATURE Macro

[Xiaohan Wang (王消寒)]

Hello Chromium developers,

tl;dr:

Now you can write:

    BASE_FEATURE(MyFeature, base::FEATURE_ENABLED_BY_DEFAULT);

which is equivalent to:

    BASE_FEATURE(kMyFeature, "MyFeature", base::FEATURE_ENABLED_BY_DEFAULT);

Why?

In most cases, the feature name and string already match, so there's no need to type it twice. While there are legitimate reasons for them to differ, typos or mismatches can also cause confusion. The new two-argument macro simplifies feature definition and helps avoid unintended discrepancies. The existing three-argument macro will continue to be supported for cases where the name and string need to be different.

Migration

Over the next few days, I'll be running an automated script (with the help of AI) to migrate existing BASE_FEATURE definitions across the code base where the name and the string already match. Any features where the name and string do not match will not be migrated. These will be logged for further analysis as a follow-up.

If you encounter any issues related to this change or the new macro, please let me know!

The work is being tracked in https://issues.chromium.org/issues/436274260.

Thanks!
Xiaohan

[Alesandro Ortiz]

Great idea, Xiaohan! I've wanted it to be like this for years, both from readability and writing perspective.

I left a comment on the issue regarding Code Search reference concerns which should be solvable. Ultimately want this to go forward! :)

Cheers,

Alesandro

[Shezan Baig]

It looks like finding the feature declaration from the usage point still works:
https://source.chromium.org/chromium/chromium/src/+/main:content/browser/accessibility/browser_accessibility_state_impl_win.cc;l=408;drc=2b2aabbe4856380ccbeabc14a18600500d13bb46

However, finding usages of the feature from the declaration point no longer works:
https://source.chromium.org/chromium/chromium/src/+/main:content/browser/accessibility/browser_accessibility_state_impl_win.cc;l=40;drc=545281df210771e78a03d6fdbcb623404da3ce54

[Xiaohan Wang (王消寒)]

Thanks for the feedback and the discussions in the bug. Sorry for the inconvenience and confusion this migration caused!

I see the code search issue and we are proposing a small tweak to the API. We'll have

    BASE_FEATURE(kMyFeature, base::FEATURE_ENABLED_BY_DEFAULT);

Instead of

    BASE_FEATURE(MyFeature, base::FEATURE_ENABLED_BY_DEFAULT);

This should fix the code search issue automatically, while preserving the benefits of this change. It will also make BASE_DECLARE_FEATURE(kMyFeature) and BASE_FEATURE(kMyFeature, ...) more consistent.

I also replied with the same info (but more details) in the bug, and will start to work on this this week.

Best,

Xiaohan

[Xiaohan Wang (王消寒)]

Thank you for all your suggestions and patience!

The migration has been completed, with the new format:

BASE_DECLARE_FEATURE(kMyFeature);
BASE_FEATURE(kMyFeature, base::FEATURE_ENABLED_BY_DEFAULT);

which is equivalent to

BASE_DECLARE_FEATURE(kMyFeature);
BASE_FEATURE(kMyFeature, "MyFeature", base::FEATURE_ENABLED_BY_DEFAULT);

Notes:

• Code search fully works!
• The 3-param macro is still supported, and existing defines are not migrated if there's a mismatch between the feature identifier and name, including typos (see mismatch summary and examples).
• A new presubmit check is added to encourage the new format instead of the old 3-param macro.
• Internal/public feature parsing logic updated to support the new macro.
• The migration involves 65 CLs, with a net reduction of 6,407 lines of code (13,900 lines removed and 7,493 added).

Special thanks to @Daniel Cheng and @Alexei Svitkine for the great support and help!

Best,

Xiaohan