Add AttachmentUploader interface and fake implementation.

BUG=

Review URL: https://codereview.chromium.org/263903004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@268245 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 216 insertions, 7 deletions

diff --git a/chrome/browser/sync/glue/shared_change_processor.cc b/chrome/browser/sync/glue/shared_change_processor.cc
index 095e50e..b89635f 100644
--- a/chrome/browser/sync/glue/shared_change_processor.cc
+++ b/chrome/browser/sync/glue/shared_change_processor.cc
@@ -11,6 +11,7 @@
 #include "components/sync_driver/sync_api_component_factory.h"
 #include "sync/api/attachments/attachment_service.h"
 #include "sync/api/attachments/fake_attachment_service.h"
+#include "sync/api/attachments/fake_attachment_uploader.h"
 #include "sync/api/sync_change.h"
 
 using base::AutoLock;
@@ -72,11 +73,17 @@ base::WeakPtr<syncer::SyncableService> SharedChangeProcessor::Connect(
     return base::WeakPtr<syncer::SyncableService>();
   }
 
+  // TODO(maniscalco): Use a shared (one per profile) thread-safe instance of
+  // AttachmentUpload instead of creating a new one per AttachmentService (bug
+  // 369536).
+  scoped_ptr<syncer::AttachmentUploader> attachment_uploader(
+      new syncer::FakeAttachmentUploader);
   // TODO(maniscalco): Replace FakeAttachmentService with a real
   // AttachmentService implementation once implemented (bug 356359).
   scoped_ptr<syncer::AttachmentService> attachment_service(
       new syncer::FakeAttachmentService(
-          sync_factory->CreateCustomAttachmentStoreForType(type)));
+          sync_factory->CreateCustomAttachmentStoreForType(type),
+          attachment_uploader.Pass()));
 
   generic_change_processor_ = processor_factory->CreateGenericChangeProcessor(
       sync_service_->GetUserShare(),
diff --git a/components/sync_driver/generic_change_processor_unittest.cc b/components/sync_driver/generic_change_processor_unittest.cc
index 0875908..2434e89 100644
--- a/components/sync_driver/generic_change_processor_unittest.cc
+++ b/components/sync_driver/generic_change_processor_unittest.cc
@@ -11,6 +11,7 @@
 #include "components/sync_driver/data_type_error_handler_mock.h"
 #include "sync/api/attachments/fake_attachment_service.h"
 #include "sync/api/attachments/fake_attachment_store.h"
+#include "sync/api/attachments/fake_attachment_uploader.h"
 #include "sync/api/fake_syncable_service.h"
 #include "sync/api/sync_change.h"
 #include "sync/api/sync_merge_result.h"
@@ -44,8 +45,11 @@ class MockAttachmentService : public syncer::FakeAttachmentService {
 };
 
 MockAttachmentService::MockAttachmentService()
-    : FakeAttachmentService(scoped_ptr<syncer::AttachmentStore>(
-          new syncer::FakeAttachmentStore(base::MessageLoopProxy::current()))) {
+    : FakeAttachmentService(
+          scoped_ptr<syncer::AttachmentStore>(new syncer::FakeAttachmentStore(
+              base::MessageLoopProxy::current())),
+          scoped_ptr<syncer::AttachmentUploader>(
+              new syncer::FakeAttachmentUploader)) {
 }
 
 MockAttachmentService::~MockAttachmentService() {
diff --git a/sync/api/attachments/attachment_uploader.cc b/sync/api/attachments/attachment_uploader.cc
new file mode 100644
index 0000000..db081b1
--- /dev/null
+++ b/sync/api/attachments/attachment_uploader.cc
@@ -0,0 +1,14 @@
+// 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/api/attachments/attachment_uploader.h"
+
+namespace syncer {
+
+AttachmentUploader::AttachmentUploader() {
+}
+AttachmentUploader::~AttachmentUploader() {
+}
+
+}  // namespace syncer
diff --git a/sync/api/attachments/attachment_uploader.h b/sync/api/attachments/attachment_uploader.h
new file mode 100644
index 0000000..325a9e7
--- /dev/null
+++ b/sync/api/attachments/attachment_uploader.h
@@ -0,0 +1,46 @@
+// 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_UPLOADER_H_
+#define SYNC_API_ATTACHMENTS_ATTACHMENT_UPLOADER_H_
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "sync/api/attachments/attachment.h"
+#include "sync/base/sync_export.h"
+
+namespace syncer {
+
+// AttachmentUploader is responsible for uploading attachments to the server.
+class SYNC_EXPORT AttachmentUploader {
+ public:
+  // The result of an UploadAttachment operation.
+  enum UploadResult {
+    UPLOAD_SUCCESS,            // No error, attachment was uploaded
+                               // successfully.
+    UPLOAD_UNSPECIFIED_ERROR,  // An unspecified error occurred.
+  };
+
+  typedef base::Callback<void(const UploadResult&, const AttachmentId&)>
+      UploadCallback;
+
+  AttachmentUploader();
+  virtual ~AttachmentUploader();
+
+  // Upload |attachment| and invoke |callback| when done.
+  //
+  // |callback| will be invoked when the operation has completed (successfully
+  // or otherwise).
+  //
+  // |callback| will receive an UploadResult code and an updated AttachmentId
+  // |containing the server address of the newly uploaded attachment.
+  virtual void UploadAttachment(const Attachment& attachment,
+                                const UploadCallback& callback) = 0;
+};
+
+}  // namespace syncer
+
+#endif  // SYNC_API_ATTACHMENTS_ATTACHMENT_UPLOADER_H_
diff --git a/sync/api/attachments/fake_attachment_service.cc b/sync/api/attachments/fake_attachment_service.cc
index 5cb01e5..f86f380 100644
--- a/sync/api/attachments/fake_attachment_service.cc
+++ b/sync/api/attachments/fake_attachment_service.cc
@@ -8,14 +8,19 @@
 #include "base/message_loop/message_loop.h"
 #include "sync/api/attachments/attachment.h"
 #include "sync/api/attachments/fake_attachment_store.h"
+#include "sync/api/attachments/fake_attachment_uploader.h"
 
 namespace syncer {
 
 FakeAttachmentService::FakeAttachmentService(
-    scoped_ptr<AttachmentStore> attachment_store)
-    : attachment_store_(attachment_store.Pass()), weak_ptr_factory_(this) {
+    scoped_ptr<AttachmentStore> attachment_store,
+    scoped_ptr<AttachmentUploader> attachment_uploader)
+    : attachment_store_(attachment_store.Pass()),
+      attachment_uploader_(attachment_uploader.Pass()),
+      weak_ptr_factory_(this) {
   DCHECK(CalledOnValidThread());
   DCHECK(attachment_store_);
+  DCHECK(attachment_uploader_);
 }
 
 FakeAttachmentService::~FakeAttachmentService() {
@@ -26,8 +31,11 @@ FakeAttachmentService::~FakeAttachmentService() {
 scoped_ptr<syncer::AttachmentService> FakeAttachmentService::CreateForTest() {
   scoped_ptr<syncer::AttachmentStore> attachment_store(
       new syncer::FakeAttachmentStore(base::MessageLoopProxy::current()));
+  scoped_ptr<AttachmentUploader> attachment_uploader(
+      new FakeAttachmentUploader);
   scoped_ptr<syncer::AttachmentService> attachment_service(
-      new syncer::FakeAttachmentService(attachment_store.Pass()));
+      new syncer::FakeAttachmentService(attachment_store.Pass(),
+                                        attachment_uploader.Pass()));
   return attachment_service.Pass();
 }
 
diff --git a/sync/api/attachments/fake_attachment_service.h b/sync/api/attachments/fake_attachment_service.h
index 5bc443a..8159e0f 100644
--- a/sync/api/attachments/fake_attachment_service.h
+++ b/sync/api/attachments/fake_attachment_service.h
@@ -10,6 +10,7 @@
 #include "sync/api/attachments/attachment_service.h"
 #include "sync/api/attachments/attachment_service_proxy.h"
 #include "sync/api/attachments/attachment_store.h"
+#include "sync/api/attachments/attachment_uploader.h"
 
 namespace syncer {
 
@@ -17,7 +18,8 @@ namespace syncer {
 class SYNC_EXPORT FakeAttachmentService : public AttachmentService,
                                           public base::NonThreadSafe {
  public:
-  explicit FakeAttachmentService(scoped_ptr<AttachmentStore> attachment_store);
+  FakeAttachmentService(scoped_ptr<AttachmentStore> attachment_store,
+                        scoped_ptr<AttachmentUploader> attachment_uploader);
   virtual ~FakeAttachmentService();
 
   // Create a FakeAttachmentService suitable for use in tests.
@@ -45,6 +47,7 @@ class SYNC_EXPORT FakeAttachmentService : public AttachmentService,
                  const AttachmentStore::Result& result);
 
   const scoped_ptr<AttachmentStore> attachment_store_;
+  const scoped_ptr<AttachmentUploader> attachment_uploader_;
   // Must be last data member.
   base::WeakPtrFactory<FakeAttachmentService> weak_ptr_factory_;
 
diff --git a/sync/api/attachments/fake_attachment_uploader.cc b/sync/api/attachments/fake_attachment_uploader.cc
new file mode 100644
index 0000000..7ed929d
--- /dev/null
+++ b/sync/api/attachments/fake_attachment_uploader.cc
@@ -0,0 +1,32 @@
+// 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/api/attachments/fake_attachment_uploader.h"
+
+#include "base/bind.h"
+#include "base/message_loop/message_loop.h"
+#include "sync/api/attachments/attachment.h"
+
+namespace syncer {
+
+FakeAttachmentUploader::FakeAttachmentUploader() {
+  DCHECK(CalledOnValidThread());
+}
+
+FakeAttachmentUploader::~FakeAttachmentUploader() {
+  DCHECK(CalledOnValidThread());
+}
+
+void FakeAttachmentUploader::UploadAttachment(const Attachment& attachment,
+                                              const UploadCallback& callback) {
+  DCHECK(CalledOnValidThread());
+  UploadResult result = UPLOAD_SUCCESS;
+  AttachmentId updated_id = attachment.GetId();
+  // TODO(maniscalco): Update the attachment id with server address information
+  // before passing it to the callback.
+  base::MessageLoop::current()->PostTask(
+      FROM_HERE, base::Bind(callback, result, updated_id));
+}
+
+}  // namespace syncer
diff --git a/sync/api/attachments/fake_attachment_uploader.h b/sync/api/attachments/fake_attachment_uploader.h
new file mode 100644
index 0000000..7e48d61
--- /dev/null
+++ b/sync/api/attachments/fake_attachment_uploader.h
@@ -0,0 +1,30 @@
+// 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_FAKE_ATTACHMENT_UPLOADER_H_
+#define SYNC_API_ATTACHMENTS_FAKE_ATTACHMENT_UPLOADER_H_
+
+#include "base/threading/non_thread_safe.h"
+#include "sync/api/attachments/attachment_uploader.h"
+
+namespace syncer {
+
+// A fake implementation of AttachmentUploader.
+class SYNC_EXPORT FakeAttachmentUploader : public AttachmentUploader,
+                                           public base::NonThreadSafe {
+ public:
+  FakeAttachmentUploader();
+  virtual ~FakeAttachmentUploader();
+
+  // AttachmentUploader implementation.
+  virtual void UploadAttachment(const Attachment& attachment,
+                                const UploadCallback& callback) OVERRIDE;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FakeAttachmentUploader);
+};
+
+}  // namespace syncer
+
+#endif  // SYNC_API_ATTACHMENTS_FAKE_ATTACHMENT_UPLOADER_H_
diff --git a/sync/api/attachments/fake_attachment_uploader_unittest.cc b/sync/api/attachments/fake_attachment_uploader_unittest.cc
new file mode 100644
index 0000000..09dccfc
--- /dev/null
+++ b/sync/api/attachments/fake_attachment_uploader_unittest.cc
@@ -0,0 +1,60 @@
+// 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/api/attachments/fake_attachment_uploader.h"
+
+#include "base/bind.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/ref_counted_memory.h"
+#include "base/message_loop/message_loop.h"
+#include "sync/api/attachments/attachment.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace syncer {
+
+namespace {
+
+const char kAttachmentData[] = "some data";
+
+}  // namespace
+
+class FakeAttachmentUploaderTest : public testing::Test {
+ protected:
+  virtual void SetUp() {
+    upload_callback_count = 0;
+    upload_callback = base::Bind(&FakeAttachmentUploaderTest::Increment,
+                                 base::Unretained(this),
+                                 &upload_callback_count);
+  }
+  base::MessageLoop message_loop;
+  FakeAttachmentUploader uploader;
+  int upload_callback_count;
+  AttachmentUploader::UploadCallback upload_callback;
+
+ private:
+  void Increment(int* success_count,
+                 const AttachmentUploader::UploadResult& result,
+                 const AttachmentId& ignored) {
+    if (result == AttachmentUploader::UPLOAD_SUCCESS) {
+      ++(*success_count);
+    }
+  }
+};
+
+// Call upload attachment several times, see that the supplied callback is
+// invoked the same number of times with a result of SUCCESS.
+TEST_F(FakeAttachmentUploaderTest, UploadAttachment) {
+  scoped_refptr<base::RefCountedString> some_data(new base::RefCountedString);
+  some_data->data() = kAttachmentData;
+  Attachment attachment1 = Attachment::Create(some_data);
+  Attachment attachment2 = Attachment::Create(some_data);
+  Attachment attachment3 = Attachment::Create(some_data);
+  uploader.UploadAttachment(attachment1, upload_callback);
+  uploader.UploadAttachment(attachment2, upload_callback);
+  uploader.UploadAttachment(attachment3, upload_callback);
+  message_loop.RunUntilIdle();
+  EXPECT_EQ(upload_callback_count, 3);
+}
+
+}  // namespace syncer
diff --git a/sync/sync_api.gypi b/sync/sync_api.gypi
index 8cee5bd..dfe05f8 100644
--- a/sync/sync_api.gypi
+++ b/sync/sync_api.gypi
@@ -25,10 +25,14 @@
     'api/attachments/attachment_service_proxy_for_test.h',
     'api/attachments/attachment_store.cc',
     'api/attachments/attachment_store.h',
+    'api/attachments/attachment_uploader.cc',
+    'api/attachments/attachment_uploader.h',
     'api/attachments/fake_attachment_service.cc',
     'api/attachments/fake_attachment_service.h',
     'api/attachments/fake_attachment_store.cc',
     'api/attachments/fake_attachment_store.h',
+    'api/attachments/fake_attachment_uploader.cc',
+    'api/attachments/fake_attachment_uploader.h',
     'api/string_ordinal.h',
     'api/syncable_service.cc',
     'api/syncable_service.h',
diff --git a/sync/sync_tests.gypi b/sync/sync_tests.gypi
index 11484e6..b7cee38 100644
--- a/sync/sync_tests.gypi
+++ b/sync/sync_tests.gypi
@@ -476,6 +476,7 @@
           'api/attachments/attachment_id_unittest.cc',
           'api/attachments/attachment_service_proxy_unittest.cc',
           'api/attachments/fake_attachment_store_unittest.cc',
+          'api/attachments/fake_attachment_uploader_unittest.cc',
           'api/sync_change_unittest.cc',
           'api/sync_data_unittest.cc',
           'api/sync_error_unittest.cc',