diff options
author | bnc <bnc@chromium.org> | 2015-03-09 16:12:55 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-03-09 23:14:13 +0000 |
commit | cd8db1dfaf165cbca88f38ecf50a5cacc09959b1 (patch) | |
tree | 7915be516bf8ddc3d7da0156749f3536e32aa00a | |
parent | 982eb90bf74535cbe5e21c4eacb759c42ff424d4 (diff) | |
download | chromium_src-cd8db1dfaf165cbca88f38ecf50a5cacc09959b1.zip chromium_src-cd8db1dfaf165cbca88f38ecf50a5cacc09959b1.tar.gz chromium_src-cd8db1dfaf165cbca88f38ecf50a5cacc09959b1.tar.bz2 |
Revert of [Sync] Refactor AttachmentStore classes. Introduce concept of referrer. (patchset #2 id:20001 of https://codereview.chromium.org/986743004/)
Reason for revert:
I think this CL is responsible for tree closing compile failure https://build.chromium.org/p/chromium.mac/builders/iOS_Device/builds/26672/steps/compile/logs/stdio.
Original issue's description:
> [Sync] Refactor AttachmentStore classes. Introduce concept of referrer.
>
> In this change:
> - Move towards following class hierarchy: http://www.plantuml.com:80/plantuml/png/ZPBTJiCm38NlynIvv4TzWsaI525nCI5jkw-on1EZQHmvwQG9U7TC4-ZeKiITvNm-EiSExbv1Hxc6VOszYs24mDHQeG6xFNcGRqBAknYLVkd0nKr40l7nmqrU1deDeRUHYrfPkrEw3LmJx848YCkWqODfkECZBIOAZuHin9abWxUo9b0Hdjt38RGJyEhwZ4YZI9kJq_mmRp0pAPKnd7pGgG8t6usTHyVe7_FPtY3mbOth9ghGDjGxTnwlaEq-ySjv-KmCwZflxvVyE5bSIa4Mw7ZGyDHvAyHurPkgkhZgz9PuoNp75tDhAUZcJ68cwkATHyfXnFWn4_PFDsKuNLuKLrFodGS-0G00
> - Remove AttachmenService::GetStore. Now attachment store is owned by
> model type, there is no need to get it from AttachmentService
> - Introduce AttachmentReferrer. There is no functionality behind it yet
> and interface is not complete for it. These will come in the next change.
>
> BUG=457735
> R=maniscalco@chromium.org
> TEST=Not exposed in chrome. Only unit tests available (sync_unit_tests)
>
> Committed: https://crrev.com/6adcada8a799057c31c0f17550c9e2747a8df847
> Cr-Commit-Position: refs/heads/master@{#319733}
TBR=maniscalco@chromium.org,cjhopman@chromium.org,pavely@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=457735
Review URL: https://codereview.chromium.org/996473005
Cr-Commit-Position: refs/heads/master@{#319760}
43 files changed, 382 insertions, 503 deletions
diff --git a/chrome/browser/sync/profile_sync_components_factory_impl.cc b/chrome/browser/sync/profile_sync_components_factory_impl.cc index 6f078f1..c71e485 100644 --- a/chrome/browser/sync/profile_sync_components_factory_impl.cc +++ b/chrome/browser/sync/profile_sync_components_factory_impl.cc @@ -640,7 +640,7 @@ OAuth2TokenService* TokenServiceProvider::GetTokenService() { scoped_ptr<syncer::AttachmentService> ProfileSyncComponentsFactoryImpl::CreateAttachmentService( - scoped_ptr<syncer::AttachmentStore> attachment_store, + const scoped_refptr<syncer::AttachmentStore>& attachment_store, const syncer::UserShare& user_share, const std::string& store_birthday, syncer::ModelType model_type, @@ -685,10 +685,12 @@ ProfileSyncComponentsFactoryImpl::CreateAttachmentService( base::TimeDelta::FromMinutes(30); const base::TimeDelta max_backoff_delay = base::TimeDelta::FromHours(4); scoped_ptr<syncer::AttachmentService> attachment_service( - new syncer::AttachmentServiceImpl( - attachment_store.Pass(), attachment_uploader.Pass(), - attachment_downloader.Pass(), delegate, initial_backoff_delay, - max_backoff_delay)); + new syncer::AttachmentServiceImpl(attachment_store, + attachment_uploader.Pass(), + attachment_downloader.Pass(), + delegate, + initial_backoff_delay, + max_backoff_delay)); return attachment_service.Pass(); } diff --git a/chrome/browser/sync/profile_sync_components_factory_impl.h b/chrome/browser/sync/profile_sync_components_factory_impl.h index ca8037a..5fe2f1d 100644 --- a/chrome/browser/sync/profile_sync_components_factory_impl.h +++ b/chrome/browser/sync/profile_sync_components_factory_impl.h @@ -66,7 +66,7 @@ class ProfileSyncComponentsFactoryImpl : public ProfileSyncComponentsFactory { base::WeakPtr<syncer::SyncableService> GetSyncableServiceForType( syncer::ModelType type) override; scoped_ptr<syncer::AttachmentService> CreateAttachmentService( - scoped_ptr<syncer::AttachmentStore> attachment_store, + const scoped_refptr<syncer::AttachmentStore>& attachment_store, const syncer::UserShare& user_share, const std::string& store_birthday, syncer::ModelType model_type, diff --git a/chrome/browser/sync/profile_sync_components_factory_mock.cc b/chrome/browser/sync/profile_sync_components_factory_mock.cc index c6181ac..2c4d64b 100644 --- a/chrome/browser/sync/profile_sync_components_factory_mock.cc +++ b/chrome/browser/sync/profile_sync_components_factory_mock.cc @@ -37,7 +37,7 @@ ProfileSyncComponentsFactoryMock::~ProfileSyncComponentsFactoryMock() {} scoped_ptr<syncer::AttachmentService> ProfileSyncComponentsFactoryMock::CreateAttachmentService( - scoped_ptr<syncer::AttachmentStore> attachment_store, + const scoped_refptr<syncer::AttachmentStore>& attachment_store, const syncer::UserShare& user_share, const std::string& store_birthday, syncer::ModelType model_type, diff --git a/chrome/browser/sync/profile_sync_components_factory_mock.h b/chrome/browser/sync/profile_sync_components_factory_mock.h index ae0a1a1..49d41f3 100644 --- a/chrome/browser/sync/profile_sync_components_factory_mock.h +++ b/chrome/browser/sync/profile_sync_components_factory_mock.h @@ -52,7 +52,7 @@ class ProfileSyncComponentsFactoryMock : public ProfileSyncComponentsFactory { MOCK_METHOD1(GetSyncableServiceForType, base::WeakPtr<syncer::SyncableService>(syncer::ModelType)); virtual scoped_ptr<syncer::AttachmentService> CreateAttachmentService( - scoped_ptr<syncer::AttachmentStore> attachment_store, + const scoped_refptr<syncer::AttachmentStore>& attachment_store, const syncer::UserShare& user_share, const std::string& store_birthday, syncer::ModelType model_type, diff --git a/components/dom_distiller/core/dom_distiller_store.h b/components/dom_distiller/core/dom_distiller_store.h index 2440ce5..103d43e 100644 --- a/components/dom_distiller/core/dom_distiller_store.h +++ b/components/dom_distiller/core/dom_distiller_store.h @@ -199,7 +199,7 @@ class DomDistillerStore : public syncer::SyncableService, scoped_ptr<syncer::SyncErrorFactory> error_factory_; scoped_ptr<leveldb_proto::ProtoDatabase<ArticleEntry> > database_; bool database_loaded_; - scoped_ptr<syncer::AttachmentStore> attachment_store_; + scoped_refptr<syncer::AttachmentStore> attachment_store_; ObserverList<DomDistillerObserver> observers_; DomDistillerModel model_; diff --git a/components/sync_driver/device_info_data_type_controller_unittest.cc b/components/sync_driver/device_info_data_type_controller_unittest.cc index e2b7cbc3..e1c4af3 100644 --- a/components/sync_driver/device_info_data_type_controller_unittest.cc +++ b/components/sync_driver/device_info_data_type_controller_unittest.cc @@ -65,7 +65,7 @@ class DeviceInfoDataTypeControllerTest : public testing::Test, } scoped_ptr<syncer::AttachmentService> CreateAttachmentService( - scoped_ptr<syncer::AttachmentStore> attachment_store, + const scoped_refptr<syncer::AttachmentStore>& attachment_store, const syncer::UserShare& user_share, const std::string& store_birthday, syncer::ModelType model_type, diff --git a/components/sync_driver/fake_generic_change_processor.cc b/components/sync_driver/fake_generic_change_processor.cc index 165173f..4d2675d 100644 --- a/components/sync_driver/fake_generic_change_processor.cc +++ b/components/sync_driver/fake_generic_change_processor.cc @@ -20,7 +20,7 @@ FakeGenericChangeProcessor::FakeGenericChangeProcessor( base::WeakPtr<syncer::SyncMergeResult>(), NULL, sync_factory, - nullptr), + scoped_refptr<syncer::AttachmentStore>()), sync_model_has_user_created_nodes_(true), sync_model_has_user_created_nodes_success_(true) { } diff --git a/components/sync_driver/generic_change_processor.cc b/components/sync_driver/generic_change_processor.cc index 8f3eff1..60d9977 100644 --- a/components/sync_driver/generic_change_processor.cc +++ b/components/sync_driver/generic_change_processor.cc @@ -94,7 +94,7 @@ GenericChangeProcessor::GenericChangeProcessor( const base::WeakPtr<syncer::SyncMergeResult>& merge_result, syncer::UserShare* user_share, SyncApiComponentFactory* sync_factory, - scoped_ptr<syncer::AttachmentStore> attachment_store) + const scoped_refptr<syncer::AttachmentStore>& attachment_store) : ChangeProcessor(error_handler), type_(type), local_service_(local_service), @@ -103,14 +103,14 @@ GenericChangeProcessor::GenericChangeProcessor( weak_ptr_factory_(this) { DCHECK(CalledOnValidThread()); DCHECK_NE(type_, syncer::UNSPECIFIED); - if (attachment_store) { + if (attachment_store.get()) { std::string store_birthday; { syncer::ReadTransaction trans(FROM_HERE, share_handle()); store_birthday = trans.GetStoreBirthday(); } attachment_service_ = sync_factory->CreateAttachmentService( - attachment_store.Pass(), *user_share, store_birthday, type, this); + attachment_store, *user_share, store_birthday, type, this); attachment_service_weak_ptr_factory_.reset( new base::WeakPtrFactory<syncer::AttachmentService>( attachment_service_.get())); diff --git a/components/sync_driver/generic_change_processor.h b/components/sync_driver/generic_change_processor.h index e49d8ac..8a94936 100644 --- a/components/sync_driver/generic_change_processor.h +++ b/components/sync_driver/generic_change_processor.h @@ -54,7 +54,7 @@ class GenericChangeProcessor : public ChangeProcessor, const base::WeakPtr<syncer::SyncMergeResult>& merge_result, syncer::UserShare* user_share, SyncApiComponentFactory* sync_factory, - scoped_ptr<syncer::AttachmentStore> attachment_store); + const scoped_refptr<syncer::AttachmentStore>& attachment_store); ~GenericChangeProcessor() override; // ChangeProcessor interface. diff --git a/components/sync_driver/generic_change_processor_factory.cc b/components/sync_driver/generic_change_processor_factory.cc index ecc766a..1a7a8d9 100644 --- a/components/sync_driver/generic_change_processor_factory.cc +++ b/components/sync_driver/generic_change_processor_factory.cc @@ -24,10 +24,13 @@ GenericChangeProcessorFactory::CreateGenericChangeProcessor( SyncApiComponentFactory* sync_factory) { DCHECK(user_share); return make_scoped_ptr(new GenericChangeProcessor( - type, error_handler, local_service, merge_result, - user_share, sync_factory, - local_service->GetAttachmentStoreForSync())) - .Pass(); + type, + error_handler, + local_service, + merge_result, + user_share, + sync_factory, + local_service->GetAttachmentStore())).Pass(); } } // namespace sync_driver diff --git a/components/sync_driver/generic_change_processor_unittest.cc b/components/sync_driver/generic_change_processor_unittest.cc index 8d30500..cb0a4fc 100644 --- a/components/sync_driver/generic_change_processor_unittest.cc +++ b/components/sync_driver/generic_change_processor_unittest.cc @@ -37,7 +37,8 @@ namespace { // A mock that keeps track of attachments passed to UploadAttachments. class MockAttachmentService : public syncer::AttachmentServiceImpl { public: - MockAttachmentService(scoped_ptr<syncer::AttachmentStore> attachment_store); + MockAttachmentService( + const scoped_refptr<syncer::AttachmentStore>& attachment_store); ~MockAttachmentService() override; void UploadAttachments( const syncer::AttachmentIdSet& attachment_ids) override; @@ -48,8 +49,8 @@ class MockAttachmentService : public syncer::AttachmentServiceImpl { }; MockAttachmentService::MockAttachmentService( - scoped_ptr<syncer::AttachmentStore> attachment_store) - : AttachmentServiceImpl(attachment_store.Pass(), + const scoped_refptr<syncer::AttachmentStore>& attachment_store) + : AttachmentServiceImpl(attachment_store, scoped_ptr<syncer::AttachmentUploader>( new syncer::FakeAttachmentUploader), scoped_ptr<syncer::AttachmentDownloader>( @@ -77,7 +78,9 @@ MockAttachmentService::attachment_id_sets() { // pass MockAttachmentService to it. class MockSyncApiComponentFactory : public SyncApiComponentFactory { public: - MockSyncApiComponentFactory() {} + MockSyncApiComponentFactory( + scoped_ptr<syncer::AttachmentService> attachment_service) + : attachment_service_(attachment_service.Pass()) {} base::WeakPtr<syncer::SyncableService> GetSyncableServiceForType( syncer::ModelType type) override { @@ -87,27 +90,17 @@ class MockSyncApiComponentFactory : public SyncApiComponentFactory { } scoped_ptr<syncer::AttachmentService> CreateAttachmentService( - scoped_ptr<syncer::AttachmentStore> attachment_store, + const scoped_refptr<syncer::AttachmentStore>& attachment_store, const syncer::UserShare& user_share, const std::string& store_birthday, syncer::ModelType model_type, syncer::AttachmentService::Delegate* delegate) override { - scoped_ptr<MockAttachmentService> attachment_service( - new MockAttachmentService(attachment_store.Pass())); - // GenericChangeProcessor takes ownership of the AttachmentService, but we - // need to have a pointer to it so we can see that it was used properly. - // Take a pointer and trust that GenericChangeProcessor does not prematurely - // destroy it. - mock_attachment_service_ = attachment_service.get(); - return attachment_service.Pass(); - } - - MockAttachmentService* GetMockAttachmentService() { - return mock_attachment_service_; + EXPECT_TRUE(attachment_service_ != NULL); + return attachment_service_.Pass(); } private: - MockAttachmentService* mock_attachment_service_; + scoped_ptr<syncer::AttachmentService> attachment_service_; }; class SyncGenericChangeProcessorTest : public testing::Test { @@ -156,15 +149,25 @@ class SyncGenericChangeProcessorTest : public testing::Test { } void ConstructGenericChangeProcessor(syncer::ModelType type) { - MockSyncApiComponentFactory sync_factory; - scoped_ptr<syncer::AttachmentStore> attachment_store = + scoped_refptr<syncer::AttachmentStore> attachment_store = syncer::AttachmentStore::CreateInMemoryStore(); - change_processor_.reset(new GenericChangeProcessor( - type, &data_type_error_handler_, - syncable_service_ptr_factory_.GetWeakPtr(), - merge_result_ptr_factory_->GetWeakPtr(), test_user_share_->user_share(), - &sync_factory, attachment_store.Pass())); - mock_attachment_service_ = sync_factory.GetMockAttachmentService(); + scoped_ptr<MockAttachmentService> mock_attachment_service( + new MockAttachmentService(attachment_store)); + // GenericChangeProcessor takes ownership of the AttachmentService, but we + // need to have a pointer to it so we can see that it was used properly. + // Take a pointer and trust that GenericChangeProcessor does not prematurely + // destroy it. + mock_attachment_service_ = mock_attachment_service.get(); + sync_factory_.reset( + new MockSyncApiComponentFactory(mock_attachment_service.Pass())); + change_processor_.reset( + new GenericChangeProcessor(type, + &data_type_error_handler_, + syncable_service_ptr_factory_.GetWeakPtr(), + merge_result_ptr_factory_->GetWeakPtr(), + test_user_share_->user_share(), + sync_factory_.get(), + attachment_store)); } void BuildChildNodes(syncer::ModelType type, int n) { @@ -208,6 +211,7 @@ class SyncGenericChangeProcessorTest : public testing::Test { DataTypeErrorHandlerMock data_type_error_handler_; scoped_ptr<syncer::TestUserShare> test_user_share_; MockAttachmentService* mock_attachment_service_; + scoped_ptr<SyncApiComponentFactory> sync_factory_; scoped_ptr<GenericChangeProcessor> change_processor_; }; diff --git a/components/sync_driver/shared_change_processor_unittest.cc b/components/sync_driver/shared_change_processor_unittest.cc index 8177e01..e76e1e4 100644 --- a/components/sync_driver/shared_change_processor_unittest.cc +++ b/components/sync_driver/shared_change_processor_unittest.cc @@ -51,7 +51,7 @@ class SyncSharedChangeProcessorTest : } virtual scoped_ptr<syncer::AttachmentService> CreateAttachmentService( - scoped_ptr<syncer::AttachmentStore> attachment_store, + const scoped_refptr<syncer::AttachmentStore>& attachment_store, const syncer::UserShare& user_share, const std::string& store_birthday, syncer::ModelType model_type, diff --git a/components/sync_driver/sync_api_component_factory.h b/components/sync_driver/sync_api_component_factory.h index 7ba8200..429ac99 100644 --- a/components/sync_driver/sync_api_component_factory.h +++ b/components/sync_driver/sync_api_component_factory.h @@ -37,7 +37,7 @@ class SyncApiComponentFactory { // provided. AttachmentService doesn't take ownership of delegate, the pointer // must be valid throughout AttachmentService lifetime. virtual scoped_ptr<syncer::AttachmentService> CreateAttachmentService( - scoped_ptr<syncer::AttachmentStore> attachment_store, + const scoped_refptr<syncer::AttachmentStore>& attachment_store, const syncer::UserShare& user_share, const std::string& store_birthday, syncer::ModelType model_type, diff --git a/components/sync_driver/ui_data_type_controller_unittest.cc b/components/sync_driver/ui_data_type_controller_unittest.cc index 0082c16..4ff695f 100644 --- a/components/sync_driver/ui_data_type_controller_unittest.cc +++ b/components/sync_driver/ui_data_type_controller_unittest.cc @@ -57,7 +57,7 @@ class SyncUIDataTypeControllerTest : public testing::Test, } scoped_ptr<syncer::AttachmentService> CreateAttachmentService( - scoped_ptr<syncer::AttachmentStore> attachment_store, + const scoped_refptr<syncer::AttachmentStore>& attachment_store, const syncer::UserShare& user_share, const std::string& store_birthday, syncer::ModelType model_type, diff --git a/sync/BUILD.gn b/sync/BUILD.gn index db83ae9..4e830bc 100644 --- a/sync/BUILD.gn +++ b/sync/BUILD.gn @@ -23,8 +23,6 @@ source_set("sync_core") { "api/attachments/attachment_metadata.h", "api/attachments/attachment_store.cc", "api/attachments/attachment_store.h", - "api/attachments/attachment_store_backend.cc", - "api/attachments/attachment_store_backend.h", "api/string_ordinal.h", "api/sync_change.cc", "api/sync_change.h", @@ -125,7 +123,7 @@ 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_frontend.cc", + "internal_api/attachments/attachment_store_handle.cc", "internal_api/attachments/attachment_uploader.cc", "internal_api/attachments/attachment_uploader_impl.cc", "internal_api/attachments/attachment_util.cc", @@ -165,7 +163,7 @@ source_set("sync_core") { "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_frontend.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/attachment_util.h", @@ -574,7 +572,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_frontend_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", diff --git a/sync/api/attachments/attachment_store.cc b/sync/api/attachments/attachment_store.cc index 7cf774e..91b3f95 100644 --- a/sync/api/attachments/attachment_store.cc +++ b/sync/api/attachments/attachment_store.cc @@ -10,53 +10,16 @@ #include "base/message_loop/message_loop.h" #include "base/sequenced_task_runner.h" #include "base/thread_task_runner_handle.h" -#include "sync/internal_api/public/attachments/attachment_store_frontend.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 { -AttachmentStore::AttachmentStore( - const scoped_refptr<AttachmentStoreFrontend>& frontend, - AttachmentReferrer referrer) - : frontend_(frontend), referrer_(referrer) { -} - -AttachmentStore::~AttachmentStore() { -} - -void AttachmentStore::Read(const AttachmentIdList& ids, - const ReadCallback& callback) { - frontend_->Read(ids, callback); -} +AttachmentStore::AttachmentStore() {} +AttachmentStore::~AttachmentStore() {} -void AttachmentStore::Write(const AttachmentList& attachments, - const WriteCallback& callback) { - frontend_->Write(referrer_, attachments, callback); -} - -void AttachmentStore::Drop(const AttachmentIdList& ids, - const DropCallback& callback) { - frontend_->Drop(referrer_, ids, callback); -} - -void AttachmentStore::ReadMetadata(const AttachmentIdList& ids, - const ReadMetadataCallback& callback) { - frontend_->ReadMetadata(ids, callback); -} - -void AttachmentStore::ReadAllMetadata(const ReadMetadataCallback& callback) { - frontend_->ReadAllMetadata(referrer_, callback); -} - -scoped_ptr<AttachmentStore> AttachmentStore::CreateAttachmentStoreForSync() - const { - scoped_ptr<AttachmentStore> attachment_store( - new AttachmentStore(frontend_, SYNC)); - return attachment_store.Pass(); -} - -scoped_ptr<AttachmentStore> AttachmentStore::CreateInMemoryStore() { +scoped_refptr<AttachmentStore> AttachmentStore::CreateInMemoryStore() { // Both frontend and backend of attachment store will live on current thread. scoped_refptr<base::SingleThreadTaskRunner> runner; if (base::ThreadTaskRunnerHandle::IsSet()) { @@ -69,38 +32,34 @@ scoped_ptr<AttachmentStore> AttachmentStore::CreateInMemoryStore() { } scoped_ptr<AttachmentStoreBackend> backend( new InMemoryAttachmentStore(runner)); - scoped_refptr<AttachmentStoreFrontend> frontend( - new AttachmentStoreFrontend(backend.Pass(), runner)); - scoped_ptr<AttachmentStore> attachment_store( - new AttachmentStore(frontend, MODEL_TYPE)); - return attachment_store.Pass(); + return scoped_refptr<AttachmentStore>( + new AttachmentStoreHandle(backend.Pass(), runner)); } -scoped_ptr<AttachmentStore> AttachmentStore::CreateOnDiskStore( +scoped_refptr<AttachmentStore> AttachmentStore::CreateOnDiskStore( const base::FilePath& path, const scoped_refptr<base::SequencedTaskRunner>& backend_task_runner, const InitCallback& callback) { scoped_ptr<OnDiskAttachmentStore> backend( new OnDiskAttachmentStore(base::ThreadTaskRunnerHandle::Get(), path)); - scoped_refptr<AttachmentStoreFrontend> frontend = - new AttachmentStoreFrontend(backend.Pass(), backend_task_runner); - scoped_ptr<AttachmentStore> attachment_store( - new AttachmentStore(frontend, MODEL_TYPE)); - frontend->Init(callback); + scoped_refptr<AttachmentStore> attachment_store = + new AttachmentStoreHandle(backend.Pass(), backend_task_runner); + attachment_store->Init(callback); + + return attachment_store; +} + +AttachmentStoreBackend::AttachmentStoreBackend( + const scoped_refptr<base::SequencedTaskRunner>& callback_task_runner) + : callback_task_runner_(callback_task_runner) { +} - return attachment_store.Pass(); +AttachmentStoreBackend::~AttachmentStoreBackend() { } -scoped_ptr<AttachmentStore> AttachmentStore::CreateMockStoreForTest( - scoped_ptr<AttachmentStoreBackend> backend) { - scoped_refptr<base::SingleThreadTaskRunner> runner = - base::ThreadTaskRunnerHandle::Get(); - scoped_refptr<AttachmentStoreFrontend> attachment_store_frontend( - new AttachmentStoreFrontend(backend.Pass(), runner)); - scoped_ptr<AttachmentStore> attachment_store( - new AttachmentStore(attachment_store_frontend, MODEL_TYPE)); - return attachment_store.Pass(); +void AttachmentStoreBackend::PostCallback(const base::Closure& callback) { + callback_task_runner_->PostTask(FROM_HERE, callback); } } // namespace syncer diff --git a/sync/api/attachments/attachment_store.h b/sync/api/attachments/attachment_store.h index e8d8f5c..b183de1 100644 --- a/sync/api/attachments/attachment_store.h +++ b/sync/api/attachments/attachment_store.h @@ -15,13 +15,14 @@ namespace base { class FilePath; +class RefCountedMemory; class SequencedTaskRunner; } // namespace base namespace syncer { -class AttachmentStoreFrontend; -class AttachmentStoreBackend; +class Attachment; +class AttachmentId; // AttachmentStore is a place to locally store and access Attachments. // @@ -31,7 +32,8 @@ class AttachmentStoreBackend; // implementations. // 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 { +class SYNC_EXPORT AttachmentStore + : public base::RefCountedThreadSafe<AttachmentStore> { public: // TODO(maniscalco): Consider udpating Read and Write methods to support // resumable transfers (bug 353292). @@ -47,14 +49,6 @@ class SYNC_EXPORT AttachmentStore { static const int RESULT_SIZE = 10; // Size of the Result enum; used for histograms. - // Each attachment can have references from sync or model type. Tracking these - // references is needed for lifetime management of attachment, it can only be - // deleted from the store when it doesn't have references. - enum AttachmentReferrer { - MODEL_TYPE, - SYNC, - }; - typedef base::Callback<void(const Result&)> InitCallback; typedef base::Callback<void(const Result&, scoped_ptr<AttachmentMap>, @@ -65,7 +59,15 @@ class SYNC_EXPORT AttachmentStore { scoped_ptr<AttachmentMetadataList>)> ReadMetadataCallback; - ~AttachmentStore(); + AttachmentStore(); + + // Asynchronously initializes attachment store. + // + // This method should not be called by consumer of this interface. It is + // called by factory methods in AttachmentStore class. When initialization is + // complete |callback| is invoked with result, in case of failure result is + // UNSPECIFIED_ERROR. + virtual void Init(const InitCallback& callback) = 0; // Asynchronously reads the attachments identified by |ids|. // @@ -79,7 +81,8 @@ class SYNC_EXPORT AttachmentStore { // // Reads on individual attachments are treated atomically; |callback| will not // read only part of an attachment. - void Read(const AttachmentIdList& ids, const ReadCallback& callback); + virtual void Read(const AttachmentIdList& ids, + const ReadCallback& callback) = 0; // Asynchronously writes |attachments| to the store. // @@ -90,7 +93,8 @@ class SYNC_EXPORT AttachmentStore { // not be written |callback|'s Result will be UNSPECIFIED_ERROR. When this // happens, some or none of the attachments may have been written // successfully. - void Write(const AttachmentList& attachments, const WriteCallback& callback); + virtual void Write(const AttachmentList& attachments, + const WriteCallback& callback) = 0; // Asynchronously drops |attchments| from this store. // @@ -101,7 +105,8 @@ class SYNC_EXPORT AttachmentStore { // could not be dropped, |callback|'s Result will be UNSPECIFIED_ERROR. When // this happens, some or none of the attachments may have been dropped // successfully. - void Drop(const AttachmentIdList& ids, const DropCallback& callback); + virtual void Drop(const AttachmentIdList& ids, + const DropCallback& callback) = 0; // Asynchronously reads metadata for the attachments identified by |ids|. // @@ -109,52 +114,72 @@ class SYNC_EXPORT AttachmentStore { // read metadata for all attachments specified in ids. If any of the // metadata entries do not exist or could not be read, |callback|'s Result // will be UNSPECIFIED_ERROR. - void ReadMetadata(const AttachmentIdList& ids, - const ReadMetadataCallback& callback); + virtual void ReadMetadata(const AttachmentIdList& ids, + const ReadMetadataCallback& callback) = 0; // Asynchronously reads metadata for all attachments in the store. // // |callback| will be invoked when finished. If any of the metadata entries // could not be read, |callback|'s Result will be UNSPECIFIED_ERROR. - void ReadAllMetadata(const ReadMetadataCallback& callback); - - // Given current AttachmentStore (this) creates separate AttachmentStore that - // will be used by sync components (AttachmentService). Resulting - // AttachmentStore is backed by the same frontend/backend. - scoped_ptr<AttachmentStore> CreateAttachmentStoreForSync() const; - - // Creates an AttachmentStore backed by in-memory implementation of attachment - // store. For now frontend lives on the same thread as backend. - static scoped_ptr<AttachmentStore> CreateInMemoryStore(); - - // Creates an AttachmentStore 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. Calling Read/Write/Drop before - // initialization completed is allowed. Later if initialization fails these - // operations will fail with STORE_INITIALIZATION_FAILED error. - static scoped_ptr<AttachmentStore> CreateOnDiskStore( + virtual void ReadAllMetadata(const ReadMetadataCallback& callback) = 0; + + // 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. Calling Read/Write/Drop + // before initialization completed is allowed. Later if initialization fails + // these operations will fail with STORE_INITIALIZATION_FAILED error. + static scoped_refptr<AttachmentStore> CreateOnDiskStore( const base::FilePath& path, const scoped_refptr<base::SequencedTaskRunner>& backend_task_runner, const InitCallback& callback); - // Creates set of AttachmentStore/AttachmentStoreFrontend instances for tests - // that provide their own implementation of AttachmentstoreBackend for - // mocking. - static scoped_ptr<AttachmentStore> CreateMockStoreForTest( - scoped_ptr<AttachmentStoreBackend> backend); + protected: + friend class base::RefCountedThreadSafe<AttachmentStore>; + virtual ~AttachmentStore(); +}; - private: - AttachmentStore(const scoped_refptr<AttachmentStoreFrontend>& frontend, - AttachmentReferrer referrer); +// Interface for AttachmentStore backends. +// +// AttachmentStoreBackend provides interface for different backends (on-disk, +// in-memory). Factory methods in AttachmentStore create corresponding backend +// and pass reference to AttachmentStoreHandle. +// All functions in AttachmentStoreBackend mirror corresponding functions in +// AttachmentStore. +// All callbacks and result codes are used directly from AttachmentStore. +// AttachmentStoreHandle only passes callbacks and results, there is no need to +// declare separate set. +class SYNC_EXPORT AttachmentStoreBackend { + public: + explicit AttachmentStoreBackend( + const scoped_refptr<base::SequencedTaskRunner>& callback_task_runner); + virtual ~AttachmentStoreBackend(); + virtual void Init(const AttachmentStore::InitCallback& callback) = 0; + virtual void Read(const AttachmentIdList& ids, + const AttachmentStore::ReadCallback& callback) = 0; + virtual void Write(const AttachmentList& attachments, + const AttachmentStore::WriteCallback& callback) = 0; + virtual void Drop(const AttachmentIdList& ids, + const AttachmentStore::DropCallback& callback) = 0; + virtual void ReadMetadata( + const AttachmentIdList& ids, + const AttachmentStore::ReadMetadataCallback& callback) = 0; + virtual void ReadAllMetadata( + const AttachmentStore::ReadMetadataCallback& callback) = 0; + + protected: + // Helper function to post callback on callback_task_runner. + void PostCallback(const base::Closure& callback); - scoped_refptr<AttachmentStoreFrontend> frontend_; - // Modification operations with attachment store will be performed on behalf - // of |referrer_|. - const AttachmentReferrer referrer_; + private: + scoped_refptr<base::SequencedTaskRunner> callback_task_runner_; - DISALLOW_COPY_AND_ASSIGN(AttachmentStore); + DISALLOW_COPY_AND_ASSIGN(AttachmentStoreBackend); }; } // namespace syncer diff --git a/sync/api/attachments/attachment_store_backend.cc b/sync/api/attachments/attachment_store_backend.cc deleted file mode 100644 index 44363cc..0000000 --- a/sync/api/attachments/attachment_store_backend.cc +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright 2015 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_backend.h" - -#include "base/location.h" -#include "base/sequenced_task_runner.h" - -namespace syncer { - -AttachmentStoreBackend::AttachmentStoreBackend( - const scoped_refptr<base::SequencedTaskRunner>& callback_task_runner) - : callback_task_runner_(callback_task_runner) { -} - -AttachmentStoreBackend::~AttachmentStoreBackend() { -} - -void AttachmentStoreBackend::PostCallback(const base::Closure& callback) { - callback_task_runner_->PostTask(FROM_HERE, callback); -} - -} // namespace syncer diff --git a/sync/api/attachments/attachment_store_backend.h b/sync/api/attachments/attachment_store_backend.h deleted file mode 100644 index 74859fe..0000000 --- a/sync/api/attachments/attachment_store_backend.h +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright 2015 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_STORE_BACKEND_H_ -#define SYNC_API_ATTACHMENTS_ATTACHMENT_STORE_BACKEND_H_ - -#include "base/callback.h" -#include "base/memory/ref_counted.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 syncer { - -// Interface for AttachmentStore backends. -// -// AttachmentStoreBackend provides interface for different backends (on-disk, -// in-memory). Factory methods in AttachmentStore create corresponding backend -// and pass reference to AttachmentStore. -// All functions in AttachmentStoreBackend mirror corresponding functions in -// AttachmentStore. -// All callbacks and result codes are used directly from AttachmentStore. -// AttachmentStoreFrontend only passes callbacks and results without modifying -// them, there is no need to declare separate set. -class SYNC_EXPORT AttachmentStoreBackend { - public: - explicit AttachmentStoreBackend( - const scoped_refptr<base::SequencedTaskRunner>& callback_task_runner); - virtual ~AttachmentStoreBackend(); - virtual void Init(const AttachmentStore::InitCallback& callback) = 0; - virtual void Read(const AttachmentIdList& ids, - const AttachmentStore::ReadCallback& callback) = 0; - virtual void Write(AttachmentStore::AttachmentReferrer referrer, - const AttachmentList& attachments, - const AttachmentStore::WriteCallback& callback) = 0; - virtual void Drop(AttachmentStore::AttachmentReferrer referrer, - const AttachmentIdList& ids, - const AttachmentStore::DropCallback& callback) = 0; - virtual void ReadMetadata( - const AttachmentIdList& ids, - const AttachmentStore::ReadMetadataCallback& callback) = 0; - virtual void ReadAllMetadata( - AttachmentStore::AttachmentReferrer referrer, - const AttachmentStore::ReadMetadataCallback& callback) = 0; - - protected: - // Helper function to post callback on callback_task_runner. - void PostCallback(const base::Closure& callback); - - private: - scoped_refptr<base::SequencedTaskRunner> callback_task_runner_; - - DISALLOW_COPY_AND_ASSIGN(AttachmentStoreBackend); -}; - -} // namespace syncer - -#endif // SYNC_API_ATTACHMENTS_ATTACHMENT_STORE_BACKEND_H_ diff --git a/sync/api/fake_syncable_service.cc b/sync/api/fake_syncable_service.cc index d2177e9..ec7b90c 100644 --- a/sync/api/fake_syncable_service.cc +++ b/sync/api/fake_syncable_service.cc @@ -26,8 +26,8 @@ void FakeSyncableService::set_process_sync_changes_error( } void FakeSyncableService::set_attachment_store( - scoped_ptr<AttachmentStore> attachment_store) { - attachment_store_ = attachment_store.Pass(); + const scoped_refptr<AttachmentStore>& attachment_store) { + attachment_store_ = attachment_store; } const AttachmentService* FakeSyncableService::attachment_service() const { @@ -70,9 +70,8 @@ SyncError FakeSyncableService::ProcessSyncChanges( return process_sync_changes_error_; } -scoped_ptr<AttachmentStore> FakeSyncableService::GetAttachmentStoreForSync() { - return attachment_store_ ? attachment_store_->CreateAttachmentStoreForSync() - : scoped_ptr<AttachmentStore>(); +scoped_refptr<AttachmentStore> FakeSyncableService::GetAttachmentStore() { + return attachment_store_; } void FakeSyncableService::SetAttachmentService( diff --git a/sync/api/fake_syncable_service.h b/sync/api/fake_syncable_service.h index 9b75647..7403ef3 100644 --- a/sync/api/fake_syncable_service.h +++ b/sync/api/fake_syncable_service.h @@ -23,7 +23,8 @@ class FakeSyncableService : public SyncableService { void set_process_sync_changes_error(const SyncError& error); // Setter for AttachmentStore. - void set_attachment_store(scoped_ptr<AttachmentStore> attachment_store); + void set_attachment_store( + const scoped_refptr<AttachmentStore>& attachment_store); // AttachmentService should be set when this syncable service is connected, // just before MergeDataAndStartSyncing. NULL is returned by default. @@ -43,7 +44,7 @@ class FakeSyncableService : public SyncableService { SyncDataList GetAllSyncData(ModelType type) const override; SyncError ProcessSyncChanges(const tracked_objects::Location& from_here, const SyncChangeList& change_list) override; - scoped_ptr<AttachmentStore> GetAttachmentStoreForSync() override; + scoped_refptr<AttachmentStore> GetAttachmentStore() override; void SetAttachmentService( scoped_ptr<AttachmentService> attachment_service) override; @@ -53,7 +54,7 @@ class FakeSyncableService : public SyncableService { SyncError process_sync_changes_error_; bool syncing_; ModelType type_; - scoped_ptr<AttachmentStore> attachment_store_; + scoped_refptr<AttachmentStore> attachment_store_; scoped_ptr<AttachmentService> attachment_service_; }; diff --git a/sync/api/syncable_service.cc b/sync/api/syncable_service.cc index f0cfe67..7b0a973 100644 --- a/sync/api/syncable_service.cc +++ b/sync/api/syncable_service.cc @@ -8,8 +8,8 @@ namespace syncer { SyncableService::~SyncableService() {} -scoped_ptr<AttachmentStore> SyncableService::GetAttachmentStoreForSync() { - return scoped_ptr<AttachmentStore>(); +scoped_refptr<AttachmentStore> SyncableService::GetAttachmentStore() { + return scoped_refptr<AttachmentStore>(); } void SyncableService::SetAttachmentService( diff --git a/sync/api/syncable_service.h b/sync/api/syncable_service.h index 0d5aae4..f83a017 100644 --- a/sync/api/syncable_service.h +++ b/sync/api/syncable_service.h @@ -66,16 +66,15 @@ class SYNC_EXPORT SyncableService SyncError ProcessSyncChanges(const tracked_objects::Location& from_here, const SyncChangeList& change_list) override = 0; - // Returns AttachmentStore for use by sync when uploading or downloading - // attachments. + // Returns AttachmentStore used by datatype. Attachment store is used by sync + // when uploading or downloading attachments. // GetAttachmentStore is called right before MergeDataAndStartSyncing. If at // that time GetAttachmentStore returns NULL then datatype is considered not // using attachments and all attempts to upload/download attachments will // fail. Default implementation returns NULL. Datatype that uses sync - // attachments should create attachment store, implement GetAttachmentStore - // to return result of AttachmentStore::CreateAttachmentStoreForSync() from - // attachment store object. - virtual scoped_ptr<AttachmentStore> GetAttachmentStoreForSync(); + // attachemnts should create attachment store and implement GetAttachmentStore + // to return pointer to it. + virtual scoped_refptr<AttachmentStore> GetAttachmentStore(); // Called by sync to provide AttachmentService to be used to download // attachments. diff --git a/sync/internal_api/attachments/attachment_service_impl.cc b/sync/internal_api/attachments/attachment_service_impl.cc index e761515..cc04408 100644 --- a/sync/internal_api/attachments/attachment_service_impl.cc +++ b/sync/internal_api/attachments/attachment_service_impl.cc @@ -110,13 +110,13 @@ AttachmentServiceImpl::GetOrDownloadState::PostResultIfAllRequestsCompleted() { } AttachmentServiceImpl::AttachmentServiceImpl( - scoped_ptr<AttachmentStore> attachment_store, + scoped_refptr<AttachmentStore> attachment_store, scoped_ptr<AttachmentUploader> attachment_uploader, scoped_ptr<AttachmentDownloader> attachment_downloader, Delegate* delegate, const base::TimeDelta& initial_backoff_delay, const base::TimeDelta& max_backoff_delay) - : attachment_store_(attachment_store.Pass()), + : attachment_store_(attachment_store), attachment_uploader_(attachment_uploader.Pass()), attachment_downloader_(attachment_downloader.Pass()), delegate_(delegate), @@ -144,14 +144,14 @@ AttachmentServiceImpl::~AttachmentServiceImpl() { // Static. scoped_ptr<syncer::AttachmentService> AttachmentServiceImpl::CreateForTest() { - scoped_ptr<syncer::AttachmentStore> attachment_store = + scoped_refptr<syncer::AttachmentStore> attachment_store = AttachmentStore::CreateInMemoryStore(); scoped_ptr<AttachmentUploader> attachment_uploader( new FakeAttachmentUploader); scoped_ptr<AttachmentDownloader> attachment_downloader( new FakeAttachmentDownloader()); scoped_ptr<syncer::AttachmentService> attachment_service( - new syncer::AttachmentServiceImpl(attachment_store.Pass(), + new syncer::AttachmentServiceImpl(attachment_store, attachment_uploader.Pass(), attachment_downloader.Pass(), NULL, @@ -160,6 +160,10 @@ scoped_ptr<syncer::AttachmentService> AttachmentServiceImpl::CreateForTest() { return attachment_service.Pass(); } +AttachmentStore* AttachmentServiceImpl::GetStore() { + return attachment_store_.get(); +} + void AttachmentServiceImpl::GetOrDownloadAttachments( const AttachmentIdList& attachment_ids, const GetOrDownloadCallback& callback) { @@ -168,7 +172,8 @@ void AttachmentServiceImpl::GetOrDownloadAttachments( new GetOrDownloadState(attachment_ids, callback)); attachment_store_->Read(attachment_ids, base::Bind(&AttachmentServiceImpl::ReadDone, - weak_ptr_factory_.GetWeakPtr(), state)); + weak_ptr_factory_.GetWeakPtr(), + state)); } void AttachmentServiceImpl::ReadDone( diff --git a/sync/internal_api/attachments/attachment_service_impl_unittest.cc b/sync/internal_api/attachments/attachment_service_impl_unittest.cc index 8519244..e72bb8f 100644 --- a/sync/internal_api/attachments/attachment_service_impl_unittest.cc +++ b/sync/internal_api/attachments/attachment_service_impl_unittest.cc @@ -8,9 +8,7 @@ #include "base/memory/weak_ptr.h" #include "base/message_loop/message_loop.h" #include "base/run_loop.h" -#include "base/thread_task_runner_handle.h" #include "base/timer/mock_timer.h" -#include "sync/api/attachments/attachment_store_backend.h" #include "sync/internal_api/public/attachments/attachment_util.h" #include "sync/internal_api/public/attachments/fake_attachment_downloader.h" #include "sync/internal_api/public/attachments/fake_attachment_uploader.h" @@ -21,46 +19,37 @@ namespace syncer { namespace { -class MockAttachmentStoreBackend - : public AttachmentStoreBackend, - public base::SupportsWeakPtr<MockAttachmentStoreBackend> { +class MockAttachmentStore : public AttachmentStore, + public base::SupportsWeakPtr<MockAttachmentStore> { public: - MockAttachmentStoreBackend( - const scoped_refptr<base::SequencedTaskRunner>& callback_task_runner) - : AttachmentStoreBackend(callback_task_runner) {} + MockAttachmentStore() {} - ~MockAttachmentStoreBackend() override {} - - void Init(const AttachmentStore::InitCallback& callback) override {} + void Init(const InitCallback& callback) override { + } void Read(const AttachmentIdList& ids, - const AttachmentStore::ReadCallback& callback) override { + const ReadCallback& callback) override { read_ids.push_back(ids); read_callbacks.push_back(callback); } - void Write(AttachmentStore::AttachmentReferrer referrer, - const AttachmentList& attachments, - const AttachmentStore::WriteCallback& callback) override { + void Write(const AttachmentList& attachments, + const WriteCallback& callback) override { write_attachments.push_back(attachments); write_callbacks.push_back(callback); } - void Drop(AttachmentStore::AttachmentReferrer referrer, - const AttachmentIdList& ids, - const AttachmentStore::DropCallback& callback) override { + void Drop(const AttachmentIdList& ids, + const DropCallback& callback) override { NOTREACHED(); } - void ReadMetadata( - const AttachmentIdList& ids, - const AttachmentStore::ReadMetadataCallback& callback) override { + void ReadMetadata(const AttachmentIdList& ids, + const ReadMetadataCallback& callback) override { NOTREACHED(); } - void ReadAllMetadata( - AttachmentStore::AttachmentReferrer referrer, - const AttachmentStore::ReadMetadataCallback& callback) override { + void ReadAllMetadata(const ReadMetadataCallback& callback) override { NOTREACHED(); } @@ -68,7 +57,7 @@ class MockAttachmentStoreBackend // returned, everything else should be reported unavailable. void RespondToRead(const AttachmentIdSet& local_attachments) { scoped_refptr<base::RefCountedString> data = new base::RefCountedString(); - AttachmentStore::ReadCallback callback = read_callbacks.back(); + ReadCallback callback = read_callbacks.back(); AttachmentIdList ids = read_ids.back(); read_callbacks.pop_back(); read_ids.pop_back(); @@ -87,9 +76,8 @@ class MockAttachmentStoreBackend unavailable_attachments->push_back(*iter); } } - AttachmentStore::Result result = unavailable_attachments->empty() - ? AttachmentStore::SUCCESS - : AttachmentStore::UNSPECIFIED_ERROR; + Result result = + unavailable_attachments->empty() ? SUCCESS : UNSPECIFIED_ERROR; base::MessageLoop::current()->PostTask( FROM_HERE, @@ -100,8 +88,8 @@ class MockAttachmentStoreBackend } // Respond to Write request with |result|. - void RespondToWrite(const AttachmentStore::Result& result) { - AttachmentStore::WriteCallback callback = write_callbacks.back(); + void RespondToWrite(const Result& result) { + WriteCallback callback = write_callbacks.back(); AttachmentList attachments = write_attachments.back(); write_callbacks.pop_back(); write_attachments.pop_back(); @@ -110,12 +98,14 @@ class MockAttachmentStoreBackend } std::vector<AttachmentIdList> read_ids; - std::vector<AttachmentStore::ReadCallback> read_callbacks; + std::vector<ReadCallback> read_callbacks; std::vector<AttachmentList> write_attachments; - std::vector<AttachmentStore::WriteCallback> write_callbacks; + std::vector<WriteCallback> write_callbacks; private: - DISALLOW_COPY_AND_ASSIGN(MockAttachmentStoreBackend); + ~MockAttachmentStore() override {} + + DISALLOW_COPY_AND_ASSIGN(MockAttachmentStore); }; class MockAttachmentDownloader @@ -195,8 +185,7 @@ class AttachmentServiceImplTest : public testing::Test, void TearDown() override { attachment_service_.reset(); - RunLoop(); - ASSERT_FALSE(attachment_store_backend_); + ASSERT_FALSE(attachment_store_); ASSERT_FALSE(attachment_uploader_); ASSERT_FALSE(attachment_downloader_); } @@ -210,15 +199,9 @@ class AttachmentServiceImplTest : public testing::Test, scoped_ptr<MockAttachmentUploader> uploader, scoped_ptr<MockAttachmentDownloader> downloader, AttachmentService::Delegate* delegate) { - // Initialize mock attachment store - scoped_refptr<base::SingleThreadTaskRunner> runner = - base::ThreadTaskRunnerHandle::Get(); - scoped_ptr<MockAttachmentStoreBackend> attachment_store_backend( - new MockAttachmentStoreBackend(runner)); - attachment_store_backend_ = attachment_store_backend->AsWeakPtr(); - scoped_ptr<AttachmentStore> attachment_store = - AttachmentStore::CreateMockStoreForTest( - attachment_store_backend.Pass()); + scoped_refptr<MockAttachmentStore> attachment_store( + new MockAttachmentStore()); + attachment_store_ = attachment_store->AsWeakPtr(); if (uploader.get()) { attachment_uploader_ = uploader->AsWeakPtr(); @@ -226,9 +209,13 @@ class AttachmentServiceImplTest : public testing::Test, if (downloader.get()) { attachment_downloader_ = downloader->AsWeakPtr(); } - attachment_service_.reset(new AttachmentServiceImpl( - attachment_store.Pass(), uploader.Pass(), downloader.Pass(), delegate, - base::TimeDelta::FromMinutes(1), base::TimeDelta::FromMinutes(8))); + attachment_service_.reset( + new AttachmentServiceImpl(attachment_store, + uploader.Pass(), + downloader.Pass(), + delegate, + base::TimeDelta::FromMinutes(1), + base::TimeDelta::FromMinutes(8))); scoped_ptr<base::MockTimer> timer_to_pass( new base::MockTimer(false, false)); @@ -260,8 +247,8 @@ class AttachmentServiceImplTest : public testing::Test, RunLoop(); if (mock_timer()->IsRunning()) { mock_timer()->Fire(); - RunLoop(); } + RunLoop(); } const std::vector<AttachmentService::GetOrDownloadResult>& @@ -277,9 +264,7 @@ class AttachmentServiceImplTest : public testing::Test, return network_change_notifier_.get(); } - MockAttachmentStoreBackend* store() { - return attachment_store_backend_.get(); - } + MockAttachmentStore* store() { return attachment_store_.get(); } MockAttachmentDownloader* downloader() { return attachment_downloader_.get(); @@ -296,7 +281,7 @@ class AttachmentServiceImplTest : public testing::Test, private: base::MessageLoop message_loop_; scoped_ptr<net::NetworkChangeNotifier> network_change_notifier_; - base::WeakPtr<MockAttachmentStoreBackend> attachment_store_backend_; + base::WeakPtr<MockAttachmentStore> attachment_store_; base::WeakPtr<MockAttachmentDownloader> attachment_downloader_; base::WeakPtr<MockAttachmentUploader> attachment_uploader_; scoped_ptr<AttachmentServiceImpl> attachment_service_; @@ -307,11 +292,14 @@ class AttachmentServiceImplTest : public testing::Test, std::vector<AttachmentId> on_attachment_uploaded_list_; }; +TEST_F(AttachmentServiceImplTest, GetStore) { + EXPECT_EQ(store(), attachment_service()->GetStore()); +} + TEST_F(AttachmentServiceImplTest, GetOrDownload_EmptyAttachmentList) { AttachmentIdList attachment_ids; attachment_service()->GetOrDownloadAttachments(attachment_ids, download_callback()); - RunLoop(); store()->RespondToRead(AttachmentIdSet()); RunLoop(); @@ -326,7 +314,6 @@ TEST_F(AttachmentServiceImplTest, GetOrDownload_Local) { download_callback()); AttachmentIdSet local_attachments; local_attachments.insert(attachment_ids[0]); - RunLoop(); store()->RespondToRead(local_attachments); RunLoop(); @@ -346,7 +333,6 @@ TEST_F(AttachmentServiceImplTest, GetOrDownload_LocalRemoteUnavailable) { // Call attachment service. attachment_service()->GetOrDownloadAttachments(attachment_ids, download_callback()); - RunLoop(); // Ensure AttachmentStore is called. EXPECT_FALSE(store()->read_ids.empty()); @@ -412,7 +398,6 @@ TEST_F(AttachmentServiceImplTest, GetOrDownload_NoDownloader) { attachment_ids.push_back(AttachmentId::Create()); attachment_service()->GetOrDownloadAttachments(attachment_ids, download_callback()); - RunLoop(); EXPECT_FALSE(store()->read_ids.empty()); AttachmentIdSet local_attachments; diff --git a/sync/internal_api/attachments/attachment_service_proxy.cc b/sync/internal_api/attachments/attachment_service_proxy.cc index 50b7a64..92104ce 100644 --- a/sync/internal_api/attachments/attachment_service_proxy.cc +++ b/sync/internal_api/attachments/attachment_service_proxy.cc @@ -49,6 +49,10 @@ AttachmentServiceProxy::AttachmentServiceProxy( AttachmentServiceProxy::~AttachmentServiceProxy() { } +AttachmentStore* AttachmentServiceProxy::GetStore() { + return NULL; +} + void AttachmentServiceProxy::GetOrDownloadAttachments( const AttachmentIdList& attachment_ids, const GetOrDownloadCallback& callback) { @@ -81,6 +85,10 @@ AttachmentServiceProxy::Core::Core( AttachmentServiceProxy::Core::~Core() { } +AttachmentStore* AttachmentServiceProxy::Core::GetStore() { + return NULL; +} + void AttachmentServiceProxy::Core::GetOrDownloadAttachments( const AttachmentIdList& attachment_ids, const GetOrDownloadCallback& callback) { diff --git a/sync/internal_api/attachments/attachment_service_proxy_unittest.cc b/sync/internal_api/attachments/attachment_service_proxy_unittest.cc index a66b39f..41af1b23 100644 --- a/sync/internal_api/attachments/attachment_service_proxy_unittest.cc +++ b/sync/internal_api/attachments/attachment_service_proxy_unittest.cc @@ -33,6 +33,8 @@ class StubAttachmentService : public AttachmentService, ~StubAttachmentService() override {} + AttachmentStore* GetStore() override { return NULL; } + void GetOrDownloadAttachments( const AttachmentIdList& attachment_ids, const GetOrDownloadCallback& callback) override { @@ -130,6 +132,10 @@ class AttachmentServiceProxyTest : public testing::Test, int count_callback_get_or_download; }; +TEST_F(AttachmentServiceProxyTest, GetStore) { + EXPECT_EQ(NULL, proxy->GetStore()); +} + // Verify that each of AttachmentServiceProxy's methods are invoked on the stub. // Verify that the methods that take callbacks invoke passed callbacks on this // thread. diff --git a/sync/internal_api/attachments/attachment_store_frontend.cc b/sync/internal_api/attachments/attachment_store_handle.cc index 97547de..5e3591b 100644 --- a/sync/internal_api/attachments/attachment_store_frontend.cc +++ b/sync/internal_api/attachments/attachment_store_handle.cc @@ -1,27 +1,26 @@ -// Copyright 2015 The Chromium Authors. All rights reserved. +// 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_frontend.h" +#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" -#include "sync/api/attachments/attachment_store_backend.h" namespace syncer { namespace { -// NoOp is needed to bind base::Passed(backend) in AttachmentStoreFrontend dtor. +// NoOp is needed to bind base::Passed(backend) in AttachmentStoreHandle dtor. // It doesn't need to do anything. void NoOp(scoped_ptr<AttachmentStoreBackend> backend) { } } // namespace -AttachmentStoreFrontend::AttachmentStoreFrontend( +AttachmentStoreHandle::AttachmentStoreHandle( scoped_ptr<AttachmentStoreBackend> backend, const scoped_refptr<base::SequencedTaskRunner>& backend_task_runner) : backend_(backend.Pass()), backend_task_runner_(backend_task_runner) { @@ -29,71 +28,61 @@ AttachmentStoreFrontend::AttachmentStoreFrontend( DCHECK(backend_task_runner_.get()); } -AttachmentStoreFrontend::~AttachmentStoreFrontend() { +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_))); + backend_task_runner_->PostTask( + FROM_HERE, base::Bind(&NoOp, base::Passed(&backend_))); } -void AttachmentStoreFrontend::Init( - const AttachmentStore::InitCallback& callback) { +void AttachmentStoreHandle::Init(const InitCallback& callback) { DCHECK(CalledOnValidThread()); backend_task_runner_->PostTask( FROM_HERE, base::Bind(&AttachmentStoreBackend::Init, base::Unretained(backend_.get()), callback)); } -void AttachmentStoreFrontend::Read( - const AttachmentIdList& ids, - const AttachmentStore::ReadCallback& callback) { +void AttachmentStoreHandle::Read(const AttachmentIdList& ids, + const ReadCallback& callback) { DCHECK(CalledOnValidThread()); backend_task_runner_->PostTask( FROM_HERE, base::Bind(&AttachmentStoreBackend::Read, base::Unretained(backend_.get()), ids, callback)); } -void AttachmentStoreFrontend::Write( - AttachmentStore::AttachmentReferrer referrer, - const AttachmentList& attachments, - const AttachmentStore::WriteCallback& callback) { +void AttachmentStoreHandle::Write(const AttachmentList& attachments, + const WriteCallback& callback) { DCHECK(CalledOnValidThread()); backend_task_runner_->PostTask( - FROM_HERE, base::Bind(&AttachmentStoreBackend::Write, - base::Unretained(backend_.get()), referrer, - attachments, callback)); + FROM_HERE, + base::Bind(&AttachmentStoreBackend::Write, + base::Unretained(backend_.get()), attachments, callback)); } -void AttachmentStoreFrontend::Drop( - AttachmentStore::AttachmentReferrer referrer, - const AttachmentIdList& ids, - const AttachmentStore::DropCallback& callback) { +void AttachmentStoreHandle::Drop(const AttachmentIdList& ids, + const DropCallback& callback) { DCHECK(CalledOnValidThread()); backend_task_runner_->PostTask( - FROM_HERE, - base::Bind(&AttachmentStoreBackend::Drop, - base::Unretained(backend_.get()), referrer, ids, callback)); + FROM_HERE, base::Bind(&AttachmentStoreBackend::Drop, + base::Unretained(backend_.get()), ids, callback)); } -void AttachmentStoreFrontend::ReadMetadata( - const AttachmentIdList& ids, - const AttachmentStore::ReadMetadataCallback& callback) { +void AttachmentStoreHandle::ReadMetadata(const AttachmentIdList& ids, + const ReadMetadataCallback& callback) { DCHECK(CalledOnValidThread()); backend_task_runner_->PostTask( FROM_HERE, base::Bind(&AttachmentStoreBackend::ReadMetadata, base::Unretained(backend_.get()), ids, callback)); } -void AttachmentStoreFrontend::ReadAllMetadata( - AttachmentStore::AttachmentReferrer referrer, - const AttachmentStore::ReadMetadataCallback& callback) { +void AttachmentStoreHandle::ReadAllMetadata( + const ReadMetadataCallback& callback) { DCHECK(CalledOnValidThread()); backend_task_runner_->PostTask( - FROM_HERE, - base::Bind(&AttachmentStoreBackend::ReadAllMetadata, - base::Unretained(backend_.get()), referrer, callback)); + FROM_HERE, base::Bind(&AttachmentStoreBackend::ReadAllMetadata, + base::Unretained(backend_.get()), callback)); } } // namespace syncer diff --git a/sync/internal_api/attachments/attachment_store_frontend_unittest.cc b/sync/internal_api/attachments/attachment_store_handle_unittest.cc index 696b43a..23c0f09 100644 --- a/sync/internal_api/attachments/attachment_store_frontend_unittest.cc +++ b/sync/internal_api/attachments/attachment_store_handle_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/internal_api/public/attachments/attachment_store_frontend.h" +#include "sync/internal_api/public/attachments/attachment_store_handle.h" #include "base/bind.h" #include "base/callback.h" @@ -13,7 +13,6 @@ #include "base/thread_task_runner_handle.h" #include "sync/api/attachments/attachment.h" #include "sync/api/attachments/attachment_id.h" -#include "sync/api/attachments/attachment_store_backend.h" #include "testing/gtest/include/gtest/gtest.h" namespace syncer { @@ -49,14 +48,12 @@ class MockAttachmentStore : public AttachmentStoreBackend { read_called_.Run(); } - void Write(AttachmentStore::AttachmentReferrer referrer, - const AttachmentList& attachments, + void Write(const AttachmentList& attachments, const AttachmentStore::WriteCallback& callback) override { write_called_.Run(); } - void Drop(AttachmentStore::AttachmentReferrer referrer, - const AttachmentIdList& ids, + void Drop(const AttachmentIdList& ids, const AttachmentStore::DropCallback& callback) override { drop_called_.Run(); } @@ -68,7 +65,6 @@ class MockAttachmentStore : public AttachmentStoreBackend { } void ReadAllMetadata( - AttachmentStore::AttachmentReferrer referrer, const AttachmentStore::ReadMetadataCallback& callback) override { read_all_metadata_called_.Run(); } @@ -84,9 +80,9 @@ class MockAttachmentStore : public AttachmentStoreBackend { } // namespace -class AttachmentStoreFrontendTest : public testing::Test { +class AttachmentStoreHandleTest : public testing::Test { protected: - AttachmentStoreFrontendTest() + AttachmentStoreHandleTest() : init_call_count_(0), read_call_count_(0), write_call_count_(0), @@ -97,21 +93,21 @@ class AttachmentStoreFrontendTest : public testing::Test { void SetUp() override { scoped_ptr<AttachmentStoreBackend> backend(new MockAttachmentStore( - base::Bind(&AttachmentStoreFrontendTest::InitCalled, + base::Bind(&AttachmentStoreHandleTest::InitCalled, base::Unretained(this)), - base::Bind(&AttachmentStoreFrontendTest::ReadCalled, + base::Bind(&AttachmentStoreHandleTest::ReadCalled, base::Unretained(this)), - base::Bind(&AttachmentStoreFrontendTest::WriteCalled, + base::Bind(&AttachmentStoreHandleTest::WriteCalled, base::Unretained(this)), - base::Bind(&AttachmentStoreFrontendTest::DropCalled, + base::Bind(&AttachmentStoreHandleTest::DropCalled, base::Unretained(this)), - base::Bind(&AttachmentStoreFrontendTest::ReadMetadataCalled, + base::Bind(&AttachmentStoreHandleTest::ReadMetadataCalled, base::Unretained(this)), - base::Bind(&AttachmentStoreFrontendTest::ReadAllMetadataCalled, + base::Bind(&AttachmentStoreHandleTest::ReadAllMetadataCalled, base::Unretained(this)), - base::Bind(&AttachmentStoreFrontendTest::DtorCalled, + base::Bind(&AttachmentStoreHandleTest::DtorCalled, base::Unretained(this)))); - attachment_store_frontend_ = new AttachmentStoreFrontend( + attachment_store_handle_ = new AttachmentStoreHandle( backend.Pass(), base::ThreadTaskRunnerHandle::Get()); } @@ -150,7 +146,7 @@ class AttachmentStoreFrontendTest : public testing::Test { } base::MessageLoop message_loop_; - scoped_refptr<AttachmentStoreFrontend> attachment_store_frontend_; + scoped_refptr<AttachmentStoreHandle> attachment_store_handle_; int init_call_count_; int read_call_count_; int write_call_count_; @@ -161,52 +157,49 @@ class AttachmentStoreFrontendTest : public testing::Test { }; // Test that method calls are forwarded to backend loop -TEST_F(AttachmentStoreFrontendTest, MethodsCalled) { +TEST_F(AttachmentStoreHandleTest, MethodsCalled) { AttachmentIdList ids; AttachmentList attachments; - attachment_store_frontend_->Init( - base::Bind(&AttachmentStoreFrontendTest::DoneWithResult)); + attachment_store_handle_->Init( + base::Bind(&AttachmentStoreHandleTest::DoneWithResult)); EXPECT_EQ(init_call_count_, 0); RunMessageLoop(); EXPECT_EQ(init_call_count_, 1); - attachment_store_frontend_->Read( - ids, base::Bind(&AttachmentStoreFrontendTest::ReadDone)); + attachment_store_handle_->Read( + ids, base::Bind(&AttachmentStoreHandleTest::ReadDone)); EXPECT_EQ(read_call_count_, 0); RunMessageLoop(); EXPECT_EQ(read_call_count_, 1); - attachment_store_frontend_->Write( - AttachmentStore::SYNC, attachments, - base::Bind(&AttachmentStoreFrontendTest::DoneWithResult)); + attachment_store_handle_->Write( + attachments, base::Bind(&AttachmentStoreHandleTest::DoneWithResult)); EXPECT_EQ(write_call_count_, 0); RunMessageLoop(); EXPECT_EQ(write_call_count_, 1); - attachment_store_frontend_->Drop( - AttachmentStore::SYNC, ids, - base::Bind(&AttachmentStoreFrontendTest::DoneWithResult)); + attachment_store_handle_->Drop( + ids, base::Bind(&AttachmentStoreHandleTest::DoneWithResult)); EXPECT_EQ(drop_call_count_, 0); RunMessageLoop(); EXPECT_EQ(drop_call_count_, 1); - attachment_store_frontend_->ReadMetadata( - ids, base::Bind(&AttachmentStoreFrontendTest::ReadMetadataDone)); + attachment_store_handle_->ReadMetadata( + ids, base::Bind(&AttachmentStoreHandleTest::ReadMetadataDone)); EXPECT_EQ(read_metadata_call_count_, 0); RunMessageLoop(); EXPECT_EQ(read_metadata_call_count_, 1); - attachment_store_frontend_->ReadAllMetadata( - AttachmentStore::SYNC, - base::Bind(&AttachmentStoreFrontendTest::ReadMetadataDone)); + attachment_store_handle_->ReadAllMetadata( + base::Bind(&AttachmentStoreHandleTest::ReadMetadataDone)); EXPECT_EQ(read_all_metadata_call_count_, 0); RunMessageLoop(); EXPECT_EQ(read_all_metadata_call_count_, 1); - // Releasing referehce to AttachmentStoreFrontend should result in + // Releasing referehce to AttachmentStoreHandle should result in // MockAttachmentStore being deleted on backend loop. - attachment_store_frontend_ = NULL; + attachment_store_handle_ = NULL; EXPECT_EQ(dtor_call_count_, 0); RunMessageLoop(); EXPECT_EQ(dtor_call_count_, 1); diff --git a/sync/internal_api/attachments/attachment_store_test_template.h b/sync/internal_api/attachments/attachment_store_test_template.h index 0d14ca5..290ef22 100644 --- a/sync/internal_api/attachments/attachment_store_test_template.h +++ b/sync/internal_api/attachments/attachment_store_test_template.h @@ -47,7 +47,7 @@ class AttachmentStoreTest : public testing::Test { protected: AttachmentStoreFactory attachment_store_factory; base::MessageLoop message_loop; - scoped_ptr<AttachmentStore> store; + scoped_refptr<AttachmentStore> store; AttachmentStore::Result result; scoped_ptr<AttachmentMap> attachments; scoped_ptr<AttachmentIdList> failed_attachment_ids; diff --git a/sync/internal_api/attachments/in_memory_attachment_store.cc b/sync/internal_api/attachments/in_memory_attachment_store.cc index ba21426..d2a2083 100644 --- a/sync/internal_api/attachments/in_memory_attachment_store.cc +++ b/sync/internal_api/attachments/in_memory_attachment_store.cc @@ -66,7 +66,6 @@ void InMemoryAttachmentStore::Read( } void InMemoryAttachmentStore::Write( - AttachmentStore::AttachmentReferrer referrer, const AttachmentList& attachments, const AttachmentStore::WriteCallback& callback) { DCHECK(CalledOnValidThread()); @@ -79,7 +78,6 @@ void InMemoryAttachmentStore::Write( } void InMemoryAttachmentStore::Drop( - AttachmentStore::AttachmentReferrer referrer, const AttachmentIdList& ids, const AttachmentStore::DropCallback& callback) { DCHECK(CalledOnValidThread()); @@ -117,7 +115,6 @@ void InMemoryAttachmentStore::ReadMetadata( } void InMemoryAttachmentStore::ReadAllMetadata( - AttachmentStore::AttachmentReferrer referrer, const AttachmentStore::ReadMetadataCallback& callback) { DCHECK(CalledOnValidThread()); AttachmentStore::Result result_code = AttachmentStore::SUCCESS; diff --git a/sync/internal_api/attachments/in_memory_attachment_store_unittest.cc b/sync/internal_api/attachments/in_memory_attachment_store_unittest.cc index c99be15..8b6843c 100644 --- a/sync/internal_api/attachments/in_memory_attachment_store_unittest.cc +++ b/sync/internal_api/attachments/in_memory_attachment_store_unittest.cc @@ -13,7 +13,7 @@ class InMemoryAttachmentStoreFactory { InMemoryAttachmentStoreFactory() {} ~InMemoryAttachmentStoreFactory() {} - scoped_ptr<AttachmentStore> CreateAttachmentStore() { + scoped_refptr<AttachmentStore> CreateAttachmentStore() { return AttachmentStore::CreateInMemoryStore(); } }; diff --git a/sync/internal_api/attachments/on_disk_attachment_store.cc b/sync/internal_api/attachments/on_disk_attachment_store.cc index ec6fc75..7b715e0 100644 --- a/sync/internal_api/attachments/on_disk_attachment_store.cc +++ b/sync/internal_api/attachments/on_disk_attachment_store.cc @@ -134,7 +134,6 @@ void OnDiskAttachmentStore::Read( } void OnDiskAttachmentStore::Write( - AttachmentStore::AttachmentReferrer referrer, const AttachmentList& attachments, const AttachmentStore::WriteCallback& callback) { DCHECK(CalledOnValidThread()); @@ -154,7 +153,6 @@ void OnDiskAttachmentStore::Write( } void OnDiskAttachmentStore::Drop( - AttachmentStore::AttachmentReferrer referrer, const AttachmentIdList& ids, const AttachmentStore::DropCallback& callback) { DCHECK(CalledOnValidThread()); @@ -208,7 +206,6 @@ void OnDiskAttachmentStore::ReadMetadata( } void OnDiskAttachmentStore::ReadAllMetadata( - AttachmentStore::AttachmentReferrer referrer, const AttachmentStore::ReadMetadataCallback& callback) { DCHECK(CalledOnValidThread()); AttachmentStore::Result result_code = diff --git a/sync/internal_api/attachments/on_disk_attachment_store_unittest.cc b/sync/internal_api/attachments/on_disk_attachment_store_unittest.cc index 4dcc8f8..e46f060 100644 --- a/sync/internal_api/attachments/on_disk_attachment_store_unittest.cc +++ b/sync/internal_api/attachments/on_disk_attachment_store_unittest.cc @@ -37,9 +37,9 @@ class OnDiskAttachmentStoreFactory { OnDiskAttachmentStoreFactory() {} ~OnDiskAttachmentStoreFactory() {} - scoped_ptr<AttachmentStore> CreateAttachmentStore() { + scoped_refptr<AttachmentStore> CreateAttachmentStore() { EXPECT_TRUE(temp_dir_.CreateUniqueTempDir()); - scoped_ptr<AttachmentStore> store; + scoped_refptr<AttachmentStore> store; AttachmentStore::Result result = AttachmentStore::UNSPECIFIED_ERROR; store = AttachmentStore::CreateOnDiskStore( temp_dir_.path(), base::ThreadTaskRunnerHandle::Get(), @@ -64,7 +64,7 @@ class OnDiskAttachmentStoreSpecificTest : public testing::Test { base::ScopedTempDir temp_dir_; base::FilePath db_path_; base::MessageLoop message_loop_; - scoped_ptr<AttachmentStore> store_; + scoped_refptr<AttachmentStore> store_; OnDiskAttachmentStoreSpecificTest() {} diff --git a/sync/internal_api/public/attachments/attachment_service.h b/sync/internal_api/public/attachments/attachment_service.h index da4b597..944a71f 100644 --- a/sync/internal_api/public/attachments/attachment_service.h +++ b/sync/internal_api/public/attachments/attachment_service.h @@ -52,6 +52,11 @@ class SYNC_EXPORT AttachmentService { AttachmentService(); virtual ~AttachmentService(); + // Return a pointer to the AttachmentStore owned by this object. + // + // May return NULL. + virtual AttachmentStore* GetStore() = 0; + // See SyncData::GetOrDownloadAttachments. virtual void GetOrDownloadAttachments( const AttachmentIdList& attachment_ids, diff --git a/sync/internal_api/public/attachments/attachment_service_impl.h b/sync/internal_api/public/attachments/attachment_service_impl.h index c3275bf..e35f548 100644 --- a/sync/internal_api/public/attachments/attachment_service_impl.h +++ b/sync/internal_api/public/attachments/attachment_service_impl.h @@ -26,9 +26,6 @@ class SYNC_EXPORT AttachmentServiceImpl public net::NetworkChangeNotifier::NetworkChangeObserver, public base::NonThreadSafe { public: - // |attachment_store| is required. UploadAttachments reads attachment data - // from it. Downloaded attachments will be written into it. - // // |attachment_uploader| is optional. If null, attachments will never be // uploaded to the sync server and |delegate|'s OnAttachmentUploaded will // never be invoked. @@ -50,7 +47,7 @@ class SYNC_EXPORT AttachmentServiceImpl // // |max_backoff_delay| the maxmium delay between upload attempts when backed // off. - AttachmentServiceImpl(scoped_ptr<AttachmentStore> attachment_store, + AttachmentServiceImpl(scoped_refptr<AttachmentStore> attachment_store, scoped_ptr<AttachmentUploader> attachment_uploader, scoped_ptr<AttachmentDownloader> attachment_downloader, Delegate* delegate, @@ -62,6 +59,7 @@ class SYNC_EXPORT AttachmentServiceImpl static scoped_ptr<syncer::AttachmentService> CreateForTest(); // AttachmentService implementation. + AttachmentStore* GetStore() override; void GetOrDownloadAttachments(const AttachmentIdList& attachment_ids, const GetOrDownloadCallback& callback) override; void UploadAttachments(const AttachmentIdSet& attachment_ids) override; @@ -97,7 +95,7 @@ class SYNC_EXPORT AttachmentServiceImpl scoped_ptr<AttachmentMap> attachments, scoped_ptr<AttachmentIdList> unavailable_attachment_ids); - scoped_ptr<AttachmentStore> attachment_store_; + scoped_refptr<AttachmentStore> attachment_store_; // May be null. const scoped_ptr<AttachmentUploader> attachment_uploader_; diff --git a/sync/internal_api/public/attachments/attachment_service_proxy.h b/sync/internal_api/public/attachments/attachment_service_proxy.h index ef6e287..3a96660 100644 --- a/sync/internal_api/public/attachments/attachment_service_proxy.h +++ b/sync/internal_api/public/attachments/attachment_service_proxy.h @@ -50,6 +50,10 @@ class SYNC_EXPORT AttachmentServiceProxy : public AttachmentService { ~AttachmentServiceProxy() override; + // AttachmentService implementation. + // + // GetStore always returns NULL. + AttachmentStore* GetStore() override; void GetOrDownloadAttachments(const AttachmentIdList& attachment_ids, const GetOrDownloadCallback& callback) override; void UploadAttachments(const AttachmentIdSet& attachment_ids) override; @@ -75,6 +79,7 @@ class SYNC_EXPORT AttachmentServiceProxy : public AttachmentService { Core(const base::WeakPtr<syncer::AttachmentService>& wrapped); // AttachmentService implementation. + AttachmentStore* GetStore() override; void GetOrDownloadAttachments( const AttachmentIdList& attachment_ids, const GetOrDownloadCallback& callback) override; diff --git a/sync/internal_api/public/attachments/attachment_store_frontend.h b/sync/internal_api/public/attachments/attachment_store_frontend.h deleted file mode 100644 index ec4a33f..0000000 --- a/sync/internal_api/public/attachments/attachment_store_frontend.h +++ /dev/null @@ -1,69 +0,0 @@ -// Copyright 2015 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_FRONTEND_H_ -#define SYNC_INTERNAL_API_PUBLIC_ATTACHMENTS_ATTACHMENT_STORE_FRONTEND_H_ - -#include "base/memory/ref_counted.h" -#include "base/memory/scoped_ptr.h" -#include "base/threading/non_thread_safe.h" -#include "sync/api/attachments/attachment_store.h" - -namespace base { -class SequencedTaskRunner; -} // namespace base - -namespace syncer { - -class AttachmentStoreBackend; - -// AttachmentStoreFrontend is helper to post AttachmentStore calls to backend on -// different thread. Backend is expected to know on which thread to post -// callbacks with results. -// AttachmentStoreFrontend takes ownership of backend. Backend is deleted on -// backend thread. -// AttachmentStoreFrontend is not thread safe, it should only be accessed from -// model thread. -// Method signatures of AttachmentStoreFrontend match exactly methods of -// AttachmentStoreBackend. -class SYNC_EXPORT AttachmentStoreFrontend - : public base::RefCounted<AttachmentStoreFrontend>, - public base::NonThreadSafe { - public: - AttachmentStoreFrontend( - scoped_ptr<AttachmentStoreBackend> backend, - const scoped_refptr<base::SequencedTaskRunner>& backend_task_runner); - - void Init(const AttachmentStore::InitCallback& callback); - - void Read(const AttachmentIdList& ids, - const AttachmentStore::ReadCallback& callback); - - void Write(AttachmentStore::AttachmentReferrer referrer, - const AttachmentList& attachments, - const AttachmentStore::WriteCallback& callback); - - void Drop(AttachmentStore::AttachmentReferrer referrer, - const AttachmentIdList& ids, - const AttachmentStore::DropCallback& callback); - - void ReadMetadata(const AttachmentIdList& ids, - const AttachmentStore::ReadMetadataCallback& callback); - - void ReadAllMetadata(AttachmentStore::AttachmentReferrer referrer, - const AttachmentStore::ReadMetadataCallback& callback); - - private: - friend class base::RefCounted<AttachmentStoreFrontend>; - virtual ~AttachmentStoreFrontend(); - - scoped_ptr<AttachmentStoreBackend> backend_; - scoped_refptr<base::SequencedTaskRunner> backend_task_runner_; - - DISALLOW_COPY_AND_ASSIGN(AttachmentStoreFrontend); -}; - -} // namespace syncer - -#endif // SYNC_INTERNAL_API_PUBLIC_ATTACHMENTS_ATTACHMENT_STORE_FRONTEND_H_ 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..bbf7910 --- /dev/null +++ b/sync/internal_api/public/attachments/attachment_store_handle.h @@ -0,0 +1,68 @@ +// 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<AttachmentStoreBackend> backend, + const scoped_refptr<base::SequencedTaskRunner>& backend_task_runner); + + // AttachmentStore implementation. + void Init(const InitCallback& callback) override; + void Read(const AttachmentIdList& id, const ReadCallback& callback) override; + void Write(const AttachmentList& attachments, + const WriteCallback& callback) override; + void Drop(const AttachmentIdList& id, const DropCallback& callback) override; + void ReadMetadata(const AttachmentIdList& ids, + const ReadMetadataCallback& callback) override; + void ReadAllMetadata(const ReadMetadataCallback& callback) override; + + private: + ~AttachmentStoreHandle() override; + + // 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<AttachmentStoreBackend> 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 index 5f85992..9fd0b85 100644 --- a/sync/internal_api/public/attachments/in_memory_attachment_store.h +++ b/sync/internal_api/public/attachments/in_memory_attachment_store.h @@ -10,7 +10,6 @@ #include "sync/api/attachments/attachment.h" #include "sync/api/attachments/attachment_id.h" #include "sync/api/attachments/attachment_store.h" -#include "sync/api/attachments/attachment_store_backend.h" #include "sync/base/sync_export.h" namespace base { @@ -33,17 +32,14 @@ class SYNC_EXPORT InMemoryAttachmentStore : public AttachmentStoreBackend, void Init(const AttachmentStore::InitCallback& callback) override; void Read(const AttachmentIdList& ids, const AttachmentStore::ReadCallback& callback) override; - void Write(AttachmentStore::AttachmentReferrer referrer, - const AttachmentList& attachments, + void Write(const AttachmentList& attachments, const AttachmentStore::WriteCallback& callback) override; - void Drop(AttachmentStore::AttachmentReferrer referrer, - const AttachmentIdList& ids, + void Drop(const AttachmentIdList& ids, const AttachmentStore::DropCallback& callback) override; void ReadMetadata( const AttachmentIdList& ids, const AttachmentStore::ReadMetadataCallback& callback) override; void ReadAllMetadata( - AttachmentStore::AttachmentReferrer referrer, const AttachmentStore::ReadMetadataCallback& callback) override; private: diff --git a/sync/internal_api/public/attachments/on_disk_attachment_store.h b/sync/internal_api/public/attachments/on_disk_attachment_store.h index 62cec5b..13ccf29 100644 --- a/sync/internal_api/public/attachments/on_disk_attachment_store.h +++ b/sync/internal_api/public/attachments/on_disk_attachment_store.h @@ -11,7 +11,6 @@ #include "sync/api/attachments/attachment.h" #include "sync/api/attachments/attachment_id.h" #include "sync/api/attachments/attachment_store.h" -#include "sync/api/attachments/attachment_store_backend.h" #include "sync/base/sync_export.h" namespace attachment_store_pb { @@ -43,17 +42,14 @@ class SYNC_EXPORT OnDiskAttachmentStore : public AttachmentStoreBackend, void Init(const AttachmentStore::InitCallback& callback) override; void Read(const AttachmentIdList& ids, const AttachmentStore::ReadCallback& callback) override; - void Write(AttachmentStore::AttachmentReferrer referrer, - const AttachmentList& attachments, + void Write(const AttachmentList& attachments, const AttachmentStore::WriteCallback& callback) override; - void Drop(AttachmentStore::AttachmentReferrer referrer, - const AttachmentIdList& ids, + void Drop(const AttachmentIdList& ids, const AttachmentStore::DropCallback& callback) override; void ReadMetadata( const AttachmentIdList& ids, const AttachmentStore::ReadMetadataCallback& callback) override; void ReadAllMetadata( - AttachmentStore::AttachmentReferrer referrer, const AttachmentStore::ReadMetadataCallback& callback) override; private: diff --git a/sync/sync.gyp b/sync/sync.gyp index 49a792a..1805955 100644 --- a/sync/sync.gyp +++ b/sync/sync.gyp @@ -67,8 +67,6 @@ 'api/attachments/attachment_metadata.h', 'api/attachments/attachment_store.cc', 'api/attachments/attachment_store.h', - 'api/attachments/attachment_store_backend.cc', - 'api/attachments/attachment_store_backend.h', 'api/string_ordinal.h', 'api/sync_change.cc', 'api/sync_change.h', @@ -169,7 +167,7 @@ '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_frontend.cc', + 'internal_api/attachments/attachment_store_handle.cc', 'internal_api/attachments/attachment_uploader.cc', 'internal_api/attachments/attachment_uploader_impl.cc', 'internal_api/attachments/attachment_util.cc', @@ -210,7 +208,7 @@ '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_frontend.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/attachment_util.h', diff --git a/sync/sync_tests.gypi b/sync/sync_tests.gypi index 8f2b35b..9bb8aa8 100644 --- a/sync/sync_tests.gypi +++ b/sync/sync_tests.gypi @@ -282,7 +282,7 @@ '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_frontend_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', |