Danilo Tedeschi (Gerrit)

unread,

Jun 24, 2026, 10:04:11 PM (23 hours ago) Jun 24

to Greg Thompson, Andy Phan, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, pdf-r...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, jshin...@chromium.org

Attention needed from Andy Phan and Greg Thompson

Danilo Tedeschi voted Commit-Queue+1

Commit-Queue

+1

Open in Gerrit

Related details

Attention is currently required from:

Andy Phan
Greg Thompson

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

satisfied_requirement

unsatisfied_requirement

open

diffy

Greg Thompson (Gerrit)

unread,

3:49 AM (18 hours ago) 3:49 AM

to Danilo Tedeschi, Andy Phan, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, pdf-r...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, jshin...@chromium.org

Attention needed from Andy Phan and Danilo Tedeschi

Greg Thompson voted and added 4 comments

Votes added by Greg Thompson

Code-Review

+1

4 comments

Commit Message

Line 14, Patchset 7 (Latest): * Core Classes: Moved LanguageTag, LanguageTagConverter, and LanguageTagMatcher from base to ::base::i18n.

Greg Thompson . unresolved

please wrap CL descriptions at 72 cols as per https://chromium.googlesource.com/chromium/src/+/main/docs/contributing.md#uploading-a-change-for-review

Line 20, Patchset 7 (Latest):

   * Bug Fix: Corrected a logic error in LanguageTag::GetExtensionString where incorrect indexing with substr(2) caused truncated extension strings and CHECK failures during unit tests.

Greg Thompson . unresolved

would you mind doing the bug fix in its own CL? it's nice to keep behavior changes independent from renames. thanks.

File base/i18n/timezone.h

Line 78, Patchset 7 (Latest): std::u16string GetDisplayName(const i18n::LanguageTag& language_tag,

Greg Thompson . unresolved

omit `i18n::`

File base/i18n/timezone.cc

Line 130, Patchset 7 (Latest):std::u16string TimeZone::GetDisplayName(const i18n::LanguageTag& language_tag,

Greg Thompson . unresolved

nit: omit `i18n::` since we're in the namespace already

Open in Gerrit

Related details

Attention is currently required from:

Andy Phan
Danilo Tedeschi

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

Danilo Tedeschi (Gerrit)

unread,

1:27 PM (8 hours ago) 1:27 PM

to Greg Thompson, Andy Phan, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, pdf-r...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, jshin...@chromium.org

Attention needed from Andy Phan

Danilo Tedeschi added 4 comments

Commit Message

Line 14, Patchset 7: * Core Classes: Moved LanguageTag, LanguageTagConverter, and LanguageTagMatcher from base to ::base::i18n.

Greg Thompson . resolved

please wrap CL descriptions at 72 cols as per https://chromium.googlesource.com/chromium/src/+/main/docs/contributing.md#uploading-a-change-for-review

Danilo Tedeschi

The guerrit format sometimes does that. I am trying to keep them in separated lines, but it doesn't always work.

Line 20, Patchset 7:

   * Bug Fix: Corrected a logic error in LanguageTag::GetExtensionString where incorrect indexing with substr(2) caused truncated extension strings and CHECK failures during unit tests.

Greg Thompson . resolved

would you mind doing the bug fix in its own CL? it's nice to keep behavior changes independent from renames. thanks.

Danilo Tedeschi

Done

File base/i18n/timezone.h

Line 78, Patchset 7: std::u16string GetDisplayName(const i18n::LanguageTag& language_tag,

Greg Thompson . resolved

omit `i18n::`

Danilo Tedeschi

Done

File base/i18n/timezone.cc

Line 130, Patchset 7:std::u16string TimeZone::GetDisplayName(const i18n::LanguageTag& language_tag,

Greg Thompson . resolved

nit: omit `i18n::` since we're in the namespace already

Danilo Tedeschi

Done

Open in Gerrit

Related details

Attention is currently required from:

Andy Phan

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

satisfied_requirement

unsatisfied_requirement

open

diffy

Andy Phan (Gerrit)

unread,

1:32 PM (8 hours ago) 1:32 PM

to Danilo Tedeschi, Greg Thompson, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, pdf-r...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, jshin...@chromium.org

Attention needed from Danilo Tedeschi

Andy Phan voted and added 2 comments

Votes added by Andy Phan

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 11 (Latest):

Andy Phan . resolved

//pdf LGTM

Commit Message

Line 28, Patchset 11 (Latest): * Migrated pdf/document_metadata.h and pdf/pdfium/pdfium_document_metadata.cc.

Andy Phan . unresolved

nit: wrap at 72 columns.

Open in Gerrit

Related details

Attention is currently required from:

Danilo Tedeschi

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

Danilo Tedeschi (Gerrit)

unread,

3:00 PM (6 hours ago) 3:00 PM

to Andy Phan, Greg Thompson, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, pdf-r...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, jshin...@chromium.org

Danilo Tedeschi voted and added 1 comment

Votes added by Danilo Tedeschi

Commit-Queue

+1

1 comment

Commit Message

Line 28, Patchset 11: * Migrated pdf/document_metadata.h and pdf/pdfium/pdfium_document_metadata.cc.

Andy Phan . resolved

nit: wrap at 72 columns.

Danilo Tedeschi

Done

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.

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

3:07 PM (6 hours ago) 3:07 PM

to Danilo Tedeschi, Andy Phan, Greg Thompson, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, pdf-r...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, jshin...@chromium.org

Chromium LUCI CQ submitted the change

Unreviewed changes

11 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.

Change information

Commit message:

Relocate LanguageTag classes and constants to base::i18n

This CL performs a major namespace reorganization for language tag
utilities in base/i18n and updates corresponding call sites in the
PDFcomponent.

Key Changes:


   * Core Classes: Moved LanguageTag, LanguageTagConverter,
and LanguageTagMatcher from base to ::base::i18n.

   * BCP47 Extensions:
       * Moved Extension, UnicodeExtension, and
PrivateUseSubtags to base::i18n.
       * Moved extension traits and helper functions
(e.g., unicode(), priv(), ext<c>()) to base::i18n::bcp47_extensions.
   * Constants: Moved predefined language tag constants
(e.g., ENGLISH(), CHINESE_CHINA()) to base::i18n::language_tags.
   * Renaming: Renamed GetKnownTag to GetKnownLanguageTag
for improved clarity and left it in ::base::i18n.

Call Site Updates:
   * Updated all internal references within base/i18n/


   * Migrated pdf/document_metadata.h and
pdf/pdfium/pdfium_document_metadata.cc.

   * Updated all unit tests to use fully qualified names

Change-Id: I3817be26ee0db5ee34f49f5b8a36fa9a70f704fd

Bug: 506198296

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7997302

Reviewed-by: Greg Thompson <g...@chromium.org>

Reviewed-by: Andy Phan <andy...@chromium.org>

Commit-Queue: Danilo Tedeschi <da...@google.com>

Cr-Commit-Position: refs/heads/main@{#1652603}

Files:

M base/i18n/bcp47_extensions.cc
M base/i18n/bcp47_extensions.h
M base/i18n/bcp47_extensions_unittest.cc
M base/i18n/language_tag.cc
M base/i18n/language_tag.h
M base/i18n/language_tag_matcher.cc
M base/i18n/language_tag_matcher.h
M base/i18n/language_tag_matcher_unittest.cc
M base/i18n/language_tag_unittest.cc
M base/i18n/tag_converters.cc
M base/i18n/tag_converters.h
M base/i18n/tags.cc
M base/i18n/tags.h
M base/i18n/timezone.cc
M base/i18n/timezone.h
M pdf/document_metadata.h
M pdf/pdfium/pdfium_document_metadata.cc
M pdf/pdfium/pdfium_engine_unittest.cc

Change size: M

Delta: 18 files changed, 122 insertions(+), 117 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Andy Phan, +1 by Greg Thompson

Open in Gerrit

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

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

Relocate LanguageTag classes and constants to base::i18n [chromium/src : main]

Danilo Tedeschi (Gerrit)

Danilo Tedeschi voted Commit-Queue+1

Related details

Greg Thompson (Gerrit)

Greg Thompson voted and added 4 comments

Votes added by Greg Thompson

4 comments

Related details

Danilo Tedeschi (Gerrit)

Danilo Tedeschi added 4 comments

Related details

Andy Phan (Gerrit)

Andy Phan voted and added 2 comments

Votes added by Andy Phan

2 comments

Related details

Danilo Tedeschi (Gerrit)

Danilo Tedeschi voted and added 1 comment

Votes added by Danilo Tedeschi

1 comment

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Unreviewed changes

Change information