Password Login Database: report correct changes from AddLogin().

components/password_manager/core/browser/login_database_unittest.cc

Index: components/password_manager/core/browser/login_database_unittest.cc
diff --git a/components/password_manager/core/browser/login_database_unittest.cc b/components/password_manager/core/browser/login_database_unittest.cc
index 7a4840e1e87c57dfc93ef12848aff30b574365da..1039746f9cdfba080edbb429fc2050c9e70ebe7f 100644
--- a/components/password_manager/core/browser/login_database_unittest.cc
+++ b/components/password_manager/core/browser/login_database_unittest.cc
@@ -22,6 +22,18 @@ using base::ASCIIToUTF16;
 using ::testing::Eq;
 
 namespace password_manager {
+namespace {
+PasswordStoreChangeList AddChangeForForm(const PasswordForm& form) {
+  return PasswordStoreChangeList(1,
+                                 PasswordStoreChange(PasswordStoreChange::ADD,
+                                                     form));
+}
+
+}  // namespace
+
+// Serialization routines for vectors implemented in login_database.cc.
+Pickle SerializeVector(const std::vector<base::string16>& vec);
+std::vector<base::string16> DeserializeVector(const Pickle& pickle);
 
 class LoginDatabaseTest : public testing::Test {
  protected:
@@ -32,14 +44,6 @@ class LoginDatabaseTest : public testing::Test {
     ASSERT_TRUE(db_.Init(file_));
   }
 
-  Pickle SerializeVector(const std::vector<base::string16>& vec) const {
-    return db_.SerializeVector(vec);
-  }
-
-  std::vector<base::string16> DeserializeVector(const Pickle& pickle) const {
-    return db_.DeserializeVector(pickle);
-  }
-
   void FormsAreEqual(const PasswordForm& expected, const PasswordForm& actual) {
     PasswordForm expected_copy(expected);
 #if defined(OS_MACOSX) && !defined(OS_IOS)
@@ -50,7 +54,7 @@ class LoginDatabaseTest : public testing::Test {
   }
 
   void TestNonHTMLFormPSLMatching(const PasswordForm::Scheme& scheme) {
-    std::vector<PasswordForm*> result;
+    ScopedVector<PasswordForm> result;
 
     base::Time now = base::Time::Now();
 
@@ -61,6 +65,7 @@ class LoginDatabaseTest : public testing::Test {
     non_html_auth.password_value = ASCIIToUTF16("test");
     non_html_auth.signon_realm = "http://example.com/Realm";
     non_html_auth.scheme = scheme;
+    non_html_auth.date_created = now;
 
     // Simple password form.
     PasswordForm html_form(non_html_auth);
@@ -71,14 +76,13 @@ class LoginDatabaseTest : public testing::Test {
     html_form.submit_element = ASCIIToUTF16("");
     html_form.signon_realm = "http://example.com/";
     html_form.scheme = PasswordForm::SCHEME_HTML;
+    html_form.date_created = now;
 
     // Add them and make sure it is there.
-    EXPECT_TRUE(db_.AddLogin(non_html_auth));
-    EXPECT_TRUE(db_.AddLogin(html_form));
-    EXPECT_TRUE(db_.GetAutofillableLogins(&result));
+    EXPECT_EQ(AddChangeForForm(non_html_auth), db_.AddLogin(non_html_auth));
+    EXPECT_EQ(AddChangeForForm(html_form), db_.AddLogin(html_form));
+    EXPECT_TRUE(db_.GetAutofillableLogins(&result.get()));
     EXPECT_EQ(2U, result.size());
-    delete result[0];
-    delete result[1];
     result.clear();
 
     PasswordForm second_non_html_auth(non_html_auth);
@@ -86,7 +90,7 @@ class LoginDatabaseTest : public testing::Test {
     second_non_html_auth.signon_realm = "http://second.example.com/Realm";
 
     // This shouldn't match anything.
-    EXPECT_TRUE(db_.GetLogins(second_non_html_auth, &result));
+    EXPECT_TRUE(db_.GetLogins(second_non_html_auth, &result.get()));
     EXPECT_EQ(0U, result.size());
 
     // Clear state.
@@ -124,7 +128,7 @@ TEST_F(LoginDatabaseTest, Logins) {
 
   // Add it and make sure it is there and that all the fields were retrieved
   // correctly.
-  EXPECT_TRUE(db_.AddLogin(form));
+  EXPECT_EQ(AddChangeForForm(form), db_.AddLogin(form));
   EXPECT_TRUE(db_.GetAutofillableLogins(&result));
   EXPECT_EQ(1U, result.size());
   FormsAreEqual(form, *result[0]);
@@ -168,7 +172,7 @@ TEST_F(LoginDatabaseTest, Logins) {
   EXPECT_EQ(0U, result.size());
 
   // Let's imagine the user logs into the secure site.
-  EXPECT_TRUE(db_.AddLogin(form4));
+  EXPECT_EQ(AddChangeForForm(form4), db_.AddLogin(form4));
   EXPECT_TRUE(db_.GetAutofillableLogins(&result));
   EXPECT_EQ(2U, result.size());
   delete result[0];
@@ -262,7 +266,7 @@ TEST_F(LoginDatabaseTest, TestPublicSuffixDomainMatching) {
   form.scheme = PasswordForm::SCHEME_HTML;
 
   // Add it and make sure it is there.
-  EXPECT_TRUE(db_.AddLogin(form));
+  EXPECT_EQ(AddChangeForForm(form), db_.AddLogin(form));
   EXPECT_TRUE(db_.GetAutofillableLogins(&result));
   EXPECT_EQ(1U, result.size());
   delete result[0];
@@ -320,7 +324,7 @@ TEST_F(LoginDatabaseTest, TestPublicSuffixDomainMatchingShouldMatchingApply) {
   form.scheme = PasswordForm::SCHEME_HTML;
 
   // Add it and make sure it is there.
-  EXPECT_TRUE(db_.AddLogin(form));
+  EXPECT_EQ(AddChangeForForm(form), db_.AddLogin(form));
   EXPECT_TRUE(db_.GetAutofillableLogins(&result));
   EXPECT_EQ(1U, result.size());
   delete result[0];
@@ -370,7 +374,7 @@ TEST_F(LoginDatabaseTest, TestPublicSuffixDomainMatchingDifferentSites) {
   form.scheme = PasswordForm::SCHEME_HTML;
 
   // Add it and make sure it is there.
-  EXPECT_TRUE(db_.AddLogin(form));
+  EXPECT_EQ(AddChangeForForm(form), db_.AddLogin(form));
   EXPECT_TRUE(db_.GetAutofillableLogins(&result));
   EXPECT_EQ(1U, result.size());
   delete result[0];
@@ -410,7 +414,7 @@ TEST_F(LoginDatabaseTest, TestPublicSuffixDomainMatchingDifferentSites) {
   form.scheme = PasswordForm::SCHEME_HTML;
 
   // Add it and make sure it is there.
-  EXPECT_TRUE(db_.AddLogin(form));
+  EXPECT_EQ(AddChangeForForm(form), db_.AddLogin(form));
   EXPECT_TRUE(db_.GetAutofillableLogins(&result));
   EXPECT_EQ(2U, result.size());
   delete result[0];
@@ -464,7 +468,7 @@ TEST_F(LoginDatabaseTest, TestPublicSuffixDomainMatchingRegexp) {
   form.scheme = PasswordForm::SCHEME_HTML;
 
   // Add it and make sure it is there.
-  EXPECT_TRUE(db_.AddLogin(form));
+  EXPECT_EQ(AddChangeForForm(form), db_.AddLogin(form));
   EXPECT_TRUE(db_.GetAutofillableLogins(&result));
   EXPECT_EQ(1U, result.size());
   delete result[0];
@@ -475,7 +479,7 @@ TEST_F(LoginDatabaseTest, TestPublicSuffixDomainMatchingRegexp) {
       GetFormWithNewSignonRealm(form, "http://www.foo-bar.com/");
 
   // Add it and make sure it is there.
-  EXPECT_TRUE(db_.AddLogin(form_dash));
+  EXPECT_EQ(AddChangeForForm(form_dash), db_.AddLogin(form_dash));
   EXPECT_TRUE(db_.GetAutofillableLogins(&result));
   EXPECT_EQ(2U, result.size());
   delete result[0];
@@ -575,7 +579,7 @@ static bool AddTimestampedLogin(LoginDatabase* db, std::string url,
   form.submit_element = ASCIIToUTF16("signIn");
   form.signon_realm = url;
   form.date_created = time;
-  return db->AddLogin(form);
+  return db->AddLogin(form) == AddChangeForForm(form);
 }
 
 static void ClearResults(std::vector<PasswordForm*>* results) {
@@ -647,7 +651,7 @@ TEST_F(LoginDatabaseTest, BlacklistedLogins) {
   form.preferred = true;
   form.blacklisted_by_user = true;
   form.scheme = PasswordForm::SCHEME_HTML;
-  EXPECT_TRUE(db_.AddLogin(form));
+  EXPECT_EQ(AddChangeForForm(form), db_.AddLogin(form));
 
   // Get all non-blacklisted logins (should be none).
   EXPECT_TRUE(db_.GetAutofillableLogins(&result));
@@ -700,7 +704,7 @@ TEST_F(LoginDatabaseTest, UpdateIncompleteCredentials) {
   incomplete_form.preferred = true;
   incomplete_form.blacklisted_by_user = false;
   incomplete_form.scheme = PasswordForm::SCHEME_HTML;
-  EXPECT_TRUE(db_.AddLogin(incomplete_form));
+  EXPECT_EQ(AddChangeForForm(incomplete_form), db_.AddLogin(incomplete_form));
 
   // A form on some website. It should trigger a match with the stored one.
   PasswordForm encountered_form;
@@ -773,7 +777,7 @@ TEST_F(LoginDatabaseTest, UpdateOverlappingCredentials) {
   incomplete_form.preferred = true;
   incomplete_form.blacklisted_by_user = false;
   incomplete_form.scheme = PasswordForm::SCHEME_HTML;
-  EXPECT_TRUE(db_.AddLogin(incomplete_form));
+  EXPECT_EQ(AddChangeForForm(incomplete_form), db_.AddLogin(incomplete_form));
 
   // Save a complete version of the previous form. Both forms could exist if
   // the user created the complete version before importing the incomplete
@@ -783,7 +787,7 @@ TEST_F(LoginDatabaseTest, UpdateOverlappingCredentials) {
   complete_form.username_element = ASCIIToUTF16("username_element");
   complete_form.password_element = ASCIIToUTF16("password_element");
   complete_form.submit_element = ASCIIToUTF16("submit");
-  EXPECT_TRUE(db_.AddLogin(complete_form));
+  EXPECT_EQ(AddChangeForForm(complete_form), db_.AddLogin(complete_form));
 
   // Make sure both passwords exist.
   ScopedVector<autofill::PasswordForm> result;
@@ -808,6 +812,26 @@ TEST_F(LoginDatabaseTest, UpdateOverlappingCredentials) {
   EXPECT_EQ(expected_form, *result[0]);
 }
 
+TEST_F(LoginDatabaseTest, DoubleAdd) {
+  PasswordForm form;
+  form.origin = GURL("http://accounts.google.com/LoginAuth");
+  form.signon_realm = "http://accounts.google.com/";
+  form.username_value = ASCIIToUTF16("my_username");
+  form.password_value = ASCIIToUTF16("my_password");
+  form.ssl_valid = false;
+  form.preferred = true;
+  form.blacklisted_by_user = false;
+  form.scheme = PasswordForm::SCHEME_HTML;
+  EXPECT_EQ(AddChangeForForm(form), db_.AddLogin(form));
+
+  // Add almost the same form again.
+  form.times_used++;
+  PasswordStoreChangeList list;
+  list.push_back(PasswordStoreChange(PasswordStoreChange::REMOVE, form));
+  list.push_back(PasswordStoreChange(PasswordStoreChange::ADD, form));
+  EXPECT_EQ(list, db_.AddLogin(form));
+}
+
 #if defined(OS_POSIX)
 // Only the current user has permission to read the database.
 //