Kenichi Ishibashi (Gerrit)

unread,

Feb 25, 2026, 7:50:42 PM (6 days ago) Feb 25

to Yoshisato Yanagisawa, Takashi Toyoshima, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, alexmo...@chromium.org, asvitkine...@chromium.org, bmcquad...@chromium.org, bnc+...@chromium.org, creis...@chromium.org, csharris...@chromium.org, ipc-securi...@chromium.org, loading-rev...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org

Attention needed from Takashi Toyoshima and Yoshisato Yanagisawa

Kenichi Ishibashi added 2 comments

File net/base/load_timing_internal_info.h

Line 50, Patchset 5 (Latest): bool accept_ch_frame_received = false;

Kenichi Ishibashi . unresolved

Unlike other fields, this one is set in //services/network, which is a kind of layer violation. I don't strongly oppose this, but can we have a comment that this field is set out side of //net, warning that any intermediate layers could modify this value?

File net/http/http_network_transaction.cc

Line 744, Patchset 5 (Latest): if (!connected_callback_start_time_.is_null()) {

Kenichi Ishibashi . unresolved

Can we use `connected_callback_delay.is_null()` instead of having a separate flag? I assume we don't care much about a case where the end time is null.

Open in Gerrit

Related details

Attention is currently required from:

Takashi Toyoshima
Yoshisato Yanagisawa

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

Kenichi Ishibashi (Gerrit)

unread,

Feb 26, 2026, 1:17:39 AM (6 days ago) Feb 26

to Yoshisato Yanagisawa, Keita Suzuki, Takashi Toyoshima, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, alexmo...@chromium.org, asvitkine...@chromium.org, bmcquad...@chromium.org, bnc+...@chromium.org, creis...@chromium.org, csharris...@chromium.org, ipc-securi...@chromium.org, loading-rev...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org

Attention needed from Takashi Toyoshima and Yoshisato Yanagisawa

Kenichi Ishibashi added 1 comment

Patchset-level comments

File-level comment, Patchset 5 (Latest):

Kenichi Ishibashi . resolved

suzukikeita@: FYI

satisfied_requirement

unsatisfied_requirement

open

diffy

Takashi Toyoshima (Gerrit)

unread,

Feb 26, 2026, 1:18:49 AM (6 days ago) Feb 26

to Yoshisato Yanagisawa, Keita Suzuki, Kenichi Ishibashi, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, alexmo...@chromium.org, asvitkine...@chromium.org, bmcquad...@chromium.org, bnc+...@chromium.org, creis...@chromium.org, csharris...@chromium.org, ipc-securi...@chromium.org, loading-rev...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org

Attention needed from Yoshisato Yanagisawa

Takashi Toyoshima added 3 comments

File components/page_load_metrics/google/browser/gws_page_load_metrics_observer.cc

Line 1027, Patchset 5 (Latest): if (!was_cached_) {

Takashi Toyoshima . unresolved

maybe
```
if (was_cached_) {
  return;
}
```
is clearer now?

Line 1034, Patchset 5 (Latest): if (IsIncognitoProfile()) {

Takashi Toyoshima . unresolved

Can we pass the result to this lambda rather than capturing `this`?

Line 1040, Patchset 5 (Latest):

Takashi Toyoshima . unresolved

maybe
```
const bool is_incognito = IsIncognitoProfile();
```
here, and pass it to the both calls.

Open in Gerrit

Related details

Attention is currently required from:

Yoshisato Yanagisawa

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

Yoshisato Yanagisawa (Gerrit)

unread,

Mar 2, 2026, 5:28:57 AM (yesterday) Mar 2

to Keita Suzuki, Kenichi Ishibashi, Takashi Toyoshima, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, alexmo...@chromium.org, asvitkine...@chromium.org, bmcquad...@chromium.org, bnc+...@chromium.org, creis...@chromium.org, csharris...@chromium.org, ipc-securi...@chromium.org, loading-rev...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org

Attention needed from Kenichi Ishibashi and Takashi Toyoshima

Yoshisato Yanagisawa voted and added 4 comments

Votes added by Yoshisato Yanagisawa

Commit-Queue

+1

4 comments

File components/page_load_metrics/google/browser/gws_page_load_metrics_observer.cc

Line 1027, Patchset 5: if (!was_cached_) {

Takashi Toyoshima . resolved

maybe
```
if (was_cached_) {
  return;
}
```
is clearer now?

Yoshisato Yanagisawa

Done

Line 1034, Patchset 5: if (IsIncognitoProfile()) {

Takashi Toyoshima . resolved

Can we pass the result to this lambda rather than capturing `this`?

Yoshisato Yanagisawa

Done

Line 1040, Patchset 5:

Takashi Toyoshima . resolved

maybe
```
const bool is_incognito = IsIncognitoProfile();
```
here, and pass it to the both calls.

Yoshisato Yanagisawa

Done

File net/base/load_timing_internal_info.h

Line 50, Patchset 5: bool accept_ch_frame_received = false;

Kenichi Ishibashi . resolved

Unlike other fields, this one is set in //services/network, which is a kind of layer violation. I don't strongly oppose this, but can we have a comment that this field is set out side of //net, warning that any intermediate layers could modify this value?

Yoshisato Yanagisawa

Done

Open in Gerrit

Related details

Attention is currently required from:

Kenichi Ishibashi
Takashi Toyoshima

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

Kenichi Ishibashi (Gerrit)

unread,

Mar 2, 2026, 7:11:36 PM (20 hours ago) Mar 2

to Yoshisato Yanagisawa, Keita Suzuki, Takashi Toyoshima, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, alexmo...@chromium.org, asvitkine...@chromium.org, bmcquad...@chromium.org, bnc+...@chromium.org, creis...@chromium.org, csharris...@chromium.org, ipc-securi...@chromium.org, loading-rev...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org

Attention needed from Takashi Toyoshima and Yoshisato Yanagisawa

Kenichi Ishibashi voted and added 2 comments

Votes added by Kenichi Ishibashi

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 6 (Latest):

Kenichi Ishibashi . resolved

lgtm

File net/http/http_network_transaction.cc

Line 744, Patchset 5: if (!connected_callback_start_time_.is_null()) {

Kenichi Ishibashi . resolved

Can we use `connected_callback_delay.is_null()` instead of having a separate flag? I assume we don't care much about a case where the end time is null.

Kenichi Ishibashi

Looks like this is done.

Open in Gerrit

Related details

Attention is currently required from:

Takashi Toyoshima
Yoshisato Yanagisawa

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

Yoshisato Yanagisawa (Gerrit)

unread,

2:20 AM (13 hours ago) 2:20 AM

to Rakina Zata Amni, Kenichi Ishibashi, Keita Suzuki, Takashi Toyoshima, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, alexmo...@chromium.org, asvitkine...@chromium.org, bmcquad...@chromium.org, bnc+...@chromium.org, creis...@chromium.org, csharris...@chromium.org, ipc-securi...@chromium.org, loading-rev...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org

Attention needed from Rakina Zata Amni and Takashi Toyoshima

Yoshisato Yanagisawa added 1 comment

File net/http/http_network_transaction.cc

Line 744, Patchset 5: if (!connected_callback_start_time_.is_null()) {

Kenichi Ishibashi . resolved

Can we use `connected_callback_delay.is_null()` instead of having a separate flag? I assume we don't care much about a case where the end time is null.

Kenichi Ishibashi

Looks like this is done.

Yoshisato Yanagisawa

yes. sorry for not updating this.

Open in Gerrit

Related details

Attention is currently required from:

Rakina Zata Amni
Takashi Toyoshima

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

Rakina Zata Amni (Gerrit)

unread,

2:58 AM (12 hours ago) 2:58 AM

to Yoshisato Yanagisawa, Kenichi Ishibashi, Keita Suzuki, Takashi Toyoshima, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, alexmo...@chromium.org, asvitkine...@chromium.org, bmcquad...@chromium.org, bnc+...@chromium.org, creis...@chromium.org, csharris...@chromium.org, ipc-securi...@chromium.org, loading-rev...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org

Attention needed from Takashi Toyoshima and Yoshisato Yanagisawa

Rakina Zata Amni voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Takashi Toyoshima
Yoshisato Yanagisawa

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

Reply all

Reply to author

Forward

Add metrics to track onConnected calls and AcceptCHFrame reception [chromium/src : main]

Kenichi Ishibashi (Gerrit)

Kenichi Ishibashi added 2 comments

Related details

Kenichi Ishibashi (Gerrit)

Kenichi Ishibashi added 1 comment

Takashi Toyoshima (Gerrit)

Takashi Toyoshima added 3 comments

Related details

Yoshisato Yanagisawa (Gerrit)

Yoshisato Yanagisawa voted and added 4 comments

Votes added by Yoshisato Yanagisawa

4 comments

Related details

Kenichi Ishibashi (Gerrit)

Kenichi Ishibashi voted and added 2 comments

Votes added by Kenichi Ishibashi

2 comments

Related details

Yoshisato Yanagisawa (Gerrit)

Yoshisato Yanagisawa added 1 comment

Related details

Rakina Zata Amni (Gerrit)

Rakina Zata Amni voted Code-Review+1

Related details