sync: Add GetAllSyncData to sync/api

chrome/browser/sync/glue/generic_change_processor.h

 // Copyright (c) 2012 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.
 
 #ifndef CHROME_BROWSER_SYNC_GLUE_GENERIC_CHANGE_PROCESSOR_H_
 #define CHROME_BROWSER_SYNC_GLUE_GENERIC_CHANGE_PROCESSOR_H_
 
 #include <vector>
 
 #include "base/compiler_specific.h"
@@ skipping 44 matching lines @@
       const syncer::ImmutableChangeRecordList& changes) OVERRIDE;
   // Passes |syncer_changes_|, built in ApplyChangesFromSyncModel, onto
   // |local_service_| by way of its ProcessSyncChanges method.
   virtual void CommitChangesFromSyncModel() OVERRIDE;
 
   // syncer::SyncChangeProcessor implementation.
   virtual syncer::SyncError ProcessSyncChanges(
       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);
 
   // Generic versions of AssociatorInterface methods. Called by
   // syncer::SyncableServiceAdapter or the DataTypeController.
   virtual bool SyncModelHasUserCreatedNodes(syncer::ModelType type,
                                             bool* has_nodes);
   virtual bool CryptoReadyIfNecessary(syncer::ModelType type);
 
@@ skipping 24 matching lines @@
   // when it starts up). As such we can't wait until Start(_) has been called,
   // and have to keep a local pointer to the user_share.
   syncer::UserShare* const share_handle_;
 
   DISALLOW_COPY_AND_ASSIGN(GenericChangeProcessor);
 };
 
 }  // namespace browser_sync
 
 #endif  // CHROME_BROWSER_SYNC_GLUE_GENERIC_CHANGE_PROCESSOR_H_