Cull autofill entries older than 60 days.

chrome/browser/webdata/autofill_entry.cc

Index: chrome/browser/webdata/autofill_entry.cc
diff --git a/chrome/browser/webdata/autofill_entry.cc b/chrome/browser/webdata/autofill_entry.cc
index fe874a58cd8c4da8be15155a99f5e15ed43ace29..d34449aeced51a171c8144891a14763a6acc0106 100644
--- a/chrome/browser/webdata/autofill_entry.cc
+++ b/chrome/browser/webdata/autofill_entry.cc
@@ -1,12 +1,12 @@
 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
+#include "chrome/browser/webdata/autofill_entry.h"

[Ilya Sherman, 2012/03/19 21:12:51]

nit: Please leave a blank line between the copyright license and the first
#include.

[GeorgeY, 2012/03/20 20:47:58]

Done.

 
 #include <algorithm>
 #include <set>
 
 #include "base/logging.h"
-#include "chrome/browser/webdata/autofill_entry.h"
 #include "base/utf_string_conversions.h"
 
 AutofillKey::AutofillKey() {}
@@ -72,7 +72,18 @@ bool AutofillEntry::operator<(const AutofillEntry& entry) const {
   return key_ < entry.key();
 }
 
-// Culls the list of timestamps to the most recent |kMaxAutofillTimeStamps|.
+bool AutofillEntry::IsExpired() const {
+  base::Time time = ExpirationTime();
+  DCHECK(!timestamps_.empty());
+  return (timestamps_.empty() || timestamps_.back() < time);

[Ilya Sherman, 2012/03/19 21:12:51]

nit: No need to check timestamps_.empty() given that you DCHECK() non-emptiness
immediately above.

[GeorgeY, 2012/03/20 20:47:58]

Old implementation *allows* for all of the timestamps deleted (theoretically),
but not the new one.

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

If so, we shouldn't have a DCHECK() ;)

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

OK, removed. Again in the current code it is a valid concurrence. In the new
code it is an error.

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

nit: If the DCHECK() will be correct in the future, you might want to add a
TODO() to add it once the migration happens.

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

Done.

+}
+
+// static
+base::Time AutofillEntry::ExpirationTime() {
+  return base::Time::Now() - base::TimeDelta::FromDays(kExpirationPeriodInDays);
+}
+
+// Culls the list of timestamps to the first and last used.
 // If sync is enabled, at every browser restart, sync will do a match up of all
 // autofill items on the server with all items on the web db. When webdb loads
 // all the items in memory(for sync to process. The method is
@@ -83,26 +94,22 @@ bool AutofillEntry::operator<(const AutofillEntry& entry) const {
 // restart. But sync when uploading to the server will only upload this culled
 // list. Meaning until restart there will be mis-match in timestamps but
 // it should correct itself at startup.
-// Also if sync is not enabled this culling would never take place.
 bool AutofillEntry::CullTimeStamps(const std::vector<base::Time>& source,
-                                       std::vector<base::Time>* result) {
+                                   std::vector<base::Time>* result) {
   DCHECK(result);
   DCHECK(&source != result);
 
   // First copy the source to result.
   result->clear();
-  result->insert(result->begin(), source.begin(), source.end());
 
-  if (source.size() <= kMaxAutofillTimeStamps)
+  if (source.size() <= 2) {
+    result->insert(result->begin(), source.begin(), source.end());
     return false;
+  }

[Ilya Sherman, 2012/03/19 21:12:51]

nit: Please leave a blank line after this if-stmt, since it contains a return
stmt within it.

[GeorgeY, 2012/03/20 20:47:58]

Done.

+  result->push_back(source.front());
+  result->push_back(source.back());
 
-  VLOG(1) << "Culling timestamps. Current count is : " << source.size();
-
-  // Regular sort does ascending order. So we pass in a comparator function.
-  partial_sort(result->begin(), result->begin() + kMaxAutofillTimeStamps,
-               result->end(), std::greater<base::Time>());
-
-  result->erase(result->begin() + kMaxAutofillTimeStamps, result->end());
+  DVLOG(1) << "Culling timestamps. Current count is : " << source.size();
 
   return true;
 }