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

sync/engine/get_updates_processor.cc

 // Copyright 2014 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 "sync/engine/get_updates_processor.h"
 
 #include <map>
 
 #include "base/debug/trace_event.h"
 #include "sync/engine/get_updates_delegate.h"
@@ skipping 266 matching lines @@
     sessions::StatusController* status) {
   status->increment_num_updates_downloaded_by(gu_response.entries_size());
 
   // The changes remaining field is used to prevent the client from looping.  If
   // that field is being set incorrectly, we're in big trouble.
   if (!gu_response.has_changes_remaining()) {
     return SERVER_RESPONSE_VALIDATION_FAILED;
   }
   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;
   } else {
     return SERVER_MORE_TO_DOWNLOAD;
   }
 }
 
-bool GetUpdatesProcessor::ProcessGetUpdatesResponse(
+syncer::SyncerError GetUpdatesProcessor::ProcessGetUpdatesResponse(
     ModelTypeSet gu_types,
     const sync_pb::GetUpdatesResponse& gu_response,
     sessions::StatusController* status_controller) {
   TypeSyncEntityMap updates_by_type;
   PartitionUpdatesByType(gu_response, gu_types, &updates_by_type);
   DCHECK_EQ(gu_types.Size(), updates_by_type.size());
 
   TypeToIndexMap progress_index_by_type;
   PartitionProgressMarkersByType(gu_response,
                                  gu_types,
                                  &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;
   PartitionContextMutationsByType(gu_response, gu_types, &context_by_type);
 
   // Iterate over these maps in parallel, processing updates for each type.
   TypeToIndexMap::iterator progress_marker_iter =
       progress_index_by_type.begin();
   TypeSyncEntityMap::iterator updates_iter = updates_by_type.begin();
   for (; (progress_marker_iter != progress_index_by_type.end()
            && updates_iter != updates_by_type.end());
        ++progress_marker_iter, ++updates_iter) {
     DCHECK_EQ(progress_marker_iter->first, updates_iter->first);
     ModelType type = progress_marker_iter->first;
 
     UpdateHandlerMap::iterator update_handler_iter =
         update_handler_map_->find(type);
 
     sync_pb::DataTypeContext context;
     TypeToIndexMap::iterator context_iter = context_by_type.find(type);
     if (context_iter != context_by_type.end())
       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.  "
           << "Type is: " << ModelTypeToString(type);
       continue;
     }
   }
   DCHECK(progress_marker_iter == progress_index_by_type.end() &&
          updates_iter == updates_by_type.end());
 
-  return true;
+  return syncer::SYNCER_OK;
 }
 
 void GetUpdatesProcessor::ApplyUpdates(
     ModelTypeSet gu_types,
     sessions::StatusController* status_controller) {
   delegate_.ApplyUpdates(gu_types, status_controller, update_handler_map_);
 }
 
 void GetUpdatesProcessor::CopyClientDebugInfo(
     sessions::DebugInfoGetter* debug_info_getter,
     sync_pb::DebugInfo* debug_info) {
   DVLOG(1) << "Copying client debug info to send.";
   debug_info_getter->GetDebugInfo(debug_info);
 }
 
 }  // namespace syncer