From 35ec6fdc64cf53e460fcf54bb1aebe291889c53b Mon Sep 17 00:00:00 2001
From: Noam Rosenthal <[email protected]>
Date: Thu, 03 Sep 2026 11:12:46 -0700
Subject: [PATCH] Reland "Use committed origin when deciding whether to dispatch navigate event"

This is a reland of commit cc3f09b3a5179e1232fa27f898fa165cb06fc98b.

Original change caused test failures in
All/NavigationControllerBrowserTest.NavigateEventNotFiredForTraversalToCSPSandboxedEntry/*
on builders where DeferSpeculativeRFHCreation is disabled or not applicable
(e.g., fuchsia-arm64-cast-receiver-rel).

Root cause:
In DidCreateNavigationRequest, when DeferSpeculativeRFHCreation is disabled,
RenderFrameHostManager::GetFrameHostForNavigation() runs synchronously at
kBeforeNetworkRequest to create a speculative RFH. At this stage, HTTP
response headers have not yet arrived. NavigationRequest::GetUrlInfo()
only estimated sandbox flags from commit_params_->frame_policy.sandbox_flags,
which is empty for main-frame CSP sandboxing. As a result, UrlInfo was
estimated as unsandboxed, causing CanUseDestinationInstance() to conclude
that the sandboxed destination SiteInstance was unsuitable.
RenderFrameHostManager::GetSiteInstanceForNavigationRequest() observed
this mismatch (request->dest_site_instance() != dest_site_instance) and
called ResetStateForSiteInstanceChange(), which reset origin_related_state_.
When MaybeDispatchNavigateEventForCrossDocumentTraversal() subsequently ran,
origin_related_state_->committed_origin was missing, causing it to fall
back to resolving the pre-redirect URL as same-origin and firing the navigate
event unexpectedly.

Fix:
When computing UrlInfo before policies are computed, consider
dest_site_instance_->GetSiteInfo().IsSandboxed() when estimating sandbox
flags for history navigations.

Original change description:
> Use committed origin when deciding whether to dispatch navigate event
>
> The committed origin can be opaque in sandboxed iframes, and is also
> pre-redirect.
>
> Bug: 511774376
> Change-Id: Ia5f4257a907274f24abb32186ae0e074eb488716
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8041464
> Reviewed-by: Charlie Reis <[email protected]>
> Commit-Queue: Noam Rosenthal <[email protected]>
> Reviewed-by: Nate Chapin <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#1662779}

Bug: 511774376
Change-Id: Id05f189baabd0ef75e52dbf040ac5463b6d23cde
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8345238
Commit-Queue: Noam Rosenthal <[email protected]>
Reviewed-by: Charlie Reis <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1691774}
---
diff --git a/content/browser/renderer_host/navigation_request.cc b/content/browser/renderer_host/navigation_request.cc
index 63b4aeb..b7e23d3 100644
--- a/content/browser/renderer_host/navigation_request.cc
+++ b/content/browser/renderer_host/navigation_request.cc
@@ -1889,6 +1889,7 @@
     origin_related_state_.emplace(OriginRelatedState{
         .item_sequence_number = frame_entry->item_sequence_number(),
         .document_sequence_number = frame_entry->document_sequence_number(),
+        .committed_origin = frame_entry->committed_origin(),
     });
   }
 
@@ -4759,10 +4760,12 @@
   //
   // If PolicyContainer::ComputePoliciesToCommit() has run
   // `policy_container_builder_` will be valid, but even if it hasn't, we can
-  // speculatively take `commit_params_->frame_policy.sandbox_flags` if we
-  // haven't received the response yet and don't have the final
-  // `policy_container_builder_`, and if the state of the kOrigin flag changes,
-  // we'll detect the change and recompute the target SiteInstance elsewhere.
+  // speculatively take `commit_params_->frame_policy.sandbox_flags` (or
+  // `dest_site_instance_` for history navigations) if we haven't received the
+  // response yet and don't have the final `policy_container_builder_`, to
+  // reduce the chance that recomputing will be needed. If the state of the
+  // kOrigin flag changes, we'll detect the change and recompute the target
+  // SiteInstance elsewhere.
   //
   // In general, about:blank documents should stay in their initiator's process.
   // If neither the initiator or about:blank is sandboxed, or if both are, then
@@ -4792,12 +4795,15 @@
       // Note: We'll end up here if this function is called before
       // ComputePoliciesToCommit(), such as when computing a speculative
       // RenderFrameHost's SiteInstance before receiving a response. In that
-      // event we use the sandbox flags in commit_params_ as a current "best
+      // event we use the sandbox flags in commit_params_ (or the destination
+      // SiteInstance, if this is a history navigation) as a current "best
       // estimate".
       has_origin_restricted_sandbox_flag =
-          (commit_params_->frame_policy.sandbox_flags &
-           network::mojom::WebSandboxFlags::kOrigin) ==
-          network::mojom::WebSandboxFlags::kOrigin;
+          ((commit_params_->frame_policy.sandbox_flags &
+            network::mojom::WebSandboxFlags::kOrigin) ==
+           network::mojom::WebSandboxFlags::kOrigin) ||
+          (dest_site_instance_ &&
+           dest_site_instance_->GetSiteInfo().IsSandboxed());
     }
 
     // It's possible that a sandbox attribute can disappear from a frame that
@@ -12030,13 +12036,19 @@
       blink::mojom::NavigationType::HISTORY_DIFFERENT_DOCUMENT) {
     return;
   }
-  // Only fire the navigate event if the destination is same-origin. Because
-  // this check is performed at navigation start time, `destination_origin` is
+  // Only fire the navigate event if the destination is same-origin. Prefer
+  // the origin recorded when the destination entry previously committed, since
+  // the URL alone does not reflect opaque origins resulting from CSP sandbox.
+  // See also PopulateSingleNavigationApiHistoryEntryVector(). Because this
+  // check is performed at navigation start time, the URL-derived fallback is
   // based on the pre-redirect URL, which is consistent with the renderer
   // process logic for firing the navigate event for non-history navigations.
-  url::Origin destination_origin = url::Origin::Resolve(
-      common_params_->url,
-      common_params_->initiator_origin.value_or(url::Origin()));
+  url::Origin destination_origin =
+      origin_related_state_ && origin_related_state_->committed_origin
+          ? *origin_related_state_->committed_origin
+          : url::Origin::Resolve(
+                common_params_->url,
+                common_params_->initiator_origin.value_or(url::Origin()));
   if (!frame_tree_node_->current_origin().IsSameOriginWith(
           destination_origin)) {
     return;
diff --git a/content/browser/renderer_host/navigation_request.h b/content/browser/renderer_host/navigation_request.h
index da65f8c..78a097fc 100644
--- a/content/browser/renderer_host/navigation_request.h
+++ b/content/browser/renderer_host/navigation_request.h
@@ -3209,6 +3209,9 @@
   struct OriginRelatedState {
     int64_t item_sequence_number;
     int64_t document_sequence_number;
+    // The origin that was recorded when the FrameNavigationEntry was previously
+    // committed, if any. May be nullopt for entries that have never committed.
+    std::optional<url::Origin> committed_origin;
   };
   std::optional<OriginRelatedState> origin_related_state_;
 
diff --git a/content/browser/renderer_host/navigation_controller_impl_browsertest.cc b/content/browser/renderer_host/navigation_controller_impl_browsertest.cc
index 7e83a45..65e762e 100644
--- a/content/browser/renderer_host/navigation_controller_impl_browsertest.cc
+++ b/content/browser/renderer_host/navigation_controller_impl_browsertest.cc
@@ -22020,6 +22020,10 @@
   EXPECT_EQ(initial_site_instance, contents()->GetSiteInstance());
 }
 
+// TODO(crbug.com/511774376): Add a browser test to verify that the navigate
+// event for a cross-document history traversal is not dispatched when the
+// destination entry was committed at an opaque origin due to CSP sandbox.
+
 IN_PROC_BROWSER_TEST_P(NavigationControllerBrowserTest,
                        NavigateToNavigationApiKey_NullCommittedOrigin) {
   // Ensure there's a history entry before the error page.

diff --git a/content/browser/renderer_host/navigation_controller_impl_browsertest.cc b/content/browser/renderer_host/navigation_controller_impl_browsertest.cc
index 7e83a45..65e762e 100644
--- a/content/browser/renderer_host/navigation_controller_impl_browsertest.cc
+++ b/content/browser/renderer_host/navigation_controller_impl_browsertest.cc
@@ -22020,6 +22020,10 @@
   EXPECT_EQ(initial_site_instance, contents()->GetSiteInstance());
 }
 
+// TODO(crbug.com/511774376): Add a browser test to verify that the navigate
+// event for a cross-document history traversal is not dispatched when the
+// destination entry was committed at an opaque origin due to CSP sandbox.
+
 IN_PROC_BROWSER_TEST_P(NavigationControllerBrowserTest,
                        NavigateToNavigationApiKey_NullCommittedOrigin) {
   // Ensure there's a history entry before the error page.