Don't send multiple username form votes for the same field.

Bug: 959776

Change-Id: I58863e2b625bd55242974283646bfff44bb04fc3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1944865
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720526}

components/password_manager/core/browser/password_form_manager_unittest.cc

@@ -50,6 +50,7 @@ using autofill::PasswordFormFillData;
 using autofill::PasswordFormGenerationData;
 using autofill::ServerFieldType;
 using autofill::SINGLE_USERNAME;
+using autofill::UNKNOWN_TYPE;
 using base::ASCIIToUTF16;
 using base::TestMockTimeTaskRunner;
 using testing::_;
@@ -2153,6 +2154,12 @@ TEST_P(PasswordFormManagerTest, UsernameFirstFlowVotes) {
   predictions.fields.push_back(field_prediction);
   possible_username_data.form_predictions = predictions;
 
+  MockFieldInfoManager mock_field_manager;
+  ON_CALL(mock_field_manager, GetFieldType(_, _))
+      .WillByDefault(Return(UNKNOWN_TYPE));
+  ON_CALL(client_, GetFieldInfoManager())
+      .WillByDefault(Return(&mock_field_manager));
+
   // Simulate submission a form without username. Data from
   // |possible_username_data| will be taken for setting username.
   FormData submitted_form = observed_form_only_password_fields_;

components/password_manager/core/browser/votes_uploader.cc

@@ -466,6 +466,10 @@ void VotesUploader::MaybeSendSingleUsernameVote(bool credentials_saved) {
   if (!single_username_vote_data_)
     return;
 
+  FieldInfoManager* field_info_manager = client_->GetFieldInfoManager();
+  if (!field_info_manager)
+    return;
+
   const FormPredictions& predictions =
       single_username_vote_data_->form_predictions;
   std::vector<FieldSignature> field_signatures;
@@ -485,6 +489,12 @@ void VotesUploader::MaybeSendSingleUsernameVote(bool credentials_saved) {
     ServerFieldType type = autofill::UNKNOWN_TYPE;
     uint32_t field_renderer_id = predictions.fields[i].renderer_id;
     if (field_renderer_id == single_username_vote_data_->renderer_id) {
+      if (field_info_manager->GetFieldType(predictions.form_signature,
+                                           predictions.fields[i].signature) !=
+          autofill::UNKNOWN_TYPE) {
+        // The vote for this field has been already sent. Don't send again.
+        return;
+      }
       type = credentials_saved && !has_username_edited_vote_
                  ? autofill::SINGLE_USERNAME
                  : autofill::NOT_USERNAME;

components/password_manager/core/browser/votes_uploader_unittest.cc

@@ -39,6 +39,7 @@ using autofill::PasswordForm;
 using autofill::ServerFieldType;
 using autofill::ServerFieldTypeSet;
 using autofill::SINGLE_USERNAME;
+using autofill::UNKNOWN_TYPE;
 using autofill::mojom::SubmissionIndicatorEvent;
 using base::ASCIIToUTF16;
 using testing::_;
@@ -89,6 +90,12 @@ class MockPasswordManagerClient : public StubPasswordManagerClient {
   MOCK_CONST_METHOD0(GetFieldInfoManager, FieldInfoManager*());
 };
 
+class MockFieldInfoManager : public FieldInfoManager {
+ public:
+  MOCK_METHOD3(AddFieldType, void(uint64_t, uint32_t, ServerFieldType));
+  MOCK_CONST_METHOD2(GetFieldType, ServerFieldType(uint64_t, uint32_t));
+};
+
 }  // namespace
 
 class VotesUploaderTest : public testing::Test {
@@ -397,6 +404,13 @@ TEST_F(VotesUploaderTest, UploadSingleUsername) {
   for (bool credentials_saved : {false, true}) {
     SCOPED_TRACE(testing::Message("credentials_saved = ") << credentials_saved);
     VotesUploader votes_uploader(&client_, false);
+
+    MockFieldInfoManager mock_field_manager;
+    ON_CALL(mock_field_manager, GetFieldType(_, _))
+        .WillByDefault(Return(UNKNOWN_TYPE));
+    ON_CALL(client_, GetFieldInfoManager())
+        .WillByDefault(Return(&mock_field_manager));
+
     constexpr uint32_t kUsernameRendererId = 101;
     constexpr uint32_t kUsernameFieldSignature = 1234;
     constexpr uint64_t kFormSignature = 1000;
@@ -454,11 +468,42 @@ TEST_F(VotesUploaderTest, SaveSingleUsernameVote) {
   // Init FieldInfoManager.
   FieldInfoManagerImpl field_info_manager(store);
   EXPECT_CALL(client_, GetFieldInfoManager())
-      .WillOnce(Return(&field_info_manager));
+      .WillRepeatedly(Return(&field_info_manager));
 
   votes_uploader.MaybeSendSingleUsernameVote(true /*  credentials_saved */);
   task_environment_.RunUntilIdle();
   store->ShutdownOnUIThread();
 }
 
+TEST_F(VotesUploaderTest, DontUploadSingleUsernameWhenAlreadyUploaded) {
+  VotesUploader votes_uploader(&client_, false);
+  constexpr uint32_t kUsernameRendererId = 101;
+  constexpr uint32_t kUsernameFieldSignature = 1234;
+  constexpr uint64_t kFormSignature = 1000;
+
+  MockFieldInfoManager mock_field_manager;
+  ON_CALL(client_, GetFieldInfoManager())
+      .WillByDefault(Return(&mock_field_manager));
+  // Simulate that the vote has been already uploaded.
+  ON_CALL(mock_field_manager,
+          GetFieldType(kFormSignature, kUsernameFieldSignature))
+      .WillByDefault(Return(SINGLE_USERNAME));
+
+  FormPredictions form_predictions;
+  form_predictions.form_signature = kFormSignature;
+
+  // Add the username field.
+  form_predictions.fields.emplace_back();
+  form_predictions.fields.back().renderer_id = kUsernameRendererId;
+  form_predictions.fields.back().signature = kUsernameFieldSignature;
+
+  votes_uploader.set_single_username_vote_data(kUsernameRendererId,
+                                               form_predictions);
+
+  // Expect no upload, since the vote has been already uploaded.
+  EXPECT_CALL(mock_autofill_download_manager_, StartUploadRequest).Times(0);
+
+  votes_uploader.MaybeSendSingleUsernameVote(true /*credentials_saved*/);
+}
+
 }  // namespace password_manager