sync: Add GetAllSyncData to sync/api

chrome/browser/sync/glue/generic_change_processor.h

Index: chrome/browser/sync/glue/generic_change_processor.h
diff --git a/chrome/browser/sync/glue/generic_change_processor.h b/chrome/browser/sync/glue/generic_change_processor.h
index a987ebeb4a5993c14749b0edd876d95fd78f6101..9a88a39683a02b988f651c0fe8301ecb9459f16d 100644
--- a/chrome/browser/sync/glue/generic_change_processor.h
+++ b/chrome/browser/sync/glue/generic_change_processor.h
@@ -62,10 +62,16 @@ class GenericChangeProcessor : public ChangeProcessor,
       const tracked_objects::Location& from_here,
       const syncer::SyncChangeList& change_list) OVERRIDE;
 
+  // Fills a list of SyncData. This should create an up to date representation
+  // of all the data known to the ChangeProcessor for |datatype|, and
+  // should match/be a subset of the server's view of that datatype.
+  virtual syncer::SyncDataList GetAllSyncData(syncer::ModelType type)

[haitaol1, 2013/08/19 21:20:06]

This seems redundant with GetSyncDataForType().

[tim (not reviewing), 2013/09/04 23:47:56]

I went back and forth between a few options here. The first was adding a
SyncError return to GetAllSyncData for all data type implementations and
removing GetSyncDataForType here.  As I started drudging through the 70 or so
files that needed updating I realized I was doing the same thing over and over
for every SyncableService -- none of them ever needed to return an error.  That
was enough to convince me it was the wrong approach and was adding complexity. 

I then backtracked to having only a single GetAllSyncData method that returned a
list, but then some unit tests proved to me that was insufficient for the
DataTypeController case, or at least overly awkward.  They shouldn't really
continue with MergeDataAndStartSyncing if there's a hard failure, although in
reality it didn't break anything in a serious way.

I then saw three remaining options.  1) add state to the DataTypeController
(which is the DataTypeErrorHandler) to track whether
OnSingleDataTypeUnrecoverableError was called during GetAllSyncData. It was bad
enough to add state to these complicated classes, but it was really awkward too
because GetAllSyncData is const and after the call we'd inspect state to see if
something changed!  2) Add state to the SharedChangeProcessor to track if it hit
an unrecoverable error.  Bad for the same reasons as 1.  And 3) -- have a
special method for direct clients of SCP and GCP that are typically doing "more
intricate stuff" than the SyncableServices, to allow these classes more
visibility into what happened.  This gives us the best of both worlds.
Simplicity in the SyncableService, and just the right level of granularity
without extra state tracking in the glue layer.

+      const OVERRIDE;
+
   // Fills |current_sync_data| with all the syncer data for the specified type.
   virtual syncer::SyncError GetSyncDataForType(
       syncer::ModelType type,
-      syncer::SyncDataList* current_sync_data);
+      syncer::SyncDataList* current_sync_data) const;
 
   // Returns the number of items for this type.
   virtual int GetSyncCountForType(syncer::ModelType type);