[DBSC] Fix request deferral logic for non-initiating requests [chromium/src : main]
0 views
Skip to first unread message
Alex Ilin (Gerrit)
Apr 9, 2026, 1:04:24 PM (5 days ago) Apr 9
to Alex Ilin, Daniel Rubery, thefrog, chromium...@chromium.org, net-r...@chromium.org
Attention needed from Daniel Rubery
Alex Ilin added 2 comments![]( "Open in Gerrit")
Patchset-level comments
File net/url_request/url_request_http_job.cc
Line 1420, Patchset 1 (Latest):
(triggered_refresh ||
result != device_bound_sessions::RefreshResult::kRefreshed)) {Alex Ilin . unresolved
I don't particularly like leaking this logic into `URLRequestHttpJob`. A better approach would probably be to split `device_bound_sessions::RefreshResult::kRefreshed` into `kRefreshedByOther` and `kRefreshedByThis`, and then contain this logic within `ShouldDefer()`.
Let me know if you want me to explore this approach.
Related details
Attention is currently required from:
- Daniel Rubery
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. |
Alex Ilin (Gerrit)
Apr 10, 2026, 4:15:07 AM (4 days ago) Apr 10
to Alex Ilin, Chromium LUCI CQ, Daniel Rubery, thefrog, chromium...@chromium.org, net-r...@chromium.org
Attention needed from Daniel Rubery
Alex Ilin added 1 comment![]( "Open in Gerrit")
File net/url_request/url_request_http_job.cc
Line 1420, Patchset 1 (Latest):
(triggered_refresh ||
result != device_bound_sessions::RefreshResult::kRefreshed)) {Alex Ilin . unresolved
I don't particularly like leaking this logic into `URLRequestHttpJob`. A better approach would probably be to split `device_bound_sessions::RefreshResult::kRefreshed` into `kRefreshedByOther` and `kRefreshedByThis`, and then contain this logic within `ShouldDefer()`.
Let me know if you want me to explore this approach.
Alex Ilin
I implemented an alternative approach in https://crrev.com/c/7747885. I like it more than the current CL.
Related details
Attention is currently required from:
- Daniel Rubery
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. |
Daniel Rubery (Gerrit)
Apr 13, 2026, 12:38:11 PM (23 hours ago) Apr 13
to Alex Ilin, chromiu...@luci-project-accounts.iam.gserviceaccount.com, thefrog, chromium...@chromium.org, net-r...@chromium.org
Attention needed from Alex Ilin
Daniel Rubery added 1 comment![]( "Open in Gerrit")
File net/url_request/url_request_http_job.cc
Line 1420, Patchset 1 (Latest):
(triggered_refresh ||
result != device_bound_sessions::RefreshResult::kRefreshed)) {Alex Ilin . resolved
Alex IlinI don't particularly like leaking this logic into `URLRequestHttpJob`. A better approach would probably be to split `device_bound_sessions::RefreshResult::kRefreshed` into `kRefreshedByOther` and `kRefreshedByThis`, and then contain this logic within `ShouldDefer()`.
Let me know if you want me to explore this approach.
I implemented an alternative approach in https://crrev.com/c/7747885. I like it more than the current CL.
Attention is currently required from:
- Alex Ilin
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Ilin (Gerrit)
Apr 13, 2026, 12:55:29 PM (23 hours ago) Apr 13
to Alex Ilin, Chromium IPC Reviews, Andrey Kosyakov, Daniel Rubery, android-bu...@system.gserviceaccount.com, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, devtools...@chromium.org, devtools-re...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, asvitkine...@chromium.org, ipc-securi...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org
Attention needed from Andrey Kosyakov, Chromium IPC Reviews and Daniel Rubery
New activity on the change![]( "Open in Gerrit")
Related details
Attention is currently required from:
- Andrey Kosyakov
- Chromium IPC Reviews
- Daniel Rubery
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Apr 13, 2026, 12:57:09 PM (23 hours ago) Apr 13
to Alex Ilin, Chromium IPC Reviews, Mike West, Andrey Kosyakov, Daniel Rubery, android-bu...@system.gserviceaccount.com, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, devtools...@chromium.org, devtools-re...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, asvitkine...@chromium.org, ipc-securi...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org
Attention needed from Andrey Kosyakov, Daniel Rubery and Mike West
Message from gwsq![]( "Open in Gerrit")
From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: mk...@chromium.org
📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).
IPC reviewer(s): mk...@chromium.org
Reviewer source(s):
mk...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)
Related details
Attention is currently required from:
- Andrey Kosyakov
- Daniel Rubery
- Mike West
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrey Kosyakov (Gerrit)
Apr 13, 2026, 1:11:28 PM (23 hours ago) Apr 13
to Alex Ilin, Chromium IPC Reviews, Mike West, Daniel Rubery, android-bu...@system.gserviceaccount.com, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, devtools...@chromium.org, devtools-re...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, asvitkine...@chromium.org, ipc-securi...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org
Attention needed from Alex Ilin, Daniel Rubery and Mike West
Andrey Kosyakov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Andrey Kosyakov
| Code-Review | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Alex Ilin
- Daniel Rubery
- Mike West
Mike West (Gerrit)
4:48 AM (7 hours ago) 4:48 AM
to Alex Ilin, Andrey Kosyakov, Chromium IPC Reviews, Daniel Rubery, android-bu...@system.gserviceaccount.com, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, devtools...@chromium.org, devtools-re...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, asvitkine...@chromium.org, ipc-securi...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org
Attention needed from Alex Ilin and Daniel Rubery
Mike West voted and added 1 comment![]( "Open in Gerrit")
Votes added by Mike West
| Code-Review | +1 |
1 comment
Patchset-level comments
Mike West . resolved
mojo lgtm
Related details
Attention is currently required from:
- Alex Ilin
- Daniel Rubery
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Ilin (Gerrit)
8:16 AM (3 hours ago) 8:16 AM
to Alex Ilin, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Daniel Rubery, thefrog, chromium...@chromium.org, net-r...@chromium.org
Alex Ilin abandoned this change.
Abandoned
Went with an alternative approach: https://crrev.com/c/7747885
Alex Ilin abandoned this change![]( "Open in Gerrit")
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Ilin (Gerrit)
9:00 AM (3 hours ago) 9:00 AM
to Alex Ilin, Mike West, Andrey Kosyakov, Chromium IPC Reviews, Daniel Rubery, android-bu...@system.gserviceaccount.com, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, devtools...@chromium.org, devtools-re...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, asvitkine...@chromium.org, ipc-securi...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org
Attention needed from Daniel Rubery
Alex Ilin voted Commit-Queue+2![]( "Open in Gerrit")
Attention is currently required from:
- Daniel Rubery
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-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)
9:59 AM (2 hours ago) 9:59 AM
to Alex Ilin, Mike West, Andrey Kosyakov, Chromium IPC Reviews, Daniel Rubery, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, devtools...@chromium.org, devtools-re...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, asvitkine...@chromium.org, ipc-securi...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org
chromiu...@luci-project-accounts.iam.gserviceaccount.com submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[DBSC] Fix request deferral logic for non-initiating requests
When multiple requests are deferred waiting for a DBSC session refresh,
the refresh context might be different from a waiting request's context
and thus the refresh might not set all of the required cookies.
Previously, all requests in the batch would be released after a refresh
completes, even if a bound cookie is still missing for some of them.
This CL changes that by allowing deferred requests to trigger a refresh
again if this request wasn't initiating a refresh initially.
Fixed: 500417358
Change-Id: Ia9d1a71a3f45708df80b1ea39f2c0438e2dacc47
Reviewed-by: Daniel Rubery <dru...@chromium.org>
Commit-Queue: Alex Ilin <alex...@chromium.org>
Reviewed-by: Andrey Kosyakov <ca...@chromium.org>
Reviewed-by: Mike West <mk...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1614415}
Files:
- M content/browser/devtools/protocol/network_handler.cc
- M net/device_bound_sessions/refresh_result.h
- M net/device_bound_sessions/session_service_impl.cc
- M net/device_bound_sessions/session_service_impl.h
- M net/device_bound_sessions/session_service_impl_unittest.cc
- M services/network/public/cpp/device_bound_sessions_mojom_traits.cc
- M services/network/public/mojom/device_bound_sessions.mojom
- M third_party/blink/public/devtools_protocol/domains/Network.pdl
- M tools/metrics/histograms/metadata/net/enums.xml
Change size: M
Delta: 9 files changed, 101 insertions(+), 6 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Andrey Kosyakov, +1 by Daniel Rubery, +1 by Mike West
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages