Implementation of OnDiskAttachmentStore.

In this change only attachment payload is stored, support for metadata
will be in separate change.
Also in_memory_attachment_store_unittest is moved into test_template.
This test is now used for both stores.

BUG=
R=maniscalco@chromium.org

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

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

sync/BUILD.gn

@@ -126,6 +126,7 @@ source_set("sync_core") {
     "internal_api/attachments/fake_attachment_downloader.cc",
     "internal_api/attachments/fake_attachment_uploader.cc",
     "internal_api/attachments/in_memory_attachment_store.cc",
+    "internal_api/attachments/on_disk_attachment_store.cc",
     "internal_api/base_node.cc",
     "internal_api/base_transaction.cc",
     "internal_api/change_record.cc",
@@ -164,6 +165,7 @@ source_set("sync_core") {
     "internal_api/public/attachments/fake_attachment_downloader.h",
     "internal_api/public/attachments/fake_attachment_uploader.h",
     "internal_api/public/attachments/in_memory_attachment_store.h",
+    "internal_api/public/attachments/on_disk_attachment_store.h",
     "internal_api/public/base/attachment_id_proto.cc",
     "internal_api/public/base/attachment_id_proto.h",
     "internal_api/public/base/cancelation_observer.cc",
@@ -565,9 +567,12 @@ test("sync_unit_tests") {
     "internal_api/attachments/attachment_service_impl_unittest.cc",
     "internal_api/attachments/attachment_service_proxy_unittest.cc",
     "internal_api/attachments/attachment_store_handle_unittest.cc",
+    "internal_api/attachments/attachment_store_test_template.h",
     "internal_api/attachments/attachment_uploader_impl_unittest.cc",
     "internal_api/attachments/fake_attachment_downloader_unittest.cc",
     "internal_api/attachments/fake_attachment_uploader_unittest.cc",
+    "internal_api/attachments/in_memory_attachment_store_unittest.cc",
+    "internal_api/attachments/on_disk_attachment_store_unittest.cc",
     "internal_api/debug_info_event_listener_unittest.cc",
     "internal_api/http_bridge_unittest.cc",
     "internal_api/js_mutation_event_observer_unittest.cc",

sync/api/attachments/attachment_store.cc

@@ -4,9 +4,14 @@
 
 #include "sync/api/attachments/attachment_store.h"
 
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/location.h"
+#include "base/sequenced_task_runner.h"
 #include "base/thread_task_runner_handle.h"
 #include "sync/internal_api/public/attachments/attachment_store_handle.h"
 #include "sync/internal_api/public/attachments/in_memory_attachment_store.h"
+#include "sync/internal_api/public/attachments/on_disk_attachment_store.h"
 
 namespace syncer {
 
@@ -24,4 +29,47 @@ scoped_refptr<AttachmentStore> AttachmentStore::CreateInMemoryStore() {
       backend.Pass(), base::ThreadTaskRunnerHandle::Get()));
 }
 
+void AttachmentStore::CreateOnDiskStore(
+    const base::FilePath& path,
+    const scoped_refptr<base::SequencedTaskRunner>& backend_task_runner,
+    const CreateCallback& callback) {
+  scoped_refptr<base::SequencedTaskRunner> frontend_task_runner =
+      base::ThreadTaskRunnerHandle::Get();
+  backend_task_runner->PostTask(FROM_HERE,
+                                base::Bind(&CreateOnDiskStoreOnBackendThread,
+                                           path,
+                                           frontend_task_runner,
+                                           callback));
+}
+
+void AttachmentStore::CreateOnDiskStoreOnBackendThread(
+    const base::FilePath& path,
+    const scoped_refptr<base::SequencedTaskRunner>& frontend_task_runner,
+    const CreateCallback& callback) {
+  scoped_ptr<OnDiskAttachmentStore> store(
+      new OnDiskAttachmentStore(frontend_task_runner));
+  Result result = store->OpenOrCreate(path);
+  if (result != SUCCESS)
+    store.reset();
+  frontend_task_runner->PostTask(FROM_HERE,
+                                 base::Bind(&CreateBackendDone,
+                                            result,
+                                            base::Passed(&store),
+                                            base::ThreadTaskRunnerHandle::Get(),
+                                            callback));
+}
+
+void AttachmentStore::CreateBackendDone(
+    const Result& result,
+    scoped_ptr<AttachmentStoreBase> backend,
+    const scoped_refptr<base::SequencedTaskRunner>& backend_task_runner,
+    const CreateCallback& callback) {
+  scoped_refptr<AttachmentStore> store;
+  if (result == SUCCESS) {
+    store = new AttachmentStoreHandle(backend.Pass(), backend_task_runner);
+  }
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::Bind(callback, result, store));
+}
+
 }  // namespace syncer

sync/api/attachments/attachment_store.h

@@ -13,7 +13,9 @@
 #include "sync/base/sync_export.h"
 
 namespace base {
+class FilePath;
 class RefCountedMemory;
+class SequencedTaskRunner;
 }  // namespace base
 
 namespace syncer {
@@ -90,15 +92,42 @@ class SYNC_EXPORT AttachmentStore
     : public AttachmentStoreBase,
       public base::RefCountedThreadSafe<AttachmentStore> {
  public:
+  typedef base::Callback<void(const Result&,
+                              const scoped_refptr<AttachmentStore>&)>
+      CreateCallback;
+
   AttachmentStore();
 
   // Creates an AttachmentStoreHandle backed by in-memory implementation of
   // attachment store. For now frontend lives on the same thread as backend.
   static scoped_refptr<AttachmentStore> CreateInMemoryStore();
 
+  // Creates an AttachmentStoreHandle backed by on-disk implementation of
+  // attachment store. Opens corresponding leveldb database located at |path|.
+  // All backend operations are scheduled to |backend_task_runner|. Opening
+  // attachment store is asynchronous, once it finishes |callback| will be
+  // called on the thread that called CreateOnDiskStore. |store| parameter of
+  // callback is only valid when |result| is SUCCESS.
+  static void CreateOnDiskStore(
+      const base::FilePath& path,
+      const scoped_refptr<base::SequencedTaskRunner>& backend_task_runner,
+      const CreateCallback& callback);
+
  protected:
   friend class base::RefCountedThreadSafe<AttachmentStore>;
   virtual ~AttachmentStore();
+
+ private:
+  static void CreateOnDiskStoreOnBackendThread(
+      const base::FilePath& path,
+      const scoped_refptr<base::SequencedTaskRunner>& frontend_task_runner,
+      const CreateCallback& callback);
+
+  static void CreateBackendDone(
+      const Result& result,
+      scoped_ptr<AttachmentStoreBase> backend,
+      const scoped_refptr<base::SequencedTaskRunner>& backend_task_runner,
+      const CreateCallback& callback);
 };
 
 }  // namespace syncer

sync/internal_api/attachments/attachment_store_handle_unittest.cc

@@ -79,17 +79,13 @@ class AttachmentStoreHandleTest : public testing::Test {
         backend.Pass(), base::ThreadTaskRunnerHandle::Get());
   }
 
-  static void ReadDone(const AttachmentStore::Result& result,
-                       scoped_ptr<AttachmentMap> attachments,
-                       scoped_ptr<AttachmentIdList> unavailable_attachments) {
+  static void DoneWithResult(const AttachmentStore::Result& result) {
     NOTREACHED();
   }
 
-  static void WriteDone(const AttachmentStore::Result& result) {
-    NOTREACHED();
-  }
-
-  static void DropDone(const AttachmentStore::Result& result) {
+  static void ReadDone(const AttachmentStore::Result& result,
+                       scoped_ptr<AttachmentMap> attachments,
+                       scoped_ptr<AttachmentIdList> unavailable_attachments) {
     NOTREACHED();
   }
 
@@ -126,13 +122,13 @@ TEST_F(AttachmentStoreHandleTest, MethodsCalled) {
   EXPECT_EQ(read_call_count_, 1);
 
   attachment_store_handle_->Write(
-      attachments, base::Bind(&AttachmentStoreHandleTest::WriteDone));
+      attachments, base::Bind(&AttachmentStoreHandleTest::DoneWithResult));
   EXPECT_EQ(write_call_count_, 0);
   RunMessageLoop();
   EXPECT_EQ(write_call_count_, 1);
 
   attachment_store_handle_->Drop(
-      ids, base::Bind(&AttachmentStoreHandleTest::DropDone));
+      ids, base::Bind(&AttachmentStoreHandleTest::DoneWithResult));
   EXPECT_EQ(drop_call_count_, 0);
   RunMessageLoop();
   EXPECT_EQ(drop_call_count_, 1);

sync/internal_api/attachments/in_memory_attachment_store_unittest.cc

@@ -4,256 +4,22 @@
 
 #include "sync/api/attachments/attachment_store.h"
 
-#include "base/bind.h"
-#include "base/memory/ref_counted_memory.h"
-#include "base/memory/scoped_ptr.h"
-#include "base/message_loop/message_loop.h"
-#include "base/thread_task_runner_handle.h"
-#include "sync/api/attachments/attachment.h"
-#include "sync/protocol/sync.pb.h"
-#include "testing/gtest/include/gtest/gtest.h"
+#include "sync/internal_api/attachments/attachment_store_test_template.h"
 
 namespace syncer {
 
-const char kTestData1[] = "test data 1";
-const char kTestData2[] = "test data 2";
+class InMemoryAttachmentStoreFactory {
+ public:
+  InMemoryAttachmentStoreFactory() {}
+  ~InMemoryAttachmentStoreFactory() {}
 
-class InMemoryAttachmentStoreTest : public testing::Test {
- protected:
-  base::MessageLoop message_loop;
-  scoped_refptr<AttachmentStore> store;
-  AttachmentStore::Result result;
-  scoped_ptr<AttachmentMap> attachments;
-  scoped_ptr<AttachmentIdList> failed_attachment_ids;
-
-  AttachmentStore::ReadCallback read_callback;
-  AttachmentStore::WriteCallback write_callback;
-  AttachmentStore::DropCallback drop_callback;
-
-  scoped_refptr<base::RefCountedString> some_data1;
-  scoped_refptr<base::RefCountedString> some_data2;
-
-  InMemoryAttachmentStoreTest()
-      : store(AttachmentStore::CreateInMemoryStore()) {}
-
-  virtual void SetUp() {
-    Clear();
-    read_callback =
-        base::Bind(&InMemoryAttachmentStoreTest::CopyResultAttachments,
-                   base::Unretained(this),
-                   &result,
-                   &attachments,
-                   &failed_attachment_ids);
-    write_callback = base::Bind(&InMemoryAttachmentStoreTest::CopyResult,
-                                base::Unretained(this),
-                                &result);
-    drop_callback = write_callback;
-
-    some_data1 = new base::RefCountedString;
-    some_data1->data() = kTestData1;
-
-    some_data2 = new base::RefCountedString;
-    some_data2->data() = kTestData2;
-  }
-
-  virtual void ClearAndPumpLoop() {
-    Clear();
-    message_loop.RunUntilIdle();
-  }
-
- private:
-  void Clear() {
-    result = AttachmentStore::UNSPECIFIED_ERROR;
-    attachments.reset();
-    failed_attachment_ids.reset();
-  }
-
-  void CopyResult(AttachmentStore::Result* destination_result,
-                  const AttachmentStore::Result& source_result) {
-    *destination_result = source_result;
-  }
-
-  void CopyResultAttachments(
-      AttachmentStore::Result* destination_result,
-      scoped_ptr<AttachmentMap>* destination_attachments,
-      scoped_ptr<AttachmentIdList>* destination_failed_attachment_ids,
-      const AttachmentStore::Result& source_result,
-      scoped_ptr<AttachmentMap> source_attachments,
-      scoped_ptr<AttachmentIdList> source_failed_attachment_ids) {
-    CopyResult(destination_result, source_result);
-    *destination_attachments = source_attachments.Pass();
-    *destination_failed_attachment_ids = source_failed_attachment_ids.Pass();
+  scoped_refptr<AttachmentStore> CreateAttachmentStore() {
+    return AttachmentStore::CreateInMemoryStore();
   }
 };
 
-// Verify that we do not overwrite existing attachments and that we do not treat
-// it as an error.
-TEST_F(InMemoryAttachmentStoreTest, Write_NoOverwriteNoError) {
-  // Create two attachments with the same id but different data.
-  Attachment attachment1 = Attachment::Create(some_data1);
-  Attachment attachment2 =
-      Attachment::CreateWithId(attachment1.GetId(), some_data2);
-
-  // Write the first one.
-  AttachmentList some_attachments;
-  some_attachments.push_back(attachment1);
-  store->Write(some_attachments, write_callback);
-  ClearAndPumpLoop();
-  EXPECT_EQ(result, AttachmentStore::SUCCESS);
-
-  // Write the second one.
-  some_attachments.clear();
-  some_attachments.push_back(attachment2);
-  store->Write(some_attachments, write_callback);
-  ClearAndPumpLoop();
-  EXPECT_EQ(result, AttachmentStore::SUCCESS);
-
-  // Read it back and see that it was not overwritten.
-  AttachmentIdList some_attachment_ids;
-  some_attachment_ids.push_back(attachment1.GetId());
-  store->Read(some_attachment_ids, read_callback);
-  ClearAndPumpLoop();
-  EXPECT_EQ(result, AttachmentStore::SUCCESS);
-  EXPECT_EQ(attachments->size(), 1U);
-  EXPECT_EQ(failed_attachment_ids->size(), 0U);
-  AttachmentMap::const_iterator a1 = attachments->find(attachment1.GetId());
-  EXPECT_TRUE(a1 != attachments->end());
-  EXPECT_TRUE(attachment1.GetData()->Equals(a1->second.GetData()));
-}
-
-// Verify that we can write some attachments and read them back.
-TEST_F(InMemoryAttachmentStoreTest, Write_RoundTrip) {
-  Attachment attachment1 = Attachment::Create(some_data1);
-  Attachment attachment2 = Attachment::Create(some_data2);
-  AttachmentList some_attachments;
-  some_attachments.push_back(attachment1);
-  some_attachments.push_back(attachment2);
-
-  store->Write(some_attachments, write_callback);
-  ClearAndPumpLoop();
-  EXPECT_EQ(result, AttachmentStore::SUCCESS);
-
-  AttachmentIdList some_attachment_ids;
-  some_attachment_ids.push_back(attachment1.GetId());
-  some_attachment_ids.push_back(attachment2.GetId());
-  store->Read(some_attachment_ids, read_callback);
-  ClearAndPumpLoop();
-  EXPECT_EQ(result, AttachmentStore::SUCCESS);
-  EXPECT_EQ(attachments->size(), 2U);
-  EXPECT_EQ(failed_attachment_ids->size(), 0U);
-
-  AttachmentMap::const_iterator a1 = attachments->find(attachment1.GetId());
-  EXPECT_TRUE(a1 != attachments->end());
-  EXPECT_TRUE(attachment1.GetData()->Equals(a1->second.GetData()));
-
-  AttachmentMap::const_iterator a2 = attachments->find(attachment2.GetId());
-  EXPECT_TRUE(a2 != attachments->end());
-  EXPECT_TRUE(attachment2.GetData()->Equals(a2->second.GetData()));
-}
-
-// Try to read two attachments when only one exists.
-TEST_F(InMemoryAttachmentStoreTest, Read_OneNotFound) {
-  Attachment attachment1 = Attachment::Create(some_data1);
-  Attachment attachment2 = Attachment::Create(some_data2);
-
-  AttachmentList some_attachments;
-  // Write attachment1 only.
-  some_attachments.push_back(attachment1);
-  store->Write(some_attachments, write_callback);
-  ClearAndPumpLoop();
-  EXPECT_EQ(result, AttachmentStore::SUCCESS);
-
-  // Try to read both attachment1 and attachment2.
-  AttachmentIdList ids;
-  ids.push_back(attachment1.GetId());
-  ids.push_back(attachment2.GetId());
-  store->Read(ids, read_callback);
-  ClearAndPumpLoop();
-
-  // See that only attachment1 was read.
-  EXPECT_EQ(result, AttachmentStore::UNSPECIFIED_ERROR);
-  EXPECT_EQ(attachments->size(), 1U);
-  EXPECT_EQ(failed_attachment_ids->size(), 1U);
-}
-
-// Try to drop two attachments when only one exists. Verify that no error occurs
-// and that the existing attachment was dropped.
-TEST_F(InMemoryAttachmentStoreTest, Drop_DropTwoButOnlyOneExists) {
-  // First, create two attachments.
-  Attachment attachment1 = Attachment::Create(some_data1);
-  Attachment attachment2 = Attachment::Create(some_data2);
-  AttachmentList some_attachments;
-  some_attachments.push_back(attachment1);
-  some_attachments.push_back(attachment2);
-  store->Write(some_attachments, write_callback);
-  ClearAndPumpLoop();
-  EXPECT_EQ(result, AttachmentStore::SUCCESS);
-
-  // Drop attachment1 only.
-  AttachmentIdList ids;
-  ids.push_back(attachment1.GetId());
-  store->Drop(ids, drop_callback);
-  ClearAndPumpLoop();
-  EXPECT_EQ(result, AttachmentStore::SUCCESS);
-
-  // See that attachment1 is gone.
-  store->Read(ids, read_callback);
-  ClearAndPumpLoop();
-  EXPECT_EQ(result, AttachmentStore::UNSPECIFIED_ERROR);
-  EXPECT_EQ(attachments->size(), 0U);
-  EXPECT_EQ(failed_attachment_ids->size(), 1U);
-  EXPECT_EQ((*failed_attachment_ids)[0], attachment1.GetId());
-
-  // Drop both attachment1 and attachment2.
-  ids.clear();
-  ids.push_back(attachment1.GetId());
-  ids.push_back(attachment2.GetId());
-  store->Drop(ids, drop_callback);
-  ClearAndPumpLoop();
-  EXPECT_EQ(result, AttachmentStore::SUCCESS);
-
-  // See that attachment2 is now gone.
-  ids.clear();
-  ids.push_back(attachment2.GetId());
-  store->Read(ids, read_callback);
-  ClearAndPumpLoop();
-  EXPECT_EQ(result, AttachmentStore::UNSPECIFIED_ERROR);
-  EXPECT_EQ(attachments->size(), 0U);
-  EXPECT_EQ(failed_attachment_ids->size(), 1U);
-  EXPECT_EQ((*failed_attachment_ids)[0], attachment2.GetId());
-}
-
-// Verify that attempting to drop an attachment that does not exist is not an
-// error.
-TEST_F(InMemoryAttachmentStoreTest, Drop_DoesNotExist) {
-  Attachment attachment1 = Attachment::Create(some_data1);
-  AttachmentList some_attachments;
-  some_attachments.push_back(attachment1);
-  store->Write(some_attachments, write_callback);
-  ClearAndPumpLoop();
-  EXPECT_EQ(result, AttachmentStore::SUCCESS);
-
-  // Drop the attachment.
-  AttachmentIdList ids;
-  ids.push_back(attachment1.GetId());
-  store->Drop(ids, drop_callback);
-  ClearAndPumpLoop();
-  EXPECT_EQ(result, AttachmentStore::SUCCESS);
-
-  // See that it's gone.
-  store->Read(ids, read_callback);
-  ClearAndPumpLoop();
-  EXPECT_EQ(result, AttachmentStore::UNSPECIFIED_ERROR);
-  EXPECT_EQ(attachments->size(), 0U);
-  EXPECT_EQ(failed_attachment_ids->size(), 1U);
-  EXPECT_EQ((*failed_attachment_ids)[0], attachment1.GetId());
-
-  // Drop again, see that no error occurs.
-  ids.clear();
-  ids.push_back(attachment1.GetId());
-  store->Drop(ids, drop_callback);
-  ClearAndPumpLoop();
-  EXPECT_EQ(result, AttachmentStore::SUCCESS);
-}
+INSTANTIATE_TYPED_TEST_CASE_P(InMemory,
+                              AttachmentStoreTest,
+                              InMemoryAttachmentStoreFactory);
 
 }  // namespace syncer

sync/internal_api/attachments/on_disk_attachment_store.cc

+// 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/internal_api/public/attachments/on_disk_attachment_store.h"
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/location.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/sequenced_task_runner.h"
+#include "sync/protocol/attachments.pb.h"
+#include "third_party/leveldatabase/src/include/leveldb/db.h"
+#include "third_party/leveldatabase/src/include/leveldb/options.h"
+#include "third_party/leveldatabase/src/include/leveldb/slice.h"
+#include "third_party/leveldatabase/src/include/leveldb/status.h"
+
+namespace syncer {
+
+namespace {
+
+// Prefix for records containing attachment data.
+const char kDataPrefix[] = "data-";
+
+const base::FilePath::CharType kLeveldbDirectory[] =
+    FILE_PATH_LITERAL("leveldb");
+}  // namespace
+
+OnDiskAttachmentStore::OnDiskAttachmentStore(
+    const scoped_refptr<base::SequencedTaskRunner>& callback_task_runner)
+    : callback_task_runner_(callback_task_runner) {
+}
+
+OnDiskAttachmentStore::~OnDiskAttachmentStore() {
+}
+
+void OnDiskAttachmentStore::Read(const AttachmentIdList& ids,
+                                 const ReadCallback& callback) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(db_);
+  scoped_ptr<AttachmentMap> result_map(new AttachmentMap());
+  scoped_ptr<AttachmentIdList> unavailable_attachments(new AttachmentIdList());
+
+  leveldb::ReadOptions read_options;
+  // Attachment content is typically large and only read once. Don't cache it on
+  // db level.
+  read_options.fill_cache = false;
+  read_options.verify_checksums = true;
+
+  AttachmentIdList::const_iterator iter = ids.begin();
+  const AttachmentIdList::const_iterator end = ids.end();
+  for (; iter != end; ++iter) {
+    const std::string key = CreateDataKeyFromAttachmentId(*iter);
+    std::string data_str;
+    leveldb::Status status = db_->Get(read_options, key, &data_str);
+    if (!status.ok()) {
+      DVLOG(1) << "DB::Get failed: status=" << status.ToString();
+      unavailable_attachments->push_back(*iter);
+      continue;
+    }
+    scoped_refptr<base::RefCountedMemory> data =
+        base::RefCountedString::TakeString(&data_str);
+    Attachment attachment = Attachment::CreateWithId(*iter, data);
+    result_map->insert(std::make_pair(*iter, attachment));
+  }
+
+  Result result_code =
+      unavailable_attachments->empty() ? SUCCESS : UNSPECIFIED_ERROR;
+  callback_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(callback,
+                 result_code,
+                 base::Passed(&result_map),
+                 base::Passed(&unavailable_attachments)));
+}
+
+void OnDiskAttachmentStore::Write(const AttachmentList& attachments,
+                                  const WriteCallback& callback) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(db_);
+  Result result_code = SUCCESS;
+
+  leveldb::ReadOptions read_options;
+  read_options.fill_cache = false;
+  read_options.verify_checksums = true;
+
+  leveldb::WriteOptions write_options;
+  write_options.sync = true;
+
+  AttachmentList::const_iterator iter = attachments.begin();
+  const AttachmentList::const_iterator end = attachments.end();
+  for (; iter != end; ++iter) {
+    const std::string key = CreateDataKeyFromAttachmentId(iter->GetId());
+
+    std::string data_str;
+    // TODO(pavely): crbug/424304 This read is expensive. When I add metadata
+    // records this read will target metadata record instead of payload record.
+    leveldb::Status status = db_->Get(read_options, key, &data_str);
+    if (status.ok()) {
+      // Entry exists, don't overwrite.
+      continue;
+    } else if (!status.IsNotFound()) {
+      // Entry exists but failed to read.
+      DVLOG(1) << "DB::Get failed: status=" << status.ToString();
+      result_code = UNSPECIFIED_ERROR;
+      continue;
+    }
+    DCHECK(status.IsNotFound());
+
+    scoped_refptr<base::RefCountedMemory> data = iter->GetData();
+    leveldb::Slice data_slice(data->front_as<char>(), data->size());
+    status = db_->Put(write_options, key, data_slice);
+    if (!status.ok()) {
+      // Failed to write.
+      DVLOG(1) << "DB::Put failed: status=" << status.ToString();
+      result_code = UNSPECIFIED_ERROR;
+    }
+  }
+  callback_task_runner_->PostTask(FROM_HERE, base::Bind(callback, result_code));
+}
+
+void OnDiskAttachmentStore::Drop(const AttachmentIdList& ids,
+                                 const DropCallback& callback) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(db_);
+  Result result_code = SUCCESS;
+  leveldb::WriteOptions write_options;
+  write_options.sync = true;
+  AttachmentIdList::const_iterator iter = ids.begin();
+  const AttachmentIdList::const_iterator end = ids.end();
+  for (; iter != end; ++iter) {
+    const std::string key = CreateDataKeyFromAttachmentId(*iter);
+    leveldb::Status status = db_->Delete(write_options, key);
+    if (!status.ok()) {
+      // DB::Delete doesn't check if record exists, it returns ok just like
+      // AttachmentStore::Drop should.
+      DVLOG(1) << "DB::Delete failed: status=" << status.ToString();
+      result_code = UNSPECIFIED_ERROR;
+    }
+  }
+  callback_task_runner_->PostTask(FROM_HERE, base::Bind(callback, result_code));
+}
+
+AttachmentStore::Result OnDiskAttachmentStore::OpenOrCreate(
+    const base::FilePath& path) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(!db_);
+  Result result_code = UNSPECIFIED_ERROR;
+  base::FilePath leveldb_path = path.Append(kLeveldbDirectory);
+
+  leveldb::DB* db;
+  leveldb::Options options;
+  options.create_if_missing = true;
+  // TODO(pavely): crbug/424287 Consider adding info_log, block_cache and
+  // filter_policy to options.
+  leveldb::Status status =
+      leveldb::DB::Open(options, leveldb_path.AsUTF8Unsafe(), &db);
+  if (!status.ok()) {
+    DVLOG(1) << "DB::Open failed: status=" << status.ToString()
+             << ", path=" << path.AsUTF8Unsafe();
+  } else {
+    db_.reset(db);
+    result_code = SUCCESS;
+  }
+  return result_code;
+}
+
+std::string OnDiskAttachmentStore::CreateDataKeyFromAttachmentId(
+    const AttachmentId& attachment_id) {
+  std::string key = kDataPrefix + attachment_id.GetProto().unique_id();
+  return key;
+}
+
+}  // namespace syncer

sync/internal_api/attachments/on_disk_attachment_store_unittest.cc

+// 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_store.h"
+
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "base/thread_task_runner_handle.h"
+#include "base/time/time.h"
+#include "sync/internal_api/attachments/attachment_store_test_template.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace syncer {
+
+namespace {
+
+void AttachmentStoreCreated(scoped_refptr<AttachmentStore>* store_dest,
+                            AttachmentStore::Result* result_dest,
+                            const AttachmentStore::Result& result,
+                            const scoped_refptr<AttachmentStore>& store) {
+  *result_dest = result;
+  *store_dest = store;
+}
+
+}  // namespace
+
+// Instantiation of common attachment store tests.
+class OnDiskAttachmentStoreFactory {
+ public:
+  OnDiskAttachmentStoreFactory() {}
+  ~OnDiskAttachmentStoreFactory() {}
+
+  scoped_refptr<AttachmentStore> CreateAttachmentStore() {
+    EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
+    scoped_refptr<AttachmentStore> store;
+    AttachmentStore::Result result = AttachmentStore::UNSPECIFIED_ERROR;
+    AttachmentStore::CreateOnDiskStore(
+        temp_dir_.path(),
+        base::ThreadTaskRunnerHandle::Get(),
+        base::Bind(&AttachmentStoreCreated, &store, &result));
+    base::RunLoop run_loop;
+    run_loop.RunUntilIdle();
+    EXPECT_EQ(result, AttachmentStore::SUCCESS);
+    return store;
+  }
+
+ private:
+  base::ScopedTempDir temp_dir_;
+};
+
+INSTANTIATE_TYPED_TEST_CASE_P(OnDisk,
+                              AttachmentStoreTest,
+                              OnDiskAttachmentStoreFactory);
+
+// Tests specific to OnDiskAttachmentStore.
+class OnDiskAttachmentStoreSpecificTest : public testing::Test {
+ public:
+  base::ScopedTempDir temp_dir_;
+  base::MessageLoop message_loop_;
+  scoped_refptr<AttachmentStore> store_;
+
+  OnDiskAttachmentStoreSpecificTest() {}
+
+  void CopyResult(AttachmentStore::Result* destination_result,
+                  const AttachmentStore::Result& source_result) {
+    *destination_result = source_result;
+  }
+
+  void CopyResultAttachments(
+      AttachmentStore::Result* destination_result,
+      const AttachmentStore::Result& source_result,
+      scoped_ptr<AttachmentMap> source_attachments,
+      scoped_ptr<AttachmentIdList> source_failed_attachment_ids) {
+    CopyResult(destination_result, source_result);
+  }
+
+  void RunLoop() {
+    base::RunLoop run_loop;
+    run_loop.RunUntilIdle();
+  }
+};
+
+// Ensure that store can be closed and reopen while retaining stored
+// attachments.
+TEST_F(OnDiskAttachmentStoreSpecificTest, CloseAndReopen) {
+  ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+  AttachmentStore::Result result;
+
+  result = AttachmentStore::UNSPECIFIED_ERROR;
+  AttachmentStore::CreateOnDiskStore(
+      temp_dir_.path(),
+      base::ThreadTaskRunnerHandle::Get(),
+      base::Bind(&AttachmentStoreCreated, &store_, &result));
+  RunLoop();
+  EXPECT_EQ(result, AttachmentStore::SUCCESS);
+
+  result = AttachmentStore::UNSPECIFIED_ERROR;
+  std::string some_data = "data";
+  Attachment attachment =
+      Attachment::Create(base::RefCountedString::TakeString(&some_data));
+  AttachmentList attachments;
+  attachments.push_back(attachment);
+  store_->Write(attachments,
+                base::Bind(&OnDiskAttachmentStoreSpecificTest::CopyResult,
+                           base::Unretained(this),
+                           &result));
+  RunLoop();
+  EXPECT_EQ(result, AttachmentStore::SUCCESS);
+
+  // Close attachment store.
+  store_ = nullptr;
+  result = AttachmentStore::UNSPECIFIED_ERROR;
+  AttachmentStore::CreateOnDiskStore(
+      temp_dir_.path(),
+      base::ThreadTaskRunnerHandle::Get(),
+      base::Bind(&AttachmentStoreCreated, &store_, &result));
+  RunLoop();
+  EXPECT_EQ(result, AttachmentStore::SUCCESS);
+
+  result = AttachmentStore::UNSPECIFIED_ERROR;
+  AttachmentIdList attachment_ids;
+  attachment_ids.push_back(attachment.GetId());
+  store_->Read(
+      attachment_ids,
+      base::Bind(&OnDiskAttachmentStoreSpecificTest::CopyResultAttachments,
+                 base::Unretained(this),
+                 &result));
+  RunLoop();
+  EXPECT_EQ(result, AttachmentStore::SUCCESS);
+}
+
+// Ensure loading corrupt attachment store fails.
+TEST_F(OnDiskAttachmentStoreSpecificTest, FailToOpen) {
+  ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+  base::FilePath db_path =
+      temp_dir_.path().Append(FILE_PATH_LITERAL("leveldb"));
+  base::CreateDirectory(db_path);
+
+  // To simulate corrupt database write garbage to CURRENT file.
+  std::string current_file_content = "abra.cadabra";
+  base::WriteFile(db_path.Append(FILE_PATH_LITERAL("CURRENT")),
+                  current_file_content.c_str(),
+                  current_file_content.size());
+
+  AttachmentStore::Result result = AttachmentStore::SUCCESS;
+  AttachmentStore::CreateOnDiskStore(
+      temp_dir_.path(),
+      base::ThreadTaskRunnerHandle::Get(),
+      base::Bind(&AttachmentStoreCreated, &store_, &result));
+  RunLoop();
+  EXPECT_EQ(result, AttachmentStore::UNSPECIFIED_ERROR);
+  EXPECT_EQ(store_.get(), nullptr);
+}
+
+}  // namespace syncer

sync/internal_api/public/attachments/on_disk_attachment_store.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_INTERNAL_API_PUBLIC_ATTACHMENTS_ON_DISK_ATTACHMENT_STORE_H_
+#define SYNC_INTERNAL_API_PUBLIC_ATTACHMENTS_ON_DISK_ATTACHMENT_STORE_H_
+
+#include "base/files/file_path.h"
+#include "base/memory/ref_counted.h"
+#include "base/threading/non_thread_safe.h"
+#include "sync/api/attachments/attachment.h"
+#include "sync/api/attachments/attachment_id.h"
+#include "sync/api/attachments/attachment_store.h"
+#include "sync/base/sync_export.h"
+
+namespace base {
+class SequencedTaskRunner;
+}  // namespace base
+
+namespace leveldb {
+class DB;
+}  // namespace leveldb
+
+namespace syncer {
+
+// On-disk implementation of AttachmentStore. Stores attachments in leveldb
+// database in |path| directory.
+class SYNC_EXPORT OnDiskAttachmentStore : public AttachmentStoreBase,
+                                          public base::NonThreadSafe {
+ public:
+  // Constructs attachment store.
+  OnDiskAttachmentStore(
+      const scoped_refptr<base::SequencedTaskRunner>& callback_task_runner);
+  virtual ~OnDiskAttachmentStore();
+
+  // AttachmentStoreBase implementation.
+  // Load opens database, creating it if needed. In the future upgrade code will
+  // be invoked from Load as well. If loading fails it posts |callback| with
+  // UNSPECIFIED_ERROR.
+  virtual void Read(const AttachmentIdList& ids,
+                    const ReadCallback& callback) override;
+  virtual void Write(const AttachmentList& attachments,
+                     const WriteCallback& callback) override;
+  virtual void Drop(const AttachmentIdList& ids,
+                    const DropCallback& callback) override;
+
+  // Opens leveldb database at |path|. It will create directory and database if
+  // it doesn't exist.
+  Result OpenOrCreate(const base::FilePath& path);
+
+ private:
+  std::string CreateDataKeyFromAttachmentId(const AttachmentId& attachment_id);
+
+  scoped_refptr<base::SequencedTaskRunner> callback_task_runner_;
+  scoped_ptr<leveldb::DB> db_;
+};
+
+}  // namespace syncer
+
+#endif  // SYNC_INTERNAL_API_PUBLIC_ATTACHMENTS_ON_DISK_ATTACHMENT_STORE_H_

sync/sync.gyp

@@ -170,6 +170,7 @@
         'internal_api/attachments/fake_attachment_downloader.cc',
         'internal_api/attachments/fake_attachment_uploader.cc',
         'internal_api/attachments/in_memory_attachment_store.cc',
+        'internal_api/attachments/on_disk_attachment_store.cc',
         'internal_api/attachments/task_queue.cc',
         'internal_api/base_node.cc',
         'internal_api/base_transaction.cc',
@@ -209,6 +210,7 @@
         'internal_api/public/attachments/fake_attachment_downloader.h',
         'internal_api/public/attachments/fake_attachment_uploader.h',
         'internal_api/public/attachments/in_memory_attachment_store.h',
+        'internal_api/public/attachments/on_disk_attachment_store.h',
         'internal_api/public/attachments/task_queue.h',
         'internal_api/public/base/attachment_id_proto.cc',
         'internal_api/public/base/attachment_id_proto.h',

sync/sync_tests.gypi

@@ -281,10 +281,12 @@
         'internal_api/attachments/attachment_service_impl_unittest.cc',
         'internal_api/attachments/attachment_service_proxy_unittest.cc',
         'internal_api/attachments/attachment_store_handle_unittest.cc',
+        'internal_api/attachments/attachment_store_test_template.h',
         'internal_api/attachments/attachment_uploader_impl_unittest.cc',
         'internal_api/attachments/fake_attachment_downloader_unittest.cc',
         'internal_api/attachments/fake_attachment_uploader_unittest.cc',
         'internal_api/attachments/in_memory_attachment_store_unittest.cc',
+        'internal_api/attachments/on_disk_attachment_store_unittest.cc',
         'internal_api/attachments/task_queue_unittest.cc',
         'internal_api/debug_info_event_listener_unittest.cc',
         'internal_api/http_bridge_unittest.cc',