Update sync API to support attachments.

sync/api/sync_data.h

Index: sync/api/sync_data.h
diff --git a/sync/api/sync_data.h b/sync/api/sync_data.h
index 43f64f9185b10f798abe3086aec60fd85310d11e..34cdf1c4d69423ec57a302ab149d2ea57867de1e 100644
--- a/sync/api/sync_data.h
+++ b/sync/api/sync_data.h
@@ -10,10 +10,15 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/callback.h"
+#include "base/stl_util.h"
 #include "base/time/time.h"
+#include "sync/api/attachments/attachment.h"
+#include "sync/api/attachments/attachment_service_proxy.h"
 #include "sync/base/sync_export.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/util/immutable.h"
+#include "sync/internal_api/public/util/weak_handle.h"
 
 namespace sync_pb {
 class EntitySpecifics;
@@ -22,15 +27,17 @@ class SyncEntity;
 
 namespace syncer {
 
+class AttachmentService;
+
 // A light-weight container for immutable sync data. Pass-by-value and storage
 // in STL containers are supported and encouraged if helpful.
 class SYNC_EXPORT SyncData {
  public:
   // Creates an empty and invalid SyncData.
   SyncData();
-   ~SyncData();
+  ~SyncData();
 
-   // Default copy and assign welcome.
+  // Default copy and assign welcome.
 
   // Helper methods for creating SyncData objects for local data.
   // The sync tag must be a string unique to this datatype and is used as a node
@@ -48,12 +55,25 @@ class SYNC_EXPORT SyncData {
       const std::string& sync_tag,
       const std::string& non_unique_title,
       const sync_pb::EntitySpecifics& specifics);
+  static SyncData CreateLocalDataWithAttachments(
+      const std::string& sync_tag,
+      const std::string& non_unique_title,
+      const sync_pb::EntitySpecifics& specifics,
+      const AttachmentList& attachments);
 
   // Helper method for creating SyncData objects originating from the syncer.
+  //
+  // TODO(maniscalco): Replace all calls to 3-arg CreateRemoteData with calls to
+  // the 5-arg version (bug 353296).
   static SyncData CreateRemoteData(
       int64 id,
       const sync_pb::EntitySpecifics& specifics,
-      const base::Time& last_modified_time);
+      const base::Time& last_modified_time,
+      const AttachmentIdList& attachment_ids,
+      const syncer::AttachmentServiceProxy& attachment_service);
+  static SyncData CreateRemoteData(int64 id,
+                                   const sync_pb::EntitySpecifics& specifics,
+                                   const base::Time& last_modified_time);
 
   // Whether this SyncData holds valid data. The only way to have a SyncData
   // without valid data is to use the default constructor.
@@ -87,6 +107,42 @@ class SYNC_EXPORT SyncData {
 
   std::string ToString() const;
 
+  // Return a list of this SyncData's attachment ids.
+  //
+  // The attachments may or may not be present on this host.

[tim (not reviewing), 2014/03/27 18:35:46]

s/host/device to be more consistent with the terminology we tend to use in sync.
host is a bit ambiguous, SyncBackendHost, etc. I was going to suggest "s/present
on this host/present locally", but that, too is ambiguous in this file since
"Local" has a special meaning.

[maniscalco, 2014/03/27 21:17:31]

Done.

+  AttachmentIdList GetAttachmentIds() const;
+
+  // Return a list of this SyncData's attachments.

[tim (not reviewing), 2014/03/27 18:35:46]

This is only ever the list of attachments intended for upload... so that comment
suggestion wouldn't really jive with "may only be called when IsLocal is true".

This is kind of an inherent problem of SyncData.  We just chatted offline about
potentially splitting state out into SyncData.local()[.tag(),.title(),...] and
SyncData.remote()[.modification_time(), id()] to make it more obvious what's
going on.  Keeping things like GetOrDownloadAttachments on SyncData seems fine
(it would dcheck internally), but for state that is actually currently hanging
off of a SyncData, that split up might really help.  

[tim (not reviewing), 2014/03/27 19:16:44]

Alternatively, there's the subclass + factory method [ e.g RemoteSyncData*
SyncData::BuildRemoteSyncDataOutOf(SyncData data)].  

For now though, given that this is mostly an existing problem let's try to
checkpoint what you have now.  Perhaps rename GetAttachments to
GetLocalAttachmentsForUpload with a bug/todo about how you plan to fix this, and
then focus on fixing it next.

[maniscalco, 2014/03/27 21:17:31]

SGTM.  Renamed to GetLocalAttachmentsForUpload.  Bug created (crbug/357305).

+  //
+  // May only be called when IsLocal() is true.
+  const AttachmentList& GetAttachments() const;
+
+  // Retrieve the attachments indentified by |attachment_ids|. Invoke |callback|
+  // with the requested attachments.
+  //
+  // May only be called when IsLocal() is false.
+  //
+  // |callback| will be invoked when the operation is complete (successfully or
+  // otherwise).
+  //
+  // Retrieving the requested attachments may require reading local storage or
+  // requesting the attachments from the network.
+  //
+  void GetOrDownloadAttachments(
+      const AttachmentIdList& attachment_ids,
+      const AttachmentService::GetOrDownloadCallback& callback);
+
+  // Drop (delete from local storage) the attachments associated with this
+  // SyncData specified in |attachment_ids|. This method will not delete
+  // attachments from the server.
+  //
+  // May only be called when IsLocal() is false.
+  //
+  // |callback| will be invoked when the operation is complete (successfully or
+  // otherwise).
+  void DropAttachments(const AttachmentIdList& attachment_ids,
+                       const AttachmentService::DropCallback& callback);
+
   // TODO(zea): Query methods for other sync properties: parent, successor, etc.
 
  private:
@@ -109,10 +165,13 @@ class SYNC_EXPORT SyncData {
   typedef Immutable<sync_pb::SyncEntity, ImmutableSyncEntityTraits>
       ImmutableSyncEntity;
 
-  // Clears |entity|.
-  SyncData(int64 id,
-           sync_pb::SyncEntity* entity,
-           const base::Time& remote_modification_time);
+  // Clears |entity| and |attachments|.
+  SyncData(
+      int64 id,
+      sync_pb::SyncEntity* entity,
+      AttachmentList* attachments,
+      const base::Time& remote_modification_time,
+      const syncer::AttachmentServiceProxy& attachment_service);
 
   // Whether this SyncData holds valid data.
   bool is_valid_;
@@ -126,6 +185,10 @@ class SYNC_EXPORT SyncData {
 
   // The actual shared sync entity being held.
   ImmutableSyncEntity immutable_entity_;
+
+  Immutable<AttachmentList> attachments_;
+
+  AttachmentServiceProxy attachment_service_;
 };
 
 // gmock printer helper.