[OutOfBlinkSandbox] Compute flags when receiving the response.

This was computed in CommitNavigation(). We would like to expose the
sandbox flags to the NavigationThrottle, which are run earlier.

This patch moves the computation immediately after receiving the
response.

Since computing sandbox flags will depends on
is_loaded_from_mhtml_archive_ in:
https://chromium-review.googlesource.com/c/chromium/src/+/2452475
both are moved.

Bug: 1136873,1041376
Change-Id: I98d33ea0a9367e87571644da648d40d0487a97cf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2467917Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818848}

content/browser/renderer_host/navigation_request.cc

@@ -1394,6 +1394,9 @@ void NavigationRequest::BeginNavigation() {
     // it immediately.
     EnterChildTraceEvent("ResponseStarted", this);
 
+    is_loaded_from_mhtml_archive_ = IsForMhtmlSubframe();
+    ComputeSandboxFlagsToCommit();
+
     // Select an appropriate RenderFrameHost.
     render_frame_host_ =
         frame_tree_node_->render_manager()->GetFrameHostForNavigation(this);
@@ -1550,6 +1553,7 @@ void NavigationRequest::ResetForCrossDocumentRestart() {
   SetState(NOT_STARTED);
   is_navigation_started_ = false;
   processing_navigation_throttle_ = false;
+  sandbox_flags_to_commit_.reset();
 
 #if defined(OS_ANDROID)
   if (navigation_handle_proxy_)
@@ -1980,6 +1984,8 @@ NavigationRequest::IsOptInIsolationRequested(const GURL& url) {
 
 void NavigationRequest::DetermineOriginIsolationEndResult(
     OptInIsolationCheckResult check_result) {
+  DCHECK_EQ(state_, WILL_PROCESS_RESPONSE);
+
   auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
   const url::Origin origin = url::Origin::Create(common_params_->url);
   const IsolationContext& isolation_context =
@@ -2015,7 +2021,7 @@ void NavigationRequest::DetermineOriginIsolationEndResult(
   // This needs to be computed separately from origin.opaque() because, per
   // https://crbug.com/1041376, we don't have a notion of the true origin yet.
   const bool is_opaque_origin_because_sandbox =
-      (ComputeSandboxFlagsToCommit() &
+      (sandbox_flags_to_commit_.value() &
        network::mojom::WebSandboxFlags::kOrigin) ==
       network::mojom::WebSandboxFlags::kOrigin;
 
@@ -2107,6 +2113,12 @@ void NavigationRequest::OnResponseStarted(
   ssl_info_ = response_head_->ssl_info;
   auth_challenge_info_ = response_head_->auth_challenge_info;
 
+  is_loaded_from_mhtml_archive_ = GetMimeType() == "multipart/related" ||
+                                  GetMimeType() == "message/rfc822" ||
+                                  IsForMhtmlSubframe();
+
+  ComputeSandboxFlagsToCommit();
+
   // The navigation may have encountered an origin policy or Origin-Isolation
   // header that requests isolation for the url's origin. Before we pick the
   // renderer, make sure we update the origin-isolation opt-ins appropriately.
@@ -3059,7 +3071,9 @@ void NavigationRequest::CommitErrorPage(
     }
   }
 
-  sandbox_flags_to_commit_ = ComputeSandboxFlagsToCommit();
+  is_loaded_from_mhtml_archive_ = false;
+  sandbox_flags_to_commit_.reset();
+  ComputeSandboxFlagsToCommit();
   ReadyToCommitNavigation(true);
   render_frame_host_->FailedNavigation(this, *common_params_, *commit_params_,
                                        has_stale_copy_in_cache_, net_error_,
@@ -3105,13 +3119,10 @@ void NavigationRequest::CommitNavigation() {
          (was_redirected_ && common_params_->url.IsAboutBlank()));
   DCHECK(!common_params_->url.SchemeIs(url::kJavaScriptScheme));
   DCHECK(!IsRendererDebugURL(common_params_->url));
+  DCHECK(sandbox_flags_to_commit_);
 
   AddOldPageInfoToCommitParamsIfNeeded();
 
-  is_loaded_from_mhtml_archive_ = GetMimeType() == "multipart/related" ||
-                                  GetMimeType() == "message/rfc822" ||
-                                  IsForMhtmlSubframe();
-
   if (IsServedFromBackForwardCache()) {
     // Navigations served from the back-forward cache must be a history
     // navigation, and thus should have a valid |pending_history_list_offset|
@@ -3181,7 +3192,6 @@ void NavigationRequest::CommitNavigation() {
     }
   }
 
-  sandbox_flags_to_commit_ = ComputeSandboxFlagsToCommit();
   CreateCoepReporter(render_frame_host_->GetProcess()->GetStoragePartition());
   coop_status_.UpdateReporterStoragePartition(
       render_frame_host_->GetProcess()->GetStoragePartition());
@@ -3880,8 +3890,9 @@ bool NavigationRequest::IsDeferredForTesting() {
   return throttle_runner_->GetDeferringThrottle() != nullptr;
 }
 
-bool NavigationRequest::IsLoadedFromMhtmlArchive() const {
-  DCHECK_LE(READY_TO_COMMIT, state_);
+bool NavigationRequest::IsLoadedFromMhtmlArchive() {
+  DCHECK(state_ >= WILL_PROCESS_RESPONSE ||
+         state_ == WILL_START_REQUEST && !NeedsUrlLoader());
   return is_loaded_from_mhtml_archive_;
 }
 
@@ -5004,27 +5015,24 @@ NavigationRequest::TakeCookieObservers() {
   return cookie_observers_.TakeReceivers();
 }
 
-network::mojom::WebSandboxFlags
-NavigationRequest::ComputeSandboxFlagsToCommit() {
+void NavigationRequest::ComputeSandboxFlagsToCommit() {
   DCHECK(commit_params_);
   DCHECK(!HasCommitted());
   DCHECK(!IsErrorPage());
+  DCHECK(!sandbox_flags_to_commit_);
 
-  network::mojom::WebSandboxFlags out =
-      commit_params_->frame_policy.sandbox_flags;
+  sandbox_flags_to_commit_ = commit_params_->frame_policy.sandbox_flags;
 
   // The response can also restrict the policy further.
   if (response_head_) {
     for (const auto& csp :
          response_head_->parsed_headers->content_security_policy) {
-      out |= csp->sandbox;
+      *sandbox_flags_to_commit_ |= csp->sandbox;
     }
   }
 
   // TODO(arthursonzogni): Add the MHTML sandbox flags here. This should
   // replicate DocumentLoader::CalculateSandboxFlags.
-
-  return out;
 }
 
 void NavigationRequest::CheckStateTransition(NavigationState state) const {

content/browser/renderer_host/navigation_request.h

@@ -544,7 +544,7 @@ class CONTENT_EXPORT NavigationRequest
   // Whether the new document loaded will be loaded from an MHTML archive.
   // Contrary to IsForMhtmlSubframe(), this isn't scoped to subframe, but can't
   // be called prior to receiving the final response.
-  bool IsLoadedFromMhtmlArchive() const;
+  bool IsLoadedFromMhtmlArchive();
 
   // Whether the new document created by this navigation will be loaded from a
   // MHTML document. In this case, the navigation will commit in the main frame
@@ -1101,9 +1101,10 @@ class CONTENT_EXPORT NavigationRequest
   // NavigationRequest is in.
   NavigationControllerImpl* GetNavigationController();
 
-  // Compute the sandbox policy of the document to be loaded. Called once when
-  // reaching the 'ReadyToCommit' stage.
-  network::mojom::WebSandboxFlags ComputeSandboxFlagsToCommit();
+  // Compute the sandbox policy of the document to be loaded. This is called
+  // once the final response is known. It is based on the current FramePolicy
+  // and the response's CSP.
+  void ComputeSandboxFlagsToCommit();
 
   // DCHECK that tranistioning from the current state to |state| valid. This
   // does nothing in non-debug builds.
@@ -1491,8 +1492,7 @@ class CONTENT_EXPORT NavigationRequest
   // made by this navigation.
   mojo::ReceiverSet<network::mojom::CookieAccessObserver> cookie_observers_;
 
-  // The sandbox flags of the document to be loaded. This is computed at
-  // 'ReadyToCommit' time.
+  // The sandbox flags of the document to be loaded.
   base::Optional<network::mojom::WebSandboxFlags> sandbox_flags_to_commit_;
 
   OptInOriginIsolationEndResult origin_isolation_end_result_ =