[Extensions] Make commands.json / commands API less special [chromium/src : main]
0 views
Skip to first unread message
Devlin Cronin (Gerrit)
Jan 7, 2026, 7:08:03 PM (2 days ago) Jan 7
to Devlin Cronin, Tim, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Tim
Devlin Cronin voted![]( "Open in Gerrit")
| Auto-Submit | +1 |
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Tim
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tim (Gerrit)
Jan 8, 2026, 1:45:41 PM (21 hours ago) Jan 8
to Devlin Cronin, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Devlin Cronin
Tim voted and added 3 comments![]( "Open in Gerrit")
Votes added by Tim
| Code-Review | +1 |
3 comments
Patchset-level comments
File-level comment, Patchset 1 (Latest):
Tim . resolved
Nice tidying up here. LGTM, just one suggestion/question around clarifying "strong C++ types" vs "C++ types"
Commit Message
Line 17, Patchset 1 (Latest):
which is exactly what we want for the commands API (we don't need
strong C++ types, since we don't use them anywhere).Tim . unresolved
This sounds a little counter to the comment above the block it has been moved to: "Schemas that should be used only to auto-generate the C++ types."
I guess this is conflation between types for manifest validation vs strong types for API JS parameter/response validation? Is there some language we can use in the gni-comment to make this more clear?
Line 23, Patchset 1 (Latest):
CommandsgetAllFunction (following our normal naming pattern).Tim . unresolved
nit: CommandsGetAllFunction
Related details
Attention is currently required from:
- Devlin Cronin
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Devlin Cronin (Gerrit)
Jan 8, 2026, 1:51:33 PM (21 hours ago) Jan 8
to Devlin Cronin, Tim, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Devlin Cronin voted and added 3 comments![]( "Open in Gerrit")
Votes added by Devlin Cronin
| Auto-Submit | +1 |
| Commit-Queue | +2 |
3 comments
Patchset-level comments
Commit Message
Line 17, Patchset 1:
which is exactly what we want for the commands API (we don't need
strong C++ types, since we don't use them anywhere).Tim . resolved
This sounds a little counter to the comment above the block it has been moved to: "Schemas that should be used only to auto-generate the C++ types."
I guess this is conflation between types for manifest validation vs strong types for API JS parameter/response validation? Is there some language we can use in the gni-comment to make this more clear?
Devlin Cronin
That comment is just patently wrong. We explicitly _don't_ use uncompiled sources for C++ types.
Let me do that in a followup to lazily use the existing try run here, since this CL doesn't touch that line.
Line 23, Patchset 1:
CommandsgetAllFunction (following our normal naming pattern).Tim . resolved
Devlin Croninnit: CommandsGetAllFunction
Done
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Jan 8, 2026, 1:54:17 PM (21 hours ago) Jan 8
to Devlin Cronin, Tim, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Unreviewed changes
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
Commit message:
[Extensions] Make commands.json / commands API less special
Right now, the commands.json schema is included in a special list in
//extensions/common/api/BUILD.gn and then functions are
"hand-registered" in ChromeExtensionsBrowserAPIProvider.
Instead:
* Move commands.json to uncompiled_sources_ in
//extensions/common/api/api_schemas.gni. This list is used for APIs
that should have the generated JSON strings and function registration,
which is exactly what we want for the commands API (we don't need
strong C++ types, since we don't use them anywhere).
* Use this to auto-register the commands function.
* To enable that, add an "implemented_in" option to commands.json (since
it's implemented in commands.h instead of commands_api.h) and rename
the API function class from GetAllCommandsFunction to
CommandsGetAllFunction (following our normal naming pattern).
Bug: None
Change-Id: Ia05dfaa6d9acbe5d5e457be25640f4081ea2a921
Reviewed-by: Tim <tjud...@chromium.org>
Auto-Submit: Devlin Cronin <rdevlin...@chromium.org>
Commit-Queue: Devlin Cronin <rdevlin...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1566427}
Files:
- M chrome/browser/extensions/api/commands/commands.cc
- M chrome/browser/extensions/api/commands/commands.h
- M chrome/browser/extensions/chrome_extensions_browser_api_provider.cc
- M chrome/common/extensions/api/BUILD.gn
- M chrome/common/extensions/api/api_sources.gni
- M chrome/common/extensions/api/commands.json
Change size: S
Delta: 6 files changed, 7 insertions(+), 8 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Tim
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages