[Passwords] Draw loading animation over unlock button inside popup

This CL removes the spinner located at the top of the passwords popup.
Instead, each button that can trigger it, will show the spinner to its
left.

See the linked bug for a screenshot and a brief screencast.

Bug: 1066470
Change-Id: I17c625e194842c55d2b313da85dcfb7712d23625
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2129709
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755851}

chrome/android/features/keyboard_accessory/internal/java/src/org/chromium/chrome/browser/keyboard_accessory/bar_component/KeyboardAccessoryMediator.java

@@ -136,7 +136,6 @@ class KeyboardAccessoryMediator
             case PopupItemId.ITEM_ID_GENERATE_PASSWORD_ENTRY:
             case PopupItemId.ITEM_ID_SHOW_ACCOUNT_CARDS:
             case PopupItemId.ITEM_ID_AUTOFILL_OPTIONS:
-            case PopupItemId.ITEM_ID_LOADING_SPINNER:
                 return false;
             case PopupItemId.ITEM_ID_AUTOCOMPLETE_ENTRY:
             case PopupItemId.ITEM_ID_PASSWORD_ENTRY:

chrome/browser/ui/autofill/autofill_popup_controller_impl.cc

@@ -425,7 +425,7 @@ bool AutofillPopupControllerImpl::RemoveSelectedLine() {
 }
 
 bool AutofillPopupControllerImpl::CanAccept(int id) {
-  return id != POPUP_ITEM_ID_SEPARATOR && id != POPUP_ITEM_ID_LOADING_SPINNER &&
+  return id != POPUP_ITEM_ID_SEPARATOR &&
          id != POPUP_ITEM_ID_INSECURE_CONTEXT_PAYMENT_DISABLED_MESSAGE &&
          id != POPUP_ITEM_ID_TITLE;
 }

chrome/browser/ui/views/autofill/autofill_popup_view_native_views.cc

@@ -7,8 +7,10 @@
 #include <algorithm>
 #include <memory>
 #include <string>
+#include <type_traits>
 #include <utility>
 
+#include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/branding_buildflags.h"
 #include "build/build_config.h"
@@ -25,6 +27,7 @@
 #include "components/autofill/core/browser/ui/suggestion.h"
 #include "components/omnibox/browser/vector_icons.h"
 #include "components/strings/grit/components_strings.h"
+#include "components/vector_icons/vector_icons.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -72,6 +75,14 @@ constexpr int kAdjacentLabelsVerticalSpacing = 2;
 // Default sice for icons in the autofill popup.
 constexpr int kIconSize = 16;
 
+// Popup footer items that use a leading icon instead of a trailing one.
+constexpr autofill::PopupItemId kItemTypesUsingLeadingIcons[] = {
+    autofill::PopupItemId::POPUP_ITEM_ID_SHOW_ACCOUNT_CARDS,
+    autofill::PopupItemId::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY,
+    autofill::PopupItemId::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN,
+    autofill::PopupItemId::
+        POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN_AND_GENERATE};
+
 int GetContentsVerticalPadding() {
   return ChromeLayoutProvider::Get()->GetDistanceMetric(
       DISTANCE_CONTENT_LIST_VERTICAL_MULTI);
@@ -117,6 +128,10 @@ gfx::ImageSkia GetIconImageByName(const std::string& icon_str) {
   if (icon_str == "globeIcon") {
     return gfx::CreateVectorIcon(kGlobeIcon, kIconSize, gfx::kChromeIconGrey);
   }
+  if (icon_str == "settingsIcon") {
+    return gfx::CreateVectorIcon(vector_icons::kSettingsIcon, kIconSize,
+                                 gfx::kChromeIconGrey);
+  }
   if (icon_str == "google") {
 #if BUILDFLAG(GOOGLE_CHROME_BRANDING)
     return gfx::CreateVectorIcon(kGoogleGLogoIcon, kIconSize,
@@ -359,32 +374,6 @@ class AutofillPopupSeparatorView : public AutofillPopupRowView {
   DISALLOW_COPY_AND_ASSIGN(AutofillPopupSeparatorView);
 };
 
-// Draws a loading throbber into the dropdown.
-class AutofillPopupLoadingSpinnerView : public AutofillPopupRowView {
- public:
-  ~AutofillPopupLoadingSpinnerView() override = default;
-
-  static AutofillPopupLoadingSpinnerView* Create(
-      AutofillPopupViewNativeViews* popup_view,
-      int line_number);
-
-  // views::View:
-  void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
-  void OnMouseEntered(const ui::MouseEvent& event) override {}
-  void OnMouseExited(const ui::MouseEvent& event) override {}
-  void OnMouseReleased(const ui::MouseEvent& event) override {}
-
- protected:
-  // AutofillPopupRowView:
-  void CreateContent() override;
-  void RefreshStyle() override;
-  std::unique_ptr<views::Background> CreateBackground() override;
-
- private:
-  AutofillPopupLoadingSpinnerView(AutofillPopupViewNativeViews* popup_view,
-                                  int line_number);
-};
-
 // Draws a row which contains a warning message.
 class AutofillPopupWarningView : public AutofillPopupRowView {
  public:
@@ -445,9 +434,7 @@ void AutofillPopupItemView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
   int pos_in_set = line_number() + 1;
   for (int i = 0; i < controller->GetLineCount(); ++i) {
     if (controller->GetSuggestionAt(i).frontend_id ==
-            autofill::POPUP_ITEM_ID_SEPARATOR ||
-        controller->GetSuggestionAt(i).frontend_id ==
-            autofill::POPUP_ITEM_ID_LOADING_SPINNER) {
+        autofill::POPUP_ITEM_ID_SEPARATOR) {
       if (i < line_number())
         --pos_in_set;
     } else {
@@ -568,7 +555,13 @@ void AutofillPopupItemView::RefreshStyle() {
   for (views::Label* label : inner_labels_) {
     label->SetAutoColorReadabilityEnabled(false);
     label->SetBackgroundColor(bk_color);
-    label->SetEnabledColor(fg_color);
+    // Set style depending on current state since the style isn't automatically
+    // adjusted after creation of the label.
+    label->SetEnabledColor(
+        label->GetEnabled()
+            ? fg_color
+            : views::style::GetColor(*this, label->GetTextContext(),
+                                     views::style::STYLE_DISABLED));
   }
   SchedulePaint();
 }
@@ -779,18 +772,19 @@ void AutofillPopupFooterView::CreateContent() {
           gfx::Insets(0, GetHorizontalMargin())));
 
   layout_manager->set_cross_axis_alignment(
-      views::BoxLayout::CrossAxisAlignment::kStretch);
+      views::BoxLayout::CrossAxisAlignment::kCenter);
 
-  const gfx::ImageSkia icon =
-      GetIconImage(controller->GetSuggestions()[line_number()]);
+  const Suggestion suggestion = controller->GetSuggestions()[line_number()];
+  const gfx::ImageSkia icon = GetIconImage(suggestion);
 
-  // A FooterView shows an icon, if any, on the trailing (right in LTR) side,
-  // but the Show Account Cards context is an anomaly. Its icon is on the
-  // leading (left in LTR) side.
   const bool use_leading_icon =
-      frontend_id() == autofill::PopupItemId::POPUP_ITEM_ID_SHOW_ACCOUNT_CARDS;
+      base::Contains(kItemTypesUsingLeadingIcons, frontend_id());
 
-  if (!icon.isNull() && use_leading_icon) {
+  if (suggestion.is_loading) {
+    SetEnabled(false);
+    AddChildView(std::make_unique<views::Throbber>())->Start();
+    AddSpacerWithSize(GetHorizontalMargin(), /*resize=*/false, layout_manager);
+  } else if (!icon.isNull() && use_leading_icon) {
     AddIcon(icon);
     AddSpacerWithSize(GetHorizontalMargin(), /*resize=*/false, layout_manager);
   }
@@ -802,6 +796,7 @@ void AutofillPopupFooterView::CreateContent() {
       AutofillPopupBaseView::GetCornerRadius());
 
   ViewWithLabel value_label = CreateValueLabel();
+  value_label.first->SetEnabled(!suggestion.is_loading);
   AddChildView(std::move(value_label.first));
   KeepLabel(value_label.second);
   AddSpacerWithSize(
@@ -891,59 +886,6 @@ AutofillPopupSeparatorView::AutofillPopupSeparatorView(
   SetFocusBehavior(FocusBehavior::NEVER);
 }
 
-/************** AutofillPopupLoadingSpinnerView **************/
-
-// static
-AutofillPopupLoadingSpinnerView* AutofillPopupLoadingSpinnerView::Create(
-    AutofillPopupViewNativeViews* popup_view,
-    int line_number) {
-  AutofillPopupLoadingSpinnerView* result =
-      new AutofillPopupLoadingSpinnerView(popup_view, line_number);
-  result->Init();
-  return result;
-}
-
-void AutofillPopupLoadingSpinnerView::GetAccessibleNodeData(
-    ui::AXNodeData* node_data) {
-  // Spinners are not selectable.
-  node_data->role = ax::mojom::Role::kSplitter;
-}
-
-void AutofillPopupLoadingSpinnerView::CreateContent() {
-  // Add a flex layout that positions the spinner in it's center.
-  auto layout = std::make_unique<views::FlexLayout>();
-  layout->SetOrientation(views::LayoutOrientation::kHorizontal);
-  layout->SetMainAxisAlignment(views::LayoutAlignment::kCenter);
-  layout->SetCrossAxisAlignment(views::LayoutAlignment::kCenter);
-  SetLayoutManager(std::move(layout));
-
-  // Add a throbber that fills the height of the row (minus its margins).
-  SetBorder(views::CreateEmptyBorder(gfx::Insets(
-      /*top=*/0, /*left=*/GetHorizontalMargin(),
-      ChromeLayoutProvider::Get()->GetDistanceMetric(
-          views::DISTANCE_RELATED_CONTROL_VERTICAL),
-      /*right=*/GetHorizontalMargin())));
-  auto throbber = std::make_unique<views::Throbber>();
-  throbber->Start();
-  AddChildView(std::move(throbber));
-}
-
-void AutofillPopupLoadingSpinnerView::RefreshStyle() {
-  SchedulePaint();
-}
-
-std::unique_ptr<views::Background>
-AutofillPopupLoadingSpinnerView::CreateBackground() {
-  return nullptr;
-}
-
-AutofillPopupLoadingSpinnerView::AutofillPopupLoadingSpinnerView(
-    AutofillPopupViewNativeViews* popup_view,
-    int line_number)
-    : AutofillPopupRowView(popup_view, line_number) {
-  SetFocusBehavior(FocusBehavior::NEVER);
-}
-
 /************** AutofillPopupWarningView **************/
 
 // static
@@ -1057,7 +999,7 @@ AutofillPopupViewNativeViews::AutofillPopupViewNativeViews(
   CreateChildViews();
 }
 
-AutofillPopupViewNativeViews::~AutofillPopupViewNativeViews() {}
+AutofillPopupViewNativeViews::~AutofillPopupViewNativeViews() = default;
 
 void AutofillPopupViewNativeViews::GetAccessibleNodeData(
     ui::AXNodeData* node_data) {
@@ -1178,11 +1120,6 @@ void AutofillPopupViewNativeViews::CreateChildViews() {
         rows_.push_back(AutofillPopupSeparatorView::Create(this, line_number));
         break;
 
-      case autofill::PopupItemId::POPUP_ITEM_ID_LOADING_SPINNER:
-        rows_.push_back(
-            AutofillPopupLoadingSpinnerView::Create(this, line_number));
-        break;
-
       case autofill::PopupItemId::
           POPUP_ITEM_ID_INSECURE_CONTEXT_PAYMENT_DISABLED_MESSAGE:
         rows_.push_back(AutofillPopupWarningView::Create(this, line_number));

chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc

@@ -44,6 +44,8 @@ const struct TypeClicks kClickTestCase[] = {
     {autofill::POPUP_ITEM_ID_USERNAME_ENTRY, 1},
     {autofill::POPUP_ITEM_ID_CREATE_HINT, 1},
     {autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY, 1},
+    {autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN, 1},
+    {autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN_AND_GENERATE, 1},
 };
 
 class TestAXEventObserver : public views::AXEventObserver {
@@ -154,13 +156,12 @@ TEST_F(AutofillPopupViewNativeViewsTest, AccessibilityTest) {
   CreateAndShowView({autofill::POPUP_ITEM_ID_DATALIST_ENTRY,
                      autofill::POPUP_ITEM_ID_SEPARATOR,
                      autofill::POPUP_ITEM_ID_AUTOCOMPLETE_ENTRY,
-                     autofill::POPUP_ITEM_ID_AUTOFILL_OPTIONS,
-                     autofill::POPUP_ITEM_ID_LOADING_SPINNER});
+                     autofill::POPUP_ITEM_ID_AUTOFILL_OPTIONS});
 
   // Select first item.
   view()->GetRowsForTesting()[0]->SetSelected(true);
 
-  EXPECT_EQ(view()->GetRowsForTesting().size(), 5u);
+  EXPECT_EQ(view()->GetRowsForTesting().size(), 4u);
 
   // Item 0.
   ui::AXNodeData node_data_0;
@@ -197,15 +198,6 @@ TEST_F(AutofillPopupViewNativeViewsTest, AccessibilityTest) {
   EXPECT_EQ(ax::mojom::Role::kMenuItem, node_data_3.role);
   EXPECT_FALSE(
       node_data_3.GetBoolAttribute(ax::mojom::BoolAttribute::kSelected));
-
-  // Item 4 (loading spinner).
-  ui::AXNodeData node_data_4;
-  view()->GetRowsForTesting()[1]->GetAccessibleNodeData(&node_data_4);
-  EXPECT_FALSE(node_data_4.HasIntAttribute(ax::mojom::IntAttribute::kPosInSet));
-  EXPECT_FALSE(node_data_4.HasIntAttribute(ax::mojom::IntAttribute::kSetSize));
-  EXPECT_EQ(ax::mojom::Role::kSplitter, node_data_4.role);
-  EXPECT_FALSE(
-      node_data_4.GetBoolAttribute(ax::mojom::BoolAttribute::kSelected));
 }
 
 TEST_F(AutofillPopupViewNativeViewsTest, Gestures) {
@@ -235,11 +227,30 @@ TEST_F(AutofillPopupViewNativeViewsTest, Gestures) {
   view()->GetRowsForTesting()[2]->OnGestureEvent(&tap_cancel);
 }
 
+TEST_F(AutofillPopupViewNativeViewsTest, ClickDisabledEntry) {
+  autofill::Suggestion opt_int_suggestion(
+      "", "", "", autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN);
+  opt_int_suggestion.is_loading = autofill::Suggestion::IsLoading(true);
+  autofill_popup_controller_.set_suggestions({opt_int_suggestion});
+  view_ = std::make_unique<autofill::AutofillPopupViewNativeViews>(
+      &autofill_popup_controller_, widget_.get());
+  widget_->SetContentsView(view_.get());
+  widget_->Show();
+
+  EXPECT_CALL(autofill_popup_controller_, AcceptSuggestion).Times(0);
+
+  gfx::Point inside_point(view()->GetRowsForTesting()[0]->x() + 1,
+                          view()->GetRowsForTesting()[0]->y() + 1);
+  ui::MouseEvent click_mouse_event(
+      ui::ET_MOUSE_PRESSED, inside_point, inside_point, ui::EventTimeForNow(),
+      ui::EF_RIGHT_MOUSE_BUTTON, ui::EF_RIGHT_MOUSE_BUTTON);
+  widget_->OnMouseEvent(&click_mouse_event);
+}
+
 TEST_P(AutofillPopupViewNativeViewsForEveryTypeTest, ShowClickTest) {
   const TypeClicks& click = GetParam();
   CreateAndShowView({click.id});
-  EXPECT_CALL(autofill_popup_controller_, AcceptSuggestion(::testing::_))
-      .Times(click.click);
+  EXPECT_CALL(autofill_popup_controller_, AcceptSuggestion).Times(click.click);
   gfx::Point center =
       view()->GetRowsForTesting()[0]->GetBoundsInScreen().CenterPoint();
 
@@ -252,9 +263,8 @@ TEST_P(AutofillPopupViewNativeViewsForEveryTypeTest, ShowClickTest) {
   view()->RemoveAllChildViews(true /* delete_children */);
 }
 
-INSTANTIATE_TEST_SUITE_P(
-    All,
-    AutofillPopupViewNativeViewsForEveryTypeTest,
-    ::testing::ValuesIn(kClickTestCase));
+INSTANTIATE_TEST_SUITE_P(All,
+                         AutofillPopupViewNativeViewsForEveryTypeTest,
+                         ::testing::ValuesIn(kClickTestCase));
 
 }  // namespace

components/autofill/core/browser/ui/popup_item_ids.h

@@ -30,7 +30,6 @@ enum PopupItemId {
   POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN = -16,
   POPUP_ITEM_ID_HIDE_AUTOFILL_SUGGESTIONS = -17,
   POPUP_ITEM_ID_USE_VIRTUAL_CARD = -18,
-  POPUP_ITEM_ID_LOADING_SPINNER = -20,
   POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN_AND_GENERATE = -21,
   POPUP_ITEM_ID_ACCOUNT_STORAGE_PASSWORD_ENTRY = -22,
   POPUP_ITEM_ID_ACCOUNT_STORAGE_USERNAME_ENTRY = -23,

components/autofill/core/browser/ui/suggestion.h

@@ -9,11 +9,14 @@
 
 #include "base/strings/string16.h"
 #include "base/strings/string_piece.h"
+#include "base/util/type_safety/strong_alias.h"
 #include "ui/gfx/image/image.h"
 
 namespace autofill {
 
 struct Suggestion {
+  using IsLoading = util::StrongAlias<class IsLoadingTag, bool>;
+
   enum MatchMode {
     PREFIX_MATCH,    // for prefix matched suggestions;
     SUBSTRING_MATCH  // for substring matched suggestions;
@@ -62,6 +65,8 @@ struct Suggestion {
   MatchMode match = PREFIX_MATCH;
   // |value| should be displayed as secondary text.
   bool is_value_secondary = false;
+  // Whether suggestion was interacted with and is now in a loading state.
+  IsLoading is_loading = IsLoading(false);
 };
 
 }  // namespace autofill

components/password_manager/core/browser/password_autofill_manager.cc

@@ -57,6 +57,8 @@ namespace password_manager {
 
 namespace {
 
+using IsLoading = autofill::Suggestion::IsLoading;
+
 constexpr base::char16 kPasswordReplacementChar = 0x2022;
 
 // Returns |username| unless it is empty. For an empty |username| returns a
@@ -195,6 +197,11 @@ void MaybeAppendManualFallback(syncer::SyncService* sync_service,
   autofill::Suggestion suggestion(
       l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_MANAGE_PASSWORDS));
   suggestion.frontend_id = autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY;
+  if (base::FeatureList::IsEnabled(
+          password_manager::features::kEnablePasswordsAccountStorage)) {
+    // The UI code will pick up an icon from the resources based on the string.
+    suggestion.icon = "settingsIcon";
+  }
   suggestions->push_back(std::move(suggestion));
 }
 
@@ -213,6 +220,7 @@ autofill::Suggestion CreateEntryToOptInToAccountStorageThenFill() {
       l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_OPT_INTO_ACCOUNT_STORE));
   suggestion.frontend_id =
       autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN;
+  suggestion.icon = CreateStoreIcon(/*uses_account_store=*/true);
   return suggestion;
 }
 
@@ -222,12 +230,7 @@ autofill::Suggestion CreateEntryToOptInToAccountStorageThenGenerate() {
       IDS_PASSWORD_MANAGER_OPT_INTO_ACCOUNT_STORED_GENERATION));
   suggestion.frontend_id =
       autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN_AND_GENERATE;
-  return suggestion;
-}
-
-autofill::Suggestion CreateLoadingSpinner() {
-  autofill::Suggestion suggestion;
-  suggestion.frontend_id = autofill::POPUP_ITEM_ID_LOADING_SPINNER;
+  suggestion.icon = CreateStoreIcon(/*uses_account_store=*/true);
   return suggestion;
 }
 
@@ -249,43 +252,24 @@ bool AreSuggestionForPasswordField(
                      });
 }
 
-std::vector<autofill::Suggestion> ReplaceUnlockButtonWithLoadingIndicator(
+std::vector<autofill::Suggestion> SetUnlockLoadingState(
     base::span<const autofill::Suggestion> suggestions,
-    autofill::PopupItemId unlock_item) {
+    autofill::PopupItemId unlock_item,
+    IsLoading is_loading) {
   DCHECK(
       unlock_item == autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN ||
       unlock_item ==
           autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN_AND_GENERATE);
   std::vector<autofill::Suggestion> new_suggestions;
-  new_suggestions.push_back(CreateLoadingSpinner());
-  std::copy_if(suggestions.begin(), suggestions.end(),
-               std::back_inserter(new_suggestions),
-               [unlock_item](const autofill::Suggestion& suggestion) {
-                 return suggestion.frontend_id != unlock_item;
-               });
-  return new_suggestions;
-}
-
-std::vector<autofill::Suggestion> ReplaceLoaderWithUnlock(
-    base::span<const autofill::Suggestion> suggestions,
-    autofill::PopupItemId unlock_item) {
-  std::vector<autofill::Suggestion> new_suggestions;
   new_suggestions.reserve(suggestions.size());
-  for (const auto& suggestion : suggestions) {
-    if (suggestion.frontend_id != autofill::POPUP_ITEM_ID_LOADING_SPINNER)
-      new_suggestions.push_back(suggestion);
-  }
-  switch (unlock_item) {
-    case autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN_AND_GENERATE:
-      new_suggestions.push_back(
-          CreateEntryToOptInToAccountStorageThenGenerate());
-      break;
-    case autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN:
-      new_suggestions.push_back(CreateEntryToOptInToAccountStorageThenFill());
-      break;
-    default:
-      NOTREACHED();
-  }
+  std::copy(suggestions.begin(), suggestions.end(),
+            std::back_inserter(new_suggestions));
+  auto unlock_iter =
+      std::find_if(new_suggestions.begin(), new_suggestions.end(),
+                   [unlock_item](const autofill::Suggestion& suggestion) {
+                     return suggestion.frontend_id == unlock_item;
+                   });
+  unlock_iter->is_loading = is_loading;
   return new_suggestions;
 }
 
@@ -342,8 +326,8 @@ void PasswordAutofillManager::OnUnlockItemAccepted(
       identity_manager->GetPrimaryAccountId(signin::ConsentLevel::kNotRequired);
   if (account_id.empty())
     return;
-  UpdatePopup(ReplaceUnlockButtonWithLoadingIndicator(
-      autofill_client_->GetPopupSuggestions(), unlock_item));
+  UpdatePopup(SetUnlockLoadingState(autofill_client_->GetPopupSuggestions(),
+                                    unlock_item, IsLoading(true)));
   autofill_client_->PinPopupView();
   password_client_->TriggerReauthForAccount(
       account_id,
@@ -698,8 +682,8 @@ void PasswordAutofillManager::OnUnlockReauthCompleted(
     }
     return;
   }
-  UpdatePopup(ReplaceLoaderWithUnlock(autofill_client_->GetPopupSuggestions(),
-                                      unlock_item));
+  UpdatePopup(SetUnlockLoadingState(autofill_client_->GetPopupSuggestions(),
+                                    unlock_item, IsLoading(false)));
 }
 
 }  //  namespace password_manager

components/password_manager/core/browser/password_autofill_manager_unittest.cc

@@ -495,16 +495,18 @@ TEST_F(PasswordAutofillManagerTest,
                                      .account_id;
   testing::Mock::VerifyAndClearExpectations(&autofill_client);
 
-  // Accepting a suggestion should trigger a call to update the popup. The first
-  // update removes the unlock button
+  // Accepting a suggestion should trigger a call to update the popup. The
+  // update puts the unlock button into a loading state.
+  std::vector<autofill::Suggestion> suggestions;
   EXPECT_CALL(
       autofill_client,
       UpdatePopup(
           SuggestionVectorIdsAre(ElementsAreArray(RemoveShowAllBeforeLollipop(
-              {autofill::POPUP_ITEM_ID_LOADING_SPINNER,
-               autofill::POPUP_ITEM_ID_PASSWORD_ENTRY,
-               autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY}))),
-          PopupType::kPasswords));
+              {autofill::POPUP_ITEM_ID_PASSWORD_ENTRY,
+               autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY,
+               autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN}))),
+          PopupType::kPasswords))
+      .WillOnce(testing::SaveArg<0>(&suggestions));
   EXPECT_CALL(autofill_client, PinPopupView);
   EXPECT_CALL(client, TriggerReauthForAccount(kAliceId, _));
   EXPECT_CALL(autofill_client, GetPopupSuggestions())
@@ -513,6 +515,8 @@ TEST_F(PasswordAutofillManagerTest,
   password_autofill_manager_->DidAcceptSuggestion(
       test_username_, autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN,
       1);
+  ASSERT_GE(suggestions.size(), 2u);
+  EXPECT_TRUE(suggestions.back().is_loading);
 }
 
 // Test that the popup is updated once "opt in and generate" is clicked.
@@ -527,16 +531,19 @@ TEST_F(PasswordAutofillManagerTest,
                                      .account_id;
   testing::Mock::VerifyAndClearExpectations(&autofill_client);
 
-  // Accepting a suggestion should trigger a call to update the popup. The first
-  // update removes the unlock button
+  // Accepting a suggestion should trigger a call to update the popup. The
+  // update puts the unlock-to-generate button in a loading state.
+  std::vector<autofill::Suggestion> suggestions;
   EXPECT_CALL(
       autofill_client,
       UpdatePopup(
           SuggestionVectorIdsAre(ElementsAreArray(RemoveShowAllBeforeLollipop(
-              {autofill::POPUP_ITEM_ID_LOADING_SPINNER,
-               autofill::POPUP_ITEM_ID_PASSWORD_ENTRY,
-               autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY}))),
-          PopupType::kPasswords));
+              {autofill::POPUP_ITEM_ID_PASSWORD_ENTRY,
+               autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY,
+               autofill::
+                   POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN_AND_GENERATE}))),
+          PopupType::kPasswords))
+      .WillOnce(testing::SaveArg<0>(&suggestions));
   EXPECT_CALL(autofill_client, PinPopupView);
   EXPECT_CALL(client, TriggerReauthForAccount(kAliceId, _));
   EXPECT_CALL(autofill_client, GetPopupSuggestions())
@@ -545,12 +552,15 @@ TEST_F(PasswordAutofillManagerTest,
   password_autofill_manager_->DidAcceptSuggestion(
       test_username_,
       autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN_AND_GENERATE, 1);
+  ASSERT_GE(suggestions.size(), 2u);
+  EXPECT_TRUE(suggestions.back().is_loading);
 }
 
 // Test that the popup is updated once "opt in and fill" is clicked.
 TEST_F(PasswordAutofillManagerTest, FailedOptInAndFillUpdatesPopup) {
   TestPasswordManagerClient client;
   NiceMock<MockAutofillClient> autofill_client;
+  std::vector<autofill::Suggestion> suggestions;
   InitializePasswordAutofillManager(&client, &autofill_client);
   client.SetAccountStorageOptIn(false);
   const CoreAccountId kAliceId = client.identity_test_env()
@@ -572,7 +582,7 @@ TEST_F(PasswordAutofillManagerTest, FailedOptInAndFillUpdatesPopup) {
     testing::Mock::VerifyAndClear(&autofill_client);
     EXPECT_CALL(autofill_client, GetPopupSuggestions)
         .WillOnce(Return(CreateTestSuggestions(
-            /*has_opt_in_and_fill=*/false, /*has_opt_in_and_generate*/ false)));
+            /*has_opt_in_and_fill=*/true, /*has_opt_in_and_generate*/ false)));
     EXPECT_CALL(client, TriggerReauthForAccount(kAliceId, _))
         .WillOnce([](const auto& unused, auto reauth_callback) {
           std::move(reauth_callback).Run(ReauthSucceeded(false));
@@ -584,18 +594,22 @@ TEST_F(PasswordAutofillManagerTest, FailedOptInAndFillUpdatesPopup) {
                 {autofill::POPUP_ITEM_ID_PASSWORD_ENTRY,
                  autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY,
                  autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN}))),
-            PopupType::kPasswords));
+            PopupType::kPasswords))
+        .WillOnce(testing::SaveArg<0>(&suggestions));
   });
 
   password_autofill_manager_->DidAcceptSuggestion(
       test_username_, autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN,
       1);
+  ASSERT_GE(suggestions.size(), 2u);
+  EXPECT_FALSE(suggestions.back().is_loading);
 }
 
 // Test that the popup is updated once "opt in and generate" is clicked.
 TEST_F(PasswordAutofillManagerTest, FailedOptInAndGenerateUpdatesPopup) {
   TestPasswordManagerClient client;
   NiceMock<MockAutofillClient> autofill_client;
+  std::vector<autofill::Suggestion> suggestions;
   InitializePasswordAutofillManager(&client, &autofill_client);
   client.SetAccountStorageOptIn(false);
   const CoreAccountId kAliceId = client.identity_test_env()
@@ -617,7 +631,7 @@ TEST_F(PasswordAutofillManagerTest, FailedOptInAndGenerateUpdatesPopup) {
     testing::Mock::VerifyAndClear(&autofill_client);
     EXPECT_CALL(autofill_client, GetPopupSuggestions)
         .WillOnce(Return(CreateTestSuggestions(
-            /*has_opt_in_and_fill=*/false, /*has_opt_in_and_generate*/ false)));
+            /*has_opt_in_and_fill=*/false, /*has_opt_in_and_generate*/ true)));
     EXPECT_CALL(client, TriggerReauthForAccount(kAliceId, _))
         .WillOnce([](const auto& unused, auto reauth_callback) {
           std::move(reauth_callback).Run(ReauthSucceeded(false));
@@ -630,12 +644,15 @@ TEST_F(PasswordAutofillManagerTest, FailedOptInAndGenerateUpdatesPopup) {
                  autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY,
                  autofill::
                      POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN_AND_GENERATE}))),
-            PopupType::kPasswords));
+            PopupType::kPasswords))
+        .WillOnce(testing::SaveArg<0>(&suggestions));
   });
 
   password_autofill_manager_->DidAcceptSuggestion(
       test_username_,
       autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN_AND_GENERATE, 1);
+  ASSERT_GE(suggestions.size(), 2u);
+  EXPECT_FALSE(suggestions.back().is_loading);
 }
 
 // Test that the popup is updated once "opt in and fill" is clicked.