Move the call to GetOriginForURLLoaderFactory slightly earlier.

There are multiple calls to GetOriginForURLLoaderFactory made from
RenderFrameHostImpl::CommitNavigation.  Instead of making these calls,
we can make a single call early during CommitNavigation and store the
result in a variable that gets reused later on.  This refactoring has
been suggested when code reviewing another CL here:
https://chromium-review.googlesource.com/c/chromium/src/+/1875273/15/content/browser/frame_host/render_frame_host_impl.cc#5420

It turns out that moving the GetOriginForURLLoaderFactory call earlier
means that it also gets called for navigations to about:srcdoc.  In this
case the URLLoaderFactory should be inherited from the parent, so the
origin calculated by GetOriginForURLLoaderFactory is not technically
needed.  OTOH, GetOriginForURLLoaderFactory should in the future
calculate the origin to commit, and therefore we should make sure it
correctly handles all the cases.  In particular, without special-casing
about:srcdoc, NavigationBrowserTest.BlockedSrcDocRendererInitiated*
tests would hit a CHECK(...CanAccessDataForOrigin...) in
GetOriginForURLLoaderFactory.  This CL fixes this.

Bug: 998247, 920634, 936310
Change-Id: I1ca851c8e574d24f05b4943f6bee186ebd50aa61
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1910865Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714952}

content/browser/frame_host/render_frame_host_impl.cc

@@ -455,6 +455,12 @@ url::Origin GetOriginForURLLoaderFactoryUnchecked(
   if (!navigation_request)
     return url::Origin();
 
+  // GetOriginForURLLoaderFactory should only be called at the ready-to-commit
+  // time, when the RFHI to commit the navigation is already known.
+  DCHECK_LE(NavigationRequest::READY_TO_COMMIT, navigation_request->state());
+  RenderFrameHostImpl* target_frame = navigation_request->GetRenderFrameHost();
+  DCHECK(target_frame);
+
   // Check if this is loadDataWithBaseUrl (which needs special treatment).
   auto& common_params = navigation_request->common_params();
   if (IsLoadDataWithBaseURL(common_params)) {
@@ -483,6 +489,16 @@ url::Origin GetOriginForURLLoaderFactoryUnchecked(
   if (navigation_request->IsForMhtmlSubframe())
     return url::Origin();
 
+  // Srcdoc subframes need to inherit their origin from their parent frame.
+  if (navigation_request->GetURL().IsAboutSrcdoc()) {
+    // Srcdoc navigations in main frames should be blocked before this function
+    // is called.  This should guarantee existence of a parent here.
+    RenderFrameHostImpl* parent = target_frame->GetParent();
+    DCHECK(parent);
+
+    return parent->GetLastCommittedOrigin();
+  }
+
   // In cases not covered above, URLLoaderFactory should be associated with the
   // origin of |common_params.url| and/or |common_params.initiator_origin|.
   return url::Origin::Resolve(
@@ -5313,6 +5329,8 @@ void RenderFrameHostImpl::CommitNavigation(
   }
   DCHECK(network_isolation_key_.IsFullyPopulated());
 
+  url::Origin origin_for_url_loader_factory =
+      GetOriginForURLLoaderFactory(navigation_request);
   std::unique_ptr<blink::URLLoaderFactoryBundleInfo>
       subresource_loader_factories;
   if ((!is_same_document || is_first_navigation) && !is_srcdoc) {
@@ -5338,8 +5356,8 @@ void RenderFrameHostImpl::CommitNavigation(
           GetContentClient()->browser()->WillCreateURLLoaderFactory(
               browser_context, this, GetProcess()->GetID(),
               ContentBrowserClient::URLLoaderFactoryType::kDocumentSubResource,
-              GetOriginForURLLoaderFactory(navigation_request),
-              &appcache_proxied_receiver, nullptr /* header_client */,
+              origin_for_url_loader_factory, &appcache_proxied_receiver,
+              nullptr /* header_client */,
               nullptr /* bypass_redirect_checks */);
       if (use_proxy) {
         appcache_remote->Clone(std::move(appcache_proxied_receiver));
@@ -5379,7 +5397,7 @@ void RenderFrameHostImpl::CommitNavigation(
       GetContentClient()->browser()->WillCreateURLLoaderFactory(
           browser_context, this, GetProcess()->GetID(),
           ContentBrowserClient::URLLoaderFactoryType::kDocumentSubResource,
-          GetOriginForURLLoaderFactory(navigation_request), &factory_receiver,
+          origin_for_url_loader_factory, &factory_receiver,
           nullptr /* header_client */, nullptr /* bypass_redirect_checks */);
       CreateWebUIURLLoaderBinding(this, scheme, std::move(factory_receiver));
       // If the renderer has webui bindings, then don't give it access to
@@ -5409,8 +5427,7 @@ void RenderFrameHostImpl::CommitNavigation(
       recreate_default_url_loader_factory_after_network_service_crash_ = true;
       bool bypass_redirect_checks =
           CreateNetworkServiceDefaultFactoryAndObserve(
-              GetOriginForURLLoaderFactory(navigation_request),
-              network_isolation_key_,
+              origin_for_url_loader_factory, network_isolation_key_,
               pending_default_factory.InitWithNewPipeAndPassReceiver());
       subresource_loader_factories->set_bypass_redirect_checks(
           bypass_redirect_checks);
@@ -5499,7 +5516,7 @@ void RenderFrameHostImpl::CommitNavigation(
       GetContentClient()->browser()->WillCreateURLLoaderFactory(
           browser_context, this, GetProcess()->GetID(),
           ContentBrowserClient::URLLoaderFactoryType::kDocumentSubResource,
-          GetOriginForURLLoaderFactory(navigation_request), &factory_receiver,
+          origin_for_url_loader_factory, &factory_receiver,
           nullptr /* header_client */, nullptr /* bypass_redirect_checks */);
       // Keep DevTools proxy last, i.e. closest to the network.
       devtools_instrumentation::WillCreateURLLoaderFactory(