Ken Okada (Gerrit)

unread,

Aug 14, 2026, 8:53:36 AM (6 days ago) Aug 14

to Taiyo Mizuhashi, Huanpo Lin, Chromium LUCI CQ, chromium...@chromium.org, android-bu...@system.gserviceaccount.com, speed-metrics...@chromium.org, csharris...@chromium.org, bmcquad...@chromium.org, loading-rev...@chromium.org, speed-metr...@chromium.org

Attention needed from Huanpo Lin and Taiyo Mizuhashi

Ken Okada added 1 comment

Patchset-level comments

File-level comment, Patchset 9 (Latest):

Ken Okada . resolved

Could you have a look?

Open in Gerrit

Related details

Attention is currently required from:

Huanpo Lin
Taiyo Mizuhashi

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

Huanpo Lin (Gerrit)

unread,

Aug 17, 2026, 10:41:58 PM (3 days ago) Aug 17

to Ken Okada, Taiyo Mizuhashi, Chromium LUCI CQ, chromium...@chromium.org, android-bu...@system.gserviceaccount.com, speed-metrics...@chromium.org, csharris...@chromium.org, bmcquad...@chromium.org, loading-rev...@chromium.org, speed-metr...@chromium.org

Attention needed from Ken Okada and Taiyo Mizuhashi

Huanpo Lin added 1 comment

File components/page_load_metrics/browser/observers/preload_serving_metrics_page_load_metrics_observer.cc

Line 168, Patchset 9 (Latest): CreateNavigationData(navigation_handle, /*used_bfcache=*/true);

Huanpo Lin . unresolved

`has_restored_from_bfcache_` was never reset previously, but currently `navigation_data_` can be reset and `used_bfcache` can be overridden, is this intended?

Open in Gerrit

Related details

Attention is currently required from:

Ken Okada
Taiyo Mizuhashi

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

Huanpo Lin (Gerrit)

unread,

Aug 17, 2026, 10:44:11 PM (3 days ago) Aug 17

to Ken Okada, Taiyo Mizuhashi, Chromium LUCI CQ, chromium...@chromium.org, android-bu...@system.gserviceaccount.com, speed-metrics...@chromium.org, csharris...@chromium.org, bmcquad...@chromium.org, loading-rev...@chromium.org, speed-metr...@chromium.org

Attention needed from Ken Okada and Taiyo Mizuhashi

Huanpo Lin added 1 comment

File components/page_load_metrics/browser/observers/preload_serving_metrics_page_load_metrics_observer.h

Line 78, Patchset 9 (Latest): std::unique_ptr<NavigationData> navigation_data_;

Huanpo Lin . unresolved

How about using `std::optional` instead?

satisfied_requirement

unsatisfied_requirement

open

diffy

Ken Okada (Gerrit)

unread,

Aug 18, 2026, 5:57:53 AM (2 days ago) Aug 18

to Taiyo Mizuhashi, Huanpo Lin, Chromium LUCI CQ, chromium...@chromium.org, android-bu...@system.gserviceaccount.com, speed-metrics...@chromium.org, csharris...@chromium.org, bmcquad...@chromium.org, loading-rev...@chromium.org, speed-metr...@chromium.org

Attention needed from Huanpo Lin and Taiyo Mizuhashi

Ken Okada added 2 comments

File components/page_load_metrics/browser/observers/preload_serving_metrics_page_load_metrics_observer.h

Line 78, Patchset 9: std::unique_ptr<NavigationData> navigation_data_;

Huanpo Lin . resolved

How about using `std::optional` instead?

Ken Okada

Unboxed. Thanks!

File components/page_load_metrics/browser/observers/preload_serving_metrics_page_load_metrics_observer.cc

Line 168, Patchset 9: CreateNavigationData(navigation_handle, /*used_bfcache=*/true);

Huanpo Lin . resolved

`has_restored_from_bfcache_` was never reset previously, but currently `navigation_data_` can be reset and `used_bfcache` can be overridden, is this intended?

Ken Okada

Yes, intended. `has_restored_from_bfcache_` is a predicate for PLMO, but what we actually need is `the nav used bfcache or not`. Revised version is more straitforward.

Open in Gerrit

Related details

Attention is currently required from:

Huanpo Lin
Taiyo Mizuhashi

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

Taiyo Mizuhashi (Gerrit)

unread,

Aug 19, 2026, 2:30:12 AM (yesterday) Aug 19

to Ken Okada, Huanpo Lin, Chromium LUCI CQ, chromium...@chromium.org, android-bu...@system.gserviceaccount.com, speed-metrics...@chromium.org, csharris...@chromium.org, bmcquad...@chromium.org, loading-rev...@chromium.org, speed-metr...@chromium.org

Attention needed from Huanpo Lin and Ken Okada

Taiyo Mizuhashi voted and added 4 comments

Votes added by Taiyo Mizuhashi

Code-Review

+1

4 comments

Patchset-level comments

File-level comment, Patchset 10 (Latest):

Taiyo Mizuhashi . resolved

LGTM % nits

File components/page_load_metrics/browser/observers/preload_serving_metrics_page_load_metrics_observer.h

Line 74, Patchset 10 (Latest): NavigationData CreateNavigationData(

Taiyo Mizuhashi . unresolved

maybe we can make this a static function?

File components/page_load_metrics/browser/observers/preload_serving_metrics_page_load_metrics_observer.cc

Line 139, Patchset 10 (Latest): std::move(corrected), navigation_data_->navigation_initiator_string,

Taiyo Mizuhashi . unresolved

nit: maybe we can omit `std::move` for base::Time*

https://source.chromium.org/chromium/chromium/src/+/main:base/time/time.h;l=24-25?q=base::TimeDelta

Line 168, Patchset 9: CreateNavigationData(navigation_handle, /*used_bfcache=*/true);

Huanpo Lin . resolved

`has_restored_from_bfcache_` was never reset previously, but currently `navigation_data_` can be reset and `used_bfcache` can be overridden, is this intended?

Ken Okada

Yes, intended. `has_restored_from_bfcache_` is a predicate for PLMO, but what we actually need is `the nav used bfcache or not`. Revised version is more straitforward.

Taiyo Mizuhashi

noticed the same thing, but acked!

Open in Gerrit

Related details

Attention is currently required from:

Huanpo Lin
Ken Okada

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

Huanpo Lin (Gerrit)

unread,

Aug 19, 2026, 3:46:35 AM (yesterday) Aug 19

to Ken Okada, Taiyo Mizuhashi, Chromium LUCI CQ, chromium...@chromium.org, android-bu...@system.gserviceaccount.com, speed-metrics...@chromium.org, csharris...@chromium.org, bmcquad...@chromium.org, loading-rev...@chromium.org, speed-metr...@chromium.org

Attention needed from Ken Okada

Huanpo Lin voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Ken Okada

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

Ken Okada (Gerrit)

unread,

Aug 19, 2026, 4:17:44 AM (yesterday) Aug 19

to Huanpo Lin, Taiyo Mizuhashi, Chromium LUCI CQ, chromium...@chromium.org, android-bu...@system.gserviceaccount.com, speed-metrics...@chromium.org, csharris...@chromium.org, bmcquad...@chromium.org, loading-rev...@chromium.org, speed-metr...@chromium.org

Ken Okada added 2 comments

File components/page_load_metrics/browser/observers/preload_serving_metrics_page_load_metrics_observer.h

Line 74, Patchset 10: NavigationData CreateNavigationData(

Taiyo Mizuhashi . resolved

maybe we can make this a static function?

Ken Okada

Done

File components/page_load_metrics/browser/observers/preload_serving_metrics_page_load_metrics_observer.cc

Line 139, Patchset 10: std::move(corrected), navigation_data_->navigation_initiator_string,

Taiyo Mizuhashi . resolved

nit: maybe we can omit `std::move` for base::Time*
https://source.chromium.org/chromium/chromium/src/+/main:base/time/time.h;l=24-25?q=base::TimeDelta

Ken Okada

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,

7:40 AM (4 hours ago) 7:40 AM

to Ken Okada, Huanpo Lin, Taiyo Mizuhashi, chromium...@chromium.org, android-bu...@system.gserviceaccount.com, speed-metrics...@chromium.org, csharris...@chromium.org, bmcquad...@chromium.org, loading-rev...@chromium.org, speed-metr...@chromium.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

10 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: components/page_load_metrics/browser/observers/preload_serving_metrics_page_load_metrics_observer.h
Insertions: 1, Deletions: 1.

The diff is too large to show. Please review the diff.
```
```
The name of the file: components/page_load_metrics/browser/observers/preload_serving_metrics_page_load_metrics_observer.cc
Insertions: 22, Deletions: 1.

The diff is too large to show. Please review the diff.
```

Change information

Commit message:

PreloadServingMetrics: Group navigation data in PreloadServingMetricsPLMO

This is a preparation of https://crrev.com/c/8255995.

This CL introduces `NavigationData` in
`PreloadServingMetricsPageLoadMetricsObserver`, which holds data of
navigation, for ease of lifecycle management.

TAG=agy
CONV=5d71549e-0061-42b8-a2c5-9289dcd6183d

Bug: 517725655

Change-Id: I8bc301b67e609d67ac7fd189d6334441ed8941d6

Co-Authored-By: Gemini <nor...@google.com>

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

Commit-Queue: Ken Okada <ken...@chromium.org>

Reviewed-by: Taiyo Mizuhashi <ta...@chromium.org>

Reviewed-by: Huanpo Lin <robe...@chromium.org>

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

Files:

M components/page_load_metrics/browser/observers/preload_serving_metrics_page_load_metrics_observer.cc
M components/page_load_metrics/browser/observers/preload_serving_metrics_page_load_metrics_observer.h

Change size: M

Delta: 2 files changed, 109 insertions(+), 93 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Taiyo Mizuhashi, +1 by Huanpo Lin

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

PreloadServingMetrics: Group navigation data in PreloadServingMetricsPLMO [chromium/src : main]

Ken Okada (Gerrit)

Ken Okada added 1 comment

Related details

Huanpo Lin (Gerrit)

Huanpo Lin added 1 comment

Related details

Huanpo Lin (Gerrit)

Huanpo Lin added 1 comment

Ken Okada (Gerrit)

Ken Okada added 2 comments

Related details

Taiyo Mizuhashi (Gerrit)

Taiyo Mizuhashi voted and added 4 comments

Votes added by Taiyo Mizuhashi

4 comments

Related details

Huanpo Lin (Gerrit)

Huanpo Lin voted Code-Review+1

Related details

Ken Okada (Gerrit)

Ken Okada added 2 comments

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information