Stephen Nusko (Gerrit)

unread,

Dec 19, 2025, 12:46:44 AM12/19/25

to Bryan Enrique Gonzalez, Mathias Bynens, Nikhil Nayunigari, (Julie)Jeongeun Kim, AyeAye, Chromium LUCI CQ, yhanad...@chromium.org, nona+...@chromium.org, chadduffin+w...@chromium.org, keithle...@chromium.org, oshima...@chromium.org, jackshira+w...@google.com, stevenjb+wa...@chromium.org, jonmann+wa...@chromium.org, chromotin...@chromium.org, tranbaod...@chromium.org, khorimoto+w...@chromium.org, jiajunz+wa...@google.com, hsuregan+wa...@chromium.org, shuche...@chromium.org, ejcaruso+wa...@chromium.org, derinel+wat...@google.com, dtseng...@chromium.org, kyungjunle...@google.com, bnc+...@chromium.org, webauthn...@chromium.org, net-r...@chromium.org, abigailbk...@google.com, yuzo+...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, francisjp...@google.com, jshin...@chromium.org, security-...@chromium.org

Attention needed from Bryan Enrique Gonzalez

Stephen Nusko added 5 comments

Patchset-level comments

File-level comment, Patchset 5 (Latest):

Stephen Nusko . resolved

Couple compile errors as well.

File base/strings/utf_string_conversion_utils.h

Line 79, Patchset 5 (Latest):UNSAFE_BUFFER_USAGE BASE_EXPORT bool ReadUnicodeCharacter(

Stephen Nusko . unresolved

Add a comment marking this as deprecated and suggest that people should migrate to the safe span versions?

File base/strings/utf_string_conversion_utils.cc

Line 157, Patchset 5 (Latest):

template void PrepareForUTF8Output(std::basic_string_view<wchar_t>,
                                   std::string*);
#endif
template void PrepareForUTF8Output(std::basic_string_view<char16_t>,

Stephen Nusko . unresolved

There are using statements for these

I think std::u16string_view and std::wstring_view

you only need base_string_view if you yourself have a template.

File ui/accessibility/platform/ax_platform_node_auralinux.cc

Line 1059, Patchset 5 (Latest): base::ReadUnicodeCharacter(std::u16string_view(text.c_str(), text.size() + 1),

Stephen Nusko . unresolved

Can we just use base::cstring_view (and the associated other types: https://source.chromium.org/chromium/chromium/src/+/main:base/strings/cstring_view.h;l=515-540;drc=a0655c229239abdb783bd381987b54add9e3b779)

File ui/shell_dialogs/select_file_dialog.cc

Line 39, Patchset 5 (Latest): if (!base::ReadUnicodeCharacter(std::u16_string_view(c_str, size), &i,

Stephen Nusko . unresolved

Can we use AsStringPiece16: https://source.chromium.org/chromium/chromium/src/+/main:base/strings/string_util_win.h;l=110;drc=35b7499d1941df1fceb8d1eb2efb32346a903df5

Open in Gerrit

Related details

Attention is currently required from:

Bryan Enrique Gonzalez

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Bryan Enrique Gonzalez (Gerrit)

unread,

Dec 19, 2025, 2:31:15 AM12/19/25

to Mathias Bynens, Nikhil Nayunigari, (Julie)Jeongeun Kim, AyeAye, Stephen Nusko, Chromium LUCI CQ, yhanad...@chromium.org, nona+...@chromium.org, chadduffin+w...@chromium.org, keithle...@chromium.org, oshima...@chromium.org, jackshira+w...@google.com, stevenjb+wa...@chromium.org, jonmann+wa...@chromium.org, chromotin...@chromium.org, tranbaod...@chromium.org, khorimoto+w...@chromium.org, jiajunz+wa...@google.com, hsuregan+wa...@chromium.org, shuche...@chromium.org, ejcaruso+wa...@chromium.org, derinel+wat...@google.com, dtseng...@chromium.org, kyungjunle...@google.com, bnc+...@chromium.org, webauthn...@chromium.org, net-r...@chromium.org, abigailbk...@google.com, yuzo+...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, francisjp...@google.com, jshin...@chromium.org, security-...@chromium.org

Attention needed from Stephen Nusko

Bryan Enrique Gonzalez added 4 comments

File base/strings/utf_string_conversion_utils.h

Line 79, Patchset 5:UNSAFE_BUFFER_USAGE BASE_EXPORT bool ReadUnicodeCharacter(

Stephen Nusko . resolved

Add a comment marking this as deprecated and suggest that people should migrate to the safe span versions?

Bryan Enrique Gonzalez

Done

File base/strings/utf_string_conversion_utils.cc

Line 157, Patchset 5:template void PrepareForUTF8Output(std::basic_string_view<wchar_t>,


                                   std::string*);
#endif
template void PrepareForUTF8Output(std::basic_string_view<char16_t>,

Stephen Nusko . resolved

There are using statements for these
I think std::u16string_view and std::wstring_view
you only need base_string_view if you yourself have a template.

Bryan Enrique Gonzalez

Done

File ui/accessibility/platform/ax_platform_node_auralinux.cc

Line 1059, Patchset 5: base::ReadUnicodeCharacter(std::u16string_view(text.c_str(), text.size() + 1),

Stephen Nusko . unresolved

Can we just use base::cstring_view (and the associated other types: https://source.chromium.org/chromium/chromium/src/+/main:base/strings/cstring_view.h;l=515-540;drc=a0655c229239abdb783bd381987b54add9e3b779)

Bryan Enrique Gonzalez

Made a quick check and seems like base::u16cstring_view doesn't include the null terminator in its length.
```
auto test_str = base::u16cstring_view(text);

CHECK_EQ(test_str.size(), text.size() + 1);
```
And that crashed!

File ui/shell_dialogs/select_file_dialog.cc

Line 39, Patchset 5: if (!base::ReadUnicodeCharacter(std::u16_string_view(c_str, size), &i,

Stephen Nusko . unresolved

Can we use AsStringPiece16: https://source.chromium.org/chromium/chromium/src/+/main:base/strings/string_util_win.h;l=110;drc=35b7499d1941df1fceb8d1eb2efb32346a903df5

Bryan Enrique Gonzalez

I think we can't because `AsStringPiece16` uses all the size of the string.
And in this function we want to truncate the string to a given size.

Open in Gerrit

Related details

Attention is currently required from:

Stephen Nusko

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Francois Pierre Doray (Gerrit)

unread,

Jan 7, 2026, 9:50:09 AM (13 days ago) Jan 7

to Bryan Enrique Gonzalez, Keishi Hattori, Mathias Bynens, Nikhil Nayunigari, (Julie)Jeongeun Kim, AyeAye, Chromium LUCI CQ, yhanad...@chromium.org, nona+...@chromium.org, chadduffin+w...@chromium.org, keithle...@chromium.org, oshima...@chromium.org, jackshira+w...@google.com, stevenjb+wa...@chromium.org, jonmann+wa...@chromium.org, chromotin...@chromium.org, tranbaod...@chromium.org, khorimoto+w...@chromium.org, jiajunz+wa...@google.com, hsuregan+wa...@chromium.org, shuche...@chromium.org, ejcaruso+wa...@chromium.org, derinel+wat...@google.com, dtseng...@chromium.org, kyungjunle...@google.com, bnc+...@chromium.org, webauthn...@chromium.org, net-r...@chromium.org, abigailbk...@google.com, yuzo+...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, francisjp...@google.com, jshin...@chromium.org, security-...@chromium.org

Attention needed from Bryan Enrique Gonzalez and Keishi Hattori

Francois Pierre Doray added 4 comments

File base/strings/escape.cc

Line 527, Patchset 7 (Latest):

  if (UTF8ToUTF16WithAdjustments(unescaped_url,
                                 &result, adjustments)) {

Francois Pierre Doray . unresolved

This can fit on a single line. `git cl format` should do it automatically.

File base/strings/utf_string_conversion_utils.cc

Line 23, Patchset 7 (Latest):

    if (!ReadUnicodeCharacter(text, &index,
                              &unused)) {

Francois Pierre Doray . unresolved

This can fit on a single line. `git cl format` should do it automatically.

Line 32, Patchset 7 (Latest):

Francois Pierre Doray . unresolved

To avoid code duplication, implement one of the 2 functions below by invoking the other function.

Line 52, Patchset 7 (Latest): base_icu::UChar32* code_point_out) {

Francois Pierre Doray . unresolved

Early return if `*char_index >= src.size()`, similar to what the function below. I think this will avoid an potentially out-of-bounds access here https://source.chromium.org/chromium/chromium/src/+/main:base/third_party/icu/icu_utf.h;l=241;drc=cac93fac787ed4ef28e8ea6e8b6282dfeddd698c

Open in Gerrit

Related details

Attention is currently required from:

Bryan Enrique Gonzalez
Keishi Hattori

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Bryan Enrique Gonzalez (Gerrit)

unread,

Jan 7, 2026, 6:42:38 PM (13 days ago) Jan 7

to James Cook, Keishi Hattori, Francois Pierre Doray, Mathias Bynens, Nikhil Nayunigari, (Julie)Jeongeun Kim, AyeAye, Chromium LUCI CQ, yhanad...@chromium.org, nona+...@chromium.org, chadduffin+w...@chromium.org, keithle...@chromium.org, oshima...@chromium.org, jackshira+w...@google.com, stevenjb+wa...@chromium.org, jonmann+wa...@chromium.org, chromotin...@chromium.org, tranbaod...@chromium.org, khorimoto+w...@chromium.org, jiajunz+wa...@google.com, hsuregan+wa...@chromium.org, shuche...@chromium.org, ejcaruso+wa...@chromium.org, derinel+wat...@google.com, dtseng...@chromium.org, kyungjunle...@google.com, bnc+...@chromium.org, webauthn...@chromium.org, net-r...@chromium.org, abigailbk...@google.com, yuzo+...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, francisjp...@google.com, jshin...@chromium.org, security-...@chromium.org

Attention needed from Francois Pierre Doray, James Cook and Keishi Hattori

Bryan Enrique Gonzalez added 4 comments

File base/strings/escape.cc

Line 527, Patchset 7:

  if (UTF8ToUTF16WithAdjustments(unescaped_url,
                                 &result, adjustments)) {

Francois Pierre Doray . resolved

This can fit on a single line. `git cl format` should do it automatically.

Bryan Enrique Gonzalez

Done

File base/strings/utf_string_conversion_utils.cc

Line 23, Patchset 7:

    if (!ReadUnicodeCharacter(text, &index,
                              &unused)) {

Francois Pierre Doray . resolved

This can fit on a single line. `git cl format` should do it automatically.

Bryan Enrique Gonzalez

Done

Line 32, Patchset 7:

Francois Pierre Doray . resolved

To avoid code duplication, implement one of the 2 functions below by invoking the other function.

Bryan Enrique Gonzalez

Done

Line 52, Patchset 7: base_icu::UChar32* code_point_out) {

Francois Pierre Doray . resolved

Early return if `*char_index >= src.size()`, similar to what the function below. I think this will avoid an potentially out-of-bounds access here https://source.chromium.org/chromium/chromium/src/+/main:base/third_party/icu/icu_utf.h;l=241;drc=cac93fac787ed4ef28e8ea6e8b6282dfeddd698c

Bryan Enrique Gonzalez

Done

Open in Gerrit

Related details

Attention is currently required from:

Francois Pierre Doray
James Cook
Keishi Hattori

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Mitsuru Oshima (Gerrit)

unread,

Jan 7, 2026, 6:46:35 PM (13 days ago) Jan 7

to Bryan Enrique Gonzalez, Daniel Cheng, Maksim Sadym, James Cook, Keishi Hattori, Francois Pierre Doray, Mathias Bynens, Nikhil Nayunigari, (Julie)Jeongeun Kim, AyeAye, Chromium LUCI CQ, yhanad...@chromium.org, nona+...@chromium.org, chadduffin+w...@chromium.org, keithle...@chromium.org, oshima...@chromium.org, jackshira+w...@google.com, stevenjb+wa...@chromium.org, jonmann+wa...@chromium.org, chromotin...@chromium.org, tranbaod...@chromium.org, khorimoto+w...@chromium.org, jiajunz+wa...@google.com, hsuregan+wa...@chromium.org, shuche...@chromium.org, ejcaruso+wa...@chromium.org, derinel+wat...@google.com, dtseng...@chromium.org, kyungjunle...@google.com, bnc+...@chromium.org, webauthn...@chromium.org, net-r...@chromium.org, abigailbk...@google.com, yuzo+...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, francisjp...@google.com, jshin...@chromium.org, security-...@chromium.org

Attention needed from Bryan Enrique Gonzalez, Daniel Cheng, Francois Pierre Doray, James Cook, Keishi Hattori and Maksim Sadym

Mitsuru Oshima added 1 comment

Patchset-level comments

File-level comment, Patchset 8 (Latest):

Mitsuru Oshima . resolved

can you resolve conflicts?

Open in Gerrit

Related details

Attention is currently required from:

Bryan Enrique Gonzalez
Daniel Cheng

Francois Pierre Doray
James Cook
Keishi Hattori

Maksim Sadym

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Francois Pierre Doray (Gerrit)

unread,

Jan 9, 2026, 1:43:09 PM (11 days ago) Jan 9

to Bryan Enrique Gonzalez, Mitsuru Oshima, Keishi Hattori, Mathias Bynens, Nikhil Nayunigari, (Julie)Jeongeun Kim, AyeAye, Chromium LUCI CQ, yhanad...@chromium.org, nona+...@chromium.org, chadduffin+w...@chromium.org, keithle...@chromium.org, oshima...@chromium.org, jackshira+w...@google.com, stevenjb+wa...@chromium.org, jonmann+wa...@chromium.org, chromotin...@chromium.org, tranbaod...@chromium.org, khorimoto+w...@chromium.org, jiajunz+wa...@google.com, hsuregan+wa...@chromium.org, shuche...@chromium.org, ejcaruso+wa...@chromium.org, derinel+wat...@google.com, dtseng...@chromium.org, kyungjunle...@google.com, bnc+...@chromium.org, webauthn...@chromium.org, net-r...@chromium.org, abigailbk...@google.com, yuzo+...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, francisjp...@google.com, jshin...@chromium.org, security-...@chromium.org

Attention needed from Bryan Enrique Gonzalez and Keishi Hattori

Francois Pierre Doray voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Bryan Enrique Gonzalez
Keishi Hattori

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Francois Pierre Doray (Gerrit)

unread,

Jan 9, 2026, 2:02:48 PM (11 days ago) Jan 9

to Bryan Enrique Gonzalez, Mitsuru Oshima, Keishi Hattori, Mathias Bynens, Nikhil Nayunigari, (Julie)Jeongeun Kim, AyeAye, Chromium LUCI CQ, yhanad...@chromium.org, nona+...@chromium.org, chadduffin+w...@chromium.org, keithle...@chromium.org, oshima...@chromium.org, jackshira+w...@google.com, stevenjb+wa...@chromium.org, jonmann+wa...@chromium.org, chromotin...@chromium.org, tranbaod...@chromium.org, khorimoto+w...@chromium.org, jiajunz+wa...@google.com, hsuregan+wa...@chromium.org, shuche...@chromium.org, ejcaruso+wa...@chromium.org, derinel+wat...@google.com, dtseng...@chromium.org, kyungjunle...@google.com, bnc+...@chromium.org, webauthn...@chromium.org, net-r...@chromium.org, abigailbk...@google.com, yuzo+...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, francisjp...@google.com, jshin...@chromium.org, security-...@chromium.org

Attention needed from Bryan Enrique Gonzalez and Keishi Hattori

Francois Pierre Doray added 1 comment

File ui/accessibility/platform/ax_platform_node_auralinux.cc

Line 1058, Patchset 8 (Latest): base_icu::UChar32 code_point;

Francois Pierre Doray . unresolved

LGTM because this preserves an existing behavior, but it is very surprising to me that we include the null terminator in the range inspected by base::ReadUnicodeCharacter. Worth asking an owner whether this was intentional? If not, the code can be simplified by passing base::u16cstring_view directly.

satisfied_requirement

unsatisfied_requirement

open

diffy

Bryan Enrique Gonzalez (Gerrit)

unread,

Jan 9, 2026, 4:41:54 PM (11 days ago) Jan 9

to chromotin...@chromium.org

Attention needed from Keishi Hattori

Bryan Enrique Gonzalez added 1 comment

Patchset-level comments

File-level comment, Patchset 8 (Latest):

Bryan Enrique Gonzalez . resolved

Splitted the CL because of changes on many files. CLs:
http://crrev.com/c/7426427

Open in Gerrit

Related details

Attention is currently required from:

Keishi Hattori

satisfied_requirement

unsatisfied_requirement

open

diffy

Bryan Enrique Gonzalez (Gerrit)

unread,

Jan 14, 2026, 4:24:33 PM (6 days ago) Jan 14

to Code Review Nudger, Francois Pierre Doray, Mitsuru Oshima, Keishi Hattori, Mathias Bynens, Nikhil Nayunigari, (Julie)Jeongeun Kim, AyeAye, Chromium LUCI CQ, yhanad...@chromium.org, nona+...@chromium.org, chadduffin+w...@chromium.org, keithle...@chromium.org, oshima...@chromium.org, jackshira+w...@google.com, stevenjb+wa...@chromium.org, jonmann+wa...@chromium.org, chromotin...@chromium.org, tranbaod...@chromium.org, khorimoto+w...@chromium.org, jiajunz+wa...@google.com, hsuregan+wa...@chromium.org, shuche...@chromium.org, ejcaruso+wa...@chromium.org, derinel+wat...@google.com, dtseng...@chromium.org, kyungjunle...@google.com, bnc+...@chromium.org, webauthn...@chromium.org, net-r...@chromium.org, abigailbk...@google.com, yuzo+...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, francisjp...@google.com, jshin...@chromium.org, security-...@chromium.org

Bryan Enrique Gonzalez abandoned this change

Related details

Attention set is empty

Submit Requirements:

Code-Coverage

Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Bryan Enrique Gonzalez (Gerrit)

unread,

11:50 AM (7 hours ago) 11:50 AM

to Code Review Nudger, Francois Pierre Doray, Mitsuru Oshima, Keishi Hattori, Mathias Bynens, Nikhil Nayunigari, (Julie)Jeongeun Kim, AyeAye, Chromium LUCI CQ, yhanad...@chromium.org, nona+...@chromium.org, chadduffin+w...@chromium.org, keithle...@chromium.org, oshima...@chromium.org, jackshira+w...@google.com, stevenjb+wa...@chromium.org, jonmann+wa...@chromium.org, chromotin...@chromium.org, tranbaod...@chromium.org, khorimoto+w...@chromium.org, jiajunz+wa...@google.com, hsuregan+wa...@chromium.org, shuche...@chromium.org, ejcaruso+wa...@chromium.org, derinel+wat...@google.com, dtseng...@chromium.org, kyungjunle...@google.com, bnc+...@chromium.org, webauthn...@chromium.org, net-r...@chromium.org, abigailbk...@google.com, yuzo+...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, francisjp...@google.com, jshin...@chromium.org, security-...@chromium.org

Bryan Enrique Gonzalez restored this change

Open in Gerrit

Related details

Attention set is empty

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

spanification: automatically spanify base/strings/utf_string_conversion_utils.cc etc. [chromium/src : main]

Stephen Nusko (Gerrit)

Stephen Nusko added 5 comments

Related details

Bryan Enrique Gonzalez (Gerrit)

Bryan Enrique Gonzalez added 4 comments

Related details

Francois Pierre Doray (Gerrit)

Francois Pierre Doray added 4 comments

Related details

Bryan Enrique Gonzalez (Gerrit)

Bryan Enrique Gonzalez added 4 comments

Related details

Mitsuru Oshima (Gerrit)

Mitsuru Oshima added 1 comment

Related details

Francois Pierre Doray (Gerrit)

Francois Pierre Doray voted Code-Review+1

Related details

Francois Pierre Doray (Gerrit)

Francois Pierre Doray added 1 comment

Bryan Enrique Gonzalez (Gerrit)

Bryan Enrique Gonzalez added 1 comment

Related details

Bryan Enrique Gonzalez (Gerrit)

Bryan Enrique Gonzalez abandoned this change

Related details

Bryan Enrique Gonzalez (Gerrit)

Bryan Enrique Gonzalez restored this change

Related details