[Password Generation] Trigger confirmation bubble when a password is saved

chrome/browser/ui/passwords/manage_passwords_bubble_model.cc

Index: chrome/browser/ui/passwords/manage_passwords_bubble_model.cc
diff --git a/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc b/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc
index e18bcc805e3dc0e19026043f8830b6f7460bc999..d5a106b508f2fb8efb2f403c887193b2b0f82adb 100644
--- a/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc
+++ b/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc
@@ -15,13 +15,14 @@
 
 using autofill::PasswordFormMap;
 using content::WebContents;
+namespace metrics_util = password_manager::metrics_util;

[Mike West, 2014/07/16 07:55:03]

Nit: Next time I'd suggest doing this kind of cleanup in a separate CL; it makes
the diff more difficult to read than it ought to be. :)

[Garrett Casto, 2014/07/17 06:42:45]

Yeah, sorry. I did this because I had a line that was over chars unless I
wrapped in mid namespace, and I thought that this was a better solution.

 
 ManagePasswordsBubbleModel::ManagePasswordsBubbleModel(
     content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
       display_disposition_(
-          password_manager::metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING),
-      dismissal_reason_(password_manager::metrics_util::NOT_DISPLAYED) {
+          metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING),
+      dismissal_reason_(metrics_util::NOT_DISPLAYED) {
   ManagePasswordsUIController* controller =
       ManagePasswordsUIController::FromWebContents(web_contents);
 
@@ -50,41 +51,42 @@ void ManagePasswordsBubbleModel::OnBubbleShown(
     ManagePasswordsBubble::DisplayReason reason) {
   if (reason == ManagePasswordsBubble::USER_ACTION) {
     if (password_manager::ui::IsPendingState(state_)) {
-      display_disposition_ =
-          password_manager::metrics_util::MANUAL_WITH_PASSWORD_PENDING;
+      display_disposition_ = metrics_util::MANUAL_WITH_PASSWORD_PENDING;
     } else if (state_ == password_manager::ui::BLACKLIST_STATE) {
-      display_disposition_ = password_manager::metrics_util::MANUAL_BLACKLISTED;
+      display_disposition_ = metrics_util::MANUAL_BLACKLISTED;
     } else {
-      display_disposition_ =
-          password_manager::metrics_util::MANUAL_MANAGE_PASSWORDS;
+      display_disposition_ = metrics_util::MANUAL_MANAGE_PASSWORDS;
     }
   } else {
-    DCHECK(password_manager::ui::IsPendingState(state_));

[Mike West, 2014/07/16 07:55:03]

Did you intentionally drop the DCHECK?

[Garrett Casto, 2014/07/17 06:42:45]

Yes, you can end up with CONFIRMATION_STATE here, which isn't a pending state.

-    display_disposition_ =
-        password_manager::metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING;
+    if (state_ == password_manager::ui::CONFIRMATION_STATE) {
+      display_disposition_ =
+          metrics_util::AUTOMATIC_GENERATED_PASSWORD_CONFIRMATION;
+    } else {
+      display_disposition_ = metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING;
+    }
   }
-  password_manager::metrics_util::LogUIDisplayDisposition(display_disposition_);
+  metrics_util::LogUIDisplayDisposition(display_disposition_);
 
   // Default to a dismissal reason of "no interaction". If the user interacts
   // with the button in such a way that it closes, we'll reset this value
   // accordingly.
-  dismissal_reason_ = password_manager::metrics_util::NO_DIRECT_INTERACTION;
+  dismissal_reason_ = metrics_util::NO_DIRECT_INTERACTION;
 }
 
 void ManagePasswordsBubbleModel::OnBubbleHidden() {
-  if (dismissal_reason_ == password_manager::metrics_util::NOT_DISPLAYED)
+  if (dismissal_reason_ == metrics_util::NOT_DISPLAYED)
     return;
 
-  password_manager::metrics_util::LogUIDismissalReason(dismissal_reason_);
+  metrics_util::LogUIDismissalReason(dismissal_reason_);
 }
 
 void ManagePasswordsBubbleModel::OnNopeClicked() {
-  dismissal_reason_ = password_manager::metrics_util::CLICKED_NOPE;
+  dismissal_reason_ = metrics_util::CLICKED_NOPE;
   state_ = password_manager::ui::PENDING_PASSWORD_STATE;
 }
 
 void ManagePasswordsBubbleModel::OnNeverForThisSiteClicked() {
-  dismissal_reason_ = password_manager::metrics_util::CLICKED_NEVER;
+  dismissal_reason_ = metrics_util::CLICKED_NEVER;
   ManagePasswordsUIController* manage_passwords_ui_controller =
       ManagePasswordsUIController::FromWebContents(web_contents());
   manage_passwords_ui_controller->NeverSavePassword();
@@ -92,7 +94,7 @@ void ManagePasswordsBubbleModel::OnNeverForThisSiteClicked() {
 }
 
 void ManagePasswordsBubbleModel::OnUnblacklistClicked() {
-  dismissal_reason_ = password_manager::metrics_util::CLICKED_UNBLACKLIST;
+  dismissal_reason_ = metrics_util::CLICKED_UNBLACKLIST;
   ManagePasswordsUIController* manage_passwords_ui_controller =
       ManagePasswordsUIController::FromWebContents(web_contents());
   manage_passwords_ui_controller->UnblacklistSite();
@@ -100,7 +102,7 @@ void ManagePasswordsBubbleModel::OnUnblacklistClicked() {
 }
 
 void ManagePasswordsBubbleModel::OnSaveClicked() {
-  dismissal_reason_ = password_manager::metrics_util::CLICKED_SAVE;
+  dismissal_reason_ = metrics_util::CLICKED_SAVE;
   ManagePasswordsUIController* manage_passwords_ui_controller =
       ManagePasswordsUIController::FromWebContents(web_contents());
   manage_passwords_ui_controller->SavePassword();
@@ -108,11 +110,11 @@ void ManagePasswordsBubbleModel::OnSaveClicked() {
 }
 
 void ManagePasswordsBubbleModel::OnDoneClicked() {
-  dismissal_reason_ = password_manager::metrics_util::CLICKED_DONE;
+  dismissal_reason_ = metrics_util::CLICKED_DONE;
 }
 
 void ManagePasswordsBubbleModel::OnManageLinkClicked() {
-  dismissal_reason_ = password_manager::metrics_util::CLICKED_MANAGE;
+  dismissal_reason_ = metrics_util::CLICKED_MANAGE;
   ManagePasswordsUIController::FromWebContents(web_contents())
       ->NavigateToPasswordManagerSettingsPage();
 }