From 6e43fb358cfb4ad3c72af10ad00d840b92460f63 Mon Sep 17 00:00:00 2001
From: Shunya Shishido <sisidovski@chromium.org>
Date: Thu, 30 Jul 2026 23:33:31 -0700
Subject: [PATCH] Fix DeclarativePerformanceObserver early failure partition check

NavigationRequest::site_info_ only reflects the target frame's
StoragePartition once StartNavigation() has run; before that it holds a
default configuration.

To prevent cross-StoragePartition information leaks from non-default
partitions (e.g., guest <webview>) while preserving early-failure
telemetry for standard main frames in the default StoragePartition, this
CL requires that either StartNavigation() has run (`state_ >=
WILL_START_REQUEST`) or the current frame already belongs to the default
StoragePartition.

This CL also introduces CanRecordEarlyNavigationFailure() to avoid code
duplication and adds comprehensive browsertests to verify both
cross-partition leak prevention and standard early-failure recording.

Bug: 537470182
Change-Id: I4c9c19a0bb7e45a726c083b74c201e1a31331d39
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8173103
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Shunya Shishido <sisidovski@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1671650}
---

diff --git a/content/browser/renderer_host/navigation_request.cc b/content/browser/renderer_host/navigation_request.cc
index 6a62ef7..76ed6e5 100644
--- a/content/browser/renderer_host/navigation_request.cc
+++ b/content/browser/renderer_host/navigation_request.cc
@@ -2245,7 +2245,8 @@
   }
 
   if (!early_navigation_failure_recorded_ && !response() &&
-      IsInPrimaryMainFrame() && net_error_ == net::ERR_ABORTED) {
+      IsInPrimaryMainFrame() && net_error_ == net::ERR_ABORTED &&
+      CanRecordEarlyNavigationFailure()) {
     DeclarativePerformanceObserver::RecordEarlyNavigationFailure(
         this, GetStoragePartitionWithCurrentSiteInfo(), net::ERR_ABORTED);
   }
@@ -5592,7 +5593,8 @@
     fast_fetch_manager_->OnRequestFailed(*this, status, skip_throttles);
   }
 
-  if (!response() && IsInPrimaryMainFrame() && status.error_code != net::OK) {
+  if (!response() && IsInPrimaryMainFrame() && status.error_code != net::OK &&
+      CanRecordEarlyNavigationFailure()) {
     DeclarativePerformanceObserver::RecordEarlyNavigationFailure(
         this, GetStoragePartitionWithCurrentSiteInfo(), status.error_code);
     early_navigation_failure_recorded_ = true;
@@ -12572,6 +12574,21 @@
          !IsPrerenderedPageActivation();
 }
 
+bool NavigationRequest::CanRecordEarlyNavigationFailure() const {
+  // `site_info_` reflects the target frame's StoragePartition only after
+  // StartNavigation() has run; before that it holds a default configuration.
+  // To prevent cross-partition leaks from non-default partitions (e.g., guest
+  // <webview>) while keeping metrics for standard primary main frames, require
+  // that StartNavigation() has run (`state_ >= WILL_START_REQUEST`) or that
+  // the current frame already belongs to the default StoragePartition.
+  return state_ >= WILL_START_REQUEST ||
+         frame_tree_node_->current_frame_host()
+             ->GetSiteInstance()
+             ->GetSecurityPrincipal()
+             .GetStoragePartitionConfig()
+             .is_default();
+}
+
 void NavigationRequest::MaybeRecordTraceEventsAndHistograms() {
   if (navigation_handle_timing_.navigation_commit_sent_time.is_null()) {
     return;
diff --git a/content/browser/renderer_host/navigation_request.h b/content/browser/renderer_host/navigation_request.h
index 192469b..c645e7913 100644
--- a/content/browser/renderer_host/navigation_request.h
+++ b/content/browser/renderer_host/navigation_request.h
@@ -2712,6 +2712,10 @@
   // eventually be replaced with the navigation timeline metrics.
   bool ShouldRecordNavigationTimelineUkm() const;
 
+  // Returns true if early navigation failure can be safely recorded without
+  // risking cross-StoragePartition information leakage.
+  bool CanRecordEarlyNavigationFailure() const;
+
   // Given the known destination origin, this updates the view transition state
   // and resources. Namely, it clears it if the view transition state and
   // resources were generated from a different origin with the given origin.
diff --git a/content/browser/renderer_host/navigation_request_browsertest.cc b/content/browser/renderer_host/navigation_request_browsertest.cc
index 1b0fdf6..197641d 100644
--- a/content/browser/renderer_host/navigation_request_browsertest.cc
+++ b/content/browser/renderer_host/navigation_request_browsertest.cc
@@ -11,11 +11,13 @@
 #include "base/functional/bind.h"
 #include "base/memory/raw_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "base/run_loop.h"
 #include "base/strings/strcat.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/test/bind.h"
 #include "base/test/metrics/histogram_tester.h"
+#include "base/values.h"
 #include "build/build_config.h"
 #include "components/history/core/browser/features.h"
 #include "content/browser/process_lock.h"
@@ -25,19 +27,24 @@
 #include "content/browser/renderer_host/process_selection_deferring_condition_runner.h"
 #include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/browser/site_instance_impl.h"
+#include "content/browser/storage_partition_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/common/content_navigation_policy.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/navigation_discard_reason.h"
 #include "content/public/browser/navigation_throttle.h"
 #include "content/public/browser/runtime_feature_state/runtime_feature_state_document_data.h"
 #include "content/public/browser/security_principal.h"
 #include "content/public/browser/site_isolation_policy.h"
+#include "content/public/browser/storage_partition.h"
+#include "content/public/browser/storage_partition_config.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/bindings_policy.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
+#include "content/public/common/isolated_world_ids.h"
 #include "content/public/common/result_codes.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/test/back_forward_cache_util.h"
@@ -69,6 +76,7 @@
 #include "services/network/public/cpp/loading_params.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "third_party/blink/public/common/chrome_debug_urls.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/common/runtime_feature_state/runtime_feature_state_context.h"
 #include "third_party/blink/public/common/runtime_feature_state/runtime_feature_state_read_context.h"
 #include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom.h"
@@ -5854,4 +5862,196 @@
             EvalJs(popup_shell->web_contents(), "document.body.textContent"));
 }
 
+namespace {
+
+base::ListValue TakeEarlyFailureReportsForTesting(
+    DeclarativePerformanceObserverStore* store,
+    const url::Origin& origin) {
+  base::ListValue out;
+  base::RunLoop loop;
+  store->TakeEarlyFailureReports(
+      origin, base::BindLambdaForTesting([&](base::ListValue res) {
+        out = std::move(res);
+        loop.Quit();
+      }));
+  loop.Run();
+  return out;
+}
+
+class NavigationRequestPartitionBrowserTest
+    : public NavigationRequestBrowserTest {
+ public:
+  NavigationRequestPartitionBrowserTest() {
+    feature_list_.InitWithFeatures(
+        {blink::features::kDeclarativePerformanceObserver,
+         features::kAbortNavigationsFromTabClosures},
+        {});
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+}  // namespace
+
+// Verifies that when a navigation request in a non-default StoragePartition is
+// aborted before reaching WILL_START_REQUEST, early failure reports are not
+// recorded in either the default StoragePartition or the frame's partition.
+IN_PROC_BROWSER_TEST_F(
+    NavigationRequestPartitionBrowserTest,
+    EarlyFailureInNonDefaultPartitionAbortedBeforeStartNavigation) {
+  CustomStoragePartitionBrowserClient modified_client(GURL("http://b.com/"));
+
+  GURL url1(embedded_test_server()->GetURL("b.com", "/title1.html"));
+  GURL url2(embedded_test_server()->GetURL("b.com", "/title2.html"));
+  const url::Origin origin = url::Origin::Create(url1);
+
+  EXPECT_TRUE(NavigateToURL(shell(), url1));
+
+  RenderFrameHostImpl* rfh = contents()->GetPrimaryMainFrame();
+  SiteInstanceImpl* si = rfh->GetSiteInstance();
+  EXPECT_FALSE(
+      si->GetSecurityPrincipal().GetStoragePartitionConfig().is_default());
+
+  BrowserContext* browser_context = contents()->GetBrowserContext();
+  auto* frame_partition =
+      static_cast<StoragePartitionImpl*>(rfh->GetStoragePartition());
+  auto* default_partition =
+      static_cast<StoragePartitionImpl*>(browser_context->GetStoragePartition(

diff --git a/content/browser/renderer_host/navigation_request_browsertest.cc b/content/browser/renderer_host/navigation_request_browsertest.cc
index 1b0fdf6..197641d 100644
--- a/content/browser/renderer_host/navigation_request_browsertest.cc
+++ b/content/browser/renderer_host/navigation_request_browsertest.cc
@@ -11,11 +11,13 @@
 #include "base/functional/bind.h"
 #include "base/memory/raw_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "base/run_loop.h"
 #include "base/strings/strcat.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/test/bind.h"
 #include "base/test/metrics/histogram_tester.h"
+#include "base/values.h"
 #include "build/build_config.h"
 #include "components/history/core/browser/features.h"
 #include "content/browser/process_lock.h"
@@ -25,19 +27,24 @@
 #include "content/browser/renderer_host/process_selection_deferring_condition_runner.h"
 #include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/browser/site_instance_impl.h"
+#include "content/browser/storage_partition_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/common/content_navigation_policy.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/navigation_discard_reason.h"
 #include "content/public/browser/navigation_throttle.h"
 #include "content/public/browser/runtime_feature_state/runtime_feature_state_document_data.h"
 #include "content/public/browser/security_principal.h"
 #include "content/public/browser/site_isolation_policy.h"
+#include "content/public/browser/storage_partition.h"
+#include "content/public/browser/storage_partition_config.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/bindings_policy.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
+#include "content/public/common/isolated_world_ids.h"
 #include "content/public/common/result_codes.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/test/back_forward_cache_util.h"
@@ -69,6 +76,7 @@
 #include "services/network/public/cpp/loading_params.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "third_party/blink/public/common/chrome_debug_urls.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/common/runtime_feature_state/runtime_feature_state_context.h"
 #include "third_party/blink/public/common/runtime_feature_state/runtime_feature_state_read_context.h"
 #include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom.h"
@@ -5854,4 +5862,196 @@
             EvalJs(popup_shell->web_contents(), "document.body.textContent"));
 }
 
+namespace {
+
+base::ListValue TakeEarlyFailureReportsForTesting(
+    DeclarativePerformanceObserverStore* store,
+    const url::Origin& origin) {
+  base::ListValue out;
+  base::RunLoop loop;
+  store->TakeEarlyFailureReports(
+      origin, base::BindLambdaForTesting([&](base::ListValue res) {
+        out = std::move(res);
+        loop.Quit();
+      }));
+  loop.Run();
+  return out;
+}
+
+class NavigationRequestPartitionBrowserTest
+    : public NavigationRequestBrowserTest {
+ public:
+  NavigationRequestPartitionBrowserTest() {
+    feature_list_.InitWithFeatures(
+        {blink::features::kDeclarativePerformanceObserver,
+         features::kAbortNavigationsFromTabClosures},
+        {});
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+}  // namespace
+
+// Verifies that when a navigation request in a non-default StoragePartition is
+// aborted before reaching WILL_START_REQUEST, early failure reports are not
+// recorded in either the default StoragePartition or the frame's partition.
+IN_PROC_BROWSER_TEST_F(
+    NavigationRequestPartitionBrowserTest,
+    EarlyFailureInNonDefaultPartitionAbortedBeforeStartNavigation) {
+  CustomStoragePartitionBrowserClient modified_client(GURL("http://b.com/"));
+
+  GURL url1(embedded_test_server()->GetURL("b.com", "/title1.html"));
+  GURL url2(embedded_test_server()->GetURL("b.com", "/title2.html"));
+  const url::Origin origin = url::Origin::Create(url1);
+
+  EXPECT_TRUE(NavigateToURL(shell(), url1));
+
+  RenderFrameHostImpl* rfh = contents()->GetPrimaryMainFrame();
+  SiteInstanceImpl* si = rfh->GetSiteInstance();
+  EXPECT_FALSE(
+      si->GetSecurityPrincipal().GetStoragePartitionConfig().is_default());
+
+  BrowserContext* browser_context = contents()->GetBrowserContext();
+  auto* frame_partition =
+      static_cast<StoragePartitionImpl*>(rfh->GetStoragePartition());
+  auto* default_partition =
+      static_cast<StoragePartitionImpl*>(browser_context->GetStoragePartition(
+          StoragePartitionConfig::CreateDefault(browser_context)));
+  EXPECT_NE(frame_partition, default_partition);
+
+  DeclarativePerformanceObserverStore* frame_store =
+      frame_partition->GetDeclarativePerformanceObserverStore();
+  DeclarativePerformanceObserverStore* default_store =
+      default_partition->GetDeclarativePerformanceObserverStore();
+  EXPECT_TRUE(frame_store);
+  EXPECT_TRUE(default_store);
+
+  {
+    base::RunLoop loop;
+    default_store->SetEarlyFailurePolicy(origin, true, loop.QuitClosure());
+    loop.Run();
+  }
+  {
+    base::RunLoop loop;
+    frame_store->SetEarlyFailurePolicy(origin, true, loop.QuitClosure());
+    loop.Run();
+  }
+  EXPECT_TRUE(default_store->HasEarlyFailurePolicy(origin));
+  EXPECT_TRUE(frame_store->HasEarlyFailurePolicy(origin));
+
+  EXPECT_TRUE(ExecJs(rfh, R"(
+      window.addEventListener('beforeunload', e => {
+        e.preventDefault();
+        e.returnValue = 'blocked';
+      });
+  )"));
+  rfh->ExecuteJavaScriptWithUserGestureForTests(
+      std::u16string(), base::NullCallback(), ISOLATED_WORLD_ID_GLOBAL);
+  rfh->DisableBeforeUnloadHangMonitorForTesting();
+
+  FrameTreeNode* root = contents()->GetPrimaryFrameTree().root();
+  EXPECT_EQ(FrameType::kPrimaryMainFrame, root->GetFrameType());
+
+  {
+    BeforeUnloadBlockingDelegate beforeunload_pauser(contents());
+
+    shell()->LoadURL(url2);
+    beforeunload_pauser.Wait();
+
+    NavigationRequest* pending = root->navigation_request();
+    EXPECT_TRUE(pending);
+    if (pending) {
+      EXPECT_EQ(NavigationRequest::WAITING_FOR_RENDERER_RESPONSE,
+                pending->state());
+      EXPECT_TRUE(pending->IsInPrimaryMainFrame());
+    }
+
+    root->ResetNavigationRequest(
+        NavigationDiscardReason::kNewOtherNavigationBrowserInitiated);
+    EXPECT_FALSE(root->navigation_request());
+  }
+
+  base::ListValue default_reports =
+      TakeEarlyFailureReportsForTesting(default_store, origin);
+  base::ListValue frame_reports =
+      TakeEarlyFailureReportsForTesting(frame_store, origin);
+
+  EXPECT_EQ(0u, default_reports.size());
+  EXPECT_EQ(0u, frame_reports.size());
+}
+
+// Verifies that when a standard navigation request in the default
+// StoragePartition is aborted before reaching WILL_START_REQUEST, early failure
+// reports are correctly recorded in the default StoragePartition.
+// Note: This test fails under the original fix CL that unconditionally
+// required `state_ >= WILL_START_REQUEST`.
+IN_PROC_BROWSER_TEST_F(
+    NavigationRequestPartitionBrowserTest,
+    EarlyFailureInDefaultPartitionAbortedBeforeStartNavigation) {
+  GURL url1(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  GURL url2(embedded_test_server()->GetURL("a.com", "/title2.html"));
+  const url::Origin origin = url::Origin::Create(url1);
+
+  EXPECT_TRUE(NavigateToURL(shell(), url1));
+
+  RenderFrameHostImpl* rfh = contents()->GetPrimaryMainFrame();
+  SiteInstanceImpl* si = rfh->GetSiteInstance();
+  EXPECT_TRUE(
+      si->GetSecurityPrincipal().GetStoragePartitionConfig().is_default());
+
+  BrowserContext* browser_context = contents()->GetBrowserContext();
+  auto* default_partition =
+      static_cast<StoragePartitionImpl*>(browser_context->GetDefaultStoragePartition());
+
+  DeclarativePerformanceObserverStore* default_store =
+      default_partition->GetDeclarativePerformanceObserverStore();
+  EXPECT_TRUE(default_store);
+
+  {
+    base::RunLoop loop;
+    default_store->SetEarlyFailurePolicy(origin, true, loop.QuitClosure());
+    loop.Run();
+  }
+  EXPECT_TRUE(default_store->HasEarlyFailurePolicy(origin));
+
+  EXPECT_TRUE(ExecJs(rfh, R"(
+      window.addEventListener('beforeunload', e => {
+        e.preventDefault();
+        e.returnValue = 'blocked';
+      });
+  )"));
+  rfh->ExecuteJavaScriptWithUserGestureForTests(
+      std::u16string(), base::NullCallback(), ISOLATED_WORLD_ID_GLOBAL);
+  rfh->DisableBeforeUnloadHangMonitorForTesting();
+
+  FrameTreeNode* root = contents()->GetPrimaryFrameTree().root();
+  EXPECT_EQ(FrameType::kPrimaryMainFrame, root->GetFrameType());
+
+  {
+    BeforeUnloadBlockingDelegate beforeunload_pauser(contents());
+
+    shell()->LoadURL(url2);
+    beforeunload_pauser.Wait();
+
+    NavigationRequest* pending = root->navigation_request();
+    EXPECT_TRUE(pending);
+    if (pending) {
+      EXPECT_EQ(NavigationRequest::WAITING_FOR_RENDERER_RESPONSE,
+                pending->state());
+      EXPECT_TRUE(pending->IsInPrimaryMainFrame());
+    }
+
+    root->ResetNavigationRequest(
+        NavigationDiscardReason::kNewOtherNavigationBrowserInitiated);
+    EXPECT_FALSE(root->navigation_request());
+  }
+
+  base::ListValue default_reports =
+      TakeEarlyFailureReportsForTesting(default_store, origin);
+
+  EXPECT_EQ(1u, default_reports.size());
+}
+
 }  // namespace content