From cd855c3707b867ced0863584154cdebf14d89fca Mon Sep 17 00:00:00 2001
From: Daniel Vogelheim <[email protected]>
Date: Wed, 16 Sep 2026 10:48:29 -0700
Subject: [PATCH] [Cookie Controls] Anchor controller to caller-provided site URL

CookieControlsController re-read the WebContents' last committed URL on
every status query and setting change. UI built on top of it (e.g. Page
Info) can be anchored to a different URL while a browser-initiated
navigation is pending, which meant the toggle could update settings for
a different site than the one shown.

Snapshot the site URL in Update() and use it for all subsequent status
queries and setting changes so they always match. Add an overload that
lets callers pass the URL they are showing, and have Page Info use it.
Refresh the snapshot on primary page changes so the long-lived omnibox
icon controller stays in sync.

Fix largely follows crbug.com/523248913#comment4

Fixed: 523248913
Change-Id: Iefcf8ca6d4376ccce2758cee2728afd7129d04f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8398929
Reviewed-by: Antonio Sartori <[email protected]>
Reviewed-by: Balazs Engedy <[email protected]>
Commit-Queue: Daniel Vogelheim <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1699054}
---
diff --git a/components/content_settings/browser/ui/cookie_controls_controller.cc b/components/content_settings/browser/ui/cookie_controls_controller.cc
index e67cbe1..259baa48 100644
--- a/components/content_settings/browser/ui/cookie_controls_controller.cc
+++ b/components/content_settings/browser/ui/cookie_controls_controller.cc
@@ -139,6 +139,13 @@
 
 void CookieControlsController::Update(content::WebContents* web_contents) {
   DCHECK(web_contents);
+  Update(web_contents, web_contents->GetLastCommittedURL());
+}
+
+void CookieControlsController::Update(content::WebContents* web_contents,
+                                      const GURL& site_url) {
+  DCHECK(web_contents);
+  site_url_ = site_url;
   if (!tab_observer_ || GetWebContents() != web_contents) {
     tab_observer_ = std::make_unique<TabObserver>(this, web_contents);
     SetStateChangedViaBypass(false);
@@ -147,7 +154,7 @@
   if (observers_.empty()) {
     return;
   }
-  auto status = GetStatus(web_contents);
+  auto status = GetStatus();
   const CookieControlsState icon_state =
       GetCookieControlsIconState(status.controls_state);
   for (auto& observer : observers_) {
@@ -157,14 +164,13 @@
   }
 }
 
-CookieControlsController::Status CookieControlsController::GetStatus(
-    content::WebContents* web_contents) {
+CookieControlsController::Status CookieControlsController::GetStatus() {
   if (!cookie_settings_->ShouldBlockThirdPartyCookies()) {
     return {CookieControlsState::kHidden,
             CookieControlsEnforcement::kNoEnforcement, base::Time()};
   }
 
-  const GURL& url = web_contents->GetLastCommittedURL();
+  const GURL& url = site_url_;
   if (url.SchemeIs(content::kChromeUIScheme) ||
       url.SchemeIs(kExtensionScheme)) {
     return {CookieControlsState::kHidden,
@@ -241,20 +247,19 @@
 
 void CookieControlsController::OnCookieBlockingEnabledForSite(
     bool block_third_party_cookies) {
-  const GURL& url = GetWebContents()->GetLastCommittedURL();
+  const GURL url = site_url_;
   should_reload_ = true;
   if (block_third_party_cookies) {
     base::RecordAction(UserMetricsAction("CookieControls.Bubble.TurnOn"));
     cookie_settings_->ResetThirdPartyCookieSetting(url);
-    Update(GetWebContents());
+    Update(GetWebContents(), url);
     return;
   }
 
   CHECK(!block_third_party_cookies);
   base::RecordAction(UserMetricsAction("CookieControls.Bubble.TurnOff"));
   cookie_settings_->SetCookieSettingForUserBypass(url);
-  Update(GetWebContents());
-  // Record metadata for the newly created exception.
+  Update(GetWebContents(), url);
   base::DictValue metadata = GetMetadata(settings_map_, url);
   metadata.Set(kActivationsCountKey, GetActivationCount(metadata) + 1);
   ApplyMetadataChanges(settings_map_, url, std::move(metadata));
@@ -279,8 +284,7 @@
   }
 
   return browsing_data::GetUniqueThirdPartyCookiesHostCount(
-      GetWebContents()->GetLastCommittedURL(),
-      *(pscs->allowed_browsing_data_model()));
+      site_url_, *(pscs->allowed_browsing_data_model()));
 }
 
 int CookieControlsController::GetBlockedThirdPartyCookiesSitesCount() const {
@@ -291,8 +295,7 @@
   }
 
   return browsing_data::GetUniqueThirdPartyCookiesHostCount(
-      GetWebContents()->GetLastCommittedURL(),
-      *(pscs->blocked_browsing_data_model()));
+      site_url_, *(pscs->blocked_browsing_data_model()));
 }
 
 int CookieControlsController::GetStatefulBounceCount() const {
@@ -309,7 +312,7 @@
   if (observers_.empty()) {
     return;
   }
-  auto status = GetStatus(GetWebContents());
+  auto status = GetStatus();
   const CookieControlsState icon_state =
       GetCookieControlsIconState(status.controls_state);
   for (auto& observer : observers_) {
@@ -372,7 +375,8 @@
 }
 
 void CookieControlsController::RecordActivationMetrics() {
-  const GURL& url = GetWebContents()->GetLastCommittedURL();
+  const GURL& url = site_url_;
+
   auto site_data_access_type =
       GetSiteDataAccessType(GetAllowedThirdPartyCookiesSitesCount(),
                             GetBlockedThirdPartyCookiesSitesCount());
@@ -488,6 +492,8 @@
     reload_count_++;
   }
   last_visited_url_ = current_url;
+  cookie_controls_->site_url_ =
+      content::WebContentsObserver::web_contents()->GetLastCommittedURL();
   cookie_controls_->UpdatePageReloadStatus(reload_count_);
 }
 
diff --git a/components/content_settings/browser/ui/cookie_controls_controller.h b/components/content_settings/browser/ui/cookie_controls_controller.h
index 6065cfd..9cc3541 100644
--- a/components/content_settings/browser/ui/cookie_controls_controller.h
+++ b/components/content_settings/browser/ui/cookie_controls_controller.h
@@ -56,9 +56,17 @@
       delete;
   ~CookieControlsController() override;
 
-  // Called when the web_contents has changed.
+  // Called when the web_contents has changed. Status is computed for the last
+  // committed URL of `web_contents`.
   void Update(content::WebContents* web_contents);
 
+  // Called when the web_contents has changed. Status is computed for
+  // `site_url`, which is also used for any setting changes made via this
+  // controller. Callers that show a specific URL to the user (e.g. Page Info)
+  // should pass that URL so the displayed state and the modified setting
+  // remain consistent even if a navigation is pending.
+  void Update(content::WebContents* web_contents, const GURL& site_url);
+
   // Updates the cookie controls icon.
   void UpdateCookieControlsIcon();
 
@@ -142,7 +150,7 @@
       bool block_third_party_cookies) override;
   void OnCookieSettingChanged() override;
 
-  Status GetStatus(content::WebContents* web_contents);
+  Status GetStatus();
 
   CookieControlsEnforcement GetEnforcementForThirdPartyCookieBlocking(
       const GURL url,
@@ -184,6 +192,11 @@
   scoped_refptr<content_settings::CookieSettings> original_cookie_settings_;
   raw_ptr<HostContentSettingsMap> settings_map_;
 
+  // The URL the controller is currently anchored to. Set in `Update()` and
+  // refreshed on primary page changes; used for status queries and setting
+  // modifications so they always agree with what the UI is showing.
+  GURL site_url_;
+
   base::ScopedObservation<content_settings::CookieSettings,
                           content_settings::CookieSettings::Observer>
       cookie_observation_{this};
diff --git a/components/page_info/page_info.cc b/components/page_info/page_info.cc
index f2ab5e7..81954bda0 100644
--- a/components/page_info/page_info.cc
+++ b/components/page_info/page_info.cc
@@ -363,7 +363,7 @@
 
     // TODO(crbug.com/40901748): SetCookieInfo is called twice, once from here
     // and once from InitializeUiState. This should be cleaned up.
-    cookie_controller_->Update(web_contents);
+    cookie_controller_->Update(web_contents, site_url_);
 
     auto* pscs = GetPageSpecificContentSettings();
     if (pscs) {
diff --git a/chrome/browser/ui/cookie_controls/cookie_controls_controller_unittest.cc b/chrome/browser/ui/cookie_controls/cookie_controls_controller_unittest.cc
index 582e027..f101988 100644
--- a/chrome/browser/ui/cookie_controls/cookie_controls_controller_unittest.cc
+++ b/chrome/browser/ui/cookie_controls/cookie_controls_controller_unittest.cc
@@ -756,3 +756,27 @@
   cookie_controls()->Update(web_contents());
   testing::Mock::VerifyAndClearExpectations(mock());
 }
+
+TEST_F(CookieControlsUserBypassTest, DisableForSiteWithPendingNavigation) {
+  // The toggle should always operate on the URL that the surrounding UI was
+  // anchored to, even if a pending navigation has caused the visible URL to
+  // diverge from the last committed URL.
+  const GURL kCommittedUrl("https://committed.test");
+  const GURL kPendingUrl("https://pending.test");
+
+  NavigateAndCommit(kCommittedUrl);
+  auto navigation = content::NavigationSimulator::CreateBrowserInitiated(
+      kPendingUrl, web_contents());
+  navigation->Start();
+  ASSERT_EQ(kCommittedUrl, web_contents()->GetLastCommittedURL());
+
+  cookie_controls()->Update(web_contents(), kPendingUrl);
+  cookie_controls()->OnCookieBlockingEnabledForSite(
+      /*block_third_party_cookies=*/false);
+  EXPECT_TRUE(cookie_settings()->IsThirdPartyAccessAllowed(kPendingUrl));
+  EXPECT_FALSE(cookie_settings()->IsThirdPartyAccessAllowed(kCommittedUrl));
+
+  cookie_controls()->OnCookieBlockingEnabledForSite(
+      /*block_third_party_cookies=*/true);
+  EXPECT_FALSE(cookie_settings()->IsThirdPartyAccessAllowed(kPendingUrl));
+}

diff --git a/chrome/browser/ui/cookie_controls/cookie_controls_controller_unittest.cc b/chrome/browser/ui/cookie_controls/cookie_controls_controller_unittest.cc
index 582e027..f101988 100644
--- a/chrome/browser/ui/cookie_controls/cookie_controls_controller_unittest.cc
+++ b/chrome/browser/ui/cookie_controls/cookie_controls_controller_unittest.cc
@@ -756,3 +756,27 @@
   cookie_controls()->Update(web_contents());
   testing::Mock::VerifyAndClearExpectations(mock());
 }
+
+TEST_F(CookieControlsUserBypassTest, DisableForSiteWithPendingNavigation) {
+  // The toggle should always operate on the URL that the surrounding UI was
+  // anchored to, even if a pending navigation has caused the visible URL to
+  // diverge from the last committed URL.
+  const GURL kCommittedUrl("https://committed.test");
+  const GURL kPendingUrl("https://pending.test");
+
+  NavigateAndCommit(kCommittedUrl);
+  auto navigation = content::NavigationSimulator::CreateBrowserInitiated(
+      kPendingUrl, web_contents());
+  navigation->Start();
+  ASSERT_EQ(kCommittedUrl, web_contents()->GetLastCommittedURL());
+
+  cookie_controls()->Update(web_contents(), kPendingUrl);
+  cookie_controls()->OnCookieBlockingEnabledForSite(
+      /*block_third_party_cookies=*/false);
+  EXPECT_TRUE(cookie_settings()->IsThirdPartyAccessAllowed(kPendingUrl));
+  EXPECT_FALSE(cookie_settings()->IsThirdPartyAccessAllowed(kCommittedUrl));
+
+  cookie_controls()->OnCookieBlockingEnabledForSite(
+      /*block_third_party_cookies=*/true);
+  EXPECT_FALSE(cookie_settings()->IsThirdPartyAccessAllowed(kPendingUrl));
+}