Write attachment to AttachmentStore after downloading

After attachment is downloaded call AttachmentStore::Write. Attachment
is only returned to datatype if Write is successful.

BUG=
R=maniscalco@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#304248}

3 files changed, 51 insertions, 7 deletions

diff --git a/sync/internal_api/attachments/attachment_service_impl.cc b/sync/internal_api/attachments/attachment_service_impl.cc
index dedb10c..10f4790 100644
--- a/sync/internal_api/attachments/attachment_service_impl.cc
+++ b/sync/internal_api/attachments/attachment_service_impl.cc
@@ -218,6 +218,20 @@ void AttachmentServiceImpl::ReadDone(
   }
 }
 
+void AttachmentServiceImpl::WriteDone(
+    const scoped_refptr<GetOrDownloadState>& state,
+    const Attachment& attachment,
+    const AttachmentStore::Result& result) {
+  switch (result) {
+    case AttachmentStore::SUCCESS:
+      state->AddAttachment(attachment);
+      break;
+    case AttachmentStore::UNSPECIFIED_ERROR:
+      state->AddUnavailableAttachmentId(attachment.GetId());
+      break;
+  }
+}
+
 void AttachmentServiceImpl::DropDone(const DropCallback& callback,
                                      const AttachmentStore::Result& result) {
   AttachmentService::DropResult drop_result =
@@ -258,9 +272,15 @@ void AttachmentServiceImpl::DownloadDone(
     const AttachmentDownloader::DownloadResult& result,
     scoped_ptr<Attachment> attachment) {
   switch (result) {
-    case AttachmentDownloader::DOWNLOAD_SUCCESS:
-      state->AddAttachment(*attachment.get());
+    case AttachmentDownloader::DOWNLOAD_SUCCESS: {
+      AttachmentList attachment_list;
+      attachment_list.push_back(*attachment.get());
+      attachment_store_->Write(
+          attachment_list,
+          base::Bind(&AttachmentServiceImpl::WriteDone,
+                     weak_ptr_factory_.GetWeakPtr(), state, *attachment.get()));
       break;
+    }
     case AttachmentDownloader::DOWNLOAD_TRANSIENT_ERROR:
     case AttachmentDownloader::DOWNLOAD_UNSPECIFIED_ERROR:
       state->AddUnavailableAttachmentId(attachment_id);
diff --git a/sync/internal_api/attachments/attachment_service_impl_unittest.cc b/sync/internal_api/attachments/attachment_service_impl_unittest.cc
index 1d6c8a6..db0cff3 100644
--- a/sync/internal_api/attachments/attachment_service_impl_unittest.cc
+++ b/sync/internal_api/attachments/attachment_service_impl_unittest.cc
@@ -312,24 +312,25 @@ TEST_F(AttachmentServiceImplTest, GetOrDownload_Local) {
 }
 
 TEST_F(AttachmentServiceImplTest, GetOrDownload_LocalRemoteUnavailable) {
-  // Create attachment list with 3 ids.
+  // Create attachment list with 4 ids.
   AttachmentIdList attachment_ids;
   attachment_ids.push_back(AttachmentId::Create());
   attachment_ids.push_back(AttachmentId::Create());
   attachment_ids.push_back(AttachmentId::Create());
+  attachment_ids.push_back(AttachmentId::Create());
   // Call attachment service.
   attachment_service()->GetOrDownloadAttachments(attachment_ids,
                                                  download_callback());
   // Ensure AttachmentStore is called.
   EXPECT_FALSE(store()->read_ids.empty());
 
-  // make AttachmentStore return only attachment 0.
+  // Make AttachmentStore return only attachment 0.
   AttachmentIdSet local_attachments;
   local_attachments.insert(attachment_ids[0]);
   store()->RespondToRead(local_attachments);
   RunLoop();
   // Ensure Downloader called with right attachment ids
-  EXPECT_EQ(2U, downloader()->download_requests.size());
+  EXPECT_EQ(3U, downloader()->download_requests.size());
 
   // Make downloader return attachment 1.
   downloader()->RespondToDownload(attachment_ids[1],
@@ -337,11 +338,29 @@ TEST_F(AttachmentServiceImplTest, GetOrDownload_LocalRemoteUnavailable) {
   RunLoop();
   // Ensure consumer callback is not called.
   EXPECT_TRUE(download_results().empty());
+  // Make AttachmentStore acknowledge writing attachment 1.
+  store()->RespondToWrite(AttachmentStore::SUCCESS);
+  RunLoop();
+  // Ensure consumer callback is not called.
+  EXPECT_TRUE(download_results().empty());
+
+  // Make downloader return attachment 2.
+  downloader()->RespondToDownload(attachment_ids[2],
+                                  AttachmentDownloader::DOWNLOAD_SUCCESS);
+  RunLoop();
+  // Ensure consumer callback is not called.
+  EXPECT_TRUE(download_results().empty());
+  // Make AttachmentStore fail writing attachment 2.
+  store()->RespondToWrite(AttachmentStore::UNSPECIFIED_ERROR);
+  RunLoop();
+  // Ensure consumer callback is not called.
+  EXPECT_TRUE(download_results().empty());
 
-  // Make downloader fail attachment 2.
+  // Make downloader fail attachment 3.
   downloader()->RespondToDownload(
-      attachment_ids[2], AttachmentDownloader::DOWNLOAD_UNSPECIFIED_ERROR);
+      attachment_ids[3], AttachmentDownloader::DOWNLOAD_UNSPECIFIED_ERROR);
   RunLoop();
+
   // Ensure callback is called
   EXPECT_FALSE(download_results().empty());
   // There should be only two attachments returned, 0 and 1.
@@ -352,6 +371,8 @@ TEST_F(AttachmentServiceImplTest, GetOrDownload_LocalRemoteUnavailable) {
               last_download_attachments().end());
   EXPECT_TRUE(last_download_attachments().find(attachment_ids[2]) ==
               last_download_attachments().end());
+  EXPECT_TRUE(last_download_attachments().find(attachment_ids[3]) ==
+              last_download_attachments().end());
 }
 
 TEST_F(AttachmentServiceImplTest, GetOrDownload_NoDownloader) {
diff --git a/sync/internal_api/public/attachments/attachment_service_impl.h b/sync/internal_api/public/attachments/attachment_service_impl.h
index ae2e8b3..c9992c0 100644
--- a/sync/internal_api/public/attachments/attachment_service_impl.h
+++ b/sync/internal_api/public/attachments/attachment_service_impl.h
@@ -82,6 +82,9 @@ class SYNC_EXPORT AttachmentServiceImpl
                 const AttachmentStore::Result& result,
                 scoped_ptr<AttachmentMap> attachments,
                 scoped_ptr<AttachmentIdList> unavailable_attachment_ids);
+  void WriteDone(const scoped_refptr<GetOrDownloadState>& state,
+                 const Attachment& attachment,
+                 const AttachmentStore::Result& result);
   void DropDone(const DropCallback& callback,
                 const AttachmentStore::Result& result);
   void UploadDone(const AttachmentUploader::UploadResult& result,