Use HttpRequestHeadersUpdateParams in ThrottlingURLLoader::FollowRedirect() [chromium/src : main]
0 views
Skip to first unread message
Hiroshige Hayashizaki (Gerrit)
May 17, 2026, 4:07:09 PM (2 days ago) May 17
to Kenichi Ishibashi, Chromium LUCI CQ, chromium...@chromium.org, Nate Chapin, blink-re...@chromium.org, blink-...@chromium.org, blink-work...@chromium.org, gavinp...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org
Attention needed from Kenichi Ishibashi
Hiroshige Hayashizaki voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Kenichi Ishibashi
Submit Requirements:
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)
May 17, 2026, 4:21:16 PM (2 days ago) May 17
to Tsuyoshi Horo, Kenichi Ishibashi, Chromium LUCI CQ, chromium...@chromium.org, Nate Chapin, blink-re...@chromium.org, blink-...@chromium.org, blink-work...@chromium.org, gavinp...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org
Attention needed from Kenichi Ishibashi and Tsuyoshi Horo
Hiroshige Hayashizaki voted and added 1 comment![]( "Open in Gerrit")
Votes added by Hiroshige Hayashizaki
| Commit-Queue | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 11 (Latest):
Hiroshige Hayashizaki . resolved
horo@ for `resource_request_sender` changes.
Related details
Attention is currently required from:
- Kenichi Ishibashi
- Tsuyoshi Horo
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tsuyoshi Horo (Gerrit)
May 17, 2026, 7:46:48 PM (2 days ago) May 17
to Hiroshige Hayashizaki, Kenichi Ishibashi, Chromium LUCI CQ, chromium...@chromium.org, Nate Chapin, blink-re...@chromium.org, blink-...@chromium.org, blink-work...@chromium.org, gavinp...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org
Attention needed from Hiroshige Hayashizaki and Kenichi Ishibashi
Tsuyoshi Horo voted and added 2 comments![]( "Open in Gerrit")
Votes added by Tsuyoshi Horo
| Code-Review | +1 |
2 comments
Patchset-level comments
Tsuyoshi Horo . resolved
lgmt
File third_party/blink/renderer/platform/loader/fetch/url_loader/resource_request_sender.cc
Line 414, Patchset 11 (Latest):
} else {
request_info_->url_loader->FollowRedirect(
std::move(request_info_->headers_update_params));
request_info_->headers_update_params.Clear();Tsuyoshi Horo . unresolved
Not directly related to this code change, but I’m slightly concerned that modifying request_info_ after the FollowRedirect() call might lead to a if this (or request_info_) is deleted during that call.
Although it makes the code a bit longer, it might be safer to extract and clear the parameters before the call to avoid any lifetime concerns:
```
auto headers_update_params = std::move(request_info_->headers_update_params);
request_info_->headers_update_params.Clear();
request_info_->url_loader->FollowRedirect(std::move(headers_update_params));
```
That said, I don't have a strong opinion on this, so please feel free to handle it as you prefer, Hayashizaki-san.
Related details
Attention is currently required from:
- Hiroshige Hayashizaki
- Kenichi Ishibashi
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. |
Kenichi Ishibashi (Gerrit)
May 17, 2026, 7:50:06 PM (2 days ago) May 17
to Hiroshige Hayashizaki, Tsuyoshi Horo, Chromium LUCI CQ, chromium...@chromium.org, Nate Chapin, blink-re...@chromium.org, blink-...@chromium.org, blink-work...@chromium.org, gavinp...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org
Attention needed from Hiroshige Hayashizaki
Kenichi Ishibashi voted and added 1 comment![]( "Open in Gerrit")
Votes added by Kenichi Ishibashi
| Code-Review | +1 |
1 comment
Patchset-level comments
Kenichi Ishibashi . resolved
lgtm
Related details
Attention is currently required from:
- Hiroshige Hayashizaki
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. |
Hiroshige Hayashizaki (Gerrit)
May 17, 2026, 9:30:52 PM (2 days ago) May 17
to Kenichi Ishibashi, Tsuyoshi Horo, Chromium LUCI CQ, chromium...@chromium.org, Nate Chapin, blink-re...@chromium.org, blink-...@chromium.org, blink-work...@chromium.org, gavinp...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org
Hiroshige Hayashizaki added 1 comment![]( "Open in Gerrit")
File third_party/blink/renderer/platform/loader/fetch/url_loader/resource_request_sender.cc
Line 414, Patchset 11:
} else {
request_info_->url_loader->FollowRedirect(
std::move(request_info_->headers_update_params));
request_info_->headers_update_params.Clear();Tsuyoshi Horo . resolved
Not directly related to this code change, but I’m slightly concerned that modifying request_info_ after the FollowRedirect() call might lead to a if this (or request_info_) is deleted during that call.
Although it makes the code a bit longer, it might be safer to extract and clear the parameters before the call to avoid any lifetime concerns:
```
auto headers_update_params = std::move(request_info_->headers_update_params);
request_info_->headers_update_params.Clear();
request_info_->url_loader->FollowRedirect(std::move(headers_update_params));
```That said, I don't have a strong opinion on this, so please feel free to handle it as you prefer, Hayashizaki-san.
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)
May 17, 2026, 9:32:05 PM (2 days ago) May 17
to Hiroki Nakagawa, Kenichi Ishibashi, Tsuyoshi Horo, Chromium LUCI CQ, chromium...@chromium.org, Nate Chapin, blink-re...@chromium.org, blink-...@chromium.org, blink-work...@chromium.org, gavinp...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org
Attention needed from Hiroki Nakagawa
Hiroshige Hayashizaki added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 12 (Latest):
Hiroshige Hayashizaki . resolved
+@nhiroki for content/browser/worker_host/worker_script_fetcher.cc OWNER.
Related details
Attention is currently required from:
- Hiroki Nakagawa
Submit Requirements:
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)
May 18, 2026, 9:48:09 PM (2 days ago) May 18
to Hiroshige Hayashizaki, Kenichi Ishibashi, Tsuyoshi Horo, Chromium LUCI CQ, chromium...@chromium.org, Nate Chapin, blink-re...@chromium.org, blink-...@chromium.org, blink-work...@chromium.org, gavinp...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org
Attention needed from Hiroshige Hayashizaki
Hiroki Nakagawa voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Hiroshige Hayashizaki
Submit Requirements:
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)
May 18, 2026, 11:39:19 PM (2 days ago) May 18
to Hiroki Nakagawa, Kenichi Ishibashi, Tsuyoshi Horo, Chromium LUCI CQ, chromium...@chromium.org, Nate Chapin, blink-re...@chromium.org, blink-...@chromium.org, blink-work...@chromium.org, gavinp...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org
Hiroshige Hayashizaki voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
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)
May 19, 2026, 12:30:04 AM (yesterday) May 19
to Hiroshige Hayashizaki, Hiroki Nakagawa, Kenichi Ishibashi, Tsuyoshi Horo, chromium...@chromium.org, Nate Chapin, blink-re...@chromium.org, blink-...@chromium.org, blink-work...@chromium.org, gavinp...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Unreviewed changes
12 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
Commit message:
Use HttpRequestHeadersUpdateParams in ThrottlingURLLoader::FollowRedirect()
Follow-up of https://crrev.com/c/7835601 that introduced
`network::HttpRequestHeadersUpdateParams`.
This CL also introduces `HttpRequestHeadersUpdateParams::MergeFrom()`
and `HttpRequestHeadersUpdateParams::Clear()`.
This CL shouldn't change the behavior (except for minor removal of
`std::string` copying in `ThrottlingURLLoader::MergeRequestHeaders()`,
which doesn't change the functional behavior).
Bug: 511306597, 434292502
Change-Id: I1c5d25a4eecc2bbd51f3ebe5d1e21e87356aae19
Reviewed-by: Hiroki Nakagawa <nhi...@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiro...@chromium.org>
Reviewed-by: Kenichi Ishibashi <ba...@chromium.org>
Reviewed-by: Tsuyoshi Horo <ho...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1632614}
Files:
- M content/browser/loader/keep_alive_url_loader.cc
- M content/browser/loader/navigation_url_loader_impl.cc
- M content/browser/worker_host/worker_script_fetcher.cc
- M services/network/public/cpp/http_request_headers_update_params.cc
- M services/network/public/cpp/http_request_headers_update_params.h
- M third_party/blink/common/loader/throttling_url_loader.cc
- M third_party/blink/common/loader/throttling_url_loader_unittest.cc
- M third_party/blink/public/common/loader/throttling_url_loader.h
- M third_party/blink/renderer/platform/loader/fetch/url_loader/resource_request_sender.cc
- M third_party/blink/renderer/platform/loader/fetch/url_loader/resource_request_sender.h
Change size: M
Delta: 10 files changed, 109 insertions(+), 135 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Kenichi Ishibashi, +1 by Tsuyoshi Horo, +1 by Hiroki Nakagawa
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages