[Sync] Add support for retrying a getupdates due to a context change

sync/engine/get_updates_processor.cc

Index: sync/engine/get_updates_processor.cc
diff --git a/sync/engine/get_updates_processor.cc b/sync/engine/get_updates_processor.cc
index d0aa9b989fd5c2c1f642211e98f0b2a1988a4762..41c9e4f033f1a2de2c60ee0dd3f2d6ce34e65a47 100644
--- a/sync/engine/get_updates_processor.cc
+++ b/sync/engine/get_updates_processor.cc
@@ -284,9 +284,10 @@ SyncerError GetUpdatesProcessor::ProcessResponse(
   }
   status->set_num_server_changes_remaining(gu_response.changes_remaining());
 
-  if (!ProcessGetUpdatesResponse(request_types, gu_response, status)) {
-    return SERVER_RESPONSE_VALIDATION_FAILED;
-  }
+  syncer::SyncerError result =
+      ProcessGetUpdatesResponse(request_types, gu_response, status);
+  if (result != syncer::SYNCER_OK)
+    return result;
 
   if (gu_response.changes_remaining() == 0) {
     return SYNCER_OK;
@@ -295,7 +296,7 @@ SyncerError GetUpdatesProcessor::ProcessResponse(
   }
 }
 
-bool GetUpdatesProcessor::ProcessGetUpdatesResponse(
+syncer::SyncerError GetUpdatesProcessor::ProcessGetUpdatesResponse(
     ModelTypeSet gu_types,
     const sync_pb::GetUpdatesResponse& gu_response,
     sessions::StatusController* status_controller) {
@@ -309,7 +310,7 @@ bool GetUpdatesProcessor::ProcessGetUpdatesResponse(
                                  &progress_index_by_type);
   if (gu_types.Size() != progress_index_by_type.size()) {
     NOTREACHED() << "Missing progress markers in GetUpdates response.";
-    return false;
+    return syncer::SERVER_RESPONSE_VALIDATION_FAILED;
   }
 
   TypeToIndexMap context_by_type;
@@ -334,11 +335,13 @@ bool GetUpdatesProcessor::ProcessGetUpdatesResponse(
       context.CopyFrom(gu_response.context_mutations(context_iter->second));
 
     if (update_handler_iter != update_handler_map_->end()) {
-      update_handler_iter->second->ProcessGetUpdatesResponse(
-          gu_response.new_progress_marker(progress_marker_iter->second),
-          context,
-          updates_iter->second,
-          status_controller);
+      if (!update_handler_iter->second->ProcessGetUpdatesResponse(

[rlarocque, 2014/04/10 01:15:29]

Could we have ProcessGetUpdatesResponse return an explicit error code?  I think
this code would be clearer if it returned a SyncerError enum..

Inferring that a false return value means DATATYPE_TRIGGERED_RETRY seems
brittle.

[Nicolas Zea, 2014/04/10 19:06:26]

Done.

+              gu_response.new_progress_marker(progress_marker_iter->second),
+              context,
+              updates_iter->second,
+              status_controller)) {
+        return syncer::DATATYPE_TRIGGERED_RETRY;
+      }
     } else {
       DLOG(WARNING)
           << "Ignoring received updates of a type we can't handle.  "
@@ -349,7 +352,7 @@ bool GetUpdatesProcessor::ProcessGetUpdatesResponse(
   DCHECK(progress_marker_iter == progress_index_by_type.end() &&
          updates_iter == updates_by_type.end());
 
-  return true;
+  return syncer::SYNCER_OK;
 }
 
 void GetUpdatesProcessor::ApplyUpdates(