From 384629dd896151bf5160319bcaa81b2f74564638 Mon Sep 17 00:00:00 2001
From: Ian Vollick <[email protected]>
Date: Tue, 01 Sep 2026 06:35:55 -0700
Subject: [PATCH] Clear selection state on PrimaryPageChanged

Updates GlicSelectionObserver::PrimaryPageChanged to clear more state
so that we do not send incorrect updates after a navigation.

credit: fix and test adapted from suggestions by Project Fortify.

Bug: 513122002
Change-Id: Ic7b1425b249f3a99e28f8187320b3881c5f9ad90
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8323947
Reviewed-by: Dave Tapuska <[email protected]>
Commit-Queue: Dave Tapuska <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1689806}
---
diff --git a/chrome/browser/glic/glic_selection_observer.cc b/chrome/browser/glic/glic_selection_observer.cc
index 356e1bd..dcc7e45d 100644
--- a/chrome/browser/glic/glic_selection_observer.cc
+++ b/chrome/browser/glic/glic_selection_observer.cc
@@ -360,12 +360,7 @@
 }
 
 void GlicSelectionObserver::PrimaryPageChanged(content::Page& page) {
-  is_hidden_on_current_page_ = false;
-  UpdatePageBlockedState();
-  if (widget_delegate_) {
-    is_explaining_ = false;
-    widget_delegate_->CloseWidget();
-  }
+  ResetSelectionState();
 }
 
 void GlicSelectionObserver::PrimaryMainFrameWasResized(bool width_changed) {
@@ -717,6 +712,7 @@
   if (selected_text.empty()) {
     if (widget_delegate_ && !is_explaining_) {
       widget_delegate_->CloseWidget();
+      generated_link_.reset();
     }
 
     if (has_sent_selection_context_) {
@@ -724,6 +720,7 @@
       has_sent_selection_context_ = false;
     }
 
+    last_selection_frame_token_.reset();
     return;
   }
 
@@ -1350,4 +1347,14 @@
   last_direction_change_time_ = base::TimeTicks();
 }
 
+void GlicSelectionObserver::ResetSelectionState() {
+  is_hidden_on_current_page_ = false;
+  is_explaining_ = false;
+  UpdatePageBlockedState();
+  // This should close the widget and clear most selection state.
+  UpdateSelectionState(u"", false);
+  // This should clear the rest.
+  ResetPendingSelection();
+}
+
 }  // namespace glic
diff --git a/chrome/browser/glic/glic_selection_observer.h b/chrome/browser/glic/glic_selection_observer.h
index a2fa2e6..4896997 100644
--- a/chrome/browser/glic/glic_selection_observer.h
+++ b/chrome/browser/glic/glic_selection_observer.h
@@ -190,6 +190,8 @@
       std::string account,
       optimization_guide::PageContextEligibility* page_context_eligibility);
 
+  void ResetSelectionState();
+
   raw_ptr<GlicKeyedService> glic_keyed_service_;
   base::CallbackListSubscription panel_state_subscription_;
   std::u16string last_selected_text_;
diff --git a/chrome/browser/glic/glic_selection_observer_interactive_uitest.cc b/chrome/browser/glic/glic_selection_observer_interactive_uitest.cc
index 4e4ee8ba..44fa7518 100644
--- a/chrome/browser/glic/glic_selection_observer_interactive_uitest.cc
+++ b/chrome/browser/glic/glic_selection_observer_interactive_uitest.cc
@@ -4,14 +4,19 @@
 
 #include "chrome/browser/glic/public/features.h"
 #include "chrome/browser/glic/test_support/glic_browser_interactive_test.h"
+#include "chrome/browser/glic/test_support/interactive_test_util.h"
 #include "chrome/browser/ui/browser_element_identifiers.h"
 #include "chrome/browser/ui/browser_window/public/browser_window_interface.h"
 #include "chrome/browser/ui/views/frame/contents_web_view.h"
 #include "chrome/test/interaction/interactive_browser_test.h"
 #include "components/tabs/public/tab_interface.h"
 #include "content/public/test/browser_test.h"
+#include "content/public/test/browser_test_utils.h"
+#include "net/dns/mock_host_resolver.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
+#include "url/origin.h"
 
 namespace glic {
 namespace {
@@ -79,5 +84,109 @@
       "Timeout waiting for selection context in Glic"));
 }
 
+// TODO(b/537847327): This test should be migrated to use GlicApiBrowserTest,
+// and not the test client.
+class GlicSelectionObserverCrossOriginNavigationTest
+    : public GlicBrowserInteractiveTest {
+ public:
+  GlicSelectionObserverCrossOriginNavigationTest() {
+    glic_test_environment().SetGlicPagePath("/glic/test_client/index.html");
+    scoped_feature_list_.InitWithFeaturesAndParameters(
+        {{features::kGlicSelectionPrompt, {{"updates_only", "true"}}}}, {});
+  }
+
+  void SetUpOnMainThread() override {
+    host_resolver()->AddRule("*", "127.0.0.1");
+    GlicBrowserInteractiveTest::SetUpOnMainThread();
+  }
+
+ protected:
+  // Checks the additional context received by the web client and ensures that
+  // it does not context content from origin A.
+  static constexpr char kSawNoAdditionalContextFromOriginA[] = R"JS(
+       () => {
+          let c = document.querySelector('#additionalContextResult');
+          return !!c &&
+              !c.innerText.includes('application/x-glic-selection') &&
+              !c.innerText.includes('origin A content');
+       }
+    )JS";
+
+  auto SelectAll() {
+    return Do([this] {
+      content::WebContents* web_contents =
+          GetTabListInterface()->GetActiveTab()->GetContents();
+      web_contents->SelectAll();
+    });
+  }
+
+  auto Wait(base::TimeDelta timeout) {
+    auto observer = std::make_unique<test::internal::WaitingStateObserver>();
+    auto observer_ptr = observer.get();
+    return Steps(
+        Do(base::BindRepeating(
+            [](test::internal::WaitingStateObserver* observer,
+               base::TimeDelta timeout) { observer->Start(timeout); },
+            base::Unretained(observer_ptr), timeout)),
+        ObserveState(glic::test::internal::kDelayState, std::move(observer)),
+        WaitForState(glic::test::internal::kDelayState, true));
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(GlicSelectionObserverCrossOriginNavigationTest,
+                       DoesNotShowStaleSelectionUi) {
+  DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(kActiveTab);
+
+  const GURL url_a =
+      embedded_test_server()->GetURL("a.test", "/glic/selection_origin_a.html");
+  const GURL url_b =
+      embedded_test_server()->GetURL("b.test", "/glic/selection_origin_b.html");
+  ASSERT_NE(url::Origin::Create(url_a), url::Origin::Create(url_b));
+
+  const DeepQuery kPathToBody{"body"};
+
+  RunTestSequence(
+      InstrumentTab(kActiveTab),
+
+      // Step 1: Load origin A and let the user "select" text.
+      // The mouse clicks ensure is_key_selection_=false so the selection is
+      // accepted; the Wait covers the 200ms debounce so last_selected_text_
+      // is populated with content from origin A.
+      NavigateWebContents(kActiveTab, url_a),
+      WaitForWebContentsPainted(kActiveTab),
+      MoveMouseTo(kActiveTab, kPathToBody), ClickMouse(), ClickMouse(),
+      SelectAll(), Wait(base::Milliseconds(400)),
+
+      // Step 2: Cross-document, cross-origin navigation to origin B in the
+      // same tab. PrimaryPageChanged() clears state, ensuring that we will not
+      // show stale selection UI in the web client.
+      NavigateWebContents(kActiveTab, url_b),
+      WaitForWebContentsPainted(kActiveTab));
+
+  // Step 3: Open the Glic panel *without touching origin B's content*. If we
+  // did not properly clear state, then OnGlobalPanelShowHide() re-ships
+  // last_selected_text_ via SendAdditionalContext(), tagged with the current
+  // tab handle.
+  ASSERT_OK_AND_ASSIGN(auto* instance, OpenGlicForActiveTab());
+  ASSERT_OK(WaitForGlicClient(instance));
+
+  // Step 4: Ensure that we did not get a selection sent to the client. If we
+  // had not cleared correctly, we would have additional context from origin A,
+  // which is incorrect since we have navigated to origin B.
+  EXPECT_TRUE(RunUntil(
+      [&]() {
+        auto* guest_frame = instance->host().GetGuestMainFrame();
+        return guest_frame &&
+               content::EvalJs(
+                   guest_frame,
+                   base::StrCat({"(", kSawNoAdditionalContextFromOriginA,
+                                 ")()"})) == true;
+      },
+      "Timeout waiting for selection context in Glic"));
+}
+
 }  // namespace
 }  // namespace glic
diff --git a/chrome/browser/glic/glic_selection_observer_unittest.cc b/chrome/browser/glic/glic_selection_observer_unittest.cc
index 0a2b9164..878286a 100644
--- a/chrome/browser/glic/glic_selection_observer_unittest.cc
+++ b/chrome/browser/glic/glic_selection_observer_unittest.cc
@@ -1109,6 +1109,11 @@
   observer->OnPageContextEligibilityChanged(
       optimization_guide::PageContextEligibilityStatus::kEligible);
 
+  // We have to reset the selected text here because setting mock eligibility
+  // causes a navigation which clears the previous selected text.
+  observer->OnTextSelectionChanged(nullptr, u"Selected Text");
+  task_environment()->FastForwardBy(base::Milliseconds(300));
+
   EXPECT_TRUE(observer->send_context_called());
   EXPECT_EQ(u"Selected Text", *observer->last_sent_context());
 }
diff --git a/chrome/test/data/glic/selection_origin_a.html b/chrome/test/data/glic/selection_origin_a.html
new file mode 100644
index 0000000..b764182
--- /dev/null
+++ b/chrome/test/data/glic/selection_origin_a.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<!--
+
+Copyright 2026 The Chromium Authors
+Use of this source code is governed by a BSD-style license that can be
+found in the LICENSE file.
+
+-->
+<html>
+<head><title>Origin A</title></head>
+<body>origin A content</body>
+</html>
diff --git a/chrome/test/data/glic/selection_origin_b.html b/chrome/test/data/glic/selection_origin_b.html
new file mode 100644
index 0000000..ee4eea2
--- /dev/null
+++ b/chrome/test/data/glic/selection_origin_b.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<!--
+
+Copyright 2026 The Chromium Authors
+Use of this source code is governed by a BSD-style license that can be
+found in the LICENSE file.
+
+-->
+<html>
+<head><title>Origin B</title></head>
+<body>origin B content</body>
+</html>

diff --git a/chrome/browser/glic/glic_selection_observer_interactive_uitest.cc b/chrome/browser/glic/glic_selection_observer_interactive_uitest.cc
index 4e4ee8ba..44fa7518 100644
--- a/chrome/browser/glic/glic_selection_observer_interactive_uitest.cc
+++ b/chrome/browser/glic/glic_selection_observer_interactive_uitest.cc
@@ -4,14 +4,19 @@
 
 #include "chrome/browser/glic/public/features.h"
 #include "chrome/browser/glic/test_support/glic_browser_interactive_test.h"
+#include "chrome/browser/glic/test_support/interactive_test_util.h"
 #include "chrome/browser/ui/browser_element_identifiers.h"
 #include "chrome/browser/ui/browser_window/public/browser_window_interface.h"
 #include "chrome/browser/ui/views/frame/contents_web_view.h"
 #include "chrome/test/interaction/interactive_browser_test.h"
 #include "components/tabs/public/tab_interface.h"
 #include "content/public/test/browser_test.h"
+#include "content/public/test/browser_test_utils.h"
+#include "net/dns/mock_host_resolver.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
+#include "url/origin.h"
 
 namespace glic {
 namespace {
@@ -79,5 +84,109 @@
       "Timeout waiting for selection context in Glic"));
 }
 
+// TODO(b/537847327): This test should be migrated to use GlicApiBrowserTest,
+// and not the test client.
+class GlicSelectionObserverCrossOriginNavigationTest
+    : public GlicBrowserInteractiveTest {
+ public:
+  GlicSelectionObserverCrossOriginNavigationTest() {
+    glic_test_environment().SetGlicPagePath("/glic/test_client/index.html");
+    scoped_feature_list_.InitWithFeaturesAndParameters(
+        {{features::kGlicSelectionPrompt, {{"updates_only", "true"}}}}, {});
+  }
+
+  void SetUpOnMainThread() override {
+    host_resolver()->AddRule("*", "127.0.0.1");
+    GlicBrowserInteractiveTest::SetUpOnMainThread();
+  }
+
+ protected:
+  // Checks the additional context received by the web client and ensures that
+  // it does not context content from origin A.
+  static constexpr char kSawNoAdditionalContextFromOriginA[] = R"JS(
+       () => {
+          let c = document.querySelector('#additionalContextResult');
+          return !!c &&
+              !c.innerText.includes('application/x-glic-selection') &&
+              !c.innerText.includes('origin A content');
+       }
+    )JS";
+
+  auto SelectAll() {
+    return Do([this] {
+      content::WebContents* web_contents =
+          GetTabListInterface()->GetActiveTab()->GetContents();
+      web_contents->SelectAll();
+    });
+  }
+
+  auto Wait(base::TimeDelta timeout) {
+    auto observer = std::make_unique<test::internal::WaitingStateObserver>();
+    auto observer_ptr = observer.get();
+    return Steps(
+        Do(base::BindRepeating(
+            [](test::internal::WaitingStateObserver* observer,
+               base::TimeDelta timeout) { observer->Start(timeout); },
+            base::Unretained(observer_ptr), timeout)),
+        ObserveState(glic::test::internal::kDelayState, std::move(observer)),
+        WaitForState(glic::test::internal::kDelayState, true));
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(GlicSelectionObserverCrossOriginNavigationTest,
+                       DoesNotShowStaleSelectionUi) {
+  DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(kActiveTab);
+
+  const GURL url_a =
+      embedded_test_server()->GetURL("a.test", "/glic/selection_origin_a.html");
+  const GURL url_b =
+      embedded_test_server()->GetURL("b.test", "/glic/selection_origin_b.html");
+  ASSERT_NE(url::Origin::Create(url_a), url::Origin::Create(url_b));
+
+  const DeepQuery kPathToBody{"body"};
+
+  RunTestSequence(
+      InstrumentTab(kActiveTab),
+
+      // Step 1: Load origin A and let the user "select" text.
+      // The mouse clicks ensure is_key_selection_=false so the selection is
+      // accepted; the Wait covers the 200ms debounce so last_selected_text_
+      // is populated with content from origin A.
+      NavigateWebContents(kActiveTab, url_a),
+      WaitForWebContentsPainted(kActiveTab),
+      MoveMouseTo(kActiveTab, kPathToBody), ClickMouse(), ClickMouse(),
+      SelectAll(), Wait(base::Milliseconds(400)),
+
+      // Step 2: Cross-document, cross-origin navigation to origin B in the
+      // same tab. PrimaryPageChanged() clears state, ensuring that we will not
+      // show stale selection UI in the web client.
+      NavigateWebContents(kActiveTab, url_b),
+      WaitForWebContentsPainted(kActiveTab));
+
+  // Step 3: Open the Glic panel *without touching origin B's content*. If we
+  // did not properly clear state, then OnGlobalPanelShowHide() re-ships
+  // last_selected_text_ via SendAdditionalContext(), tagged with the current
+  // tab handle.
+  ASSERT_OK_AND_ASSIGN(auto* instance, OpenGlicForActiveTab());
+  ASSERT_OK(WaitForGlicClient(instance));
+
+  // Step 4: Ensure that we did not get a selection sent to the client. If we
+  // had not cleared correctly, we would have additional context from origin A,
+  // which is incorrect since we have navigated to origin B.
+  EXPECT_TRUE(RunUntil(
+      [&]() {
+        auto* guest_frame = instance->host().GetGuestMainFrame();
+        return guest_frame &&
+               content::EvalJs(
+                   guest_frame,
+                   base::StrCat({"(", kSawNoAdditionalContextFromOriginA,
+                                 ")()"})) == true;
+      },
+      "Timeout waiting for selection context in Glic"));
+}
+
 }  // namespace
 }  // namespace glic
diff --git a/chrome/browser/glic/glic_selection_observer_unittest.cc b/chrome/browser/glic/glic_selection_observer_unittest.cc
index 0a2b9164..878286a 100644
--- a/chrome/browser/glic/glic_selection_observer_unittest.cc
+++ b/chrome/browser/glic/glic_selection_observer_unittest.cc
@@ -1109,6 +1109,11 @@
   observer->OnPageContextEligibilityChanged(
       optimization_guide::PageContextEligibilityStatus::kEligible);
 
+  // We have to reset the selected text here because setting mock eligibility
+  // causes a navigation which clears the previous selected text.
+  observer->OnTextSelectionChanged(nullptr, u"Selected Text");
+  task_environment()->FastForwardBy(base::Milliseconds(300));
+
   EXPECT_TRUE(observer->send_context_called());
   EXPECT_EQ(u"Selected Text", *observer->last_sent_context());
 }
diff --git a/chrome/test/data/glic/selection_origin_a.html b/chrome/test/data/glic/selection_origin_a.html
new file mode 100644
index 0000000..b764182
--- /dev/null
+++ b/chrome/test/data/glic/selection_origin_a.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<!--
+
+Copyright 2026 The Chromium Authors
+Use of this source code is governed by a BSD-style license that can be
+found in the LICENSE file.
+
+-->
+<html>
+<head><title>Origin A</title></head>
+<body>origin A content</body>
+</html>
diff --git a/chrome/test/data/glic/selection_origin_b.html b/chrome/test/data/glic/selection_origin_b.html
new file mode 100644
index 0000000..ee4eea2
--- /dev/null
+++ b/chrome/test/data/glic/selection_origin_b.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<!--
+
+Copyright 2026 The Chromium Authors
+Use of this source code is governed by a BSD-style license that can be
+found in the LICENSE file.
+
+-->
+<html>
+<head><title>Origin B</title></head>
+<body>origin B content</body>
+</html>