From c3968d605c3d42a036dd6fd6cc4193457e7a6464 Mon Sep 17 00:00:00 2001
From: Christoph Schwering <[email protected]>
Date: Mon, 13 Jul 2026 06:16:29 -0700
Subject: [PATCH] [Autofill] Hide popup in RenderFrameHostStateChanged()

The CL replaces RenderFrameDeleted() and DidFinishNavigation() with
RenderFrameHostStateChanged()... but keeps RenderFrameDeleted() again
because of a bug in RenderFrameHostStateChanged().

The intention is that RenderFrameHostStateChanged() is also
notified when a navigation in a parent frame makes the descendant
frame go into bfcache.

Bug: 514038302
Change-Id: I3fae9459774c66233a6236e473ab77c46a62225a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8071266
Reviewed-by: Jan Keitel <[email protected]>
Commit-Queue: Christoph Schwering <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1661091}
---

diff --git a/chrome/browser/ui/autofill/autofill_keyboard_accessory_controller_impl_unittest.cc b/chrome/browser/ui/autofill/autofill_keyboard_accessory_controller_impl_unittest.cc
index db7b0ad6..3c1edf4 100644
--- a/chrome/browser/ui/autofill/autofill_keyboard_accessory_controller_impl_unittest.cc
+++ b/chrome/browser/ui/autofill/autofill_keyboard_accessory_controller_impl_unittest.cc
@@ -185,10 +185,10 @@
                                          SuggestionType::kAutocompleteEntry)});
 
   EXPECT_CALL(manager().external_delegate(),
-              OnSuggestionsHidden(SuggestionHidingReason::kNavigation));
+              OnSuggestionsHidden(SuggestionHidingReason::kRendererEvent));
 
   client().suggestion_controller(manager()).Hide(
-      SuggestionHidingReason::kNavigation);
+      SuggestionHidingReason::kRendererEvent);
 }
 
 TEST_F(AutofillKeyboardAccessoryControllerImplTest,
diff --git a/chrome/browser/ui/autofill/autofill_popup_hide_helper.cc b/chrome/browser/ui/autofill/autofill_popup_hide_helper.cc
index 45ca6723..b4600df 100644
--- a/chrome/browser/ui/autofill/autofill_popup_hide_helper.cc
+++ b/chrome/browser/ui/autofill/autofill_popup_hide_helper.cc
@@ -69,23 +69,42 @@
   }
 }
 
-void AutofillPopupHideHelper::RenderFrameDeleted(
-    content::RenderFrameHost* rfh) {
+void AutofillPopupHideHelper::RenderFrameHostStateChanged(
+    content::RenderFrameHost* rfh,
+    content::RenderFrameHost::LifecycleState old_state,
+    content::RenderFrameHost::LifecycleState new_state) {
+  auto should_hide_popup = [](content::RenderFrameHost::LifecycleState state) {
+    switch (state) {
+      case content::RenderFrameHost::LifecycleState::kActive:
+        return false;
+      case content::RenderFrameHost::LifecycleState::kPendingCommit:
+      case content::RenderFrameHost::LifecycleState::kPrerendering:
+      case content::RenderFrameHost::LifecycleState::kInBackForwardCache:
+      case content::RenderFrameHost::LifecycleState::kPendingDeletion:
+        return true;
+    }
+    NOTREACHED();
+  };
+
   // If the popup menu has been triggered from within an iframe and that frame
   // is deleted, hide the popup. This is necessary because the popup may
   // actually be shown by the `AutofillExternalDelegate` of an ancestor frame,
   // which is not notified about `rfh`'s destruction and therefore won't close
   // the popup.
-  if (rfh_id_ == rfh->GetGlobalId()) {
+  if (rfh_id_ == rfh->GetGlobalId() && should_hide_popup(new_state)) {
     hiding_callback_.Run(SuggestionHidingReason::kRendererEvent);
   }
 }
 
-void AutofillPopupHideHelper::DidFinishNavigation(
-    content::NavigationHandle* navigation_handle) {
-  if (rfh_id_ == navigation_handle->GetPreviousRenderFrameHostId() &&
-      !navigation_handle->IsSameDocument()) {
-    hiding_callback_.Run(SuggestionHidingReason::kNavigation);
+void AutofillPopupHideHelper::RenderFrameDeleted(
+    content::RenderFrameHost* rfh) {
+  // RenderFrameHostStateChanged() is not called when on FrameTree::Shutdown():
+  // crbug.com/40693086.
+  // For the primary frame tree, this is caught by WebContentsDestroyed(), but
+  // for embedded frame trees we observe RenderFrameDeleted() to compensate for
+  // the missing RenderFrameHostStateChanged().
+  if (rfh_id_ == rfh->GetGlobalId()) {
+    hiding_callback_.Run(SuggestionHidingReason::kRendererEvent);
   }
 }
 
diff --git a/chrome/browser/ui/autofill/autofill_popup_hide_helper.h b/chrome/browser/ui/autofill/autofill_popup_hide_helper.h
index 34a5d434..80e5198 100644
--- a/chrome/browser/ui/autofill/autofill_popup_hide_helper.h
+++ b/chrome/browser/ui/autofill/autofill_popup_hide_helper.h
@@ -16,7 +16,6 @@
 #endif  // BUILDFLAG(IS_ANDROID)
 
 namespace content {
-class NavigationHandle;
 class RenderFrameHost;
 class RenderWidgetHost;
 enum class Visibility;
@@ -70,9 +69,11 @@
       content::RenderWidgetHost* render_widget_host) override;
   void PrimaryMainFrameWasResized(bool width_changed) override;
   void OnVisibilityChanged(content::Visibility visibility) override;
+  void RenderFrameHostStateChanged(
+      content::RenderFrameHost* render_frame_host,
+      content::RenderFrameHost::LifecycleState old_state,
+      content::RenderFrameHost::LifecycleState new_state) override;
   void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
-  void DidFinishNavigation(
-      content::NavigationHandle* navigation_handle) override;
 
 #if !BUILDFLAG(IS_ANDROID)
   // ZoomObserver:
diff --git a/chrome/browser/ui/autofill/autofill_suggestion_controller_unittest.cc b/chrome/browser/ui/autofill/autofill_suggestion_controller_unittest.cc
index 3eba931..9d7c1926 100644
--- a/chrome/browser/ui/autofill/autofill_suggestion_controller_unittest.cc
+++ b/chrome/browser/ui/autofill/autofill_suggestion_controller_unittest.cc
@@ -493,15 +493,7 @@
   ShowSuggestions(manager(), {SuggestionType::kAddressEntry});
   test::GenerateTestAutofillPopup(&manager().external_delegate());
   // The navigation generates a PrimaryMainFrameWasResized callback.
-  SuggestionHidingReason reason;
-  // On Android, keyboard accessory is not hidden if the Chrome native widget
-  // changes its size. The keyboard accessory is still hidden because the input
-  // field looses.
-  if constexpr (BUILDFLAG(IS_ANDROID)) {
-    reason = SuggestionHidingReason::kNavigation;
-  } else {
-    reason = SuggestionHidingReason::kWidgetChanged;
-  }
+  SuggestionHidingReason reason = SuggestionHidingReason::kRendererEvent;
   EXPECT_CALL(client().suggestion_controller(manager()), Hide(reason));
   NavigateAndCommitFrame(main_frame(), GURL("https://bar.com/"));
   // Verify and clear before TearDown() closes the popup.
@@ -514,14 +506,8 @@
        HideInSubFrameOnSubFrameNavigation) {
   ShowSuggestions(sub_manager(), {SuggestionType::kAddressEntry});
   test::GenerateTestAutofillPopup(&sub_manager().external_delegate());
-  if (sub_frame()->ShouldChangeRenderFrameHostOnSameSiteNavigation()) {
-    // If the RenderFrameHost changes, a RenderFrameDeleted will fire first.
-    EXPECT_CALL(client().suggestion_controller(sub_manager()),
-                Hide(SuggestionHidingReason::kRendererEvent));
-  } else {
-    EXPECT_CALL(client().suggestion_controller(sub_manager()),
-                Hide(SuggestionHidingReason::kNavigation));
-  }
+  EXPECT_CALL(client().suggestion_controller(sub_manager()),
+              Hide(SuggestionHidingReason::kRendererEvent));
   NavigateAndCommitFrame(sub_frame(), GURL("https://bar.com/"));
   // Verify and clear before TearDown() closes the popup.
   Mock::VerifyAndClearExpectations(
@@ -539,15 +525,7 @@
        HideInSubFrameOnMainFrameNavigation) {
   ShowSuggestions(sub_manager(), {SuggestionType::kAddressEntry});
   test::GenerateTestAutofillPopup(&sub_manager().external_delegate());
-  SuggestionHidingReason reason;
-  // On Android, keyboard accessory is not hidden if the Chrome native widget
-  // changes its size. The keyboard accessory is still hidden because the input
-  // field looses.
-  if constexpr (BUILDFLAG(IS_ANDROID)) {
-    reason = SuggestionHidingReason::kRendererEvent;
-  } else {
-    reason = SuggestionHidingReason::kWidgetChanged;
-  }
+  SuggestionHidingReason reason = SuggestionHidingReason::kRendererEvent;
   EXPECT_CALL(client().suggestion_controller(sub_manager()), Hide(reason));
   NavigateAndCommitFrame(main_frame(), GURL("https://bar.com/"));
 }
diff --git a/chrome/browser/ui/autofill/email_verifier/email_verification_popup_controller.cc b/chrome/browser/ui/autofill/email_verifier/email_verification_popup_controller.cc
index ed79c0b4..b380205c 100644
--- a/chrome/browser/ui/autofill/email_verifier/email_verification_popup_controller.cc
+++ b/chrome/browser/ui/autofill/email_verifier/email_verification_popup_controller.cc
@@ -22,9 +22,6 @@
     case SuggestionHidingReason::kEndEditing:
       return EmailVerificationPopupController::EvpPermissionUiStatus::
           kUserAborted;
-    case SuggestionHidingReason::kNavigation:
-      return EmailVerificationPopupController::EvpPermissionUiStatus::
-          kNavigation;
     case SuggestionHidingReason::kTabGone:
       return EmailVerificationPopupController::EvpPermissionUiStatus::kTabGone;
     case SuggestionHidingReason::kWidgetChanged:
diff --git a/components/autofill/core/browser/suggestions/suggestion_hiding_reason.h b/components/autofill/core/browser/suggestions/suggestion_hiding_reason.h
index 1c93163..42d22bb 100644
--- a/components/autofill/core/browser/suggestions/suggestion_hiding_reason.h
+++ b/components/autofill/core/browser/suggestions/suggestion_hiding_reason.h
@@ -25,8 +25,6 @@
   kFocusChanged = 3,
   // Scrolling or zooming into the page displaces the popup.
   kContentAreaMoved = 4,
-  // A navigation on the page or frame level.
-  kNavigation = 5,
   // The popup is or would become empty.
   kNoSuggestions = 6,

diff --git a/chrome/browser/ui/autofill/autofill_keyboard_accessory_controller_impl_unittest.cc b/chrome/browser/ui/autofill/autofill_keyboard_accessory_controller_impl_unittest.cc
index db7b0ad6..3c1edf4 100644
--- a/chrome/browser/ui/autofill/autofill_keyboard_accessory_controller_impl_unittest.cc
+++ b/chrome/browser/ui/autofill/autofill_keyboard_accessory_controller_impl_unittest.cc
@@ -185,10 +185,10 @@
                                          SuggestionType::kAutocompleteEntry)});
 
   EXPECT_CALL(manager().external_delegate(),
-              OnSuggestionsHidden(SuggestionHidingReason::kNavigation));
+              OnSuggestionsHidden(SuggestionHidingReason::kRendererEvent));
 
   client().suggestion_controller(manager()).Hide(
-      SuggestionHidingReason::kNavigation);
+      SuggestionHidingReason::kRendererEvent);
 }
 
 TEST_F(AutofillKeyboardAccessoryControllerImplTest,
diff --git a/chrome/browser/ui/autofill/autofill_suggestion_controller_unittest.cc b/chrome/browser/ui/autofill/autofill_suggestion_controller_unittest.cc
index 3eba931..9d7c1926 100644
--- a/chrome/browser/ui/autofill/autofill_suggestion_controller_unittest.cc
+++ b/chrome/browser/ui/autofill/autofill_suggestion_controller_unittest.cc
@@ -493,15 +493,7 @@
   ShowSuggestions(manager(), {SuggestionType::kAddressEntry});
   test::GenerateTestAutofillPopup(&manager().external_delegate());
   // The navigation generates a PrimaryMainFrameWasResized callback.
-  SuggestionHidingReason reason;
-  // On Android, keyboard accessory is not hidden if the Chrome native widget
-  // changes its size. The keyboard accessory is still hidden because the input
-  // field looses.
-  if constexpr (BUILDFLAG(IS_ANDROID)) {
-    reason = SuggestionHidingReason::kNavigation;
-  } else {
-    reason = SuggestionHidingReason::kWidgetChanged;
-  }
+  SuggestionHidingReason reason = SuggestionHidingReason::kRendererEvent;
   EXPECT_CALL(client().suggestion_controller(manager()), Hide(reason));
   NavigateAndCommitFrame(main_frame(), GURL("https://bar.com/"));
   // Verify and clear before TearDown() closes the popup.
@@ -514,14 +506,8 @@
        HideInSubFrameOnSubFrameNavigation) {
   ShowSuggestions(sub_manager(), {SuggestionType::kAddressEntry});
   test::GenerateTestAutofillPopup(&sub_manager().external_delegate());
-  if (sub_frame()->ShouldChangeRenderFrameHostOnSameSiteNavigation()) {
-    // If the RenderFrameHost changes, a RenderFrameDeleted will fire first.
-    EXPECT_CALL(client().suggestion_controller(sub_manager()),
-                Hide(SuggestionHidingReason::kRendererEvent));
-  } else {
-    EXPECT_CALL(client().suggestion_controller(sub_manager()),
-                Hide(SuggestionHidingReason::kNavigation));
-  }
+  EXPECT_CALL(client().suggestion_controller(sub_manager()),
+              Hide(SuggestionHidingReason::kRendererEvent));
   NavigateAndCommitFrame(sub_frame(), GURL("https://bar.com/"));
   // Verify and clear before TearDown() closes the popup.
   Mock::VerifyAndClearExpectations(
@@ -539,15 +525,7 @@
        HideInSubFrameOnMainFrameNavigation) {
   ShowSuggestions(sub_manager(), {SuggestionType::kAddressEntry});
   test::GenerateTestAutofillPopup(&sub_manager().external_delegate());
-  SuggestionHidingReason reason;
-  // On Android, keyboard accessory is not hidden if the Chrome native widget
-  // changes its size. The keyboard accessory is still hidden because the input
-  // field looses.
-  if constexpr (BUILDFLAG(IS_ANDROID)) {
-    reason = SuggestionHidingReason::kRendererEvent;
-  } else {
-    reason = SuggestionHidingReason::kWidgetChanged;
-  }
+  SuggestionHidingReason reason = SuggestionHidingReason::kRendererEvent;
   EXPECT_CALL(client().suggestion_controller(sub_manager()), Hide(reason));
   NavigateAndCommitFrame(main_frame(), GURL("https://bar.com/"));
 }