diff options
19 files changed, 493 insertions, 235 deletions
diff --git a/chrome/browser/sync/profile_sync_components_factory_impl.cc b/chrome/browser/sync/profile_sync_components_factory_impl.cc index 345845c..bc1829c 100644 --- a/chrome/browser/sync/profile_sync_components_factory_impl.cc +++ b/chrome/browser/sync/profile_sync_components_factory_impl.cc @@ -65,7 +65,6 @@ #include "content/public/browser/browser_thread.h" #include "google_apis/gaia/oauth2_token_service_request.h" #include "net/url_request/url_request_context_getter.h" -#include "sync/api/attachments/fake_attachment_store.h" #include "sync/api/syncable_service.h" #include "sync/internal_api/public/attachments/attachment_downloader.h" #include "sync/internal_api/public/attachments/attachment_service.h" diff --git a/chrome/browser/sync/profile_sync_components_factory_mock.cc b/chrome/browser/sync/profile_sync_components_factory_mock.cc index f113d33..ac0a70c 100644 --- a/chrome/browser/sync/profile_sync_components_factory_mock.cc +++ b/chrome/browser/sync/profile_sync_components_factory_mock.cc @@ -8,7 +8,7 @@ #include "components/sync_driver/local_device_info_provider_mock.h" #include "components/sync_driver/model_associator.h" #include "content/public/browser/browser_thread.h" -#include "sync/api/attachments/fake_attachment_store.h" +#include "sync/api/attachments/attachment_store.h" #include "sync/internal_api/public/attachments/attachment_service_impl.h" using sync_driver::AssociatorInterface; diff --git a/components/sync_driver/device_info_sync_service_unittest.cc b/components/sync_driver/device_info_sync_service_unittest.cc index c1200d76..f0a6906 100644 --- a/components/sync_driver/device_info_sync_service_unittest.cc +++ b/components/sync_driver/device_info_sync_service_unittest.cc @@ -153,10 +153,10 @@ class DeviceInfoSyncServiceTest : public testing::Test, protected: int num_device_info_changed_callbacks_; + base::MessageLoopForUI message_loop_; scoped_ptr<LocalDeviceInfoProviderMock> local_device_; scoped_ptr<DeviceInfoSyncService> sync_service_; scoped_ptr<TestChangeProcessor> sync_processor_; - base::MessageLoopForUI message_loop_; }; // Sync with empty initial data. diff --git a/components/sync_driver/generic_change_processor_unittest.cc b/components/sync_driver/generic_change_processor_unittest.cc index f3c580e..e84cf5e 100644 --- a/components/sync_driver/generic_change_processor_unittest.cc +++ b/components/sync_driver/generic_change_processor_unittest.cc @@ -12,7 +12,7 @@ #include "components/sync_driver/data_type_error_handler_mock.h" #include "components/sync_driver/sync_api_component_factory.h" #include "sync/api/attachments/attachment_id.h" -#include "sync/api/attachments/fake_attachment_store.h" +#include "sync/api/attachments/attachment_store.h" #include "sync/api/fake_syncable_service.h" #include "sync/api/sync_change.h" #include "sync/api/sync_merge_result.h" @@ -147,8 +147,8 @@ class SyncGenericChangeProcessorTest : public testing::Test { } void ConstructGenericChangeProcessor(syncer::ModelType type) { - scoped_refptr<syncer::AttachmentStore> attachment_store( - new syncer::FakeAttachmentStore(base::MessageLoopProxy::current())); + scoped_refptr<syncer::AttachmentStore> attachment_store = + syncer::AttachmentStore::CreateInMemoryStore(); scoped_ptr<MockAttachmentService> mock_attachment_service( new MockAttachmentService(attachment_store)); // GenericChangeProcessor takes ownership of the AttachmentService, but we diff --git a/sync/BUILD.gn b/sync/BUILD.gn index d5cae95..d32575b 100644 --- a/sync/BUILD.gn +++ b/sync/BUILD.gn @@ -20,8 +20,6 @@ source_set("sync_core") { "api/attachments/attachment_id.h", "api/attachments/attachment_store.cc", "api/attachments/attachment_store.h", - "api/attachments/fake_attachment_store.cc", - "api/attachments/fake_attachment_store.h", "api/string_ordinal.h", "api/syncable_service.cc", "api/syncable_service.h", @@ -122,16 +120,12 @@ source_set("sync_core") { "internal_api/attachments/attachment_service_impl.cc", "internal_api/attachments/attachment_service_proxy.cc", "internal_api/attachments/attachment_service_proxy_for_test.cc", + "internal_api/attachments/attachment_store_handle.cc", "internal_api/attachments/attachment_uploader.cc", "internal_api/attachments/attachment_uploader_impl.cc", "internal_api/attachments/fake_attachment_downloader.cc", "internal_api/attachments/fake_attachment_uploader.cc", - "internal_api/attachments/public/attachment_downloader.h", - "internal_api/attachments/public/attachment_service.h", - "internal_api/attachments/public/attachment_service_impl.h", - "internal_api/attachments/public/attachment_service_proxy_for_test.h", - "internal_api/attachments/public/attachment_service_proxy.h", - "internal_api/attachments/public/attachment_uploader.h", + "internal_api/attachments/in_memory_attachment_store.cc", "internal_api/base_node.cc", "internal_api/base_transaction.cc", "internal_api/change_record.cc", @@ -158,10 +152,18 @@ source_set("sync_core") { "internal_api/js_sync_manager_observer.h", "internal_api/protocol_event_buffer.cc", "internal_api/protocol_event_buffer.h", + "internal_api/public/attachments/attachment_downloader.h", "internal_api/public/attachments/attachment_downloader_impl.h", + "internal_api/public/attachments/attachment_service.h", + "internal_api/public/attachments/attachment_service_impl.h", + "internal_api/public/attachments/attachment_service_proxy.h", + "internal_api/public/attachments/attachment_service_proxy_for_test.h", + "internal_api/public/attachments/attachment_store_handle.h", + "internal_api/public/attachments/attachment_uploader.h", "internal_api/public/attachments/attachment_uploader_impl.h", "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/base/attachment_id_proto.cc", "internal_api/public/base/attachment_id_proto.h", "internal_api/public/base/cancelation_observer.cc", @@ -561,6 +563,7 @@ test("sync_unit_tests") { "internal_api/attachments/attachment_downloader_impl_unittest.cc", "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_uploader_impl_unittest.cc", "internal_api/attachments/fake_attachment_downloader_unittest.cc", "internal_api/attachments/fake_attachment_uploader_unittest.cc", diff --git a/sync/api/attachments/attachment_store.cc b/sync/api/attachments/attachment_store.cc index e22b040..ad4489e 100644 --- a/sync/api/attachments/attachment_store.cc +++ b/sync/api/attachments/attachment_store.cc @@ -4,9 +4,24 @@ #include "sync/api/attachments/attachment_store.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" + namespace syncer { +AttachmentStoreBase::AttachmentStoreBase() {} +AttachmentStoreBase::~AttachmentStoreBase() {} + AttachmentStore::AttachmentStore() {} AttachmentStore::~AttachmentStore() {} +scoped_refptr<AttachmentStore> AttachmentStore::CreateInMemoryStore() { + // Both frontend and backend of attachment store will live on current thread. + scoped_ptr<AttachmentStoreBase> backend( + new InMemoryAttachmentStore(base::ThreadTaskRunnerHandle::Get())); + return scoped_refptr<AttachmentStore>(new AttachmentStoreHandle( + backend.Pass(), base::ThreadTaskRunnerHandle::Get())); +} + } // namespace syncer diff --git a/sync/api/attachments/attachment_store.h b/sync/api/attachments/attachment_store.h index d487f8c..45ccaf9 100644 --- a/sync/api/attachments/attachment_store.h +++ b/sync/api/attachments/attachment_store.h @@ -25,10 +25,8 @@ class AttachmentId; // // Destroying this object does not necessarily cancel outstanding async // operations. If you need cancel like semantics, use WeakPtr in the callbacks. -class SYNC_EXPORT AttachmentStore : public base::RefCounted<AttachmentStore> { +class SYNC_EXPORT AttachmentStoreBase { public: - AttachmentStore(); - // TODO(maniscalco): Consider udpating Read and Write methods to support // resumable transfers (bug 353292). @@ -43,6 +41,9 @@ class SYNC_EXPORT AttachmentStore : public base::RefCounted<AttachmentStore> { typedef base::Callback<void(const Result&)> WriteCallback; typedef base::Callback<void(const Result&)> DropCallback; + AttachmentStoreBase(); + virtual ~AttachmentStoreBase(); + // Asynchronously reads the attachments identified by |ids|. // // |callback| will be invoked when finished. AttachmentStore will attempt to @@ -81,9 +82,22 @@ class SYNC_EXPORT AttachmentStore : public base::RefCounted<AttachmentStore> { // successfully. virtual void Drop(const AttachmentIdList& ids, const DropCallback& callback) = 0; +}; + +// AttachmentStore is an interface exposed to data type and AttachmentService +// code. Also contains factory methods for default implementations. +class SYNC_EXPORT AttachmentStore + : public AttachmentStoreBase, + public base::RefCountedThreadSafe<AttachmentStore> { + public: + 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(); protected: - friend class base::RefCounted<AttachmentStore>; + friend class base::RefCountedThreadSafe<AttachmentStore>; virtual ~AttachmentStore(); }; diff --git a/sync/api/attachments/fake_attachment_store.cc b/sync/api/attachments/fake_attachment_store.cc deleted file mode 100644 index d20a9b5..0000000 --- a/sync/api/attachments/fake_attachment_store.cc +++ /dev/null @@ -1,128 +0,0 @@ -// 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_store.h" - -#include "base/bind.h" -#include "base/location.h" -#include "base/memory/ref_counted_memory.h" -#include "base/sequenced_task_runner.h" -#include "base/single_thread_task_runner.h" -#include "base/thread_task_runner_handle.h" -#include "sync/api/attachments/attachment.h" - -namespace syncer { - -// Backend is where all the work happens. -class FakeAttachmentStore::Backend - : public base::RefCountedThreadSafe<FakeAttachmentStore::Backend> { - public: - // Construct a Backend that posts its results to |frontend_task_runner|. - Backend( - const scoped_refptr<base::SingleThreadTaskRunner>& frontend_task_runner); - - void Read(const AttachmentIdList& ids, const ReadCallback& callback); - void Write(const AttachmentList& attachments, const WriteCallback& callback); - void Drop(const AttachmentIdList& ids, const DropCallback& callback); - - private: - friend class base::RefCountedThreadSafe<Backend>; - - ~Backend(); - - scoped_refptr<base::SingleThreadTaskRunner> frontend_task_runner_; - AttachmentMap attachments_; -}; - -FakeAttachmentStore::Backend::Backend( - const scoped_refptr<base::SingleThreadTaskRunner>& frontend_task_runner) - : frontend_task_runner_(frontend_task_runner) {} - -FakeAttachmentStore::Backend::~Backend() {} - -void FakeAttachmentStore::Backend::Read(const AttachmentIdList& ids, - const ReadCallback& callback) { - Result result_code = SUCCESS; - AttachmentIdList::const_iterator id_iter = ids.begin(); - AttachmentIdList::const_iterator id_end = ids.end(); - scoped_ptr<AttachmentMap> result_map(new AttachmentMap); - scoped_ptr<AttachmentIdList> unavailable_attachments(new AttachmentIdList); - for (; id_iter != id_end; ++id_iter) { - const AttachmentId& id = *id_iter; - syncer::AttachmentMap::iterator attachment_iter = - attachments_.find(*id_iter); - if (attachment_iter != attachments_.end()) { - const Attachment& attachment = attachment_iter->second; - result_map->insert(std::make_pair(id, attachment)); - } else { - unavailable_attachments->push_back(id); - } - } - if (!unavailable_attachments->empty()) { - result_code = UNSPECIFIED_ERROR; - } - frontend_task_runner_->PostTask( - FROM_HERE, - base::Bind(callback, - result_code, - base::Passed(&result_map), - base::Passed(&unavailable_attachments))); -} - -void FakeAttachmentStore::Backend::Write(const AttachmentList& attachments, - const WriteCallback& callback) { - AttachmentList::const_iterator iter = attachments.begin(); - AttachmentList::const_iterator end = attachments.end(); - for (; iter != end; ++iter) { - attachments_.insert(std::make_pair(iter->GetId(), *iter)); - } - frontend_task_runner_->PostTask(FROM_HERE, base::Bind(callback, SUCCESS)); -} - -void FakeAttachmentStore::Backend::Drop(const AttachmentIdList& ids, - const DropCallback& callback) { - Result result = SUCCESS; - AttachmentIdList::const_iterator ids_iter = ids.begin(); - AttachmentIdList::const_iterator ids_end = ids.end(); - for (; ids_iter != ids_end; ++ids_iter) { - AttachmentMap::iterator attachments_iter = attachments_.find(*ids_iter); - if (attachments_iter != attachments_.end()) { - attachments_.erase(attachments_iter); - } - } - frontend_task_runner_->PostTask(FROM_HERE, base::Bind(callback, result)); -} - -FakeAttachmentStore::FakeAttachmentStore( - const scoped_refptr<base::SequencedTaskRunner>& backend_task_runner) - : backend_(new Backend(base::ThreadTaskRunnerHandle::Get())), - backend_task_runner_(backend_task_runner) {} - -FakeAttachmentStore::~FakeAttachmentStore() {} - -void FakeAttachmentStore::Read(const AttachmentIdList& ids, - const ReadCallback& callback) { - backend_task_runner_->PostTask( - FROM_HERE, - base::Bind(&FakeAttachmentStore::Backend::Read, backend_, ids, callback)); -} - -void FakeAttachmentStore::Write(const AttachmentList& attachments, - const WriteCallback& callback) { - backend_task_runner_->PostTask( - FROM_HERE, - base::Bind(&FakeAttachmentStore::Backend::Write, - backend_, - attachments, - callback)); -} - -void FakeAttachmentStore::Drop(const AttachmentIdList& ids, - const DropCallback& callback) { - backend_task_runner_->PostTask( - FROM_HERE, - base::Bind(&FakeAttachmentStore::Backend::Drop, backend_, ids, callback)); -} - -} // namespace syncer diff --git a/sync/api/attachments/fake_attachment_store.h b/sync/api/attachments/fake_attachment_store.h deleted file mode 100644 index 9ac7006..0000000 --- a/sync/api/attachments/fake_attachment_store.h +++ /dev/null @@ -1,60 +0,0 @@ -// 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_STORE_H_ -#define SYNC_API_ATTACHMENTS_FAKE_ATTACHMENT_STORE_H_ - -#include <map> - -#include "base/memory/ref_counted.h" -#include "base/memory/scoped_ptr.h" -#include "base/stl_util.h" -#include "sync/api/attachments/attachment_store.h" -#include "sync/base/sync_export.h" - -namespace base { -class SequencedTaskRunner; -class SingleThreadTaskRunner; -} // namespace base - -namespace sync_pb { -class AttachmentId; -} // namespace sync_pb - -namespace syncer { - -class Attachment; - -// A in-memory only implementation of AttachmentStore used for testing. -// -// Requires that the current thread has a MessageLoop. -class SYNC_EXPORT FakeAttachmentStore : public AttachmentStore { - public: - // Construct a FakeAttachmentStore whose "IO" will be performed in - // |backend_task_runner|. - explicit FakeAttachmentStore( - const scoped_refptr<base::SequencedTaskRunner>& backend_task_runner); - - // AttachmentStore implementation. - virtual void Read(const AttachmentIdList& id, - const ReadCallback& callback) OVERRIDE; - virtual void Write(const AttachmentList& attachments, - const WriteCallback& callback) OVERRIDE; - virtual void Drop(const AttachmentIdList& id, - const DropCallback& callback) OVERRIDE; - - private: - class Backend; - - virtual ~FakeAttachmentStore(); - - scoped_refptr<Backend> backend_; - scoped_refptr<base::SequencedTaskRunner> backend_task_runner_; - - DISALLOW_COPY_AND_ASSIGN(FakeAttachmentStore); -}; - -} // namespace syncer - -#endif // SYNC_API_ATTACHMENTS_FAKE_ATTACHMENT_STORE_H_ diff --git a/sync/internal_api/attachments/attachment_service_impl.cc b/sync/internal_api/attachments/attachment_service_impl.cc index 227fd8b..dedb10c 100644 --- a/sync/internal_api/attachments/attachment_service_impl.cc +++ b/sync/internal_api/attachments/attachment_service_impl.cc @@ -11,7 +11,6 @@ #include "base/thread_task_runner_handle.h" #include "base/time/time.h" #include "sync/api/attachments/attachment.h" -#include "sync/api/attachments/fake_attachment_store.h" #include "sync/internal_api/public/attachments/fake_attachment_downloader.h" #include "sync/internal_api/public/attachments/fake_attachment_uploader.h" @@ -145,8 +144,8 @@ AttachmentServiceImpl::~AttachmentServiceImpl() { // Static. scoped_ptr<syncer::AttachmentService> AttachmentServiceImpl::CreateForTest() { - scoped_refptr<syncer::AttachmentStore> attachment_store( - new syncer::FakeAttachmentStore(base::ThreadTaskRunnerHandle::Get())); + scoped_refptr<syncer::AttachmentStore> attachment_store = + AttachmentStore::CreateInMemoryStore(); scoped_ptr<AttachmentUploader> attachment_uploader( new FakeAttachmentUploader); scoped_ptr<AttachmentDownloader> attachment_downloader( diff --git a/sync/internal_api/attachments/attachment_service_impl_unittest.cc b/sync/internal_api/attachments/attachment_service_impl_unittest.cc index 85bbb6a..e802e2b 100644 --- a/sync/internal_api/attachments/attachment_service_impl_unittest.cc +++ b/sync/internal_api/attachments/attachment_service_impl_unittest.cc @@ -16,6 +16,8 @@ namespace syncer { +namespace { + class MockAttachmentStore : public AttachmentStore, public base::SupportsWeakPtr<MockAttachmentStore> { public: @@ -150,6 +152,8 @@ class MockAttachmentUploader DISALLOW_COPY_AND_ASSIGN(MockAttachmentUploader); }; +} // namespace + class AttachmentServiceImplTest : public testing::Test, public AttachmentService::Delegate { protected: diff --git a/sync/internal_api/attachments/attachment_store_handle.cc b/sync/internal_api/attachments/attachment_store_handle.cc new file mode 100644 index 0000000..bfb637a --- /dev/null +++ b/sync/internal_api/attachments/attachment_store_handle.cc @@ -0,0 +1,70 @@ +// 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/attachment_store_handle.h" + +#include "base/bind.h" +#include "base/location.h" +#include "base/sequenced_task_runner.h" +#include "sync/api/attachments/attachment.h" + +namespace syncer { + +namespace { + +// NoOp is needed to bind base::Passed(backend) in AttachmentStoreHandle dtor. +// It doesn't need to do anything. +void NoOp(scoped_ptr<AttachmentStoreBase> backend) { +} + +} // namespace + +AttachmentStoreHandle::AttachmentStoreHandle( + scoped_ptr<AttachmentStoreBase> backend, + const scoped_refptr<base::SequencedTaskRunner>& backend_task_runner) + : backend_(backend.Pass()), backend_task_runner_(backend_task_runner) { + DCHECK(backend_); + DCHECK(backend_task_runner_.get()); +} + +AttachmentStoreHandle::~AttachmentStoreHandle() { + DCHECK(backend_); + // To delete backend post task that doesn't do anything, but binds backend + // through base::Passed. This way backend will be deleted regardless whether + // task runs or not. + backend_task_runner_->PostTask( + FROM_HERE, base::Bind(&NoOp, base::Passed(&backend_))); +} + +void AttachmentStoreHandle::Read(const AttachmentIdList& ids, + const ReadCallback& callback) { + DCHECK(CalledOnValidThread()); + backend_task_runner_->PostTask(FROM_HERE, + base::Bind(&AttachmentStoreBase::Read, + base::Unretained(backend_.get()), + ids, + callback)); +} + +void AttachmentStoreHandle::Write(const AttachmentList& attachments, + const WriteCallback& callback) { + DCHECK(CalledOnValidThread()); + backend_task_runner_->PostTask(FROM_HERE, + base::Bind(&AttachmentStoreBase::Write, + base::Unretained(backend_.get()), + attachments, + callback)); +} + +void AttachmentStoreHandle::Drop(const AttachmentIdList& ids, + const DropCallback& callback) { + DCHECK(CalledOnValidThread()); + backend_task_runner_->PostTask(FROM_HERE, + base::Bind(&AttachmentStoreBase::Drop, + base::Unretained(backend_.get()), + ids, + callback)); +} + +} // namespace syncer diff --git a/sync/internal_api/attachments/attachment_store_handle_unittest.cc b/sync/internal_api/attachments/attachment_store_handle_unittest.cc new file mode 100644 index 0000000..e99f6e1 --- /dev/null +++ b/sync/internal_api/attachments/attachment_store_handle_unittest.cc @@ -0,0 +1,148 @@ +// 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/attachment_store_handle.h" + +#include "base/bind.h" +#include "base/callback.h" +#include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" +#include "base/message_loop/message_loop.h" +#include "base/run_loop.h" +#include "base/thread_task_runner_handle.h" +#include "sync/api/attachments/attachment.h" +#include "sync/api/attachments/attachment_id.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace syncer { + +namespace { + +class MockAttachmentStore : public AttachmentStoreBase { + public: + MockAttachmentStore(const base::Closure& read_called, + const base::Closure& write_called, + const base::Closure& drop_called, + const base::Closure& dtor_called) + : read_called_(read_called), + write_called_(write_called), + drop_called_(drop_called), + dtor_called_(dtor_called) {} + + virtual ~MockAttachmentStore() { + dtor_called_.Run(); + } + + virtual void Read(const AttachmentIdList& ids, + const ReadCallback& callback) OVERRIDE { + read_called_.Run(); + } + + virtual void Write(const AttachmentList& attachments, + const WriteCallback& callback) OVERRIDE { + write_called_.Run(); + } + + virtual void Drop(const AttachmentIdList& ids, + const DropCallback& callback) OVERRIDE { + drop_called_.Run(); + } + + base::Closure read_called_; + base::Closure write_called_; + base::Closure drop_called_; + base::Closure dtor_called_; +}; + +} // namespace + +class AttachmentStoreHandleTest : public testing::Test { + protected: + AttachmentStoreHandleTest() + : read_call_count_(0), + write_call_count_(0), + drop_call_count_(0), + dtor_call_count_(0) {} + + virtual void SetUp() { + scoped_ptr<AttachmentStoreBase> backend(new MockAttachmentStore( + base::Bind(&AttachmentStoreHandleTest::ReadCalled, + base::Unretained(this)), + base::Bind(&AttachmentStoreHandleTest::WriteCalled, + base::Unretained(this)), + base::Bind(&AttachmentStoreHandleTest::DropCalled, + base::Unretained(this)), + base::Bind(&AttachmentStoreHandleTest::DtorCalled, + base::Unretained(this)))); + attachment_store_handle_ = new AttachmentStoreHandle( + backend.Pass(), base::ThreadTaskRunnerHandle::Get()); + } + + static void ReadDone(const AttachmentStore::Result& result, + scoped_ptr<AttachmentMap> attachments, + scoped_ptr<AttachmentIdList> unavailable_attachments) { + NOTREACHED(); + } + + static void WriteDone(const AttachmentStore::Result& result) { + NOTREACHED(); + } + + static void DropDone(const AttachmentStore::Result& result) { + NOTREACHED(); + } + + void ReadCalled() { ++read_call_count_; } + + void WriteCalled() { ++write_call_count_; } + + void DropCalled() { ++drop_call_count_; } + + void DtorCalled() { ++dtor_call_count_; } + + void RunMessageLoop() { + base::RunLoop run_loop; + run_loop.RunUntilIdle(); + } + + base::MessageLoop message_loop_; + scoped_refptr<AttachmentStoreHandle> attachment_store_handle_; + int read_call_count_; + int write_call_count_; + int drop_call_count_; + int dtor_call_count_; +}; + +// Test that method calls are forwarded to backend loop +TEST_F(AttachmentStoreHandleTest, MethodsCalled) { + AttachmentIdList ids; + AttachmentList attachments; + + attachment_store_handle_->Read( + ids, base::Bind(&AttachmentStoreHandleTest::ReadDone)); + EXPECT_EQ(read_call_count_, 0); + RunMessageLoop(); + EXPECT_EQ(read_call_count_, 1); + + attachment_store_handle_->Write( + attachments, base::Bind(&AttachmentStoreHandleTest::WriteDone)); + EXPECT_EQ(write_call_count_, 0); + RunMessageLoop(); + EXPECT_EQ(write_call_count_, 1); + + attachment_store_handle_->Drop( + ids, base::Bind(&AttachmentStoreHandleTest::DropDone)); + EXPECT_EQ(drop_call_count_, 0); + RunMessageLoop(); + EXPECT_EQ(drop_call_count_, 1); + + // Releasing referehce to AttachmentStoreHandle should result in + // MockAttachmentStore being deleted on backend loop. + attachment_store_handle_ = NULL; + EXPECT_EQ(dtor_call_count_, 0); + RunMessageLoop(); + EXPECT_EQ(dtor_call_count_, 1); +} + +} // namespace syncer diff --git a/sync/internal_api/attachments/in_memory_attachment_store.cc b/sync/internal_api/attachments/in_memory_attachment_store.cc new file mode 100644 index 0000000..c1e366d --- /dev/null +++ b/sync/internal_api/attachments/in_memory_attachment_store.cc @@ -0,0 +1,80 @@ +// 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/in_memory_attachment_store.h" + +#include "base/bind.h" +#include "base/callback.h" +#include "base/location.h" +#include "base/memory/scoped_ptr.h" + +namespace syncer { + +InMemoryAttachmentStore::InMemoryAttachmentStore( + const scoped_refptr<base::SingleThreadTaskRunner>& callback_task_runner) + : callback_task_runner_(callback_task_runner) { + // Object is created on one thread but used on another. + DetachFromThread(); +} + +InMemoryAttachmentStore::~InMemoryAttachmentStore() { +} + +void InMemoryAttachmentStore::Read(const AttachmentIdList& ids, + const ReadCallback& callback) { + DCHECK(CalledOnValidThread()); + Result result_code = SUCCESS; + AttachmentIdList::const_iterator id_iter = ids.begin(); + AttachmentIdList::const_iterator id_end = ids.end(); + scoped_ptr<AttachmentMap> result_map(new AttachmentMap); + scoped_ptr<AttachmentIdList> unavailable_attachments(new AttachmentIdList); + for (; id_iter != id_end; ++id_iter) { + const AttachmentId& id = *id_iter; + syncer::AttachmentMap::iterator attachment_iter = + attachments_.find(*id_iter); + if (attachment_iter != attachments_.end()) { + const Attachment& attachment = attachment_iter->second; + result_map->insert(std::make_pair(id, attachment)); + } else { + unavailable_attachments->push_back(id); + } + } + if (!unavailable_attachments->empty()) { + result_code = UNSPECIFIED_ERROR; + } + callback_task_runner_->PostTask( + FROM_HERE, + base::Bind(callback, + result_code, + base::Passed(&result_map), + base::Passed(&unavailable_attachments))); +} + +void InMemoryAttachmentStore::Write(const AttachmentList& attachments, + const WriteCallback& callback) { + DCHECK(CalledOnValidThread()); + AttachmentList::const_iterator iter = attachments.begin(); + AttachmentList::const_iterator end = attachments.end(); + for (; iter != end; ++iter) { + attachments_.insert(std::make_pair(iter->GetId(), *iter)); + } + callback_task_runner_->PostTask(FROM_HERE, base::Bind(callback, SUCCESS)); +} + +void InMemoryAttachmentStore::Drop(const AttachmentIdList& ids, + const DropCallback& callback) { + DCHECK(CalledOnValidThread()); + Result result = SUCCESS; + AttachmentIdList::const_iterator ids_iter = ids.begin(); + AttachmentIdList::const_iterator ids_end = ids.end(); + for (; ids_iter != ids_end; ++ids_iter) { + AttachmentMap::iterator attachments_iter = attachments_.find(*ids_iter); + if (attachments_iter != attachments_.end()) { + attachments_.erase(attachments_iter); + } + } + callback_task_runner_->PostTask(FROM_HERE, base::Bind(callback, result)); +} + +} // namespace syncer diff --git a/sync/api/attachments/fake_attachment_store_unittest.cc b/sync/internal_api/attachments/in_memory_attachment_store_unittest.cc index 71b9e50..9d3d1ad 100644 --- a/sync/api/attachments/fake_attachment_store_unittest.cc +++ b/sync/internal_api/attachments/in_memory_attachment_store_unittest.cc @@ -2,7 +2,7 @@ // 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_store.h" +#include "sync/api/attachments/attachment_store.h" #include "base/bind.h" #include "base/memory/ref_counted_memory.h" @@ -18,10 +18,10 @@ namespace syncer { const char kTestData1[] = "test data 1"; const char kTestData2[] = "test data 2"; -class FakeAttachmentStoreTest : public testing::Test { +class InMemoryAttachmentStoreTest : public testing::Test { protected: base::MessageLoop message_loop; - scoped_refptr<FakeAttachmentStore> store; + scoped_refptr<AttachmentStore> store; AttachmentStore::Result result; scoped_ptr<AttachmentMap> attachments; scoped_ptr<AttachmentIdList> failed_attachment_ids; @@ -33,18 +33,20 @@ class FakeAttachmentStoreTest : public testing::Test { scoped_refptr<base::RefCountedString> some_data1; scoped_refptr<base::RefCountedString> some_data2; - FakeAttachmentStoreTest() - : store(new FakeAttachmentStore(base::ThreadTaskRunnerHandle::Get())) {} + InMemoryAttachmentStoreTest() + : store(AttachmentStore::CreateInMemoryStore()) {} virtual void SetUp() { Clear(); - read_callback = base::Bind(&FakeAttachmentStoreTest::CopyResultAttachments, - base::Unretained(this), - &result, - &attachments, - &failed_attachment_ids); - write_callback = base::Bind( - &FakeAttachmentStoreTest::CopyResult, base::Unretained(this), &result); + 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; @@ -86,7 +88,7 @@ class FakeAttachmentStoreTest : public testing::Test { // Verify that we do not overwrite existing attachments and that we do not treat // it as an error. -TEST_F(FakeAttachmentStoreTest, Write_NoOverwriteNoError) { +TEST_F(InMemoryAttachmentStoreTest, Write_NoOverwriteNoError) { // Create two attachments with the same id but different data. Attachment attachment1 = Attachment::Create(some_data1); Attachment attachment2 = @@ -120,7 +122,7 @@ TEST_F(FakeAttachmentStoreTest, Write_NoOverwriteNoError) { } // Verify that we can write some attachments and read them back. -TEST_F(FakeAttachmentStoreTest, Write_RoundTrip) { +TEST_F(InMemoryAttachmentStoreTest, Write_RoundTrip) { Attachment attachment1 = Attachment::Create(some_data1); Attachment attachment2 = Attachment::Create(some_data2); AttachmentList some_attachments; @@ -150,7 +152,7 @@ TEST_F(FakeAttachmentStoreTest, Write_RoundTrip) { } // Try to read two attachments when only one exists. -TEST_F(FakeAttachmentStoreTest, Read_OneNotFound) { +TEST_F(InMemoryAttachmentStoreTest, Read_OneNotFound) { Attachment attachment1 = Attachment::Create(some_data1); Attachment attachment2 = Attachment::Create(some_data2); @@ -176,7 +178,7 @@ TEST_F(FakeAttachmentStoreTest, Read_OneNotFound) { // Try to drop two attachments when only one exists. Verify that no error occurs // and that the existing attachment was dropped. -TEST_F(FakeAttachmentStoreTest, Drop_DropTwoButOnlyOneExists) { +TEST_F(InMemoryAttachmentStoreTest, Drop_DropTwoButOnlyOneExists) { // First, create two attachments. Attachment attachment1 = Attachment::Create(some_data1); Attachment attachment2 = Attachment::Create(some_data2); @@ -223,7 +225,7 @@ TEST_F(FakeAttachmentStoreTest, Drop_DropTwoButOnlyOneExists) { // Verify that attempting to drop an attachment that does not exist is not an // error. -TEST_F(FakeAttachmentStoreTest, Drop_DoesNotExist) { +TEST_F(InMemoryAttachmentStoreTest, Drop_DoesNotExist) { Attachment attachment1 = Attachment::Create(some_data1); AttachmentList some_attachments; some_attachments.push_back(attachment1); diff --git a/sync/internal_api/public/attachments/attachment_store_handle.h b/sync/internal_api/public/attachments/attachment_store_handle.h new file mode 100644 index 0000000..28b5f2a --- /dev/null +++ b/sync/internal_api/public/attachments/attachment_store_handle.h @@ -0,0 +1,66 @@ +// 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_ATTACHMENT_STORE_HANDLE_H_ +#define SYNC_INTERNAL_API_PUBLIC_ATTACHMENTS_ATTACHMENT_STORE_HANDLE_H_ + +#include <map> + +#include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" +#include "base/stl_util.h" +#include "base/threading/non_thread_safe.h" +#include "sync/api/attachments/attachment_store.h" +#include "sync/base/sync_export.h" + +namespace base { +class SequencedTaskRunner; +} // namespace base + +namespace sync_pb { +class AttachmentId; +} // namespace sync_pb + +namespace syncer { + +class Attachment; + +// AttachmentStoreHandle is helper to post AttachmentStore calls to backend on +// different thread. Backend is expected to know on which thread to post +// callbacks with results. +// AttachmentStoreHandle takes ownership of backend. Backend is deleted on +// backend thread. +// AttachmentStoreHandle is not thread safe, it should only be accessed from +// model thread. +class SYNC_EXPORT AttachmentStoreHandle : public AttachmentStore, + public base::NonThreadSafe { + public: + AttachmentStoreHandle( + scoped_ptr<AttachmentStoreBase> backend, + const scoped_refptr<base::SequencedTaskRunner>& backend_task_runner); + + // AttachmentStore implementation. + virtual void Read(const AttachmentIdList& id, + const ReadCallback& callback) OVERRIDE; + virtual void Write(const AttachmentList& attachments, + const WriteCallback& callback) OVERRIDE; + virtual void Drop(const AttachmentIdList& id, + const DropCallback& callback) OVERRIDE; + + private: + virtual ~AttachmentStoreHandle(); + + // AttachmentStoreHandle controls backend's lifetime. It is safe for + // AttachmentStoreHandle to bind backend through base::Unretained for posts. + // Backend is deleted on backend_task_runner, after that backend_ pointer is + // invalid. + scoped_ptr<AttachmentStoreBase> backend_; + scoped_refptr<base::SequencedTaskRunner> backend_task_runner_; + + DISALLOW_COPY_AND_ASSIGN(AttachmentStoreHandle); +}; + +} // namespace syncer + +#endif // SYNC_INTERNAL_API_PUBLIC_ATTACHMENTS_ATTACHMENT_STORE_HANDLE_H_ diff --git a/sync/internal_api/public/attachments/in_memory_attachment_store.h b/sync/internal_api/public/attachments/in_memory_attachment_store.h new file mode 100644 index 0000000..06a982b --- /dev/null +++ b/sync/internal_api/public/attachments/in_memory_attachment_store.h @@ -0,0 +1,43 @@ +// 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_IN_MEMORY_ATTACHMENT_STORE_H_ +#define SYNC_INTERNAL_API_PUBLIC_ATTACHMENTS_IN_MEMORY_ATTACHMENT_STORE_H_ + +#include "base/memory/ref_counted.h" +#include "base/single_thread_task_runner.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 syncer { + +// An in-memory implementation of AttachmentStore used for testing. +// InMemoryAttachmentStore is not threadsafe, it lives on backend thread and +// posts callbacks with results on |callback_task_runner|. +class SYNC_EXPORT InMemoryAttachmentStore : public AttachmentStoreBase, + public base::NonThreadSafe { + public: + InMemoryAttachmentStore( + const scoped_refptr<base::SingleThreadTaskRunner>& callback_task_runner); + virtual ~InMemoryAttachmentStore(); + + // AttachmentStoreBase implementation. + 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; + + private: + scoped_refptr<base::SingleThreadTaskRunner> callback_task_runner_; + AttachmentMap attachments_; +}; + +} // namespace syncer + +#endif // SYNC_INTERNAL_API_PUBLIC_ATTACHMENTS_IN_MEMORY_ATTACHMENT_STORE_H_ diff --git a/sync/sync.gyp b/sync/sync.gyp index 6372fb70..c5532be 100644 --- a/sync/sync.gyp +++ b/sync/sync.gyp @@ -63,8 +63,6 @@ 'api/attachments/attachment_id.h', 'api/attachments/attachment_store.cc', 'api/attachments/attachment_store.h', - 'api/attachments/fake_attachment_store.cc', - 'api/attachments/fake_attachment_store.h', 'api/string_ordinal.h', 'api/sync_change.cc', 'api/sync_change.h', @@ -165,10 +163,12 @@ 'internal_api/attachments/attachment_service_impl.cc', 'internal_api/attachments/attachment_service_proxy.cc', 'internal_api/attachments/attachment_service_proxy_for_test.cc', + 'internal_api/attachments/attachment_store_handle.cc', 'internal_api/attachments/attachment_uploader.cc', 'internal_api/attachments/attachment_uploader_impl.cc', '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/task_queue.cc', 'internal_api/base_node.cc', 'internal_api/base_transaction.cc', @@ -196,16 +196,18 @@ 'internal_api/js_sync_manager_observer.h', 'internal_api/protocol_event_buffer.cc', 'internal_api/protocol_event_buffer.h', - 'internal_api/attachments/public/attachment_downloader.h', - 'internal_api/attachments/public/attachment_service.h', - 'internal_api/attachments/public/attachment_service_impl.h', - 'internal_api/attachments/public/attachment_service_proxy_for_test.h', - 'internal_api/attachments/public/attachment_service_proxy.h', - 'internal_api/attachments/public/attachment_uploader.h', + 'internal_api/public/attachments/attachment_downloader.h', 'internal_api/public/attachments/attachment_downloader_impl.h', + 'internal_api/public/attachments/attachment_service.h', + 'internal_api/public/attachments/attachment_service_impl.h', + 'internal_api/public/attachments/attachment_service_proxy.h', + 'internal_api/public/attachments/attachment_service_proxy_for_test.h', + 'internal_api/public/attachments/attachment_store_handle.h', + 'internal_api/public/attachments/attachment_uploader.h', 'internal_api/public/attachments/attachment_uploader_impl.h', '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/task_queue.h', 'internal_api/public/base/attachment_id_proto.cc', 'internal_api/public/base/attachment_id_proto.h', diff --git a/sync/sync_tests.gypi b/sync/sync_tests.gypi index 32ed597..6e3c03a 100644 --- a/sync/sync_tests.gypi +++ b/sync/sync_tests.gypi @@ -260,7 +260,6 @@ 'sources': [ 'api/attachments/attachment_id_unittest.cc', 'api/attachments/attachment_unittest.cc', - 'api/attachments/fake_attachment_store_unittest.cc', 'api/sync_change_unittest.cc', 'api/sync_data_unittest.cc', 'api/sync_error_unittest.cc', @@ -281,9 +280,11 @@ 'internal_api/attachments/attachment_downloader_impl_unittest.cc', '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_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/task_queue_unittest.cc', 'internal_api/debug_info_event_listener_unittest.cc', 'internal_api/http_bridge_unittest.cc', |