Correctly handle expired and truncated visits in typed url sync

chrome/browser/sync/glue/typed_url_model_associator.cc

Index: chrome/browser/sync/glue/typed_url_model_associator.cc
diff --git a/chrome/browser/sync/glue/typed_url_model_associator.cc b/chrome/browser/sync/glue/typed_url_model_associator.cc
index f49711f101f79bcbdb2b6b3c9965cc72ccdfc183..7d904dc693dd6c82a6d0f82129518704e0e48235 100644
--- a/chrome/browser/sync/glue/typed_url_model_associator.cc
+++ b/chrome/browser/sync/glue/typed_url_model_associator.cc
@@ -83,6 +83,7 @@ bool TypedUrlModelAssociator::FixupURLAndGetVisits(
   // create a new visit whose timestamp is the same as the last_visit time.
   // This is a workaround for http://crbug.com/84258.
   if (visits->empty()) {
+    VLOG(1) << "Found empty visits for URL: " << url->url();
     history::VisitRow visit(
         url->id(), url->last_visit(), 0, content::PAGE_TRANSITION_TYPED, 0);
     visits->push_back(visit);
@@ -119,6 +120,7 @@ bool TypedUrlModelAssociator::AssociateModels(SyncError* error) {
        ix != typed_urls.end(); ++ix) {
     if (IsAbortPending())
       return false;
+    DCHECK(visit_vectors.find(ix->id()) == visit_vectors.end());
     if (!FixupURLAndGetVisits(
             history_backend_, &(*ix), &(visit_vectors[ix->id()]))) {
       error->Reset(FROM_HERE, "Could not get the url's visits.", model_type());
@@ -252,7 +254,6 @@ bool TypedUrlModelAssociator::AssociateModels(SyncError* error) {
       }
       const sync_pb::TypedUrlSpecifics& typed_url(
         sync_child_node.GetTypedUrlSpecifics());
-
       sync_child_id = sync_child_node.GetSuccessorId();
 
       // Ignore old sync nodes that don't have any transition data stored with
@@ -553,6 +554,7 @@ TypedUrlModelAssociator::MergeResult TypedUrlModelAssociator::MergeUrls(
   size_t history_num_visits = visits->size();
   size_t node_visit_index = 0;
   size_t history_visit_index = 0;
+  base::Time earliest_history_time = (*visits)[0].visit_time;
   // Walk through the two sets of visits and figure out if any new visits were
   // added on either side.
   while (node_visit_index < node_num_visits ||
@@ -575,11 +577,16 @@ TypedUrlModelAssociator::MergeResult TypedUrlModelAssociator::MergeUrls(
       // Found a visit in the sync node that doesn't exist in the history DB, so
       // add it to our list of new visits and set the appropriate flag so the
       // caller will update the history DB.
-      different |= DIFF_LOCAL_VISITS_ADDED;
-      new_visits->push_back(history::VisitInfo(
-          node_time,
-          content::PageTransitionFromInt(
-              node.visit_transitions(node_visit_index))));
+      // If the node visit is older than any existing visit in the history DB,
+      // don't re-add it - this keeps us from resurrecting visits that were
+      // aged out locally.
+      if (node_time > earliest_history_time) {
+        different |= DIFF_LOCAL_VISITS_ADDED;
+        new_visits->push_back(history::VisitInfo(
+            node_time,
+            content::PageTransitionFromInt(
+                node.visit_transitions(node_visit_index))));
+      }
       // This visit is added to visits below.
       ++node_visit_index;
     } else {
@@ -724,7 +731,13 @@ void TypedUrlModelAssociator::DiffVisits(
     base::Time new_visit_time =
         base::Time::FromInternalValue(new_url.visits(new_index));
     if (old_visits[old_index].visit_time < new_visit_time) {
-      removed_visits->push_back(old_visits[old_index]);
+      if (new_index > 0) {
+        // If there are visits missing from the start of the node, that
+        // means that they were probably clipped off due to our code that
+        // limits the size of the sync nodes - don't delete them from our
+        // local history.
+        removed_visits->push_back(old_visits[old_index]);
+      }
       ++old_index;
     } else if (old_visits[old_index].visit_time > new_visit_time) {
       new_visits->push_back(history::VisitInfo(