Add probability of culling if sync is enabled to https://chromiumcodereview.appspot.com/9585020/ In…

chrome/browser/webdata/autocomplete_syncable_service.cc

Index: chrome/browser/webdata/autocomplete_syncable_service.cc
diff --git a/chrome/browser/webdata/autocomplete_syncable_service.cc b/chrome/browser/webdata/autocomplete_syncable_service.cc
index 3b3ecb1b2cb64812d766542376557fc7bcef1cc1..83da6d49f6066ca3737f4587d1dbb3fb1a654939 100644
--- a/chrome/browser/webdata/autocomplete_syncable_service.cc
+++ b/chrome/browser/webdata/autocomplete_syncable_service.cc
@@ -6,6 +6,7 @@
 
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/rand_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sync/api/sync_error.h"
@@ -29,30 +30,60 @@ 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) {
-      different = true;
+  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 sync_time_begin = base::Time::FromInternalValue(
+        autofill.usage_timestamp(0));
+    base::Time sync_time_end = base::Time::FromInternalValue(
+        autofill.usage_timestamp(timestamps_count - 1));
+    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 (new_timestamps->back() != timestamps.back() ||
+          new_timestamps->back() != sync_time_end) {
+        new_timestamps->push_back(
+            timestamps.back() > sync_time_end ? timestamps.back() :
+                                                sync_time_end);
+      }
+      return true;
+    } else {
+      new_timestamps->insert(new_timestamps->begin(),
+                             timestamps.begin(),
+                             timestamps.end());
+      return false;
     }
   }
+}
 
-  if (different) {
-    new_timestamps->insert(new_timestamps->begin(),
-                           timestamp_union.begin(),
-                           timestamp_union.end());
-  }
-  return different;
+bool CullSyncedData() {

[Ilya Sherman, 2012/04/03 00:13:36]

nit: Actually, perhaps "ShouldCullSyncedData()"?

[GeorgeY, 2012/04/03 00:28:44]

sure

+  // To set probability to 10% - set it to 0.1, 5% to 0.05, etc.
+  static double kCullingProbability = 0.0;
+  return (base::RandDouble() < kCullingProbability);
 }
 
 }  // namespace
@@ -101,24 +132,58 @@ SyncError AutocompleteSyncableService::MergeDataAndStartSyncing(
   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,

[Nicolas Zea, 2012/04/03 00:39:09]

This logic should also be protected by CullSyncedData(), else a new client that
attempts to sync will start deleting all the old data.

[GeorgeY, 2012/04/03 16:55:02]

Done.

+          CreateSyncData(synced_expired_entries[i])));
+    }
+  }
+
+  if (need_to_cull_data && CullSyncedData()) {
+    // 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);
@@ -169,6 +234,7 @@ SyncError AutocompleteSyncableService::ProcessSyncChanges(
   std::vector<AutofillEntry> entries;
   scoped_ptr<AutocompleteEntryMap> db_entries;
   std::vector<AutofillEntry> new_entries;
+  std::vector<AutofillEntry> ignored_entries;
 
   SyncError list_processing_error;
 
@@ -192,7 +258,8 @@ SyncError AutocompleteSyncableService::ProcessSyncChanges(
                 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())
@@ -217,6 +284,17 @@ SyncError AutocompleteSyncableService::ProcessSyncChanges(
   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;
@@ -264,7 +342,8 @@ bool AutocompleteSyncableService::SaveChangesToWebData(
 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());
@@ -282,12 +361,20 @@ void AutocompleteSyncableService::CreateOrUpdateEntry(
     // 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;
@@ -295,11 +382,14 @@ void AutocompleteSyncableService::CreateOrUpdateEntry(
         autofill_specifics, it->second.second->timestamps(), &timestamps);
     if (different) {
       AutofillEntry new_entry(it->second.second->key(), timestamps);
-      new_entries->push_back(new_entry);
-
-      // Update the sync db if the list of timestamps have changed.
-      *(it->second.second) = new_entry;
-      it->second.first = SyncChange::ACTION_UPDATE;
+      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;
+        it->second.first = SyncChange::ACTION_UPDATE;
+      }
     } else {
       loaded_data->erase(it);
     }
@@ -358,10 +448,12 @@ void AutocompleteSyncableService::ActOnChanges(
         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:
@@ -375,6 +467,8 @@ void AutocompleteSyncableService::ActOnChanges(
                   << " Failed processing change:"
                   << " Error:" << error.message();
   }
+  // |keys_to_ignore_| are only needed for the very first notification.
+  keys_to_ignore_.clear();
 }
 
 SyncData AutocompleteSyncableService::CreateSyncData(