Revert "[Passwords] Fixing sending votes for dynamic password forms."

This reverts commit ff1f7499.

Reason for revert: https://crbug.com/1158136

Original change's description:
> [Passwords] Fixing sending votes for dynamic password forms.
>
> VotesUploader uploades password votes on form submission. These votes
> are used to generate server predictions that are requested and received
> on the pageload. Thus if the form is changed dynamically, it makes sense
> to send votes for the form signature that is seen on the pageload.
> This CL starts using initial form signatures instead of signatures
> seen at the moment of submission.
>
> Bug: 1157048
> Change-Id: I13f51852c2b9447625ae6d20cd335e481dcab4e8
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2578900
> Reviewed-by: Clemens Arbesser <arbesser@google.com>
> Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
> Commit-Queue: Maria Kazinova <kazinova@google.com>
> Cr-Commit-Position: refs/heads/master@{#835202}

TBR=jdoerrie@chromium.org,arbesser@google.com,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com,kazinova@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1157048
Bug: 1158136
Change-Id: Ica25a2f7dcaf9b76f4084854d19ecb6a19494dc9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593638
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837544}

components/autofill_assistant/browser/website_login_manager_impl.cc

@@ -180,9 +180,7 @@ class WebsiteLoginManagerImpl::UpdatePasswordRequest
                 client->IsCommittedMainFrameSecure(),
                 client->GetUkmSourceId(),
                 client->GetPrefs())),
-        votes_uploader_(client,
-                        true /* is_possible_change_password_form */,
-                        CalculateFormSignature(form_data_)) {
+        votes_uploader_(client, true /* is_possible_change_password_form */) {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
     password_manager::PasswordStore::FormDigest digest(

components/password_manager/core/browser/multi_store_password_save_manager_unittest.cc

@@ -77,8 +77,7 @@ class MultiStorePasswordSaveManagerTest : public testing::Test {
  public:
   MultiStorePasswordSaveManagerTest()
       : votes_uploader_(&client_,
-                        false /* is_possible_change_password_form */,
-                        autofill::FormSignature()) {
+                        false /* is_possible_change_password_form */) {
     GURL origin = GURL("https://accounts.google.com/a/ServiceLoginAuth");
     GURL action = GURL("https://accounts.google.com/a/ServiceLogin");
     GURL psl_origin = GURL("https://myaccounts.google.com/a/ServiceLoginAuth");
@@ -153,9 +152,6 @@ class MultiStorePasswordSaveManagerTest : public testing::Test {
         client_.IsCommittedMainFrameSecure(), client_.GetUkmSourceId(),
         /*pref_service=*/nullptr);
 
-    votes_uploader_.set_initial_form_signature(
-        CalculateFormSignature(observed_form_));
-
     auto mock_profile_form_saver = std::make_unique<NiceMock<MockFormSaver>>();
     mock_profile_form_saver_ = mock_profile_form_saver.get();
 

components/password_manager/core/browser/password_form_manager.cc

@@ -894,10 +894,7 @@ PasswordFormManager::PasswordFormManager(
       password_save_manager_(std::move(password_save_manager)),
       // TODO(https://crbug.com/831123): set correctly
       // |is_possible_change_password_form| in |votes_uploader_| constructor
-      votes_uploader_(client,
-                      false /* is_possible_change_password_form */,
-                      observed_form() ? CalculateFormSignature(*observed_form())
-                                      : FormSignature()) {
+      votes_uploader_(client, false /* is_possible_change_password_form */) {
   form_fetcher_->AddConsumer(this);
   if (!metrics_recorder_) {
     metrics_recorder_ = base::MakeRefCounted<PasswordFormMetricsRecorder>(

components/password_manager/core/browser/password_form_manager_unittest.cc

@@ -2845,30 +2845,6 @@ TEST_F(PasswordFormManagerTestWithMockedSaver,
   EXPECT_TRUE(parsed_submitted_form.username_value.empty());
 }
 
-// Tests that if the initially observed form differs from the submitted form,
-// voting data should contain the initial form signature.
-TEST_P(PasswordFormManagerTest, VotingOnDynamicallyChangedForm) {
-  PasswordForm initial_form;
-  initial_form.form_data = non_password_form_;
-  CreateFormManager(non_password_form_);
-  fetcher_->NotifyFetchCompleted();
-
-  // Simulate observing form change.
-  form_manager_->FillForm(observed_form_, {});
-
-  // Simulate form filling and submission.
-  EXPECT_TRUE(
-      form_manager_->ProvisionallySave(submitted_form_, &driver_, nullptr));
-
-  EXPECT_CALL(
-      mock_autofill_download_manager_,
-      StartUploadRequest(SignatureIsSameAs(initial_form), _, _, _, _, _));
-
-  EXPECT_TRUE(CalculateFormSignature(initial_form.form_data).value() !=
-              CalculateFormSignature(submitted_form_).value());
-  form_manager_->Save();
-}
-
 }  // namespace
 
 }  // namespace password_manager

components/password_manager/core/browser/password_save_manager_impl_unittest.cc

@@ -195,9 +195,7 @@ class PasswordSaveManagerImplTest : public testing::Test,
                                     public testing::WithParamInterface<bool> {
  public:
   PasswordSaveManagerImplTest()
-      : votes_uploader_(&client_,
-                        false /* is_possible_change_password_form */,
-                        autofill::FormSignature()),
+      : votes_uploader_(&client_, false /* is_possible_change_password_form */),
         task_runner_(new TestMockTimeTaskRunner) {
     GURL origin = GURL("https://accounts.google.com/a/ServiceLoginAuth");
     GURL action = GURL("https://accounts.google.com/a/ServiceLogin");
@@ -301,9 +299,6 @@ class PasswordSaveManagerImplTest : public testing::Test,
     auto mock_form_saver = std::make_unique<NiceMock<MockFormSaver>>();
     mock_form_saver_ = mock_form_saver.get();
 
-    votes_uploader_.set_initial_form_signature(
-        CalculateFormSignature(observed_form_));
-
     if (!GetParam()) {
       password_save_manager_impl_ =
           std::make_unique<PasswordSaveManagerImpl>(std::move(mock_form_saver));

components/password_manager/core/browser/votes_uploader.cc

@@ -189,11 +189,9 @@ SingleUsernameVoteData::SingleUsernameVoteData(SingleUsernameVoteData&& other) =
 SingleUsernameVoteData::~SingleUsernameVoteData() = default;
 
 VotesUploader::VotesUploader(PasswordManagerClient* client,
-                             bool is_possible_change_password_form,
-                             autofill::FormSignature form_signature)
+                             bool is_possible_change_password_form)
     : client_(client),
-      is_possible_change_password_form_(is_possible_change_password_form),
-      initial_observed_form_signature_(form_signature) {}
+      is_possible_change_password_form_(is_possible_change_password_form) {}
 
 VotesUploader::VotesUploader(const VotesUploader& other) = default;
 VotesUploader::~VotesUploader() = default;
@@ -300,7 +298,6 @@ bool VotesUploader::UploadPasswordVote(
   // re-uses credentials, a vote about the saved form is sent. If the user saves
   // credentials, the observed and pending forms are the same.
   FormStructure form_structure(form_to_upload.form_data);
-  form_structure.set_form_signature(initial_observed_form_signature_);
   form_structure.set_submission_event(submitted_form.submission_event);
 
   ServerFieldTypeSet available_field_types;
@@ -405,7 +402,6 @@ void VotesUploader::UploadFirstLoginVotes(
   }
 
   FormStructure form_structure(form_to_upload.form_data);
-  form_structure.set_form_signature(initial_observed_form_signature_);
   form_structure.set_submission_event(form_to_upload.submission_event);
 
   FieldTypeMap field_types = {

components/password_manager/core/browser/votes_uploader.h

@@ -54,8 +54,7 @@ struct SingleUsernameVoteData {
 class VotesUploader {
  public:
   VotesUploader(PasswordManagerClient* client,
-                bool is_possible_change_password_form,
-                autofill::FormSignature form_signature);
+                bool is_possible_change_password_form);
   VotesUploader(const VotesUploader& other);
   ~VotesUploader();
 
@@ -169,11 +168,6 @@ class VotesUploader {
     single_username_vote_data_.emplace(renderer_id, form_predictions);
   }
 
-  void set_initial_form_signature(
-      autofill::FormSignature observed_form_signature) {
-    initial_observed_form_signature_ = observed_form_signature;
-  }
-
  private:
   // The outcome of the form classifier.
   enum FormClassifierOutcome {
@@ -251,10 +245,6 @@ class VotesUploader {
   // observed form.
   std::map<autofill::FieldRendererId, base::string16> initial_values_;
 
-  // Form signature of the initially observed form. Used to send votes on
-  // dynamic password forms.
-  autofill::FormSignature initial_observed_form_signature_;
-
   base::Optional<SingleUsernameVoteData> single_username_vote_data_;
 };
 

components/password_manager/core/browser/votes_uploader_unittest.cc

@@ -30,7 +30,6 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 using autofill::AutofillDownloadManager;
-using autofill::CalculateFormSignature;
 using autofill::CONFIRMATION_PASSWORD;
 using autofill::FieldSignature;
 using autofill::FormData;
@@ -129,7 +128,6 @@ class VotesUploaderTest : public testing::Test {
       form_to_upload_.form_data.fields.push_back(field);
       submitted_form_.form_data.fields.push_back(field);
     }
-    initial_form_signature_ = CalculateFormSignature(form_to_upload_.form_data);
   }
 
  protected:
@@ -146,11 +144,10 @@ class VotesUploaderTest : public testing::Test {
   PasswordForm submitted_form_;
 
   std::string login_form_signature_ = "123";
-  FormSignature initial_form_signature_;
 };
 
 TEST_F(VotesUploaderTest, UploadPasswordVoteUpdate) {
-  VotesUploader votes_uploader(&client_, true, initial_form_signature_);
+  VotesUploader votes_uploader(&client_, true);
   base::string16 new_password_element = GetFieldNameByIndex(3);
   base::string16 confirmation_element = GetFieldNameByIndex(11);
   form_to_upload_.new_password_element = new_password_element;
@@ -179,7 +176,7 @@ TEST_F(VotesUploaderTest, UploadPasswordVoteUpdate) {
 }
 
 TEST_F(VotesUploaderTest, UploadPasswordVoteSave) {
-  VotesUploader votes_uploader(&client_, false, initial_form_signature_);
+  VotesUploader votes_uploader(&client_, false);
   base::string16 password_element = GetFieldNameByIndex(5);
   base::string16 confirmation_element = GetFieldNameByIndex(12);
   form_to_upload_.password_element = password_element;
@@ -223,8 +220,7 @@ TEST_F(VotesUploaderTest, InitialValueDetection) {
 
   form_data.fields = {other_field, username_field};
 
-  VotesUploader votes_uploader(&client_, true,
-                               CalculateFormSignature(form_data));
+  VotesUploader votes_uploader(&client_, true);
   votes_uploader.StoreInitialFieldValues(form_data);
 
   form_data.fields.at(1).value = ASCIIToUTF16("user entered value");
@@ -250,7 +246,7 @@ TEST_F(VotesUploaderTest, InitialValueDetection) {
 }
 
 TEST_F(VotesUploaderTest, GeneratePasswordAttributesVote) {
-  VotesUploader votes_uploader(&client_, true, initial_form_signature_);
+  VotesUploader votes_uploader(&client_, true);
   // Checks that randomization distorts information about present and missed
   // character classes, but a true value is still restorable with aggregation
   // of many distorted reports.
@@ -318,7 +314,7 @@ TEST_F(VotesUploaderTest, GeneratePasswordAttributesVote) {
 }
 
 TEST_F(VotesUploaderTest, GeneratePasswordSpecialSymbolVote) {
-  VotesUploader votes_uploader(&client_, true, initial_form_signature_);
+  VotesUploader votes_uploader(&client_, true);
 
   const base::string16 password_value = ASCIIToUTF16("password-withsymbols!");
   const int kNumberOfRuns = 2000;
@@ -364,7 +360,7 @@ TEST_F(VotesUploaderTest, GeneratePasswordAttributesVote_OneCharacterPassword) {
   // password has only one character.
   FormData form;
   FormStructure form_structure(form);
-  VotesUploader votes_uploader(&client_, true, CalculateFormSignature(form));
+  VotesUploader votes_uploader(&client_, true);
   votes_uploader.GeneratePasswordAttributesVote(ASCIIToUTF16("1"),
                                                 &form_structure);
   base::Optional<std::pair<PasswordAttribute, bool>> vote =
@@ -377,7 +373,7 @@ TEST_F(VotesUploaderTest, GeneratePasswordAttributesVote_OneCharacterPassword) {
 TEST_F(VotesUploaderTest, GeneratePasswordAttributesVote_AllAsciiCharacters) {
   FormData form;
   FormStructure form_structure(form);
-  VotesUploader votes_uploader(&client_, true, CalculateFormSignature(form));
+  VotesUploader votes_uploader(&client_, true);
   votes_uploader.GeneratePasswordAttributesVote(
       base::UTF8ToUTF16("0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqr"
                         "stuvwxyz!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~"),
@@ -396,7 +392,7 @@ TEST_F(VotesUploaderTest, GeneratePasswordAttributesVote_NonAsciiPassword) {
         "ລະຫັດຜ່ານ-l", "စကားဝှက်ကို3", "პაროლი", "पारण शब्द"}) {
     FormData form;
     FormStructure form_structure(form);
-    VotesUploader votes_uploader(&client_, true, CalculateFormSignature(form));
+    VotesUploader votes_uploader(&client_, true);
     votes_uploader.GeneratePasswordAttributesVote(base::UTF8ToUTF16(password),
                                                   &form_structure);
     base::Optional<std::pair<PasswordAttribute, bool>> vote =
@@ -407,7 +403,7 @@ TEST_F(VotesUploaderTest, GeneratePasswordAttributesVote_NonAsciiPassword) {
 }
 
 TEST_F(VotesUploaderTest, NoSingleUsernameDataNoUpload) {
-  VotesUploader votes_uploader(&client_, false, initial_form_signature_);
+  VotesUploader votes_uploader(&client_, false);
   EXPECT_CALL(mock_autofill_download_manager_,
               StartUploadRequest(_, _, _, _, _, _))
       .Times(0);
@@ -417,6 +413,7 @@ TEST_F(VotesUploaderTest, NoSingleUsernameDataNoUpload) {
 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(_, _))
@@ -428,8 +425,6 @@ TEST_F(VotesUploaderTest, UploadSingleUsername) {
     constexpr FieldSignature kUsernameFieldSignature(1234);
     constexpr FormSignature kFormSignature(1000);
 
-    VotesUploader votes_uploader(&client_, false, kFormSignature);
-
     FormPredictions form_predictions;
     form_predictions.form_signature = kFormSignature;
     // Add a non-username field.
@@ -459,12 +454,11 @@ TEST_F(VotesUploaderTest, UploadSingleUsername) {
 }
 
 TEST_F(VotesUploaderTest, SaveSingleUsernameVote) {
+  VotesUploader votes_uploader(&client_, false);
   constexpr autofill::FieldRendererId kUsernameRendererId(101);
   constexpr autofill::FieldSignature kUsernameFieldSignature(1234);
   constexpr autofill::FormSignature kFormSignature(1000);
 
-  VotesUploader votes_uploader(&client_, false, kFormSignature);
-
   FormPredictions form_predictions;
   form_predictions.form_signature = kFormSignature;
 
@@ -499,12 +493,11 @@ TEST_F(VotesUploaderTest, SaveSingleUsernameVote) {
 }
 
 TEST_F(VotesUploaderTest, DontUploadSingleUsernameWhenAlreadyUploaded) {
+  VotesUploader votes_uploader(&client_, false);
   constexpr autofill::FieldRendererId kUsernameRendererId(101);
   constexpr autofill::FieldSignature kUsernameFieldSignature(1234);
   constexpr autofill::FormSignature kFormSignature(1000);
 
-  VotesUploader votes_uploader(&client_, false, kFormSignature);
-
   MockFieldInfoManager mock_field_manager;
   ON_CALL(client_, GetFieldInfoManager())
       .WillByDefault(Return(&mock_field_manager));
@@ -530,32 +523,4 @@ TEST_F(VotesUploaderTest, DontUploadSingleUsernameWhenAlreadyUploaded) {
   votes_uploader.MaybeSendSingleUsernameVote(true /*credentials_saved*/);
 }
 
-// Tests that if the initial form signature stored by the votes uploader
-// differs from the form signature of the submitted form, upload
-// data should contain the initial form signature.
-TEST_F(VotesUploaderTest, UploadPasswordVoteOnDynamicallyChangedForm) {
-  PasswordForm initial_form;
-  FormData initial_form_data;
-  initial_form_data.name = base::ASCIIToUTF16("some_form");
-  FormFieldData initial_field_data;
-  initial_field_data.name = base::ASCIIToUTF16("some_field");
-  initial_form_data.fields.push_back(initial_field_data);
-  initial_form.form_data = initial_form_data;
-
-  // Create votes uploader and store the initially observed signature.
-  FormSignature initial_signature = CalculateFormSignature(initial_form_data);
-  VotesUploader votes_uploader(&client_, false, initial_signature);
-
-  EXPECT_CALL(
-      mock_autofill_download_manager_,
-      StartUploadRequest(SignatureIsSameAs(initial_form), _, _, _, _, _));
-
-  EXPECT_TRUE(initial_signature.value() !=
-              CalculateFormSignature(form_to_upload_.form_data).value());
-
-  // Upload password votes on a changed form.
-  EXPECT_TRUE(votes_uploader.UploadPasswordVote(
-      form_to_upload_, submitted_form_, PASSWORD, login_form_signature_));
-}
-
 }  // namespace password_manager