[Passwords] Introduce PendingBubbleController

This CL is one of many refactoring CLs that would split the
ManagePasswordsBubbleModel into different controllers one per view.

Bug: 1044034
Change-Id: I80836c2ce3bb89efcd6765108a59922a5459368c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2022672
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737312}

chrome/browser/ui/BUILD.gn

@@ -1067,6 +1067,8 @@ jumbo_static_library("ui") {
       "passwords/bubble_controllers/items_bubble_controller.h",
       "passwords/bubble_controllers/password_bubble_controller_base.cc",
       "passwords/bubble_controllers/password_bubble_controller_base.h",
+      "passwords/bubble_controllers/pending_bubble_controller.cc",
+      "passwords/bubble_controllers/pending_bubble_controller.h",
       "passwords/bubble_controllers/sign_in_promo_bubble_controller.cc",
       "passwords/bubble_controllers/sign_in_promo_bubble_controller.h",
       "passwords/credential_leak_dialog_controller.h",

chrome/browser/ui/passwords/bubble_controllers/pending_bubble_controller.h

+// Copyright 2020 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_PASSWORDS_BUBBLE_CONTROLLERS_PENDING_BUBBLE_CONTROLLER_H_
+#define CHROME_BROWSER_UI_PASSWORDS_BUBBLE_CONTROLLERS_PENDING_BUBBLE_CONTROLLER_H_
+
+#include "base/memory/weak_ptr.h"
+#include "chrome/browser/ui/passwords/bubble_controllers/password_bubble_controller_base.h"
+#include "chrome/browser/ui/passwords/manage_passwords_bubble_model.h"
+#include "components/password_manager/core/browser/manage_passwords_referrer.h"
+
+class PasswordsModelDelegate;
+
+// This controller provides data and actions for the PasswordPendingView.
+class PendingBubbleController : public PasswordBubbleControllerBase {
+ public:
+  explicit PendingBubbleController(
+      base::WeakPtr<PasswordsModelDelegate> delegate,
+      ManagePasswordsBubbleModel::DisplayReason display_reason);
+  ~PendingBubbleController() override;
+
+  // Called by the view code when the save/update button is clicked by the user.
+  void OnSaveClicked();
+
+  // Called by the view code when the "Nope" button in clicked by the user in
+  // update bubble.
+  void OnNopeUpdateClicked();
+
+  // Called by the view code when the "Never for this site." button in clicked
+  // by the user.
+  void OnNeverForThisSiteClicked();
+
+  // Called by the view code when username or password is corrected using
+  // the username correction or password selection features in PendingView.
+  void OnCredentialEdited(base::string16 new_username,
+                          base::string16 new_password);
+
+  // The password bubble can switch its state between "save" and "update"
+  // depending on the user input. |state_| only captures the correct state on
+  // creation. This method returns true iff the current state is "update".
+  bool IsCurrentStateUpdate() const;
+
+  // Returns true iff the bubble is supposed to show the footer about syncing
+  // to Google account.
+  bool ShouldShowFooter() const;
+
+  // Returns the ID of the picture to show above the title.
+  int GetTopIllustration(bool dark_mode) 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.
+  bool ReplaceToShowPromotionIfNeeded();
+
+  // Returns true if passwords revealing is not locked or re-authentication is
+  // not available on the given platform. Otherwise, the method schedules
+  // re-authentication and bubble reopen (the current bubble will be destroyed),
+  // and returns false immediately. New bubble will reveal the passwords if the
+  // re-authentication is successful.
+  bool RevealPasswords();
+
+#if defined(PASSWORD_STORE_SELECT_ENABLED)
+  // Called by the view when the account store checkbox is toggled.
+  void OnToggleAccountStore(bool is_checked);
+
+  // Returns true iff the password account store is used.
+  bool IsUsingAccountStore();
+#endif  // defined(PASSWORD_STORE_SELECT_ENABLED)
+
+  password_manager::ui::State state() const { return state_; }
+
+  const autofill::PasswordForm& pending_password() const {
+    return pending_password_;
+  }
+
+  bool are_passwords_revealed_when_bubble_is_opened() const {
+    return are_passwords_revealed_when_bubble_is_opened_;
+  }
+
+  bool enable_editing() const { return enable_editing_; }
+
+#if defined(UNIT_TEST)
+  void set_clock(base::Clock* clock) { clock_ = clock; }
+
+  void allow_passwords_revealing() {
+    password_revealing_requires_reauth_ = false;
+  }
+
+  bool password_revealing_requires_reauth() const {
+    return password_revealing_requires_reauth_;
+  }
+#endif
+
+ private:
+  // PasswordBubbleControllerBase methods:
+  base::string16 GetTitle() const override;
+  void ReportInteractions() override;
+
+  // URL of the page from where this bubble was triggered.
+  GURL origin_;
+  password_manager::ui::State state_;
+  base::string16 title_;
+  autofill::PasswordForm pending_password_;
+  std::vector<autofill::PasswordForm> local_credentials_;
+  password_manager::InteractionsStats interaction_stats_;
+  password_manager::metrics_util::UIDisplayDisposition display_disposition_;
+
+  // True iff password revealing should require re-auth for privacy reasons.
+  bool password_revealing_requires_reauth_;
+
+  // True iff bubble should pop up with revealed password value.
+  bool are_passwords_revealed_when_bubble_is_opened_;
+
+  // True iff username/password editing should be enabled.
+  bool enable_editing_;
+
+  // Dismissal reason for a password bubble.
+  password_manager::metrics_util::UIDismissalReason dismissal_reason_;
+
+  // Used to retrieve the current time, in base::Time units.
+  base::Clock* clock_;
+};
+
+#endif  // CHROME_BROWSER_UI_PASSWORDS_BUBBLE_CONTROLLERS_PENDING_BUBBLE_CONTROLLER_H_

chrome/browser/ui/passwords/manage_passwords_bubble_model.h

@@ -46,27 +46,6 @@ class ManagePasswordsBubbleModel {
   // closed. Otherwise, it is called later on when the model is destroyed.
   void OnBubbleClosing();
 
-  // Called by the view code when the "Nope" button in clicked by the user in
-  // update bubble.
-  void OnNopeUpdateClicked();
-
-  // Called by the view code when the "Never for this site." button in clicked
-  // by the user.
-  void OnNeverForThisSiteClicked();
-
-  // Called by the view code when username or password is corrected using
-  // the username correction or password selection features in PendingView.
-  void OnCredentialEdited(base::string16 new_username,
-                          base::string16 new_password);
-
-  // Called by the view code when the save/update button is clicked by the user.
-  void OnSaveClicked();
-
-#if defined(PASSWORD_STORE_SELECT_ENABLED)
-  // Called by the view when the account store checkbox is toggled.
-  void OnToggleAccountStore(bool is_checked);
-#endif  // defined(PASSWORD_STORE_SELECT_ENABLED)
-
   password_manager::ui::State state() const { return state_; }
 
   const base::string16& title() const { return title_; }
@@ -78,56 +57,15 @@ class ManagePasswordsBubbleModel {
     return are_passwords_revealed_when_bubble_is_opened_;
   }
 
-#if defined(UNIT_TEST)
-  void allow_passwords_revealing() {
-    password_revealing_requires_reauth_ = false;
-  }
-
-  bool password_revealing_requires_reauth() const {
-    return password_revealing_requires_reauth_;
-  }
-#endif
-
   bool enable_editing() const { return enable_editing_; }
 
   Profile* GetProfile() const;
   content::WebContents* GetWebContents() const;
 
-  // The password bubble can switch its state between "save" and "update"
-  // depending on the user input. |state_| only captures the correct state on
-  // creation. This method returns true iff the current state is "update".
-  bool IsCurrentStateUpdate() const;
-
-  // Returns true iff the bubble is supposed to show the footer about syncing
-  // to Google account.
-  bool ShouldShowFooter() const;
-
-  // Returns the ID of the picture to show above the title.
-  int GetTopIllustration(bool dark_mode) 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.
-  bool ReplaceToShowPromotionIfNeeded();
-
   void SetClockForTesting(base::Clock* clock);
 
-  // Returns true if passwords revealing is not locked or re-authentication is
-  // not available on the given platform. Otherwise, the method schedules
-  // re-authentication and bubble reopen (the current bubble will be destroyed),
-  // and returns false immediately. New bubble will reveal the passwords if the
-  // re-authentication is successful.
-  bool RevealPasswords();
-
-#if defined(PASSWORD_STORE_SELECT_ENABLED)
-  // Returns true iff the password account store is used.
-  bool IsUsingAccountStore();
-#endif  // defined(PASSWORD_STORE_SELECT_ENABLED)
-
  private:
   class InteractionKeeper;
-  // Updates |title_| for the PENDING_PASSWORD_STATE.
-  void UpdatePendingStateTitle();
 
   // URL of the page from where this bubble was triggered.
   GURL origin_;
@@ -136,9 +74,6 @@ class ManagePasswordsBubbleModel {
   autofill::PasswordForm pending_password_;
   std::vector<autofill::PasswordForm> local_credentials_;
 
-  // Responsible for recording all the interactions required.
-  std::unique_ptr<InteractionKeeper> interaction_keeper_;
-
   // A bridge to ManagePasswordsUIController instance.
   base::WeakPtr<PasswordsModelDelegate> delegate_;
 
@@ -146,9 +81,6 @@ class ManagePasswordsBubbleModel {
   // the bubble is closing.
   bool interaction_reported_;
 
-  // True iff password revealing should require re-auth for privacy reasons.
-  bool password_revealing_requires_reauth_;
-
   // True iff bubble should pop up with revealed password value.
   bool are_passwords_revealed_when_bubble_is_opened_;
 

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

@@ -296,7 +296,7 @@ IN_PROC_BROWSER_TEST_F(PasswordBubbleInteractiveUiTest,
   // PasswordBubbleViewBase::PendingView:: ButtonPressed(), and
   // simulate the OS event queue by posting a task.
   auto press_button = [](PasswordBubbleViewBase* bubble, bool* ran) {
-    bubble->model()->OnNeverForThisSiteClicked();
+    bubble->Cancel();
     *ran = true;
   };
 

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

@@ -132,7 +132,10 @@ PasswordBubbleViewBase::PasswordBubbleViewBase(
   // bubble controllers.
   if (delegate->GetState() != password_manager::ui::AUTO_SIGNIN_STATE &&
       delegate->GetState() != password_manager::ui::CONFIRMATION_STATE &&
-      delegate->GetState() != password_manager::ui::MANAGE_STATE) {
+      delegate->GetState() != password_manager::ui::MANAGE_STATE &&
+      delegate->GetState() != password_manager::ui::PENDING_PASSWORD_STATE &&
+      delegate->GetState() !=
+          password_manager::ui::PENDING_PASSWORD_UPDATE_STATE) {
     model_ = std::make_unique<ManagePasswordsBubbleModel>(
         delegate, reason == AUTOMATIC
                       ? ManagePasswordsBubbleModel::AUTOMATIC

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

@@ -14,6 +14,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/passwords/manage_passwords_view_utils.h"
 #include "chrome/browser/ui/passwords/password_dialog_prompts.h"
+#include "chrome/browser/ui/passwords/passwords_model_delegate.h"
 #include "chrome/browser/ui/views/accessibility/non_accessible_image_view.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
 #include "chrome/browser/ui/views/chrome_typography.h"
@@ -253,18 +254,22 @@ PasswordPendingView::PasswordPendingView(content::WebContents* web_contents,
                              anchor_view,
                              reason,
                              /*auto_dismissable=*/false),
-      is_update_bubble_(model()->state() ==
+      controller_(PasswordsModelDelegateFromWebContents(web_contents),
+                  reason == AUTOMATIC
+                      ? ManagePasswordsBubbleModel::AUTOMATIC
+                      : ManagePasswordsBubbleModel::USER_ACTION),
+      is_update_bubble_(controller_.state() ==
                         password_manager::ui::PENDING_PASSWORD_UPDATE_STATE),
       sign_in_promo_(nullptr),
       username_dropdown_(nullptr),
       password_view_button_(nullptr),
       password_dropdown_(nullptr),
       are_passwords_revealed_(
-          model()->are_passwords_revealed_when_bubble_is_opened()) {
-  DCHECK(model()->state() == password_manager::ui::PENDING_PASSWORD_STATE ||
-         model()->state() ==
+          controller_.are_passwords_revealed_when_bubble_is_opened()) {
+  DCHECK(controller_.state() == password_manager::ui::PENDING_PASSWORD_STATE ||
+         controller_.state() ==
              password_manager::ui::PENDING_PASSWORD_UPDATE_STATE);
-  const autofill::PasswordForm& password_form = model()->pending_password();
+  const autofill::PasswordForm& password_form = controller_.pending_password();
   if (password_form.IsFederatedCredential()) {
     // The credential to be saved doesn't contain password but just the identity
     // provider (e.g. "Sign in with Google"). Thus, the layout is different.
@@ -274,7 +279,7 @@ PasswordPendingView::PasswordPendingView(content::WebContents* web_contents,
     CredentialsItemView* credential_view = new CredentialsItemView(
         this, titles.first, titles.second, &password_form,
         content::BrowserContext::GetDefaultStoragePartition(
-            model()->GetProfile())
+            controller_.GetProfile())
             ->GetURLLoaderFactoryForBrowserProcess()
             .get());
     credential_view->SetEnabled(false);
@@ -301,7 +306,7 @@ PasswordPendingView::PasswordPendingView(content::WebContents* web_contents,
                         std::move(password_view_button));
 #if defined(PASSWORD_STORE_SELECT_ENABLED)
     account_store_checkbox_ = MaybeAppendAccountCheckboxRow(
-        layout, model()->IsUsingAccountStore(),
+        layout, controller_.IsUsingAccountStore(),
         password_dropdown_->GetPreferredSize().height(), this);
 #endif  // defined(PASSWORD_STORE_SELECT_ENABLED)
   }
@@ -317,17 +322,17 @@ views::View* PasswordPendingView::GetUsernameTextfieldForTest() const {
 PasswordPendingView::~PasswordPendingView() = default;
 
 PasswordBubbleControllerBase* PasswordPendingView::GetController() {
-  return nullptr;
+  return &controller_;
 }
 
 const PasswordBubbleControllerBase* PasswordPendingView::GetController() const {
-  return nullptr;
+  return &controller_;
 }
 
 bool PasswordPendingView::Accept() {
   UpdateUsernameAndPasswordInModel();
-  model()->OnSaveClicked();
-  if (model()->ReplaceToShowPromotionIfNeeded()) {
+  controller_.OnSaveClicked();
+  if (controller_.ReplaceToShowPromotionIfNeeded()) {
     ReplaceWithPromo();
     return false;  // Keep open.
   }
@@ -337,10 +342,10 @@ bool PasswordPendingView::Accept() {
 bool PasswordPendingView::Cancel() {
   UpdateUsernameAndPasswordInModel();
   if (is_update_bubble_) {
-    model()->OnNopeUpdateClicked();
+    controller_.OnNopeUpdateClicked();
     return true;
   }
-  model()->OnNeverForThisSiteClicked();
+  controller_.OnNeverForThisSiteClicked();
   return true;
 }
 
@@ -353,7 +358,7 @@ void PasswordPendingView::ButtonPressed(views::Button* sender,
 #if defined(PASSWORD_STORE_SELECT_ENABLED)
   DCHECK(sender);
   if (sender == account_store_checkbox_) {
-    model()->OnToggleAccountStore(account_store_checkbox_->GetChecked());
+    controller_.OnToggleAccountStore(account_store_checkbox_->GetChecked());
     return;
   }
 #endif  // defined(PASSWORD_STORE_SELECT_ENABLED)
@@ -363,12 +368,12 @@ void PasswordPendingView::ButtonPressed(views::Button* sender,
 
 void PasswordPendingView::OnContentChanged(
     views::EditableCombobox* editable_combobox) {
-  bool is_update_state_before = model()->IsCurrentStateUpdate();
+  bool is_update_state_before = controller_.IsCurrentStateUpdate();
   bool is_ok_button_enabled_before =
       IsDialogButtonEnabled(ui::DIALOG_BUTTON_OK);
   UpdateUsernameAndPasswordInModel();
   // Maybe the buttons should be updated.
-  if (is_update_state_before != model()->IsCurrentStateUpdate() ||
+  if (is_update_state_before != controller_.IsCurrentStateUpdate() ||
       is_ok_button_enabled_before !=
           IsDialogButtonEnabled(ui::DIALOG_BUTTON_OK)) {
     UpdateDialogButtons();
@@ -400,8 +405,8 @@ views::View* PasswordPendingView::GetInitiallyFocusedView() {
 
 bool PasswordPendingView::IsDialogButtonEnabled(ui::DialogButton button) const {
   return button != ui::DIALOG_BUTTON_OK ||
-         model()->pending_password().IsFederatedCredential() ||
-         !model()->pending_password().password_value.empty();
+         controller_.pending_password().IsFederatedCredential() ||
+         !controller_.pending_password().password_value.empty();
 }
 
 gfx::ImageSkia PasswordPendingView::GetWindowIcon() {
@@ -422,14 +427,14 @@ void PasswordPendingView::AddedToWidget() {
 }
 
 void PasswordPendingView::OnThemeChanged() {
-  if (int id = model()->GetTopIllustration(
+  if (int id = controller_.GetTopIllustration(
           color_utils::IsDark(GetBubbleFrameView()->GetBackgroundColor()))) {
     GetBubbleFrameView()->SetHeaderView(CreateHeaderImage(id));
   }
 }
 
 void PasswordPendingView::TogglePasswordVisibility() {
-  if (!are_passwords_revealed_ && !model()->RevealPasswords())
+  if (!are_passwords_revealed_ && !controller_.RevealPasswords())
     return;
 
   are_passwords_revealed_ = !are_passwords_revealed_;
@@ -441,15 +446,16 @@ void PasswordPendingView::TogglePasswordVisibility() {
 void PasswordPendingView::UpdateUsernameAndPasswordInModel() {
   if (!username_dropdown_ && !password_dropdown_)
     return;
-  base::string16 new_username = model()->pending_password().username_value;
-  base::string16 new_password = model()->pending_password().password_value;
+  base::string16 new_username = controller_.pending_password().username_value;
+  base::string16 new_password = controller_.pending_password().password_value;
   if (username_dropdown_) {
     new_username = username_dropdown_->GetText();
     base::TrimString(new_username, base::ASCIIToUTF16(" "), &new_username);
   }
   if (password_dropdown_)
     new_password = password_dropdown_->GetText();
-  model()->OnCredentialEdited(std::move(new_username), std::move(new_password));
+  controller_.OnCredentialEdited(std::move(new_username),
+                                 std::move(new_password));
 }
 
 void PasswordPendingView::ReplaceWithPromo() {
@@ -466,8 +472,8 @@ void PasswordPendingView::ReplaceWithPromo() {
   SetLayoutManager(std::make_unique<views::FillLayout>());
   set_margins(ChromeLayoutProvider::Get()->GetDialogInsetsForContentType(
       views::TEXT, views::TEXT));
-  if (model()->state() == password_manager::ui::CHROME_SIGN_IN_PROMO_STATE) {
-    sign_in_promo_ = new PasswordSignInPromoView(model()->GetWebContents());
+  if (controller_.state() == password_manager::ui::CHROME_SIGN_IN_PROMO_STATE) {
+    sign_in_promo_ = new PasswordSignInPromoView(controller_.GetWebContents());
     AddChildView(sign_in_promo_);
   } else {
     NOTREACHED();
@@ -490,7 +496,7 @@ void PasswordPendingView::UpdateDialogButtons() {
       (ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL));
   DialogDelegate::set_button_label(
       ui::DIALOG_BUTTON_OK,
-      l10n_util::GetStringUTF16(model()->IsCurrentStateUpdate()
+      l10n_util::GetStringUTF16(controller_.IsCurrentStateUpdate()
                                     ? IDS_PASSWORD_MANAGER_UPDATE_BUTTON
                                     : IDS_PASSWORD_MANAGER_SAVE_BUTTON));
   DialogDelegate::set_button_label(
@@ -501,7 +507,7 @@ void PasswordPendingView::UpdateDialogButtons() {
 }
 
 std::unique_ptr<views::View> PasswordPendingView::CreateFooterView() {
-  if (!model()->ShouldShowFooter())
+  if (!controller_.ShouldShowFooter())
     return nullptr;
   auto label = std::make_unique<views::Label>(
       l10n_util::GetStringUTF16(IDS_SAVE_PASSWORD_FOOTER),

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

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_PASSWORDS_PASSWORD_PENDING_VIEW_H_
 #define CHROME_BROWSER_UI_VIEWS_PASSWORDS_PASSWORD_PENDING_VIEW_H_
 
+#include "chrome/browser/ui/passwords/bubble_controllers/pending_bubble_controller.h"
 #include "chrome/browser/ui/views/passwords/password_bubble_view_base.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/editable_combobox/editable_combobox_listener.h"
@@ -69,6 +70,8 @@ class PasswordPendingView : public PasswordBubbleViewBase,
   void UpdateDialogButtons();
   std::unique_ptr<views::View> CreateFooterView();
 
+  PendingBubbleController controller_;
+
   // True iff it is an update password bubble on creation. False iff it is a
   // save bubble.
   const bool is_update_bubble_;

chrome/test/BUILD.gn

@@ -4058,10 +4058,10 @@ test("unit_tests") {
       "../browser/ui/passwords/bubble_controllers/auto_sign_in_bubble_controller_unittest.cc",
       "../browser/ui/passwords/bubble_controllers/generation_confirmation_bubble_controller_unittest.cc",
       "../browser/ui/passwords/bubble_controllers/items_bubble_controller_unittest.cc",
+      "../browser/ui/passwords/bubble_controllers/pending_bubble_controller_unittest.cc",
       "../browser/ui/passwords/bubble_controllers/sign_in_promo_bubble_controller_unittest.cc",
       "../browser/ui/passwords/credential_leak_dialog_controller_impl_unittest.cc",
       "../browser/ui/passwords/credential_manager_dialog_controller_impl_unittest.cc",
-      "../browser/ui/passwords/manage_passwords_bubble_model_unittest.cc",
       "../browser/ui/recently_audible_helper_unittest.cc",
       "../browser/ui/search/ntp_user_data_logger_unittest.cc",
       "../browser/ui/search/search_ipc_router_policy_unittest.cc",