Skip to content

Commit

Permalink
Remove calls to IsolationInfo::frame_origin(
Browse files Browse the repository at this point in the history
Triple keying network state partitions resulted in unacceptable increased latency. This CL updates call sites of IsolationInfo::frame_origin() to account for the fact that it might not return the actual frame_origin() as we explore double keying network state.

request_initiator in origin_policy was reverted to it's previous state of using the fetch_url_ value. This is known to be incorrect but the decision was made to remove this codepath in a follow up rather than break expectations of tests since origin_policy is not used. Context comment on this CL: https://chromium-review.googlesource.com/c/chromium/src/+/3442039/comments/837a571d_06303810

Full context: https://docs.google.com/document/d/1hm-t6sEn2xR7YN_R1qS1zTWYhyunCK6HoGSrssoQd3A/edit?usp=sharing

This is part 2 of a 3 part plan to experiment with double keying network state.
1. Remove calls to Storagekey::FromNetworkIsolationInfo. (merged: https://chromium-review.googlesource.com/c/chromium/src/+/3441257)
2. Update call sites of IsolationInfo::frame_origin to use a different way of getting the frame_origin.
3. Set up a feature flag that forces IsolationInfo and NIK frame_origins to empty origins.

Bug: 1294930
Change-Id: Ie6e8826fc502c283f9a68aa350031f35d2fe1cb3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3442039
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Brianna Goldstein <brgoldstein@google.com>
Cr-Commit-Position: refs/heads/main@{#977160}
  • Loading branch information
brgoldstein authored and Chromium LUCI CQ committed Mar 3, 2022
1 parent c445776 commit 777ebb6
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 12 deletions.
11 changes: 3 additions & 8 deletions content/browser/renderer_host/render_frame_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3483,14 +3483,9 @@ void RenderFrameHostImpl::DidNavigate(
navigation_request->GetWebBundleURL().is_valid())
? url::Origin::Create(navigation_request->GetWebBundleURL())
: GetLastCommittedOrigin();
// TODO(https://crbug.com/1164508): Remove this check once origin is computed
// correctly by the browser side.
if (!origin.IsSameOriginWith(
GetIsolationInfoForSubresources().frame_origin().value())) {
isolation_info_ =
ComputeIsolationInfoInternal(origin, isolation_info_.request_type(),
navigation_request->anonymous());
}

isolation_info_ = ComputeIsolationInfoInternal(
origin, isolation_info_.request_type(), navigation_request->anonymous());

// Separately, update the frame's last successful URL except for net error
// pages, since those do not end up in the correct process after transfers
Expand Down
8 changes: 7 additions & 1 deletion net/reporting/reporting_uploader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,13 @@ class ReportingUploaderImpl : public ReportingUploader, URLRequest::Delegate {
// in the case of V1 reporting endpoints, and will be null for V0 reports.
upload->request->set_site_for_cookies(
upload->isolation_info.site_for_cookies());
upload->request->set_initiator(upload->isolation_info.frame_origin());
// Prior to using `isolation_info` directly here we built the
// `upload->network_isolation_key` to create the set the `isolation_info`.
// As experiments roll out to determine whether network partitions should be
// double or triple keyed the isolation_info might have a null value for
// `frame_origin`. Thus we should again get it from `network_isolation_key`
// until we can trust `isolation_info::frame_origin`.
upload->request->set_initiator(upload->report_origin);
upload->request->set_isolation_info(upload->isolation_info);

upload->request->SetExtraRequestHeaderByName(
Expand Down
2 changes: 1 addition & 1 deletion services/network/origin_policy/origin_policy_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ void OriginPolicyFetcher::FetchPolicy(mojom::URLLoaderFactory* factory) {
std::unique_ptr<ResourceRequest> policy_request =
std::make_unique<ResourceRequest>();
policy_request->url = fetch_url_;
policy_request->request_initiator = isolation_info_.frame_origin().value();
policy_request->request_initiator = url::Origin::Create(fetch_url_);
policy_request->credentials_mode = network::mojom::CredentialsMode::kOmit;
policy_request->redirect_mode = network::mojom::RedirectMode::kError;

Expand Down
2 changes: 0 additions & 2 deletions services/network/restricted_cookie_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -356,8 +356,6 @@ RestrictedCookieManager::RestrictedCookieManager(
cookie_partition_key_)),
first_party_sets_enabled_(first_party_sets_enabled) {
DCHECK(cookie_store);
CHECK(origin_ == isolation_info_.frame_origin().value() ||
role_ != mojom::RestrictedCookieManagerRole::SCRIPT);
}

RestrictedCookieManager::~RestrictedCookieManager() {
Expand Down

0 comments on commit 777ebb6

Please sign in to comment.