[Passwords] Set the in_store field in PasswordStoreChange

This patches make sure the set the |in_store| member of the forms
returned in PasswordStoreChange.
Observers can draw the wrong conclusion about the location of those
forms if the field isn't set properly.

Bug: 1119286
Change-Id: I46d6bc7678f1535418c58cb1df3db151bb263bbf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2416077Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807962}

components/password_manager/core/browser/login_database.cc

@@ -1123,6 +1123,7 @@ PasswordStoreChangeList LoginDatabase::AddLogin(const PasswordForm& form,
   const bool success = s.Run();
   if (success) {
     // If success, the row never existed so password was not changed.
+    FillFormInStore(&form_with_encrypted_password);
     list.emplace_back(PasswordStoreChange::ADD,
                       std::move(form_with_encrypted_password),
                       db_.GetLastInsertRowId(),
@@ -1141,8 +1142,11 @@ PasswordStoreChangeList LoginDatabase::AddLogin(const PasswordForm& form,
       db_.GetCachedStatement(SQL_FROM_HERE, add_replace_statement_.c_str()));
   BindAddStatement(form_with_encrypted_password, &s);
   if (s.Run()) {
-    list.emplace_back(PasswordStoreChange::REMOVE, form,
+    PasswordForm removed_form = form;
+    FillFormInStore(&removed_form);
+    list.emplace_back(PasswordStoreChange::REMOVE, removed_form,
                       old_primary_key_password.primary_key);
+    FillFormInStore(&form_with_encrypted_password);
     list.emplace_back(PasswordStoreChange::ADD,
                       std::move(form_with_encrypted_password),
                       db_.GetLastInsertRowId(), password_changed);
@@ -1238,6 +1242,7 @@ PasswordStoreChangeList LoginDatabase::UpdateLogin(const PasswordForm& form,
         form.password_value != old_primary_key_password.decrypted_password;
     PasswordForm form_with_encrypted_password = form;
     form_with_encrypted_password.encrypted_password = encrypted_password;
+    FillFormInStore(&form_with_encrypted_password);
     list.emplace_back(PasswordStoreChange::UPDATE,
                       std::move(form_with_encrypted_password),
                       old_primary_key_password.primary_key, password_changed);
@@ -1274,7 +1279,9 @@ bool LoginDatabase::RemoveLogin(const PasswordForm& form,
     return false;
   }
   if (changes) {
-    changes->emplace_back(PasswordStoreChange::REMOVE, form,
+    PasswordForm removed_form = form;
+    FillFormInStore(&removed_form);
+    changes->emplace_back(PasswordStoreChange::REMOVE, removed_form,
                           old_primary_key_password.primary_key,
                           /*password_changed=*/true);
   }
@@ -1311,6 +1318,7 @@ bool LoginDatabase::RemoveLoginByPrimaryKey(int primary_key,
     return false;
   }
   if (changes) {
+    FillFormInStore(&form);
     changes->emplace_back(PasswordStoreChange::REMOVE, std::move(form),
                           primary_key, /*password_changed=*/true);
   }
@@ -1963,9 +1971,8 @@ FormRetrievalResult LoginDatabase::StatementToForms(
   key_to_form_map->clear();
   while (statement->Step()) {
     auto new_form = std::make_unique<PasswordForm>();
-    new_form->in_store = is_account_store()
-                             ? PasswordForm::Store::kAccountStore
-                             : PasswordForm::Store::kProfileStore;
+    FillFormInStore(new_form.get());
+
     int primary_key = -1;
     EncryptionResult result = InitPasswordFormFromStatement(
         *statement, /*decrypt_and_fill_password_value=*/true, &primary_key,
@@ -2080,4 +2087,9 @@ void LoginDatabase::InitializeStatementStrings(const SQLTableBuilder& builder) {
                                all_unique_key_column_names;
 }
 
+void LoginDatabase::FillFormInStore(PasswordForm* form) const {
+  form->in_store = is_account_store() ? PasswordForm::Store::kAccountStore
+                                      : PasswordForm::Store::kProfileStore;
+}
+
 }  // namespace password_manager

components/password_manager/core/browser/login_database.h

@@ -329,6 +329,10 @@ class LoginDatabase : public PasswordStoreSync::MetadataStore {
   // fragments based on |builder|.
   void InitializeStatementStrings(const SQLTableBuilder& builder);
 
+  // Sets the `in_store` member of `form` to either kProfileStore or
+  // kAccountStore depending on the value of `is_account_store_`.
+  void FillFormInStore(autofill::PasswordForm* form) const;
+
   const base::FilePath db_path_;
   const IsAccountStore is_account_store_;
 

components/password_manager/core/browser/login_database_unittest.cc

@@ -2607,4 +2607,34 @@ TEST_F(LoginDatabaseTest, GetLoginsEncryptedPassword) {
   ASSERT_FALSE(forms[0]->encrypted_password.empty());
 }
 
+class LoginDatabaseForAccountStoreTest : public testing::Test {
+ protected:
+  void SetUp() override {
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    file_ = temp_dir_.GetPath().AppendASCII("TestMetadataStoreMacDatabase");
+    OSCryptMocker::SetUp();
+
+    db_ = std::make_unique<LoginDatabase>(file_, IsAccountStore(true));
+    ASSERT_TRUE(db_->Init());
+  }
+
+  void TearDown() override { OSCryptMocker::TearDown(); }
+
+  LoginDatabase& db() { return *db_; }
+
+  base::ScopedTempDir temp_dir_;
+  base::FilePath file_;
+  std::unique_ptr<LoginDatabase> db_;
+  base::test::TaskEnvironment task_environment_;
+};
+
+TEST_F(LoginDatabaseForAccountStoreTest, AddLogins) {
+  PasswordForm form;
+  GenerateExamplePasswordForm(&form);
+
+  PasswordStoreChangeList changes = db().AddLogin(form);
+  ASSERT_EQ(1U, changes.size());
+  EXPECT_EQ(PasswordForm::Store::kAccountStore, changes[0].form().in_store);
+}
+
 }  // namespace password_manager