Commit d23ef67c authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

Navigations can use specific origin in |request_initiator_site_lock|.

Now that |CommonNavigationParams::initiator_origin| is available
(and trustworthy - see https://crbug.com/919144#c12 and #c7)
RenderFrameHostImpl (and its helper function
GetOriginForURLLoaderFactory) can calculate an exact origin lock
(rather than falling back to a site-URL-based lock).

Bug: 918967, 888079
Change-Id: I3b180a785d5171bb924761e44fafb482314c1132
Reviewed-on: https://chromium-review.googlesource.com/c/1450237
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635124}
parent 38f4ef93
...@@ -418,27 +418,15 @@ void LogRendererKillCrashKeys(const GURL& site_url) { ...@@ -418,27 +418,15 @@ void LogRendererKillCrashKeys(const GURL& site_url) {
base::debug::SetCrashKeyString(site_url_key, site_url.spec()); base::debug::SetCrashKeyString(site_url_key, site_url.spec());
} }
base::Optional<url::Origin> GetOriginForURLLoaderFactory( url::Origin GetOriginForURLLoaderFactory(
GURL target_url, const CommonNavigationParams& common_params) {
SiteInstanceImpl* site_instance) { // Use |initiator_origin| for navigations to about:blank and about:srcdoc.
// TODO(lukasza, nasko): https://crbug.com/888079: Use exact origin, instead //
// of falling back to site URL for about:blank and about:srcdoc. // |initiator_origin| is provided for all renderer-initiated navigations;
if (target_url.SchemeIs(url::kAboutScheme)) { // for browser-initiated navigations a unique origin should be used.
// |site_instance|'s site URL cannot be used as GURL target_url = common_params.url;
// |request_initiator_site_lock| unless the site requires a dedicated if (target_url.SchemeIs(url::kAboutScheme))
// process. Otherwise b.com may share a process associated with a.com, in return common_params.initiator_origin.value_or(url::Origin());
// a SiteInstance with |site_url| set to "http://a.com" (and/or
// "http://nonisolated.invalid" in the future) and in that scenario
// |request_initiator| for requests from b.com should NOT be locked to
// a.com.
if (!SiteInstanceImpl::ShouldLockToOrigin(
site_instance->GetBrowserContext(),
site_instance->GetIsolationContext(), site_instance->GetSiteURL()))
return base::nullopt;
return SiteInstanceImpl::GetRequestInitiatorSiteLock(
site_instance->GetSiteURL());
}
// In cases not covered above, URLLoaderFactory should be associated with the // In cases not covered above, URLLoaderFactory should be associated with the
// origin of |target_url|. This works fine for all URLs, including data: URLs // origin of |target_url|. This works fine for all URLs, including data: URLs
...@@ -4583,8 +4571,7 @@ void RenderFrameHostImpl::CommitNavigation( ...@@ -4583,8 +4571,7 @@ void RenderFrameHostImpl::CommitNavigation(
recreate_default_url_loader_factory_after_network_service_crash_ = true; recreate_default_url_loader_factory_after_network_service_crash_ = true;
bool bypass_redirect_checks = bool bypass_redirect_checks =
CreateNetworkServiceDefaultFactoryAndObserve( CreateNetworkServiceDefaultFactoryAndObserve(
GetOriginForURLLoaderFactory(common_params.url, GetOriginForURLLoaderFactory(common_params),
GetSiteInstance()),
mojo::MakeRequest(&default_factory_info)); mojo::MakeRequest(&default_factory_info));
subresource_loader_factories->set_bypass_redirect_checks( subresource_loader_factories->set_bypass_redirect_checks(
bypass_redirect_checks); bypass_redirect_checks);
...@@ -4647,10 +4634,8 @@ void RenderFrameHostImpl::CommitNavigation( ...@@ -4647,10 +4634,8 @@ void RenderFrameHostImpl::CommitNavigation(
GetContentClient()->browser()->WillCreateURLLoaderFactory( GetContentClient()->browser()->WillCreateURLLoaderFactory(
browser_context, this, GetProcess()->GetID(), browser_context, this, GetProcess()->GetID(),
false /* is_navigation */, false /* is_navigation */,
GetOriginForURLLoaderFactory(common_params.url, GetSiteInstance()) GetOriginForURLLoaderFactory(common_params), &factory_request,
.value_or(url::Origin()), nullptr /* header_client */, nullptr /* bypass_redirect_checks */);
&factory_request, nullptr /* header_client */,
nullptr /* bypass_redirect_checks */);
// Keep DevTools proxy last, i.e. closest to the network. // Keep DevTools proxy last, i.e. closest to the network.
devtools_instrumentation::WillCreateURLLoaderFactory( devtools_instrumentation::WillCreateURLLoaderFactory(
this, false /* is_navigation */, false /* is_download */, this, false /* is_navigation */, false /* is_download */,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment