[Connection-Allowlist] Non-optional network_restrictions_id [chromium/src : main]
Shivani Sharma (Gerrit)
Shivani Sharma added 1 comment![]( "Open in Gerrit")
PTAL at the overall change, thanks!
Related details
- Andrew Verge
- Xiaochen Zhou
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrew Verge (Gerrit)
Andrew Verge added 7 comments![]( "Open in Gerrit")
/*network_restrictions_id=*/base::UnguessableToken(),Should this be GetTestNetworkRestrictionsId?
network_restrictions_id_(base::UnguessableToken()) {
TRACE_EVENT("navigation", "NavigationRequest::NavigationRequest",Please fix this WARNING reported by ClangTidy: check: readability-redundant-member-init
initializer for member 'network_restri...
check: readability-redundant-member-init
initializer for member 'network_restrictions_id_' is redundant (https://clang.llvm.org/extra/clang-tidy/checks/readability/redundant-member-init.html)
(Note: You can add `Skip-Clang-Tidy-Checks: readability-redundant-member-init` footer to the CL description to skip the check)
(Lint observed on `linux-clang-tidy-rel`, but not on `android-clang-tidy-rel` or `mac-clang-tidy-rel`)
if (navigation_request->IsSameDocument()) {In this case, should we set the ID on the request to be the ID of the current document? If we're trying to make the ID non-optional, I do think it's beneficial to have it populated with the "right" value for consistency, in places where we can predict what that value is.
/*network_restrictions_id=*/network::GetTODONetworkRestrictionsId()),Note that here and below, the current Service Worker CL I have up for review will implement these.
// required by the interface but has not been implemented yet.Maybe add a comment here that we'll clean up this function eventually, unlike the other two.
mojo_base.mojom.UnguessableToken network_restrictions_id);Not blocking for this CL, but now that this is not optional, we should consider moving this higher up in the arguments list given its importance (I got this feedback on one of my Websocket/Webtransport CLs this week.)
TEST_F(WebSocketFactoryTest, CreateWebSocketBypassWithNoOpRestrictionsID) {Do we need these tests? Seems a little out of place to test the NoOp ID just in this one spot.
Related details
- Shivani Sharma
- Xiaochen Zhou
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. |
Xiaochen Zhou (Gerrit)
Xiaochen Zhou added 2 comments![]( "Open in Gerrit")
if (!IsNetworkForNetworkRestrictionsIdAndUrlAllowed(network_restrictions_id,Non-blocking comment: In IsNetworkForNetworkRestrictionsIdAndUrlAllowed, perhaps consider early return if the the id == GetNoOpNetworkRestrictionsId(), so we can avoid a map lookup.
const base::UnguessableToken& GetNoOpNetworkRestrictionsId();We would like to require developers to choose one of the followings for network restriction id param:
- The context's ID.
- Or use one of the 3 functions here.
But passing a `base::UnguessableToken::Null()` or `base::UnguessableToken{}` still compiles. Developers not aware of CA perhaps think this is reasonable.
Perhaps we can have a wrapper class around base::UnguessableToken that disables the default constructor (marking it private). And it offers factory methods like these 3 and force developer to choose one.
A lot more work is required though for adding the wrapper class.
Related details
- Shivani Sharma
Shivani Sharma (Gerrit)
Shivani Sharma added 9 comments![]( "Open in Gerrit")
/*network_restrictions_id=*/base::UnguessableToken(),Shivani SharmaShould this be GetTestNetworkRestrictionsId?
Done
network_restrictions_id_(base::UnguessableToken()) {
TRACE_EVENT("navigation", "NavigationRequest::NavigationRequest",Please fix this WARNING reported by ClangTidy: check: readability-redundant-member-init
initializer for member 'network_restri...
check: readability-redundant-member-init
initializer for member 'network_restrictions_id_' is redundant (https://clang.llvm.org/extra/clang-tidy/checks/readability/redundant-member-init.html)
(Note: You can add `Skip-Clang-Tidy-Checks: readability-redundant-member-init` footer to the CL description to skip the check)
(Lint observed on `linux-clang-tidy-rel`, but not on `android-clang-tidy-rel` or `mac-clang-tidy-rel`)
Done
In this case, should we set the ID on the request to be the ID of the current document? If we're trying to make the ID non-optional, I do think it's beneficial to have it populated with the "right" value for consistency, in places where we can predict what that value is.
Done
/*network_restrictions_id=*/network::GetTODONetworkRestrictionsId()),Note that here and below, the current Service Worker CL I have up for review will implement these.
ack
if (!IsNetworkForNetworkRestrictionsIdAndUrlAllowed(network_restrictions_id,Non-blocking comment: In IsNetworkForNetworkRestrictionsIdAndUrlAllowed, perhaps consider early return if the the id == GetNoOpNetworkRestrictionsId(), so we can avoid a map lookup.
// required by the interface but has not been implemented yet.Maybe add a comment here that we'll clean up this function eventually, unlike the other two.
Done
const base::UnguessableToken& GetNoOpNetworkRestrictionsId();We would like to require developers to choose one of the followings for network restriction id param:
- The context's ID.
- Or use one of the 3 functions here.
But passing a `base::UnguessableToken::Null()` or `base::UnguessableToken{}` still compiles. Developers not aware of CA perhaps think this is reasonable.
Perhaps we can have a wrapper class around base::UnguessableToken that disables the default constructor (marking it private). And it offers factory methods like these 3 and force developer to choose one.
A lot more work is required though for adding the wrapper class.
Created 520464337 and added a TODO here. I think it will be a nice addition but ok to be unblocking for launch.
mojo_base.mojom.UnguessableToken network_restrictions_id);Not blocking for this CL, but now that this is not optional, we should consider moving this higher up in the arguments list given its importance (I got this feedback on one of my Websocket/Webtransport CLs this week.)
ack
TEST_F(WebSocketFactoryTest, CreateWebSocketBypassWithNoOpRestrictionsID) {Do we need these tests? Seems a little out of place to test the NoOp ID just in this one spot.
- Andrew Verge
- Xiaochen Zhou
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Xiaochen Zhou (Gerrit)
Xiaochen Zhou voted Code-Review+1![]( "Open in Gerrit")
- Andrew Verge
- Shivani Sharma
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrew Verge (Gerrit)
Andrew Verge voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Shivani Sharma
Shivani Sharma (Gerrit)
Shivani Sharma added 1 comment![]( "Open in Gerrit")
Thanks!
bashi@: PTAL at services/network*, thanks!
alexmos@: PTAL at content/browser*, thanks!
Related details
- Alex Moshchuk
- Kenichi Ishibashi
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kenichi Ishibashi (Gerrit)
Kenichi Ishibashi voted and added 1 comment![]( "Open in Gerrit")
Votes added by Kenichi Ishibashi
| Code-Review | +1 |
1 comment
Related details
- Alex Moshchuk
- Shivani Sharma
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Moshchuk (Gerrit)
Alex Moshchuk voted and added 5 comments![]( "Open in Gerrit")
Votes added by Alex Moshchuk
| Code-Review | +1 |
5 comments
Thanks, content/ LGTM with a few notes.
#include "services/network/public/cpp/constants.h"It seems that this is included and used in both the browser process code and network service process code. Does that mean that the browser process and NS process will have differing values for network::GetNoOpNetworkRestrictionsId(), etc, since we will create a unique token in each process? Is that ok?
base::optional_ref<base::UnguessableToken> network_restrictions_id)Would this also need to be updated to remove the optional at some point?
return base::UnguessableToken::Null();General note: it's invalid to send an empty UnguessableToken over Mojo - [see here](https://source.chromium.org/chromium/chromium/src/+/main:base/unguessable_token.h;l=47-50;drc=cdc524b176956e1aa3d4bb4e3b4d7b156b6cbaa9). I wonder if that can cause any issues if this is read for an initial empty document of a brand new tab, or something where we don't otherwise create or inherit the network restrictions ID. I guess that document is unlikely to be scripted by another frame to do a network request that would need this, but it could potentially happen in a corner case via extension content scripts or DevTools.
So I wonder if it should be invalid to return a null token here.
/*network_restrictions_id=*/base::UnguessableToken());GetTestNetworkRestrictionsId()?
Related details
- Shivani Sharma
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
Shivani Sharma (Gerrit)
Shivani Sharma added 4 comments![]( "Open in Gerrit")
#include "services/network/public/cpp/constants.h"It seems that this is included and used in both the browser process code and network service process code. Does that mean that the browser process and NS process will have differing values for network::GetNoOpNetworkRestrictionsId(), etc, since we will create a unique token in each process? Is that ok?
That's a great point.
The Test and TODO constants are ok to be in both processes with different values.
The NoOp constant should only be necessary for populating the id in the browser process and network service should be agnostic to the value. If we tried to do early return based on NoOp before checking the network_restriction_ids_ map in the n/w service then it wouldn't really be working on the right NoOp value and other than that there is no use for it in the n/w service. Moving it to the content/browser layer makes sense but some of the layers like device/ cannot depend on content/ so instead I am adding a dcheck for it to only be invoked in the browser process.
base::optional_ref<base::UnguessableToken> network_restrictions_id)Would this also need to be updated to remove the optional at some point?
Done
return base::UnguessableToken::Null();General note: it's invalid to send an empty UnguessableToken over Mojo - [see here](https://source.chromium.org/chromium/chromium/src/+/main:base/unguessable_token.h;l=47-50;drc=cdc524b176956e1aa3d4bb4e3b4d7b156b6cbaa9). I wonder if that can cause any issues if this is read for an initial empty document of a brand new tab, or something where we don't otherwise create or inherit the network restrictions ID. I guess that document is unlikely to be scripted by another frame to do a network request that would need this, but it could potentially happen in a corner case via extension content scripts or DevTools.
So I wonder if it should be invalid to return a null token here.
Thanks! I was adding the changes to always have this be initialized to no-op id in the follow up CL, but actually moving these changes here so this CL is safe. Also added DCHECKs for id to be non-empty before the network context mojo calls are called.
/*network_restrictions_id=*/base::UnguessableToken());Shivani SharmaGetTestNetworkRestrictionsId()?
Done
Related details
- Alex Moshchuk
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. |
Xiaochen Zhou (Gerrit)
Xiaochen Zhou voted and added 2 comments![]( "Open in Gerrit")
Votes added by Xiaochen Zhou
| Code-Review | +1 |
2 comments
* **Selecting the ID at Calling Sites**:It seems the note about `GetTODONetworkRestrictionsId()` has been removed, is it intentional?
Related details
- Alex Moshchuk
- Andrew Verge
- Kenichi Ishibashi
- Shivani Sharma
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. |
Shivani Sharma (Gerrit)
Shivani Sharma added 1 comment![]( "Open in Gerrit")
* **Selecting the ID at Calling Sites**:It seems the note about `GetTODONetworkRestrictionsId()` has been removed, is it intentional?
Yes I am thinking that TODO will be removed soon after CA impl completion so doesn't really need to be in the doc.
Related details
- Alex Moshchuk
- Andrew Verge
- Kenichi Ishibashi
Alex Moshchuk (Gerrit)
Alex Moshchuk voted and added 5 comments![]( "Open in Gerrit")
Votes added by Alex Moshchuk
| Code-Review | +1 |
5 comments
#include "services/network/public/cpp/constants.h"Shivani SharmaIt seems that this is included and used in both the browser process code and network service process code. Does that mean that the browser process and NS process will have differing values for network::GetNoOpNetworkRestrictionsId(), etc, since we will create a unique token in each process? Is that ok?
That's a great point.
The Test and TODO constants are ok to be in both processes with different values.The NoOp constant should only be necessary for populating the id in the browser process and network service should be agnostic to the value. If we tried to do early return based on NoOp before checking the network_restriction_ids_ map in the n/w service then it wouldn't really be working on the right NoOp value and other than that there is no use for it in the n/w service. Moving it to the content/browser layer makes sense but some of the layers like device/ cannot depend on content/ so instead I am adding a dcheck for it to only be invoked in the browser process.
Ack. I'm not sure if there's a better way to check for the browser process from the network service code (I suppose we can't access switches::kProcessType :/); I'll leave that part to network service reviewers.
I noticed that this will probably come up for Xiaochen's suggestion in https://chromium-review.git.corp.google.com/c/chromium/src/+/7899247/comment/a32c8401_91fbd03f/ to return early for NoOp case. I agree that a cleaner way forward would be with some sort of wrapper or union type (as crbug.com/520464337 suggests), where the bypass_allowlist case is more explicit, or sending the NoOp token to the network service if it needs to be in sync there.
DCHECK(!network_restrictions_id.is_empty());nit: prefer CHECK in new code
DCHECK(!network_restrictions_id.is_empty());(CHECKs here as well)
return base::UnguessableToken::Null();Shivani SharmaGeneral note: it's invalid to send an empty UnguessableToken over Mojo - [see here](https://source.chromium.org/chromium/chromium/src/+/main:base/unguessable_token.h;l=47-50;drc=cdc524b176956e1aa3d4bb4e3b4d7b156b6cbaa9). I wonder if that can cause any issues if this is read for an initial empty document of a brand new tab, or something where we don't otherwise create or inherit the network restrictions ID. I guess that document is unlikely to be scripted by another frame to do a network request that would need this, but it could potentially happen in a corner case via extension content scripts or DevTools.
So I wonder if it should be invalid to return a null token here.
Thanks! I was adding the changes to always have this be initialized to no-op id in the follow up CL, but actually moving these changes here so this CL is safe. Also added DCHECKs for id to be non-empty before the network context mojo calls are called.
Acknowledged
DCHECKBrowserProcess();s/DCHECK/CHECK/g here as well, and it should also probably have a comment about why this is only safe to call from the browser process (i.e., covering the shared token discussion and maybe a TODO pointing to the bug you filed).
Related details
- Andrew Verge
- Kenichi Ishibashi
Shivani Sharma (Gerrit)
Shivani Sharma added 4 comments![]( "Open in Gerrit")
#include "services/network/public/cpp/constants.h"Shivani SharmaIt seems that this is included and used in both the browser process code and network service process code. Does that mean that the browser process and NS process will have differing values for network::GetNoOpNetworkRestrictionsId(), etc, since we will create a unique token in each process? Is that ok?
Alex MoshchukThat's a great point.
The Test and TODO constants are ok to be in both processes with different values.The NoOp constant should only be necessary for populating the id in the browser process and network service should be agnostic to the value. If we tried to do early return based on NoOp before checking the network_restriction_ids_ map in the n/w service then it wouldn't really be working on the right NoOp value and other than that there is no use for it in the n/w service. Moving it to the content/browser layer makes sense but some of the layers like device/ cannot depend on content/ so instead I am adding a dcheck for it to only be invoked in the browser process.
Ack. I'm not sure if there's a better way to check for the browser process from the network service code (I suppose we can't access switches::kProcessType :/); I'll leave that part to network service reviewers.
I noticed that this will probably come up for Xiaochen's suggestion in https://chromium-review.git.corp.google.com/c/chromium/src/+/7899247/comment/a32c8401_91fbd03f/ to return early for NoOp case. I agree that a cleaner way forward would be with some sort of wrapper or union type (as crbug.com/520464337 suggests), where the bypass_allowlist case is more explicit, or sending the NoOp token to the network service if it needs to be in sync there.
Acknowledged
DCHECK(!network_restrictions_id.is_empty());nit: prefer CHECK in new code
Done but with NotFatalUntil for M165 (over 6 months from now) to be on the safe side. Hope that's fine.
DCHECK(!network_restrictions_id.is_empty());Shivani Sharma(CHECKs here as well)
Done
s/DCHECK/CHECK/g here as well, and it should also probably have a comment about why this is only safe to call from the browser process (i.e., covering the shared token discussion and maybe a TODO pointing to the bug you filed).
- Alex Moshchuk
- Andrew Verge
- Kenichi Ishibashi
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. |
Andrew Verge (Gerrit)
Andrew Verge voted Code-Review+1![]( "Open in Gerrit")
- Alex Moshchuk
- Kenichi Ishibashi
- Shivani Sharma
Kenichi Ishibashi (Gerrit)
Kenichi Ishibashi voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Alex Moshchuk
- Shivani Sharma
Shivani Sharma (Gerrit)
Shivani Sharma added 1 comment![]( "Open in Gerrit")
kenrb@: PTAL at device/fido/* and services/network/public/mojom/network_context.mojom, thanks!
Related details
- Alex Moshchuk
- Ken Buchanan
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 Buchanan (Gerrit)
Ken Buchanan voted and added 1 comment![]( "Open in Gerrit")
Votes added by Ken Buchanan
| Code-Review | +1 |
1 comment
- Alex Moshchuk
- Shivani Sharma
Shivani Sharma (Gerrit)
Shivani Sharma added 1 comment![]( "Open in Gerrit")
- Alex Moshchuk
- Ryan Sturm
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. |
Ryan Sturm (Gerrit)
Ryan Sturm voted Code-Review+1![]( "Open in Gerrit")
- Alex Moshchuk
- Shivani Sharma
Shivani Sharma (Gerrit)
Shivani Sharma added 1 comment![]( "Open in Gerrit")
dullweber@, PTAL at *private_ai*, thanks!
My understanding is that PrivateAI WebSocket connections are browser-wide operations not associated with any page/frame, so no Connection Allowlist restrictions should apply, thus the CL adds the no-op network_restricitons_id to it.
Related details
- Alex Moshchuk
- Christian Dullweber
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. |
Shivani Sharma (Gerrit)
Shivani Sharma added 1 comment![]( "Open in Gerrit")
torne@: PTAL at aw_preconnector, thanks!
My understanding is that the Preconnect in this file is a Preconnect initiated at the Profile level and hence is out of scope of connection allowlists which applies to document/workers initiated Preconnect and other requests. Hence passing a No-op network_restrictions_id.
(https://wicg.github.io/connection-allowlists/#threat-model)
Related details
- Alex Moshchuk
- Christian Dullweber
- Richard Coles
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. |
Richard Coles (Gerrit)
Richard Coles voted and added 1 comment![]( "Open in Gerrit")
Votes added by Richard Coles
| Code-Review | +1 |
1 comment
- Alex Moshchuk
- Christian Dullweber
- Shivani Sharma
Alex Moshchuk (Gerrit)
Alex Moshchuk voted and added 1 comment![]( "Open in Gerrit")
Votes added by Alex Moshchuk
| Code-Review | +1 |
1 comment
DCHECK(!network_restrictions_id.is_empty());Shivani Sharmanit: prefer CHECK in new code
Done but with NotFatalUntil for M165 (over 6 months from now) to be on the safe side. Hope that's fine.
- Christian Dullweber
- Shivani Sharma
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
Shivani Sharma (Gerrit)
Shivani Sharma voted and added 1 comment![]( "Open in Gerrit")
Votes added by Shivani Sharma
| Commit-Queue | +1 |
1 comment
chrome-sig...@google.com: PTAL at components/signin/internal/identity_manager/account_fetcher_factory_gaia.cc, thanks!
My understanding is that account capabilities fetches are browser-wide sign-in operations not associated with any page/frame, so no Connection Allowlist restrictions should apply. Hence passing a no-op network_restrictions_id.
Related details
- Christian Dullweber
- Chrome Signin Team
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Message from gwsq![]( "Open in Gerrit")
Reviewer source(s):
my...@google.com is from context(googleclient/chrome/chromium_gwsq/components/signin/config.gwsq)
Related details
- Christian Dullweber
- Menghan Yang
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Shivani Sharma (Gerrit)
Shivani Sharma added 1 comment![]( "Open in Gerrit")
Adding chrome-sig...@google.com again as the automatically assigned reviewer was OOO
Related details
- Christian Dullweber
- Chrome Signin Team
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Message from gwsq![]( "Open in Gerrit")
Shadowed: arthurm...@google.com
Reviewer source(s):
arthurm...@google.com, msa...@chromium.org is from context(googleclient/chrome/chromium_gwsq/components/signin/config.gwsq)
Related details
- Arthur Milchior
- Christian Dullweber
- Mihai Sardarescu
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arthur Milchior (Gerrit)
Arthur Milchior added 1 comment![]( "Open in Gerrit")
it seems i own no file here, so let’s remove myself.
Related details
- Christian Dullweber
- Mihai Sardarescu
- Shivani Sharma
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Christian Dullweber (Gerrit)
Christian Dullweber voted Code-Review+1![]( "Open in Gerrit")
- Mihai Sardarescu
- Shivani Sharma
Mihai Sardarescu (Gerrit)
Mihai Sardarescu voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Shivani Sharma
Code-Coverage
Code-Owners
Mihai Sardarescu (Gerrit)
Mihai Sardarescu added 1 comment![]( "Open in Gerrit")
LGTM for changes in account_fetcher_factory_gaia.cc
Shivani Sharma (Gerrit)
Shivani Sharma voted and added 1 comment![]( "Open in Gerrit")
Votes added by Shivani Sharma
| Commit-Queue | +2 |
1 comment
Thanks all for the review!
Related details
Shivani Sharma (Gerrit)
Shivani Sharma voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
[Connection-Allowlist] Non-optional network_restrictions_id
This CL should have no functional changes.
This CL is the first in a series that make network_restrictions_id
non-optional so that consumers (existing and future) can make a
well thought-out decision for how Connection Allowlists apply to their
network request. Next CL will also make URLLoaderFactory params
network_restriction_id to be non-optional.
It does the following:
- Always creates a network_restrictions_id for a document (workers
already did that).
- Adding `GetNoOpNetworkRestrictionsId()` for cases where CA should not
be applied.
- Adding `GetTODONetworkRestrictionsId()` and
`GetTestNetworkRestrictionsId()` to
explicitly categorize placeholder IDs used in unimplemented code paths
and tests, respectively.
- Updating signatures of NetworkContext functions to pass the
non-optional network restrictions ID.
Bug: 492456054
- M android_webview/browser/aw_preconnector.cc
- M chrome/browser/navigation_predictor/search_engine_preconnector.cc
- M chrome/browser/net/network_context_configuration_browsertest.cc
- M chrome/browser/net/websocket_browsertest.cc
- M chrome/browser/predictors/loading_predictor.cc
- M chrome/browser/predictors/loading_predictor_browsertest.cc
- M chrome/browser/predictors/loading_predictor_unittest.cc
- M chrome/browser/predictors/network_hints_handler_impl.cc
- M components/private_ai/websocket_client.cc
- M components/private_ai/websocket_client_unittest.cc
- M components/signin/internal/identity_manager/account_fetcher_factory_gaia.cc
- M content/browser/compute_pressure/pressure_service_for_worker_unittest.cc
- M content/browser/devtools/protocol/devtools_network_resource_loader_browsertest.cc
- M content/browser/devtools/protocol/network_handler.cc
- M content/browser/devtools/service_worker_devtools_agent_host.cc
- M content/browser/interest_group/ad_auction_service_impl.cc
- M content/browser/loader/navigation_early_hints_manager.cc
- M content/browser/loader/navigation_early_hints_manager_unittest.cc
- M content/browser/preloading/preconnect/preconnect_manager_impl.cc
- M content/browser/preloading/preconnect/preconnect_manager_impl.h
- M content/browser/preloading/preconnect/preconnect_manager_impl_unittest.cc
- M content/browser/renderer_host/document_associated_data.cc
- M content/browser/renderer_host/document_associated_data.h
- M content/browser/renderer_host/navigation_request.cc
- M content/browser/renderer_host/navigation_request.h
- M content/browser/renderer_host/network_restrictions_navigation_throttle.cc
- M content/browser/renderer_host/render_frame_host_impl.cc
- M content/browser/renderer_host/render_frame_host_impl.h
- M content/browser/renderer_host/render_frame_host_impl_browsertest.cc
- M content/browser/service_worker/service_worker_register_job.cc
- M content/browser/storage_partition_impl.cc
- M content/browser/websockets/websocket_connector_impl.cc
- M content/browser/websockets/websocket_connector_impl.h
- M content/browser/webtransport/web_transport_connector_impl.cc
- M content/browser/webtransport/web_transport_connector_impl.h
- M content/browser/worker_host/dedicated_worker_host.cc
- M content/browser/worker_host/dedicated_worker_host.h
- M content/browser/worker_host/dedicated_worker_host_factory_impl.cc
- M content/browser/worker_host/dedicated_worker_host_factory_impl.h
- M content/browser/worker_host/dedicated_worker_service_impl_unittest.cc
- M content/browser/worker_host/worker_browsertest.cc
- M content/browser/worker_host/worker_script_fetcher.cc
- M content/browser/worker_host/worker_script_fetcher.h
- M content/public/browser/preconnect_manager.h
- M content/public/browser/render_frame_host.h
- M device/fido/BUILD.gn
- M device/fido/cable/fido_tunnel_device.cc
- M device/fido/cable/v2_authenticator.cc
- M device/fido/cable/v2_discovery_unittest.cc
- M device/fido/cable/v2_test_util.cc
- M device/fido/enclave/enclave_websocket_client.cc
- M docs/connection_allowlist_design.md
- M services/network/network_context.cc
- M services/network/network_context.h
- M services/network/network_context_unittest.cc
- M services/network/public/cpp/constants.cc
- M services/network/public/cpp/constants.h
- M services/network/public/mojom/network_context.mojom
- M services/network/test/test_network_context.h
- M services/network/web_transport_unittest.cc
- M services/network/websocket_factory.cc
- M services/network/websocket_factory.h
- M services/network/websocket_factory_unittest.cc
Code-Review: +1 by Kenichi Ishibashi, +1 by Mihai Sardarescu, +1 by Christian Dullweber, +1 by Andrew Verge, +1 by Richard Coles, +1 by Ryan Sturm, +1 by Xiaochen Zhou, +1 by Alex Moshchuk, +1 by Ken Buchanan
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |