Spanify `RedactionTool` (1/2) [chromium/src : main]
0 views
Skip to first unread message
Kalvin Lee (Gerrit)
Feb 5, 2026, 1:19:28 AM (2 days ago) Feb 5
to Xiangdong Kong, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, fuzzin...@chromium.org, oshima...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Xiangdong Kong
Kalvin Lee added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Related details
Attention is currently required from:
- Xiangdong Kong
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Xiangdong Kong (Gerrit)
Feb 5, 2026, 12:18:20 PM (2 days ago) Feb 5
to Kalvin Lee, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, fuzzin...@chromium.org, oshima...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Kalvin Lee
Xiangdong Kong voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Kalvin Lee
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kalvin Lee (Gerrit)
Feb 5, 2026, 10:07:16 PM (2 days ago) Feb 5
to Vasilii Sukhanov, Emilia Paz, Michelle Chen, Xiangdong Kong, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, fuzzin...@chromium.org, oshima...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Emilia Paz, Michelle Chen and Vasilii Sukhanov
Kalvin Lee added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Kalvin Lee . resolved
I'll let Gerrit call out who owns what --- apologies for letting this refactor CL get so unwieldy.
Related details
Attention is currently required from:
- Emilia Paz
- Michelle Chen
- Vasilii Sukhanov
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michelle Chen (Gerrit)
Feb 5, 2026, 10:09:03 PM (2 days ago) Feb 5
to Kalvin Lee, Vasilii Sukhanov, Emilia Paz, Xiangdong Kong, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, fuzzin...@chromium.org, oshima...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Emilia Paz, Kalvin Lee and Vasilii Sukhanov
Michelle Chen voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Emilia Paz
- Kalvin Lee
- Vasilii Sukhanov
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vasilii Sukhanov (Gerrit)
Feb 6, 2026, 7:37:01 AM (yesterday) Feb 6
to Kalvin Lee, Michelle Chen, Emilia Paz, Xiangdong Kong, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, fuzzin...@chromium.org, oshima...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Emilia Paz and Kalvin Lee
Vasilii Sukhanov added 5 comments![]( "Open in Gerrit")
File chrome/common/extensions/extension_constants.h
Line 362, Patchset 3 (Latest):
nullptr, // Null-terminated array.Vasilii Sukhanov . unresolved
It's not new in this CL but I'm curious why this one is needed.
Line 329, Patchset 3 (Latest):
std::to_array<const char* const>({Vasilii Sukhanov . unresolved
Can it be `std::string_view`?
File components/feedback/redaction_tool/redaction_tool.h
Line 173, Patchset 3 (Latest):
// Null-terminated list of first party extension IDs. We need to have thisVasilii Sukhanov . unresolved
As the span has its size, maybe it doesn't have to be NULL terminated anymore?
Line 60, Patchset 3 (Latest):
RedactionTool(base::span<const char* const> first_party_extension_ids,Vasilii Sukhanov . unresolved
Same question
Line 56, Patchset 3 (Latest):
base::span<const char* const> first_party_extension_ids =Vasilii Sukhanov . unresolved
Can we switch to `string_view`?
Related details
Attention is currently required from:
- Emilia Paz
- Kalvin Lee
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. |
Kalvin Lee (Gerrit)
Feb 6, 2026, 11:18:04 AM (yesterday) Feb 6
to Michelle Chen, Vasilii Sukhanov, Emilia Paz, Xiangdong Kong, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, fuzzin...@chromium.org, oshima...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Emilia Paz and Vasilii Sukhanov
Kalvin Lee added 5 comments![]( "Open in Gerrit")
File chrome/common/extensions/extension_constants.h
Line 362, Patchset 3 (Latest):
nullptr, // Null-terminated array.Vasilii Sukhanov . resolved
It's not new in this CL but I'm curious why this one is needed.
Kalvin Lee
This is a sentinel value to stop iteration (if you look down in the impl of `RedacitionTool`). This is also addressed in the downstream CL, where we no longer need a sentinel.
Line 329, Patchset 3 (Latest):
std::to_array<const char* const>({Vasilii Sukhanov . resolved
Can it be `std::string_view`?
Kalvin Lee
This is done in the downstream CL. I noticed the refactor could be a bit more self-contained if we kept it as `const char* const` for now. This change involves more code owners than usual, so I prefer to minimize changes wherever possible.
If you really prefer, I could squash that CL into this one. That would smuggle the change to `std::string_view` here along with the business logic change, which marginally increases the chance I made a silly mistake that causes the whole CL to get reverted...
File components/feedback/redaction_tool/redaction_tool.h
Line 173, Patchset 3 (Latest):
// Null-terminated list of first party extension IDs. We need to have thisVasilii Sukhanov . resolved
As the span has its size, maybe it doesn't have to be NULL terminated anymore?
Kalvin Lee
Downstream CL
Line 60, Patchset 3 (Latest):
RedactionTool(base::span<const char* const> first_party_extension_ids,Vasilii Sukhanov . resolved
Kalvin LeeSame question
Downstream CL
Line 56, Patchset 3 (Latest):
base::span<const char* const> first_party_extension_ids =Vasilii Sukhanov . resolved
Can we switch to `string_view`?
Attention is currently required from:
- Emilia Paz
- Vasilii Sukhanov
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vasilii Sukhanov (Gerrit)
Feb 6, 2026, 12:49:53 PM (24 hours ago) Feb 6
to Kalvin Lee, Michelle Chen, Emilia Paz, Xiangdong Kong, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, fuzzin...@chromium.org, oshima...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Emilia Paz and Kalvin Lee
Vasilii Sukhanov voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Emilia Paz
- Kalvin Lee
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Emilia Paz (Gerrit)
Feb 6, 2026, 1:28:23 PM (23 hours ago) Feb 6
to Kalvin Lee, Vasilii Sukhanov, Michelle Chen, Xiangdong Kong, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, fuzzin...@chromium.org, oshima...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Kalvin Lee
Emilia Paz added 1 comment![]( "Open in Gerrit")
Attention is currently required from:
- Kalvin Lee
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Emilia Paz (Gerrit)
Feb 6, 2026, 1:55:54 PM (23 hours ago) Feb 6
to Kalvin Lee, Vasilii Sukhanov, Michelle Chen, Xiangdong Kong, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, fuzzin...@chromium.org, oshima...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Kalvin Lee
Emilia Paz voted and added 1 comment![]( "Open in Gerrit")
Votes added by Emilia Paz
| Code-Review | +1 |
1 comment
Patchset-level comments
Emilia Paz . resolved
whops forgot to hit the +1 button
Related details
Attention is currently required from:
- Kalvin Lee
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kalvin Lee (Gerrit)
12:10 AM (13 hours ago) 12:10 AM
to Vasilii Sukhanov, Michelle Chen, Xiangdong Kong, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, fuzzin...@chromium.org, oshima...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Kalvin Lee voted Commit-Queue+2![]( "Open in Gerrit")
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)
1:07 AM (12 hours ago) 1:07 AM
to Kalvin Lee, Emilia Paz, Vasilii Sukhanov, Michelle Chen, Xiangdong Kong, AyeAye, chromium...@chromium.org, fuzzin...@chromium.org, oshima...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Spanify `RedactionTool` (1/2)
Broadly,
1. Convert `kBuiltInFirstPartyExtensionIds` to a `std::array` and
inline it directly in the associated header. This exposes its size
everywhere and makes it compatible with `base::span`.
2. Move away from trafficking `const char* const*`. Use `base::span`
where possible, and default it to the empty span where `nullptr`
would be passed.
Bug: 439455382
Change-Id: If3dbbdc088ece9e760ce359c8c63caab2accb8b9
Reviewed-by: Vasilii Sukhanov <vas...@chromium.org>
Reviewed-by: Emilia Paz <emil...@chromium.org>
Reviewed-by: Xiangdong Kong <xiangd...@google.com>
Commit-Queue: Kalvin Lee <kd...@chromium.org>
Reviewed-by: Michelle Chen <miche...@google.com>
Cr-Commit-Position: refs/heads/main@{#1581291}
Files:
- M chrome/browser/error_reporting/chrome_js_error_report_processor.cc
- M chrome/browser/extensions/api/feedback_private/chrome_feedback_private_delegate.cc
- M chrome/browser/feedback/system_logs/about_system_logs_fetcher.cc
- M chrome/browser/feedback/system_logs/chrome_system_logs_fetcher.cc
- M chrome/browser/support_tool/ash/chrome_user_logs_data_collector_unittest.cc
- M chrome/browser/support_tool/ash/network_health_data_collector_unittest.cc
- M chrome/browser/support_tool/ash/network_routes_data_collector_unittest.cc
- M chrome/browser/support_tool/ash/shill_data_collector_unittest.cc
- M chrome/browser/support_tool/ash/system_logs_data_collector_unittest.cc
- M chrome/browser/support_tool/ash/system_state_data_collector_unittest.cc
- M chrome/browser/support_tool/signin_data_collector_browsertest.cc
- M chrome/browser/support_tool/support_tool_handler.cc
- M chrome/browser/support_tool/system_log_source_data_collector_adaptor_unittest.cc
- M chrome/browser/support_tool/updater_data_collector_unittest.cc
- M chrome/common/extensions/BUILD.gn
- D chrome/common/extensions/extension_constants.cc
- M chrome/common/extensions/extension_constants.h
- M chromeos/ash/components/specialized_features/feedback.cc
- M components/feedback/redaction_tool/redaction_tool.cc
- M components/feedback/redaction_tool/redaction_tool.h
- M components/feedback/redaction_tool_fuzzer.cc
- M components/feedback/system_logs/system_logs_fetcher.cc
- M components/feedback/system_logs/system_logs_fetcher.h
- M extensions/browser/api/feedback_private/feedback_service.cc
- M extensions/browser/api/feedback_private/log_source_access_manager.cc
- M extensions/shell/browser/system_logs/shell_system_logs_fetcher.cc
Change size: M
Delta: 26 files changed, 86 insertions(+), 101 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Vasilii Sukhanov, +1 by Emilia Paz, +1 by Michelle Chen, +1 by Xiangdong Kong
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
luci-bisection@appspot.gserviceaccount.com (Gerrit)
1:49 AM (11 hours ago) 1:49 AM
to Kalvin Lee, Chromium LUCI CQ, Vasilii Sukhanov, Michelle Chen, Xiangdong Kong, AyeAye, chromium...@chromium.org, fuzzin...@chromium.org, oshima...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
luci-bi...@appspot.gserviceaccount.com has created a revert of this change![]( "Open in Gerrit")
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. |
Jason Robert Rauch (Gerrit)
3:56 AM (9 hours ago) 3:56 AM
to Kalvin Lee, Chromium LUCI CQ, Vasilii Sukhanov, Michelle Chen, Xiangdong Kong, AyeAye, chromium...@chromium.org, fuzzin...@chromium.org, oshima...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Jason Robert Rauch added 1 comment![]( "Open in Gerrit")
Patchset-level comments
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. |
Jason Robert Rauch (Gerrit)
5:58 AM (7 hours ago) 5:58 AM
to Kalvin Lee, Chromium LUCI CQ, Vasilii Sukhanov, Michelle Chen, Xiangdong Kong, AyeAye, chromium...@chromium.org, fuzzin...@chromium.org, oshima...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Jason Robert Rauch added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Jason Robert Rauch . resolved
merge
Jason Robert Rauch (Gerrit)
6:03 AM (7 hours ago) 6:03 AM
to Kalvin Lee, Chromium LUCI CQ, Vasilii Sukhanov, Michelle Chen, Xiangdong Kong, AyeAye, chromium...@chromium.org, fuzzin...@chromium.org, oshima...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Jason Robert Rauch added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Jason Robert Rauch . resolved
so did it get wiped out ?? the tree wont open so ill take it by your neasyness of bluntly ignoring me while begging for help means I got got,,,,
Reply all
Reply to author
Forward
0 new messages