Cull autofill entries older than 60 days.

chrome/browser/webdata/autofill_table.cc

Index: chrome/browser/webdata/autofill_table.cc
diff --git a/chrome/browser/webdata/autofill_table.cc b/chrome/browser/webdata/autofill_table.cc
index bba0b838df6c3ad84e9f2455f43bf373ae87ceed..96e2ed5387ef111feda0db6b0ebf8d9388f6692a 100644
--- a/chrome/browser/webdata/autofill_table.cc
+++ b/chrome/browser/webdata/autofill_table.cc
@@ -23,6 +23,7 @@
 #include "chrome/browser/autofill/personal_data_manager.h"
 #include "chrome/browser/password_manager/encryptor.h"
 #include "chrome/browser/webdata/autofill_change.h"
+#include "chrome/browser/webdata/autofill_entry.h"
 #include "chrome/common/guid.h"
 #include "sql/statement.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -425,8 +426,17 @@ bool AutofillTable::RemoveFormElementsAddedBetween(
       return false;
     }
     bool was_removed = false;
-    if (!AddToCountOfFormElement(itr->a, -how_many, &was_removed))
-      return false;
+    // We store at most 2 time stamps. If we remove both of them we should
+    // delete the corresponding data. if we delete only one it could still be
+    // the last timestamp for the data.
+    if (how_many == 2 || (how_many == 1 && CountTimestampsData(itr->a) == 0)) {

[Ilya Sherman, 2012/03/20 21:25:13]

How about just always checking CountTimestampsData() and not worrying about the
value of |how_many|?  In fact, that seems to be what AddToCountOfFormElement()
already does...  If we need the special-casing, could you please explain why?

[GeorgeY, 2012/03/21 20:56:40]

Different counts. The other count functions are returning/updating |count|
property in the autofill table, that can be arbitrary big.
CountTimestampsData() on the other hand does a select count(*) on the
autofill_dates table to verify if any of the timestamps do remain. As after
initial cleanup we store at most two timestamps and we know how many we deleted,
it does not make sense to do additional select if we do not need it.

[Ilya Sherman, 2012/03/21 21:20:53]

Ah, ok, I understand what you mean about this being different counts.  I do
prefer that we always do the additional select, though, rather than relying on
|how_many|, because there's a small chance that this method will be called
before we've culled the timestamps down to just two.

[GeorgeY, 2012/03/21 22:10:56]

Done.

+      was_removed = true;
+      if (!RemoveFormElementForID(itr->a))
+        return false;
+    } else {
+      if (!AddToCountOfFormElement(itr->a, -how_many, &was_removed))
+        return false;
+    }
     AutofillChange::Type change_type =
         was_removed ? AutofillChange::REMOVE : AutofillChange::UPDATE;
     changes->push_back(AutofillChange(change_type,
@@ -436,6 +446,77 @@ bool AutofillTable::RemoveFormElementsAddedBetween(
   return true;
 }
 
+bool AutofillTable::RemoveExpiredFormElements(
+    std::vector<AutofillChange>* changes) {
+  DCHECK(changes);
+
+  base::Time delete_end = AutofillEntry::ExpirationTime();
+  // Query for the pair_id, name, and value of all form elements that
+  // were last used before the given time.

[Ilya Sherman, 2012/03/20 21:25:13]

nit: "the given time" -> "|delete_end|"

[GeorgeY, 2012/03/21 20:56:40]

Done.

+  sql::Statement select_for_delete(db_->GetUniqueStatement(
+      "SELECT DISTINCT pair_id, name, value "
+      "FROM autofill WHERE pair_id NOT IN "
+      "(SELECT DISTINCT pair_id "
+      "FROM autofill_dates WHERE date_created >= ?)"));
+  select_for_delete.BindInt64(0, delete_end.ToTimeT());
+  AutofillElementList entries_to_delete;
+  while (select_for_delete.Step()) {
+    entries_to_delete.push_back(MakeTuple(select_for_delete.ColumnInt64(0),
+                                          select_for_delete.ColumnString16(1),
+                                          select_for_delete.ColumnString16(2)));
+  }
+
+  if (!select_for_delete.Succeeded())
+    return false;
+
+  sql::Statement delete_data_statement(db_->GetUniqueStatement(
+      "DELETE FROM autofill WHERE pair_id NOT IN ("
+      "SELECT pair_id FROM autofill_dates WHERE date_created >= ?)"));
+  delete_data_statement.BindInt64(0, delete_end.ToTimeT());
+  if (!delete_data_statement.Run())
+    return false;
+
+  sql::Statement delete_times_statement(db_->GetUniqueStatement(
+      "DELETE FROM autofill_dates WHERE pair_id NOT IN ("
+      "SELECT pair_id FROM autofill_dates WHERE date_created >= ?)"));
+  delete_times_statement.BindInt64(0, delete_end.ToTimeT());
+  if (!delete_times_statement.Run())
+    return false;
+
+  // Cull remaining entries' timestamps.
+  std::vector<AutofillEntry> entries;
+  if (!GetAllAutofillEntries(&entries))
+    return false;
+  sql::Statement cull_date_entry(db_->GetUniqueStatement(
+      "DELETE FROM autofill_dates "
+      "WHERE pair_id == (SELECT pair_id FROM autofill "
+                         "WHERE name = ? and value = ?)"
+      "AND date_created != ? AND date_created != ?"));
+  for (size_t i = 0; i < entries.size(); ++i) {
+    cull_date_entry.BindString16(0, entries[i].key().name());
+    cull_date_entry.BindString16(1, entries[i].key().value());
+    cull_date_entry.BindInt64(2,
+        entries[i].timestamps().empty() ? 0 :
+        entries[i].timestamps().front().ToTimeT());
+    cull_date_entry.BindInt64(3,
+        entries[i].timestamps().empty() ? 0 :
+        entries[i].timestamps().back().ToTimeT());
+    if (!cull_date_entry.Run())
+      return false;
+    cull_date_entry.Reset();
+  }
+
+  changes->clear();
+  changes->reserve(entries_to_delete.size());
+
+  for (AutofillElementList::iterator it = entries_to_delete.begin();
+       it != entries_to_delete.end(); ++it) {
+    changes->push_back(AutofillChange(
+        AutofillChange::REMOVE, AutofillKey(it->b, it->c)));
+  }
+  return true;
+}
+
 bool AutofillTable::RemoveFormElementForTimeRange(int64 pair_id,
                                                   const Time& delete_begin,
                                                   const Time& delete_end,
@@ -455,6 +536,16 @@ bool AutofillTable::RemoveFormElementForTimeRange(int64 pair_id,
   return result;
 }
 
+int AutofillTable::CountTimestampsData(int64 pair_id) {
+  sql::Statement s(db_->GetUniqueStatement(
+      "SELECT COUNT(*) FROM autofill_dates WHERE pair_id = ?"));
+  s.BindInt64(0, pair_id);
+  if (!s.Step())

[Ilya Sherman, 2012/03/20 21:25:13]

nit: Should this if-stmt include a NOTREACHED()?

[GeorgeY, 2012/03/21 20:56:40]

Done.

+    return 0;
+  else
+    return s.ColumnInt(0);
+}
+
 bool AutofillTable::AddToCountOfFormElement(int64 pair_id,
                                             int delta,
                                             bool* was_removed) {
@@ -552,6 +643,19 @@ bool AutofillTable::InsertPairIDAndDate(int64 pair_id,
   return s.Run();
 }
 
+bool AutofillTable::DeleteLastAccess(int64 pair_id) {
+  // Inner SELECT selects the newest |date_created| for a given |pair_id|.
+  // DELETE deletes only that entry.
+  sql::Statement s(db_->GetUniqueStatement(
+      "DELETE FROM autofill_dates WHERE pair_id = ? and date_created IN "
+      "(SELECT date_created FROM autofill_dates WHERE pair_id = ? "
+      "ORDER BY date_created DESC LIMIT 1)"));
+  s.BindInt64(0, pair_id);
+  s.BindInt64(1, pair_id);
+
+  return s.Run();
+}
+
 bool AutofillTable::AddFormFieldValuesTime(
     const std::vector<FormField>& elements,
     std::vector<AutofillChange>* changes,
@@ -733,6 +837,11 @@ bool AutofillTable::AddFormFieldValueTime(const FormField& element,
   if (!SetCountOfFormElement(pair_id, count + 1))
     return false;
 
+  // If we already have more than 2 times delete last one, before adding new
+  // one.
+  if (count >= 2 && !DeleteLastAccess(pair_id))
+    return false;
+
   if (!InsertPairIDAndDate(pair_id, time))
     return false;