[Password Manager] Introduce editable username, password selector and eye icon for update cases

This CL is Step 3 & Step 4 from the description of crbug.com/800318.

Bug: 800318
Change-Id: I50d8936f80d193044c159a7d0f12285aaecfdeb6
Reviewed-on: https://chromium-review.googlesource.com/927361
Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540518}

chrome/browser/ui/BUILD.gn

@@ -2847,8 +2847,6 @@ split_static_library("ui") {
       "views/passwords/auto_signin_first_run_dialog_view.h",
       "views/passwords/credentials_item_view.cc",
       "views/passwords/credentials_item_view.h",
-      "views/passwords/credentials_selection_view.cc",
-      "views/passwords/credentials_selection_view.h",
       "views/passwords/password_auto_sign_in_view.cc",
       "views/passwords/password_auto_sign_in_view.h",
       "views/passwords/password_bubble_view_base.cc",

chrome/browser/ui/passwords/manage_passwords_bubble_model.cc

@@ -536,6 +536,23 @@ bool ManagePasswordsBubbleModel::ShouldShowMultipleAccountUpdateUI() const {
          local_credentials_.size() > 1 && !password_overridden_;
 }
 
+base::string16 ManagePasswordsBubbleModel::GetInitialUsername() const {
+  const base::string16& captured_username = pending_password_.username_value;
+  if (!ShouldShowMultipleAccountUpdateUI())
+    return captured_username;
+  DCHECK_EQ(password_manager::ui::PENDING_PASSWORD_UPDATE_STATE, state_);
+  DCHECK_GT(local_credentials_.size(), 1u);
+  size_t preferred_form_index = 0;
+  for (size_t index = 0; index < local_credentials_.size(); ++index) {
+    if (local_credentials_.at(index).username_value == captured_username)
+      return captured_username;
+    if (local_credentials_.at(index).preferred)
+      preferred_form_index = index;
+  }
+
+  return local_credentials_.at(preferred_form_index).username_value;
+}
+
 bool ManagePasswordsBubbleModel::ReplaceToShowPromotionIfNeeded() {
   DCHECK_EQ(password_manager::ui::PENDING_PASSWORD_STATE, state_);
   PrefService* prefs = GetProfile()->GetPrefs();

chrome/browser/ui/passwords/manage_passwords_bubble_model.h

@@ -144,6 +144,9 @@ class ManagePasswordsBubbleModel {
   // should be presented.
   bool ShouldShowMultipleAccountUpdateUI() const;
 
+  // Returns the value for the username field when the bubble is opened.
+  base::string16 GetInitialUsername() const;
+
   // Returns true and updates the internal state iff the Save bubble should
   // switch to show a promotion after the password was saved. Otherwise,
   // returns false and leaves the current state.

chrome/browser/ui/passwords/manage_passwords_bubble_model_unittest.cc

@@ -156,7 +156,8 @@ class ManagePasswordsBubbleModelTest : public ::testing::Test {
   void SetUpWithState(password_manager::ui::State state,
                       ManagePasswordsBubbleModel::DisplayReason reason);
   void PretendPasswordWaiting();
-  void PretendUpdatePasswordWaiting();
+  void PretendUpdatePasswordWaiting(
+      const autofill::PasswordForm& pending_password);
   void PretendAutoSigningIn();
   void PretendManagingPasswords();
 
@@ -166,6 +167,7 @@ class ManagePasswordsBubbleModelTest : public ::testing::Test {
 
   static password_manager::InteractionsStats GetTestStats();
   static autofill::PasswordForm GetPendingPassword();
+  static std::vector<std::unique_ptr<autofill::PasswordForm>> GetCurrentForms();
 
  private:
   content::TestBrowserThreadBundle thread_bundle_;
@@ -201,10 +203,12 @@ void ManagePasswordsBubbleModelTest::PretendPasswordWaiting() {
                  ManagePasswordsBubbleModel::AUTOMATIC);
 }
 
-void ManagePasswordsBubbleModelTest::PretendUpdatePasswordWaiting() {
-  autofill::PasswordForm form = GetPendingPassword();
-  EXPECT_CALL(*controller(), GetPendingPassword()).WillOnce(ReturnRef(form));
-  std::vector<std::unique_ptr<autofill::PasswordForm>> forms;
+void ManagePasswordsBubbleModelTest::PretendUpdatePasswordWaiting(
+    const autofill::PasswordForm& pending_password) {
+  EXPECT_CALL(*controller(), GetPendingPassword())
+      .WillOnce(ReturnRef(pending_password));
+  std::vector<std::unique_ptr<autofill::PasswordForm>> forms =
+      GetCurrentForms();
   EXPECT_CALL(*controller(), GetCurrentForms()).WillOnce(ReturnRef(forms));
   EXPECT_CALL(*controller(), IsPasswordOverridden()).WillOnce(Return(false));
   SetUpWithState(password_manager::ui::PENDING_PASSWORD_UPDATE_STATE,
@@ -219,8 +223,8 @@ void ManagePasswordsBubbleModelTest::PretendAutoSigningIn() {
 }
 
 void ManagePasswordsBubbleModelTest::PretendManagingPasswords() {
-  std::vector<std::unique_ptr<autofill::PasswordForm>> forms(1);
-  forms[0].reset(new autofill::PasswordForm(GetPendingPassword()));
+  std::vector<std::unique_ptr<autofill::PasswordForm>> forms =
+      GetCurrentForms();
   EXPECT_CALL(*controller(), GetCurrentForms()).WillOnce(ReturnRef(forms));
   SetUpWithState(password_manager::ui::MANAGE_STATE,
                  ManagePasswordsBubbleModel::USER_ACTION);
@@ -263,6 +267,21 @@ autofill::PasswordForm ManagePasswordsBubbleModelTest::GetPendingPassword() {
   return form;
 }
 
+// static
+std::vector<std::unique_ptr<autofill::PasswordForm>>
+ManagePasswordsBubbleModelTest::GetCurrentForms() {
+  std::vector<std::unique_ptr<autofill::PasswordForm>> forms(3);
+  autofill::PasswordForm another_form(GetPendingPassword());
+  another_form.username_value = base::ASCIIToUTF16("another_username");
+  forms[0].reset(new autofill::PasswordForm(another_form));
+  forms[1].reset(new autofill::PasswordForm(GetPendingPassword()));
+  autofill::PasswordForm preferred_form(GetPendingPassword());
+  preferred_form.username_value = base::ASCIIToUTF16("preferred_username");
+  preferred_form.preferred = true;
+  forms[2].reset(new autofill::PasswordForm(preferred_form));
+  return forms;
+}
+
 TEST_F(ManagePasswordsBubbleModelTest, CloseWithoutInteraction) {
   PretendPasswordWaiting();
 
@@ -333,7 +352,7 @@ TEST_F(ManagePasswordsBubbleModelTest, PopupAutoSigninToast) {
 }
 
 TEST_F(ManagePasswordsBubbleModelTest, ClickUpdate) {
-  PretendUpdatePasswordWaiting();
+  PretendUpdatePasswordWaiting(GetPendingPassword());
 
   autofill::PasswordForm form;
   EXPECT_CALL(*controller(), UpdatePassword(form));
@@ -341,6 +360,20 @@ TEST_F(ManagePasswordsBubbleModelTest, ClickUpdate) {
   DestroyModelAndVerifyControllerExpectations();
 }
 
+TEST_F(ManagePasswordsBubbleModelTest, GetInitialUsername_MatchedUsername) {
+  PretendUpdatePasswordWaiting(GetPendingPassword());
+  EXPECT_EQ(base::UTF8ToUTF16(kUsername), model()->GetInitialUsername());
+}
+
+TEST_F(ManagePasswordsBubbleModelTest, GetInitialUsername_PreferredCredential) {
+  autofill::PasswordForm form = GetPendingPassword();
+  // There is no matched username, the preferred credential should be selected.
+  form.username_value = base::UTF8ToUTF16("captcha_code");
+  PretendUpdatePasswordWaiting(form);
+  EXPECT_EQ(base::UTF8ToUTF16("preferred_username"),
+            model()->GetInitialUsername());
+}
+
 TEST_F(ManagePasswordsBubbleModelTest, EditCredential) {
   PretendPasswordWaiting();
   EXPECT_CALL(*GetStore(), RemoveSiteStatsImpl(GURL(kSiteOrigin).GetOrigin()));
@@ -562,7 +595,7 @@ TEST_F(ManagePasswordsBubbleModelTest, RecordUKMs) {
                              : CredentialSourceType::kPasswordManager));
 
           if (update)
-            PretendUpdatePasswordWaiting();
+            PretendUpdatePasswordWaiting(GetPendingPassword());
           else
             PretendPasswordWaiting();
 

chrome/browser/ui/views/passwords/credentials_selection_view.cc

-// Copyright 2015 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chrome/browser/ui/views/passwords/credentials_selection_view.h"
-
-#include <stddef.h>
-
-#include "chrome/browser/ui/passwords/manage_passwords_bubble_model.h"
-#include "chrome/browser/ui/views/harmony/chrome_layout_provider.h"
-#include "chrome/browser/ui/views/harmony/chrome_typography.h"
-#include "components/password_manager/core/browser/password_manager_metrics_util.h"
-#include "ui/base/models/simple_combobox_model.h"
-#include "ui/base/resource/resource_bundle.h"
-#include "ui/views/controls/combobox/combobox.h"
-#include "ui/views/controls/label.h"
-#include "ui/views/layout/grid_layout.h"
-
-namespace {
-
-views::Label* GeneratePasswordLabel(const autofill::PasswordForm& form) {
-  views::Label* label =
-      new views::Label(form.password_value, CONTEXT_DEPRECATED_SMALL);
-  label->SetHorizontalAlignment(gfx::ALIGN_CENTER);
-  label->SetObscured(true);
-  return label;
-}
-
-}  // namespace
-
-CredentialsSelectionView::CredentialsSelectionView(
-    ManagePasswordsBubbleModel* manage_passwords_bubble_model)
-    : password_forms_(&manage_passwords_bubble_model->local_credentials()),
-      default_index_(0),
-      is_default_best_match_(false),
-      is_default_preferred_(false),
-      action_reported_(false) {
-  DCHECK(!password_forms_->empty());
-
-  // Layout.
-  views::GridLayout* layout =
-      SetLayoutManager(std::make_unique<views::GridLayout>(this));
-
-  // ColumnSet.
-  int column_set_id = 0;
-  views::ColumnSet* column_set = layout->AddColumnSet(column_set_id);
-  column_set->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL, 1,
-                        views::GridLayout::FIXED, 0, 0);
-  ChromeLayoutProvider* layout_provider = ChromeLayoutProvider::Get();
-  const int inner_padding = layout_provider->GetDistanceMetric(
-      views::DISTANCE_RELATED_LABEL_HORIZONTAL);
-  column_set->AddPaddingColumn(0, inner_padding);
-  column_set->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL, 1,
-                        views::GridLayout::FIXED, 0, 0);
-  column_set->AddPaddingColumn(0, inner_padding);
-
-  // The username combobox and password label.
-  layout->StartRow(0, column_set_id);
-  GenerateUsernameCombobox(
-      manage_passwords_bubble_model->pending_password().username_value);
-  layout->AddView(combobox_.get());
-  views::Label* label =
-      GeneratePasswordLabel(manage_passwords_bubble_model->pending_password());
-  layout->AddView(label);
-
-  GetLayoutManager()->Layout(this);
-}
-
-CredentialsSelectionView::~CredentialsSelectionView() {
-  ReportUserActionOnce(true, -1);
-  // |combobox_| has a pointer to |combobox_model_|, so |combobox_| should be
-  // deleted before deleting of |combobox_model_|. To ensure this, let's delete
-  // it now.
-  combobox_.reset();
-}
-
-const autofill::PasswordForm*
-CredentialsSelectionView::GetSelectedCredentials() {
-  DCHECK_EQ(password_forms_->size(),
-            static_cast<size_t>(combobox_->model()->GetItemCount()));
-  ReportUserActionOnce(false, combobox_->selected_index());
-  return &password_forms_->at(combobox_->selected_index());
-}
-
-void CredentialsSelectionView::GenerateUsernameCombobox(
-    const base::string16& best_matched_username) {
-  std::vector<base::string16> usernames;
-  size_t best_matched_username_index = password_forms_->size();
-  size_t preferred_form_index = password_forms_->size();
-  for (size_t index = 0; index < password_forms_->size(); ++index) {
-    usernames.push_back(password_forms_->at(index).username_value);
-    if (password_forms_->at(index).username_value == best_matched_username) {
-      best_matched_username_index = index;
-    }
-    if (password_forms_->at(index).preferred) {
-      preferred_form_index = index;
-    }
-  }
-
-  combobox_model_.reset(new ui::SimpleComboboxModel(usernames));
-  combobox_.reset(new views::Combobox(combobox_model_.get()));
-
-  if (best_matched_username_index < password_forms_->size()) {
-    is_default_best_match_ = true;
-    default_index_ = best_matched_username_index;
-    combobox_->SetSelectedIndex(best_matched_username_index);
-  } else if (preferred_form_index < password_forms_->size()) {
-    is_default_preferred_ = true;
-    default_index_ = preferred_form_index;
-    combobox_->SetSelectedIndex(preferred_form_index);
-  }
-}
-
-void CredentialsSelectionView::ReportUserActionOnce(bool was_update_rejected,
-                                                    int selected_index) {
-  if (action_reported_)
-    return;
-  password_manager::metrics_util::MultiAccountUpdateBubbleUserAction action;
-  if (was_update_rejected) {
-    if (is_default_best_match_) {
-      action = password_manager::metrics_util::
-          DEFAULT_ACCOUNT_MATCHED_BY_PASSWORD_USER_REJECTED_UPDATE;
-    } else if (is_default_preferred_) {
-      action = password_manager::metrics_util::
-          DEFAULT_ACCOUNT_PREFERRED_USER_REJECTED_UPDATE;
-    } else {
-      action = password_manager::metrics_util::
-          DEFAULT_ACCOUNT_FIRST_USER_REJECTED_UPDATE;
-    }
-  } else if (selected_index == default_index_) {
-    if (is_default_best_match_) {
-      action = password_manager::metrics_util::
-          DEFAULT_ACCOUNT_MATCHED_BY_PASSWORD_USER_NOT_CHANGED;
-    } else if (is_default_preferred_) {
-      action = password_manager::metrics_util::
-          DEFAULT_ACCOUNT_PREFERRED_USER_NOT_CHANGED;
-    } else {
-      action = password_manager::metrics_util::
-          DEFAULT_ACCOUNT_FIRST_USER_NOT_CHANGED;
-    }
-  } else {
-    if (is_default_best_match_) {
-      action = password_manager::metrics_util::
-          DEFAULT_ACCOUNT_MATCHED_BY_PASSWORD_USER_CHANGED;
-    } else if (is_default_preferred_) {
-      action = password_manager::metrics_util::
-          DEFAULT_ACCOUNT_PREFERRED_USER_CHANGED;
-    } else {
-      action =
-          password_manager::metrics_util::DEFAULT_ACCOUNT_FIRST_USER_CHANGED;
-    }
-  }
-
-  password_manager::metrics_util::LogMultiAccountUpdateBubbleUserAction(action);
-  action_reported_ = true;
-}

chrome/browser/ui/views/passwords/credentials_selection_view.h

-// Copyright 2015 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-#ifndef CHROME_BROWSER_UI_VIEWS_PASSWORDS_CREDENTIALS_SELECTION_VIEW_H_
-#define CHROME_BROWSER_UI_VIEWS_PASSWORDS_CREDENTIALS_SELECTION_VIEW_H_
-
-#include <memory>
-#include <vector>
-
-#include "base/macros.h"
-#include "components/autofill/core/common/password_form.h"
-#include "ui/views/view.h"
-
-namespace views {
-class Combobox;
-}
-
-namespace ui {
-class SimpleComboboxModel;
-}
-
-class ManagePasswordsBubbleModel;
-
-// A view where the user can select a credential.
-class CredentialsSelectionView : public views::View {
- public:
-  explicit CredentialsSelectionView(
-      ManagePasswordsBubbleModel* manage_passwords_bubble_model);
-  ~CredentialsSelectionView() override;
-
-  // This methods also reports a user action.
-  const autofill::PasswordForm* GetSelectedCredentials();
-
- private:
-  void GenerateUsernameCombobox(const base::string16& best_matched_username);
-  void ReportUserActionOnce(bool was_update_rejected, int selected_index);
-
-  const std::vector<autofill::PasswordForm>* password_forms_;
-  std::unique_ptr<views::Combobox> combobox_;
-  std::unique_ptr<ui::SimpleComboboxModel> combobox_model_;
-  int default_index_;
-  bool is_default_best_match_;
-  bool is_default_preferred_;
-  bool action_reported_;
-
-  DISALLOW_COPY_AND_ASSIGN(CredentialsSelectionView);
-};
-
-#endif  // CHROME_BROWSER_UI_VIEWS_PASSWORDS_CREDENTIALS_SELECTION_VIEW_H_

chrome/browser/ui/views/passwords/password_items_view.cc

@@ -107,9 +107,9 @@ std::unique_ptr<views::Label> CreateUsernameLabel(
 }
 
 std::unique_ptr<views::Textfield> CreateUsernameEditable(
-    const autofill::PasswordForm& form) {
+    const base::string16& initial_username) {
   auto editable = std::make_unique<views::Textfield>();
-  editable->SetText(form.username_value);
+  editable->SetText(initial_username);
   editable->SetAccessibleName(
       l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_USERNAME_LABEL));
   // In case of long username, ensure that the beginning of value is visible.

chrome/browser/ui/views/passwords/password_items_view.h

@@ -28,7 +28,7 @@ std::unique_ptr<views::Label> CreatePasswordLabel(
     int federation_message_id,
     bool is_password_visible);
 std::unique_ptr<views::Textfield> CreateUsernameEditable(
-    const autofill::PasswordForm& form);
+    const base::string16& initial_username);
 
 // A dialog for managing stored password and federated login information for a
 // specific site. A user can remove managed credentials for the site via this

chrome/browser/ui/views/passwords/password_pending_view.cc

@@ -12,7 +12,6 @@
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/browser/ui/views/harmony/chrome_layout_provider.h"
-#include "chrome/browser/ui/views/passwords/credentials_selection_view.h"
 #include "chrome/browser/ui/views/passwords/password_items_view.h"
 #include "chrome/browser/ui/views/passwords/password_sign_in_promo_view.h"
 #include "chrome/grit/generated_resources.h"
@@ -199,50 +198,31 @@ PasswordPendingView::PasswordPendingView(content::WebContents* web_contents,
       password_dropdown_(nullptr),
       password_label_(nullptr),
       are_passwords_revealed_(
-          model()->are_passwords_revealed_when_bubble_is_opened()),
-      selection_view_(nullptr) {
-  if (is_update_bubble_) {
-    // Credential row.
-    if (model()->ShouldShowMultipleAccountUpdateUI()) {
-      SetLayoutManager(std::make_unique<views::FillLayout>());
-      selection_view_ = new CredentialsSelectionView(model());
-      AddChildView(selection_view_);
-    } else {
-      const autofill::PasswordForm& password_form = model()->pending_password();
-      views::GridLayout* layout =
-          SetLayoutManager(std::make_unique<views::GridLayout>(this));
-      PasswordPendingView::BuildCredentialRows(
-          layout, CreateUsernameLabel(password_form).release(),
-          CreatePasswordLabel(password_form,
-                              IDS_PASSWORD_MANAGER_SIGNIN_VIA_FEDERATION, false)
-              .release(),
-          nullptr, /* password_view_button */
-          true /* show_password_label */);
-    }
+          model()->are_passwords_revealed_when_bubble_is_opened()) {
+  DCHECK(model()->state() == password_manager::ui::PENDING_PASSWORD_STATE ||
+         model()->state() ==
+             password_manager::ui::PENDING_PASSWORD_UPDATE_STATE);
+  const autofill::PasswordForm& password_form = model()->pending_password();
+  if (model()->enable_editing()) {
+    username_field_ =
+        CreateUsernameEditable(model()->GetInitialUsername()).release();
   } else {
-    DCHECK(model()->state() == password_manager::ui::PENDING_PASSWORD_STATE);
-    // Create credentials row.
-    const autofill::PasswordForm& password_form = model()->pending_password();
-    const bool is_password_credential =
-        password_form.federation_origin.unique();
-    if (model()->enable_editing()) {
-      username_field_ = CreateUsernameEditable(password_form).release();
-    } else {
-      username_field_ = CreateUsernameLabel(password_form).release();
-    }
-
-    CreatePasswordField();
-
-    if (is_password_credential) {
-      password_view_button_ =
-          CreatePasswordViewButton(this, are_passwords_revealed_).release();
-    }
-
-    CreateAndSetLayout(is_password_credential);
-    if (model()->enable_editing() &&
-        model()->pending_password().username_value.empty()) {
-      initially_focused_view_ = username_field_;
-    }
+    username_field_ = CreateUsernameLabel(password_form).release();
+  }
+
+  const bool is_password_credential = password_form.federation_origin.unique();
+
+  CreatePasswordField();
+
+  if (is_password_credential) {
+    password_view_button_ =
+        CreatePasswordViewButton(this, are_passwords_revealed_).release();
+  }
+
+  CreateAndSetLayout(is_password_credential);
+  if (model()->enable_editing() &&
+      model()->pending_password().username_value.empty()) {
+    initially_focused_view_ = username_field_;
   }
 }
 
@@ -297,12 +277,8 @@ PasswordPendingView::~PasswordPendingView() = default;
 
 bool PasswordPendingView::Accept() {
   if (is_update_bubble_) {
-    if (selection_view_) {
-      // Multi account case.
-      model()->OnUpdateClicked(*selection_view_->GetSelectedCredentials());
-    } else {
-      model()->OnUpdateClicked(model()->pending_password());
-    }
+    UpdateUsernameAndPasswordInModel();
+    model()->OnUpdateClicked(model()->pending_password());
     return true;
   }
   if (sign_in_promo_)

chrome/browser/ui/views/passwords/password_pending_view.h

@@ -18,7 +18,6 @@ class StyledLabel;
 class ToggleImageButton;
 }  // namespace views
 
-class CredentialsSelectionView;
 class DesktopIOSPromotionBubbleView;
 class PasswordSignInPromoView;
 
@@ -96,10 +95,6 @@ class PasswordPendingView : public PasswordBubbleViewBase,
 
   bool are_passwords_revealed_;
 
-  // The view for selecting a username from the list of existing usernames
-  // (applicable only to an update bubble).
-  CredentialsSelectionView* selection_view_;
-
   DISALLOW_COPY_AND_ASSIGN(PasswordPendingView);
 };
 

components/password_manager/core/browser/password_form_manager.cc

@@ -1157,6 +1157,8 @@ void PasswordFormManager::CreatePendingCredentials() {
   pending_credentials_.preferred = submitted_form_->preferred;
   pending_credentials_.form_has_autofilled_value =
       submitted_form_->form_has_autofilled_value;
+  pending_credentials_.all_possible_passwords =
+      submitted_form_->all_possible_passwords;
   CopyFieldPropertiesMasks(*submitted_form_, &pending_credentials_);
 
   // If we're dealing with an API-driven provisionally saved form, then take