Invoke AttachmentUploader and update AttachmentIds.

sync/api/attachments/attachment_service.h

 // 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.
 
 #ifndef SYNC_API_ATTACHMENTS_ATTACHMENT_SERVICE_H_
 #define SYNC_API_ATTACHMENTS_ATTACHMENT_SERVICE_H_
 
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 36 matching lines @@
   // The result of a StoreAttachments operation.
   enum StoreResult {
     STORE_SUCCESS,            // No error, all attachments stored (at least
                               // locally).
     STORE_UNSPECIFIED_ERROR,  // An unspecified error occurred. Some or all
                               // attachments may not have been stored.
   };
 
   typedef base::Callback<void(const StoreResult&)> StoreCallback;
 
+  // An interface that embedder code implements to be notified about different
+  // events that originate from AttachmentService.
+  class Delegate {

[maniscalco, 2014/05/09 18:12:11]

I'm thinking about in which thread the delegate will be called.  It probably
deserves a comment that it will be called in the same thread from which the
AttachmentService was called.

[pavely, 2014/05/13 23:06:44]

Done.

+   public:
+    virtual ~Delegate() {}
+
+    // Attachment is uploaded to server and attachment_id is updated with server
+    // url.
+    virtual void OnAttachmentUploaded(const AttachmentId& attachment_id) = 0;
+  };
+
   AttachmentService();
   virtual ~AttachmentService();
 
+  // Set delegate pointer. AttachmentService doesn't own delegate.

[maniscalco, 2014/05/09 18:12:11]

Can we avoid adding a no-op implementation here?  By putting this setter on the
interface (AttachmentService) we're telling the world that every
AttachmentService is capable of getting a delegate, but that's not really true
(e.g. AttachmentServiceProxy).  Let's chat about this one.  I see that we wan't
just pass the delegate to the ctor of the derived type because GCP *is* the
delegate so it's not immediately obvious to me how we can accomplish this.

[pavely, 2014/05/13 23:06:44]

The reason was to not add implementation of it to AttachmentServiceProxy , but
you are right, I'd rather add NOTREACHED there.

On 2014/05/09 18:12:11, maniscalco wrote:

+  virtual void SetDelegate(Delegate* delegate) {}

[maniscalco, 2014/05/09 18:12:11]

In the future we may move the delegate functionality out of GCP at which point
the AS may outlive its delegate.  I suggest updating AS to take a Delegate via
WeakPtr.

[pavely, 2014/05/13 23:06:44]

Right now interface simple, explicit and will blow up if developer does
something wrong. It is similar to AddObserver/RemoveObserver pattern:
AttachmentService already exists when GCP is created, GCP sets itself as
delegate at init and removes when it goes away.

WeakPtr on the other hand offer pattern where mistakes go unnoticed. It will
silently work without reporting any errors, only sometimes (maybe often) it
won't update IDs after upload and we wouldn't even know it.

I'd say let's keep it simple for now and change it to WeakPtr when we have case
for it.
On 2014/05/09 18:12:11, maniscalco wrote:

+
   // See SyncData::GetOrDownloadAttachments.
   virtual void GetOrDownloadAttachments(
       const AttachmentIdList& attachment_ids,
       const GetOrDownloadCallback& callback) = 0;
 
   // See SyncData::DropAttachments.
   virtual void DropAttachments(const AttachmentIdList& attachment_ids,
                                const DropCallback& callback) = 0;
 
   // Store |attachments| on device and (eventually) upload them to the server.
@@ skipping 11 matching lines @@
   // This method should be called when a SyncData is about to be updated so we
   // can remove unreferenced attachments from local storage and ensure new
   // attachments are persisted and uploaded to the sync server.
   virtual void OnSyncDataUpdate(const AttachmentIdList& old_attachment_ids,
                                 const SyncData& updated_sync_data) = 0;
 };
 
 }  // namespace syncer
 
 #endif  // SYNC_API_ATTACHMENTS_ATTACHMENT_SERVICE_H_