Cull autofill entries older than 60 days.

chrome/browser/webdata/autocomplete_syncable_service.cc

 // Copyright (c) 2012 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/autocomplete_syncable_service.h"
 
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/profiles/profile.h"
@@ skipping 11 matching lines @@
 
 using content::BrowserThread;
 
 namespace {
 
 const char kAutofillEntryNamespaceTag[] = "autofill_entry|";
 
 // Merges timestamps from the |autofill| entry and |timestamps|. Returns
 // true if they were different, false if they were the same.
 // All of the timestamp vectors are assummed to be sorted, resulting vector is
-// sorted as well.
+// sorted as well. Only two timestamps - the earliest and the latest are stored.
 bool MergeTimestamps(const sync_pb::AutofillSpecifics& autofill,
                      const std::vector<base::Time>& timestamps,
                      std::vector<base::Time>* new_timestamps) {
   DCHECK(new_timestamps);
-  std::set<base::Time> timestamp_union(timestamps.begin(),
-                                       timestamps.end());
 
+  new_timestamps->clear();
   size_t timestamps_count = autofill.usage_timestamp_size();
-
-  bool different = timestamps.size() != timestamps_count;
-  for (size_t i = 0; i < timestamps_count; ++i) {
-    if (timestamp_union.insert(base::Time::FromInternalValue(
-            autofill.usage_timestamp(i))).second) {
+  if (timestamps_count == 0 && timestamps.empty()) {
+    return false;
+  } else if (timestamps_count == 0) {
+    new_timestamps->insert(new_timestamps->begin(),
+                           timestamps.begin(),
+                           timestamps.end());
+    return true;
+  } else if (timestamps.empty()) {
+    new_timestamps->reserve(2);
+    new_timestamps->push_back(base::Time::FromInternalValue(
+        autofill.usage_timestamp(0));
+    if (timestamps_count > 1) {
+      new_timestamps->push_back(base::Time::FromInternalValue(
+          autofill.usage_timestamp(timestamps_count - 1));
+    }
+    return true;
+  } else {
+    base::Time time_begin = base::Time::FromInternalValue(
+        autofill.usage_timestamp(0);
+    base::Time time_end = base::Time::FromInternalValue(
+        autofill.usage_timestamp(timestamps_count - 1);
+    bool different = (timestamps_count != timestamps.size());
+    if (timestamps.front() > time_begin) {

[Nicolas Zea, 2012/03/26 20:58:22]

I don't think this is quite right. You're not setting different = true if
timestamps.front() < time_begin. Also, operator> returns true if lhs is more
recent than rhs (it compares microseconds since epoch). So you're updating
timestamps[0] only if it's already more recent, when I believe the more recent
one is the one you want to keep, right? Also you're modifying timestamps instead
of new_timestamps here.

In addition, I'd prefer that the count of timestamps not be a consideration when
comparing with sync data, given that other clients will continue to track all
the timestamps (and the fact that previously this code unioned means they'll
continue to add the timestamps back in when they restart sync).

how about something like:
if (timestamps.front() != sync_time_begin || timestamps.back() != sync_time_end)
{
  new_timestamps.push_back(timestamps.front() > sync_time_begin ?
timestamps.front() : sync_time_begin);
  if (timestamps.size() > 1 || sync_timestamps_count > 1)
    new_timestamps.push_back(timestamps.back() < sync_time_end ?
timestamps.back() : sync_time_end);
  return true;
}
return false;

This way you only care about the first/last sync timestamps and you return
different if they're not already set properly.

[GeorgeY, 2012/03/26 21:19:47]

sure. works as well, though slightly modified (your code does not work if both
have only one different timestamp, for example)

+      if (timestamps.size() == 1)
+        timestamps.push_back(timestamps[0]);
+      timestamps[0] = time_begin;
       different = true;
     }
+    if (timestamps.back() < time_end) {
+      if (timestamps.size() == 1)
+        timestamps.push_back(time_end);
+      else
+        timestamps[1] = time_end;
+      different = true;
+    }
+    return different;
   }
-
-  if (different) {
-    new_timestamps->insert(new_timestamps->begin(),
-                           timestamp_union.begin(),
-                           timestamp_union.end());
-  }
-  return different;
 }
 
 }  // namespace
 
 AutocompleteSyncableService::AutocompleteSyncableService(
     WebDataService* web_data_service)
     : web_data_service_(web_data_service) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
   DCHECK(web_data_service_);
   notification_registrar_.Add(
@@ skipping 28 matching lines @@
 
   AutocompleteEntryMap new_db_entries;
   for (std::vector<AutofillEntry>::iterator it = entries.begin();
        it != entries.end(); ++it) {
     new_db_entries[it->key()] = std::make_pair(SyncChange::ACTION_ADD, it);
   }
 
   sync_processor_ = sync_processor.Pass();
 
   std::vector<AutofillEntry> new_synced_entries;
+  std::vector<AutofillEntry> synced_expired_entries;
   // Go through and check for all the entries that sync already knows about.
   // CreateOrUpdateEntry() will remove entries that are same with the synced
   // ones from |new_db_entries|.
   for (SyncDataList::const_iterator sync_iter = initial_sync_data.begin();
        sync_iter != initial_sync_data.end(); ++sync_iter) {
-    CreateOrUpdateEntry(*sync_iter, &new_db_entries, &new_synced_entries);
+    CreateOrUpdateEntry(*sync_iter, &new_db_entries,
+                        &new_synced_entries, &synced_expired_entries);
   }
 
+  // Check if newly received items need culling.
+  bool need_to_cull_data = !synced_expired_entries.empty();
+
   if (!SaveChangesToWebData(new_synced_entries))
     return SyncError(FROM_HERE, "Failed to update webdata.", model_type());
 
   WebDataService::NotifyOfMultipleAutofillChanges(web_data_service_);
+  keys_to_ignore_.clear();
 
   SyncChangeList new_changes;
   for (AutocompleteEntryMap::iterator i = new_db_entries.begin();
        i != new_db_entries.end(); ++i) {
-    new_changes.push_back(
-        SyncChange(i->second.first, CreateSyncData(*(i->second.second))));
+    // Sync back only the data that appeared after
+    // |AutofillEntry::ExpirationTime()|.
+    if (!i->second.second->IsExpired()) {
+      new_changes.push_back(
+          SyncChange(i->second.first, CreateSyncData(*(i->second.second))));
+    } else {
+      need_to_cull_data = true;
+      // Key is not on the server and is too old, it will not ever be synced -
+      // delete it locally.
+      if (i->second.first == SyncChange::ACTION_ADD)
+        keys_to_ignore_.insert(i->first);
+    }
+  }
+
+  // Delete only the changes never synced to the db as they are too old.
+  for (size_t i = 0; i < synced_expired_entries.size(); ++i) {
+    // Key is on the server and not local and is too old, we need to notify
+    // sync that it has expired.
+    if (keys_to_ignore_.find(synced_expired_entries[i].key()) ==
+        keys_to_ignore_.end()) {
+      new_changes.push_back(SyncChange(SyncChange::ACTION_DELETE,
+          CreateSyncData(synced_expired_entries[i])));
+    }
+  }
+
+  if (need_to_cull_data) {
+    // This will schedule deletion operation later on DB thread and we will
+    // be notified on the results of the deletion and deletes will be synced to
+    // the sync.
+    web_data_service_->RemoveExpiredFormElements();
   }
 
   SyncError error = sync_processor_->ProcessSyncChanges(FROM_HERE, new_changes);
 
   return error;
 }
 
 void AutocompleteSyncableService::StopSyncing(syncable::ModelType type) {
   DCHECK(CalledOnValidThread());
   DCHECK_EQ(syncable::AUTOFILL, type);
@@ skipping 30 matching lines @@
   if (!sync_processor_.get()) {
     SyncError error(FROM_HERE, "Models not yet associated.",
                     syncable::AUTOFILL);
     return error;
   }
 
   // Data is loaded only if we get new ADD/UPDATE change.
   std::vector<AutofillEntry> entries;
   scoped_ptr<AutocompleteEntryMap> db_entries;
   std::vector<AutofillEntry> new_entries;
+  std::vector<AutofillEntry> ignored_entries;
 
   SyncError list_processing_error;
 
   for (SyncChangeList::const_iterator i = change_list.begin();
        i != change_list.end() && !list_processing_error.IsSet(); ++i) {
     DCHECK(i->IsValid());
     switch (i->change_type()) {
       case SyncChange::ACTION_ADD:
       case SyncChange::ACTION_UPDATE:
         if (!db_entries.get()) {
           if (!LoadAutofillData(&entries)) {
             return SyncError(
                 FROM_HERE,
                 "Could not get the autocomplete data from WebDatabase.",
                 model_type());
           }
           db_entries.reset(new AutocompleteEntryMap);
           for (std::vector<AutofillEntry>::iterator it = entries.begin();
                it != entries.end(); ++it) {
             (*db_entries)[it->key()] =
                 std::make_pair(SyncChange::ACTION_ADD, it);
           }
         }
-        CreateOrUpdateEntry(i->sync_data(), db_entries.get(), &new_entries);
+        CreateOrUpdateEntry(i->sync_data(), db_entries.get(),
+                            &new_entries, &ignored_entries);
         break;
       case SyncChange::ACTION_DELETE: {
         DCHECK(i->sync_data().GetSpecifics().has_autofill())
             << "Autofill specifics data not present on delete!";
         const sync_pb::AutofillSpecifics& autofill =
             i->sync_data().GetSpecifics().autofill();
         if (autofill.has_value()) {
           list_processing_error = AutofillEntryDelete(autofill);
         } else {
           DLOG(WARNING)
               << "Delete for old-style autofill profile being dropped!";
         }
       } break;
       default:
         NOTREACHED() << "Unexpected sync change state.";
         return SyncError(FROM_HERE, "ProcessSyncChanges failed on ChangeType " +
                          SyncChange::ChangeTypeToString(i->change_type()),
                          syncable::AUTOFILL);
     }
   }
 
   if (!SaveChangesToWebData(new_entries))
     return SyncError(FROM_HERE, "Failed to update webdata.", model_type());
 
+  // Remove already expired data.
+  for (size_t i = 0; i < ignored_entries.size(); ++i) {
+    if (db_entries.get() &&
+        db_entries->find(ignored_entries[i].key()) != db_entries->end()) {
+      bool success = web_data_service_->GetDatabase()->GetAutofillTable()->
+              RemoveFormElement(ignored_entries[i].key().name(),
+                                ignored_entries[i].key().value());
+      DCHECK(success);
+    }
+  }
+
   WebDataService::NotifyOfMultipleAutofillChanges(web_data_service_);
 
   return list_processing_error;
 }
 
 void AutocompleteSyncableService::Observe(int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   DCHECK_EQ(chrome::NOTIFICATION_AUTOFILL_ENTRIES_CHANGED, type);
 
@@ skipping 27 matching lines @@
           GetAutofillTable()->UpdateAutofillEntries(new_entries)) {
     return false;
   }
   return true;
 }
 
 // Creates or updates an autocomplete entry based on |data|.
 void AutocompleteSyncableService::CreateOrUpdateEntry(
     const SyncData& data,
     AutocompleteEntryMap* loaded_data,
-    std::vector<AutofillEntry>* new_entries) {
+    std::vector<AutofillEntry>* new_entries,
+    std::vector<AutofillEntry>* ignored_entries) {
   const sync_pb::EntitySpecifics& specifics = data.GetSpecifics();
   const sync_pb::AutofillSpecifics& autofill_specifics(
       specifics.autofill());
 
   if (!autofill_specifics.has_value()) {
     DLOG(WARNING)
         << "Add/Update for old-style autofill profile being dropped!";
     return;
   }
 
   AutofillKey key(autofill_specifics.name().c_str(),
                   autofill_specifics.value().c_str());
   AutocompleteEntryMap::iterator it = loaded_data->find(key);
   if (it == loaded_data->end()) {
     // New entry.
     std::vector<base::Time> timestamps;
     size_t timestamps_count = autofill_specifics.usage_timestamp_size();
-    timestamps.resize(timestamps_count);
-    for (size_t ts = 0; ts < timestamps_count; ++ts) {
-      timestamps[ts] = base::Time::FromInternalValue(
-            autofill_specifics.usage_timestamp(ts));
+    timestamps.reserve(2);
+    if (timestamps_count) {
+      timestamps.push_back(base::Time::FromInternalValue(
+          autofill_specifics.usage_timestamp(0)))
     }
-    new_entries->push_back(AutofillEntry(key, timestamps));
+    if (timestamps_count > 1) {
+      timestamps.push_back(base::Time::FromInternalValue(
+          autofill_specifics.usage_timestamp(timestamps_count - 1)))
+    }
+    AutofillEntry new_entry(key, timestamps);
+    if (new_entry.IsExpired())
+      ignored_entries->push_back(new_entry);
+    else
+      new_entries->push_back(new_entry);
   } else {
     // Entry already present - merge if necessary.
     std::vector<base::Time> timestamps;
     bool different = MergeTimestamps(
         autofill_specifics, it->second.second->timestamps(), &timestamps);
     if (different) {
       AutofillEntry new_entry(it->second.second->key(), timestamps);
-      new_entries->push_back(new_entry);
+      if (new_entry.IsExpired())
+        ignored_entries->push_back(new_entry);
+      else
+        new_entries->push_back(new_entry);
 
       // Update the sync db if the list of timestamps have changed.
       *(it->second.second) = new_entry;

[Nicolas Zea, 2012/03/26 20:58:22]

This code has no effect if the entry is expired right? Perhaps move into the
else case of the if statement above?

[GeorgeY, 2012/03/26 21:19:47]

Sure. It will get deleted in any case.

       it->second.first = SyncChange::ACTION_UPDATE;
     } else {
       loaded_data->erase(it);
     }
   }
 }
 
 // static
 void AutocompleteSyncableService::WriteAutofillEntry(
     const AutofillEntry& entry, sync_pb::EntitySpecifics* autofill_specifics) {
@@ skipping 39 matching lines @@
         }
         AutofillEntry entry(change->key(), timestamps);
         SyncChange::SyncChangeType change_type =
            (change->type() == AutofillChange::ADD) ?
                SyncChange::ACTION_ADD : SyncChange::ACTION_UPDATE;
         new_changes.push_back(SyncChange(change_type,
                                          CreateSyncData(entry)));
         break;
       }
       case AutofillChange::REMOVE: {
-        std::vector<base::Time> timestamps;
-        AutofillEntry entry(change->key(), timestamps);
-        new_changes.push_back(SyncChange(SyncChange::ACTION_DELETE,
-                                         CreateSyncData(entry)));
+        if (keys_to_ignore_.find(change->key()) == keys_to_ignore_.end()) {
+          std::vector<base::Time> timestamps;
+          AutofillEntry entry(change->key(), timestamps);
+          new_changes.push_back(SyncChange(SyncChange::ACTION_DELETE,
+                                           CreateSyncData(entry)));
+        }
         break;
       }
       default:
         NOTREACHED();
         break;
     }
   }
   SyncError error = sync_processor_->ProcessSyncChanges(FROM_HERE, new_changes);
   if (error.IsSet()) {
     DLOG(WARNING) << "[AUTOCOMPLETE SYNC]"
                   << " Failed processing change:"
                   << " Error:" << error.message();
   }
+  // |keys_to_ignore_| are only needed for the very first notification.
+  keys_to_ignore_.clear();
 }
 
 SyncData AutocompleteSyncableService::CreateSyncData(
     const AutofillEntry& entry) const {
   sync_pb::EntitySpecifics autofill_specifics;
   WriteAutofillEntry(entry, &autofill_specifics);
   std::string tag(KeyToTag(UTF16ToUTF8(entry.key().name()),
                            UTF16ToUTF8(entry.key().value())));
   return SyncData::CreateLocalData(tag, tag, autofill_specifics);
 }
 
 // static
 std::string AutocompleteSyncableService::KeyToTag(const std::string& name,
                                                   const std::string& value) {
   std::string ns(kAutofillEntryNamespaceTag);
   return ns + net::EscapePath(name) + "|" + net::EscapePath(value);
 }