[Password Generation] Send a boolean flag of whether user changed generated password

components/password_manager/core/browser/password_manager.cc

Index: components/password_manager/core/browser/password_manager.cc
diff --git a/components/password_manager/core/browser/password_manager.cc b/components/password_manager/core/browser/password_manager.cc
index 804aa5d4348ce113c20c7fc59ab3fa4754090ce6..5a27fd005f5d519451a8a82ac1cf6abb121fe0e2 100644
--- a/components/password_manager/core/browser/password_manager.cc
+++ b/components/password_manager/core/browser/password_manager.cc
@@ -197,27 +197,23 @@ void PasswordManager::OnPresaveGeneratedPassword(
   DCHECK(client_->IsSavingAndFillingEnabledForCurrentPage());
   PasswordFormManager* form_manager = GetMatchingPendingManager(form);
   if (form_manager) {
-    form_manager->form_saver()->PresaveGeneratedPassword(form);
+    form_manager->PresaveGeneratedPassword(form);
+    UMA_HISTOGRAM_BOOLEAN("PasswordManager.GeneratedFormHasNoFormManager",

[vabr (Chromium), 2017/06/16 12:32:29]

Could you test setting the histograms in the unittest? (Here and line 206
below.)

[kolos1, 2017/06/21 13:46:12]

Let me do it in a separate CL. I believe it is not good place for this metric
anymore.

[kolos1, 2017/06/21 13:48:13]

ooops, I actually added tests for that in password_manager_unittest.cc

+                          false);
     return;
   }
+
+  UMA_HISTOGRAM_BOOLEAN("PasswordManager.GeneratedFormHasNoFormManager", true);
 }
 
-void PasswordManager::SetHasGeneratedPasswordForForm(
-    password_manager::PasswordManagerDriver* driver,
-    const PasswordForm& form,
-    bool password_is_generated) {
+void PasswordManager::OnPasswordNoLongerGenerated(const PasswordForm& form) {
   DCHECK(client_->IsSavingAndFillingEnabledForCurrentPage());
 
   PasswordFormManager* form_manager = GetMatchingPendingManager(form);
   if (form_manager) {
-    if (!password_is_generated)
-      form_manager->form_saver()->RemovePresavedPassword();
-    form_manager->set_has_generated_password(password_is_generated);
+    form_manager->PasswordNoLongerGenerated();

[vabr (Chromium), 2017/06/16 12:32:29]

Is there a way to test that this forwarding to PasswordFormManager, and the one
on line 200, works?

[kolos1, 2017/06/21 13:46:12]

I believe this forwarding is already tested indirectionally. Generation stuff
wouldn't work w/o that.  

[vabr (Chromium), 2017/06/21 14:18:33]

"wouldn't work" assumes that our specification of what means "work" in the
current tests is complete, which is hard to prove.
However, there is unfortunately still not a good way to mock a PFM, so no easy
way to unittest this (I hope to fix it with the refactoring manual fallbacks for
saving). So I'm fine with not unit-testing it here.

     return;
   }
-
-  UMA_HISTOGRAM_BOOLEAN("PasswordManager.GeneratedFormHasNoFormManager",
-                        password_is_generated);
 }
 
 void PasswordManager::SetGenerationElementAndReasonForForm(