Move password popup building suggestions into helper methods

This CL should have no visible effect. (maybe metrics, see below)

Currently, the same popup suggestions are built in two different places
and a third one reuses one of the methods. For updating, there will be
at least two more code paths building suggestions which cannot reuse the
current code.
To prevent code duplication, this CL moves building the suggestions into
helper methods.

One notable change: previously, we would record the metric
"PasswordManager.ShowAllSavedPasswordsShownContext" when the suggestion
entry was added to the list of suggestions. This might have recorded the
metric prematurely since there is another check that occasionally
prevents showing the popup (and with it, the "Show all" button).

Bug: 1043963
Change-Id: Ia305c9102d6f1b2f6430e02617e83de02f9253e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2020704
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736273}

components/password_manager/core/browser/password_autofill_manager.cc

@@ -172,14 +172,47 @@ void GetSuggestions(const autofill::PasswordFormFillData& fill_data,
   }
 }
 
-bool ShouldShowManualFallbackForPreLollipop(syncer::SyncService* sync_service) {
+// Reauth doesn't work in Android L which prevents copying and revealing
+// credentials. Therefore, users have no benefit in visiting the settings page.
+void MaybeAppendManualFallback(syncer::SyncService* sync_service,
+                               std::vector<autofill::Suggestion>* suggestions) {
 #if defined(OS_ANDROID)
-  return base::android::BuildInfo::GetInstance()->sdk_int() >=
-             base::android::SDK_VERSION_LOLLIPOP ||
-         password_manager_util::IsSyncingWithNormalEncryption(sync_service);
-#else
-  return true;
+  if (base::android::BuildInfo::GetInstance()->sdk_int() <
+          base::android::SDK_VERSION_LOLLIPOP &&
+      !password_manager_util::IsSyncingWithNormalEncryption(sync_service))
+    return;
 #endif
+  autofill::Suggestion suggestion(
+      l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_MANAGE_PASSWORDS));
+  suggestion.frontend_id = autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY;
+  suggestions->push_back(std::move(suggestion));
+}
+
+autofill::Suggestion CreateGenerationEntry() {
+  autofill::Suggestion suggestion(
+      l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_GENERATE_PASSWORD));
+  // The UI code will pick up an icon from the resources based on the string.
+  suggestion.icon = "keyIcon";
+  suggestion.frontend_id = autofill::POPUP_ITEM_ID_GENERATE_PASSWORD_ENTRY;
+  return suggestion;
+}
+
+autofill::Suggestion CreateAccountStorageOptInEntry() {
+  // TODO(crbug.com/1024332): Add proper (translated) string.
+  autofill::Suggestion suggestion(
+      base::ASCIIToUTF16("Use passwords stored in your Google account"));
+  suggestion.frontend_id =
+      autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPTIN;
+  return suggestion;
+}
+
+bool ContainsOtherThanManagePasswords(
+    const std::vector<autofill::Suggestion> suggestions) {
+  return std::any_of(suggestions.begin(), suggestions.end(),
+                     [](const auto& suggestion) {
+                       return suggestion.frontend_id !=
+                              autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY;
+                     });
 }
 
 }  // namespace
@@ -318,113 +351,33 @@ void PasswordAutofillManager::OnShowPasswordSuggestions(
     const base::string16& typed_username,
     int options,
     const gfx::RectF& bounds) {
-  bool show_account_storage_optin = false;
-  if (password_client_) {
-    show_account_storage_optin = password_client_->GetPasswordFeatureManager()
-                                     ->ShouldShowAccountStorageOptIn();
-  }
-
-  if (!fill_data_ && !show_account_storage_optin) {
-    // Probably the credential was deleted in the mean time.
-    return;
-  }
-  std::vector<autofill::Suggestion> suggestions;
-  if (fill_data_) {
-    GetSuggestions(*fill_data_, typed_username, page_favicon_,
-                   (options & autofill::SHOW_ALL) != 0,
-                   (options & autofill::IS_PASSWORD_FIELD) != 0, &suggestions);
-  }
-
-  if (suggestions.empty() && !show_account_storage_optin) {
-    autofill_client_->HideAutofillPopup(
-        autofill::PopupHidingReason::kNoSuggestions);
-    return;
-  }
-
-  if (ShouldShowManualFallbackForPreLollipop(
-          autofill_client_->GetSyncService())) {
-    autofill::Suggestion suggestion(
-        l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_MANAGE_PASSWORDS));
-    suggestion.frontend_id = autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY;
-    suggestions.push_back(suggestion);
-
-    metrics_util::LogContextOfShowAllSavedPasswordsShown(
-        metrics_util::ShowAllSavedPasswordsContext::kPassword);
-  }
-
-  if (show_account_storage_optin) {
-    // TODO(crbug.com/1024332): Add proper (translated) string.
-    autofill::Suggestion suggestion(
-        base::ASCIIToUTF16("Use passwords stored in your Google account"));
-    suggestion.frontend_id =
-        autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPTIN;
-    suggestions.push_back(suggestion);
-  }
-
-  if (!password_manager_driver_->CanShowAutofillUi())
-    return;
-
-  metrics_util::LogPasswordDropdownShown(
-      metrics_util::PasswordDropdownState::kStandard,
-      password_client_->IsIncognito());
-  autofill_client_->ShowAutofillPopup(bounds, text_direction, suggestions,
-                                      false, autofill::PopupType::kPasswords,
-                                      weak_ptr_factory_.GetWeakPtr());
+  ShowPopup(
+      bounds, text_direction,
+      BuildSuggestions(ShowAllPasswords(options & autofill::SHOW_ALL),
+                       ForPasswordField(options & autofill::IS_PASSWORD_FIELD),
+                       typed_username, OffersGeneration(false),
+                       ShowPasswordSuggestions(true)));
 }
 
 bool PasswordAutofillManager::MaybeShowPasswordSuggestions(
     const gfx::RectF& bounds,
     base::i18n::TextDirection text_direction) {
-  if (!fill_data_)
-    return false;
-  OnShowPasswordSuggestions(text_direction, base::string16(),
-                            autofill::SHOW_ALL | autofill::IS_PASSWORD_FIELD,
-                            bounds);
-  return true;
+  return ShowPopup(
+      bounds, text_direction,
+      BuildSuggestions(ShowAllPasswords(true), ForPasswordField(true),
+                       base::string16(), OffersGeneration(false),
+                       ShowPasswordSuggestions(true)));
 }
 
 bool PasswordAutofillManager::MaybeShowPasswordSuggestionsWithGeneration(
     const gfx::RectF& bounds,
     base::i18n::TextDirection text_direction,
     bool show_password_suggestions) {
-  if (!fill_data_)
-    return false;
-  std::vector<autofill::Suggestion> suggestions;
-  if (show_password_suggestions) {
-    GetSuggestions(*fill_data_, base::string16(), page_favicon_,
-                   true /* show_all */, true /* is_password_field */,
-                   &suggestions);
-  }
-  // Add 'Generation' option.
-  // The UI code will pick up an icon from the resources based on the string.
-  autofill::Suggestion suggestion(
-      l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_GENERATE_PASSWORD));
-  suggestion.icon = "keyIcon";
-  suggestion.frontend_id = autofill::POPUP_ITEM_ID_GENERATE_PASSWORD_ENTRY;
-  suggestions.push_back(suggestion);
-
-  // Add "Manage passwords".
-  if (ShouldShowManualFallbackForPreLollipop(
-          autofill_client_->GetSyncService())) {
-    autofill::Suggestion suggestion(
-        l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_MANAGE_PASSWORDS));
-    suggestion.frontend_id = autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY;
-    suggestions.push_back(suggestion);
-
-    metrics_util::LogContextOfShowAllSavedPasswordsShown(
-        metrics_util::ShowAllSavedPasswordsContext::kPassword);
-  }
-
-  if (!password_manager_driver_->CanShowAutofillUi())
-    return false;
-
-  metrics_util::LogPasswordDropdownShown(
-      metrics_util::PasswordDropdownState::kStandardGenerate,
-      password_client_->IsIncognito());
-  autofill_client_->ShowAutofillPopup(bounds, text_direction, suggestions,
-                                      false, autofill::PopupType::kPasswords,
-                                      weak_ptr_factory_.GetWeakPtr());
-  return true;
+  return ShowPopup(
+      bounds, text_direction,
+      BuildSuggestions(ShowAllPasswords(true), ForPasswordField(true),
+                       base::string16(), OffersGeneration(true),
+                       ShowPasswordSuggestions(show_password_suggestions)));
 }
 
 void PasswordAutofillManager::DidNavigateMainFrame() {
@@ -446,6 +399,82 @@ bool PasswordAutofillManager::PreviewSuggestionForTest(
 ////////////////////////////////////////////////////////////////////////////////
 // PasswordAutofillManager, private:
 
+std::vector<autofill::Suggestion> PasswordAutofillManager::BuildSuggestions(
+    ShowAllPasswords show_all_passwords,
+    ForPasswordField for_password_field,
+    const base::string16& typed_username,
+    OffersGeneration offers_generation,
+    ShowPasswordSuggestions show_password_suggestions) {
+  std::vector<autofill::Suggestion> suggestions;
+  bool show_account_storage_optin =
+      !offers_generation && password_client_ &&
+      password_client_->GetPasswordFeatureManager()
+          ->ShouldShowAccountStorageOptIn();
+
+  if (!fill_data_ && !show_account_storage_optin) {
+    // Probably the credential was deleted in the mean time.
+    return suggestions;
+  }
+
+  // Add password suggestions if they exist and were requested.
+  if (show_password_suggestions && fill_data_) {
+    GetSuggestions(*fill_data_, typed_username, page_favicon_,
+                   show_all_passwords.value(), for_password_field.value(),
+                   &suggestions);
+  }
+
+  // Add password generation entry, if available.
+  if (offers_generation)
+    suggestions.push_back(CreateGenerationEntry());
+
+  // Add "Manage all passwords" link to settings.
+  MaybeAppendManualFallback(autofill_client_->GetSyncService(), &suggestions);
+
+  // Add button to opt into using the account storage for passwords.
+  if (show_account_storage_optin)
+    suggestions.push_back(CreateAccountStorageOptInEntry());
+
+  return suggestions;
+}
+
+void PasswordAutofillManager::LogMetricsForSuggestions(
+    const std::vector<autofill::Suggestion>& suggestions) const {
+  metrics_util::PasswordDropdownState dropdown_state =
+      metrics_util::PasswordDropdownState::kStandard;
+  for (const auto& suggestion : suggestions) {
+    switch (suggestion.frontend_id) {
+      case autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY:
+        metrics_util::LogContextOfShowAllSavedPasswordsShown(
+            metrics_util::ShowAllSavedPasswordsContext::kPassword);
+        continue;
+      case autofill::POPUP_ITEM_ID_GENERATE_PASSWORD_ENTRY:
+        dropdown_state = metrics_util::PasswordDropdownState::kStandardGenerate;
+        continue;
+    }
+  }
+  metrics_util::LogPasswordDropdownShown(dropdown_state,
+                                         password_client_->IsIncognito());
+}
+
+bool PasswordAutofillManager::ShowPopup(
+    const gfx::RectF& bounds,
+    base::i18n::TextDirection text_direction,
+    const std::vector<autofill::Suggestion>& suggestions) {
+  if (!password_manager_driver_->CanShowAutofillUi())
+    return false;
+  if (!ContainsOtherThanManagePasswords(suggestions)) {
+    autofill_client_->HideAutofillPopup(
+        autofill::PopupHidingReason::kNoSuggestions);
+    return false;
+  }
+  LogMetricsForSuggestions(suggestions);
+  autofill_client_->ShowAutofillPopup(bounds, text_direction, suggestions,
+                                      /*autoselect_first_suggestion=*/false,
+                                      autofill::PopupType::kPasswords,
+                                      weak_ptr_factory_.GetWeakPtr());
+  return true;
+}
+
 bool PasswordAutofillManager::FillSuggestion(const base::string16& username) {
   autofill::PasswordAndMetadata password_and_meta_data;
   if (fill_data_ && GetPasswordAndMetadataForUsername(

components/password_manager/core/browser/password_autofill_manager.h

@@ -12,6 +12,7 @@
 #include "base/i18n/rtl.h"
 #include "base/macros.h"
 #include "base/task/cancelable_task_tracker.h"
+#include "base/util/type_safety/strong_alias.h"
 #include "components/autofill/core/browser/autofill_client.h"
 #include "components/autofill/core/browser/ui/autofill_popup_delegate.h"
 #include "components/autofill/core/common/password_form_fill_data.h"
@@ -105,6 +106,29 @@ class PasswordAutofillManager : public autofill::AutofillPopupDelegate {
 #endif  // defined(UNIT_TEST)
 
  private:
+  using ForPasswordField = util::StrongAlias<class ForPasswordFieldTag, bool>;
+  using OffersGeneration = util::StrongAlias<class OffersGenerationTag, bool>;
+  using ShowAllPasswords = util::StrongAlias<class ShowAllPasswordsTag, bool>;
+  using ShowPasswordSuggestions =
+      util::StrongAlias<class ShowPasswordSuggestionsTag, bool>;
+
+  // Builds the suggestions used to show or update the autofill popup.
+  std::vector<autofill::Suggestion> BuildSuggestions(
+      ShowAllPasswords show_all_passwords,
+      ForPasswordField for_password_field,
+      const base::string16& typed_username,
+      OffersGeneration for_generation,
+      ShowPasswordSuggestions show_password_suggestions);
+
+  // Called just before showing a popup to log which |suggestions| were shown.
+  void LogMetricsForSuggestions(
+      const std::vector<autofill::Suggestion>& suggestions) const;
+
+  // Validates and forwards the given objects to the autofill client.
+  bool ShowPopup(const gfx::RectF& bounds,
+                 base::i18n::TextDirection text_direction,
+                 const std::vector<autofill::Suggestion>& suggestions);
+
   // Attempts to fill the password associated with user name |username|, and
   // returns true if it was successful.
   bool FillSuggestion(const base::string16& username);

components/password_manager/core/browser/password_autofill_manager_unittest.cc

@@ -27,6 +27,7 @@
 #include "components/autofill/core/common/form_field_data.h"
 #include "components/autofill/core/common/password_form_fill_data.h"
 #include "components/favicon/core/test/mock_favicon_service.h"
+#include "components/password_manager/core/browser/mock_password_feature_manager.h"
 #include "components/password_manager/core/browser/password_manager.h"
 #include "components/password_manager/core/browser/password_manager_metrics_util.h"
 #include "components/password_manager/core/browser/stub_password_manager_client.h"
@@ -60,9 +61,14 @@ using autofill::SuggestionVectorIconsAre;
 using autofill::SuggestionVectorIdsAre;
 using autofill::SuggestionVectorLabelsAre;
 using autofill::SuggestionVectorValuesAre;
+using favicon_base::FaviconImageCallback;
 using testing::_;
 using testing::ElementsAreArray;
+using testing::Invoke;
+using testing::NiceMock;
 using testing::Return;
+using testing::SizeIs;
+using testing::Unused;
 
 using UkmEntry = ukm::builders::PageWithPassword;
 
@@ -79,6 +85,7 @@ constexpr char kDropdownSelectedHistogram[] =
     "PasswordManager.PasswordDropdownItemSelected";
 constexpr char kDropdownShownHistogram[] =
     "PasswordManager.PasswordDropdownShown";
+const gfx::Image kTestFavicon = gfx::test::CreateImage(16, 16);
 
 class MockPasswordManagerDriver : public StubPasswordManagerDriver {
  public:
@@ -97,13 +104,23 @@ class TestPasswordManagerClient : public StubPasswordManagerClient {
   MockPasswordManagerDriver* mock_driver() { return &driver_; }
   const GURL& GetMainFrameURL() const override { return main_frame_url_; }
 
+  PasswordFeatureManager* GetPasswordFeatureManager() const override {
+    return feature_manager_.get();
+  }
+
+  void SetAccountStorageOptIn(bool opt_in) {
+    ON_CALL(*feature_manager_.get(), ShouldShowAccountStorageOptIn)
+        .WillByDefault(Return(!opt_in));
+  }
+
   MOCK_METHOD0(GeneratePassword, void());
   MOCK_METHOD0(GetFaviconService, favicon::FaviconService*());
-  MOCK_METHOD1(NavigateToManagePasswordsPage,
-               void(password_manager::ManagePasswordsReferrer));
+  MOCK_METHOD1(NavigateToManagePasswordsPage, void(ManagePasswordsReferrer));
 
  private:
   MockPasswordManagerDriver driver_;
+  std::unique_ptr<MockPasswordFeatureManager> feature_manager_{
+      new NiceMock<MockPasswordFeatureManager>};
   GURL main_frame_url_;
 };
 
@@ -117,6 +134,8 @@ class MockAutofillClient : public autofill::TestAutofillClient {
                     bool autoselect_first_suggestion,
                     PopupType popup_type,
                     base::WeakPtr<autofill::AutofillPopupDelegate> delegate));
+  MOCK_METHOD2(UpdatePopup,
+               void(const std::vector<autofill::Suggestion>&, PopupType));
   MOCK_METHOD1(HideAutofillPopup, void(autofill::PopupHidingReason));
   MOCK_METHOD1(ExecuteCommand, void(int));
 };
@@ -146,6 +165,14 @@ std::vector<std::string> GetIconsList(std::vector<std::string> icons) {
   return icons;
 }
 
+base::CancelableTaskTracker::TaskId
+RespondWithTestIcon(Unused, FaviconImageCallback callback, Unused) {
+  favicon_base::FaviconImageResult image_result;
+  image_result.image = kTestFavicon;
+  std::move(callback).Run(image_result);
+  return 1;
+}
+
 }  // namespace
 
 class PasswordAutofillManagerTest : public testing::Test {
@@ -167,9 +194,8 @@ class PasswordAutofillManagerTest : public testing::Test {
     fill_data_.password_field = password_field;
   }
 
-  void InitializePasswordAutofillManager(
-      TestPasswordManagerClient* client,
-      autofill::AutofillClient* autofill_client) {
+  void InitializePasswordAutofillManager(TestPasswordManagerClient* client,
+                                         MockAutofillClient* autofill_client) {
     password_autofill_manager_.reset(new PasswordAutofillManager(
         client->mock_driver(), autofill_client, client));
     favicon::MockFaviconService favicon_service;
@@ -183,6 +209,14 @@ class PasswordAutofillManagerTest : public testing::Test {
     EXPECT_CALL(*client, GetFaviconService()).WillRepeatedly(Return(nullptr));
   }
 
+  autofill::PasswordFormFillData CreateTestFormFillData() {
+    autofill::PasswordFormFillData data;
+    data.username_field.value = test_username_;
+    data.password_field.value = test_password_;
+    data.preferred_realm = "http://foo.com/";
+    return data;
+  }
+
  protected:
   autofill::PasswordFormFillData& fill_data() { return fill_data_; }
 
@@ -200,8 +234,7 @@ class PasswordAutofillManagerTest : public testing::Test {
 };
 
 TEST_F(PasswordAutofillManagerTest, FillSuggestion) {
-  std::unique_ptr<TestPasswordManagerClient> client(
-      new TestPasswordManagerClient);
+  auto client = std::make_unique<TestPasswordManagerClient>();
   InitializePasswordAutofillManager(client.get(), nullptr);
 
   EXPECT_CALL(*client->mock_driver(),
@@ -220,8 +253,7 @@ TEST_F(PasswordAutofillManagerTest, FillSuggestion) {
 }
 
 TEST_F(PasswordAutofillManagerTest, PreviewSuggestion) {
-  std::unique_ptr<TestPasswordManagerClient> client(
-      new TestPasswordManagerClient);
+  auto client = std::make_unique<TestPasswordManagerClient>();
   InitializePasswordAutofillManager(client.get(), nullptr);
 
   EXPECT_CALL(*client->mock_driver(),
@@ -245,32 +277,20 @@ TEST_F(PasswordAutofillManagerTest, ExternalDelegatePasswordSuggestions) {
   for (bool is_suggestion_on_password_field : {false, true}) {
     SCOPED_TRACE(testing::Message() << "is_suggestion_on_password_field = "
                                     << is_suggestion_on_password_field);
-    std::unique_ptr<TestPasswordManagerClient> client(
-        new TestPasswordManagerClient);
-    std::unique_ptr<MockAutofillClient> autofill_client(new MockAutofillClient);
+    auto client = std::make_unique<TestPasswordManagerClient>();
+    auto autofill_client = std::make_unique<NiceMock<MockAutofillClient>>();
     InitializePasswordAutofillManager(client.get(), autofill_client.get());
 
-    gfx::RectF element_bounds;
-    autofill::PasswordFormFillData data;
-    data.username_field.value = test_username_;
-    data.password_field.value = test_password_;
-    data.preferred_realm = "http://foo.com/";
+    // Load filling and favicon data.
+    autofill::PasswordFormFillData data = CreateTestFormFillData();
     favicon::MockFaviconService favicon_service;
     EXPECT_CALL(*client, GetFaviconService())
         .WillOnce(Return(&favicon_service));
-    favicon_base::FaviconImageCallback saved_callback;
     EXPECT_CALL(favicon_service, GetFaviconImageForPageURL(data.origin, _, _))
-        .WillOnce([&](auto, favicon_base::FaviconImageCallback callback, auto) {
-          saved_callback = std::move(callback);
-          return 1;
-        });
+        .WillOnce(Invoke(RespondWithTestIcon));
     password_autofill_manager_->OnAddPasswordFillData(data);
 
-    // Resolve the favicon.
-    favicon_base::FaviconImageResult image_result;
-    image_result.image = gfx::test::CreateImage(16, 16);
-    std::move(saved_callback).Run(image_result);
-
+    // Assemble IDs we expect: an entry and show all passwords button.
     std::vector<autofill::PopupItemId> ids = {
         is_suggestion_on_password_field
             ? autofill::POPUP_ITEM_ID_PASSWORD_ENTRY
@@ -278,11 +298,13 @@ TEST_F(PasswordAutofillManagerTest, ExternalDelegatePasswordSuggestions) {
     if (!IsPreLollipopAndroid()) {
       ids.push_back(autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY);
     }
+
+    // Show the popup and verify the suggestions.
     std::vector<Suggestion> suggestions;
     EXPECT_CALL(
         *autofill_client,
-        ShowAutofillPopup(
-            _, _, SuggestionVectorIdsAre(testing::ElementsAreArray(ids)), false,
+        ShowAutofillPopup(_, _, SuggestionVectorIdsAre(ElementsAreArray(ids)),
+                          /*autoselect_first_suggestion=*/false,
                           PopupType::kPasswords, _))
         .WillOnce(testing::SaveArg<2>(&suggestions));
 
@@ -290,10 +312,10 @@ TEST_F(PasswordAutofillManagerTest, ExternalDelegatePasswordSuggestions) {
         is_suggestion_on_password_field ? autofill::IS_PASSWORD_FIELD : 0;
     password_autofill_manager_->OnShowPasswordSuggestions(
         base::i18n::RIGHT_TO_LEFT, base::string16(), show_suggestion_options,
-        element_bounds);
+        gfx::RectF());
     ASSERT_GE(suggestions.size(), 1u);
-    EXPECT_TRUE(gfx::test::AreImagesEqual(suggestions[0].custom_icon,
-                                          image_result.image));
+    EXPECT_TRUE(
+        gfx::test::AreImagesEqual(suggestions[0].custom_icon, kTestFavicon));
 
     EXPECT_CALL(*client->mock_driver(),
                 FillSuggestion(test_username_, test_password_));
@@ -314,19 +336,41 @@ TEST_F(PasswordAutofillManagerTest, ExternalDelegatePasswordSuggestions) {
   }
 }
 
+// Test that the popup is updated once remote suggestions are unlocked.
+TEST_F(PasswordAutofillManagerTest, ShowUnlockButton) {
+  auto client = std::make_unique<TestPasswordManagerClient>();
+  auto autofill_client = std::make_unique<NiceMock<MockAutofillClient>>();
+  InitializePasswordAutofillManager(client.get(), autofill_client.get());
+  client->SetAccountStorageOptIn(false);
+
+  // Assemble IDs we expect: an entry, show all passwords and unlock button.
+  std::vector<autofill::PopupItemId> ids = {
+      autofill::POPUP_ITEM_ID_PASSWORD_ENTRY};
+  if (!IsPreLollipopAndroid()) {
+    ids.push_back(autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY);
+  }
+  ids.push_back(autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPTIN);
+
+  // Show the popup and verify the suggestions.
+  EXPECT_CALL(
+      *autofill_client,
+      ShowAutofillPopup(_, _, SuggestionVectorIdsAre(ElementsAreArray(ids)),
+                        /*autoselect_first_suggestion=*/false,
+                        PopupType::kPasswords, _));
+  password_autofill_manager_->OnShowPasswordSuggestions(
+      base::i18n::RIGHT_TO_LEFT, base::string16(),
+      autofill::SHOW_ALL | autofill::IS_PASSWORD_FIELD, gfx::RectF());
+}
+
 // Test that OnShowPasswordSuggestions correctly matches the given FormFieldData
 // to the known PasswordFormFillData, and extracts the right suggestions.
 TEST_F(PasswordAutofillManagerTest, ExtractSuggestions) {
-  std::unique_ptr<TestPasswordManagerClient> client(
-      new TestPasswordManagerClient);
-  std::unique_ptr<MockAutofillClient> autofill_client(new MockAutofillClient);
+  auto client = std::make_unique<TestPasswordManagerClient>();
+  auto autofill_client = std::make_unique<NiceMock<MockAutofillClient>>();
   InitializePasswordAutofillManager(client.get(), autofill_client.get());
 
   gfx::RectF element_bounds;
-  autofill::PasswordFormFillData data;
-  data.username_field.value = test_username_;
-  data.password_field.value = test_password_;
-  data.preferred_realm = "http://foo.com/";
+  autofill::PasswordFormFillData data = CreateTestFormFillData();
 
   autofill::PasswordAndMetadata additional;
   additional.realm = "https://foobarrealm.org";
@@ -348,17 +392,17 @@ TEST_F(PasswordAutofillManagerTest, ExtractSuggestions) {
               SuggestionVectorLabelsAre(testing::AllOf(
                   testing::Contains(base::UTF8ToUTF16("foo.com")),
                   testing::Contains(base::UTF8ToUTF16("foobarrealm.org"))))),
-          false, PopupType::kPasswords, _));
+          /*autoselect_first_suggestion=*/false, PopupType::kPasswords, _));
   password_autofill_manager_->OnShowPasswordSuggestions(
       base::i18n::RIGHT_TO_LEFT, base::string16(), 0, element_bounds);
 
   // Now simulate displaying suggestions matching "John".
-  EXPECT_CALL(
-      *autofill_client,
+  EXPECT_CALL(*autofill_client,
               ShowAutofillPopup(element_bounds, _,
-                        SuggestionVectorValuesAre(testing::ElementsAreArray(
+                                SuggestionVectorValuesAre(ElementsAreArray(
                                     GetSuggestionList({additional_username}))),
-                        false, PopupType::kPasswords, _));
+                                /*autoselect_first_suggestion=*/false,
+                                PopupType::kPasswords, _));
   password_autofill_manager_->OnShowPasswordSuggestions(
       base::i18n::RIGHT_TO_LEFT, base::ASCIIToUTF16("John"), 0, element_bounds);
 
@@ -367,9 +411,9 @@ TEST_F(PasswordAutofillManagerTest, ExtractSuggestions) {
       *autofill_client,
       ShowAutofillPopup(
           element_bounds, _,
-          SuggestionVectorValuesAre(testing::ElementsAreArray(
+          SuggestionVectorValuesAre(ElementsAreArray(
               GetSuggestionList({test_username_, additional_username}))),
-          false, PopupType::kPasswords, _));
+          /*autoselect_first_suggestion=*/false, PopupType::kPasswords, _));
   password_autofill_manager_->OnShowPasswordSuggestions(
       base::i18n::RIGHT_TO_LEFT, base::ASCIIToUTF16("xyz"), autofill::SHOW_ALL,
       element_bounds);
@@ -379,9 +423,8 @@ TEST_F(PasswordAutofillManagerTest, ExtractSuggestions) {
 // applications are displayed as the labels of suggestions on the UI (for
 // matches of all levels of preferredness).
 TEST_F(PasswordAutofillManagerTest, PrettifiedAndroidRealmsAreShownAsLabels) {
-  std::unique_ptr<TestPasswordManagerClient> client(
-      new TestPasswordManagerClient);
-  std::unique_ptr<MockAutofillClient> autofill_client(new MockAutofillClient);
+  auto client = std::make_unique<TestPasswordManagerClient>();
+  auto autofill_client = std::make_unique<NiceMock<MockAutofillClient>>();
   InitializePasswordAutofillManager(client.get(), autofill_client.get());
 
   autofill::PasswordFormFillData data;
@@ -402,22 +445,19 @@ TEST_F(PasswordAutofillManagerTest, PrettifiedAndroidRealmsAreShownAsLabels) {
                                         "android://com.example1.android/")),
                                     testing::Contains(base::ASCIIToUTF16(
                                         "android://com.example2.android/")))),
-                                false, PopupType::kPasswords, _));
+                                /*autoselect_first_suggestion=*/false,
+                                PopupType::kPasswords, _));
   password_autofill_manager_->OnShowPasswordSuggestions(
       base::i18n::RIGHT_TO_LEFT, base::string16(), 0, gfx::RectF());
 }
 
 TEST_F(PasswordAutofillManagerTest, FillSuggestionPasswordField) {
-  std::unique_ptr<TestPasswordManagerClient> client(
-      new TestPasswordManagerClient);
-  std::unique_ptr<MockAutofillClient> autofill_client(new MockAutofillClient);
+  auto client = std::make_unique<TestPasswordManagerClient>();
+  auto autofill_client = std::make_unique<NiceMock<MockAutofillClient>>();
   InitializePasswordAutofillManager(client.get(), autofill_client.get());
 
   gfx::RectF element_bounds;
-  autofill::PasswordFormFillData data;
-  data.username_field.value = test_username_;
-  data.password_field.value = test_password_;
-  data.preferred_realm = "http://foo.com/";
+  autofill::PasswordFormFillData data = CreateTestFormFillData();
 
   autofill::PasswordAndMetadata additional;
   additional.realm = "https://foobarrealm.org";
@@ -426,12 +466,12 @@ TEST_F(PasswordAutofillManagerTest, FillSuggestionPasswordField) {
 
   password_autofill_manager_->OnAddPasswordFillData(data);
 
-  EXPECT_CALL(
-      *autofill_client,
+  EXPECT_CALL(*autofill_client,
               ShowAutofillPopup(element_bounds, _,
-                        SuggestionVectorValuesAre(testing::ElementsAreArray(
+                                SuggestionVectorValuesAre(ElementsAreArray(
                                     GetSuggestionList({test_username_}))),
-                        false, PopupType::kPasswords, _));
+                                /*autoselect_first_suggestion=*/false,
+                                PopupType::kPasswords, _));
   password_autofill_manager_->OnShowPasswordSuggestions(
       base::i18n::RIGHT_TO_LEFT, test_username_, autofill::IS_PASSWORD_FIELD,
       element_bounds);
@@ -444,9 +484,8 @@ TEST_F(PasswordAutofillManagerTest, DisplaySuggestionsWithMatchingTokens) {
   features.InitAndEnableFeature(
       autofill::features::kAutofillTokenPrefixMatching);
 
-  std::unique_ptr<TestPasswordManagerClient> client(
-      new TestPasswordManagerClient);
-  std::unique_ptr<MockAutofillClient> autofill_client(new MockAutofillClient);
+  auto client = std::make_unique<TestPasswordManagerClient>();
+  auto autofill_client = std::make_unique<NiceMock<MockAutofillClient>>();
   InitializePasswordAutofillManager(client.get(), autofill_client.get());
 
   gfx::RectF element_bounds;
@@ -463,12 +502,13 @@ TEST_F(PasswordAutofillManagerTest, DisplaySuggestionsWithMatchingTokens) {
 
   password_autofill_manager_->OnAddPasswordFillData(data);
 
-  EXPECT_CALL(*autofill_client,
+  EXPECT_CALL(
+      *autofill_client,
       ShowAutofillPopup(
           element_bounds, _,
           SuggestionVectorValuesAre(testing::UnorderedElementsAreArray(
               GetSuggestionList({username, additional_username}))),
-                  false, PopupType::kPasswords, _));
+          /*autoselect_first_suggestion=*/false, PopupType::kPasswords, _));
   password_autofill_manager_->OnShowPasswordSuggestions(
       base::i18n::RIGHT_TO_LEFT, base::ASCIIToUTF16("foo"), 0, element_bounds);
 }
@@ -480,9 +520,8 @@ TEST_F(PasswordAutofillManagerTest, NoSuggestionForNonPrefixTokenMatch) {
   features.InitAndEnableFeature(
       autofill::features::kAutofillTokenPrefixMatching);
 
-  std::unique_ptr<TestPasswordManagerClient> client(
-      new TestPasswordManagerClient);
-  std::unique_ptr<MockAutofillClient> autofill_client(new MockAutofillClient);
+  auto client = std::make_unique<TestPasswordManagerClient>();
+  auto autofill_client = std::make_unique<NiceMock<MockAutofillClient>>();
   InitializePasswordAutofillManager(client.get(), autofill_client.get());
 
   gfx::RectF element_bounds;
@@ -513,9 +552,8 @@ TEST_F(PasswordAutofillManagerTest,
   features.InitAndEnableFeature(
       autofill::features::kAutofillTokenPrefixMatching);
 
-  std::unique_ptr<TestPasswordManagerClient> client(
-      new TestPasswordManagerClient);
-  std::unique_ptr<MockAutofillClient> autofill_client(new MockAutofillClient);
+  auto client = std::make_unique<TestPasswordManagerClient>();
+  auto autofill_client = std::make_unique<NiceMock<MockAutofillClient>>();
   InitializePasswordAutofillManager(client.get(), autofill_client.get());
 
   gfx::RectF element_bounds;
@@ -532,12 +570,12 @@ TEST_F(PasswordAutofillManagerTest,
 
   password_autofill_manager_->OnAddPasswordFillData(data);
 
-  EXPECT_CALL(
-      *autofill_client,
+  EXPECT_CALL(*autofill_client,
               ShowAutofillPopup(element_bounds, _,
-                        SuggestionVectorValuesAre(testing::ElementsAreArray(
+                                SuggestionVectorValuesAre(ElementsAreArray(
                                     GetSuggestionList({additional_username}))),
-                        false, PopupType::kPasswords, _));
+                                /*autoselect_first_suggestion=*/false,
+                                PopupType::kPasswords, _));
   password_autofill_manager_->OnShowPasswordSuggestions(
       base::i18n::RIGHT_TO_LEFT, base::ASCIIToUTF16("foo@exam"), 0,
       element_bounds);
@@ -552,9 +590,8 @@ TEST_F(PasswordAutofillManagerTest,
   features.InitAndEnableFeature(
       autofill::features::kAutofillTokenPrefixMatching);
 
-  std::unique_ptr<TestPasswordManagerClient> client(
-      new TestPasswordManagerClient);
-  std::unique_ptr<MockAutofillClient> autofill_client(new MockAutofillClient);
+  auto client = std::make_unique<TestPasswordManagerClient>();
+  auto autofill_client = std::make_unique<NiceMock<MockAutofillClient>>();
   InitializePasswordAutofillManager(client.get(), autofill_client.get());
 
   gfx::RectF element_bounds;
@@ -571,12 +608,13 @@ TEST_F(PasswordAutofillManagerTest,
 
   password_autofill_manager_->OnAddPasswordFillData(data);
 
-  EXPECT_CALL(*autofill_client,
+  EXPECT_CALL(
+      *autofill_client,
       ShowAutofillPopup(
           element_bounds, _,
-                  SuggestionVectorValuesAre(testing::ElementsAreArray(
+          SuggestionVectorValuesAre(ElementsAreArray(
               GetSuggestionList({username, additional_username}))),
-                  false, PopupType::kPasswords, _));
+          /*autoselect_first_suggestion=*/false, PopupType::kPasswords, _));
   password_autofill_manager_->OnShowPasswordSuggestions(
       base::i18n::RIGHT_TO_LEFT, base::ASCIIToUTF16("foo"), false,
       element_bounds);
@@ -584,9 +622,8 @@ TEST_F(PasswordAutofillManagerTest,
 
 TEST_F(PasswordAutofillManagerTest, PreviewAndFillEmptyUsernameSuggestion) {
   // Initialize PasswordAutofillManager with credentials without username.
-  std::unique_ptr<TestPasswordManagerClient> client(
-      new TestPasswordManagerClient);
-  std::unique_ptr<MockAutofillClient> autofill_client(new MockAutofillClient);
+  auto client = std::make_unique<TestPasswordManagerClient>();
+  auto autofill_client = std::make_unique<NiceMock<MockAutofillClient>>();
   fill_data().username_field.value.clear();
   InitializePasswordAutofillManager(client.get(), autofill_client.get());
 
@@ -597,7 +634,8 @@ TEST_F(PasswordAutofillManagerTest, PreviewAndFillEmptyUsernameSuggestion) {
   EXPECT_CALL(*autofill_client, ShowAutofillPopup);
   gfx::RectF element_bounds;
   password_autofill_manager_->OnShowPasswordSuggestions(
-      base::i18n::RIGHT_TO_LEFT, base::string16(), false, element_bounds);
+      base::i18n::RIGHT_TO_LEFT, base::string16(),
+      /*autoselect_first_suggestion=*/false, element_bounds);
 
   // Check that preview of the empty username works.
   EXPECT_CALL(*client->mock_driver(),
@@ -626,29 +664,25 @@ TEST_F(PasswordAutofillManagerTest, ShowAllPasswordsOptionOnPasswordField) {
       "PasswordManager.ShowAllSavedPasswordsAcceptedContext";
   base::HistogramTester histograms;
 
-  MockAutofillClient autofill_client;
+  NiceMock<MockAutofillClient> autofill_client;
   auto client = std::make_unique<TestPasswordManagerClient>();
-  auto manager =
-      std::make_unique<password_manager::PasswordManager>(client.get());
+  auto manager = std::make_unique<PasswordManager>(client.get());
   InitializePasswordAutofillManager(client.get(), &autofill_client);
 
   ON_CALL(*(client->mock_driver()), GetPasswordManager())
       .WillByDefault(testing::Return(manager.get()));
 
   gfx::RectF element_bounds;
-  autofill::PasswordFormFillData data;
-  data.username_field.value = test_username_;
-  data.password_field.value = test_password_;
-  data.origin = GURL("https://foo.test");
+  autofill::PasswordFormFillData data = CreateTestFormFillData();
 
   password_autofill_manager_->OnAddPasswordFillData(data);
 
-  EXPECT_CALL(
-      autofill_client,
+  EXPECT_CALL(autofill_client,
               ShowAutofillPopup(element_bounds, _,
-                        SuggestionVectorValuesAre(testing::ElementsAreArray(
+                                SuggestionVectorValuesAre(ElementsAreArray(
                                     GetSuggestionList({test_username_}))),
-                        false, PopupType::kPasswords, _));
+                                /*autoselect_first_suggestion=*/false,
+                                PopupType::kPasswords, _));
 
   password_autofill_manager_->OnShowPasswordSuggestions(
       base::i18n::RIGHT_TO_LEFT, test_username_, autofill::IS_PASSWORD_FIELD,
@@ -664,10 +698,8 @@ TEST_F(PasswordAutofillManagerTest, ShowAllPasswordsOptionOnPasswordField) {
         metrics_util::ShowAllSavedPasswordsContext::kPassword, 1);
     // Clicking at the "Show all passwords row" should trigger a call to open
     // the Password Manager settings page and hide the popup.
-    EXPECT_CALL(
-        *client,
-        NavigateToManagePasswordsPage(
-            password_manager::ManagePasswordsReferrer::kPasswordDropdown));
+    EXPECT_CALL(*client, NavigateToManagePasswordsPage(
+                             ManagePasswordsReferrer::kPasswordDropdown));
     EXPECT_CALL(
         autofill_client,
         HideAutofillPopup(autofill::PopupHidingReason::kAcceptSuggestion));
@@ -697,8 +729,8 @@ TEST_F(PasswordAutofillManagerTest, ShowAllPasswordsOptionOnPasswordField) {
       ukm::TestUkmRecorder::ExpectEntryMetric(
           entry, UkmEntry::kPageLevelUserActionName,
           static_cast<int64_t>(
-              password_manager::PasswordManagerMetricsRecorder::
-                  PageLevelUserAction::kShowAllPasswordsWhileSomeAreSuggested));
+              PasswordManagerMetricsRecorder::PageLevelUserAction::
+                  kShowAllPasswordsWhileSomeAreSuggested));
     }
   } else {
     EXPECT_THAT(histograms.GetAllSamples(kShownContextHistogram),
@@ -712,23 +744,20 @@ TEST_F(PasswordAutofillManagerTest, ShowAllPasswordsOptionOnPasswordField) {
 // fields of login forms.
 TEST_F(PasswordAutofillManagerTest, ShowAllPasswordsOptionOnNonPasswordField) {
   auto client = std::make_unique<TestPasswordManagerClient>();
-  auto autofill_client = std::make_unique<MockAutofillClient>();
+  auto autofill_client = std::make_unique<NiceMock<MockAutofillClient>>();
   InitializePasswordAutofillManager(client.get(), autofill_client.get());
 
   gfx::RectF element_bounds;
-  autofill::PasswordFormFillData data;
-  data.username_field.value = test_username_;
-  data.password_field.value = test_password_;
-  data.origin = GURL("https://foo.test");
+  autofill::PasswordFormFillData data = CreateTestFormFillData();
 
   password_autofill_manager_->OnAddPasswordFillData(data);
 
-  EXPECT_CALL(
-      *autofill_client,
+  EXPECT_CALL(*autofill_client,
               ShowAutofillPopup(element_bounds, _,
-                        SuggestionVectorValuesAre(testing::ElementsAreArray(
+                                SuggestionVectorValuesAre(ElementsAreArray(
                                     GetSuggestionList({test_username_}))),
-                        false, PopupType::kPasswords, _));
+                                /*autoselect_first_suggestion=*/false,
+                                PopupType::kPasswords, _));
   password_autofill_manager_->OnShowPasswordSuggestions(
       base::i18n::RIGHT_TO_LEFT, test_username_, 0, element_bounds);
 }
@@ -736,7 +765,7 @@ TEST_F(PasswordAutofillManagerTest, ShowAllPasswordsOptionOnNonPasswordField) {
 TEST_F(PasswordAutofillManagerTest,
        MaybeShowPasswordSuggestionsWithGenerationNoCredentials) {
   auto client = std::make_unique<TestPasswordManagerClient>();
-  auto autofill_client = std::make_unique<MockAutofillClient>();
+  auto autofill_client = std::make_unique<NiceMock<MockAutofillClient>>();
   password_autofill_manager_.reset(new PasswordAutofillManager(
       client->mock_driver(), autofill_client.get(), client.get()));
 
@@ -752,14 +781,11 @@ TEST_F(PasswordAutofillManagerTest,
        MaybeShowPasswordSuggestionsWithGenerationSomeCredentials) {
   base::HistogramTester histograms;
   auto client = std::make_unique<TestPasswordManagerClient>();
-  auto autofill_client = std::make_unique<MockAutofillClient>();
+  auto autofill_client = std::make_unique<NiceMock<MockAutofillClient>>();
   InitializePasswordAutofillManager(client.get(), autofill_client.get());
 
   gfx::RectF element_bounds;
-  autofill::PasswordFormFillData data;
-  data.username_field.value = test_username_;
-  data.password_field.value = test_password_;
-  data.origin = GURL("https://foo.test");
+  autofill::PasswordFormFillData data = CreateTestFormFillData();
 
   favicon::MockFaviconService favicon_service;
   EXPECT_CALL(*client, GetFaviconService()).WillOnce(Return(&favicon_service));
@@ -777,7 +803,7 @@ TEST_F(PasswordAutofillManagerTest,
                     GetSuggestionList({test_username_, generation_string}))),
                 SuggestionVectorIconsAre(
                     ElementsAreArray(GetIconsList({"globeIcon", "keyIcon"})))),
-          false, PopupType::kPasswords, _));
+          /*autoselect_first_suggestion=*/false, PopupType::kPasswords, _));
   EXPECT_TRUE(
       password_autofill_manager_->MaybeShowPasswordSuggestionsWithGeneration(
           element_bounds, base::i18n::RIGHT_TO_LEFT,
@@ -801,14 +827,11 @@ TEST_F(PasswordAutofillManagerTest,
 TEST_F(PasswordAutofillManagerTest,
        MaybeShowPasswordSuggestionsWithOmittedCredentials) {
   auto client = std::make_unique<TestPasswordManagerClient>();
-  auto autofill_client = std::make_unique<MockAutofillClient>();
+  auto autofill_client = std::make_unique<NiceMock<MockAutofillClient>>();
   InitializePasswordAutofillManager(client.get(), autofill_client.get());
 
   gfx::RectF element_bounds;
-  autofill::PasswordFormFillData data;
-  data.username_field.value = test_username_;
-  data.password_field.value = test_password_;
-  data.origin = GURL("https://foo.test");
+  autofill::PasswordFormFillData data = CreateTestFormFillData();
 
   favicon::MockFaviconService favicon_service;
   EXPECT_CALL(*client, GetFaviconService()).WillOnce(Return(&favicon_service));
@@ -826,7 +849,7 @@ TEST_F(PasswordAutofillManagerTest,
                     ElementsAreArray(GetSuggestionList({generation_string}))),
                 SuggestionVectorIconsAre(
                     ElementsAreArray(GetIconsList({"keyIcon"})))),
-          false, PopupType::kPasswords, _));
+          /*autoselect_first_suggestion=*/false, PopupType::kPasswords, _));
 
   EXPECT_TRUE(
       password_autofill_manager_->MaybeShowPasswordSuggestionsWithGeneration(
@@ -838,9 +861,8 @@ TEST_F(PasswordAutofillManagerTest, DisplayAccountSuggestionsIndicatorIcon) {
   base::test::ScopedFeatureList features;
   features.InitAndEnableFeature(features::kEnablePasswordsAccountStorage);
 
-  std::unique_ptr<TestPasswordManagerClient> client(
-      new TestPasswordManagerClient);
-  std::unique_ptr<MockAutofillClient> autofill_client(new MockAutofillClient);
+  auto client = std::make_unique<TestPasswordManagerClient>();
+  auto autofill_client = std::make_unique<NiceMock<MockAutofillClient>>();
   InitializePasswordAutofillManager(client.get(), autofill_client.get());
 
   gfx::RectF element_bounds;
@@ -852,7 +874,9 @@ TEST_F(PasswordAutofillManagerTest, DisplayAccountSuggestionsIndicatorIcon) {
   password_autofill_manager_->OnAddPasswordFillData(data);
 
   std::vector<autofill::Suggestion> suggestions;
-  EXPECT_CALL(*autofill_client, ShowAutofillPopup(element_bounds, _, _, false,
+  EXPECT_CALL(*autofill_client,
+              ShowAutofillPopup(element_bounds, _, _,
+                                /*autoselect_first_suggestion=*/false,
                                 PopupType::kPasswords, _))
       .WillOnce(testing::SaveArg<2>(&suggestions));
   password_autofill_manager_->OnShowPasswordSuggestions(