[Password Manager] Don't upload FIRST_USE vote for new passwords

When a credential is used for the first time, we upload a FIRST_USE
crowdsourcing vote, voting the fields as USERNAME and PASSWORD, even if
the use results in a new password value being saved. This was to be
resilient against cases where the password manager saves the wrong
password in the original form. However, it is semantically incorrect,
both in the above case and also when applied to change-password forms.

We change the vote so that we don't upload a vote for the password
field.

This does not apply to usernames, because a new username is treated as
a new credential. In the case where we add a username to an old
credential, we keep setting the KNOWN_VALUE flag, because this event
only happens as a correction and doesn't say something different about
the field. Also, the fact that the user accepted the prompt is a strong
indication that the field contains a proper username.

Bug: 840384
Change-Id: Iff20a022abc50a4dab2f32f46e13e33621eb6129
Reviewed-on: https://chromium-review.googlesource.com/1071523
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561870}

components/password_manager/core/browser/password_form_manager.cc

@@ -937,14 +937,19 @@ void PasswordFormManager::UploadFirstLoginVotes(
   if (!autofill_manager->ShouldUploadForm(form_structure))
     return;
 
+  FieldTypeMap field_types = {
+      {form_to_upload.username_element, autofill::USERNAME}};
+  VoteTypeMap vote_types = {
+      {form_to_upload.username_element,
+       autofill::AutofillUploadContents::Field::FIRST_USE}};
+  if (!password_overridden_) {
+    field_types[form_to_upload.password_element] = autofill::PASSWORD;
+    vote_types[form_to_upload.password_element] =
+        autofill::AutofillUploadContents::Field::FIRST_USE;
+  }
+
   autofill::ServerFieldTypeSet available_field_types;
-  LabelFields({{form_to_upload.username_element, autofill::USERNAME},
-               {form_to_upload.password_element, autofill::PASSWORD}},
-              {{form_to_upload.username_element,
-                autofill::AutofillUploadContents::Field::FIRST_USE},
-               {form_to_upload.password_element,
-                autofill::AutofillUploadContents::Field::FIRST_USE}},
-              &form_structure, &available_field_types);
+  LabelFields(field_types, vote_types, &form_structure, &available_field_types);
   SetKnownValueFlag(&form_structure);
 
   // Force uploading as these events are relatively rare and we want to make
@@ -962,11 +967,23 @@ void PasswordFormManager::UploadFirstLoginVotes(
 }
 
 void PasswordFormManager::SetKnownValueFlag(autofill::FormStructure* form) {
+  DCHECK(!password_overridden_ ||
+         best_matches_.find(pending_credentials_.username_value) !=
+             best_matches_.end())
+      << "The credential is being overriden, but it does not exist in "
+         "the best matches.";
+
+  const base::string16& known_username = pending_credentials_.username_value;
+  // If we are updating a password, the known value is the old password, not
+  // the new one.
+  const base::string16& known_password =
+      password_overridden_ ? best_matches_[known_username]->password_value
+                           : pending_credentials_.password_value;
+
   for (auto& field : *form) {
     if (field->value.empty())
       continue;
-    if (pending_credentials_.username_value == field->value ||
-        pending_credentials_.password_value == field->value) {
+    if (known_username == field->value || known_password == field->value) {
       field->properties_mask |= autofill::FieldPropertiesFlags::KNOWN_VALUE;
     }
   }

components/password_manager/core/browser/password_form_manager_unittest.cc

@@ -4362,8 +4362,6 @@ TEST_F(PasswordFormManagerTest,
 }
 
 TEST_F(PasswordFormManagerTest, FirstLoginVote) {
-  PasswordForm different_password(*saved_match());
-  different_password.password_value = ASCIIToUTF16("DifferentPassword");
 
   PasswordForm old_without_username = *saved_match();
   old_without_username.username_value.clear();
@@ -4375,7 +4373,6 @@ TEST_F(PasswordFormManagerTest, FirstLoginVote) {
   } test_cases[] = {
       {{saved_match()}, "Credential reused"},
       {{psl_saved_match()}, "PSL credential reused"},
-      {{&different_password}, "Different password"},  // I.e. update password.
       {{&old_without_username},
        "Submitted credential adds a username to a stored credential without "
        "one"},
@@ -4491,6 +4488,59 @@ TEST_F(PasswordFormManagerTest, FirstLoginVote_NoVote) {
   }
 }
 
+// If we update an existing credential with a new password, only the username is
+// a known value.
+TEST_F(PasswordFormManagerTest,
+       FirstLoginVote_UpdatePasswordVotesOnlyForUsername) {
+  PasswordForm different_password(*saved_match());
+  different_password.password_value = ASCIIToUTF16("DifferentPassword");
+  fake_form_fetcher()->SetNonFederated({&different_password}, 0u);
+
+  PasswordForm submitted_form =
+      CreateMinimalCrowdsourcableForm(*observed_form());
+  submitted_form.username_value = saved_match()->username_value;
+  submitted_form.password_value = saved_match()->password_value;
+  submitted_form.form_data.fields[0].value = submitted_form.username_value;
+  submitted_form.form_data.fields[1].value = submitted_form.password_value;
+  submitted_form.preferred = true;
+  EXPECT_TRUE(FormStructure(submitted_form.form_data).ShouldBeUploaded());
+
+  form_manager()->ProvisionallySave(submitted_form);
+
+  // The username and password fields contain stored values. This should be
+  // signaled in the vote.
+  std::map<base::string16, autofill::FieldPropertiesMask>
+      expected_field_properties = {
+          {submitted_form.username_element, FieldPropertiesFlags::KNOWN_VALUE},
+          {submitted_form.password_element, 0}};
+
+  // All votes should be FIRST_USE.
+  std::map<base::string16, autofill::AutofillUploadContents::Field::VoteType>
+      expected_vote_types = {
+          {submitted_form.username_element,
+           autofill::AutofillUploadContents::Field::FIRST_USE}};
+
+  // Unrelated vote
+  EXPECT_CALL(
+      *client()->mock_driver()->mock_autofill_download_manager(),
+      StartUploadRequest(SignatureIsSameAs(different_password), _, _, _, _))
+      .Times(AtMost(1));
+  // First login vote
+  EXPECT_CALL(
+      *client()->mock_driver()->mock_autofill_download_manager(),
+      StartUploadRequest(
+          AllOf(SignatureIsSameAs(submitted_form),
+                UploadedAutofillTypesAre(FieldTypeMap(
+                    {{submitted_form.username_element, autofill::USERNAME},
+                     {submitted_form.password_element, autofill::UNKNOWN_TYPE},
+                     {ASCIIToUTF16("petname"), autofill::UNKNOWN_TYPE}})),
+                UploadedFieldPropertiesMasksAre(expected_field_properties),
+                VoteTypesAre(expected_vote_types)),
+          _, autofill::ServerFieldTypeSet({autofill::USERNAME}), _, true));
+
+  form_manager()->Save();
+}
+
 // Values on a submitted form should be marked as KNOWN_VALUE only if they match
 // values from the credential which was used to log in. Other stored credentials
 // are ignored.