Differentiate between ADD and CHANGED when adding new autofill entries.

Adding a new autofill entry can either result in an ADD (when the name/value par was never seen before) or with an UPDATE (when the name/value pair already exists).

Review URL: http://codereview.chromium.org/507053

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@35108 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 97 insertions, 34 deletions

diff --git a/chrome/browser/webdata/web_data_service.cc b/chrome/browser/webdata/web_data_service.cc
index 36d8834..4407cdd 100644
--- a/chrome/browser/webdata/web_data_service.cc
+++ b/chrome/browser/webdata/web_data_service.cc
@@ -634,17 +634,9 @@ void WebDataService::AddFormFieldValuesImpl(
   InitializeDatabaseIfNecessary();
   const std::vector<FormField>& form_fields = request->GetArgument();
   if (db_ && !request->IsCancelled()) {
-    if (!db_->AddFormFieldValues(form_fields))
-      NOTREACHED();
-
     AutofillChangeList changes;
-    for (std::vector<FormField>::const_iterator itr = form_fields.begin();
-         itr != form_fields.end();
-         itr++) {
-      changes.push_back(
-          AutofillChange(AutofillChange::ADD,
-                         AutofillKey(itr->name(), itr->value())));
-    }
+    if (!db_->AddFormFieldValues(form_fields, &changes))
+      NOTREACHED();
     request->SetResult(
         new WDResult<AutofillChangeList>(AUTOFILL_CHANGES, changes));
     ScheduleCommit();
diff --git a/chrome/browser/webdata/web_database.cc b/chrome/browser/webdata/web_database.cc
index edeb0d6..2ecf827 100644
--- a/chrome/browser/webdata/web_database.cc
+++ b/chrome/browser/webdata/web_database.cc
@@ -804,18 +804,20 @@ bool WebDatabase::GetAllLogins(std::vector<PasswordForm*>* forms,
   return s.Succeeded();
 }
 
-bool WebDatabase::AddFormFieldValues(const std::vector<FormField>& elements) {
-  return AddFormFieldValuesTime(elements, Time::Now());
+bool WebDatabase::AddFormFieldValues(const std::vector<FormField>& elements,
+                                     std::vector<AutofillChange>* changes) {
+  return AddFormFieldValuesTime(elements, changes, Time::Now());
 }
 
 bool WebDatabase::AddFormFieldValuesTime(const std::vector<FormField>& elements,
+                                         std::vector<AutofillChange>* changes,
                                          base::Time time) {
   bool result = true;
   for (std::vector<FormField>::const_iterator
        itr = elements.begin();
        itr != elements.end();
        itr++) {
-    result = result && AddFormFieldValueTime(*itr, time);
+    result = result && AddFormFieldValueTime(*itr, changes, time);
   }
   return result;
 }
@@ -945,11 +947,13 @@ bool WebDatabase::SetCountOfFormElement(int64 pair_id, int count) {
   return true;
 }
 
-bool WebDatabase::AddFormFieldValue(const FormField& element) {
-  return AddFormFieldValueTime(element, base::Time::Now());
+bool WebDatabase::AddFormFieldValue(const FormField& element,
+                                    std::vector<AutofillChange>* changes) {
+  return AddFormFieldValueTime(element, changes, base::Time::Now());
 }
 
 bool WebDatabase::AddFormFieldValueTime(const FormField& element,
+                                        std::vector<AutofillChange>* changes,
                                         base::Time time) {
   int count = 0;
   int64 pair_id;
@@ -960,8 +964,18 @@ bool WebDatabase::AddFormFieldValueTime(const FormField& element,
   if (count == 0 && !InsertFormElement(element, &pair_id))
     return false;
 
-  return SetCountOfFormElement(pair_id, count + 1) &&
-      InsertPairIDAndDate(pair_id, time);
+  if (!SetCountOfFormElement(pair_id, count + 1))
+    return false;
+
+  if (!InsertPairIDAndDate(pair_id, time))
+    return false;
+
+  AutofillChange::Type change_type =
+      count == 0 ? AutofillChange::ADD : AutofillChange::UPDATE;
+  changes->push_back(
+      AutofillChange(change_type,
+                     AutofillKey(element.name(), element.value())));
+  return true;
 }
 
 bool WebDatabase::GetFormValuesForElementName(const string16& name,
diff --git a/chrome/browser/webdata/web_database.h b/chrome/browser/webdata/web_database.h
index 24bba2d..22c482a 100644
--- a/chrome/browser/webdata/web_database.h
+++ b/chrome/browser/webdata/web_database.h
@@ -130,12 +130,17 @@ class WebDatabase {
   //
   //////////////////////////////////////////////////////////////////////////////
 
-  // Records the form elements in |elements| in the database in the autofill
-  // table.
-  bool AddFormFieldValues(const std::vector<webkit_glue::FormField>& elements);
-
-  // Records a single form element in in the database in the autofill table.
-  bool AddFormFieldValue(const webkit_glue::FormField& element);
+  // Records the form elements in |elements| in the database in the
+  // autofill table.  A list of all added and updated autofill entries
+  // is returned in the changes out parameter.
+  bool AddFormFieldValues(const std::vector<webkit_glue::FormField>& elements,
+                          std::vector<AutofillChange>* changes);
+
+  // Records a single form element in in the database in the autofill
+  // table. A list of all added and updated autofill entries is
+  // returned in the changes out parameter.
+  bool AddFormFieldValue(const webkit_glue::FormField& element,
+                         std::vector<AutofillChange>* changes);
 
   // Retrieves a vector of all values which have been recorded in the autofill
   // table as the value in a form element with name |name| and which start with
@@ -208,14 +213,17 @@ class WebDatabase {
 
  private:
   FRIEND_TEST(WebDatabaseTest, Autofill);
+  FRIEND_TEST(WebDatabaseTest, Autofill_AddChanges);
   FRIEND_TEST(WebDatabaseTest, Autofill_RemoveBetweenChanges);
 
   // Methods for adding autofill entries at a specified time.  For
   // testing only.
   bool AddFormFieldValuesTime(
       const std::vector<webkit_glue::FormField>& elements,
+      std::vector<AutofillChange>* changes,
       base::Time time);
   bool AddFormFieldValueTime(const webkit_glue::FormField& element,
+                             std::vector<AutofillChange>* changes,
                              base::Time time);
 
   // Removes empty values for autofill that were incorrectly stored in the DB
diff --git a/chrome/browser/webdata/web_database_unittest.cc b/chrome/browser/webdata/web_database_unittest.cc
index 74647c8..5725c7c6 100644
--- a/chrome/browser/webdata/web_database_unittest.cc
+++ b/chrome/browser/webdata/web_database_unittest.cc
@@ -49,7 +49,7 @@ std::ostream& operator<<(std::ostream& os, const AutofillChange& change) {
 
 class WebDatabaseTest : public testing::Test {
  protected:
-
+  typedef std::vector<AutofillChange> AutofillChangeList;
   virtual void SetUp() {
     PathService::Get(chrome::DIR_TEST_DATA, &file_);
     const std::string test_db = "TestWebDatabase" +
@@ -417,32 +417,37 @@ TEST_F(WebDatabaseTest, Autofill) {
 
   // Simulate the submission of a handful of entries in a field called "Name",
   // some more often than others.
+  AutofillChangeList changes;
   EXPECT_TRUE(db.AddFormFieldValue(
       FormField(string16(),
                 ASCIIToUTF16("Name"),
                 string16(),
-                ASCIIToUTF16("Superman"))));
+                ASCIIToUTF16("Superman")),
+      &changes));
   std::vector<string16> v;
   for (int i = 0; i < 5; i++) {
     EXPECT_TRUE(db.AddFormFieldValue(
         FormField(string16(),
                   ASCIIToUTF16("Name"),
                   string16(),
-                  ASCIIToUTF16("Clark Kent"))));
+                  ASCIIToUTF16("Clark Kent")),
+        &changes));
   }
   for (int i = 0; i < 3; i++) {
     EXPECT_TRUE(db.AddFormFieldValue(
         FormField(string16(),
                   ASCIIToUTF16("Name"),
                   string16(),
-                  ASCIIToUTF16("Clark Sutter"))));
+                  ASCIIToUTF16("Clark Sutter")),
+        &changes));
   }
   for (int i = 0; i < 2; i++) {
     EXPECT_TRUE(db.AddFormFieldValue(
         FormField(string16(),
                   ASCIIToUTF16("Favorite Color"),
                   string16(),
-                  ASCIIToUTF16("Green"))));
+                  ASCIIToUTF16("Green")),
+        &changes));
   }
 
   int count = 0;
@@ -511,7 +516,7 @@ TEST_F(WebDatabaseTest, Autofill) {
 
   // Removing all elements since the beginning of this function should remove
   // everything from the database.
-  std::vector<AutofillChange> changes;
+  changes.clear();
   EXPECT_TRUE(db.RemoveFormElementsAddedBetween(t1, Time(), &changes));
 
   const AutofillChange expected_changes[] = {
@@ -550,19 +555,23 @@ TEST_F(WebDatabaseTest, Autofill) {
   EXPECT_TRUE(db.AddFormFieldValue(FormField(string16(),
                                              ASCIIToUTF16("blank"),
                                              string16(),
-                                             string16())));
+                                             string16()),
+                                   &changes));
   EXPECT_TRUE(db.AddFormFieldValue(FormField(string16(),
                                              ASCIIToUTF16("blank"),
                                              string16(),
-                                             ASCIIToUTF16(" "))));
+                                             ASCIIToUTF16(" ")),
+                                   &changes));
   EXPECT_TRUE(db.AddFormFieldValue(FormField(string16(),
                                              ASCIIToUTF16("blank"),
                                              string16(),
-                                             ASCIIToUTF16("      "))));
+                                             ASCIIToUTF16("      ")),
+                                   &changes));
   EXPECT_TRUE(db.AddFormFieldValue(FormField(string16(),
                                              ASCIIToUTF16("blank"),
                                              string16(),
-                                             kValue)));
+                                             kValue),
+                                   &changes));
 
   // They should be stored normally as the DB layer does not check for empty
   // values.
@@ -590,20 +599,23 @@ TEST_F(WebDatabaseTest, Autofill_RemoveBetweenChanges) {
   Time t1 = Time::Now();
   Time t2 = t1 + one_day;
 
+  AutofillChangeList changes;
   EXPECT_TRUE(db.AddFormFieldValueTime(
       FormField(string16(),
                 ASCIIToUTF16("Name"),
                 string16(),
                 ASCIIToUTF16("Superman")),
+      &changes,
       t1));
   EXPECT_TRUE(db.AddFormFieldValueTime(
       FormField(string16(),
                 ASCIIToUTF16("Name"),
                 string16(),
                 ASCIIToUTF16("Superman")),
+      &changes,
       t2));
 
-  std::vector<AutofillChange> changes;
+  changes.clear();
   EXPECT_TRUE(db.RemoveFormElementsAddedBetween(t1, t2, &changes));
   ASSERT_EQ(1U, changes.size());
   EXPECT_EQ(AutofillChange(AutofillChange::UPDATE,
@@ -620,6 +632,43 @@ TEST_F(WebDatabaseTest, Autofill_RemoveBetweenChanges) {
             changes[0]);
 }
 
+TEST_F(WebDatabaseTest, Autofill_AddChanges) {
+  WebDatabase db;
+  ASSERT_EQ(sql::INIT_OK, db.Init(file_));
+
+  TimeDelta one_day(TimeDelta::FromDays(1));
+  Time t1 = Time::Now();
+  Time t2 = t1 + one_day;
+
+  AutofillChangeList changes;
+  EXPECT_TRUE(db.AddFormFieldValueTime(
+      FormField(string16(),
+                ASCIIToUTF16("Name"),
+                string16(),
+                ASCIIToUTF16("Superman")),
+      &changes,
+      t1));
+  ASSERT_EQ(1U, changes.size());
+  EXPECT_EQ(AutofillChange(AutofillChange::ADD,
+                             AutofillKey(ASCIIToUTF16("Name"),
+                                         ASCIIToUTF16("Superman"))),
+            changes[0]);
+
+  changes.clear();
+  EXPECT_TRUE(db.AddFormFieldValueTime(
+      FormField(string16(),
+                ASCIIToUTF16("Name"),
+                string16(),
+                ASCIIToUTF16("Superman")),
+      &changes,
+      t2));
+  ASSERT_EQ(1U, changes.size());
+  EXPECT_EQ(AutofillChange(AutofillChange::UPDATE,
+                             AutofillKey(ASCIIToUTF16("Name"),
+                                         ASCIIToUTF16("Superman"))),
+            changes[0]);
+}
+
 static bool AddTimestampedLogin(WebDatabase* db, std::string url,
                                 const std::string& unique_string,
                                 const Time& time) {