Prefetch: Cancel unrelated prefetch [chromium/src : main]
Ken Okada (Gerrit)
Ken Okada added 1 comment![]( "Open in Gerrit")
Related details
- Hiroki Nakagawa
- Hiroshige Hayashizaki
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hiroki Nakagawa (Gerrit)
Hiroki Nakagawa added 9 comments![]( "Open in Gerrit")
Thanks!
control it.Can you add VirtualTestSuites for this feature flag for more test coverage?
: public blink::URLLoaderThrottle {Why is this `URLLoaderThrottle`, not `NavigationThrottle`?
Should we create this throttle only if this request is for main frame navigation in a primary page? Otherwise, iframe navigation and subresource requests might cancel prefetches?
->GetPrefetchKeys();We don't need to create `NavigationClaimingPrefetches` when it doesn't exist?
```
if (auto* data = NavigationClaimingPrefetches::GetForNavigationHandle(
*frame_tree_node->navigation_request())) {
prefetch_service->CancelUnrelatedPrefetchForNavigation(
data->GetPrefetchKeys());
}
```
// return &that->web_contents();Was this comment used for debugging? Can we remove this?
// Due to the limitation of `StartPrefetch()`, we can't write a test of theCan we briefly explain the limitation so that code readers don't need to open the crbug below to know it?
// Due to the limitation of `StartPrefetch()` that allows only one prefetch to be triggered, we can't write ...
}`const_cast` is not necessary:
```
const PrefetchContainer* PrefetchService::GetPrefetchContainerForTesting(
const PrefetchKey& key) const {
auto it = owned_prefetches_.find(key);
if (it != owned_prefetches_.end()) {
return it->second.get();
}
return nullptr;
}
```
Related details
- Hiroshige Hayashizaki
- Ken Okada
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. |
Hiroshige Hayashizaki (Gerrit)
Hiroshige Hayashizaki added 5 comments![]( "Open in Gerrit")
const base::flat_set<PrefetchKey>& keys) {I suspect `key` is actually not needed, because the `PrefetchContainer` used for serving always has `PrefetchMatchResolverAction::ActionKind::kMaybeServe` in sucessful cases, and thus such PrefetchContainer is already excluded from target.
(Theoretically(?) there might be cases where it was `kMaybeServe` when checking for serving but later failed and no longer `kMaybeServe` here, but anyway probably we can cancel such cases as they anyway are failing)
prefetch_container->GetMatchResolverAction().kind() ==Could you add a comment to explain the intent for this condition?
Perhaps we don't want to cancel prefetches if they already receives successful responses, but not sure/not clear.
PrefetchMatchResolverAction::ActionKind::kMaybeServe ||IIUC this is equivalent to `LoadState::kDeterminedHead` or `LoadState::kCompleted` and I think `LoadState`-based check is much simpler.
keys.contains(prefetch_container->key());Also I think it's better to limit to prefetches with the same initiators to avoid cancelling prefetches from other Documents / other sites.
if (!is_not_target) {```
if (prefetch_container->GetMatchResolverAction().kind() !=
PrefetchMatchResolverAction::ActionKind::kMaybeServe &&
!keys.contains(prefetch_container->key()) {
```
is much easier to read.
Related details
- Ken Okada
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. |
Ken Okada (Gerrit)
Ken Okada voted and added 13 comments![]( "Open in Gerrit")
Votes added by Ken Okada
| Commit-Queue | +1 |
Can you add VirtualTestSuites for this feature flag for more test coverage?
Done
FrameTreeNodeId frame_tree_node_id_;Ken Okadaconst
Done
: public blink::URLLoaderThrottle {Why is this `URLLoaderThrottle`, not `NavigationThrottle`?
- `NavigationRequest::WillStartRequest()` is too early. `NavigationURLLoaderImpl::Start()` and prefetch matching follows it.
- Other hooks are too late.
- Another option was doing something like prefetch matching at, e.g., the head of `NavigationRequest`, rather than utilizing existing prefetch matching. But we (chat with hiroshige@) rejected it to keep PrefetchURLLoaderInterceptor/PrefetchMatchResolver "SSoT" of prefetch matching.
Added as comments.
Should we create this throttle only if this request is for main frame navigation in a primary page? Otherwise, iframe navigation and subresource requests might cancel prefetches?
Thanks! Right.
Done.
We don't need to create `NavigationClaimingPrefetches` when it doesn't exist?
```
if (auto* data = NavigationClaimingPrefetches::GetForNavigationHandle(
*frame_tree_node->navigation_request())) {
prefetch_service->CancelUnrelatedPrefetchForNavigation(
data->GetPrefetchKeys());
}
```
Done with adding else clause.
Was this comment used for debugging? Can we remove this?
Done
// Due to the limitation of `StartPrefetch()`, we can't write a test of theCan we briefly explain the limitation so that code readers don't need to open the crbug below to know it?
// Due to the limitation of `StartPrefetch()` that allows only one prefetch to be triggered, we can't write ...
`StartPrefetch()` allows triggering only one prefetch.
Added a comment.
I suspect `key` is actually not needed, because the `PrefetchContainer` used for serving always has `PrefetchMatchResolverAction::ActionKind::kMaybeServe` in sucessful cases, and thus such PrefetchContainer is already excluded from target.
(Theoretically(?) there might be cases where it was `kMaybeServe` when checking for serving but later failed and no longer `kMaybeServe` here, but anyway probably we can cancel such cases as they anyway are failing)
That's right! Thanks!
Updated.
prefetch_container->GetMatchResolverAction().kind() ==Could you add a comment to explain the intent for this condition?
Perhaps we don't want to cancel prefetches if they already receives successful responses, but not sure/not clear.
Done
PrefetchMatchResolverAction::ActionKind::kMaybeServe ||IIUC this is equivalent to `LoadState::kDeterminedHead` or `LoadState::kCompleted` and I think `LoadState`-based check is much simpler.
Done
keys.contains(prefetch_container->key());Also I think it's better to limit to prefetches with the same initiators to avoid cancelling prefetches from other Documents / other sites.
The purpose of this feature/experiment (at least this phase) is investigating effect of network and other resource congestion on performance metrics. (ParallelPrefetch: increase possibility, CancelUnrelatedPrefetch: decrease.)
So, it's nice to maximize the targets and the effect.
```
if (prefetch_container->GetMatchResolverAction().kind() !=
PrefetchMatchResolverAction::ActionKind::kMaybeServe &&
!keys.contains(prefetch_container->key()) {
```is much easier to read.
Done
`const_cast` is not necessary:
```
const PrefetchContainer* PrefetchService::GetPrefetchContainerForTesting(
const PrefetchKey& key) const {
auto it = owned_prefetches_.find(key);
if (it != owned_prefetches_.end()) {
return it->second.get();
}
return nullptr;
}
```
- Hiroki Nakagawa
- Hiroshige Hayashizaki
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. |
Hiroshige Hayashizaki (Gerrit)
Hiroshige Hayashizaki added 2 comments![]( "Open in Gerrit")
: public blink::URLLoaderThrottle {Ken OkadaWhy is this `URLLoaderThrottle`, not `NavigationThrottle`?
- `NavigationRequest::WillStartRequest()` is too early. `NavigationURLLoaderImpl::Start()` and prefetch matching follows it.
- Other hooks are too late.
- Another option was doing something like prefetch matching at, e.g., the head of `NavigationRequest`, rather than utilizing existing prefetch matching. But we (chat with hiroshige@) rejected it to keep PrefetchURLLoaderInterceptor/PrefetchMatchResolver "SSoT" of prefetch matching.
Added as comments.
Memo: I'm wondering we should migrate this and `PrerenderURLLoaderThrottle` out of `URLLoaderThrottle` (perhaps by introducing more direct code in `NavigationURLLoaderImpl`), but this can/should be done separately/later not to block the experiment.
"bases": [],Shouldn't we list some bases here?
Related details
- Hiroki Nakagawa
- Ken Okada
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. |
Ken Okada (Gerrit)
Ken Okada added 1 comment![]( "Open in Gerrit")
"bases": [],Shouldn't we list some bases here?
I'm not so sure as (I think) it's not documented well... Lot's of suite uses `[]` and I guess that it's something like `"ALL"`.
Related details
- Hiroki Nakagawa
- Hiroshige Hayashizaki
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. |
Ken Okada (Gerrit)
Ken Okada added 1 comment![]( "Open in Gerrit")
keys.contains(prefetch_container->key());Ken OkadaAlso I think it's better to limit to prefetches with the same initiators to avoid cancelling prefetches from other Documents / other sites.
The purpose of this feature/experiment (at least this phase) is investigating effect of network and other resource congestion on performance metrics. (ParallelPrefetch: increase possibility, CancelUnrelatedPrefetch: decrease.)
So, it's nice to maximize the targets and the effect.
Chatted.
Hiroshige Hayashizaki (Gerrit)
Hiroshige Hayashizaki voted and added 5 comments![]( "Open in Gerrit")
Votes added by Hiroshige Hayashizaki
| Code-Review | +1 |
5 comments
GURL url_navigated =nit optional: it's clearer to show the correspondance bewteen the URLs in the code and comment, e.g.
Adding a comment like:
```
// URL V
```
or renaming to `url_V` etc.
ditto below.
// - Start navigation X for URL V.URL U?
// - It doesn't cancel A as it is related.Just to check: How this test is passing? I thought `CancelUnrelatedPrefetchForNavigation()` is called on the initial navigation (i.e. just before the network request to URL V) and thus prefetch A is cancelled there, but surely I'm missing something.
// Cancel unrelated prefetches.nit: `Cancels`.
"bases": [],Ken OkadaShouldn't we list some bases here?
I'm not so sure as (I think) it's not documented well... Lot's of suite uses `[]` and I guess that it's something like `"ALL"`.
I feel running all is too broad -- but anyway I'll defer to virtual test owner.
Related details
- Hiroki Nakagawa
- Ken Okada
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. |
Hiroki Nakagawa (Gerrit)
Hiroki Nakagawa voted and added 3 comments![]( "Open in Gerrit")
Votes added by Hiroki Nakagawa
| Code-Review | +1 |
3 comments
: public blink::URLLoaderThrottle {Ken OkadaWhy is this `URLLoaderThrottle`, not `NavigationThrottle`?
Hiroshige Hayashizaki
- `NavigationRequest::WillStartRequest()` is too early. `NavigationURLLoaderImpl::Start()` and prefetch matching follows it.
- Other hooks are too late.
- Another option was doing something like prefetch matching at, e.g., the head of `NavigationRequest`, rather than utilizing existing prefetch matching. But we (chat with hiroshige@) rejected it to keep PrefetchURLLoaderInterceptor/PrefetchMatchResolver "SSoT" of prefetch matching.
Added as comments.
Memo: I'm wondering we should migrate this and `PrerenderURLLoaderThrottle` out of `URLLoaderThrottle` (perhaps by introducing more direct code in `NavigationURLLoaderImpl`), but this can/should be done separately/later not to block the experiment.
That makes sense, thanks!
"bases": [],Ken OkadaShouldn't we list some bases here?
Hiroshige HayashizakiI'm not so sure as (I think) it's not documented well... Lot's of suite uses `[]` and I guess that it's something like `"ALL"`.
I feel running all is too broad -- but anyway I'll defer to virtual test owner.
Should we run this only for speculation rules (external/wpt/speculation-rules/), as other WPTs don't trigger speculation rules prefetch in the first place?
Related details
- Ken Okada
Hiroshige Hayashizaki (Gerrit)
Hiroshige Hayashizaki voted and added 2 comments![]( "Open in Gerrit")
Votes added by Hiroshige Hayashizaki
| Code-Review | +1 |
2 comments
// - Start navigation X for URL V.Hiroshige HayashizakiURL U?
Done
// Cancel unrelated prefetches.Hiroshige Hayashizakinit: `Cancels`.
Done
Related details
- Hiroki Nakagawa
- Ken Okada
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. |
Hiroshige Hayashizaki (Gerrit)
Hiroshige Hayashizaki added 1 comment![]( "Open in Gerrit")
"bases": [],Ken OkadaShouldn't we list some bases here?
Hiroshige HayashizakiI'm not so sure as (I think) it's not documented well... Lot's of suite uses `[]` and I guess that it's something like `"ALL"`.
Hiroki NakagawaI feel running all is too broad -- but anyway I'll defer to virtual test owner.
Should we run this only for speculation rules (external/wpt/speculation-rules/), as other WPTs don't trigger speculation rules prefetch in the first place?
+1 for just `external/wpt/speculation-rules` unless we have specific reasons.
Ken Okada (Gerrit)
Ken Okada added 5 comments![]( "Open in Gerrit")
: public blink::URLLoaderThrottle {Ken OkadaWhy is this `URLLoaderThrottle`, not `NavigationThrottle`?
Hiroshige Hayashizaki
- `NavigationRequest::WillStartRequest()` is too early. `NavigationURLLoaderImpl::Start()` and prefetch matching follows it.
- Other hooks are too late.
- Another option was doing something like prefetch matching at, e.g., the head of `NavigationRequest`, rather than utilizing existing prefetch matching. But we (chat with hiroshige@) rejected it to keep PrefetchURLLoaderInterceptor/PrefetchMatchResolver "SSoT" of prefetch matching.
Added as comments.
Hiroki NakagawaMemo: I'm wondering we should migrate this and `PrerenderURLLoaderThrottle` out of `URLLoaderThrottle` (perhaps by introducing more direct code in `NavigationURLLoaderImpl`), but this can/should be done separately/later not to block the experiment.
That makes sense, thanks!
Done
GURL url_navigated =nit optional: it's clearer to show the correspondance bewteen the URLs in the code and comment, e.g.
Adding a comment like:
```
// URL V
```or renaming to `url_V` etc.
ditto below.
Updated as "URL `unl_navigated`" etc.
// - Start navigation X for URL V.URL U?
Done
Just to check: How this test is passing? I thought `CancelUnrelatedPrefetchForNavigation()` is called on the initial navigation (i.e. just before the network request to URL V) and thus prefetch A is cancelled there, but surely I'm missing something.
Ah, good catch. Sorry. This was passed because the prefetch is servable as the test waits the prefetch. Using ControllableHttpResponse, it fails.
Updated expectation. Unfortunately, this case is unavoidable.
Chatted and mark resolved.
// Cancel unrelated prefetches.nit: `Cancels`.
- Hiroki Nakagawa
- Hiroshige Hayashizaki
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. |
Ken Okada (Gerrit)
Ken Okada added 1 comment![]( "Open in Gerrit")
"bases": [],Ken OkadaShouldn't we list some bases here?
Hiroshige HayashizakiI'm not so sure as (I think) it's not documented well... Lot's of suite uses `[]` and I guess that it's something like `"ALL"`.
Hiroki NakagawaI feel running all is too broad -- but anyway I'll defer to virtual test owner.
Hiroshige HayashizakiShould we run this only for speculation rules (external/wpt/speculation-rules/), as other WPTs don't trigger speculation rules prefetch in the first place?
+1 for just `external/wpt/speculation-rules` unless we have specific reasons.
https://chromium-review.git.corp.google.com/c/chromium/src/+/7792117
My understanding was wrong. Added external/wpt/speculation-rules and http.
Related details
- Hiroki Nakagawa
- Hiroshige Hayashizaki
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hiroki Nakagawa (Gerrit)
Hiroki Nakagawa voted and added 1 comment![]( "Open in Gerrit")
Votes added by Hiroki Nakagawa
| Code-Review | +1 |
1 comment
"bases": ["external/wpt/speculation-rules", "http"],Which tests do we want to run under "http"? Is it possible to narrow down the scope like "external/wpt/speculation-rules"?
Related details
- Hiroshige Hayashizaki
- Ken Okada
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. |
Ken Okada (Gerrit)
Ken Okada added 1 comment![]( "Open in Gerrit")
"bases": ["external/wpt/speculation-rules", "http"],Which tests do we want to run under "http"? Is it possible to narrow down the scope like "external/wpt/speculation-rules"?
E.g. http/tests/inspector-protocol/prefetch. Narrowed down to http/tests/inspector-protocol.
Related details
- Hiroshige Hayashizaki
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hiroshige Hayashizaki (Gerrit)
Hiroshige Hayashizaki voted Code-Review+1![]( "Open in Gerrit")
- Ken Okada
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ken Okada (Gerrit)
Ken Okada added 1 comment![]( "Open in Gerrit")
Owner review. Could you have a look?
dsv@
- content/browser/devtools/protocol/preload_handler.cc
toyoshim@
- content/browser/loader/url_loader_throttles.cc
jonathanjlee@
- third_party/blink/web_tests/TestLists/content_shell.filter
- third_party/blink/web_tests/VirtualTestSuites
Related details
- Danil Somsikov
- Jonathan Lee
- Takashi Toyoshima
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Lee (Gerrit)
Jonathan Lee added 1 comment![]( "Open in Gerrit")
"platforms": [
"Android",
"Fuchsia",
"Linux",
"Mac",
"Webview",
"Win"
],
"bases": ["external/wpt/speculation-rules", "http/tests/inspector-protocol"],Is it possible to pick more specific tests to run, or only run them on Linux? See [these guidelines].
Looking at https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/2663655/overview, [`virtual/prefetch-cancel-unrelated-prefetch/external/wpt/speculation-rules/`][1] runs ~500 tests and makes up 1.8% of `headless_shell_wpt_tests` (see "timing stats" in top right).
(I didn't see any try builds for only `virtual/prefetch-cancel-unrelated-prefetch/http/tests/inspector-protocol/` - please dry-run and check the timing for `blink_web_tests`.)
If this is a wide-reaching architectural change where many tests could regress, another alternative would be to create new [flag-specific suites] and only run them on CI builders. This avoids overloading the CQ test fleet (at the cost of catching regressions later).
[0]: https://chromium-layout-test-archives.storage.googleapis.com/results.html?json=chromium/try/linux-rel/2663655/blink_web_tests%20%28with%20patch%29/full_results_jsonp.js
[1]: https://chromium-layout-test-archives.storage.googleapis.com/results.html?json=chromium/try/linux-rel/2663655/headless_shell_wpt_tests%20%28with%20patch%29/full_results_jsonp.js
[these guidelines]: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/VIRTUAL_OWNERS;l=12-20;drc=3c723a4b2022e4e47e6c95f7f6ad6a6931c85eae;bpv=0;bpt=0
[flag-specific suites]: https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_tests.md#choosing-between-flag_specific-and-virtual-test-suite
Related details
- Danil Somsikov
- Ken Okada
- Takashi Toyoshima
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. |
Ken Okada (Gerrit)
Ken Okada added 1 comment![]( "Open in Gerrit")
"platforms": [
"Android",
"Fuchsia",
"Linux",
"Mac",
"Webview",
"Win"
],
"bases": ["external/wpt/speculation-rules", "http/tests/inspector-protocol"],Is it possible to pick more specific tests to run, or only run them on Linux? See [these guidelines].
Looking at https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/2663655/overview, [`virtual/prefetch-cancel-unrelated-prefetch/external/wpt/speculation-rules/`][1] runs ~500 tests and makes up 1.8% of `headless_shell_wpt_tests` (see "timing stats" in top right).
(I didn't see any try builds for only `virtual/prefetch-cancel-unrelated-prefetch/http/tests/inspector-protocol/` - please dry-run and check the timing for `blink_web_tests`.)
If this is a wide-reaching architectural change where many tests could regress, another alternative would be to create new [flag-specific suites] and only run them on CI builders. This avoids overloading the CQ test fleet (at the cost of catching regressions later).
[0]: https://chromium-layout-test-archives.storage.googleapis.com/results.html?json=chromium/try/linux-rel/2663655/blink_web_tests%20%28with%20patch%29/full_results_jsonp.js
[1]: https://chromium-layout-test-archives.storage.googleapis.com/results.html?json=chromium/try/linux-rel/2663655/headless_shell_wpt_tests%20%28with%20patch%29/full_results_jsonp.js
[these guidelines]: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/VIRTUAL_OWNERS;l=12-20;drc=3c723a4b2022e4e47e6c95f7f6ad6a6931c85eae;bpv=0;bpt=0
[flag-specific suites]: https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_tests.md#choosing-between-flag_specific-and-virtual-test-suite
- Danil Somsikov
- Jonathan Lee
- Takashi Toyoshima
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Takashi Toyoshima (Gerrit)
Takashi Toyoshima added 1 comment![]( "Open in Gerrit")
// Copyright 2026 The Chromium Authorsjust in case, why this is implemented as a URLLoaderThrottle rather than NavigationThrottle? Are you interested in subresource load hooks too?
Related details
- Danil Somsikov
- Jonathan Lee
- Ken Okada
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. |
Jonathan Lee (Gerrit)
Jonathan Lee voted and added 1 comment![]( "Open in Gerrit")
Votes added by Jonathan Lee
| Code-Review | +1 |
1 comment
"platforms": [
"Android",
"Fuchsia",
"Linux",
"Mac",
"Webview",
"Win"
],
"bases": ["external/wpt/speculation-rules", "http/tests/inspector-protocol"],Ken OkadaIs it possible to pick more specific tests to run, or only run them on Linux? See [these guidelines].
Looking at https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/2663655/overview, [`virtual/prefetch-cancel-unrelated-prefetch/external/wpt/speculation-rules/`][1] runs ~500 tests and makes up 1.8% of `headless_shell_wpt_tests` (see "timing stats" in top right).
(I didn't see any try builds for only `virtual/prefetch-cancel-unrelated-prefetch/http/tests/inspector-protocol/` - please dry-run and check the timing for `blink_web_tests`.)
If this is a wide-reaching architectural change where many tests could regress, another alternative would be to create new [flag-specific suites] and only run them on CI builders. This avoids overloading the CQ test fleet (at the cost of catching regressions later).
[0]: https://chromium-layout-test-archives.storage.googleapis.com/results.html?json=chromium/try/linux-rel/2663655/blink_web_tests%20%28with%20patch%29/full_results_jsonp.js
[1]: https://chromium-layout-test-archives.storage.googleapis.com/results.html?json=chromium/try/linux-rel/2663655/headless_shell_wpt_tests%20%28with%20patch%29/full_results_jsonp.js
[these guidelines]: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/VIRTUAL_OWNERS;l=12-20;drc=3c723a4b2022e4e47e6c95f7f6ad6a6931c85eae;bpv=0;bpt=0
[flag-specific suites]: https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_tests.md#choosing-between-flag_specific-and-virtual-test-suite
Thanks! Only run them on Linux.
- Danil Somsikov
- Ken Okada
Ken Okada (Gerrit)
Ken Okada added 1 comment![]( "Open in Gerrit")
// Copyright 2026 The Chromium Authorsjust in case, why this is implemented as a URLLoaderThrottle rather than NavigationThrottle? Are you interested in subresource load hooks too?
Good question. It's because the hooks of NavigationThrottle is very coarse.
https://chromium-review.git.corp.google.com/c/chromium/src/+/7761929/comment/d11ccb11_b63dae2b/
Related details
- Danil Somsikov
- Takashi Toyoshima
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. |
Danil Somsikov (Gerrit)
Danil Somsikov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Danil Somsikov
| Code-Review | +1 |
1 comment
Related details
- Ken Okada
- Takashi Toyoshima
Takashi Toyoshima (Gerrit)
Takashi Toyoshima voted and added 2 comments![]( "Open in Gerrit")
Votes added by Takashi Toyoshima
| Code-Review | +1 |
2 comments
// Copyright 2026 The Chromium AuthorsKen Okadajust in case, why this is implemented as a URLLoaderThrottle rather than NavigationThrottle? Are you interested in subresource load hooks too?
Good question. It's because the hooks of NavigationThrottle is very coarse.
https://chromium-review.git.corp.google.com/c/chromium/src/+/7761929/comment/d11ccb11_b63dae2b/
Ah, interesting. WillStartNavigation is too early, and there is no good timing hook for this. OK, SGTM.
// Only non prerender navigations are targets.optional: maybe non-prerender?
Related details
- Ken Okada
Code-Coverage
Code-Owners
Ken Okada (Gerrit)
Ken Okada voted and added 1 comment![]( "Open in Gerrit")
Votes added by Ken Okada
| Commit-Queue | +2 |
1 comment
// Copyright 2026 The Chromium AuthorsKen Okadajust in case, why this is implemented as a URLLoaderThrottle rather than NavigationThrottle? Are you interested in subresource load hooks too?
Takashi ToyoshimaGood question. It's because the hooks of NavigationThrottle is very coarse.
https://chromium-review.git.corp.google.com/c/chromium/src/+/7761929/comment/d11ccb11_b63dae2b/
Ah, interesting. WillStartNavigation is too early, and there is no good timing hook for this. OK, SGTM.
Ken Okada (Gerrit)
Ken Okada voted and added 1 comment![]( "Open in Gerrit")
Votes added by Ken Okada
| Commit-Queue | +2 |
1 comment
// Only non prerender navigations are targets.Ken Okadaoptional: maybe non-prerender?
Done
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
24 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: content/browser/preloading/prefetch/cancel_unrelated_prefetch_url_loader_throttle.cc
Insertions: 1, Deletions: 1.
@@ -28,7 +28,7 @@
return nullptr;
}
- // Only non prerender navigations are targets.
+ // Only non-prerender navigations are targets.
if (!frame_tree_node->IsOutermostMainFrame()) {
return nullptr;
```
Change information
Prefetch: Cancel unrelated prefetch
Triggering prefetch is costly. The most impactful scenario is when
prefetch is triggered, and navigation starts immediately afterward, but
the prefetch is not used for navigation and then becomes a network
conflict.
This CL tries to mitigate it by introducing a cancel mechanism:
- Records prefetches that a navigation is claiming:
`NavigationClaimingPrefetches`.
- Cancels unrelated, not completed prefetches just before starting
network request of the navigation:
`CancelUnrelatedPrefetchURLLoaderThrottle`.
This CL also adds a feature flag `PrefetchCancelUnrelatedPrefetch` to
control it.
- M content/browser/BUILD.gn
- M content/browser/devtools/protocol/preload_handler.cc
- M content/browser/loader/url_loader_throttles.cc
- A content/browser/preloading/prefetch/cancel_unrelated_prefetch_url_loader_throttle.cc
- A content/browser/preloading/prefetch/cancel_unrelated_prefetch_url_loader_throttle.h
- M content/browser/preloading/prefetch/nav_prefetch_browsertest.cc
- M content/browser/preloading/prefetch/prefetch_container.cc
- M content/browser/preloading/prefetch/prefetch_features.cc
- M content/browser/preloading/prefetch/prefetch_features.h
- M content/browser/preloading/prefetch/prefetch_service.cc
- M content/browser/preloading/prefetch/prefetch_service.h
- M content/browser/preloading/prefetch/prefetch_status.h
- M third_party/blink/public/devtools_protocol/domains/Preload.pdl
- M third_party/blink/web_tests/TestLists/chrome.filter
- M third_party/blink/web_tests/TestLists/content_shell.filter
- M third_party/blink/web_tests/VirtualTestSuites
- A third_party/blink/web_tests/virtual/prefetch-cancel-unrelated-prefetch/README.md
- M tools/metrics/histograms/enums.xml
Code-Review: +1 by Jonathan Lee, +1 by Takashi Toyoshima, +1 by Hiroshige Hayashizaki, +1 by Hiroki Nakagawa, +1 by Danil Somsikov
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Anthi Orfanou (Gerrit)
Anthi Orfanou added 1 comment![]( "Open in Gerrit")
Clank gardener here: Two tests from this CL fail cosnsitently on builder https://ci.chromium.org/ui/p/chromium/builders/ci/android-bfcache-rel/40479/overview
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Anthi Orfanou (Gerrit)
Anthi Orfanou added 1 comment![]( "Open in Gerrit")
Clank gardener here: Two tests from this CL fail cosnsitently on builder https://ci.chromium.org/ui/p/chromium/builders/ci/android-bfcache-rel/40479/overview
@ken...@chromium.org this is a rather big CL so I am a bit hesitant to revert it, on the other hand since the test is new and broken on a builder it might be the cleanest way.
Related details
- Ken Okada
Anthi Orfanou (Gerrit)
Anthi Orfanou has created a revert of this change![]( "Open in Gerrit")
Related details
- Ken Okada
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |