Clear non needed data from FormData before saving.

That's good both for privacy and storage reasons.

Bug: 831123

Change-Id: Ie41b6d1f25fe37f86aea9eb4625d8aa878e0a52e
Reviewed-on: https://chromium-review.googlesource.com/1249205
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595055}

components/password_manager/core/browser/form_saver_impl.cc

@@ -16,10 +16,32 @@
 #include "url/gurl.h"
 #include "url/origin.h"
 
+using autofill::FormData;
+using autofill::FormFieldData;
 using autofill::PasswordForm;
 
 namespace password_manager {
 
+namespace {
+
+// Remove all information from |form| that is not required for signature
+// calculation.
+void SanitizeFormData(FormData* form) {
+  form->main_frame_origin = url::Origin();
+  for (FormFieldData& field : form->fields) {
+    field.label.clear();
+    field.value.clear();
+    field.autocomplete_attribute.clear();
+    field.option_values.clear();
+    field.option_contents.clear();
+    field.placeholder.clear();
+    field.css_classes.clear();
+    field.id.clear();
+  }
+}
+
+}  // namespace
+
 FormSaverImpl::FormSaverImpl(PasswordStore* store) : store_(store) {
   DCHECK(store);
 }
@@ -53,11 +75,13 @@ void FormSaverImpl::Update(
 }
 
 void FormSaverImpl::PresaveGeneratedPassword(const PasswordForm& generated) {
+  auto form = std::make_unique<PasswordForm>(generated);
+  SanitizeFormData(&form->form_data);
   if (presaved_)
-    store_->UpdateLoginWithPrimaryKey(generated, *presaved_);
+    store_->UpdateLoginWithPrimaryKey(*form, *presaved_);
   else
-    store_->AddLogin(generated);
-  presaved_.reset(new PasswordForm(generated));
+    store_->AddLogin(*form);
+  presaved_ = std::move(form);
 }
 
 void FormSaverImpl::RemovePresavedPassword() {
@@ -72,7 +96,7 @@ std::unique_ptr<FormSaver> FormSaverImpl::Clone() {
   auto result = std::make_unique<FormSaverImpl>(store_);
   if (presaved_)
     result->presaved_ = std::make_unique<PasswordForm>(*presaved_);
-  return std::move(result);
+  return result;
 }
 
 void FormSaverImpl::SaveImpl(
@@ -85,18 +109,20 @@ void FormSaverImpl::SaveImpl(
   DCHECK(!pending.blacklisted_by_user);
 
   UpdatePreferredLoginState(pending.username_value, best_matches);
+  PasswordForm sanitized_pending(pending);
+  SanitizeFormData(&sanitized_pending.form_data);
   if (presaved_) {
-    store_->UpdateLoginWithPrimaryKey(pending, *presaved_);
+    store_->UpdateLoginWithPrimaryKey(sanitized_pending, *presaved_);
     presaved_ = nullptr;
   } else if (is_new_login) {
-    store_->AddLogin(pending);
-    if (!pending.username_value.empty())
-      DeleteEmptyUsernameCredentials(pending, best_matches);
+    store_->AddLogin(sanitized_pending);
+    if (!sanitized_pending.username_value.empty())
+      DeleteEmptyUsernameCredentials(sanitized_pending, best_matches);
   } else {
     if (old_primary_key)
-      store_->UpdateLoginWithPrimaryKey(pending, *old_primary_key);
+      store_->UpdateLoginWithPrimaryKey(sanitized_pending, *old_primary_key);
     else
-      store_->UpdateLogin(pending);
+      store_->UpdateLogin(sanitized_pending);
   }
 
   if (credentials_to_update) {
@@ -115,6 +141,7 @@ void FormSaverImpl::UpdatePreferredLoginState(
         form.username_value != preferred_username) {
       // This wasn't the selected login but it used to be preferred.
       PasswordForm update(form);
+      SanitizeFormData(&update.form_data);
       update.preferred = false;
       store_->UpdateLogin(update);
     }

components/password_manager/core/browser/form_saver_impl_unittest.cc

@@ -20,6 +20,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
+using autofill::FormFieldData;
 using autofill::PasswordForm;
 using base::ASCIIToUTF16;
 using base::StringPiece;
@@ -530,4 +531,37 @@ TEST_F(FormSaverImplTest, PresaveGeneratedPassword_CloneSurvives) {
   clone->PresaveGeneratedPassword(generated);
 }
 
+// Check that on saving the pending form |form_data| is sanitized.
+TEST_F(FormSaverImplTest, FormDataSanitized) {
+  PasswordForm pending = CreatePending("nameofuser", "wordToP4a55");
+  FormFieldData field;
+  field.name = ASCIIToUTF16("name");
+  field.form_control_type = "password";
+  field.value = ASCIIToUTF16("value");
+  field.label = ASCIIToUTF16("label");
+  field.placeholder = ASCIIToUTF16("placeholder");
+  field.id = ASCIIToUTF16("id");
+  field.css_classes = ASCIIToUTF16("css_classes");
+  pending.form_data.fields.push_back(field);
+
+  for (bool presave : {false, true}) {
+    PasswordForm saved;
+    EXPECT_CALL(*mock_store_, AddLogin(_)).WillOnce(SaveArg<0>(&saved));
+    if (presave)
+      form_saver_.PresaveGeneratedPassword(pending);
+    else
+      form_saver_.Save(pending, {});
+
+    ASSERT_EQ(1u, saved.form_data.fields.size());
+    const FormFieldData& saved_field = saved.form_data.fields[0];
+    EXPECT_EQ(ASCIIToUTF16("name"), saved_field.name);
+    EXPECT_EQ("password", saved_field.form_control_type);
+    EXPECT_TRUE(saved_field.value.empty());
+    EXPECT_TRUE(saved_field.label.empty());
+    EXPECT_TRUE(saved_field.placeholder.empty());
+    EXPECT_TRUE(saved_field.id.empty());
+    EXPECT_TRUE(saved_field.css_classes.empty());
+  }
+}
+
 }  // namespace password_manager

components/password_manager/core/browser/password_manager_unittest.cc

@@ -19,6 +19,8 @@
 #include "base/test/scoped_feature_list.h"
 #include "base/test/test_mock_time_task_runner.h"
 #include "build/build_config.h"
+#include "components/autofill/core/common/form_data.h"
+#include "components/autofill/core/common/form_field_data.h"
 #include "components/password_manager/core/browser/form_fetcher_impl.h"
 #include "components/password_manager/core/browser/mock_password_store.h"
 #include "components/password_manager/core/browser/new_password_form_manager.h"
@@ -39,6 +41,8 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using autofill::FormData;
+using autofill::FormFieldData;
 using autofill::PasswordForm;
 using base::ASCIIToUTF16;
 using base::TestMockTimeTaskRunner;
@@ -149,6 +153,20 @@ ACTION(InvokeEmptyConsumerWithForms) {
 
 ACTION_P(SaveToScopedPtr, scoped) { scoped->reset(arg0); }
 
+void SanitizeFormData(FormData* form) {
+  form->main_frame_origin = url::Origin();
+  for (FormFieldData& field : form->fields) {
+    field.label.clear();
+    field.value.clear();
+    field.autocomplete_attribute.clear();
+    field.option_values.clear();
+    field.option_contents.clear();
+    field.placeholder.clear();
+    field.css_classes.clear();
+    field.id.clear();
+  }
+}
+
 }  // namespace
 
 class PasswordManagerTest : public testing::Test {
@@ -1781,20 +1799,26 @@ TEST_F(PasswordManagerTest, PasswordGenerationPresavePassword) {
 
   // The user accepts a generated password.
   form.password_value = base::ASCIIToUTF16("password");
-  EXPECT_CALL(*store_, AddLogin(form)).WillOnce(Return());
+  PasswordForm sanitized_form(form);
+  SanitizeFormData(&sanitized_form.form_data);
+
+  EXPECT_CALL(*store_, AddLogin(sanitized_form)).WillOnce(Return());
   manager()->OnPresaveGeneratedPassword(form);
 
   // The user updates the generated password.
   PasswordForm updated_form(form);
   updated_form.password_value = base::ASCIIToUTF16("password_12345");
-  EXPECT_CALL(*store_, UpdateLoginWithPrimaryKey(updated_form, form))
+  PasswordForm sanitized_updated_form(updated_form);
+  SanitizeFormData(&sanitized_updated_form.form_data);
+  EXPECT_CALL(*store_,
+              UpdateLoginWithPrimaryKey(sanitized_updated_form, sanitized_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());
+  EXPECT_CALL(*store_, RemoveLogin(sanitized_updated_form)).WillOnce(Return());
   manager()->OnPasswordNoLongerGenerated(updated_form);
 }
 
@@ -1830,6 +1854,7 @@ TEST_F(PasswordManagerTest, PasswordGenerationPresavePasswordAndLogin) {
     SCOPED_TRACE(testing::Message("found_matched_logins_in_store = ")
                  << found_matched_logins_in_store);
     PasswordForm form(MakeFormWithOnlyNewPasswordField());
+    SanitizeFormData(&form.form_data);
     std::vector<PasswordForm> observed = {form};
     if (found_matched_logins_in_store) {
       EXPECT_CALL(*store_, GetLogins(_, _))