From 79ae4197ad5742782ad57644adbbc7f33fe93d98 Mon Sep 17 00:00:00 2001
From: Jakob Koblinsky <[email protected]>
Date: Mon, 21 Sep 2026 16:14:59 -0700
Subject: [PATCH] [Omnibox] Only suppress status icon during load when security is NONE

Original CL crrev.com/c/8269455 introduced status icon suppression
during pending HTTP/HTTPS navigations in StatusMediator to avoid
transient jank from the default (i) info icon appearing when navigating
via the omnibox.

However, as reported in crbug.com/553248404, this introduced a security
issue: an insecure page could trigger a hanging renderer-initiated
navigation, which caused StatusMediator to unconditionally suppress the
status icon, masking the warning/dangerous indicator.

This CL resolves the issue by moving the suppression check out of
StatusMediator and into LocationBarModel#getSecurityIconResource,
aligning it with the existing toolbar refactor logic. Suppression is now
only applied when the security level is ConnectionSecurityLevel.NONE.

For renderer-initiated navigations, GetVisibleEntry() remains pinned to
the committed page, so the security level stays WARNING (or DANGEROUS)
and is never suppressed. For browser-initiated omnibox navigations,
GetVisibleEntry() switches to the pending entry, whose uninitialized SSL
evaluates to NONE, properly suppressing the neutral icon until commit.

TAG=agy
CONV=70ec631e-7344-4c1a-ba92-f5dda47b8c57
Bug: 553248404

Change-Id: I45e0ea9e54775a7bae75c5a992d0cf63aef315ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8419770
Reviewed-by: Sky Malice <[email protected]>
Commit-Queue: Jakob Koblinsky <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1702169}
---
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/toolbar/LocationBarModelUnitTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/toolbar/LocationBarModelUnitTest.java
index 5378428..0c9a9078d 100644
--- a/chrome/android/junit/src/org/chromium/chrome/browser/toolbar/LocationBarModelUnitTest.java
+++ b/chrome/android/junit/src/org/chromium/chrome/browser/toolbar/LocationBarModelUnitTest.java
@@ -53,6 +53,7 @@
 import org.chromium.chrome.browser.theme.ThemeUtils;
 import org.chromium.chrome.browser.ui.theme.BrandedColorScheme;
 import org.chromium.components.embedder_support.util.UrlConstants;
+import org.chromium.components.omnibox.OmniboxFeatureList;
 import org.chromium.components.omnibox.OmniboxUrlEmphasizerJni;
 import org.chromium.components.security_state.ConnectionMaliciousContentStatus;
 import org.chromium.components.security_state.ConnectionSecurityLevel;
@@ -167,6 +168,16 @@
         mLocationBarModel.destroy();
     }
 
+    private void setUpLoadingHttpOrHttpsPage() {
+        mLocationBarModel.initializeWithNative();
+        doReturn(true).when(mRegularTabMock).isInitialized();
+        doReturn(true).when(mRegularTabMock).isLoading();
+        doReturn(mExampleGurl)
+                .when(mLocationBarModelJni)
+                .getUrlOfVisibleNavigationEntry(Mockito.anyLong());
+        mLocationBarModel.setTab(mRegularTabMock, mRegularProfileMock);
+    }
+
     @Test
     public void getProfile_RegularTab_ReturnsRegularProfile() {
         mLocationBarModel.setTab(mRegularTabMock, mRegularProfileMock);
@@ -478,7 +489,6 @@
     }
 
     @Test
-    @EnableFeatures({ChromeFeatureList.ANDROID_BOTTOM_BAR})
     public void
             getSecurityIconResource_connectionNone_nonHttpOrHttps_returnsOmniboxInfoEvenWhenLoading() {
         mLocationBarModel.initializeWithNative();
@@ -509,20 +519,43 @@
     }
 
     @Test
-    @EnableFeatures({ChromeFeatureList.ANDROID_BOTTOM_BAR})
-    public void getSecurityIconResource_connectionNone_httpOrHttps_suppressesIconDuringLoading() {
-        mLocationBarModel.initializeWithNative();
-        doReturn(true).when(mRegularTabMock).isInitialized();
-        doReturn(true).when(mRegularTabMock).isLoading();
-        doReturn(new GURL("https://example.com"))
-                .when(mLocationBarModelJni)
-                .getUrlOfVisibleNavigationEntry(Mockito.anyLong());
-        mLocationBarModel.setTab(mRegularTabMock, mRegularProfileMock);
+    public void getSecurityIconResource_suppressesIconDuringLoading() {
+        setUpLoadingHttpOrHttpsPage();
 
+        // NONE level -> icon suppressed.
         assertResourceIdIs(
                 Resources.ID_NULL,
                 ConnectionSecurityLevel.NONE,
                 ConnectionMaliciousContentStatus.NONE);
+        // WARNING level -> icon not suppressed.
+        assertResourceIdIs(
+                R.drawable.omnibox_not_secure_warning,
+                ConnectionSecurityLevel.WARNING,
+                ConnectionMaliciousContentStatus.NONE);
+        // DANGEROUS level -> icon not suppressed.
+        assertResourceIdIs(
+                R.drawable.omnibox_dangerous,
+                ConnectionSecurityLevel.DANGEROUS,
+                ConnectionMaliciousContentStatus.NONE);
+        // SECURE level -> icon not suppressed.
+        assertResourceIdIs(
+                R.drawable.omnibox_https_valid_page_info,
+                ConnectionSecurityLevel.SECURE,
+                ConnectionMaliciousContentStatus.NONE);
+    }
+
+    @Test
+    @DisableFeatures({
+        OmniboxFeatureList.SUPPRESS_STATUS_ICON_DURING_HTTP_NAVIGATION,
+        ChromeFeatureList.ANDROID_BOTTOM_BAR
+    })
+    public void getSecurityIconResource_connectionNone_httpOrHttps_notSuppressedWhenDisabled() {
+        setUpLoadingHttpOrHttpsPage();
+
+        assertResourceIdIs(
+                R.drawable.omnibox_info,
+                ConnectionSecurityLevel.NONE,
+                ConnectionMaliciousContentStatus.NONE);
     }
 
     @Test
diff --git a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/status/StatusMediator.java b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/status/StatusMediator.java
index 9bc13ac..b74def4 100644
--- a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/status/StatusMediator.java
+++ b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/status/StatusMediator.java
@@ -52,7 +52,6 @@
 import org.chromium.components.browser_ui.util.DrawableUtils;
 import org.chromium.components.content_settings.CookieControlsBridge;
 import org.chromium.components.content_settings.CookieControlsObserver;
-import org.chromium.components.embedder_support.util.UrlUtilities;
 import org.chromium.components.metrics.OmniboxEventProtosIntDef.PageClassification;
 import org.chromium.components.omnibox.AutocompleteInput;
 import org.chromium.components.omnibox.AutocompleteInput.AutocompleteState;
@@ -605,8 +604,6 @@
                         || displayState == DisplayState.DRAFTING
                         || displayState == DisplayState.DRAFTING_NO_FOCUS;
 
-        boolean showBlankIcon = false;
-
         if (PageClassificationUtils.isHubOrTabSearch(
                 mLocationBarDataProvider.getPageClassification(/* prefetch= */ false))) {
             mPermissionStatusHandler.reset(/* shouldDismissNativePrompt= */ false);
@@ -645,13 +642,6 @@
             clickListener = mFuseboxOnPlusButtonClicked;
             descRes = R.string.accessibility_omnibox_open_context_popup;
             doubleTapDescriptionRes = Resources.ID_NULL;
-        } else if (OmniboxFeatures.sSuppressStatusIconDuringHttpNavigation.isEnabled()
-                && hasPendingHttpOrHttpsNavigation()) {
-            // Prevent jank due to the info (i) icon appearing during page navigation. But if the
-            // destination page isn't http, then it's fine to show the info icon, because that's
-            // what it'll end up being anyway.
-            mPermissionStatusHandler.reset(/* shouldDismissNativePrompt= */ false);
-            showBlankIcon = true;
         } else if (maybeUpdateStatusIconForSearchEngineIcon()) {
             mPermissionStatusHandler.reset(/* shouldDismissNativePrompt= */ true);
             // No need to proceed further if we've already updated it for the search engine icon.
@@ -690,9 +680,7 @@
         }
 
         StatusIconResource statusIcon = null;
-        if (showBlankIcon) {
-            statusIcon = null;
-        } else if (customDrawable != null) {
+        if (customDrawable != null) {
             statusIcon = new StatusIconResource(customDrawable);
         } else if (bitmap != null) {
             statusIcon = new StatusIconResource(/* iconIdentifier= */ null, bitmap, tintRes);
@@ -812,11 +800,6 @@
         return url != null && !OmniboxUrlUtils.isNtpUrl(url);
     }
 
-    private boolean hasPendingHttpOrHttpsNavigation() {
-        GURL url = getPendingUrl();
-        return url != null && UrlUtilities.isHttpOrHttps(url);
-    }
-
     /** Returns status icon resource for the user-selected default search engine. */
     private StatusIconResource getStatusIconResourceForSearchEngineIcon() {
         StatusIconResource extensionIcon = getStatusIconResourceForExtensionSuppliedDse();
diff --git a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/status/StatusMediatorUnitTest.java b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/status/StatusMediatorUnitTest.java
index 8b4cdce..aa7df2a 100644
--- a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/status/StatusMediatorUnitTest.java
+++ b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/status/StatusMediatorUnitTest.java
@@ -219,10 +219,6 @@
         return mModel.get(StatusProperties.STATUS_ICON_RESOURCE).getIconRes();
     }
 
-    private void assertModelIconResId(int resId) {
-        assertEquals(resId, mModel.get(StatusProperties.STATUS_ICON_RESOURCE).getIconRes());
-    }
-
     @Test
     public void testPermissionIconShown() {
         verify(mPermissionDialogController).addObserver(mPermissionObserverCaptor.capture());
@@ -1432,59 +1428,6 @@
     }
 
     @Test
-    public void statusIcon_blankWhenPendingHttpNavigation() {
-        mMediator.updateSecurityIcon(
-                R.drawable.ic_settings_tune_24dp, /* tintList= */ 0, /* desc= */ 0);
-
-        assertModelIconResId(R.drawable.ic_settings_tune_24dp);
-
-        doReturn(mNavigationEntry).when(mNavigationController).getPendingEntry();
-        doReturn(JUnitTestGURLs.BLUE_1).when(mNavigationEntry).getUrl();
-        mMediator.updateSecurityIcon(R.drawable.ic_info_24dp, /* tintList= */ 0, /* desc= */ 0);
-
-        assertNull(mModel.get(StatusProperties.STATUS_ICON_RESOURCE));
-
-        doReturn(null).when(mNavigationController).getPendingEntry();
-        mMediator.updateSecurityIcon(R.drawable.ic_info_24dp, /* tintList= */ 0, /* desc= */ 0);
-
-        assertModelIconResId(R.drawable.ic_info_24dp);
-    }
-
-    @Test
-    @DisableFeatures(OmniboxFeatureList.SUPPRESS_STATUS_ICON_DURING_HTTP_NAVIGATION)
-    public void statusIcon_notBlankWhenPendingHttpNavigation_killSwitch() {
-        mMediator.updateSecurityIcon(
-                R.drawable.ic_settings_tune_24dp, /* tintList= */ 0, /* desc= */ 0);
-
-        assertModelIconResId(R.drawable.ic_settings_tune_24dp);
-
-        doReturn(mNavigationEntry).when(mNavigationController).getPendingEntry();
-        doReturn(JUnitTestGURLs.BLUE_1).when(mNavigationEntry).getUrl();
-        mMediator.updateSecurityIcon(R.drawable.ic_info_24dp, /* tintList= */ 0, /* desc= */ 0);
-
-        assertModelIconResId(R.drawable.ic_info_24dp);
-    }
-
-    @Test
-    public void statusIcon_infoIconWhenPendingNonHttpNavigation() {
-        mMediator.updateSecurityIcon(
-                R.drawable.ic_settings_tune_24dp, /* tintList= */ 0, /* desc= */ 0);
-
-        assertModelIconResId(R.drawable.ic_settings_tune_24dp);
-
-        doReturn(mNavigationEntry).when(mNavigationController).getPendingEntry();
-        doReturn(JUnitTestGURLs.CHROME_ABOUT).when(mNavigationEntry).getUrl();
-        mMediator.updateSecurityIcon(R.drawable.ic_info_24dp, /* tintList= */ 0, /* desc= */ 0);
-
-        assertModelIconResId(R.drawable.ic_info_24dp);
-
-        doReturn(null).when(mNavigationController).getPendingEntry();
-        mMediator.updateSecurityIcon(R.drawable.ic_info_24dp, /* tintList= */ 0, /* desc= */ 0);
-
-        assertModelIconResId(R.drawable.ic_info_24dp);
-    }
-
-    @Test
     public void statusIcon_searchEngineIconShownWhenPendingNavigationToNtp() {
         doReturn(true).when(mNewTabPageDelegate).isCurrentlyVisible();
         doReturn(JUnitTestGURLs.NTP_URL).when(mNavigationEntry).getUrl();
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 5b8b4165..dcb9517 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
@@ -52,6 +52,7 @@
 import org.chromium.components.embedder_support.util.UrlUtilities;
 import org.chromium.components.metrics.OmniboxEventProtosIntDef.PageClassification;
 import org.chromium.components.omnibox.AutocompleteSchemeClassifier;
+import org.chromium.components.omnibox.OmniboxFeatures;
 import org.chromium.components.omnibox.OmniboxUrlEmphasizer;
 import org.chromium.components.omnibox.SecurityStatusIcon;
 import org.chromium.components.security_state.ConnectionMaliciousContentStatus;
@@ -865,12 +866,11 @@
         }
 
         // Suppress neutral/info icon during page load to avoid transition jank before
-        // SSL state is resolved for HTTP/HTTPS URLs if the toolbar refactor is enabled.
+        // SSL state is resolved for HTTP/HTTPS URLs.
         // Non-HTTP(S) schemes (e.g. chrome://, file://) never transition to SECURE and
         // should show their neutral icon immediately.
-        // TODO(crbug.com/553488661): We are now doing suppression by default in
-        // LocationBarMediator#updateLocationBarIcon. See about removing suppression here then.
-        if (ToolbarVariationUtils.isToolbarUiRefactorEnabled(mContext)
+        if ((ToolbarVariationUtils.isToolbarUiRefactorEnabled(mContext)
+                        || OmniboxFeatures.sSuppressStatusIconDuringHttpNavigation.isEnabled())
                 && securityLevel == ConnectionSecurityLevel.NONE
                 && isLoading()
                 && UrlUtilities.isHttpOrHttps(getCurrentGurl())) {