From 6d251b0b989b3bac6e4cce2ac205b489c78138b5 Mon Sep 17 00:00:00 2001
From: Peilin Wang <[email protected]>
Date: Mon, 21 Sep 2026 15:31:19 -0700
Subject: [PATCH] [Navigation] Reset SameDocNavigation flags more often

Bug: 514078082
Change-Id: I7b9ce94a0f2f3eb1d00255807840dd84a441d3ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8431388
Reviewed-by: Patrick Noland <[email protected]>
Commit-Queue: Peilin Wang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1702119}
---
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/tab/TabWebContentsObserver.java b/chrome/android/java/src/org/chromium/chrome/browser/tab/TabWebContentsObserver.java
index a10da110..dbba6c0 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/tab/TabWebContentsObserver.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/tab/TabWebContentsObserver.java
@@ -326,7 +326,18 @@
             }
             mLastUrl = navigation.getUrl();
 
-            if (!navigation.hasCommitted()) return;
+            if (!navigation.hasCommitted()) {
+                // A navigation that started but never committed (e.g. a same-document
+                // traversal cancelled from the Navigation API's 'navigate' event) must
+                // still tell observers the navigation is over. Otherwise
+                // LocationBarModel's same-document debounce stays latched and silently
+                // drops later URL and security state updates.
+                observers.rewind();
+                while (observers.hasNext()) {
+                    observers.next().onDidFinishNavigationEnd();
+                }
+                return;
+            }
 
             mTab.updateTitle();
             mTab.handleDidFinishNavigation(
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/toolbar/LocationBarModelTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/toolbar/LocationBarModelTest.java
index 09906745..56b9447 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/toolbar/LocationBarModelTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/toolbar/LocationBarModelTest.java
@@ -32,6 +32,7 @@
 import org.chromium.base.test.params.ParameterizedRunner;
 import org.chromium.base.test.util.Batch;
 import org.chromium.base.test.util.CommandLineFlags;
+import org.chromium.base.test.util.CriteriaHelper;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.Features.DisableFeatures;
 import org.chromium.base.test.util.UrlUtils;
@@ -46,6 +47,7 @@
 import org.chromium.chrome.browser.profiles.ProfileManager;
 import org.chromium.chrome.browser.tab.MockTab;
 import org.chromium.chrome.browser.tab.Tab;
+import org.chromium.chrome.browser.tab.TabObserver;
 import org.chromium.chrome.browser.tab.TabSelectionType;
 import org.chromium.chrome.browser.toolbar.top.ToolbarLayout;
 import org.chromium.chrome.browser.url_constants.UrlConstantResolver;
@@ -54,12 +56,16 @@
 import org.chromium.chrome.test.transit.ChromeTransitTestRules;
 import org.chromium.chrome.test.transit.page.WebPageStation;
 import org.chromium.chrome.test.util.ChromeTabUtils;
+import org.chromium.content_public.browser.NavigationHandle;
+import org.chromium.content_public.browser.WebContents;
+import org.chromium.content_public.browser.test.util.JavaScriptUtils;
 import org.chromium.url.GURL;
 import org.chromium.url.JUnitTestGURLs;
 
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 
 /** Tests for LocationBarModel. */
@@ -346,6 +352,135 @@
         verify(mLocationBarObserver, atLeast(1)).onSecurityStateChanged();
     }
 
+    /**
+     * Repro for b/514078082: a same-document traversal that is aborted via the Navigation API never
+     * results in TabObserver#onDidFinishNavigationEnd() being dispatched (TabWebContentsObserver
+     * early-returns on !navigation.hasCommitted()), so LocationBarModel#mIsInSameDocNav stays
+     * latched and all but the first subsequent security state change is silently dropped.
+     */
+    @Test
+    @MediumTest
+    public void testAbortedSameDocTraversalLatchesSameDocNavFlag() throws Exception {
+        ChromeTabbedActivity activity = mActivityTestRule.getActivity();
+        LocationBarModel locationBarModel =
+                activity.getToolbarManager().getLocationBarModelForTesting();
+
+        mActivityTestRule.loadUrl(
+                mActivityTestRule
+                        .getTestServer()
+                        .getURL("/chrome/test/data/android/navigate/simple.html"));
+
+        Tab tab = ThreadUtils.runOnUiThreadBlocking(() -> activity.getActivityTab());
+        WebContents webContents = ThreadUtils.runOnUiThreadBlocking(tab::getWebContents);
+        AtomicBoolean sawAbortedSameDocNav = new AtomicBoolean();
+        AtomicBoolean sawNavigationEndAfterAbort = new AtomicBoolean();
+        TabObserver navObserver =
+                new TabObserver() {
+                    @Override
+                    public void onDidFinishNavigationInPrimaryMainFrame(
+                            Tab tab, NavigationHandle navigation) {
+                        if (navigation.isSameDocument() && !navigation.hasCommitted()) {
+                            sawAbortedSameDocNav.set(true);
+                        }
+                    }
+
+                    @Override
+                    public void onDidFinishNavigationEnd() {
+                        // The committed pushState that sets up the traversal also dispatches
+                        // this, so only the dispatch following the aborted navigation counts.
+                        if (sawAbortedSameDocNav.get()) {
+                            sawNavigationEndAfterAbort.set(true);
+                        }
+                    }
+                };
+        ThreadUtils.runOnUiThreadBlocking(() -> tab.addObserver(navObserver));
+
+        // Start a same-document traversal and cancel it from the 'navigate' event.
+        JavaScriptUtils.executeJavaScriptAndWaitForResult(
+                webContents,
+                "(function() {"
+                        + "  history.pushState({}, '', '#a');"
+                        + "  navigation.addEventListener('navigate', e => {"
+                        + "    if (e.navigationType === 'traverse') e.preventDefault();"
+                        + "  });"
+                        + "  history.back();"
+                        + "  return 'ok';"
+                        + "})()");
+
+        CriteriaHelper.pollUiThread(
+                sawAbortedSameDocNav::get,
+                "Never saw a same-document DidFinishNavigation with hasCommitted() == false");
+        ThreadUtils.runOnUiThreadBlocking(() -> tab.removeObserver(navObserver));
+
+        Assert.assertTrue(
+                "onDidFinishNavigationEnd() was never dispatched for the aborted navigation,"
+                        + " so LocationBarModel's same-document debounce stays latched.",
+                sawNavigationEndAfterAbort.get());
+
+        // With the flag latched, the first notification is delivered and every later one is
+        // dropped, so a page can pin the omnibox security state.
+        mLocationBarObserver = mock(LocationBarDataProvider.Observer.class);
+        ThreadUtils.runOnUiThreadBlocking(
+                () -> {
+                    locationBarModel.addObserver(mLocationBarObserver);
+                    locationBarModel.notifySecurityStateChanged();
+                    locationBarModel.notifySecurityStateChanged();
+                });
+
+        verify(mLocationBarObserver, times(2)).onSecurityStateChanged();
+    }
+
+    /**
+     * The same-document debounce is scoped to one navigation in one tab. If setTab() carries a
+     * latched debounce into the incoming tab, updateVisibleGurl() short-circuits while
+     * broadcastUrlChanged() still fires (because isTabChanging is true), so the omnibox redraws
+     * using the *previous* tab's cached URL.
+     */
+    @Test
+    @MediumTest
+    public void testSetTabResetsSameDocNavFlags() {
+        ChromeTabbedActivity activity = mActivityTestRule.getActivity();
+        LocationBarModel locationBarModel =
+                activity.getToolbarManager().getLocationBarModelForTesting();
+
+        String urlA =
+                mActivityTestRule
+                        .getTestServer()
+                        .getURL("/chrome/test/data/android/navigate/one.html");
+        String urlB =
+                mActivityTestRule
+                        .getTestServer()
+                        .getURL("/chrome/test/data/android/navigate/two.html");
+
+        mActivityTestRule.loadUrl(urlA);
+        // Pre-load the second tab. Switching to a frozen tab dispatches onContentChanged, which
+        // resets the flags on its own and would mask the bug this test is guarding against.
+        mActivityTestRule.loadUrlInNewTab(urlB, /* incognito= */ false);
+        ChromeTabUtils.switchTabInCurrentTabModel(activity, 0);
+
+        assertEquals(
+                urlA,
+                ThreadUtils.runOnUiThreadBlocking(
+                        () -> locationBarModel.getCurrentGurl().getSpec()));
+
+        // Latch the debounce on tab A, the state an aborted same-document traversal leaves behind.
+        ThreadUtils.runOnUiThreadBlocking(
+                () -> {
+                    locationBarModel.notifyDidStartNavigation(/* isSameDocument= */ true);
+                    // isTabChanging=true forces the broadcast that arms the URL flag without
+                    // requiring an actual URL change.
+                    locationBarModel.notifyUrlChanged(/* isTabChanging= */ true);
+                    locationBarModel.notifySecurityStateChanged();
+                });
+
+        ChromeTabUtils.switchTabInCurrentTabModel(activity, 1);
+
+        CriteriaHelper.pollUiThread(
+                () -> urlB.equals(locationBarModel.getCurrentGurl().getSpec()),
+                "Omnibox kept the previous tab's URL after switching tabs while the"
+                        + " same-document debounce was latched.");
+    }
+
     private void assertDisplayAndEditText(
             ToolbarDataProvider dataProvider, String displayText, String editText) {
         ThreadUtils.runOnUiThreadBlocking(
diff --git a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/LocationBarModel.java b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/LocationBarModel.java
index 4c9e0393..5b8b4165 100644
--- a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/LocationBarModel.java
+++ b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/LocationBarModel.java
@@ -308,6 +308,9 @@
 
         boolean isTabChanging = mTab != tab;
         Tab previousTab = mTab;
+        if (isTabChanging) {
+            resetSameDocNavFlags();
+        }
         mTab = tab;
         mProfile = profile;
         performProfileDependentInitializationIfRequired();