fix auto spanification for latin string conversions [chromium/src : main]

[Daniel Angulo (Gerrit)]

Attention needed from Kalvin Lee and Kyle Charbonneau

Daniel Angulo voted Commit-Queue+1

Commit-Queue

+1

Open in Gerrit

Related details

Attention is currently required from:

• Kalvin Lee
• Kyle Charbonneau

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• Review-Enforcement

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

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I1497384ca9dc7909cbfde9347dcd14d61987f7c1

Gerrit-Change-Number: 7298359

Gerrit-PatchSet: 2

Gerrit-Owner: Daniel Angulo <angd...@google.com>

Gerrit-Reviewer: Daniel Angulo <angd...@google.com>

Gerrit-Reviewer: Kalvin Lee <kd...@chromium.org>

Gerrit-Reviewer: Kyle Charbonneau <kyle...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Kalvin Lee <kd...@chromium.org>

Gerrit-Attention: Kyle Charbonneau <kyle...@chromium.org>

Gerrit-Comment-Date: Tue, 23 Dec 2025 16:45:54 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: Yes

[Kalvin Lee (Gerrit)]

Attention needed from Daniel Angulo and Kyle Charbonneau

Kalvin Lee added 2 comments

File base/strings/latin1_string_conversions.h

Line 29, Patchset 2 (Latest):// TODO(crbug.com/40284755): implement spanified version.

Kalvin Lee . unresolved

This comment can go away, and you can also tie this CL to this bug 😊

File base/strings/latin1_string_conversions.cc

Line 17, Patchset 2 (Latest): return std::u16string(utf16.begin(), utf16.end());

Kalvin Lee . unresolved

Looking at the rest of this CL (and checking CodeSearch for callers of this function), I don't think this branch is ever taken. Can it be removed?

And if it can be removed, could this whole function be collapsed into the two call sites (one of which is newly introduced) inside `string16_mojom_traits.cc`? I'm guessing that the commented concern about binary size is no longer applicable.

Open in Gerrit

Related details

Attention is currently required from:

• Daniel Angulo
• Kyle Charbonneau

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

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I1497384ca9dc7909cbfde9347dcd14d61987f7c1

Gerrit-Change-Number: 7298359

Gerrit-PatchSet: 2

Gerrit-Owner: Daniel Angulo <angd...@google.com>

Gerrit-Reviewer: Daniel Angulo <angd...@google.com>

Gerrit-Reviewer: Kalvin Lee <kd...@chromium.org>

Gerrit-Reviewer: Kyle Charbonneau <kyle...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Kyle Charbonneau <kyle...@chromium.org>

Gerrit-Attention: Daniel Angulo <angd...@google.com>

Gerrit-Comment-Date: Wed, 24 Dec 2025 06:39:40 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Daniel Angulo (Gerrit)]

Attention needed from Kalvin Lee and Kyle Charbonneau

Daniel Angulo added 2 comments

File base/strings/latin1_string_conversions.h

Line 29, Patchset 2:// TODO(crbug.com/40284755): implement spanified version.

Kalvin Lee . resolved

This comment can go away, and you can also tie this CL to this bug 😊

Daniel Angulo

sure

File base/strings/latin1_string_conversions.cc

Line 17, Patchset 2: return std::u16string(utf16.begin(), utf16.end());

Kalvin Lee . unresolved

Looking at the rest of this CL (and checking CodeSearch for callers of this function), I don't think this branch is ever taken. Can it be removed?

And if it can be removed, could this whole function be collapsed into the two call sites (one of which is newly introduced) inside `string16_mojom_traits.cc`? I'm guessing that the commented concern about binary size is no longer applicable.

Daniel Angulo

I think it can be removed. Do you mean it would be better to delete latin1_string_conversions.cc?

Open in Gerrit

Related details

Attention is currently required from:

• Kalvin Lee
• Kyle Charbonneau

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

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I1497384ca9dc7909cbfde9347dcd14d61987f7c1

Gerrit-Change-Number: 7298359

Gerrit-PatchSet: 3

Gerrit-Owner: Daniel Angulo <angd...@google.com>

Gerrit-Reviewer: Daniel Angulo <angd...@google.com>

Gerrit-Reviewer: Kalvin Lee <kd...@chromium.org>

Gerrit-Reviewer: Kyle Charbonneau <kyle...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Kalvin Lee <kd...@chromium.org>

Gerrit-Attention: Kyle Charbonneau <kyle...@chromium.org>

Gerrit-Comment-Date: Fri, 26 Dec 2025 16:39:37 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Kalvin Lee <kd...@chromium.org>