From a67260db5f3cc8c182306c070207c9e4e143da5f Mon Sep 17 00:00:00 2001
From: Eva Su <evasu@chromium.org>
Date: Tue, 15 Sep 2026 19:43:30 -0700
Subject: [PATCH] [Desktop Android] Navigate to main page on menu model updates on Android

When tab navigation or model updates occur while the site permissions
subpage is open in the Android extensions menu, the subpage previously
remained open and updated its selection listeners for the newly
committed page origin. This diverged from desktop behavior where the
menu returns to the main page upon navigation.

This CL updates ExtensionsMenuMediator to transition the current page
back to the main menu page when onModelChanged() is received while on
the site permissions page, synchronizing menu entries with the updated
model. It also validates site permissions page state before
transitioning to the subpage and ensures origin consistency during site
access updates.

Fixed: 518068748
Change-Id: Ifff14d53ff6d5839baeb9b168c3e8bca45106f0e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8414215
Commit-Queue: Eva Su <evasu@chromium.org>
Reviewed-by: Masa Fujita <massan@google.com>
Cr-Commit-Position: refs/heads/main@{#1698606}
---
diff --git a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/extensions/ExtensionsMenuMediator.java b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/extensions/ExtensionsMenuMediator.java
index 7cea644..269de4c 100644
--- a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/extensions/ExtensionsMenuMediator.java
+++ b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/extensions/ExtensionsMenuMediator.java
@@ -182,30 +182,31 @@
     }
 
     /**
-     * Called when the native extensions menu model has changed. This method checks which page is
-     * currently visible and pulls the relevant data from native to update the UI.
+     * Called when the native extensions menu model has changed. This method navigates back to the
+     * main page if the site permissions page is open and pulls the relevant data from native to
+     * update the UI.
      */
     @Override
     public void onModelChanged() {
-        if (getCurrentPage() == ExtensionsMenuProperties.Page.MAIN) {
-            int optionalSection = mMenuBridge.getOptionalSection();
-            mMainPageModel.set(ExtensionsMenuProperties.OPTIONAL_SECTION_TYPE, optionalSection);
-
-            if (optionalSection == ExtensionsMenuTypes.OptionalSectionType.HOST_ACCESS_REQUESTS) {
-                mMainPageModel.set(
-                        ExtensionsMenuProperties.HOST_ACCESS_REQUESTS,
-                        mMenuBridge.getHostAccessRequests());
-            } else {
-                mMainPageModel.set(
-                        ExtensionsMenuProperties.HOST_ACCESS_REQUESTS, new ArrayList<>());
-            }
-
-            updateMenuEntries();
-        } else if (getCurrentPage() == ExtensionsMenuProperties.Page.SITE_PERMISSIONS) {
-            String extensionId =
-                    mSitePermissionsPageModel.get(SitePermissionsPageProperties.EXTENSION_ID);
-            updateSitePermissionsPage(extensionId);
+        if (getCurrentPage() == ExtensionsMenuProperties.Page.SITE_PERMISSIONS) {
+            // If the site permissions page is open, navigate back to the main page to
+            // ensure settings are not applied to an unexpected origin.
+            mMainPageModel.set(
+                    ExtensionsMenuProperties.CURRENT_PAGE, ExtensionsMenuProperties.Page.MAIN);
         }
+
+        int optionalSection = mMenuBridge.getOptionalSection();
+        mMainPageModel.set(ExtensionsMenuProperties.OPTIONAL_SECTION_TYPE, optionalSection);
+
+        if (optionalSection == ExtensionsMenuTypes.OptionalSectionType.HOST_ACCESS_REQUESTS) {
+            mMainPageModel.set(
+                    ExtensionsMenuProperties.HOST_ACCESS_REQUESTS,
+                    mMenuBridge.getHostAccessRequests());
+        } else {
+            mMainPageModel.set(ExtensionsMenuProperties.HOST_ACCESS_REQUESTS, new ArrayList<>());
+        }
+
+        updateMenuEntries();
     }
 
     @Override
@@ -265,10 +266,9 @@
             return;
         }
 
-        // Return to the main page when extension is removed and had the site permissions page
-        // opened.
-        mMainPageModel.set(
-                ExtensionsMenuProperties.CURRENT_PAGE, ExtensionsMenuProperties.Page.MAIN);
+        // Return to the main page and refresh the menu entries when an extension with the site
+        // permissions page opened is removed.
+        onModelChanged();
     }
 
     @Override
@@ -433,12 +433,12 @@
     private void onSitePermissionsButtonClicked(String extensionId) {
         mSitePermissionsPageModel.set(SitePermissionsPageProperties.EXTENSION_ID, extensionId);
 
-        updateSitePermissionsPage(extensionId);
-
-        // Set current page to site permissions page.
-        mMainPageModel.set(
-                ExtensionsMenuProperties.CURRENT_PAGE,
-                ExtensionsMenuProperties.Page.SITE_PERMISSIONS);
+        if (updateSitePermissionsPage(extensionId)) {
+            // Set current page to site permissions page.
+            mMainPageModel.set(
+                    ExtensionsMenuProperties.CURRENT_PAGE,
+                    ExtensionsMenuProperties.Page.SITE_PERMISSIONS);
+        }
     }
 
     private void openUrlFromMenu(String url) {
@@ -541,14 +541,16 @@
      * Updates the site permissions page for the given extension.
      *
      * @param extensionId The ID of the extension to show permissions for.
+     * @return Whether the site permissions page was successfully updated.
      */
-    private void updateSitePermissionsPage(String extensionId) {
+    private boolean updateSitePermissionsPage(String extensionId) {
         ExtensionsMenuTypes.ExtensionSitePermissionsState sitePermissionsState =
                 mMenuBridge.getExtensionSitePermissionsState(extensionId);
         if (sitePermissionsState == null) {
             mMainPageModel.set(
                     ExtensionsMenuProperties.CURRENT_PAGE, ExtensionsMenuProperties.Page.MAIN);
-            return;
+            onModelChanged();
+            return false;
         }
 
         mSitePermissionsPageModel.set(
@@ -577,6 +579,7 @@
                 SitePermissionsPageProperties.SHOW_REQUESTS_TOGGLE_CLICK_LISTENER,
                 (buttonView, isChecked) ->
                         mMenuBridge.onShowRequestsTogglePressed(extensionId, isChecked));
+        return true;
     }
 
     /** Updates the zero state visibility. */
diff --git a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/extensions/ExtensionsMenuMediatorTest.java b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/extensions/ExtensionsMenuMediatorTest.java
index bd23cb0c..f2de5268 100644
--- a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/extensions/ExtensionsMenuMediatorTest.java
+++ b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/extensions/ExtensionsMenuMediatorTest.java
@@ -1510,6 +1510,161 @@
     }
 
     /**
+     * Tests that when model change occurs (e.g., page navigation) while the site permissions page
+     * is opened, the mediator navigates back to the main page.
+     */
+    @Test
+    public void testOnModelChanged_WhenSitePermissionsPageOpened_NavigatesToMainPage() {
+        // Mock that the site permissions page is visible for extension "id_a".
+        when(mMenuPropertyModel.get(ExtensionsMenuProperties.CURRENT_PAGE))
+                .thenReturn(ExtensionsMenuProperties.Page.SITE_PERMISSIONS);
+        when(mSitePermissionsPropertyModel.get(SitePermissionsPageProperties.EXTENSION_ID))
+                .thenReturn("id_a");
+
+        when(mExtensionsMenuBridgeJniMock.getOptionalSection(anyLong()))
+                .thenReturn(ExtensionsMenuTypes.OptionalSectionType.NONE);
+        when(mExtensionsMenuBridgeJniMock.getMenuEntries(anyLong())).thenReturn(new ArrayList<>());
+
+        mMenuMediator.onModelChanged();
+
+        // Verify that the menu navigated back to the main page.
+        verify(mMenuPropertyModel)
+                .set(ExtensionsMenuProperties.CURRENT_PAGE, ExtensionsMenuProperties.Page.MAIN);
+
+        // Verify that the site permissions state is not queried or updated on model change.
+        verify(mExtensionsMenuBridgeJniMock, never())
+                .getExtensionSitePermissionsState(anyLong(), any());
+        verify(mSitePermissionsPropertyModel, never())
+                .set(eq(SitePermissionsPageProperties.ON_SITE_ACCESS_SELECTED_LISTENER), any());
+    }
+
+    /**
+     * Tests that when a page navigation occurs while viewing the site permissions page, the
+     * mediator returns to the main page and does not rebind the site access listener with the
+     * updated origin.
+     */
+    @Test
+    public void testSitePermissionsPage_NavigationRevertsToMainPageWithoutRebindingListener() {
+        final String originA = "https://example.com";
+        final String originB = "https://sample-site.org";
+
+        ExtensionsMenuTypes.ControlState enabledOff =
+                new ExtensionsMenuTypes.ControlState(
+                        ExtensionsMenuTypes.ControlState.Status.ENABLED,
+                        "",
+                        "",
+                        "",
+                        /* isOn= */ false,
+                        /* icon= */ null);
+        ExtensionsMenuTypes.ControlState enabledOn =
+                new ExtensionsMenuTypes.ControlState(
+                        ExtensionsMenuTypes.ControlState.Status.ENABLED,
+                        "",
+                        "",
+                        "",
+                        /* isOn= */ true,
+                        /* icon= */ null);
+        ExtensionsMenuTypes.ExtensionSitePermissionsState stateA =
+                new ExtensionsMenuTypes.ExtensionSitePermissionsState(
+                        "Ext A", null, enabledOff, enabledOff, enabledOn, enabledOn, originA);
+        ExtensionsMenuTypes.ExtensionSitePermissionsState stateB =
+                new ExtensionsMenuTypes.ExtensionSitePermissionsState(
+                        "Ext A", null, enabledOff, enabledOff, enabledOn, enabledOn, originB);
+
+        when(mMenuPropertyModel.get(ExtensionsMenuProperties.CURRENT_PAGE))
+                .thenReturn(ExtensionsMenuProperties.Page.SITE_PERMISSIONS);
+        when(mSitePermissionsPropertyModel.get(SitePermissionsPageProperties.EXTENSION_ID))
+                .thenReturn("id_a");
+        when(mExtensionsMenuBridgeJniMock.getExtensionSitePermissionsState(anyLong(), eq("id_a")))
+                .thenReturn(stateA, stateB);
+
+        // Trigger model change when site permissions page is open.
+        mMenuMediator.onModelChanged();
+
+        // Verify mediator transitions back to main page.
+        verify(mMenuPropertyModel)
+                .set(ExtensionsMenuProperties.CURRENT_PAGE, ExtensionsMenuProperties.Page.MAIN);
+
+        // Verify listener was never set for originB on model changed.
+        verify(mSitePermissionsPropertyModel, never())
+                .set(eq(SitePermissionsPageProperties.ON_SITE_ACCESS_SELECTED_LISTENER), any());
+    }
+
+    /**
+     * Tests that clicking the site permissions button when permissions cannot be customized (e.g.,
+     * state is null) remains on the main page.
+     */
+    @Test
+    public void testSitePermissionsPage_WhenStateNull_RemainsOnMainPage() {
+        String extensionName = "Extension A";
+        Bitmap extensionIcon = ICON_RED;
+        List<ExtensionsMenuTypes.MenuEntryState> entries = new ArrayList<>();
+        entries.add(
+                ExtensionTestUtils.createMenuEntryWithHostPermissions(
+                        "id_a", extensionName, extensionIcon, /* isPinned= */ false));
+        when(mExtensionsMenuBridgeJniMock.getMenuEntries(anyLong())).thenReturn(entries);
+
+        // Mock null state for extension site permissions.
+        when(mExtensionsMenuBridgeJniMock.getExtensionSitePermissionsState(anyLong(), eq("id_a")))
+                .thenReturn(null);
+
+        // Open extensions menu.
+        mBridgeCaptor.getValue().onReady();
+        ListItem itemA = mActionModels.get(0);
+        View.OnClickListener listener =
+                itemA.model.get(ExtensionsMenuItemProperties.SITE_PERMISSIONS_BUTTON_ON_CLICK);
+        listener.onClick(null);
+
+        // Verify page never transitions to SITE_PERMISSIONS.
+        verify(mMenuPropertyModel, never())
+                .set(
+                        ExtensionsMenuProperties.CURRENT_PAGE,
+                        ExtensionsMenuProperties.Page.SITE_PERMISSIONS);
+    }
+
+    /**
+     * Tests that when an extension is removed while its site permissions page is open, the menu
+     * navigates back to the main page and refreshes the menu entries.
+     */
+    @Test
+    public void testOnActionRemoved_WhenSitePermissionsPageOpened_NavigatesToMainPage() {
+        List<ExtensionsMenuTypes.MenuEntryState> entries = new ArrayList<>();
+        entries.add(
+                ExtensionTestUtils.createSimpleMenuEntry(
+                        "id_a", "Extension A", ICON_RED, /* isPinned= */ false));
+        entries.add(
+                ExtensionTestUtils.createSimpleMenuEntry(
+                        "id_b", "Extension B", ICON_BLUE, /* isPinned= */ false));
+        when(mExtensionsMenuBridgeJniMock.getMenuEntries(anyLong())).thenReturn(entries);
+
+        // Open extensions menu.
+        mBridgeCaptor.getValue().onReady();
+        clearInvocations(mMenuPropertyModel);
+
+        // Mock site permissions page open for "id_a".
+        when(mMenuPropertyModel.get(ExtensionsMenuProperties.CURRENT_PAGE))
+                .thenReturn(ExtensionsMenuProperties.Page.SITE_PERMISSIONS);
+        when(mSitePermissionsPropertyModel.get(SitePermissionsPageProperties.EXTENSION_ID))
+                .thenReturn("id_a");
+        when(mExtensionsMenuBridgeJniMock.getMenuEntry(anyLong(), eq(0)))
+                .thenReturn(entries.get(0));
+
+        // When id_a is removed from native side, only id_b remains in menu entries.
+        List<ExtensionsMenuTypes.MenuEntryState> remainingEntries = new ArrayList<>();
+        remainingEntries.add(entries.get(1));
+        when(mExtensionsMenuBridgeJniMock.getMenuEntries(anyLong())).thenReturn(remainingEntries);
+
+        mBridgeCaptor.getValue().onActionRemoved(0);
+
+        // Verify navigation back to MAIN page.
+        verify(mMenuPropertyModel)
+                .set(ExtensionsMenuProperties.CURRENT_PAGE, ExtensionsMenuProperties.Page.MAIN);
+        // Verify action models were reconstructed with remaining entries.
+        assertEquals(1, mActionModels.size());
+        assertItemAt(0, "Extension B", ICON_BLUE);
+    }
+
+    /**
      * Tests that an extension marked as enterprise (installed by policy) is correctly identified
      * and represented in the menu item property model.
      */
diff --git a/chrome/browser/ui/extensions/extensions_menu_view_model_browsertest.cc b/chrome/browser/ui/extensions/extensions_menu_view_model_browsertest.cc
index 015254b..70eb7dc 100644
--- a/chrome/browser/ui/extensions/extensions_menu_view_model_browsertest.cc
+++ b/chrome/browser/ui/extensions/extensions_menu_view_model_browsertest.cc
@@ -420,6 +420,47 @@
             PermissionsManager::UserSiteAccess::kOnClick);
 }
 
+// Tests that the extensions menu view model fails to update site access if the
+// origin changes.
+IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewModelBrowserTest,
+                       UpdateSiteAccess_FailsIfOriginChanges) {
+  // Add extension that requests host permissions and withheld site access.
+  scoped_refptr<const extensions::Extension> extension =
+      AddExtensionWithHostPermission("Extension", "*://example.com/*");
+  extensions::ScriptingPermissionsModifier modifier(profile(), extension);
+  modifier.SetWithholdHostPermissions(true);
+
+  // Navigate to a site the extension requested access to.
+  const GURL urlA =
+      embedded_test_server()->GetURL("example.com", "/simple.html");
+  ASSERT_TRUE(NavigateToURL(GetActiveWebContents(), urlA));
+  content::WebContents* web_contents = GetActiveWebContents();
+  auto originA = web_contents->GetPrimaryMainFrame()->GetLastCommittedOrigin();
+
+  // Verify site interaction and site access are withheld.
+  EXPECT_EQ(permissions_helper()->GetSiteInteraction(*extension, web_contents),
+            SitePermissionsHelper::SiteInteraction::kWithheld);
+  EXPECT_EQ(permissions_manager()->GetUserSiteAccess(
+                *extension, web_contents->GetLastCommittedURL()),
+            PermissionsManager::UserSiteAccess::kOnClick);
+
+  // Simulate navigation to another site before user clicks.
+  const GURL urlB = embedded_test_server()->GetURL("other.com", "/simple.html");
+  ASSERT_TRUE(NavigateToURL(GetActiveWebContents(), urlB));
+
+  // Try to update site access using the previous origin.
+  menu_model()->UpdateSiteAccess(extension->id(), originA,
+                                 PermissionsManager::UserSiteAccess::kOnSite);
+
+  // Verify that site access was NOT granted because the origin changed.
+  EXPECT_EQ(permissions_helper()->GetSiteInteraction(*extension, web_contents),
+            SitePermissionsHelper::SiteInteraction::kNone);
+
+  // And site access for example.com should still be kOnClick (withheld).
+  EXPECT_EQ(permissions_manager()->GetUserSiteAccess(*extension, urlA),
+            PermissionsManager::UserSiteAccess::kOnClick);
+}
+
 // Tests that the extensions menu view model correctly revokes site access to an
 // extension that requests hosts permissions and access is currently granted.
 IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewModelBrowserTest,

diff --git a/chrome/browser/ui/extensions/extensions_menu_view_model_browsertest.cc b/chrome/browser/ui/extensions/extensions_menu_view_model_browsertest.cc
index 015254b..70eb7dc 100644
--- a/chrome/browser/ui/extensions/extensions_menu_view_model_browsertest.cc
+++ b/chrome/browser/ui/extensions/extensions_menu_view_model_browsertest.cc
@@ -420,6 +420,47 @@
             PermissionsManager::UserSiteAccess::kOnClick);
 }
 
+// Tests that the extensions menu view model fails to update site access if the
+// origin changes.
+IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewModelBrowserTest,
+                       UpdateSiteAccess_FailsIfOriginChanges) {
+  // Add extension that requests host permissions and withheld site access.
+  scoped_refptr<const extensions::Extension> extension =
+      AddExtensionWithHostPermission("Extension", "*://example.com/*");
+  extensions::ScriptingPermissionsModifier modifier(profile(), extension);
+  modifier.SetWithholdHostPermissions(true);
+
+  // Navigate to a site the extension requested access to.
+  const GURL urlA =
+      embedded_test_server()->GetURL("example.com", "/simple.html");
+  ASSERT_TRUE(NavigateToURL(GetActiveWebContents(), urlA));
+  content::WebContents* web_contents = GetActiveWebContents();
+  auto originA = web_contents->GetPrimaryMainFrame()->GetLastCommittedOrigin();
+
+  // Verify site interaction and site access are withheld.
+  EXPECT_EQ(permissions_helper()->GetSiteInteraction(*extension, web_contents),
+            SitePermissionsHelper::SiteInteraction::kWithheld);
+  EXPECT_EQ(permissions_manager()->GetUserSiteAccess(
+                *extension, web_contents->GetLastCommittedURL()),
+            PermissionsManager::UserSiteAccess::kOnClick);
+
+  // Simulate navigation to another site before user clicks.
+  const GURL urlB = embedded_test_server()->GetURL("other.com", "/simple.html");
+  ASSERT_TRUE(NavigateToURL(GetActiveWebContents(), urlB));
+
+  // Try to update site access using the previous origin.
+  menu_model()->UpdateSiteAccess(extension->id(), originA,
+                                 PermissionsManager::UserSiteAccess::kOnSite);
+
+  // Verify that site access was NOT granted because the origin changed.
+  EXPECT_EQ(permissions_helper()->GetSiteInteraction(*extension, web_contents),
+            SitePermissionsHelper::SiteInteraction::kNone);
+
+  // And site access for example.com should still be kOnClick (withheld).
+  EXPECT_EQ(permissions_manager()->GetUserSiteAccess(*extension, urlA),
+            PermissionsManager::UserSiteAccess::kOnClick);
+}
+
 // Tests that the extensions menu view model correctly revokes site access to an
 // extension that requests hosts permissions and access is currently granted.
 IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewModelBrowserTest,