Record font fallback metrics by script in GWS PLMO [chromium/src : main]

0 views
Skip to first unread message

Keita Suzuki (Gerrit)

unread,
Jun 17, 2026, 8:46:19 PM (13 days ago) Jun 17
to Stephen Chenney, chromium...@chromium.org, Dirk Schulze, Chromium Metrics Reviews, asvitkine...@chromium.org, kinuko...@chromium.org, bmcquad...@chromium.org, csharris...@chromium.org, loading-rev...@chromium.org, fmalit...@chromium.org, ipc-securi...@chromium.org, speed-metr...@chromium.org, speed-metrics...@chromium.org, drott+bl...@chromium.org, core-timi...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, fserb...@chromium.org, blink-reviews-p...@chromium.org

Keita Suzuki has uploaded the change for review

Commit message

Record font fallback metrics by script in GWS PLMO

[DSEPrewarm] Record font fallback metrics by script in GWS PLMO

This CL extends the font loading performance metrics to report fallback
count and duration grouped by Unicode script.

Specifically:
- We track fallback count and duration per UScriptCode in FontPerformance.
- We plumb this script-specific data through WebPerformanceMetricsForReporting
and the Page Load Metrics Mojo pipeline (FontLoadingMetrics).
- In GWSPageLoadMetricsObserver, we log the new metrics for FCP, AFTEnd, and
Complete milestones:
- PageLoad.Clients.GoogleSearch.FontLoading.FallbackCountByScript.<Script>.<Milestone>
- PageLoad.Clients.GoogleSearch.FontLoading.FallbackDurationByScript.<Script>.<Milestone>
Supported scripts are Latin, Han, Hangul, Hiragana, Katakana, Arabic,
Bengali, Devanagari, Cyrillic, and others are grouped under "Other".

Bug: 517003594

TAG=agy
CONV=75c1922d-ce5a-4662-bba8-8ef1f1050292
Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55

Change diff


Change information

Files:
  • M chrome/browser/page_load_metrics/observers/gws_page_load_metrics_observer_unittest.cc
  • M components/page_load_metrics/common/page_load_metrics.mojom
  • M components/page_load_metrics/google/browser/gws_page_load_metrics_observer.cc
  • M components/page_load_metrics/renderer/metrics_render_frame_observer.cc
  • M third_party/blink/public/web/web_performance_metrics_for_reporting.h
  • M third_party/blink/renderer/core/exported/web_performance_metrics_for_reporting.cc
  • M third_party/blink/renderer/core/timing/performance_timing_for_reporting.cc
  • M third_party/blink/renderer/core/timing/performance_timing_for_reporting.h
  • M third_party/blink/renderer/platform/fonts/font_cache.cc
  • M third_party/blink/renderer/platform/fonts/font_performance.cc
  • M third_party/blink/renderer/platform/fonts/font_performance.h
  • M tools/metrics/histograms/metadata/page/histograms.xml
Change size: L
Delta: 12 files changed, 322 insertions(+), 20 deletions(-)
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newchange
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
Gerrit-Change-Number: 7960697
Gerrit-PatchSet: 1
Gerrit-Owner: Keita Suzuki <suzuk...@chromium.org>
Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
Gerrit-CC: Stephen Chenney <sche...@chromium.org>
satisfied_requirement
unsatisfied_requirement
open
diffy

Keita Suzuki (Gerrit)

unread,
Jun 17, 2026, 8:50:57 PM (13 days ago) Jun 17
to android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, asvitkine...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, bmcquad...@chromium.org, core-timi...@chromium.org, csharris...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org

Keita Suzuki added 4 comments

File components/page_load_metrics/common/page_load_metrics.mojom
Line 190, Patchset 1 (Latest):struct ScriptFallbackInfo {
Keita Suzuki . unresolved

For each struct member please add comments.

File components/page_load_metrics/google/browser/gws_page_load_metrics_observer.cc
Line 452, Patchset 1 (Latest):std::string_view GetScriptSuffix(int32_t script_code) {
Keita Suzuki . unresolved

instead of adding suffix, can we simply record a enumerated histogram for all the scripts?

File third_party/blink/public/web/web_performance_metrics_for_reporting.h
Line 48, Patchset 1 (Latest):struct ScriptFontFallbackDetailsForReporting {
Keita Suzuki . unresolved

also add comments here to each member.

File third_party/blink/renderer/platform/fonts/font_performance.h
Line 29, Patchset 1 (Latest): static void Reset();
Keita Suzuki . unresolved

why was this change required?

Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
    Gerrit-Change-Number: 7960697
    Gerrit-PatchSet: 1
    Gerrit-Owner: Keita Suzuki <suzuk...@chromium.org>
    Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
    Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
    Gerrit-CC: Stephen Chenney <sche...@chromium.org>
    Gerrit-Comment-Date: Thu, 18 Jun 2026 00:50:25 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Keita Suzuki (Gerrit)

    unread,
    Jun 17, 2026, 11:23:32 PM (13 days ago) Jun 17
    to android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, asvitkine...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, bmcquad...@chromium.org, core-timi...@chromium.org, csharris...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org

    Keita Suzuki added 4 comments

    File third_party/blink/public/web/web_performance_metrics_for_reporting.h
    Line 48, Patchset 1:struct ScriptFontFallbackDetailsForReporting {
    Keita Suzuki . resolved

    also add comments here to each member.

    Keita Suzuki

    Done

    File third_party/blink/renderer/platform/fonts/font_performance.h
    Line 8, Patchset 5 (Latest):#include <unicode/uscript.h>
    Keita Suzuki . unresolved

    You don't need to include `<unicode/uscript.h>` here since you only use `int32_t` for the script code in this header. Removing it can help improve compilation times.

    Line 29, Patchset 1: static void Reset();
    Keita Suzuki . resolved

    why was this change required?

    Keita Suzuki

    Done

    File third_party/blink/renderer/platform/fonts/font_performance.cc
    Line 44, Patchset 5 (Latest): base::TimeDelta time) {
    Keita Suzuki . unresolved

    The commit message mentions tracking "fallback count and duration grouped by Unicode script", and logging `PageLoad.Clients.GoogleSearch.FontLoading.FallbackDurationByScript.<Script>.<Milestone>`. However, `time` is not used in the script-specific map here and the duration histogram isn't added.

    Should the duration also be tracked as initially intended, or should the commit message be updated to reflect that only counts are tracked?

    Open in Gerrit

    Related details

    Attention set is empty
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
    Gerrit-Change-Number: 7960697
    Gerrit-PatchSet: 5
    Gerrit-Owner: Keita Suzuki <suzuk...@chromium.org>
    Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
    Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
    Gerrit-CC: Stephen Chenney <sche...@chromium.org>
    Gerrit-Comment-Date: Thu, 18 Jun 2026 03:22:57 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Keita Suzuki <suzuk...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Keita Suzuki (Gerrit)

    unread,
    Jun 18, 2026, 12:40:31 AM (13 days ago) Jun 18
    to android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, asvitkine...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, bmcquad...@chromium.org, core-timi...@chromium.org, csharris...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org

    Keita Suzuki added 2 comments

    File third_party/blink/renderer/platform/fonts/font_performance.h
    Line 8, Patchset 5:#include <unicode/uscript.h>
    Keita Suzuki . resolved

    You don't need to include `<unicode/uscript.h>` here since you only use `int32_t` for the script code in this header. Removing it can help improve compilation times.

    Keita Suzuki

    Done

    File third_party/blink/renderer/platform/fonts/font_performance.cc
    Line 44, Patchset 5: base::TimeDelta time) {
    Keita Suzuki . resolved

    The commit message mentions tracking "fallback count and duration grouped by Unicode script", and logging `PageLoad.Clients.GoogleSearch.FontLoading.FallbackDurationByScript.<Script>.<Milestone>`. However, `time` is not used in the script-specific map here and the duration histogram isn't added.

    Should the duration also be tracked as initially intended, or should the commit message be updated to reflect that only counts are tracked?

    Keita Suzuki

    Done

    Open in Gerrit

    Related details

    Attention set is empty
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
    Gerrit-Change-Number: 7960697
    Gerrit-PatchSet: 9
    Gerrit-Owner: Keita Suzuki <suzuk...@chromium.org>
    Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
    Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
    Gerrit-CC: Stephen Chenney <sche...@chromium.org>
    Gerrit-Comment-Date: Thu, 18 Jun 2026 04:40:02 +0000
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Keita Suzuki (Gerrit)

    unread,
    Jun 18, 2026, 12:40:50 AM (13 days ago) Jun 18
    to android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, asvitkine...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, bmcquad...@chromium.org, core-timi...@chromium.org, csharris...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org

    Keita Suzuki added 2 comments

    File components/page_load_metrics/common/page_load_metrics.mojom
    Line 190, Patchset 1:struct ScriptFallbackInfo {
    Keita Suzuki . resolved

    For each struct member please add comments.

    Keita Suzuki

    Done

    File components/page_load_metrics/google/browser/gws_page_load_metrics_observer.cc
    Line 452, Patchset 1:std::string_view GetScriptSuffix(int32_t script_code) {
    Keita Suzuki . resolved

    instead of adding suffix, can we simply record a enumerated histogram for all the scripts?

    Keita Suzuki

    Done

    Open in Gerrit

    Related details

    Attention set is empty
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not satisfiedCode-Owners
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
      Gerrit-Change-Number: 7960697
      Gerrit-PatchSet: 9
      Gerrit-Owner: Keita Suzuki <suzuk...@chromium.org>
      Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
      Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
      Gerrit-CC: Stephen Chenney <sche...@chromium.org>
      Gerrit-Comment-Date: Thu, 18 Jun 2026 04:40:26 +0000
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Keita Suzuki (Gerrit)

      unread,
      Jun 18, 2026, 3:59:24 AM (13 days ago) Jun 18
      to android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, asvitkine...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, bmcquad...@chromium.org, core-timi...@chromium.org, csharris...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org

      Keita Suzuki added 3 comments

      File components/page_load_metrics/google/browser/gws_page_load_metrics_observer.cc
      Line 477, Patchset 13 (Latest):}
      Keita Suzuki . unresolved

      If `type` doesn't match any case (e.g., an invalid enum value passed via IPC from a compromised renderer), control will fall off the end of this function without returning a value, resulting in Undefined Behavior.

      Please add `NOTREACHED();` or return a default string (e.g., `"Other"`) at the end of the function.

      File components/page_load_metrics/renderer/metrics_render_frame_observer.cc
      Line 925, Patchset 13 (Latest): for (const auto& details : perf.GetScriptFontFallbackDetails()) {
      Keita Suzuki . unresolved

      Multiple `details` entries can map to the same `mojom::ScriptType`. For example, multiple unlisted scripts will map to `kOther`, and different entries with `is_emoji == true` will map to `kEmoji`.

      Because `RecordFontMetrics` in the browser logs exactly one sample per item in this array, pushing them separately will cause the browser to record multiple independent histogram samples for the same script in a single page load.

      You should aggregate `details.fallback_count` by `mojom::ScriptType` before populating `font_metrics->script_fallback_metrics`. For example:

      ```cpp
      std::map<mojom::ScriptType, uint32_t> aggregated;
      for (const auto& details : perf.GetScriptFontFallbackDetails()) {
      aggregated[MapToMojoScriptType(details.script_code, details.is_emoji)] +=
      details.fallback_count;
      }
      for (const auto& [type, count] : aggregated) {
      auto info = mojom::ScriptFallbackInfo::New();
      info->script_type = type;
      info->fallback_count = count;
      font_metrics->script_fallback_metrics.push_back(std::move(info));
      }
      ```
      File third_party/blink/renderer/platform/fonts/font_cache.cc
      Line 230, Patchset 13 (Latest): FontPerformance::AddSystemFallbackFontTime(script_code, is_emoji,
      Keita Suzuki . unresolved

      `timer.Elapsed()` evaluates as an argument at the end of this block, which means the time spent querying ICU (`uscript_getScript`) will be included in the measurement.

      You should capture the elapsed time immediately after `PlatformFallbackFontForCharacter()` to avoid artificially inflating the fallback duration metric.

      ```cpp
      base::TimeDelta elapsed = timer.Elapsed();
      int32_t script_code = USCRIPT_INVALID_CODE;
      // ...
      FontPerformance::AddSystemFallbackFontTime(script_code, is_emoji, elapsed);
      ```
      Open in Gerrit

      Related details

      Attention set is empty
      Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement is not satisfiedCode-Owners
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • requirement is not satisfiedReview-Enforcement
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
        Gerrit-Change-Number: 7960697
        Gerrit-PatchSet: 13
        Gerrit-Owner: Keita Suzuki <suzuk...@chromium.org>
        Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
        Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
        Gerrit-CC: Stephen Chenney <sche...@chromium.org>
        Gerrit-Comment-Date: Thu, 18 Jun 2026 07:58:52 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Keita Suzuki (Gerrit)

        unread,
        Jun 18, 2026, 4:26:26 AM (13 days ago) Jun 18
        to android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, asvitkine...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, bmcquad...@chromium.org, core-timi...@chromium.org, csharris...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org

        Keita Suzuki added 3 comments

        File components/page_load_metrics/google/browser/gws_page_load_metrics_observer.cc
        Line 477, Patchset 13:}
        Keita Suzuki . resolved

        If `type` doesn't match any case (e.g., an invalid enum value passed via IPC from a compromised renderer), control will fall off the end of this function without returning a value, resulting in Undefined Behavior.

        Please add `NOTREACHED();` or return a default string (e.g., `"Other"`) at the end of the function.

        Keita Suzuki

        Done

        File components/page_load_metrics/renderer/metrics_render_frame_observer.cc
        Line 925, Patchset 13: for (const auto& details : perf.GetScriptFontFallbackDetails()) {
        Keita Suzuki . resolved

        Multiple `details` entries can map to the same `mojom::ScriptType`. For example, multiple unlisted scripts will map to `kOther`, and different entries with `is_emoji == true` will map to `kEmoji`.

        Because `RecordFontMetrics` in the browser logs exactly one sample per item in this array, pushing them separately will cause the browser to record multiple independent histogram samples for the same script in a single page load.

        You should aggregate `details.fallback_count` by `mojom::ScriptType` before populating `font_metrics->script_fallback_metrics`. For example:

        ```cpp
        std::map<mojom::ScriptType, uint32_t> aggregated;
        for (const auto& details : perf.GetScriptFontFallbackDetails()) {
        aggregated[MapToMojoScriptType(details.script_code, details.is_emoji)] +=
        details.fallback_count;
        }
        for (const auto& [type, count] : aggregated) {
        auto info = mojom::ScriptFallbackInfo::New();
        info->script_type = type;
        info->fallback_count = count;
        font_metrics->script_fallback_metrics.push_back(std::move(info));
        }
        ```
        Keita Suzuki

        Done

        File third_party/blink/renderer/platform/fonts/font_cache.cc
        Line 230, Patchset 13: FontPerformance::AddSystemFallbackFontTime(script_code, is_emoji,
        Keita Suzuki . resolved

        `timer.Elapsed()` evaluates as an argument at the end of this block, which means the time spent querying ICU (`uscript_getScript`) will be included in the measurement.

        You should capture the elapsed time immediately after `PlatformFallbackFontForCharacter()` to avoid artificially inflating the fallback duration metric.

        ```cpp
        base::TimeDelta elapsed = timer.Elapsed();
        int32_t script_code = USCRIPT_INVALID_CODE;
        // ...
        FontPerformance::AddSystemFallbackFontTime(script_code, is_emoji, elapsed);
        ```
        Keita Suzuki

        Done

        Open in Gerrit

        Related details

        Attention set is empty
        Submit Requirements:
          • requirement satisfiedCode-Coverage
          • requirement is not satisfiedCode-Owners
          • requirement is not satisfiedCode-Review
          • requirement is not satisfiedReview-Enforcement
          Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
          Gerrit-MessageType: comment
          Gerrit-Project: chromium/src
          Gerrit-Branch: main
          Gerrit-Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
          Gerrit-Change-Number: 7960697
          Gerrit-PatchSet: 15
          Gerrit-Owner: Keita Suzuki <suzuk...@chromium.org>
          Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
          Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
          Gerrit-CC: Stephen Chenney <sche...@chromium.org>
          Gerrit-Comment-Date: Thu, 18 Jun 2026 08:26:02 +0000
          Gerrit-HasComments: Yes
          Gerrit-Has-Labels: No
          Comment-In-Reply-To: Keita Suzuki <suzuk...@chromium.org>
          satisfied_requirement
          unsatisfied_requirement
          open
          diffy

          Keita Suzuki (Gerrit)

          unread,
          Jun 18, 2026, 4:58:28 AM (13 days ago) Jun 18
          to Takashi Toyoshima, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, asvitkine...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, bmcquad...@chromium.org, core-timi...@chromium.org, csharris...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
          Attention needed from Takashi Toyoshima

          Keita Suzuki voted and added 1 comment

          Votes added by Keita Suzuki

          Commit-Queue+1

          1 comment

          Patchset-level comments
          File-level comment, Patchset 18 (Latest):
          Keita Suzuki . resolved

          PTAL. THanks!

          Open in Gerrit

          Related details

          Attention is currently required from:
          • Takashi Toyoshima
          Submit Requirements:
          • requirement satisfiedCode-Coverage
          • requirement is not satisfiedCode-Owners
          • requirement is not satisfiedCode-Review
          • requirement is not satisfiedReview-Enforcement
          Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
          Gerrit-MessageType: comment
          Gerrit-Project: chromium/src
          Gerrit-Branch: main
          Gerrit-Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
          Gerrit-Change-Number: 7960697
          Gerrit-PatchSet: 18
          Gerrit-Owner: Keita Suzuki <suzuk...@chromium.org>
          Gerrit-Reviewer: Keita Suzuki <suzuk...@chromium.org>
          Gerrit-Reviewer: Takashi Toyoshima <toyo...@chromium.org>
          Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
          Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
          Gerrit-CC: Stephen Chenney <sche...@chromium.org>
          Gerrit-Attention: Takashi Toyoshima <toyo...@chromium.org>
          Gerrit-Comment-Date: Thu, 18 Jun 2026 08:57:50 +0000
          Gerrit-HasComments: Yes
          Gerrit-Has-Labels: Yes
          satisfied_requirement
          unsatisfied_requirement
          open
          diffy

          Takashi Toyoshima (Gerrit)

          unread,
          Jun 22, 2026, 7:08:02 PM (8 days ago) Jun 22
          to Keita Suzuki, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, asvitkine...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, bmcquad...@chromium.org, core-timi...@chromium.org, csharris...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
          Attention needed from Keita Suzuki

          Takashi Toyoshima added 14 comments

          File chrome/browser/page_load_metrics/observers/gws_page_load_metrics_observer_unittest.cc
          Line 5, Patchset 19 (Latest):#include <unicode/uscript.h>
          Takashi Toyoshima . unresolved

          I think this is `#include "third_party/icu/source/common/unicode/uscript.h"` though this still can pass compiling.
          But you may not need this in this file?

          Line 703, Patchset 19 (Latest): font_loading_metrics->fallback_duration = base::Milliseconds(150);
          font_loading_metrics->fallback_count = 14;
          font_loading_metrics->fallback_initial_duration = base::Milliseconds(42);
          font_loading_metrics->shape_cache_hit_count = 80;
          font_loading_metrics->shape_cache_miss_count = 20;
          Takashi Toyoshima . unresolved

          We initialize several fields with expected values here, and check them below after line 746. So, can we use const variables for them to set here and use them as expected values below so that we should not make a mistake?

          File components/page_load_metrics/common/page_load_metrics.mojom
          Line 191, Patchset 19 (Latest):// This is mapped from Blink's internal UScriptCode or fallback priority.
          Takashi Toyoshima . unresolved

          Can we use `LINT.IfChange` here? The other size is in the histograms.xml

          File components/page_load_metrics/renderer/metrics_render_frame_observer.cc
          Line 7, Patchset 19 (Latest):#include <unicode/uscript.h>
          Takashi Toyoshima . unresolved

          ditto

          Line 53, Patchset 19 (Latest):mojom::ScriptType MapToMojoScriptType(int32_t script_code, bool is_emoji) {
          Takashi Toyoshima . unresolved

          Keep using UScriptCode here and there.

          File third_party/blink/public/web/web_performance_metrics_for_reporting.h
          Line 48, Patchset 19 (Latest):struct ScriptFontFallbackDetailsForReporting {
          Takashi Toyoshima . unresolved

          Can we keep UScriptCode and size_t?
          See other relevant comments below.

          File third_party/blink/renderer/platform/fonts/font_cache.cc
          Line 32, Patchset 19 (Latest):#include <unicode/uscript.h>
          Takashi Toyoshima . unresolved

          ditto

          Line 224, Patchset 19 (Latest): int32_t script_code = USCRIPT_INVALID_CODE;
          bool is_emoji = IsNonTextFallbackPriority(fallback_priority);
          UErrorCode err = U_ZERO_ERROR;
          UScriptCode script = uscript_getScript(lookup_char, &err);
          if (U_SUCCESS(err)) {
          script_code = static_cast<int32_t>(script);
          }
          Takashi Toyoshima . unresolved
          Let's keep the original type as we can as possible while detecting.
          ```
          bool is_emoji = IsNonTextFallbackPriority(fallback_priority);
          UErrorCode err = U_ZERO_ERROR;
          UScriptCode script = uscript_getScript(lookup_char, &err);
          if (U_FAILURE(err)) {
          script = USCRIPT_INVALID_CODE;
          }
          ```
          File third_party/blink/renderer/platform/fonts/font_performance.h
          Line 70, Patchset 19 (Latest): static const std::map<ScriptKey, uint32_t>& GetScriptFallbackCounts();
          Takashi Toyoshima . unresolved

          Using `size_t` can clarify the meaning as this is a counter.

          Line 67, Patchset 19 (Latest): static void AddSystemFallbackFontTime(int32_t script_code,
          Takashi Toyoshima . unresolved

          Can we still use UScriptCode?

          Line 26, Patchset 19 (Latest): bool operator<(const ScriptKey& other) const {
          return std::tie(script, is_emoji) <
          std::tie(other.script, other.is_emoji);
          }
          Takashi Toyoshima . unresolved
          You can skip this manual code by using the following code in C++20.
          ```
          friend auto operator<=>(const ScriptKey&, const ScriptKey&) = default;
          ```

          ```#include <compare>``` may be needed.

          Line 24, Patchset 19 (Latest): int32_t script;
          Takashi Toyoshima . unresolved

          keep UScriptCode

          File third_party/blink/renderer/platform/fonts/font_performance.cc
          Line 45, Patchset 19 (Latest): if (!IsMainThread()) [[unlikely]] {
          Takashi Toyoshima . unresolved

          Just in case, is this the expected case?
          If so, you can add some comments, and why it's ok to skip non-main thread use cases.

          File tools/metrics/histograms/metadata/page/histograms.xml
          Line 76, Patchset 19 (Latest):
          Takashi Toyoshima . unresolved

          LINT.IfChange here

          Open in Gerrit

          Related details

          Attention is currently required from:
          • Keita Suzuki
          Submit Requirements:
            • requirement satisfiedCode-Coverage
            • requirement is not satisfiedCode-Owners
            • requirement is not satisfiedCode-Review
            • requirement is not satisfiedNo-Unresolved-Comments
            • requirement is not satisfiedReview-Enforcement
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: chromium/src
            Gerrit-Branch: main
            Gerrit-Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
            Gerrit-Change-Number: 7960697
            Gerrit-PatchSet: 19
            Gerrit-Owner: Keita Suzuki <suzuk...@chromium.org>
            Gerrit-Reviewer: Keita Suzuki <suzuk...@chromium.org>
            Gerrit-Reviewer: Takashi Toyoshima <toyo...@chromium.org>
            Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
            Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
            Gerrit-CC: Stephen Chenney <sche...@chromium.org>
            Gerrit-Attention: Keita Suzuki <suzuk...@chromium.org>
            Gerrit-Comment-Date: Mon, 22 Jun 2026 23:07:26 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: No
            satisfied_requirement
            unsatisfied_requirement
            open
            diffy

            Takashi Toyoshima (Gerrit)

            unread,
            Jun 22, 2026, 7:15:56 PM (8 days ago) Jun 22
            to Keita Suzuki, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, asvitkine...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, bmcquad...@chromium.org, core-timi...@chromium.org, csharris...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
            Attention needed from Keita Suzuki

            Takashi Toyoshima added 2 comments

            File components/page_load_metrics/common/page_load_metrics.mojom
            Line 191, Patchset 19 (Latest):// This is mapped from Blink's internal UScriptCode or fallback priority.
            Takashi Toyoshima . unresolved

            Also, can we say this is based on ICU's UScriptType, and how you modified it with what kind of intentions.

            File third_party/blink/renderer/platform/fonts/font_performance.h
            Line 32, Patchset 19 (Latest): static void Reset() {
            Takashi Toyoshima . unresolved

            maybe CHECK(IsMainThread()); and also other methods need the check?

            Gerrit-Comment-Date: Mon, 22 Jun 2026 23:15:28 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: No
            satisfied_requirement
            unsatisfied_requirement
            open
            diffy

            Takashi Toyoshima (Gerrit)

            unread,
            Jun 23, 2026, 2:59:38 AM (8 days ago) Jun 23
            to Keita Suzuki, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, asvitkine...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, bmcquad...@chromium.org, core-timi...@chromium.org, csharris...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
            Attention needed from Keita Suzuki

            Takashi Toyoshima added 11 comments

            File components/page_load_metrics/google/browser/gws_page_load_metrics_observer.cc
            Line 450, Patchset 20 (Latest):}
            Takashi Toyoshima . unresolved

            insert an empty line

            File third_party/blink/public/web/web_performance_metrics_for_reporting.h
            Line 8, Patchset 20 (Latest):#include <unicode/uscript.h>
            Takashi Toyoshima . unresolved

            #include "third_party/icu/source/common/unicode/uscript.h"
            Sorry, I might have missed this.

            Line 48, Patchset 19:struct ScriptFontFallbackDetailsForReporting {
            Takashi Toyoshima . unresolved

            Can we keep UScriptCode and size_t?
            See other relevant comments below.

            Takashi Toyoshima

            maybe resolved?

            File third_party/blink/renderer/platform/fonts/font_cache.cc
            Line 224, Patchset 19: int32_t script_code = USCRIPT_INVALID_CODE;

            bool is_emoji = IsNonTextFallbackPriority(fallback_priority);
            UErrorCode err = U_ZERO_ERROR;
            UScriptCode script = uscript_getScript(lookup_char, &err);
            if (U_SUCCESS(err)) {
            script_code = static_cast<int32_t>(script);
            }
            Takashi Toyoshima . unresolved
            Let's keep the original type as we can as possible while detecting.
            ```
            bool is_emoji = IsNonTextFallbackPriority(fallback_priority);
            UErrorCode err = U_ZERO_ERROR;
            UScriptCode script = uscript_getScript(lookup_char, &err);
            if (U_FAILURE(err)) {
            script = USCRIPT_INVALID_CODE;
            }
            ```
            Takashi Toyoshima

            resolved?

            File third_party/blink/renderer/platform/fonts/font_performance.h
            Line 8, Patchset 20 (Latest):#include <unicode/uscript.h>
            Takashi Toyoshima . unresolved

            #include "third_party/icu/source/common/unicode/uscript.h"

            Line 70, Patchset 19: static const std::map<ScriptKey, uint32_t>& GetScriptFallbackCounts();
            Takashi Toyoshima . unresolved

            Using `size_t` can clarify the meaning as this is a counter.

            Takashi Toyoshima

            resolved?

            Line 67, Patchset 19: static void AddSystemFallbackFontTime(int32_t script_code,
            Takashi Toyoshima . unresolved

            Can we still use UScriptCode?

            Takashi Toyoshima

            resolved?

            Line 26, Patchset 19: bool operator<(const ScriptKey& other) const {

            return std::tie(script, is_emoji) <
            std::tie(other.script, other.is_emoji);
            }
            Takashi Toyoshima . unresolved
            You can skip this manual code by using the following code in C++20.
            ```
            friend auto operator<=>(const ScriptKey&, const ScriptKey&) = default;
            ```

            ```#include <compare>``` may be needed.

            Takashi Toyoshima

            resolved?

            Line 24, Patchset 19: int32_t script;
            Takashi Toyoshima . unresolved

            keep UScriptCode

            Takashi Toyoshima

            resolved?

            File third_party/blink/renderer/platform/fonts/font_performance.cc
            Line 45, Patchset 19: if (!IsMainThread()) [[unlikely]] {
            Takashi Toyoshima . resolved

            Just in case, is this the expected case?
            If so, you can add some comments, and why it's ok to skip non-main thread use cases.

            Takashi Toyoshima

            I will drop this comment as this is just consistent with other methods.

            File tools/metrics/histograms/metadata/page/histograms.xml
            Takashi Toyoshima . unresolved

            LINT.IfChange here

            Takashi Toyoshima

            resolved?

            Open in Gerrit

            Related details

            Attention is currently required from:
            • Keita Suzuki
            Submit Requirements:
            • requirement satisfiedCode-Coverage
            • requirement is not satisfiedCode-Owners
            • requirement is not satisfiedCode-Review
            • requirement is not satisfiedNo-Unresolved-Comments
            • requirement is not satisfiedReview-Enforcement
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: chromium/src
            Gerrit-Branch: main
            Gerrit-Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
            Gerrit-Change-Number: 7960697
            Gerrit-PatchSet: 20
            Gerrit-Owner: Keita Suzuki <suzuk...@chromium.org>
            Gerrit-Reviewer: Keita Suzuki <suzuk...@chromium.org>
            Gerrit-Reviewer: Takashi Toyoshima <toyo...@chromium.org>
            Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
            Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
            Gerrit-CC: Stephen Chenney <sche...@chromium.org>
            Gerrit-Attention: Keita Suzuki <suzuk...@chromium.org>
            Gerrit-Comment-Date: Tue, 23 Jun 2026 06:59:02 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: No
            Comment-In-Reply-To: Takashi Toyoshima <toyo...@chromium.org>
            satisfied_requirement
            unsatisfied_requirement
            open
            diffy

            Keita Suzuki (Gerrit)

            unread,
            Jun 23, 2026, 4:19:44 AM (8 days ago) Jun 23
            to Takashi Toyoshima, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, asvitkine...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, bmcquad...@chromium.org, core-timi...@chromium.org, csharris...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
            Attention needed from Takashi Toyoshima

            Keita Suzuki voted and added 19 comments

            Votes added by Keita Suzuki

            Commit-Queue+1

            19 comments

            Patchset-level comments
            File-level comment, Patchset 21 (Latest):
            Keita Suzuki . resolved

            Sorry! I was discussing a couple of things with AI agent and some how the comments slipped off. Updated!

            File chrome/browser/page_load_metrics/observers/gws_page_load_metrics_observer_unittest.cc
            Line 5, Patchset 19:#include <unicode/uscript.h>
            Takashi Toyoshima . resolved

            I think this is `#include "third_party/icu/source/common/unicode/uscript.h"` though this still can pass compiling.
            But you may not need this in this file?

            Keita Suzuki

            Done. Since we refactored the test to use the Mojo ScriptType enum instead of ICU's UScriptCode constants, this include is indeed no longer needed. I have removed it entirely.

            Line 703, Patchset 19: font_loading_metrics->fallback_duration = base::Milliseconds(150);

            font_loading_metrics->fallback_count = 14;
            font_loading_metrics->fallback_initial_duration = base::Milliseconds(42);
            font_loading_metrics->shape_cache_hit_count = 80;
            font_loading_metrics->shape_cache_miss_count = 20;
            Takashi Toyoshima . resolved

            We initialize several fields with expected values here, and check them below after line 746. So, can we use const variables for them to set here and use them as expected values below so that we should not make a mistake?

            Keita Suzuki

            Done. Defined constexpr constants for all simulated/expected values (e.g., kFallbackDuration, kFallbackCount, kLatinFallbackCount, etc.) at the beginning of the test case and used them for both setup and verification.

            File components/page_load_metrics/common/page_load_metrics.mojom
            Line 191, Patchset 19:// This is mapped from Blink's internal UScriptCode or fallback priority.
            Takashi Toyoshima . resolved

            Can we use `LINT.IfChange` here? The other size is in the histograms.xml

            Keita Suzuki

            Done. Added LINT.IfChange here and a corresponding LINT.ThenChange in histograms.xml around the GWSFontFallbackScript variants.

            Line 191, Patchset 19:// This is mapped from Blink's internal UScriptCode or fallback priority.
            Takashi Toyoshima . resolved

            Also, can we say this is based on ICU's UScriptType, and how you modified it with what kind of intentions.

            Keita Suzuki

            Done. Expanded the comment to clarify that this is a curated subset of ICU's UScriptCode (plus kEmoji) designed specifically for tracking high-impact font fallback categories in UMA.

            File components/page_load_metrics/google/browser/gws_page_load_metrics_observer.cc
            Line 450, Patchset 20:}
            Takashi Toyoshima . resolved

            insert an empty line

            Keita Suzuki

            Done. Inserted an empty line.

            File components/page_load_metrics/renderer/metrics_render_frame_observer.cc
            Line 7, Patchset 19:#include <unicode/uscript.h>
            Takashi Toyoshima . resolved

            ditto

            Keita Suzuki

            Done. Updated the include to the full path: #include "third_party/icu/source/common/unicode/uscript.h".

            Line 53, Patchset 19:mojom::ScriptType MapToMojoScriptType(int32_t script_code, bool is_emoji) {
            Takashi Toyoshima . resolved

            Keep using UScriptCode here and there.

            Keita Suzuki

            Done. Updated MapToMojoScriptType to take UScriptCode directly, avoiding the early cast to int32_t.

            File third_party/blink/public/web/web_performance_metrics_for_reporting.h
            Line 8, Patchset 20:#include <unicode/uscript.h>
            Takashi Toyoshima . unresolved

            #include "third_party/icu/source/common/unicode/uscript.h"
            Sorry, I might have missed this.

            Keita Suzuki

            Unfortunately, it seems like including "third_party/icu/source/common/unicode/uscript.h" violates Blink's checkdeps rules, which forbid direct dependencies on that directory from Blink. We must use the short <unicode/uscript.h> here.

            ```
            You added one or more #includes that violate checkdeps rules.
            third_party/blink/public/web/web_performance_metrics_for_reporting.h
            Illegal include: "third_party/icu/source/common/unicode/uscript.h"
            Because of "-third_party/icu/source/common/unicode" from third_party's include_rules.
            ```
            Line 48, Patchset 19:struct ScriptFontFallbackDetailsForReporting {
            Takashi Toyoshima . resolved

            Can we keep UScriptCode and size_t?
            See other relevant comments below.

            Keita Suzuki

            Done. Included <unicode/uscript.h> and updated ScriptFontFallbackDetailsForReporting to use UScriptCode for script_code and size_t for fallback_count.

            File third_party/blink/renderer/platform/fonts/font_cache.cc
            Line 224, Patchset 19: int32_t script_code = USCRIPT_INVALID_CODE;
            bool is_emoji = IsNonTextFallbackPriority(fallback_priority);
            UErrorCode err = U_ZERO_ERROR;
            UScriptCode script = uscript_getScript(lookup_char, &err);
            if (U_SUCCESS(err)) {
            script_code = static_cast<int32_t>(script);
            }
            Takashi Toyoshima . resolved
            Let's keep the original type as we can as possible while detecting.
            ```
            bool is_emoji = IsNonTextFallbackPriority(fallback_priority);
            UErrorCode err = U_ZERO_ERROR;
            UScriptCode script = uscript_getScript(lookup_char, &err);
            if (U_FAILURE(err)) {
            script = USCRIPT_INVALID_CODE;
            }
            ```
            Keita Suzuki

            Done. Adopted your suggested simplification: we now obtain UScriptCode script directly and pass it to FontPerformance after checking for failure, avoiding the separate script_code variable and early casting.

            File third_party/blink/renderer/platform/fonts/font_performance.h
            Line 8, Patchset 20:#include <unicode/uscript.h>
            Takashi Toyoshima . unresolved

            #include "third_party/icu/source/common/unicode/uscript.h"

            Keita Suzuki

            ditto.

            Line 70, Patchset 19: static const std::map<ScriptKey, uint32_t>& GetScriptFallbackCounts();
            Takashi Toyoshima . resolved

            Using `size_t` can clarify the meaning as this is a counter.

            Keita Suzuki

            Done. Updated GetScriptFallbackCounts and the underlying map value type to size_t. Also updated system_fallback_count_ and its getter SystemFallbackFontCount() to use size_t as well.

            Line 67, Patchset 19: static void AddSystemFallbackFontTime(int32_t script_code,
            Takashi Toyoshima . resolved

            Can we still use UScriptCode?

            Keita Suzuki

            Done. Updated AddSystemFallbackFontTime to take UScriptCode.

            Line 32, Patchset 19: static void Reset() {
            Takashi Toyoshima . unresolved

            maybe CHECK(IsMainThread()); and also other methods need the check?

            Keita Suzuki

            seems like font fallback can legally happen on worker threads (e.g., via OffscreenCanvas in Workers), so a CHECK might cause crashes.

            Line 26, Patchset 19: bool operator<(const ScriptKey& other) const {
            return std::tie(script, is_emoji) <
            std::tie(other.script, other.is_emoji);
            }
            Takashi Toyoshima . resolved
            You can skip this manual code by using the following code in C++20.
            ```
            friend auto operator<=>(const ScriptKey&, const ScriptKey&) = default;
            ```

            ```#include <compare>``` may be needed.

            Keita Suzuki

            This is extremely good to know! Simplified the struct by using C++20's default three-way comparison operator and including <compare>.

            Line 24, Patchset 19: int32_t script;
            Takashi Toyoshima . resolved

            keep UScriptCode

            Keita Suzuki

            Done. Changed ScriptKey::script type to UScriptCode.

            File third_party/blink/renderer/platform/fonts/font_performance.cc
            Line 45, Patchset 19: if (!IsMainThread()) [[unlikely]] {
            Takashi Toyoshima . resolved

            Just in case, is this the expected case?
            If so, you can add some comments, and why it's ok to skip non-main thread use cases.

            Keita Suzuki

            Done. Added a comment explaining that FontPerformance collects metrics for Page Load Metrics (main-thread only). Since FontCache is thread-specific (via FontGlobalContext), font fallback can be legally triggered on worker threads (e.g. via OffscreenCanvas in Workers), but we safely ignore those to avoid thread-safety/locking overhead on these global static metrics.

            File tools/metrics/histograms/metadata/page/histograms.xml
            Line 76, Patchset 19:
            Takashi Toyoshima . resolved

            LINT.IfChange here

            Keita Suzuki

            Done. Added LINT.ThenChange pointing back to the Mojo ScriptType enum.

            Open in Gerrit

            Related details

            Attention is currently required from:
            • Takashi Toyoshima
            Submit Requirements:
            • requirement satisfiedCode-Coverage
            • requirement is not satisfiedCode-Owners
            • requirement is not satisfiedCode-Review
            • requirement is not satisfiedNo-Unresolved-Comments
            • requirement is not satisfiedReview-Enforcement
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: chromium/src
            Gerrit-Branch: main
            Gerrit-Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
            Gerrit-Change-Number: 7960697
            Gerrit-PatchSet: 21
            Gerrit-Owner: Keita Suzuki <suzuk...@chromium.org>
            Gerrit-Reviewer: Keita Suzuki <suzuk...@chromium.org>
            Gerrit-Reviewer: Takashi Toyoshima <toyo...@chromium.org>
            Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
            Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
            Gerrit-CC: Stephen Chenney <sche...@chromium.org>
            Gerrit-Attention: Takashi Toyoshima <toyo...@chromium.org>
            Gerrit-Comment-Date: Tue, 23 Jun 2026 08:19:16 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: Yes
            Comment-In-Reply-To: Takashi Toyoshima <toyo...@chromium.org>
            satisfied_requirement
            unsatisfied_requirement
            open
            diffy

            Takashi Toyoshima (Gerrit)

            unread,
            Jun 23, 2026, 4:50:29 AM (8 days ago) Jun 23
            to Keita Suzuki, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, asvitkine...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, bmcquad...@chromium.org, core-timi...@chromium.org, csharris...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
            Attention needed from Keita Suzuki

            Takashi Toyoshima voted and added 2 comments

            Votes added by Takashi Toyoshima

            Code-Review+1

            2 comments

            File third_party/blink/public/web/web_performance_metrics_for_reporting.h
            Line 8, Patchset 20:#include <unicode/uscript.h>
            Takashi Toyoshima . resolved

            #include "third_party/icu/source/common/unicode/uscript.h"
            Sorry, I might have missed this.

            Keita Suzuki

            Unfortunately, it seems like including "third_party/icu/source/common/unicode/uscript.h" violates Blink's checkdeps rules, which forbid direct dependencies on that directory from Blink. We must use the short <unicode/uscript.h> here.

            ```
            You added one or more #includes that violate checkdeps rules.
            third_party/blink/public/web/web_performance_metrics_for_reporting.h
            Illegal include: "third_party/icu/source/common/unicode/uscript.h"
            Because of "-third_party/icu/source/common/unicode" from third_party's include_rules.
            ```
            Takashi Toyoshima

            Oh, you are right.
            It seems the ICU headers are handled as system headers in Blink.

            File third_party/blink/renderer/platform/fonts/font_performance.h
            Line 8, Patchset 20:#include <unicode/uscript.h>
            Takashi Toyoshima . resolved

            #include "third_party/icu/source/common/unicode/uscript.h"

            Keita Suzuki

            ditto.

            Takashi Toyoshima

            Acknowledged

            Open in Gerrit

            Related details

            Attention is currently required from:
            • Keita Suzuki
            Submit Requirements:
              • requirement satisfiedCode-Coverage
              • requirement is not satisfiedCode-Owners
              • requirement satisfiedCode-Review
              • requirement is not satisfiedNo-Unresolved-Comments
              • requirement satisfiedReview-Enforcement
              Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
              Gerrit-MessageType: comment
              Gerrit-Project: chromium/src
              Gerrit-Branch: main
              Gerrit-Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
              Gerrit-Change-Number: 7960697
              Gerrit-PatchSet: 21
              Gerrit-Owner: Keita Suzuki <suzuk...@chromium.org>
              Gerrit-Reviewer: Keita Suzuki <suzuk...@chromium.org>
              Gerrit-Reviewer: Takashi Toyoshima <toyo...@chromium.org>
              Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
              Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
              Gerrit-CC: Stephen Chenney <sche...@chromium.org>
              Gerrit-Attention: Keita Suzuki <suzuk...@chromium.org>
              Gerrit-Comment-Date: Tue, 23 Jun 2026 08:49:54 +0000
              Gerrit-HasComments: Yes
              Gerrit-Has-Labels: Yes
              Comment-In-Reply-To: Takashi Toyoshima <toyo...@chromium.org>
              Comment-In-Reply-To: Keita Suzuki <suzuk...@chromium.org>
              satisfied_requirement
              unsatisfied_requirement
              open
              diffy

              Keita Suzuki (Gerrit)

              unread,
              Jun 23, 2026, 8:37:33 AM (8 days ago) Jun 23
              to Takashi Toyoshima, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, asvitkine...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, bmcquad...@chromium.org, core-timi...@chromium.org, csharris...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org

              Keita Suzuki added 2 comments

              File third_party/blink/renderer/platform/fonts/font_cache.cc
              Line 32, Patchset 19:#include <unicode/uscript.h>
              Takashi Toyoshima . resolved

              ditto

              Keita Suzuki

              Done

              File third_party/blink/renderer/platform/fonts/font_performance.h
              Line 32, Patchset 19: static void Reset() {
              Takashi Toyoshima . resolved

              maybe CHECK(IsMainThread()); and also other methods need the check?

              Keita Suzuki

              seems like font fallback can legally happen on worker threads (e.g., via OffscreenCanvas in Workers), so a CHECK might cause crashes.

              Keita Suzuki

              Done

              Open in Gerrit

              Related details

              Attention set is empty
              Submit Requirements:
                • requirement satisfiedCode-Coverage
                • requirement is not satisfiedCode-Owners
                • requirement satisfiedCode-Review
                • requirement satisfiedReview-Enforcement
                Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
                Gerrit-MessageType: comment
                Gerrit-Project: chromium/src
                Gerrit-Branch: main
                Gerrit-Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
                Gerrit-Change-Number: 7960697
                Gerrit-PatchSet: 21
                Gerrit-Owner: Keita Suzuki <suzuk...@chromium.org>
                Gerrit-Reviewer: Keita Suzuki <suzuk...@chromium.org>
                Gerrit-Reviewer: Takashi Toyoshima <toyo...@chromium.org>
                Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
                Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
                Gerrit-CC: Stephen Chenney <sche...@chromium.org>
                Gerrit-Comment-Date: Tue, 23 Jun 2026 12:37:01 +0000
                Gerrit-HasComments: Yes
                Gerrit-Has-Labels: No
                satisfied_requirement
                unsatisfied_requirement
                open
                diffy

                Keita Suzuki (Gerrit)

                unread,
                Jun 23, 2026, 9:23:57 AM (8 days ago) Jun 23
                to Kentaro Hara, Takashi Toyoshima, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, asvitkine...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, bmcquad...@chromium.org, core-timi...@chromium.org, csharris...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
                Attention needed from Kentaro Hara

                Keita Suzuki added 1 comment

                Patchset-level comments
                Keita Suzuki . resolved

                @har...@chromium.org, could I get your review on third_party/blink/public/web/ web_performance_metrics_for_reporting.h?
                This adds struct member and a Getter definition for renderer performance reporting.

                Thanks!

                Open in Gerrit

                Related details

                Attention is currently required from:
                • Kentaro Hara
                Submit Requirements:
                • requirement satisfiedCode-Coverage
                • requirement is not satisfiedCode-Owners
                • requirement satisfiedCode-Review
                • requirement satisfiedReview-Enforcement
                Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
                Gerrit-MessageType: comment
                Gerrit-Project: chromium/src
                Gerrit-Branch: main
                Gerrit-Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
                Gerrit-Change-Number: 7960697
                Gerrit-PatchSet: 21
                Gerrit-Owner: Keita Suzuki <suzuk...@chromium.org>
                Gerrit-Reviewer: Keita Suzuki <suzuk...@chromium.org>
                Gerrit-Reviewer: Kentaro Hara <har...@chromium.org>
                Gerrit-Reviewer: Takashi Toyoshima <toyo...@chromium.org>
                Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
                Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
                Gerrit-CC: Stephen Chenney <sche...@chromium.org>
                Gerrit-Attention: Kentaro Hara <har...@chromium.org>
                Gerrit-Comment-Date: Tue, 23 Jun 2026 13:23:18 +0000
                Gerrit-HasComments: Yes
                Gerrit-Has-Labels: No
                satisfied_requirement
                unsatisfied_requirement
                open
                diffy

                Kentaro Hara (Gerrit)

                unread,
                Jun 23, 2026, 9:25:08 AM (8 days ago) Jun 23
                to Keita Suzuki, Takashi Toyoshima, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, asvitkine...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, bmcquad...@chromium.org, core-timi...@chromium.org, csharris...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
                Attention needed from Keita Suzuki

                Kentaro Hara voted Code-Review+1

                Code-Review+1
                Open in Gerrit

                Related details

                Attention is currently required from:
                • Keita Suzuki
                Submit Requirements:
                • requirement satisfiedCode-Coverage
                • requirement satisfiedCode-Owners
                • requirement satisfiedCode-Review
                • requirement satisfiedReview-Enforcement
                Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
                Gerrit-MessageType: comment
                Gerrit-Project: chromium/src
                Gerrit-Branch: main
                Gerrit-Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
                Gerrit-Change-Number: 7960697
                Gerrit-PatchSet: 21
                Gerrit-Owner: Keita Suzuki <suzuk...@chromium.org>
                Gerrit-Reviewer: Keita Suzuki <suzuk...@chromium.org>
                Gerrit-Reviewer: Kentaro Hara <har...@chromium.org>
                Gerrit-Reviewer: Takashi Toyoshima <toyo...@chromium.org>
                Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
                Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
                Gerrit-CC: Stephen Chenney <sche...@chromium.org>
                Gerrit-Attention: Keita Suzuki <suzuk...@chromium.org>
                Gerrit-Comment-Date: Tue, 23 Jun 2026 13:24:44 +0000
                Gerrit-HasComments: No
                Gerrit-Has-Labels: Yes
                satisfied_requirement
                open
                diffy

                Keita Suzuki (Gerrit)

                unread,
                Jun 23, 2026, 9:28:51 AM (8 days ago) Jun 23
                to Kentaro Hara, Takashi Toyoshima, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, asvitkine...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, bmcquad...@chromium.org, core-timi...@chromium.org, csharris...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org

                Keita Suzuki voted Commit-Queue+2

                Commit-Queue+2
                Open in Gerrit

                Related details

                Attention set is empty
                Submit Requirements:
                • requirement satisfiedCode-Coverage
                • requirement satisfiedCode-Owners
                • requirement satisfiedCode-Review
                • requirement satisfiedReview-Enforcement
                Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
                Gerrit-MessageType: comment
                Gerrit-Project: chromium/src
                Gerrit-Branch: main
                Gerrit-Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
                Gerrit-Change-Number: 7960697
                Gerrit-PatchSet: 21
                Gerrit-Owner: Keita Suzuki <suzuk...@chromium.org>
                Gerrit-Reviewer: Keita Suzuki <suzuk...@chromium.org>
                Gerrit-Reviewer: Kentaro Hara <har...@chromium.org>
                Gerrit-Reviewer: Takashi Toyoshima <toyo...@chromium.org>
                Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
                Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
                Gerrit-CC: Stephen Chenney <sche...@chromium.org>
                Gerrit-Comment-Date: Tue, 23 Jun 2026 13:28:15 +0000
                Gerrit-HasComments: No
                Gerrit-Has-Labels: Yes
                satisfied_requirement
                open
                diffy

                Chromium LUCI CQ (Gerrit)

                unread,
                Jun 23, 2026, 9:34:43 AM (8 days ago) Jun 23
                to Keita Suzuki, Kentaro Hara, Takashi Toyoshima, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, asvitkine...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, bmcquad...@chromium.org, core-timi...@chromium.org, csharris...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org

                Chromium LUCI CQ submitted the change

                Change information

                Commit message:
                [DSEPrewarm] Record font fallback metrics by script in GWS PLMO

                This CL extends the font loading performance metrics to report fallback
                count grouped by Unicode script (and Emoji) using separate histograms.

                Specifically:
                - We track fallback count per script/emoji in FontPerformance.

                - We plumb this script-specific data through WebPerformanceMetricsForReporting
                and the Page Load Metrics Mojo pipeline (FontLoadingMetrics).
                - In GWSPageLoadMetricsObserver, we log:
                - PageLoad.Clients.GoogleSearch.FontLoading.FallbackCount.<Script>.<Milestone>
                (where <Script> is Latn, Hani, Hang, Hira, Kana, Arab, Beng, Deva, Cyrl,
                Zyyy, Emoji, or Other)
                Bug: 517003594
                Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
                Commit-Queue: Keita Suzuki <suzuk...@chromium.org>
                Reviewed-by: Kentaro Hara <har...@chromium.org>
                Reviewed-by: Takashi Toyoshima <toyo...@chromium.org>
                Cr-Commit-Position: refs/heads/main@{#1650950}
                Files:
                • M chrome/browser/page_load_metrics/observers/gws_page_load_metrics_observer_unittest.cc
                • M components/page_load_metrics/common/page_load_metrics.mojom
                • M components/page_load_metrics/google/browser/gws_page_load_metrics_observer.cc
                • M components/page_load_metrics/renderer/metrics_render_frame_observer.cc
                • M third_party/blink/public/web/web_performance_metrics_for_reporting.h
                • M third_party/blink/renderer/core/exported/web_performance_metrics_for_reporting.cc
                • M third_party/blink/renderer/core/timing/performance_timing_for_reporting.cc
                • M third_party/blink/renderer/core/timing/performance_timing_for_reporting.h
                • M third_party/blink/renderer/platform/fonts/font_cache.cc
                • M third_party/blink/renderer/platform/fonts/font_performance.cc
                • M third_party/blink/renderer/platform/fonts/font_performance.h
                • M tools/metrics/histograms/metadata/page/histograms.xml
                Change size: L
                Delta: 12 files changed, 350 insertions(+), 29 deletions(-)
                Branch: refs/heads/main
                Submit Requirements:
                • requirement satisfiedCode-Review: +1 by Takashi Toyoshima, +1 by Kentaro Hara
                Open in Gerrit
                Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
                Gerrit-MessageType: merged
                Gerrit-Project: chromium/src
                Gerrit-Branch: main
                Gerrit-Change-Id: Ia167e98af1d7bc26b60cc5aabf948a4af9839b55
                Gerrit-Change-Number: 7960697
                Gerrit-PatchSet: 22
                Gerrit-Owner: Keita Suzuki <suzuk...@chromium.org>
                Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
                Gerrit-Reviewer: Keita Suzuki <suzuk...@chromium.org>
                Gerrit-Reviewer: Kentaro Hara <har...@chromium.org>
                Gerrit-Reviewer: Takashi Toyoshima <toyo...@chromium.org>
                open
                diffy
                satisfied_requirement
                Reply all
                Reply to author
                Forward
                0 new messages