From 83f649e5bfece81f7b064c4924ba5febc9cffd4c Mon Sep 17 00:00:00 2001
From: Ireneusz Szulc <[email protected]>
Date: Fri, 03 Jul 2026 08:30:06 -0700
Subject: [PATCH] Autofill: Invalidate pending deletion callbacks on suggestion updates

Prevent stale index deletion in AutofillKeyboardAccessoryControllerImpl
when the suggestions list updates while a deletion confirmation dialog
is open.

Bug: 513792983
Change-Id: If5aa50a3a08f73f0fd4ec25ab38a5e596a6a6964
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8032761
Commit-Queue: Ireneusz Szulc <[email protected]>
Reviewed-by: Christoph Schwering <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1656594}
---
diff --git a/chrome/browser/ui/autofill/autofill_keyboard_accessory_controller_impl.cc b/chrome/browser/ui/autofill/autofill_keyboard_accessory_controller_impl.cc
index 8df1879..859805c 100644
--- a/chrome/browser/ui/autofill/autofill_keyboard_accessory_controller_impl.cc
+++ b/chrome/browser/ui/autofill/autofill_keyboard_accessory_controller_impl.cc
@@ -459,6 +459,9 @@
     AutofillMetrics::SingleEntryRemovalMethod removal_method) {
   CHECK_EQ(removal_method,
            AutofillMetrics::SingleEntryRemovalMethod::kKeyboardAccessory);
+  if (base::checked_cast<size_t>(index) >= suggestions_.size()) {
+    return false;
+  }
   RemovalConfirmationText removal_text;
   if (!GetRemovalConfirmationText(index, &removal_text)) {
     return false;
@@ -469,30 +472,28 @@
       removal_text.confirm_button_text,
       base::BindOnce(
           &AutofillKeyboardAccessoryControllerImpl::OnDeletionDialogClosed,
-          GetWeakPtr(), index));
+          GetWeakPtr(), suggestions_[index]));
   return true;
 }
 
 void AutofillKeyboardAccessoryControllerImpl::OnDeletionDialogClosed(
-    int index,
+    const Suggestion& suggestion,
     bool confirmed) {
-  // This function might be called in a callback, so ensure the list index is
-  // still in bounds. If not, terminate the removing and consider it failed.
-  // TODO(crbug.com/40766704): Replace these checks with a stronger identifier.
-  if (base::checked_cast<size_t>(index) >= suggestions_.size()) {
+  auto it = std::ranges::find(suggestions_, suggestion);
+  if (it == suggestions_.end()) {
     return;
   }
   CHECK_EQ(suggestions_.size(), labels_.size());
 
   const FillingProduct filling_product =
-      GetFillingProductFromSuggestionType(GetSuggestionAt(index).type);
+      GetFillingProductFromSuggestionType(suggestion.type);
 
   if (filling_product == FillingProduct::kAddress && web_contents_) {
     PersonalDataManager* pdm = PersonalDataManagerFactory::GetForBrowserContext(
         web_contents_->GetBrowserContext());
 
-    const auto* payload = std::get_if<Suggestion::AutofillProfilePayload>(
-        &GetSuggestionAt(index).payload);
+    const auto* payload =
+        std::get_if<Suggestion::AutofillProfilePayload>(&suggestion.payload);
     if (pdm && payload) {
       const AutofillProfile* profile =
           pdm->address_data_manager().GetProfileByGUID(payload->guid.value());
@@ -507,7 +508,7 @@
     return;
   }
 
-  if (!delegate_->RemoveSuggestion(suggestions_[index])) {
+  if (!delegate_->RemoveSuggestion(suggestion)) {
     return;
   }
   switch (filling_product) {
@@ -538,7 +539,8 @@
   }
 
   // Remove the deleted element.
-  suggestions_.erase(suggestions_.begin() + index);
+  const size_t index = std::distance(suggestions_.begin(), it);
+  suggestions_.erase(it);
   labels_.erase(labels_.begin() + index);
 
   if (HasSuggestions()) {
diff --git a/chrome/browser/ui/autofill/autofill_keyboard_accessory_controller_impl.h b/chrome/browser/ui/autofill/autofill_keyboard_accessory_controller_impl.h
index ac604a8e..d246fac 100644
--- a/chrome/browser/ui/autofill/autofill_keyboard_accessory_controller_impl.h
+++ b/chrome/browser/ui/autofill/autofill_keyboard_accessory_controller_impl.h
@@ -111,10 +111,9 @@
   // of `suggestions_`.
   void OrderSuggestionsAndCreateLabels();
 
-  // Reacts to the result of a deletion dialog by attempting to delete the
-  // suggestion at `index` if the dialog `confirmed` deletion and by emitting
-  // metrics.
-  void OnDeletionDialogClosed(int index, bool confirmed);
+  // Reacts to the result of a deletion dialog by attempting to delete
+  // `suggestion` if the dialog `confirmed` deletion and by emitting metrics.
+  void OnDeletionDialogClosed(const Suggestion& suggestion, bool confirmed);
 
   // Hides the view and asynchronously deletes itself.
   void HideViewAndDie();
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 ffd4d78..db7b0ad6 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
@@ -403,6 +403,46 @@
       AutofillMetrics::SingleEntryRemovalMethod::kKeyboardAccessory));
 }
 
+// Tests that if suggestions are updated while a deletion confirmation dialog is
+// open, confirming the deletion of the old suggestion does not result in
+// deleting a wrong suggestion at a stale index.
+TEST_F(AutofillKeyboardAccessoryControllerImplTest,
+       RemoveSuggestion_StaleIndexDeletesWrongSuggestion) {
+  const auto suggestion1 =
+      Suggestion(u"Autocomplete entry 1", SuggestionType::kAutocompleteEntry);
+  const auto suggestion2 =
+      Suggestion(u"Autocomplete entry 2", SuggestionType::kAutocompleteEntry);
+
+  ShowSuggestions(manager(), {suggestion1, suggestion2});
+  ASSERT_TRUE(client().popup_view());
+
+  base::OnceCallback<void(bool)> captured_deletion_callback;
+  EXPECT_CALL(*client().popup_view(), ConfirmDeletion)
+      .WillOnce([&](const std::u16string& title, const std::u16string& body,
+                    const std::u16string& body_link,
+                    const std::u16string& confirm_button_text,
+                    base::OnceCallback<void(bool)> deletion_callback) {
+        captured_deletion_callback = std::move(deletion_callback);
+      });
+
+  // User long-presses suggestion at index 0
+  EXPECT_TRUE(client().suggestion_controller(manager()).RemoveSuggestion(
+      /*index=*/0,
+      AutofillMetrics::SingleEntryRemovalMethod::kKeyboardAccessory));
+  ASSERT_FALSE(captured_deletion_callback.is_null());
+
+  // While dialog is pending, suggestions list changes
+  const auto suggestion3 =
+      Suggestion(u"Autocomplete entry 3", SuggestionType::kAutocompleteEntry);
+  ShowSuggestions(manager(), {suggestion2, suggestion3});
+
+  // When user confirms deletion dialog, suggestion1 is no longer in
+  // suggestions_, so RemoveSuggestion is NEVER called.
+  EXPECT_CALL(manager().external_delegate(), RemoveSuggestion).Times(0);
+
+  std::move(captured_deletion_callback).Run(/*confirmed=*/true);
+}
+
 // When a suggestion is accepted, the popup is hidden inside
 // `delegate->DidAcceptSuggestion()`. On Android, some code is still being
 // executed after hiding. This test makes sure no use-after-free, null pointer

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 ffd4d78..db7b0ad6 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
@@ -403,6 +403,46 @@
       AutofillMetrics::SingleEntryRemovalMethod::kKeyboardAccessory));
 }
 
+// Tests that if suggestions are updated while a deletion confirmation dialog is
+// open, confirming the deletion of the old suggestion does not result in
+// deleting a wrong suggestion at a stale index.
+TEST_F(AutofillKeyboardAccessoryControllerImplTest,
+       RemoveSuggestion_StaleIndexDeletesWrongSuggestion) {
+  const auto suggestion1 =
+      Suggestion(u"Autocomplete entry 1", SuggestionType::kAutocompleteEntry);
+  const auto suggestion2 =
+      Suggestion(u"Autocomplete entry 2", SuggestionType::kAutocompleteEntry);
+
+  ShowSuggestions(manager(), {suggestion1, suggestion2});
+  ASSERT_TRUE(client().popup_view());
+
+  base::OnceCallback<void(bool)> captured_deletion_callback;
+  EXPECT_CALL(*client().popup_view(), ConfirmDeletion)
+      .WillOnce([&](const std::u16string& title, const std::u16string& body,
+                    const std::u16string& body_link,
+                    const std::u16string& confirm_button_text,
+                    base::OnceCallback<void(bool)> deletion_callback) {
+        captured_deletion_callback = std::move(deletion_callback);
+      });
+
+  // User long-presses suggestion at index 0
+  EXPECT_TRUE(client().suggestion_controller(manager()).RemoveSuggestion(
+      /*index=*/0,
+      AutofillMetrics::SingleEntryRemovalMethod::kKeyboardAccessory));
+  ASSERT_FALSE(captured_deletion_callback.is_null());
+
+  // While dialog is pending, suggestions list changes
+  const auto suggestion3 =
+      Suggestion(u"Autocomplete entry 3", SuggestionType::kAutocompleteEntry);
+  ShowSuggestions(manager(), {suggestion2, suggestion3});
+
+  // When user confirms deletion dialog, suggestion1 is no longer in
+  // suggestions_, so RemoveSuggestion is NEVER called.
+  EXPECT_CALL(manager().external_delegate(), RemoveSuggestion).Times(0);
+
+  std::move(captured_deletion_callback).Run(/*confirmed=*/true);
+}
+
 // When a suggestion is accepted, the popup is hidden inside
 // `delegate->DidAcceptSuggestion()`. On Android, some code is still being
 // executed after hiding. This test makes sure no use-after-free, null pointer