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

The flag helps to find the sites where passwords generated by Chrome doesn't meet site's requirements

BUG=699529

Review-Url: https://codereview.chromium.org/2937033002
Cr-Commit-Position: refs/heads/master@{#481210}

chrome/browser/ui/autofill/password_generation_popup_controller_impl.cc

@@ -62,9 +62,9 @@ PasswordGenerationPopupControllerImpl::GetOrCreate(
     previous->Hide();
 
   PasswordGenerationPopupControllerImpl* controller =
-      new PasswordGenerationPopupControllerImpl(
-          bounds, form, max_length, password_manager, driver, observer,
-          web_contents, container_view);
+      new PasswordGenerationPopupControllerImpl(bounds, form, max_length,
+                                                driver, observer, web_contents,
+                                                container_view);
   return controller->GetWeakPtr();
 }
 
@@ -72,14 +72,12 @@ PasswordGenerationPopupControllerImpl::PasswordGenerationPopupControllerImpl(
     const gfx::RectF& bounds,
     const PasswordForm& form,
     int max_length,
-    password_manager::PasswordManager* password_manager,
     password_manager::PasswordManagerDriver* driver,
     PasswordGenerationPopupObserver* observer,
     content::WebContents* web_contents,
     gfx::NativeView container_view)
     : view_(NULL),
       form_(form),
-      password_manager_(password_manager),
       driver_(driver),
       observer_(observer),
       generator_(new PasswordGenerator(max_length)),
@@ -148,7 +146,6 @@ void PasswordGenerationPopupControllerImpl::PasswordAccepted() {
     return;
 
   driver_->GeneratedPasswordAccepted(current_password_);
-  password_manager_->SetHasGeneratedPasswordForForm(driver_, form_, true);
   Hide();
 }
 

chrome/browser/ui/autofill/password_generation_popup_controller_impl.h

@@ -76,7 +76,6 @@ class PasswordGenerationPopupControllerImpl
       const gfx::RectF& bounds,
       const PasswordForm& form,
       int max_length,
-      password_manager::PasswordManager* password_manager,
       password_manager::PasswordManagerDriver* driver,
       PasswordGenerationPopupObserver* observer,
       content::WebContents* web_contents,
@@ -127,7 +126,6 @@ class PasswordGenerationPopupControllerImpl
   void CalculateBounds();
 
   PasswordForm form_;
-  password_manager::PasswordManager* password_manager_;
   password_manager::PasswordManagerDriver* driver_;
 
   // May be NULL.

chrome/browser/ui/autofill/password_generation_popup_view_browsertest.cc

@@ -28,7 +28,6 @@ class TestPasswordGenerationPopupController :
             gfx::RectF(0, 0, 10, 10),
             PasswordForm(),
             10,
-            nullptr /* PasswordManager*/,
             password_manager::ContentPasswordManagerDriverFactory::
                 FromWebContents(web_contents)
                     ->GetDriverForFrame(web_contents->GetMainFrame()),

components/autofill/content/renderer/password_generation_agent.cc

@@ -334,6 +334,7 @@ void PasswordGenerationAgent::FormNotBlacklisted(const PasswordForm& form) {
 void PasswordGenerationAgent::GeneratedPasswordAccepted(
     const base::string16& password) {
   password_is_generated_ = true;
+  password_edited_ = false;
   password_generation::LogPasswordGenerationEvent(
       password_generation::PASSWORD_ACCEPTED);
   LogMessage(Logger::STRING_GENERATION_RENDERER_GENERATED_PASSWORD_ACCEPTED);
@@ -565,6 +566,7 @@ void PasswordGenerationAgent::HidePopup() {
 void PasswordGenerationAgent::PasswordNoLongerGenerated() {
   // Do not treat the password as generated, either here or in the browser.
   password_is_generated_ = false;
+  password_edited_ = false;
   generation_element_.SetShouldRevealPassword(false);
   for (blink::WebInputElement& password :
        generation_form_data_->password_elements)

components/autofill/core/browser/autofill_field.h

@@ -96,6 +96,13 @@ class AutofillField : public FormFieldData {
     return generation_type_;
   }
 
+  void set_generated_password_changed(bool generated_password_changed) {
+    generated_password_changed_ = generated_password_changed;
+  }
+  bool generated_password_changed() const {
+    return generated_password_changed_;
+  }
+
   void set_form_classifier_outcome(
       AutofillUploadContents::Field::FormClassifierOutcome outcome) {
     form_classifier_outcome_ = outcome;
@@ -176,6 +183,9 @@ class AutofillField : public FormFieldData {
   // The type of password generation event, if it happened.
   AutofillUploadContents::Field::PasswordGenerationType generation_type_;
 
+  // Whether the generated password was changed by user.
+  bool generated_password_changed_;
+
   // The outcome of HTML parsing based form classifier.
   AutofillUploadContents::Field::FormClassifierOutcome form_classifier_outcome_;
 

components/autofill/core/browser/form_structure.cc

@@ -1098,8 +1098,11 @@ void FormStructure::EncodeFormForUpload(AutofillUploadContents* upload) const {
 
       AutofillUploadContents::Field* added_field = upload->add_field();
       added_field->set_autofill_type(field_type);
-      if (field->generation_type())
+      if (field->generation_type()) {
         added_field->set_generation_type(field->generation_type());
+        added_field->set_generated_password_changed(
+            field->generated_password_changed());
+      }
 
       if (field->form_classifier_outcome()) {
         added_field->set_form_classifier_outcome(

components/autofill/core/browser/form_structure_unittest.cc

@@ -2363,6 +2363,7 @@ TEST_F(FormStructureTest,
       form_structure->field(i)->set_generation_type(
           AutofillUploadContents::Field::
               MANUALLY_TRIGGERED_GENERATION_ON_SIGN_UP_FORM);
+      form_structure->field(i)->set_generated_password_changed(true);
       form_structure->field(i)->set_form_classifier_outcome(
           AutofillUploadContents::Field::GENERATION_ELEMENT);
     } else {
@@ -2436,6 +2437,7 @@ TEST_F(FormStructureTest,
           MANUALLY_TRIGGERED_GENERATION_ON_SIGN_UP_FORM);
   upload_password_field->set_properties_mask(FieldPropertiesFlags::HAD_FOCUS |
                                              FieldPropertiesFlags::USER_TYPED);
+  upload_password_field->set_generated_password_changed(true);
 
   std::string expected_upload_string;
   ASSERT_TRUE(upload.SerializeToString(&expected_upload_string));

components/autofill/core/browser/proto/server.proto

@@ -33,7 +33,7 @@ message AutofillQueryResponseContents {
 
 // This message contains information about the field types in a single form.
 // It is sent by the toolbar to contribute to the field type statistics.
-// Next available id: 22
+// Next available id: 23
 message AutofillUploadContents {
   required string client_version = 1;
   required fixed64 form_signature = 2;
@@ -98,6 +98,10 @@ message AutofillUploadContents {
     // The value of the id attribute, if it differs from the name attribute.
     // Otherwise, this field is absent.
     optional string id = 21;
+
+    // True iff the user changed generated password. If there was no generation,
+    // the field is absent.
+    optional bool generated_password_changed = 22;
   }
   // Signature of the form action host (e.g. Hash64Bit("example.com")).
   optional fixed64 action_signature = 13;

components/password_manager/content/browser/content_password_manager_driver.cc

@@ -272,8 +272,7 @@ void ContentPasswordManagerDriver::PasswordNoLongerGenerated(
           password_form.origin,
           BadMessageReason::CPMD_BAD_ORIGIN_PASSWORD_NO_LONGER_GENERATED))
     return;
-  GetPasswordManager()->SetHasGeneratedPasswordForForm(this, password_form,
-                                                       false);
+  GetPasswordManager()->OnPasswordNoLongerGenerated(password_form);
 }
 
 void ContentPasswordManagerDriver::SaveGenerationFieldDetectedByClassifier(

components/password_manager/core/browser/password_form_manager.cc

@@ -217,6 +217,7 @@ PasswordFormManager::PasswordFormManager(
       is_new_login_(true),
       has_autofilled_(false),
       has_generated_password_(false),
+      generated_password_changed_(false),
       is_manual_generation_(false),
       generation_popup_was_shown_(false),
       form_classifier_outcome_(kNoOutcome),
@@ -441,6 +442,25 @@ void PasswordFormManager::Update(
                       old_primary_key ? &old_primary_key.value() : nullptr);
 }
 
+void PasswordFormManager::PresaveGeneratedPassword(
+    const autofill::PasswordForm& form) {
+  form_saver()->PresaveGeneratedPassword(form);
+  metrics_recorder_.SetHasGeneratedPassword(true);
+  if (has_generated_password_) {
+    generated_password_changed_ = true;
+  } else {
+    SetHasGeneratedPassword(true);
+    generated_password_changed_ = false;
+  }
+}
+
+void PasswordFormManager::PasswordNoLongerGenerated() {
+  DCHECK(has_generated_password_);
+  form_saver()->RemovePresavedPassword();
+  SetHasGeneratedPassword(false);
+  generated_password_changed_ = false;
+}
+
 void PasswordFormManager::SetSubmittedForm(const autofill::PasswordForm& form) {
   bool is_change_password_form =
       !form.new_password_value.empty() && !form.password_value.empty();
@@ -859,6 +879,7 @@ void PasswordFormManager::AddGeneratedVote(
     autofill::AutofillField* field = form_structure->field(i);
     if (field->name == generation_element_) {
       field->set_generation_type(type);
+      field->set_generated_password_changed(generated_password_changed_);
       break;
     }
   }
@@ -1194,10 +1215,10 @@ void PasswordFormManager::OnNeverClicked() {
 }
 
 void PasswordFormManager::OnNoInteraction(bool is_update) {
-  if (is_update)
+  if (is_update) {
     UploadPasswordVote(observed_form_, autofill::PROBABLY_NEW_PASSWORD,
                        std::string());
-  else {
+  } else {
     UploadPasswordVote(pending_credentials_, autofill::UNKNOWN_TYPE,
                        std::string());
   }

components/password_manager/core/browser/password_form_manager.h

@@ -153,11 +153,26 @@ class PasswordFormManager : public FormFetcher::Consumer {
   void LogSubmitPassed();
   void LogSubmitFailed();
 
+  // Called when generated password is accepted or changed by user.
+  void PresaveGeneratedPassword(const autofill::PasswordForm& form);
+
+  // Called when user removed a generated password.
+  void PasswordNoLongerGenerated();
+
   // These functions are used to determine if this form has had it's password
   // auto generated by the browser.
   bool has_generated_password() const { return has_generated_password_; }
   void SetHasGeneratedPassword(bool generated_password);
 
+  // These functions are used to determine if this form has generated password
+  // changed by user.
+  bool generated_password_changed() const {
+    return generated_password_changed_;
+  }
+  void set_generated_password_changed(bool generated_password_changed) {
+    generated_password_changed_ = generated_password_changed;
+  }
+
   bool is_manual_generation() { return is_manual_generation_; }
   void set_is_manual_generation(bool is_manual_generation) {
     is_manual_generation_ = is_manual_generation;
@@ -458,6 +473,9 @@ class PasswordFormManager : public FormFetcher::Consumer {
   // Whether this form has an auto generated password.
   bool has_generated_password_;
 
+  // Whether this form has a generated password changed by user.
+  bool generated_password_changed_;
+
   // Whether password generation was manually triggered.
   bool is_manual_generation_;
 

components/password_manager/core/browser/password_form_manager_unittest.cc

@@ -90,6 +90,9 @@ class MockFormSaver : public StubFormSaver {
                void(const autofill::PasswordForm& pending,
                     std::map<base::string16, const PasswordForm*>* best_matches,
                     const autofill::PasswordForm** preferred_match));
+  MOCK_METHOD1(PresaveGeneratedPassword,
+               void(const autofill::PasswordForm& generated));
+  MOCK_METHOD0(RemovePresavedPassword, void());
 
   // Convenience downcasting method.
   static MockFormSaver& Get(PasswordFormManager* form_manager) {
@@ -153,9 +156,10 @@ MATCHER_P3(CheckUploadedAutofillTypesAndSignature,
   return true;
 }
 
-MATCHER_P2(CheckUploadedGenerationTypesAndSignature,
+MATCHER_P3(CheckUploadedGenerationTypesAndSignature,
            form_signature,
            expected_generation_types,
+           generated_password_changed,
            "Unexpected generation types or form signature") {
   if (form_signature != arg.FormSignatureAsStr()) {
     // Unexpected form's signature.
@@ -184,6 +188,12 @@ MATCHER_P2(CheckUploadedGenerationTypesAndSignature,
                       << ", but found " << field->generation_type();
         return false;
       }
+
+      if (field->generation_type() !=
+          autofill::AutofillUploadContents::Field::IGNORED_GENERATION_POPUP) {
+        EXPECT_EQ(generated_password_changed,
+                  field->generated_password_changed());
+      }
     }
   }
   return true;
@@ -605,11 +615,13 @@ class PasswordFormManagerTest : public testing::Test {
   void GeneratedVoteUploadTest(bool is_manual_generation,
                                bool is_change_password_form,
                                bool has_generated_password,
+                               bool generated_password_changed,
                                SavePromptInteraction interaction) {
     SCOPED_TRACE(testing::Message()
                  << "is_manual_generation=" << is_manual_generation
                  << " is_change_password_form=" << is_change_password_form
                  << " has_generated_password=" << has_generated_password
+                 << " generated_password_changed=" << generated_password_changed
                  << " interaction=" << interaction);
     PasswordForm form(*observed_form());
     form.form_data = saved_match()->form_data;
@@ -656,6 +668,8 @@ class PasswordFormManagerTest : public testing::Test {
     form_manager.set_generation_element(generation_element);
     form_manager.set_generation_popup_was_shown(true);
     form_manager.SetHasGeneratedPassword(has_generated_password);
+    if (has_generated_password)
+      form_manager.set_generated_password_changed(generated_password_changed);
 
     // Figure out expected generation event type.
     autofill::AutofillUploadContents::Field::PasswordGenerationType
@@ -673,7 +687,8 @@ class PasswordFormManagerTest : public testing::Test {
         *client()->mock_driver()->mock_autofill_download_manager(),
         StartUploadRequest(
             CheckUploadedGenerationTypesAndSignature(
-                form_structure.FormSignatureAsStr(), expected_generation_types),
+                form_structure.FormSignatureAsStr(), expected_generation_types,
+                generated_password_changed),
             false, expected_available_field_types, std::string(), true));
 
     form_manager.ProvisionallySave(
@@ -2529,7 +2544,6 @@ TEST_F(PasswordFormManagerTest, TestNotUpdateWhenOnlyPSLMatched) {
 TEST_F(PasswordFormManagerTest,
        TestSavingOnChangePasswordFormGenerationNoStoredForms) {
   fake_form_fetcher()->SetNonFederated(std::vector<const PasswordForm*>(), 0u);
-  form_manager()->SetHasGeneratedPassword(true);
 
   // User submits change password form and there is no stored credentials.
   PasswordForm credentials = *observed_form();
@@ -2538,6 +2552,7 @@ TEST_F(PasswordFormManagerTest,
   credentials.new_password_element = ASCIIToUTF16("NewPasswd");
   credentials.new_password_value = ASCIIToUTF16("new_password");
   credentials.preferred = true;
+  form_manager()->PresaveGeneratedPassword(credentials);
   form_manager()->ProvisionallySave(
       credentials, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES);
 
@@ -2564,7 +2579,6 @@ TEST_F(PasswordFormManagerTest,
 
 TEST_F(PasswordFormManagerTest, TestUpdatingOnChangePasswordFormGeneration) {
   fake_form_fetcher()->SetNonFederated({saved_match()}, 0u);
-  form_manager()->SetHasGeneratedPassword(true);
 
   // User submits credentials for the change password form, and old password is
   // coincide with password from an existing credentials, so stored credentials
@@ -2575,6 +2589,7 @@ TEST_F(PasswordFormManagerTest, TestUpdatingOnChangePasswordFormGeneration) {
   credentials.new_password_element = ASCIIToUTF16("NewPasswd");
   credentials.new_password_value = ASCIIToUTF16("new_password");
   credentials.preferred = true;
+  form_manager()->PresaveGeneratedPassword(credentials);
   form_manager()->ProvisionallySave(
       credentials, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES);
 
@@ -2600,7 +2615,6 @@ TEST_F(PasswordFormManagerTest, TestUpdatingOnChangePasswordFormGeneration) {
 TEST_F(PasswordFormManagerTest,
        TestSavingOnChangePasswordFormGenerationNoMatchedForms) {
   fake_form_fetcher()->SetNonFederated({saved_match()}, 0u);
-  form_manager()->SetHasGeneratedPassword(true);
 
   // User submits credentials for the change password form, and old password is
   // not coincide with password from existing credentials, so new credentials
@@ -2612,6 +2626,7 @@ TEST_F(PasswordFormManagerTest,
   credentials.new_password_element = ASCIIToUTF16("NewPasswd");
   credentials.new_password_value = ASCIIToUTF16("new_password");
   credentials.preferred = true;
+  form_manager()->PresaveGeneratedPassword(credentials);
   form_manager()->ProvisionallySave(
       credentials, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES);
 
@@ -2710,15 +2725,42 @@ TEST_F(PasswordFormManagerTest, GeneratedVoteUpload) {
   for (bool is_manual_generation : kFalseTrue) {
     for (bool is_change_password_form : kFalseTrue) {
       for (bool has_generated_password : kFalseTrue) {
-        for (SavePromptInteraction interaction : kSavePromptInterations) {
-          GeneratedVoteUploadTest(is_manual_generation, is_change_password_form,
-                                  has_generated_password, interaction);
+        for (bool generated_password_changed : kFalseTrue) {
+          for (SavePromptInteraction interaction : kSavePromptInterations) {
+            GeneratedVoteUploadTest(is_manual_generation,
+                                    is_change_password_form,
+                                    has_generated_password,
+                                    generated_password_changed, interaction);
+          }
         }
       }
     }
   }
 }
 
+TEST_F(PasswordFormManagerTest, PresaveGeneratedPasswordAndRemoveIt) {
+  PasswordForm credentials = *observed_form();
+
+  // Simulate the user accepted a generated password.
+  EXPECT_CALL(MockFormSaver::Get(form_manager()),
+              PresaveGeneratedPassword(credentials));
+  form_manager()->PresaveGeneratedPassword(credentials);
+  EXPECT_TRUE(form_manager()->has_generated_password());
+  EXPECT_FALSE(form_manager()->generated_password_changed());
+
+  // Simulate the user changed the presaved password.
+  credentials.password_value = ASCIIToUTF16("changed_password");
+  EXPECT_CALL(MockFormSaver::Get(form_manager()),
+              PresaveGeneratedPassword(credentials));
+  form_manager()->PresaveGeneratedPassword(credentials);
+  EXPECT_TRUE(form_manager()->has_generated_password());
+  EXPECT_TRUE(form_manager()->generated_password_changed());
+
+  // Simulate the user removed the presaved password.
+  EXPECT_CALL(MockFormSaver::Get(form_manager()), RemovePresavedPassword());
+  form_manager()->PasswordNoLongerGenerated();
+}
+
 TEST_F(PasswordFormManagerTest, FormClassifierVoteUpload) {
   const bool kFalseTrue[] = {false, true};
   for (bool found_generation_element : kFalseTrue) {

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",
+                          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->SetHasGeneratedPassword(password_is_generated);
+    form_manager->PasswordNoLongerGenerated();
     return;
   }
-
-  UMA_HISTOGRAM_BOOLEAN("PasswordManager.GeneratedFormHasNoFormManager",
-                        password_is_generated);
 }
 
 void PasswordManager::SetGenerationElementAndReasonForForm(

components/password_manager/core/browser/password_manager.h

@@ -101,14 +101,10 @@ class PasswordManager : public LoginModel {
   void GenerationAvailableForForm(const autofill::PasswordForm& form);
 
   // Presaves the form with generated password.
-  void OnPresaveGeneratedPassword(const autofill::PasswordForm& password_form);
+  void OnPresaveGeneratedPassword(const autofill::PasswordForm& form);
 
-  // Update the state of generation for this form.
-  // If |password_is_generated| == false, removes the presaved form.
-  void SetHasGeneratedPasswordForForm(
-      password_manager::PasswordManagerDriver* driver,
-      const autofill::PasswordForm& form,
-      bool password_is_generated);
+  // Stops treating a password as generated.
+  void OnPasswordNoLongerGenerated(const autofill::PasswordForm& form);
 
   // Update the generation element and whether generation was triggered
   // manually.

components/password_manager/core/browser/password_manager_unittest.cc

@@ -14,6 +14,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "components/password_manager/core/browser/form_fetcher_impl.h"
 #include "components/password_manager/core/browser/mock_password_store.h"
@@ -306,7 +307,8 @@ TEST_F(PasswordManagerTest, GeneratedPasswordFormSubmitEmptyStore) {
   // Simulate the user generating the password and submitting the form.
   EXPECT_CALL(client_, IsSavingAndFillingEnabledForCurrentPage())
       .WillRepeatedly(Return(true));
-  manager()->SetHasGeneratedPasswordForForm(&driver_, form, true);
+  EXPECT_CALL(*store_, AddLogin(_));
+  manager()->OnPresaveGeneratedPassword(form);
   OnPasswordFormSubmitted(form);
 
   // The user should not need to confirm saving as they have already given
@@ -315,7 +317,8 @@ TEST_F(PasswordManagerTest, GeneratedPasswordFormSubmitEmptyStore) {
   // occured.
   EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0);
   PasswordForm form_to_save;
-  EXPECT_CALL(*store_, AddLogin(_)).WillOnce(SaveArg<0>(&form_to_save));
+  EXPECT_CALL(*store_, UpdateLoginWithPrimaryKey(_, _))
+      .WillOnce(SaveArg<0>(&form_to_save));
   EXPECT_CALL(client_, AutomaticPasswordSaveIndicator());
 
   // Now the password manager waits for the navigation to complete.
@@ -1219,7 +1222,8 @@ TEST_F(PasswordManagerTest, PasswordGeneration_FailedSubmission) {
 
   EXPECT_CALL(client_, IsSavingAndFillingEnabledForCurrentPage())
       .WillRepeatedly(Return(true));
-  manager()->SetHasGeneratedPasswordForForm(&driver_, form, true);
+  EXPECT_CALL(*store_, AddLogin(_));
+  manager()->OnPresaveGeneratedPassword(form);
 
   // Do not save generated password when the password form reappears.
   EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0);
@@ -1246,7 +1250,8 @@ TEST_F(PasswordManagerTest, PasswordGenerationPasswordEdited_FailedSubmission) {
 
   EXPECT_CALL(client_, IsSavingAndFillingEnabledForCurrentPage())
       .WillRepeatedly(Return(true));
-  manager()->SetHasGeneratedPasswordForForm(&driver_, form, true);
+  EXPECT_CALL(*store_, AddLogin(_));
+  manager()->OnPresaveGeneratedPassword(form);
 
   // Simulate user editing and submitting a different password. Verify that
   // the edited password is the one that is saved.
@@ -1279,11 +1284,13 @@ TEST_F(PasswordManagerTest,
 
   EXPECT_CALL(client_, IsSavingAndFillingEnabledForCurrentPage())
       .WillRepeatedly(Return(true));
-  manager()->SetHasGeneratedPasswordForForm(&driver_, form, true);
+  EXPECT_CALL(*store_, AddLogin(_));
+  manager()->OnPresaveGeneratedPassword(form);
 
   // Simulate user removing generated password and adding a new one.
   form.new_password_value = ASCIIToUTF16("different_password");
-  manager()->SetHasGeneratedPasswordForForm(&driver_, form, false);
+  EXPECT_CALL(*store_, RemoveLogin(_));
+  manager()->OnPasswordNoLongerGenerated(form);
 
   OnPasswordFormSubmitted(form);
 
@@ -1311,11 +1318,13 @@ TEST_F(PasswordManagerTest,
 
   EXPECT_CALL(client_, IsSavingAndFillingEnabledForCurrentPage())
       .WillRepeatedly(Return(true));
-  manager()->SetHasGeneratedPasswordForForm(&driver_, form, true);
+  EXPECT_CALL(*store_, AddLogin(_));
+  manager()->OnPresaveGeneratedPassword(form);
 
   // Simulate user removing generated password and adding a new one.
   form.new_password_value = ASCIIToUTF16("different_password");
-  manager()->SetHasGeneratedPasswordForForm(&driver_, form, false);
+  EXPECT_CALL(*store_, RemoveLogin(_));
+  manager()->OnPasswordNoLongerGenerated(form);
 
   OnPasswordFormSubmitted(form);
 
@@ -1342,7 +1351,8 @@ TEST_F(PasswordManagerTest, PasswordGenerationUsernameChanged) {
 
   EXPECT_CALL(client_, IsSavingAndFillingEnabledForCurrentPage())
       .WillRepeatedly(Return(true));
-  manager()->SetHasGeneratedPasswordForForm(&driver_, form, true);
+  EXPECT_CALL(*store_, AddLogin(_));
+  manager()->OnPresaveGeneratedPassword(form);
 
   // Simulate user changing the password and username, without ever completely
   // deleting the password.
@@ -1352,7 +1362,8 @@ TEST_F(PasswordManagerTest, PasswordGenerationUsernameChanged) {
 
   EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0);
   PasswordForm form_to_save;
-  EXPECT_CALL(*store_, AddLogin(_)).WillOnce(SaveArg<0>(&form_to_save));
+  EXPECT_CALL(*store_, UpdateLoginWithPrimaryKey(_, _))
+      .WillOnce(SaveArg<0>(&form_to_save));
   EXPECT_CALL(client_, AutomaticPasswordSaveIndicator());
 
   observed.clear();
@@ -1375,11 +1386,12 @@ TEST_F(PasswordManagerTest, PasswordGenerationPresavePassword) {
   manager()->OnPasswordFormsParsed(&driver_, observed);
   manager()->OnPasswordFormsRendered(&driver_, observed, true);
 
+  base::HistogramTester histogram_tester;
+
   // The user accepts a generated password.
   form.password_value = base::ASCIIToUTF16("password");
   EXPECT_CALL(*store_, AddLogin(form)).WillOnce(Return());
   manager()->OnPresaveGeneratedPassword(form);
-  manager()->SetHasGeneratedPasswordForForm(&driver_, form, true);
 
   // The user updates the generated password.
   PasswordForm updated_form(form);
@@ -1387,10 +1399,36 @@ TEST_F(PasswordManagerTest, PasswordGenerationPresavePassword) {
   EXPECT_CALL(*store_, UpdateLoginWithPrimaryKey(updated_form, form))
       .WillOnce(Return());
   manager()->OnPresaveGeneratedPassword(updated_form);
+  histogram_tester.ExpectUniqueSample(
+      "PasswordManager.GeneratedFormHasNoFormManager", false, 2);
 
   // The user removes the generated password.
   EXPECT_CALL(*store_, RemoveLogin(updated_form)).WillOnce(Return());
-  manager()->SetHasGeneratedPasswordForForm(&driver_, updated_form, false);
+  manager()->OnPasswordNoLongerGenerated(updated_form);
+}
+
+TEST_F(PasswordManagerTest, PasswordGenerationPresavePassword_NoFormManager) {
+  // Checks that GeneratedFormHasNoFormManager metric is sent if there is no
+  // corresponding PasswordFormManager for the given form. It should be uncommon
+  // case.
+  std::vector<PasswordForm> observed;
+  EXPECT_CALL(client_, IsSavingAndFillingEnabledForCurrentPage())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*store_, GetLogins(_, _))
+      .WillRepeatedly(WithArg<1>(InvokeEmptyConsumerWithForms()));
+  manager()->OnPasswordFormsParsed(&driver_, observed);
+  manager()->OnPasswordFormsRendered(&driver_, observed, true);
+
+  base::HistogramTester histogram_tester;
+
+  // The user accepts a generated password.
+  PasswordForm form(MakeFormWithOnlyNewPasswordField());
+  form.password_value = base::ASCIIToUTF16("password");
+  EXPECT_CALL(*store_, AddLogin(_)).Times(0);
+
+  manager()->OnPresaveGeneratedPassword(form);
+  histogram_tester.ExpectUniqueSample(
+      "PasswordManager.GeneratedFormHasNoFormManager", true, 1);
 }
 
 TEST_F(PasswordManagerTest, PasswordGenerationPresavePasswordAndLogin) {
@@ -1428,7 +1466,6 @@ TEST_F(PasswordManagerTest, PasswordGenerationPresavePasswordAndLogin) {
     // The user accepts generated password and makes successful login.
     EXPECT_CALL(*store_, AddLogin(form)).WillOnce(Return());
     manager()->OnPresaveGeneratedPassword(form);
-    manager()->SetHasGeneratedPasswordForForm(&driver_, form, true);
     ::testing::Mock::VerifyAndClearExpectations(store_.get());
 
     if (!found_matched_logins_in_store)

ios/chrome/browser/passwords/password_generation_agent.mm

@@ -356,9 +356,8 @@ passwordManagerDriver:(password_manager::PasswordManagerDriver*)driver
       return;
     if (strongSelf->_passwordManager) {
       // Might be null in tests.
-      strongSelf->_passwordManager->SetHasGeneratedPasswordForForm(
-          strongSelf->_passwordManagerDriver,
-          *strongSelf->_possibleAccountCreationForm, true);
+      strongSelf->_passwordManager->OnPresaveGeneratedPassword(
+          *strongSelf->_possibleAccountCreationForm);
     }
     if (strongSelf->_accessoryViewReadyCompletion) {
       strongSelf->_accessoryViewReadyCompletion(