diff options
author | stanisc <stanisc@chromium.org> | 2015-12-03 13:38:40 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-12-03 21:40:19 +0000 |
commit | c490a4d2c3b423494f54d169c538d641af4cca64 (patch) | |
tree | c2e9ff751655c32fc52781c997f40bd2ca9d09b9 /sync | |
parent | b357b25045f5f8b8080fb26489287fe6af55a7b6 (diff) | |
download | chromium_src-c490a4d2c3b423494f54d169c538d641af4cca64.zip chromium_src-c490a4d2c3b423494f54d169c538d641af4cca64.tar.gz chromium_src-c490a4d2c3b423494f54d169c538d641af4cca64.tar.bz2 |
USS: Introduce dependency of SharedModelTypeProcessor on ModelTypeService
BUG=561821
Review URL: https://codereview.chromium.org/1486363002
Cr-Commit-Position: refs/heads/master@{#363066}
Diffstat (limited to 'sync')
-rw-r--r-- | sync/BUILD.gn | 2 | ||||
-rw-r--r-- | sync/api/model_type_service.h | 5 | ||||
-rw-r--r-- | sync/internal_api/public/shared_model_type_processor.h | 13 | ||||
-rw-r--r-- | sync/internal_api/public/test/fake_model_type_service.h | 43 | ||||
-rw-r--r-- | sync/internal_api/shared_model_type_processor.cc | 15 | ||||
-rw-r--r-- | sync/internal_api/shared_model_type_processor_unittest.cc | 17 | ||||
-rw-r--r-- | sync/internal_api/sync_context_proxy_impl_unittest.cc | 6 | ||||
-rw-r--r-- | sync/internal_api/test/fake_model_type_service.cc | 41 | ||||
-rw-r--r-- | sync/sessions/model_type_registry_unittest.cc | 20 | ||||
-rw-r--r-- | sync/sync_tests.gypi | 2 |
10 files changed, 128 insertions, 36 deletions
diff --git a/sync/BUILD.gn b/sync/BUILD.gn index edeaf34dd..e18ff18 100644 --- a/sync/BUILD.gn +++ b/sync/BUILD.gn @@ -554,12 +554,14 @@ static_library("test_support_sync_core") { static_library("test_support_sync_internal_api") { testonly = true sources = [ + "internal_api/public/test/fake_model_type_service.h", "internal_api/public/test/fake_sync_manager.h", "internal_api/public/test/null_sync_context_proxy.h", "internal_api/public/test/sync_manager_factory_for_profile_sync_test.h", "internal_api/public/test/test_entry_factory.h", "internal_api/public/test/test_internal_components_factory.h", "internal_api/public/test/test_user_share.h", + "internal_api/test/fake_model_type_service.cc", "internal_api/test/fake_sync_manager.cc", "internal_api/test/null_sync_context_proxy.cc", "internal_api/test/sync_manager_factory_for_profile_sync_test.cc", diff --git a/sync/api/model_type_service.h b/sync/api/model_type_service.h index 6ae8bd3..f7ad9cd 100644 --- a/sync/api/model_type_service.h +++ b/sync/api/model_type_service.h @@ -13,12 +13,9 @@ #include "sync/api/entity_change.h" #include "sync/api/entity_data.h" #include "sync/api/model_type_change_processor.h" +#include "sync/api/sync_error.h" #include "sync/base/sync_export.h" -namespace syncer { -class SyncError; -} // namespace syncer - namespace syncer_v2 { class DataBatch; diff --git a/sync/internal_api/public/shared_model_type_processor.h b/sync/internal_api/public/shared_model_type_processor.h index a7d8f0f..00e914a 100644 --- a/sync/internal_api/public/shared_model_type_processor.h +++ b/sync/internal_api/public/shared_model_type_processor.h @@ -12,6 +12,7 @@ #include "base/memory/weak_ptr.h" #include "base/threading/non_thread_safe.h" #include "sync/api/model_type_change_processor.h" +#include "sync/api/model_type_service.h" #include "sync/api/sync_error.h" #include "sync/base/sync_export.h" #include "sync/internal_api/public/base/model_type.h" @@ -23,7 +24,6 @@ namespace syncer_v2 { struct ActivationContext; class CommitQueue; class ModelTypeEntity; -class ModelTypeStore; // A sync component embedded on the synced type's thread that helps to handle // communication between sync and model type threads. @@ -32,8 +32,7 @@ class SYNC_EXPORT_PRIVATE SharedModelTypeProcessor public ModelTypeChangeProcessor, base::NonThreadSafe { public: - SharedModelTypeProcessor(syncer::ModelType type, - base::WeakPtr<ModelTypeStore> store); + SharedModelTypeProcessor(syncer::ModelType type, ModelTypeService* service); ~SharedModelTypeProcessor() override; typedef base::Callback<void(syncer::SyncError, scoped_ptr<ActivationContext>)> @@ -134,10 +133,10 @@ class SYNC_EXPORT_PRIVATE SharedModelTypeProcessor // them across restarts, and keep them in sync with our progress markers. UpdateMap pending_updates_map_; - // Store is supplied by model type implementation. SharedModelTypeProcessor - // uses store for persisting sync related data (entity state and data type - // state). - base::WeakPtr<ModelTypeStore> store_; + // ModelTypeService linked to this processor. + // The service owns this processor instance so the pointer should never + // become invalid. + ModelTypeService* const service_; // We use two different WeakPtrFactories because we want the pointers they // issue to have different lifetimes. When asked to disconnect from the sync diff --git a/sync/internal_api/public/test/fake_model_type_service.h b/sync/internal_api/public/test/fake_model_type_service.h new file mode 100644 index 0000000..b64ed5d --- /dev/null +++ b/sync/internal_api/public/test/fake_model_type_service.h @@ -0,0 +1,43 @@ +// 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_TEST_FAKE_MODEL_TYPE_SERVICE_H_ +#define SYNC_INTERNAL_API_PUBLIC_TEST_FAKE_MODEL_TYPE_SERVICE_H_ + +#include "sync/api/data_batch.h" +#include "sync/api/metadata_batch.h" +#include "sync/api/metadata_change_list.h" +#include "sync/api/model_type_service.h" + +namespace syncer_v2 { + +// A non-functional implementation of ModelTypeService for +// testing purposes. +class FakeModelTypeService : public ModelTypeService { + public: + FakeModelTypeService(); + ~FakeModelTypeService() override; + + scoped_ptr<MetadataChangeList> CreateMetadataChangeList() override; + + syncer::SyncError MergeSyncData( + scoped_ptr<MetadataChangeList> metadata_change_list, + EntityDataList entity_data_list) override; + + syncer::SyncError ApplySyncChanges( + scoped_ptr<MetadataChangeList> metadata_change_list, + EntityChangeList entity_changes) override; + + void LoadMetadata(MetadataCallback callback) override; + + void GetData(ClientKeyList client_keys, DataCallback callback) override; + + void GetAllData(DataCallback callback) override; + + std::string GetClientTag(const EntityData& entity_data) override; +}; + +} // namespace syncer_v2 + +#endif // SYNC_INTERNAL_API_PUBLIC_TEST_FAKE_MODEL_TYPE_SERVICE_H_ diff --git a/sync/internal_api/shared_model_type_processor.cc b/sync/internal_api/shared_model_type_processor.cc index db511df..97cf8e5 100644 --- a/sync/internal_api/shared_model_type_processor.cc +++ b/sync/internal_api/shared_model_type_processor.cc @@ -69,15 +69,16 @@ void ModelTypeProcessorProxy::OnUpdateReceived( } // namespace -SharedModelTypeProcessor::SharedModelTypeProcessor( - syncer::ModelType type, - base::WeakPtr<ModelTypeStore> store) +SharedModelTypeProcessor::SharedModelTypeProcessor(syncer::ModelType type, + ModelTypeService* service) : type_(type), is_enabled_(false), is_connected_(false), - store_(store), + service_(service), weak_ptr_factory_for_ui_(this), - weak_ptr_factory_for_sync_(this) {} + weak_ptr_factory_for_sync_(this) { + DCHECK(service); +} SharedModelTypeProcessor::~SharedModelTypeProcessor() {} @@ -272,8 +273,8 @@ void SharedModelTypeProcessor::OnUpdateReceived( ModelTypeEntity* entity = nullptr; EntityMap::const_iterator it = entities_.find(client_tag_hash); if (it == entities_.end()) { - // TODO(stanisc): crbug/561821: Get client_tag from the service. - std::string client_tag = client_tag_hash; + // Let the service define |client_tag| based on the entity data. + std::string client_tag = service_->GetClientTag(data); scoped_ptr<ModelTypeEntity> scoped_entity = ModelTypeEntity::CreateNew( client_tag, client_tag_hash, data.id, data.creation_time); diff --git a/sync/internal_api/shared_model_type_processor_unittest.cc b/sync/internal_api/shared_model_type_processor_unittest.cc index 2cc9998..6bbed17 100644 --- a/sync/internal_api/shared_model_type_processor_unittest.cc +++ b/sync/internal_api/shared_model_type_processor_unittest.cc @@ -11,6 +11,7 @@ #include "sync/internal_api/public/activation_context.h" #include "sync/internal_api/public/base/model_type.h" #include "sync/internal_api/public/non_blocking_sync_common.h" +#include "sync/internal_api/public/test/fake_model_type_service.h" #include "sync/protocol/sync.pb.h" #include "sync/syncable/syncable_util.h" #include "sync/test/engine/mock_commit_queue.h" @@ -41,7 +42,8 @@ static const syncer::ModelType kModelType = syncer::PREFERENCES; // - Writes to permanent storage. (TODO) // - Callbacks into the model. (TODO) // - Requests to the sync thread. Tested with MockCommitQueue. -class SharedModelTypeProcessorTest : public ::testing::Test { +class SharedModelTypeProcessorTest : public ::testing::Test, + public FakeModelTypeService { public: SharedModelTypeProcessorTest(); ~SharedModelTypeProcessorTest() override; @@ -129,6 +131,9 @@ class SharedModelTypeProcessorTest : public ::testing::Test { void StartDone(syncer::SyncError error, scoped_ptr<ActivationContext> context); + // FakeModelTypeService overrides. + std::string GetClientTag(const EntityData& entity_data) override; + // This sets ThreadTaskRunnerHandle on the current thread, which the type // processor will pick up as the sync task runner. base::MessageLoop sync_loop_; @@ -145,9 +150,7 @@ class SharedModelTypeProcessorTest : public ::testing::Test { SharedModelTypeProcessorTest::SharedModelTypeProcessorTest() : mock_queue_(new MockCommitQueue()), mock_queue_ptr_(mock_queue_), - type_processor_( - new SharedModelTypeProcessor(kModelType, - base::WeakPtr<ModelTypeStore>())) {} + type_processor_(new SharedModelTypeProcessor(kModelType, this)) {} SharedModelTypeProcessorTest::~SharedModelTypeProcessorTest() {} @@ -338,6 +341,12 @@ SharedModelTypeProcessorTest::GenerateEncryptedSpecifics( return specifics; } +std::string SharedModelTypeProcessorTest::GetClientTag( + const EntityData& entity_data) { + // The tag is the preference name - see GenerateSpecifics. + return entity_data.specifics.preference().name(); +} + size_t SharedModelTypeProcessorTest::GetNumCommitRequestLists() { return mock_queue_->GetNumCommitRequestLists(); } diff --git a/sync/internal_api/sync_context_proxy_impl_unittest.cc b/sync/internal_api/sync_context_proxy_impl_unittest.cc index 880bb0b..171d9b2 100644 --- a/sync/internal_api/sync_context_proxy_impl_unittest.cc +++ b/sync/internal_api/sync_context_proxy_impl_unittest.cc @@ -11,6 +11,7 @@ #include "sync/internal_api/public/base/model_type.h" #include "sync/internal_api/public/shared_model_type_processor.h" #include "sync/internal_api/public/sync_context.h" +#include "sync/internal_api/public/test/fake_model_type_service.h" #include "sync/internal_api/sync_context_proxy_impl.h" #include "sync/sessions/model_type_registry.h" #include "sync/test/engine/mock_nudge_handler.h" @@ -19,7 +20,7 @@ namespace syncer_v2 { -class SyncContextProxyImplTest : public ::testing::Test { +class SyncContextProxyImplTest : public ::testing::Test, FakeModelTypeService { public: SyncContextProxyImplTest() : sync_task_runner_(base::ThreadTaskRunnerHandle::Get()), @@ -54,8 +55,7 @@ class SyncContextProxyImplTest : public ::testing::Test { } scoped_ptr<SharedModelTypeProcessor> CreateModelTypeProcessor() { - return make_scoped_ptr(new SharedModelTypeProcessor( - syncer::THEMES, base::WeakPtr<ModelTypeStore>())); + return make_scoped_ptr(new SharedModelTypeProcessor(syncer::THEMES, this)); } private: diff --git a/sync/internal_api/test/fake_model_type_service.cc b/sync/internal_api/test/fake_model_type_service.cc new file mode 100644 index 0000000..d31388f --- /dev/null +++ b/sync/internal_api/test/fake_model_type_service.cc @@ -0,0 +1,41 @@ +// 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/internal_api/public/test/fake_model_type_service.h" + +namespace syncer_v2 { + +FakeModelTypeService::FakeModelTypeService() {} + +FakeModelTypeService::~FakeModelTypeService() {} + +scoped_ptr<MetadataChangeList> +FakeModelTypeService::CreateMetadataChangeList() { + return scoped_ptr<MetadataChangeList>(); +} + +syncer::SyncError FakeModelTypeService::MergeSyncData( + scoped_ptr<MetadataChangeList> metadata_change_list, + EntityDataList entity_data_list) { + return syncer::SyncError(); +} + +syncer::SyncError FakeModelTypeService::ApplySyncChanges( + scoped_ptr<MetadataChangeList> metadata_change_list, + EntityChangeList entity_changes) { + return syncer::SyncError(); +} + +void FakeModelTypeService::LoadMetadata(MetadataCallback callback) {} + +void FakeModelTypeService::GetData(ClientKeyList client_keys, + DataCallback callback) {} + +void FakeModelTypeService::GetAllData(DataCallback callback) {} + +std::string FakeModelTypeService::GetClientTag(const EntityData& entity_data) { + return std::string(); +} + +} // namespace syncer_v2 diff --git a/sync/sessions/model_type_registry_unittest.cc b/sync/sessions/model_type_registry_unittest.cc index 6db266a..30ffbff 100644 --- a/sync/sessions/model_type_registry_unittest.cc +++ b/sync/sessions/model_type_registry_unittest.cc @@ -9,6 +9,7 @@ #include "sync/internal_api/public/activation_context.h" #include "sync/internal_api/public/base/model_type.h" #include "sync/internal_api/public/shared_model_type_processor.h" +#include "sync/internal_api/public/test/fake_model_type_service.h" #include "sync/sessions/model_type_registry.h" #include "sync/test/engine/fake_model_worker.h" #include "sync/test/engine/mock_nudge_handler.h" @@ -17,17 +18,8 @@ namespace syncer { -namespace { - -scoped_ptr<syncer_v2::SharedModelTypeProcessor> MakeModelTypeProcessor( - ModelType type) { - return make_scoped_ptr(new syncer_v2::SharedModelTypeProcessor( - type, base::WeakPtr<syncer_v2::ModelTypeStore>())); -} - -} // namespace - -class ModelTypeRegistryTest : public ::testing::Test { +class ModelTypeRegistryTest : public ::testing::Test, + syncer_v2::FakeModelTypeService { public: ModelTypeRegistryTest(); void SetUp() override; @@ -54,6 +46,12 @@ class ModelTypeRegistryTest : public ::testing::Test { return context.Pass(); } + protected: + scoped_ptr<syncer_v2::SharedModelTypeProcessor> MakeModelTypeProcessor( + ModelType type) { + return make_scoped_ptr(new syncer_v2::SharedModelTypeProcessor(type, this)); + } + private: syncable::Directory* directory(); diff --git a/sync/sync_tests.gypi b/sync/sync_tests.gypi index bc0c307..940102d 100644 --- a/sync/sync_tests.gypi +++ b/sync/sync_tests.gypi @@ -183,12 +183,14 @@ 'test_support_sync_core', ], 'sources': [ + 'internal_api/public/test/fake_model_type_service.h', 'internal_api/public/test/fake_sync_manager.h', 'internal_api/public/test/null_sync_context_proxy.h', 'internal_api/public/test/sync_manager_factory_for_profile_sync_test.h', 'internal_api/public/test/test_entry_factory.h', 'internal_api/public/test/test_internal_components_factory.h', 'internal_api/public/test/test_user_share.h', + 'internal_api/test/fake_model_type_service.cc', 'internal_api/test/fake_sync_manager.cc', 'internal_api/test/null_sync_context_proxy.cc', 'internal_api/test/sync_manager_factory_for_profile_sync_test.cc', |