From 3c9575830013f5e70842c52c5757122e83b4dea8 Mon Sep 17 00:00:00 2001 From: pavely Date: Thu, 10 Sep 2015 15:19:28 -0700 Subject: [PATCH] [Sync] Introduce ModelTypeStore interface In this change: - Add empty ModelTypeStore interface - Pass WeakPtr to store into ModelTypeProcessorImpl::ctor. BUG=517663 R=stanisc@chromium.org Review URL: https://codereview.chromium.org/1311363009 Cr-Commit-Position: refs/heads/master@{#348255} --- ..._blocking_data_type_controller_unittest.cc | 3 ++- sync/BUILD.gn | 2 ++ sync/api/model_type_store.cc | 13 ++++++++++ sync/api/model_type_store.h | 20 ++++++++++++++++ sync/engine/model_type_processor_impl.cc | 5 +++- sync/engine/model_type_processor_impl.h | 9 ++++++- .../model_type_processor_impl_unittest.cc | 4 +++- .../sync_context_proxy_impl_unittest.cc | 12 ++++++---- sync/sessions/model_type_registry_unittest.cc | 24 +++++++++++-------- sync/sync.gyp | 2 ++ 10 files changed, 76 insertions(+), 18 deletions(-) create mode 100644 sync/api/model_type_store.cc create mode 100644 sync/api/model_type_store.h diff --git a/components/sync_driver/non_blocking_data_type_controller_unittest.cc b/components/sync_driver/non_blocking_data_type_controller_unittest.cc index c0e0ecb198cd47..ef31daa1425bc7 100644 --- a/components/sync_driver/non_blocking_data_type_controller_unittest.cc +++ b/components/sync_driver/non_blocking_data_type_controller_unittest.cc @@ -120,7 +120,8 @@ class MockSyncContextProxy : public syncer_v2::SyncContextProxy { class NonBlockingDataTypeControllerTest : public testing::Test { public: NonBlockingDataTypeControllerTest() - : type_processor_(syncer::DICTIONARY), + : type_processor_(syncer::DICTIONARY, + base::WeakPtr()), model_thread_(new base::TestSimpleTaskRunner()), sync_thread_(new base::TestSimpleTaskRunner()), controller_(syncer::DICTIONARY, true), diff --git a/sync/BUILD.gn b/sync/BUILD.gn index 2bc56fdda7a3fc..34b57b7f97f6dc 100644 --- a/sync/BUILD.gn +++ b/sync/BUILD.gn @@ -25,6 +25,8 @@ source_set("sync_core") { "api/attachments/attachment_store.h", "api/attachments/attachment_store_backend.cc", "api/attachments/attachment_store_backend.h", + "api/model_type_store.cc", + "api/model_type_store.h", "api/string_ordinal.h", "api/sync_change.cc", "api/sync_change.h", diff --git a/sync/api/model_type_store.cc b/sync/api/model_type_store.cc new file mode 100644 index 00000000000000..2555ddc87b3487 --- /dev/null +++ b/sync/api/model_type_store.cc @@ -0,0 +1,13 @@ +// 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/model_type_store.h" + +namespace syncer_v2 { + +ModelTypeStore::ModelTypeStore() {} + +ModelTypeStore::~ModelTypeStore() {} + +} // namespace sync_v2 diff --git a/sync/api/model_type_store.h b/sync/api/model_type_store.h new file mode 100644 index 00000000000000..817238e2ea39a7 --- /dev/null +++ b/sync/api/model_type_store.h @@ -0,0 +1,20 @@ +// 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_MODEL_TYPE_STORE_H_ +#define SYNC_API_MODEL_TYPE_STORE_H_ + +namespace syncer_v2 { + +// Interface for store used by ModelTypeProcessor for persisting sync related +// data (entity state and data type state). +class ModelTypeStore { + public: + ModelTypeStore(); + virtual ~ModelTypeStore(); +}; + +} // namespace syncer_v2 + +#endif // SYNC_API_MODEL_TYPE_STORE_H_ diff --git a/sync/engine/model_type_processor_impl.cc b/sync/engine/model_type_processor_impl.cc index 3afd428d1dae76..55aa73b25cb3a8 100644 --- a/sync/engine/model_type_processor_impl.cc +++ b/sync/engine/model_type_processor_impl.cc @@ -13,10 +13,13 @@ namespace syncer_v2 { -ModelTypeProcessorImpl::ModelTypeProcessorImpl(syncer::ModelType type) +ModelTypeProcessorImpl::ModelTypeProcessorImpl( + syncer::ModelType type, + base::WeakPtr store) : type_(type), is_preferred_(false), is_connected_(false), + store_(store), weak_ptr_factory_for_ui_(this), weak_ptr_factory_for_sync_(this) {} diff --git a/sync/engine/model_type_processor_impl.h b/sync/engine/model_type_processor_impl.h index fb626e865566f6..04a31419c9e13c 100644 --- a/sync/engine/model_type_processor_impl.h +++ b/sync/engine/model_type_processor_impl.h @@ -18,6 +18,7 @@ namespace syncer_v2 { class CommitQueue; class ModelTypeEntity; +class ModelTypeStore; class SyncContextProxy; // A sync component embedded on the synced type's thread that helps to handle @@ -25,7 +26,8 @@ class SyncContextProxy; class SYNC_EXPORT_PRIVATE ModelTypeProcessorImpl : public ModelTypeProcessor, base::NonThreadSafe { public: - ModelTypeProcessorImpl(syncer::ModelType type); + ModelTypeProcessorImpl(syncer::ModelType type, + base::WeakPtr store); ~ModelTypeProcessorImpl() override; // Returns true if this object believes that sync is preferred for this type. @@ -141,6 +143,11 @@ class SYNC_EXPORT_PRIVATE ModelTypeProcessorImpl : public ModelTypeProcessor, // them across restarts, and keep them in sync with our progress markers. UpdateMap pending_updates_map_; + // Store is supplied by model type implementation. ModelTypeProcessorImpl + // uses store for persisting sync related data (entity state and data type + // state). + base::WeakPtr store_; + // We use two different WeakPtrFactories because we want the pointers they // issue to have different lifetimes. When asked to disconnect from the sync // thread, we want to make sure that no tasks generated as part of the diff --git a/sync/engine/model_type_processor_impl_unittest.cc b/sync/engine/model_type_processor_impl_unittest.cc index c1b6b88cdf0563..8f57d6993c487d 100644 --- a/sync/engine/model_type_processor_impl_unittest.cc +++ b/sync/engine/model_type_processor_impl_unittest.cc @@ -135,7 +135,9 @@ ModelTypeProcessorImplTest::ModelTypeProcessorImplTest() : mock_queue_(new MockCommitQueue()), injectable_sync_context_proxy_( new InjectableSyncContextProxy(mock_queue_)), - type_processor_(new ModelTypeProcessorImpl(kModelType)) {} + type_processor_( + new ModelTypeProcessorImpl(kModelType, + base::WeakPtr())) {} ModelTypeProcessorImplTest::~ModelTypeProcessorImplTest() { } diff --git a/sync/internal_api/sync_context_proxy_impl_unittest.cc b/sync/internal_api/sync_context_proxy_impl_unittest.cc index 00fe80c1543aef..f4981d7d386e66 100644 --- a/sync/internal_api/sync_context_proxy_impl_unittest.cc +++ b/sync/internal_api/sync_context_proxy_impl_unittest.cc @@ -58,7 +58,8 @@ class SyncContextProxyImplTest : public ::testing::Test { // Try to connect a type to a SyncContext that has already shut down. TEST_F(SyncContextProxyImplTest, FailToConnect1) { - ModelTypeProcessorImpl themes_sync_proxy(syncer::THEMES); + ModelTypeProcessorImpl themes_sync_proxy(syncer::THEMES, + base::WeakPtr()); DisableSync(); themes_sync_proxy.Enable(GetProxy()); @@ -69,7 +70,8 @@ TEST_F(SyncContextProxyImplTest, FailToConnect1) { // Try to connect a type to a SyncContext as it shuts down. TEST_F(SyncContextProxyImplTest, FailToConnect2) { - ModelTypeProcessorImpl themes_sync_proxy(syncer::THEMES); + ModelTypeProcessorImpl themes_sync_proxy(syncer::THEMES, + base::WeakPtr()); themes_sync_proxy.Enable(GetProxy()); DisableSync(); @@ -81,7 +83,8 @@ TEST_F(SyncContextProxyImplTest, FailToConnect2) { // Tests the case where the type's sync proxy shuts down first. TEST_F(SyncContextProxyImplTest, TypeDisconnectsFirst) { scoped_ptr themes_sync_proxy( - new ModelTypeProcessorImpl(syncer::THEMES)); + new ModelTypeProcessorImpl(syncer::THEMES, + base::WeakPtr())); themes_sync_proxy->Enable(GetProxy()); base::RunLoop run_loop_; @@ -94,7 +97,8 @@ TEST_F(SyncContextProxyImplTest, TypeDisconnectsFirst) { // Tests the case where the sync thread shuts down first. TEST_F(SyncContextProxyImplTest, SyncDisconnectsFirst) { scoped_ptr themes_sync_proxy( - new ModelTypeProcessorImpl(syncer::THEMES)); + new ModelTypeProcessorImpl(syncer::THEMES, + base::WeakPtr())); themes_sync_proxy->Enable(GetProxy()); base::RunLoop run_loop_; diff --git a/sync/sessions/model_type_registry_unittest.cc b/sync/sessions/model_type_registry_unittest.cc index 648a6a8f6d176d..9ab1d29774ba35 100644 --- a/sync/sessions/model_type_registry_unittest.cc +++ b/sync/sessions/model_type_registry_unittest.cc @@ -17,8 +17,6 @@ namespace syncer { -using syncer_v2::ModelTypeProcessorImpl; - class ModelTypeRegistryTest : public ::testing::Test { public: ModelTypeRegistryTest(); @@ -145,8 +143,10 @@ TEST_F(ModelTypeRegistryTest, SetEnabledDirectoryTypes_OffAndOn) { } TEST_F(ModelTypeRegistryTest, NonBlockingTypes) { - ModelTypeProcessorImpl themes_sync_proxy(syncer::THEMES); - ModelTypeProcessorImpl sessions_sync_proxy(syncer::SESSIONS); + syncer_v2::ModelTypeProcessorImpl themes_sync_proxy( + syncer::THEMES, base::WeakPtr()); + syncer_v2::ModelTypeProcessorImpl sessions_sync_proxy( + syncer::SESSIONS, base::WeakPtr()); scoped_refptr task_runner = new base::DeferredSequencedTaskRunner( base::ThreadTaskRunnerHandle::Get()); @@ -176,8 +176,10 @@ TEST_F(ModelTypeRegistryTest, NonBlockingTypes) { } TEST_F(ModelTypeRegistryTest, NonBlockingTypesWithDirectoryTypes) { - ModelTypeProcessorImpl themes_sync_proxy(syncer::THEMES); - ModelTypeProcessorImpl sessions_sync_proxy(syncer::SESSIONS); + syncer_v2::ModelTypeProcessorImpl themes_sync_proxy( + syncer::THEMES, base::WeakPtr()); + syncer_v2::ModelTypeProcessorImpl sessions_sync_proxy( + syncer::SESSIONS, base::WeakPtr()); scoped_refptr task_runner = new base::DeferredSequencedTaskRunner( base::ThreadTaskRunnerHandle::Get()); @@ -224,10 +226,12 @@ TEST_F(ModelTypeRegistryTest, NonBlockingTypesWithDirectoryTypes) { } TEST_F(ModelTypeRegistryTest, DeletionOrdering) { - scoped_ptr themes_sync_proxy( - new ModelTypeProcessorImpl(syncer::THEMES)); - scoped_ptr sessions_sync_proxy( - new ModelTypeProcessorImpl(syncer::SESSIONS)); + scoped_ptr themes_sync_proxy( + new syncer_v2::ModelTypeProcessorImpl( + syncer::THEMES, base::WeakPtr())); + scoped_ptr sessions_sync_proxy( + new syncer_v2::ModelTypeProcessorImpl( + syncer::SESSIONS, base::WeakPtr())); scoped_refptr task_runner = new base::DeferredSequencedTaskRunner( base::ThreadTaskRunnerHandle::Get()); diff --git a/sync/sync.gyp b/sync/sync.gyp index 37eb37546f26b4..177c3329b06608 100644 --- a/sync/sync.gyp +++ b/sync/sync.gyp @@ -76,6 +76,8 @@ 'api/attachments/attachment_store.h', 'api/attachments/attachment_store_backend.cc', 'api/attachments/attachment_store_backend.h', + 'api/model_type_store.cc', + 'api/model_type_store.h', 'api/string_ordinal.h', 'api/sync_change.cc', 'api/sync_change.h',