Clean up ManagePasswordsBubbleModel after removing Cocoa bubble.

The class contained some functions used only on Mac. With this CL the "Update password" bubble is gone.

Bug: 832676
Change-Id: Ifebaa0edcd445ee64605fd85fc6b2b24a2ba6646
Reviewed-on: https://chromium-review.googlesource.com/1065973
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561475}

chrome/browser/password_manager/password_manager_test_base.cc

@@ -358,7 +358,9 @@ void BubbleObserver::AcceptSavePrompt() const {
 void BubbleObserver::AcceptUpdatePrompt(
     const autofill::PasswordForm& form) const {
   ASSERT_TRUE(IsUpdatePromptAvailable());
-  passwords_ui_controller_->UpdatePassword(form);
+  passwords_ui_controller_->SavePassword(
+      passwords_ui_controller_->GetPendingPassword().username_value,
+      passwords_ui_controller_->GetPendingPassword().password_value);
   EXPECT_FALSE(IsUpdatePromptAvailable());
 }
 

chrome/browser/ui/passwords/manage_passwords_bubble_model.cc

@@ -380,20 +380,6 @@ void ManagePasswordsBubbleModel::OnSaveClicked() {
   }
 }
 
-void ManagePasswordsBubbleModel::OnUpdateClicked(
-    const autofill::PasswordForm& password_form) {
-  if (delegate_)
-    delegate_->UpdatePassword(password_form);
-}
-
-void ManagePasswordsBubbleModel::OnDoneClicked() {
-  interaction_keeper_->set_dismissal_reason(metrics_util::CLICKED_DONE);
-}
-
-void ManagePasswordsBubbleModel::OnOKClicked() {
-  interaction_keeper_->set_dismissal_reason(metrics_util::CLICKED_OK);
-}
-
 void ManagePasswordsBubbleModel::OnManageClicked() {
   interaction_keeper_->set_dismissal_reason(metrics_util::CLICKED_MANAGE);
   if (delegate_)
@@ -466,11 +452,6 @@ content::WebContents* ManagePasswordsBubbleModel::GetWebContents() const {
   return delegate_ ? delegate_->GetWebContents() : nullptr;
 }
 
-bool ManagePasswordsBubbleModel::ShouldShowMultipleAccountUpdateUI() const {
-  return state_ == password_manager::ui::PENDING_PASSWORD_UPDATE_STATE &&
-         local_credentials_.size() > 1;
-}
-
 bool ManagePasswordsBubbleModel::IsCurrentStateUpdate() const {
   DCHECK(state_ == password_manager::ui::PENDING_PASSWORD_UPDATE_STATE ||
          state_ == password_manager::ui::PENDING_PASSWORD_STATE);

chrome/browser/ui/passwords/manage_passwords_bubble_model.h

@@ -63,17 +63,6 @@ class ManagePasswordsBubbleModel {
   // Called by the view code when the save/update button is clicked by the user.
   void OnSaveClicked();
 
-  // Called by the view code when the update link is clicked by the user.
-  // TODO(vasilii): remove when the cocoa bubble is gone.
-  void OnUpdateClicked(const autofill::PasswordForm& password_form);
-
-  // Called by the view code when the "Done" button is clicked by the user.
-  // TODO(vasilii): remove when the cocoa bubble is gone.
-  void OnDoneClicked();
-
-  // TODO(vasilii): remove when the cocoa bubble is gone.
-  void OnOKClicked();
-
   // Called by the view code when the manage button is clicked by the user.
   void OnManageClicked();
 
@@ -143,11 +132,6 @@ class ManagePasswordsBubbleModel {
   Profile* GetProfile() const;
   content::WebContents* GetWebContents() const;
 
-  // Returns true iff the multiple account selection prompt for account update
-  // should be presented.
-  // TODO(vasilii): remove when the cocoa bubble is gone.
-  bool ShouldShowMultipleAccountUpdateUI() 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".

chrome/browser/ui/passwords/manage_passwords_bubble_model_unittest.cc

@@ -375,14 +375,6 @@ TEST_F(ManagePasswordsBubbleModelTest, ClickManage) {
   DestroyModelExpectReason(password_manager::metrics_util::CLICKED_MANAGE);
 }
 
-TEST_F(ManagePasswordsBubbleModelTest, ClickDone) {
-  PretendManagingPasswords();
-
-  model()->OnDoneClicked();
-  EXPECT_EQ(password_manager::ui::MANAGE_STATE, model()->state());
-  DestroyModelExpectReason(password_manager::metrics_util::CLICKED_DONE);
-}
-
 TEST_F(ManagePasswordsBubbleModelTest, PopupAutoSigninToast) {
   PretendAutoSigningIn();
 

chrome/browser/ui/passwords/manage_passwords_ui_controller.cc

@@ -428,24 +428,6 @@ void ManagePasswordsUIController::SavePassword(const base::string16& username,
   bubble_status_ = SHOWN_PENDING_ICON_UPDATE;
 }
 
-void ManagePasswordsUIController::UpdatePassword(
-    const autofill::PasswordForm& password_form) {
-  DCHECK_EQ(password_manager::ui::PENDING_PASSWORD_UPDATE_STATE, GetState());
-  UMA_HISTOGRAM_BOOLEAN("PasswordManager.PasswordUpdatedWithManualFallback",
-                        BubbleIsManualFallbackForSaving());
-  if (GetPasswordFormMetricsRecorder() && BubbleIsManualFallbackForSaving()) {
-    GetPasswordFormMetricsRecorder()->RecordDetailedUserAction(
-        password_manager::PasswordFormMetricsRecorder::DetailedUserAction::
-            kTriggeredManualFallbackForUpdating);
-  }
-
-  save_fallback_timer_.Stop();
-  UpdatePasswordInternal(password_form);
-  ClearPopUpFlagForBubble();
-  passwords_data_.TransitionToState(password_manager::ui::MANAGE_STATE);
-  UpdateBubbleAndIconVisibility();
-}
-
 void ManagePasswordsUIController::ChooseCredential(
     const autofill::PasswordForm& form,
     password_manager::CredentialType credential_type) {
@@ -533,13 +515,6 @@ void ManagePasswordsUIController::SavePasswordInternal() {
   form_manager->Save();
 }
 
-void ManagePasswordsUIController::UpdatePasswordInternal(
-    const autofill::PasswordForm& password_form) {
-  password_manager::PasswordFormManagerForUI* form_manager =
-      passwords_data_.form_manager();
-  form_manager->Update(password_form);
-}
-
 void ManagePasswordsUIController::NeverSavePasswordInternal() {
   password_manager::PasswordFormManagerForUI* form_manager =
       passwords_data_.form_manager();

chrome/browser/ui/passwords/manage_passwords_ui_controller.h

@@ -122,7 +122,6 @@ class ManagePasswordsUIController
   void OnPasswordsRevealed() override;
   void SavePassword(const base::string16& username,
                     const base::string16& password) override;
-  void UpdatePassword(const autofill::PasswordForm& password_form) override;
   void ChooseCredential(
       const autofill::PasswordForm& form,
       password_manager::CredentialType credential_type) override;
@@ -149,8 +148,6 @@ class ManagePasswordsUIController
   // The pieces of saving and blacklisting passwords that interact with
   // FormManager, split off into internal functions for testing/mocking.
   virtual void SavePasswordInternal();
-  virtual void UpdatePasswordInternal(
-      const autofill::PasswordForm& password_form);
   virtual void NeverSavePasswordInternal();
 
   // Called when a PasswordForm is autofilled, when a new PasswordForm is

chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc

@@ -111,8 +111,6 @@ class TestManagePasswordsUIController : public ManagePasswordsUIController {
  private:
   void UpdateBubbleAndIconVisibility() override;
   void SavePasswordInternal() override {}
-  void UpdatePasswordInternal(
-      const autofill::PasswordForm& password_form) override {}
   void NeverSavePasswordInternal() override;
   bool ShowAuthenticationDialog() override { return true; }
 
@@ -901,12 +899,14 @@ TEST_F(ManagePasswordsUIControllerTest, PasswordUpdated) {
   controller()->OnUpdatePasswordSubmitted(std::move(test_form_manager));
 
   ExpectIconStateIs(password_manager::ui::PENDING_PASSWORD_UPDATE_STATE);
-  EXPECT_CALL(*controller(), OnUpdateBubbleAndIconVisibility());
   base::HistogramTester histogram_tester;
-  controller()->UpdatePassword(autofill::PasswordForm());
+  controller()->SavePassword(test_local_form().username_value,
+                             test_local_form().password_value);
   ExpectIconStateIs(password_manager::ui::MANAGE_STATE);
   histogram_tester.ExpectUniqueSample(
-      "PasswordManager.PasswordUpdatedWithManualFallback", false, 1);
+      "PasswordManager.PasswordSavedWithManualFallback", false, 1);
+  EXPECT_CALL(*controller(), OnUpdateBubbleAndIconVisibility());
+  controller()->OnBubbleHidden();
 }
 
 TEST_F(ManagePasswordsUIControllerTest, SavePendingStatePasswordAutofilled) {
@@ -971,12 +971,8 @@ TEST_F(ManagePasswordsUIControllerTest, ManualFallbackForSaving_UseFallback) {
     EXPECT_FALSE(controller()->opened_automatic_bubble());
 
     // A user clicks on omnibox icon, opens the bubble and press Save/Update.
-    if (is_update) {
-      controller()->UpdatePassword(autofill::PasswordForm());
-    } else {
-      controller()->SavePassword(test_local_form().username_value,
-                                 test_local_form().password_value);
-    }
+    controller()->SavePassword(test_local_form().username_value,
+                               test_local_form().password_value);
 
     // Fake navigation so that the old form manager gets destroyed and
     // reports its metrics. Need to close the bubble, otherwise the bubble
@@ -998,19 +994,10 @@ TEST_F(ManagePasswordsUIControllerTest, ManualFallbackForSaving_UseFallback) {
     auto* entry = entries[0];
     EXPECT_EQ(source_id, entry->source_id);
 
-    if (is_update) {
-      histogram_tester.ExpectUniqueSample(
-          "PasswordManager.PasswordUpdatedWithManualFallback", true, 1);
-      test_ukm_recorder.ExpectEntryMetric(
-          entry, UkmEntry::kUser_Action_TriggeredManualFallbackForUpdatingName,
-          1u);
-    } else {
-      histogram_tester.ExpectUniqueSample(
-          "PasswordManager.PasswordSavedWithManualFallback", true, 1);
-      test_ukm_recorder.ExpectEntryMetric(
-          entry, UkmEntry::kUser_Action_TriggeredManualFallbackForSavingName,
-          1u);
-    }
+    histogram_tester.ExpectUniqueSample(
+        "PasswordManager.PasswordSavedWithManualFallback", true, 1);
+    test_ukm_recorder.ExpectEntryMetric(
+        entry, UkmEntry::kUser_Action_TriggeredManualFallbackForSavingName, 1u);
   }
 }
 

chrome/browser/ui/passwords/passwords_model_delegate.h

@@ -94,9 +94,6 @@ class PasswordsModelDelegate {
   virtual void SavePassword(const base::string16& username,
                             const base::string16& password) = 0;
 
-  // Called from the model when the user chooses to update a password.
-  virtual void UpdatePassword(const autofill::PasswordForm& password_form) = 0;
-
   // Called from the dialog controller when the user chooses a credential.
   // Controller can be destroyed inside the method.
   virtual void ChooseCredential(

components/password_manager/core/browser/password_form_metrics_recorder.cc

@@ -39,14 +39,14 @@ PasswordFormMetricsRecorder::BubbleDismissalReason GetBubbleDismissalReason(
 
     // Ignore these for metrics collection:
     case metrics_util::CLICKED_MANAGE:
-    case metrics_util::CLICKED_DONE:
-    case metrics_util::CLICKED_OK:
     case metrics_util::CLICKED_BRAND_NAME:
     case metrics_util::CLICKED_PASSWORDS_DASHBOARD:
     case metrics_util::AUTO_SIGNIN_TOAST_TIMEOUT:
       break;
 
     // These should not reach here:
+    case metrics_util::CLICKED_DONE_OBSOLETE:
+    case metrics_util::CLICKED_OK_OBSOLETE:
     case metrics_util::CLICKED_UNBLACKLIST_OBSOLETE:
     case metrics_util::CLICKED_CREDENTIAL_OBSOLETE:
     case metrics_util::AUTO_SIGNIN_TOAST_CLICKED_OBSOLETE:
@@ -118,14 +118,13 @@ PasswordFormMetricsRecorder::~PasswordFormMetricsRecorder() {
         ukm_entry_builder_.SetUser_Action_TriggeredManualFallbackForSaving(
             action.second);
         break;
-      case DetailedUserAction::kTriggeredManualFallbackForUpdating:
-        ukm_entry_builder_.SetUser_Action_TriggeredManualFallbackForUpdating(
-            action.second);
-        break;
       case DetailedUserAction::kCorrectedUsernameInForm:
         ukm_entry_builder_.SetUser_Action_CorrectedUsernameInForm(
             action.second);
         break;
+      case DetailedUserAction::kObsoleteTriggeredManualFallbackForUpdating:
+        NOTREACHED();
+        break;
     }
   }
 

components/password_manager/core/browser/password_form_metrics_recorder.h

@@ -151,7 +151,7 @@ class PasswordFormMetricsRecorder
     kEditedUsernameInBubble = 100,
     kSelectedDifferentPasswordInBubble = 101,
     kTriggeredManualFallbackForSaving = 102,
-    kTriggeredManualFallbackForUpdating = 103,
+    kObsoleteTriggeredManualFallbackForUpdating = 103,  // unused
 
     // Interactions with form.
     kCorrectedUsernameInForm = 200,

components/password_manager/core/browser/password_manager_metrics_util.h

@@ -50,10 +50,10 @@ enum UIDismissalReason {
   CLICKED_CANCEL,
   CLICKED_NEVER,
   CLICKED_MANAGE,
-  CLICKED_DONE,
+  CLICKED_DONE_OBSOLETE,         // obsolete
   CLICKED_UNBLACKLIST_OBSOLETE,  // obsolete.
-  CLICKED_OK,
-  CLICKED_CREDENTIAL_OBSOLETE,  // obsolete.
+  CLICKED_OK_OBSOLETE,           // obsolete
+  CLICKED_CREDENTIAL_OBSOLETE,   // obsolete.
   AUTO_SIGNIN_TOAST_TIMEOUT,
   AUTO_SIGNIN_TOAST_CLICKED_OBSOLETE,  // obsolete.
   CLICKED_BRAND_NAME,

tools/metrics/histograms/histograms.xml

@@ -65704,6 +65704,9 @@ uploading your change for review.
 
 <histogram name="PasswordManager.PasswordUpdatedWithManualFallback"
     enum="BooleanPasswordSavedWithFallback">
+  <obsolete>
+    Deprecated as of 05/2018.
+  </obsolete>
   <owner>kolos@chromium.org</owner>
   <summary>
     Measures whether users update passwords with automatic prompt or manual

tools/metrics/ukm/ukm.xml

@@ -2424,6 +2424,9 @@ be describing additional metrics about the same event.
     </summary>
   </metric>
   <metric name="User.Action.TriggeredManualFallbackForUpdating">
+    <obsolete>
+      Deprecated. Use TriggeredManualFallbackForSaving instead.
+    </obsolete>
     <summary>
       Counts how many times the user triggered the manual fallback for password
       updating. This is only populated if events happened.