Adding Parent Key to CompromisedCredentials

This change will be used by sync to match Compromised Credentials with
PasswordForms when
PasswordStore::InvokeAndNotifyAboutCompromisedPasswordsChange called.

Bug: 1137775
Change-Id: I281bed54093f99f56b7ccdbddc263b07ee33dd35
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2612973
Commit-Queue: Viktor Semeniuk <vsemeniuk@google.com>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842505}

components/password_manager/core/browser/insecure_credentials_table.cc

@@ -19,16 +19,19 @@ std::vector<CompromisedCredentials> StatementToCompromisedCredentials(
     sql::Statement* s) {
   std::vector<CompromisedCredentials> results;
   while (s->Step()) {
-    std::string signon_realm = s->ColumnString(0);
-    base::string16 username = s->ColumnString16(1);
+    int parent_key = s->ColumnInt64(0);
+    std::string signon_realm = s->ColumnString(1);
+    base::string16 username = s->ColumnString16(2);
     CompromiseType insecurity_type =
-        static_cast<CompromiseType>(s->ColumnInt64(2));
+        static_cast<CompromiseType>(s->ColumnInt64(3));
     base::Time create_time = base::Time::FromDeltaSinceWindowsEpoch(
-        (base::TimeDelta::FromMicroseconds(s->ColumnInt64(3))));
-    bool is_muted = !!s->ColumnInt64(4);
-
-    results.emplace_back(std::move(signon_realm), std::move(username),
-                         create_time, insecurity_type, IsMuted(is_muted));
+        (base::TimeDelta::FromMicroseconds(s->ColumnInt64(4))));
+    bool is_muted = !!s->ColumnInt64(5);
+    CompromisedCredentials issue(std::move(signon_realm), std::move(username),
+                                 create_time, insecurity_type,
+                                 IsMuted(is_muted));
+    issue.parent_key = FormPrimaryKey(parent_key);
+    results.emplace_back(std::move(issue));
   }
   return results;
 }
@@ -163,7 +166,7 @@ std::vector<CompromisedCredentials> InsecureCredentialsTable::GetRows(
 
   sql::Statement s(db_->GetCachedStatement(
       SQL_FROM_HERE,
-      base::StringPrintf("SELECT signon_realm, username_value, "
+      base::StringPrintf("SELECT parent_id, signon_realm, username_value, "
                          "insecurity_type, create_time, is_muted FROM %s "
                          "INNER JOIN logins ON parent_id = logins.id "
                          "WHERE signon_realm = ? ",
@@ -180,7 +183,7 @@ std::vector<CompromisedCredentials> InsecureCredentialsTable::GetRows(
 
   sql::Statement s(db_->GetCachedStatement(
       SQL_FROM_HERE,
-      base::StringPrintf("SELECT signon_realm, username_value, "
+      base::StringPrintf("SELECT parent_id, signon_realm, username_value, "
                          "insecurity_type, create_time, is_muted FROM %s "
                          "INNER JOIN logins ON parent_id = logins.id "
                          "WHERE parent_id = ? ",
@@ -258,7 +261,7 @@ std::vector<CompromisedCredentials> InsecureCredentialsTable::GetAllRows() {
 
   sql::Statement s(db_->GetCachedStatement(
       SQL_FROM_HERE,
-      base::StringPrintf("SELECT signon_realm, username_value, "
+      base::StringPrintf("SELECT parent_id, signon_realm, username_value, "
                          "insecurity_type, create_time, is_muted FROM %s "
                          "INNER JOIN logins ON parent_id = logins.id",
                          kTableName)

components/password_manager/core/browser/insecure_credentials_table.h

@@ -60,6 +60,8 @@ struct CompromisedCredentials {
   CompromisedCredentials& operator=(CompromisedCredentials&& rhs);
   ~CompromisedCredentials();
 
+  // The primary key of an affected Login.
+  FormPrimaryKey parent_key{-1};
   // The signon_realm of the website where the credentials were compromised.
   std::string signon_realm;
   // The value of the compromised username.

components/password_manager/core/browser/insecure_credentials_table_unittest.cc

@@ -65,6 +65,15 @@ class InsecureCredentialsTableTest : public testing::Test {
     ASSERT_TRUE(login_db_->Init());
   }
 
+  std::vector<int> GetParentIds(
+      base::span<const CompromisedCredentials> credentials) {
+    std::vector<int> ids;
+    ids.reserve(credentials.size());
+    for (const auto& credential : credentials)
+      ids.push_back(credential.parent_key.value());
+    return ids;
+  }
+
   CompromisedCredentials& test_data() { return test_data_; }
   PasswordForm& test_form() { return test_form_; }
   InsecureCredentialsTable* db() {
@@ -118,6 +127,7 @@ TEST_F(InsecureCredentialsTableTest, SameSignonRealmDifferentUsername) {
               ElementsAre(compromised_credentials1, compromised_credentials2));
   EXPECT_THAT(db()->GetRows(test_data().signon_realm),
               ElementsAre(compromised_credentials1, compromised_credentials2));
+  EXPECT_THAT(GetParentIds(db()->GetAllRows()), ElementsAre(1, 2));
 }
 
 TEST_F(InsecureCredentialsTableTest, SameUsernameDifferentSignonRealm) {
@@ -135,6 +145,7 @@ TEST_F(InsecureCredentialsTableTest, SameUsernameDifferentSignonRealm) {
               ElementsAre(compromised_credentials1, compromised_credentials2));
   EXPECT_THAT(db()->GetRows(test_data().signon_realm),
               ElementsAre(compromised_credentials1));
+  EXPECT_THAT(GetParentIds(db()->GetAllRows()), ElementsAre(1, 2));
 }
 
 TEST_F(InsecureCredentialsTableTest, SameSignonRealmAndUsernameDifferentTime) {
@@ -147,6 +158,7 @@ TEST_F(InsecureCredentialsTableTest, SameSignonRealmAndUsernameDifferentTime) {
   // It should return false because of unique constraints.
   EXPECT_FALSE(db()->AddRow(compromised_credentials2));
   EXPECT_THAT(db()->GetAllRows(), ElementsAre(compromised_credentials1));
+  EXPECT_THAT(GetParentIds(db()->GetAllRows()), ElementsAre(1));
 }
 
 TEST_F(InsecureCredentialsTableTest,
@@ -167,6 +179,7 @@ TEST_F(InsecureCredentialsTableTest,
   EXPECT_THAT(db()->GetRows(test_data().signon_realm),
               ElementsAre(compromised_credentials1, compromised_credentials2,
                           compromised_credentials3));
+  EXPECT_THAT(GetParentIds(db()->GetAllRows()), ElementsAre(1, 1, 1));
 }
 
 TEST_F(InsecureCredentialsTableTest, RemoveRow) {
@@ -174,6 +187,7 @@ TEST_F(InsecureCredentialsTableTest, RemoveRow) {
   EXPECT_TRUE(db()->AddRow(test_data()));
   EXPECT_THAT(db()->GetRows(test_data().signon_realm),
               ElementsAre(test_data()));
+  EXPECT_THAT(GetParentIds(db()->GetAllRows()), ElementsAre(1));
 
   EXPECT_TRUE(db()->RemoveRow(test_data().signon_realm, test_data().username,
                               RemoveCompromisedCredentialsReason::kUpdate));
@@ -204,6 +218,7 @@ TEST_F(InsecureCredentialsTableTest, RemoveRowsCreatedBetween) {
   EXPECT_THAT(db()->GetAllRows(),
               ElementsAre(compromised_credentials1, compromised_credentials2,
                           compromised_credentials3));
+  EXPECT_THAT(GetParentIds(db()->GetAllRows()), ElementsAre(1, 2, 3));
 
   EXPECT_TRUE(db()->RemoveRowsByUrlAndTime(base::NullCallback(),
                                            base::Time::FromTimeT(15),
@@ -366,6 +381,7 @@ TEST_F(InsecureCredentialsTableTest, GetAllRowsWithId) {
   EXPECT_THAT(
       db()->GetRows(FormPrimaryKey(1)),
       UnorderedElementsAre(compromised_credentials1, compromised_credentials2));
+  EXPECT_THAT(GetParentIds(db()->GetAllRows()), ElementsAre(1, 1));
 
   test_form().username_value = base::ASCIIToUTF16(kUsername2);
   test_data().username = test_form().username_value;
@@ -377,6 +393,7 @@ TEST_F(InsecureCredentialsTableTest, GetAllRowsWithId) {
   EXPECT_TRUE(db()->AddRow(compromised_credentials1));
   EXPECT_THAT(db()->GetRows(FormPrimaryKey(2)),
               UnorderedElementsAre(compromised_credentials1));
+  EXPECT_THAT(GetParentIds(db()->GetAllRows()), ElementsAre(1, 1, 2));
 }
 
 }  // namespace