From e9d91ff8ae5d3c6412db8bc81aefdf62bb7f5200 Mon Sep 17 00:00:00 2001 From: "haitaol@chromium.org" Date: Fri, 31 May 2013 21:42:39 +0000 Subject: Prioritize configuration of data types in data type manager. Download and associate high-priority types (control types, priority preference and managed users for now) before other types. BUG=236456 Review URL: https://chromiumcodereview.appspot.com/15067016 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@203486 0039d316-1c4b-4281-b951-d872f2087c98 --- .../sync/glue/backend_data_type_configurer.h | 10 +- chrome/browser/sync/glue/data_type_manager_impl.cc | 255 +++++++++++++++------ chrome/browser/sync/glue/data_type_manager_impl.h | 66 +++++- .../sync/glue/data_type_manager_impl_unittest.cc | 246 +++++++++++++++++++- .../browser/sync/glue/fake_data_type_controller.cc | 2 +- .../browser/sync/glue/model_association_manager.cc | 58 ++--- .../browser/sync/glue/model_association_manager.h | 46 ++-- .../glue/model_association_manager_unittest.cc | 108 ++++++--- chrome/browser/sync/glue/sync_backend_host.cc | 2 +- .../sync/glue/sync_backend_host_unittest.cc | 4 +- .../sync/profile_sync_service_autofill_unittest.cc | 7 +- .../profile_sync_service_preference_unittest.cc | 7 +- 12 files changed, 616 insertions(+), 195 deletions(-) (limited to 'chrome/browser') diff --git a/chrome/browser/sync/glue/backend_data_type_configurer.h b/chrome/browser/sync/glue/backend_data_type_configurer.h index 2848247..38bdf9a3 100644 --- a/chrome/browser/sync/glue/backend_data_type_configurer.h +++ b/chrome/browser/sync/glue/backend_data_type_configurer.h @@ -19,9 +19,13 @@ namespace browser_sync { class BackendDataTypeConfigurer { public: enum DataTypeConfigState { - ENABLED, // Actively being synced. - DISABLED, // Not syncing. Disabled by user. - FAILED, // Not syncing due to unrecoverable error. + CONFIGURE_ACTIVE, // Actively being configured. Data of such types + // will be downloaded if not present locally. + CONFIGURE_INACTIVE, // Already configured or to be configured in future. + // Data of such types is left as it is, no + // downloading or purging. + DISABLED, // Not syncing. Disabled by user. + FAILED, // Not syncing due to unrecoverable error. }; typedef std::map DataTypeConfigStateMap; diff --git a/chrome/browser/sync/glue/data_type_manager_impl.cc b/chrome/browser/sync/glue/data_type_manager_impl.cc index 5675c01..01e3679 100644 --- a/chrome/browser/sync/glue/data_type_manager_impl.cc +++ b/chrome/browser/sync/glue/data_type_manager_impl.cc @@ -21,11 +21,15 @@ #include "chrome/browser/sync/glue/data_type_controller.h" #include "chrome/browser/sync/glue/data_type_manager_observer.h" #include "content/public/browser/browser_thread.h" +#include "sync/internal_api/public/data_type_debug_info_listener.h" using content::BrowserThread; namespace browser_sync { +DataTypeManagerImpl::AssociationTypesInfo::AssociationTypesInfo() {} +DataTypeManagerImpl::AssociationTypesInfo::~AssociationTypesInfo() {} + DataTypeManagerImpl::DataTypeManagerImpl( const syncer::WeakHandle& debug_info_listener, @@ -39,9 +43,8 @@ DataTypeManagerImpl::DataTypeManagerImpl( needs_reconfigure_(false), last_configure_reason_(syncer::CONFIGURE_REASON_UNKNOWN), weak_ptr_factory_(this), - model_association_manager_(debug_info_listener, - controllers, - this), + debug_info_listener_(debug_info_listener), + model_association_manager_(controllers, this), observer_(observer), failed_datatypes_handler_(failed_datatypes_handler) { DCHECK(configurer_); @@ -86,12 +89,12 @@ void DataTypeManagerImpl::ConfigureImpl( } last_requested_types_ = desired_types; + last_configure_reason_ = reason; // Only proceed if we're in a steady state or blocked. if (state_ != STOPPED && state_ != CONFIGURED && state_ != BLOCKED) { DVLOG(1) << "Received configure request while configuration in flight. " << "Postponing until current configuration complete."; needs_reconfigure_ = true; - last_configure_reason_ = reason; return; } @@ -99,16 +102,21 @@ void DataTypeManagerImpl::ConfigureImpl( } BackendDataTypeConfigurer::DataTypeConfigStateMap -DataTypeManagerImpl::BuildDataTypeConfigStateMap() const { - // In three steps: - // 1. Add last_requested_types_ as ENABLED. - // 2. Set other types as DISABLED. - // 3. Overwrite state of failed types according to failed_datatypes_handler_. +DataTypeManagerImpl::BuildDataTypeConfigStateMap( + const syncer::ModelTypeSet& types_being_configured) const { + // In four steps: + // 1. Add last_requested_types_ as CONFIGURE_INACTIVE. + // 2. Flip |types_being_configured| to CONFIGURE_ACTIVE. + // 3. Set other types as DISABLED. + // 4. Overwrite state of failed types according to failed_datatypes_handler_. BackendDataTypeConfigurer::DataTypeConfigStateMap config_state_map; BackendDataTypeConfigurer::SetDataTypesState( - BackendDataTypeConfigurer::ENABLED, last_requested_types_, + BackendDataTypeConfigurer::CONFIGURE_INACTIVE, last_requested_types_, &config_state_map); BackendDataTypeConfigurer::SetDataTypesState( + BackendDataTypeConfigurer::CONFIGURE_ACTIVE, + types_being_configured, &config_state_map); + BackendDataTypeConfigurer::SetDataTypesState( BackendDataTypeConfigurer::DISABLED, syncer::Difference(syncer::UserTypes(), last_requested_types_), &config_state_map); @@ -129,6 +137,8 @@ void DataTypeManagerImpl::Restart(syncer::ConfigureReason reason) { DVLOG(1) << "Restarting..."; model_association_manager_.Initialize(last_requested_types_); last_restart_time_ = base::Time::Now(); + configure_result_ = ConfigureResult(); + configure_result_.status = OK; DCHECK(state_ == STOPPED || state_ == CONFIGURED || state_ == BLOCKED); @@ -139,22 +149,49 @@ void DataTypeManagerImpl::Restart(syncer::ConfigureReason reason) { model_association_manager_.StopDisabledTypes(); + download_types_queue_ = PrioritizeTypes(last_requested_types_); + association_types_queue_ = std::queue(); + // Tell the backend about the new set of data types we wish to sync. // The task will be invoked when updates are downloaded. state_ = DOWNLOAD_PENDING; configurer_->ConfigureDataTypes( reason, - BuildDataTypeConfigStateMap(), + BuildDataTypeConfigStateMap(download_types_queue_.front()), base::Bind(&DataTypeManagerImpl::DownloadReady, - weak_ptr_factory_.GetWeakPtr()), + weak_ptr_factory_.GetWeakPtr(), + base::Time::Now(), syncer::ModelTypeSet()), base::Bind(&DataTypeManagerImpl::OnDownloadRetry, weak_ptr_factory_.GetWeakPtr())); } -bool DataTypeManagerImpl::ProcessReconfigure() { - if (!needs_reconfigure_) { - return false; - } +TypeSetPriorityList DataTypeManagerImpl::PrioritizeTypes( + const syncer::ModelTypeSet& types) { + syncer::ModelTypeSet high_priority_types; + high_priority_types.PutAll(syncer::ControlTypes()); + high_priority_types.PutAll(syncer::PriorityUserTypes()); + high_priority_types.RetainAll(types); + + syncer::ModelTypeSet low_priority_types = + syncer::Difference(types, high_priority_types); + + TypeSetPriorityList result; + if (!high_priority_types.Empty()) + result.push(high_priority_types); + if (!low_priority_types.Empty()) + result.push(low_priority_types); + return result; +} + +void DataTypeManagerImpl::ProcessReconfigure() { + DCHECK(needs_reconfigure_); + + // Wait for current download and association to finish. + if (!(download_types_queue_.empty() && association_types_queue_.empty())) + return; + + model_association_manager_.ResetForReconfiguration(); + // An attempt was made to reconfigure while we were already configuring. // This can be because a passphrase was accepted or the user changed the // set of desired types. Either way, |last_requested_types_| will contain @@ -175,24 +212,24 @@ bool DataTypeManagerImpl::ProcessReconfigure() { last_configure_reason_)); needs_reconfigure_ = false; - last_configure_reason_ = syncer::CONFIGURE_REASON_UNKNOWN; - return true; } void DataTypeManagerImpl::OnDownloadRetry() { - DCHECK_EQ(state_, DOWNLOAD_PENDING); + DCHECK(state_ == DOWNLOAD_PENDING || state_ == CONFIGURING); observer_->OnConfigureRetry(); } void DataTypeManagerImpl::DownloadReady( + base::Time download_start_time, + syncer::ModelTypeSet high_priority_types_before, syncer::ModelTypeSet first_sync_types, syncer::ModelTypeSet failed_configuration_types) { - DCHECK_EQ(state_, DOWNLOAD_PENDING); + DCHECK(state_ == DOWNLOAD_PENDING || state_ == CONFIGURING); // Ignore |failed_configuration_types| if we need to reconfigure // anyway. if (needs_reconfigure_) { - model_association_manager_.ResetForReconfiguration(); + download_types_queue_ = TypeSetPriorityList(); ProcessReconfigure(); return; } @@ -203,20 +240,97 @@ void DataTypeManagerImpl::DownloadReady( "Configuration failed for types " + syncer::ModelTypeSetToString(failed_configuration_types); syncer::SyncError error(FROM_HERE, error_msg, - failed_configuration_types.First().Get()); + failed_configuration_types.First().Get()); Abort(UNRECOVERABLE_ERROR, error); return; } state_ = CONFIGURING; - model_association_manager_.SetFirstSyncTypesAndDownloadTime( - first_sync_types, base::Time::Now() - last_restart_time_); - model_association_manager_.StartAssociationAsync(); + // Pop and associate download-ready types. + syncer::ModelTypeSet ready_types = download_types_queue_.front(); + download_types_queue_.pop(); + + AssociationTypesInfo association_info; + association_info.types = ready_types; + association_info.first_sync_types = first_sync_types; + association_info.download_start_time = download_start_time; + association_info.download_ready_time = base::Time::Now(); + association_info.high_priority_types_before = high_priority_types_before; + association_types_queue_.push(association_info); + if (association_types_queue_.size() == 1u) + StartNextAssociation(); + + // Download types of low priority while configuring types of high priority. + if (!download_types_queue_.empty()) { + configurer_->ConfigureDataTypes( + last_configure_reason_, + BuildDataTypeConfigStateMap(download_types_queue_.front()), + base::Bind(&DataTypeManagerImpl::DownloadReady, + weak_ptr_factory_.GetWeakPtr(), + base::Time::Now(), + syncer::Union(ready_types, high_priority_types_before)), + base::Bind(&DataTypeManagerImpl::OnDownloadRetry, + weak_ptr_factory_.GetWeakPtr())); + } +} + +void DataTypeManagerImpl::StartNextAssociation() { + CHECK(!association_types_queue_.empty()); + + association_types_queue_.front().association_request_time = + base::Time::Now(); + model_association_manager_.StartAssociationAsync( + association_types_queue_.front().types); +} + +void DataTypeManagerImpl::OnSingleDataTypeAssociationDone( + syncer::ModelType type, + const syncer::DataTypeAssociationStats& association_stats) { + DCHECK(!association_types_queue_.empty()); + + if (!debug_info_listener_.IsInitialized()) + return; + + syncer::DataTypeConfigurationStats configuration_stats; + configuration_stats.model_type = type; + configuration_stats.association_stats = association_stats; + + AssociationTypesInfo& info = association_types_queue_.front(); + configuration_stats.download_wait_time = + info.download_start_time - last_restart_time_; + if (info.first_sync_types.Has(type)) { + configuration_stats.download_time = + info.download_ready_time - info.download_start_time; + } + configuration_stats.association_wait_time_for_high_priority = + info.association_request_time - info.download_ready_time; + configuration_stats.high_priority_types_configured_before = + info.high_priority_types_before; + configuration_stats.same_priority_types_configured_before = + info.configured_types; + + info.configured_types.Put(type); + + debug_info_listener_.Call( + FROM_HERE, + &syncer::DataTypeDebugInfoListener::OnSingleDataTypeConfigureComplete, + configuration_stats); } void DataTypeManagerImpl::OnModelAssociationDone( const DataTypeManager::ConfigureResult& result) { + DCHECK(state_ == STOPPING || state_ == CONFIGURING); + + if (state_ == STOPPING) + return; + + if (needs_reconfigure_) { + association_types_queue_ = std::queue(); + ProcessReconfigure(); + return; + } + if (result.status == ABORTED || result.status == UNRECOVERABLE_ERROR) { Abort(result.status, result.failed_data_types.size() >= 1 ? result.failed_data_types.front() : @@ -224,29 +338,29 @@ void DataTypeManagerImpl::OnModelAssociationDone( return; } - if (ProcessReconfigure()) { - return; - } - if (result.status == CONFIGURE_BLOCKED) { - failed_datatypes_info_.insert(failed_datatypes_info_.end(), - result.failed_data_types.begin(), - result.failed_data_types.end()); SetBlockedAndNotify(); return; } DCHECK(result.status == PARTIAL_SUCCESS || result.status == OK); - state_ = CONFIGURED; - failed_datatypes_info_.insert(failed_datatypes_info_.end(), - result.failed_data_types.begin(), - result.failed_data_types.end()); - ConfigureResult configure_result(result.status, - result.requested_types, - failed_datatypes_info_, - result.waiting_to_start); - NotifyDone(configure_result); - failed_datatypes_info_.clear(); + + if (result.status == PARTIAL_SUCCESS) + configure_result_.status = PARTIAL_SUCCESS; + configure_result_.requested_types.PutAll(result.requested_types); + configure_result_.failed_data_types.insert( + configure_result_.failed_data_types.end(), + result.failed_data_types.begin(), + result.failed_data_types.end()); + configure_result_.waiting_to_start.PutAll(result.waiting_to_start); + + association_types_queue_.pop(); + if (!association_types_queue_.empty()) { + StartNextAssociation(); + } else if (download_types_queue_.empty()) { + state_ = CONFIGURED; + NotifyDone(configure_result_); + } } void DataTypeManagerImpl::OnTypesLoaded() { @@ -265,42 +379,25 @@ void DataTypeManagerImpl::Stop() { if (state_ == STOPPED) return; - // If we are currently configuring, then the current type is in a - // partially started state. Abort the startup of the current type, - // which will synchronously invoke the start callback. - if (state_ == CONFIGURING) { - model_association_manager_.Stop(); - state_ = STOPPED; + bool need_to_notify = state_ == DOWNLOAD_PENDING || state_ == CONFIGURING; + StopImpl(); - return; - } - - const bool download_pending = state_ == DOWNLOAD_PENDING; - state_ = STOPPING; - if (download_pending) { - // If Stop() is called while waiting for download, cancel all - // outstanding tasks. - weak_ptr_factory_.InvalidateWeakPtrs(); - Abort(ABORTED, syncer::SyncError()); - return; + if (need_to_notify) { + ConfigureResult result(ABORTED, + last_requested_types_, + std::list(), + syncer::ModelTypeSet()); + NotifyDone(result); } - - FinishStop(); -} - -void DataTypeManagerImpl::FinishStop() { - DCHECK(state_== CONFIGURING || state_ == STOPPING || state_ == BLOCKED || - state_ == DOWNLOAD_PENDING); - model_association_manager_.Stop(); - - failed_datatypes_info_.clear(); - state_ = STOPPED; } void DataTypeManagerImpl::Abort(ConfigureStatus status, const syncer::SyncError& error) { + DCHECK(state_ == DOWNLOAD_PENDING || state_ == CONFIGURING); + + StopImpl(); + DCHECK_NE(OK, status); - FinishStop(); std::list error_list; if (error.IsSet()) error_list.push_back(error); @@ -311,6 +408,19 @@ void DataTypeManagerImpl::Abort(ConfigureStatus status, NotifyDone(result); } +void DataTypeManagerImpl::StopImpl() { + state_ = STOPPING; + + // Invalidate weak pointer to drop download callbacks. + weak_ptr_factory_.InvalidateWeakPtrs(); + + // Stop all data types. This may trigger association callback but the + // callback will do nothing because state is set to STOPPING above. + model_association_manager_.Stop(); + + state_ = STOPPED; +} + void DataTypeManagerImpl::NotifyStart() { observer_->OnConfigureStart(); } @@ -353,6 +463,9 @@ DataTypeManager::State DataTypeManagerImpl::state() const { } void DataTypeManagerImpl::SetBlockedAndNotify() { + // Drop download callbacks. + weak_ptr_factory_.InvalidateWeakPtrs(); + state_ = BLOCKED; AddToConfigureTime(); DVLOG(1) << "Accumulated spent configuring: " diff --git a/chrome/browser/sync/glue/data_type_manager_impl.h b/chrome/browser/sync/glue/data_type_manager_impl.h index 6bbbb0d..778cfd2 100644 --- a/chrome/browser/sync/glue/data_type_manager_impl.h +++ b/chrome/browser/sync/glue/data_type_manager_impl.h @@ -7,8 +7,8 @@ #include "chrome/browser/sync/glue/data_type_manager.h" -#include #include +#include #include #include "base/basictypes.h" @@ -30,6 +30,10 @@ namespace browser_sync { class DataTypeController; class DataTypeManagerObserver; +// List of data types grouped by priority and ordered from high priority to +// low priority. +typedef std::queue TypeSetPriorityList; + class DataTypeManagerImpl : public DataTypeManager, public ModelAssociationResultProcessor { public: @@ -55,6 +59,9 @@ class DataTypeManagerImpl : public DataTypeManager, virtual State state() const OVERRIDE; // |ModelAssociationResultProcessor| implementation. + virtual void OnSingleDataTypeAssociationDone( + syncer::ModelType type, + const syncer::DataTypeAssociationStats& association_stats) OVERRIDE; virtual void OnModelAssociationDone( const DataTypeManager::ConfigureResult& result) OVERRIDE; virtual void OnTypesLoaded() OVERRIDE; @@ -65,18 +72,24 @@ class DataTypeManagerImpl : public DataTypeManager, return &model_association_manager_; } + protected: + // Divide |types| into sets by their priorities and return the sets from + // high priority to low priority. + virtual TypeSetPriorityList PrioritizeTypes( + const syncer::ModelTypeSet& types); + private: - // Stops all data types. - void FinishStop(); + // Abort configuration and stop all data types due to configuration errors. void Abort(ConfigureStatus status, const syncer::SyncError& error); - // If there's a pending reconfigure, processes it and returns true. - // Otherwise, returns false. - bool ProcessReconfigure(); + // Post a task to reconfigure when no downloading or association are running. + void ProcessReconfigure(); void Restart(syncer::ConfigureReason reason); - void DownloadReady(syncer::ModelTypeSet first_sync_types, + void DownloadReady(base::Time download_start_time, + syncer::ModelTypeSet high_priority_types_before, + syncer::ModelTypeSet first_sync_types, syncer::ModelTypeSet failed_configuration_types); // Notification from the SBH that download failed due to a transient @@ -93,7 +106,14 @@ class DataTypeManagerImpl : public DataTypeManager, void ConfigureImpl(TypeSet desired_types, syncer::ConfigureReason reason); BackendDataTypeConfigurer::DataTypeConfigStateMap - BuildDataTypeConfigStateMap() const; + BuildDataTypeConfigStateMap( + const syncer::ModelTypeSet& types_being_configured) const; + + // Start association of next batch of data types after association of + // previous batch finishes. + void StartNextAssociation(); + + void StopImpl(); BackendDataTypeConfigurer* configurer_; // Map of all data type controllers that are available for sync. @@ -120,10 +140,11 @@ class DataTypeManagerImpl : public DataTypeManager, // to the DONE/BLOCKED state. base::TimeDelta configure_time_delta_; - // Collects the list of errors resulting from failing to start a type. This - // would eventually be sent to the listeners after all the types have - // been given a chance to start. - std::list failed_datatypes_info_; + // Sync's datatype debug info listener, which we pass model association + // statistics to. + const syncer::WeakHandle + debug_info_listener_; + ModelAssociationManager model_association_manager_; // DataTypeManager must have only one observer -- the ProfileSyncService that @@ -134,6 +155,27 @@ class DataTypeManagerImpl : public DataTypeManager, // configuring backend. const FailedDatatypesHandler* failed_datatypes_handler_; + // Types waiting to be downloaded. + TypeSetPriorityList download_types_queue_; + + // Types waiting for association and related time tracking info. + struct AssociationTypesInfo { + AssociationTypesInfo(); + ~AssociationTypesInfo(); + syncer::ModelTypeSet types; + syncer::ModelTypeSet first_sync_types; + base::Time download_start_time; + base::Time download_ready_time; + base::Time association_request_time; + syncer::ModelTypeSet high_priority_types_before; + syncer::ModelTypeSet configured_types; + }; + std::queue association_types_queue_; + + // Configuration result. This would eventually be sent to the listeners after + // all the types have been given a chance to start. + ConfigureResult configure_result_; + DISALLOW_COPY_AND_ASSIGN(DataTypeManagerImpl); }; diff --git a/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc b/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc index 7477b08..bcccb62 100644 --- a/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc +++ b/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc @@ -45,14 +45,29 @@ class FakeBackendDataTypeConfigurer : public BackendDataTypeConfigurer { ModelTypeSet)>& ready_task, const base::Callback& retry_callback) OVERRIDE { last_ready_task_ = ready_task; + + if (!expected_configure_types_.Empty()) { + EXPECT_TRUE( + expected_configure_types_.Equals( + GetDataTypesInState(CONFIGURE_ACTIVE, config_state_map))) + << syncer::ModelTypeSetToString(expected_configure_types_) + << " v.s. " + << syncer::ModelTypeSetToString( + GetDataTypesInState(CONFIGURE_ACTIVE, config_state_map)); + } } base::Callback last_ready_task() const { return last_ready_task_; } + void set_expected_configure_types(syncer::ModelTypeSet types) { + expected_configure_types_ = types; + } + private: base::Callback last_ready_task_; + syncer::ModelTypeSet expected_configure_types_; }; // Mock DataTypeManagerObserver implementation. @@ -74,6 +89,38 @@ DataTypeManager::ConfigureStatus GetStatus( return result.status; } +class TestDataTypeManager : public DataTypeManagerImpl { + public: + TestDataTypeManager( + const syncer::WeakHandle& + debug_info_listener, + BackendDataTypeConfigurer* configurer, + const DataTypeController::TypeMap* controllers, + DataTypeManagerObserver* observer, + const FailedDatatypesHandler* failed_datatypes_handler) + : DataTypeManagerImpl(debug_info_listener, configurer, controllers, + observer, failed_datatypes_handler) {} + + void set_priority_list(const TypeSetPriorityList& list) { + priority_list_ = list; + } + + protected: + virtual TypeSetPriorityList PrioritizeTypes( + const syncer::ModelTypeSet& types) OVERRIDE { + TypeSetPriorityList result; + if (priority_list_.empty()) { + result.push(types); + } else { + result = priority_list_; + } + return result; + } + + private: + TypeSetPriorityList priority_list_; +}; + // The actual test harness class, parametrized on nigori state (i.e., tests are // run both configuring with nigori, and configuring without). class SyncDataTypeManagerImplTest : public testing::Test { @@ -87,7 +134,7 @@ class SyncDataTypeManagerImplTest : public testing::Test { protected: virtual void SetUp() { dtm_.reset( - new DataTypeManagerImpl( + new TestDataTypeManager( syncer::WeakHandle(), &configurer_, &controllers_, @@ -118,7 +165,8 @@ class SyncDataTypeManagerImplTest : public testing::Test { void FinishDownload(const DataTypeManager& dtm, ModelTypeSet types_to_configure, ModelTypeSet failed_download_types) { - EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm.state()); + EXPECT_TRUE(DataTypeManager::DOWNLOAD_PENDING == dtm.state() || + DataTypeManager::CONFIGURING == dtm.state()); ASSERT_FALSE(configurer_.last_ready_task().is_null()); configurer_.last_ready_task().Run( syncer::Difference(types_to_configure, failed_download_types), @@ -149,7 +197,7 @@ class SyncDataTypeManagerImplTest : public testing::Test { DataTypeController::TypeMap controllers_; FakeBackendDataTypeConfigurer configurer_; DataTypeManagerObserverMock observer_; - scoped_ptr dtm_; + scoped_ptr dtm_; }; // Set up a DTM with no controllers, configure it, finish downloading, @@ -790,4 +838,196 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureDuringPurge) { EXPECT_EQ(DataTypeManager::CONFIGURED, dtm_->state()); } +TEST_F(SyncDataTypeManagerImplTest, PrioritizedConfiguration) { + AddController(BOOKMARKS); + AddController(PREFERENCES); + + TypeSetPriorityList priority_list; + priority_list.push(ModelTypeSet(PREFERENCES)); + priority_list.push(ModelTypeSet(BOOKMARKS)); + dtm_->set_priority_list(priority_list); + + // Initial configure. + SetConfigureStartExpectation(); + SetConfigureDoneExpectation(DataTypeManager::OK); + + // Initially only PREFERENCES is configured. + configurer_.set_expected_configure_types(ModelTypeSet(PREFERENCES)); + Configure(dtm_.get(), ModelTypeSet(BOOKMARKS, PREFERENCES)); + EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state()); + + // BOOKMARKS is configured after download of PREFERENCES finishes. + configurer_.set_expected_configure_types(ModelTypeSet(BOOKMARKS)); + FinishDownload(*dtm_, ModelTypeSet(PREFERENCES), ModelTypeSet()); + EXPECT_EQ(DataTypeManager::CONFIGURING, dtm_->state()); + + FinishDownload(*dtm_, ModelTypeSet(BOOKMARKS), ModelTypeSet()); + GetController(PREFERENCES)->FinishStart(DataTypeController::OK); + EXPECT_EQ(DataTypeManager::CONFIGURING, dtm_->state()); + + GetController(BOOKMARKS)->FinishStart(DataTypeController::OK); + EXPECT_EQ(DataTypeManager::CONFIGURED, dtm_->state()); +} + +TEST_F(SyncDataTypeManagerImplTest, PrioritizedConfigurationReconfigure) { + AddController(BOOKMARKS); + AddController(PREFERENCES); + AddController(APPS); + + TypeSetPriorityList priority_list; + priority_list.push(ModelTypeSet(PREFERENCES)); + priority_list.push(ModelTypeSet(BOOKMARKS)); + dtm_->set_priority_list(priority_list); + + // Initial configure. + SetConfigureStartExpectation(); + SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureBlockedExpectation(); + + // Reconfigure while associating PREFERENCES and downloading BOOKMARKS. + configurer_.set_expected_configure_types(ModelTypeSet(PREFERENCES)); + Configure(dtm_.get(), ModelTypeSet(BOOKMARKS, PREFERENCES)); + EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state()); + + configurer_.set_expected_configure_types(ModelTypeSet(BOOKMARKS)); + FinishDownload(*dtm_, ModelTypeSet(PREFERENCES), ModelTypeSet()); + EXPECT_EQ(DataTypeManager::CONFIGURING, dtm_->state()); + + // Enable syncing for APPS. + priority_list.back() = ModelTypeSet(BOOKMARKS, APPS); + dtm_->set_priority_list(priority_list); + Configure(dtm_.get(), ModelTypeSet(BOOKMARKS, PREFERENCES, APPS)); + EXPECT_EQ(DataTypeManager::CONFIGURING, dtm_->state()); + + // Reconfiguration starts after downloading and association of previous + // types finish. + FinishDownload(*dtm_, ModelTypeSet(BOOKMARKS), ModelTypeSet()); + GetController(PREFERENCES)->FinishStart(DataTypeController::OK); + EXPECT_EQ(DataTypeManager::BLOCKED, dtm_->state()); + + configurer_.set_expected_configure_types(ModelTypeSet(PREFERENCES)); + ui_loop_.RunUntilIdle(); // Let reconfigure closure run. + EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state()); + + configurer_.set_expected_configure_types(ModelTypeSet(BOOKMARKS, APPS)); + FinishDownload(*dtm_, ModelTypeSet(PREFERENCES), ModelTypeSet()); + EXPECT_EQ(DataTypeManager::CONFIGURING, dtm_->state()); + + FinishDownload(*dtm_, ModelTypeSet(BOOKMARKS, APPS), ModelTypeSet()); + EXPECT_EQ(DataTypeManager::CONFIGURING, dtm_->state()); + + // Skip calling FinishStart() for PREFENCES because it's already started in + // first configuration. + GetController(BOOKMARKS)->FinishStart(DataTypeController::OK); + GetController(APPS)->FinishStart(DataTypeController::OK); + EXPECT_EQ(DataTypeManager::CONFIGURED, dtm_->state()); +} + +TEST_F(SyncDataTypeManagerImplTest, PrioritizedConfigurationStop) { + AddController(BOOKMARKS); + AddController(PREFERENCES); + + TypeSetPriorityList priority_list; + priority_list.push(ModelTypeSet(PREFERENCES)); + priority_list.push(ModelTypeSet(BOOKMARKS)); + dtm_->set_priority_list(priority_list); + + // Initial configure. + SetConfigureStartExpectation(); + + // Initially only PREFERENCES is configured. + configurer_.set_expected_configure_types(ModelTypeSet(PREFERENCES)); + Configure(dtm_.get(), ModelTypeSet(BOOKMARKS, PREFERENCES)); + EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state()); + + // BOOKMARKS is configured after download of PREFERENCES finishes. + configurer_.set_expected_configure_types(ModelTypeSet(BOOKMARKS)); + FinishDownload(*dtm_, ModelTypeSet(PREFERENCES), ModelTypeSet()); + EXPECT_EQ(DataTypeManager::CONFIGURING, dtm_->state()); + + // PERFERENCES controller is associating while BOOKMARKS is downloading. + EXPECT_EQ(DataTypeController::ASSOCIATING, + GetController(PREFERENCES)->state()); + EXPECT_EQ(DataTypeController::NOT_RUNNING, GetController(BOOKMARKS)->state()); + + dtm_->Stop(); + EXPECT_EQ(DataTypeManager::STOPPED, dtm_->state()); + EXPECT_EQ(DataTypeController::NOT_RUNNING, + GetController(PREFERENCES)->state()); + EXPECT_EQ(DataTypeController::NOT_RUNNING, GetController(BOOKMARKS)->state()); +} + +TEST_F(SyncDataTypeManagerImplTest, PrioritizedConfigurationDownloadError) { + AddController(BOOKMARKS); + AddController(PREFERENCES); + + TypeSetPriorityList priority_list; + priority_list.push(ModelTypeSet(PREFERENCES)); + priority_list.push(ModelTypeSet(BOOKMARKS)); + dtm_->set_priority_list(priority_list); + + // Initial configure. + SetConfigureStartExpectation(); + SetConfigureDoneExpectation(DataTypeManager::UNRECOVERABLE_ERROR); + + // Initially only PREFERENCES is configured. + configurer_.set_expected_configure_types(ModelTypeSet(PREFERENCES)); + Configure(dtm_.get(), ModelTypeSet(BOOKMARKS, PREFERENCES)); + EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state()); + + // BOOKMARKS is configured after download of PREFERENCES finishes. + configurer_.set_expected_configure_types(ModelTypeSet(BOOKMARKS)); + FinishDownload(*dtm_, ModelTypeSet(PREFERENCES), ModelTypeSet()); + EXPECT_EQ(DataTypeManager::CONFIGURING, dtm_->state()); + + // PERFERENCES controller is associating while BOOKMARKS is downloading. + EXPECT_EQ(DataTypeController::ASSOCIATING, + GetController(PREFERENCES)->state()); + EXPECT_EQ(DataTypeController::NOT_RUNNING, GetController(BOOKMARKS)->state()); + + // Make BOOKMARKS download fail. + FinishDownload(*dtm_, ModelTypeSet(), ModelTypeSet(BOOKMARKS)); + EXPECT_EQ(DataTypeManager::STOPPED, dtm_->state()); + EXPECT_EQ(DataTypeController::NOT_RUNNING, + GetController(PREFERENCES)->state()); + EXPECT_EQ(DataTypeController::NOT_RUNNING, GetController(BOOKMARKS)->state()); +} + +TEST_F(SyncDataTypeManagerImplTest, PrioritizedConfigurationAssociationError) { + AddController(BOOKMARKS); + AddController(PREFERENCES); + + TypeSetPriorityList priority_list; + priority_list.push(ModelTypeSet(PREFERENCES)); + priority_list.push(ModelTypeSet(BOOKMARKS)); + dtm_->set_priority_list(priority_list); + + // Initial configure. + SetConfigureStartExpectation(); + SetConfigureDoneExpectation(DataTypeManager::UNRECOVERABLE_ERROR); + + // Initially only PREFERENCES is configured. + configurer_.set_expected_configure_types(ModelTypeSet(PREFERENCES)); + Configure(dtm_.get(), ModelTypeSet(BOOKMARKS, PREFERENCES)); + EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state()); + + // BOOKMARKS is configured after download of PREFERENCES finishes. + configurer_.set_expected_configure_types(ModelTypeSet(BOOKMARKS)); + FinishDownload(*dtm_, ModelTypeSet(PREFERENCES), ModelTypeSet()); + EXPECT_EQ(DataTypeManager::CONFIGURING, dtm_->state()); + + // PERFERENCES controller is associating while BOOKMARKS is downloading. + EXPECT_EQ(DataTypeController::ASSOCIATING, + GetController(PREFERENCES)->state()); + EXPECT_EQ(DataTypeController::NOT_RUNNING, GetController(BOOKMARKS)->state()); + + // Make PREFERENCES association fail. + GetController(PREFERENCES)->FinishStart( + DataTypeController::UNRECOVERABLE_ERROR); + EXPECT_EQ(DataTypeManager::STOPPED, dtm_->state()); + EXPECT_EQ(DataTypeController::NOT_RUNNING, + GetController(PREFERENCES)->state()); + EXPECT_EQ(DataTypeController::NOT_RUNNING, GetController(BOOKMARKS)->state()); +} + } // namespace browser_sync diff --git a/chrome/browser/sync/glue/fake_data_type_controller.cc b/chrome/browser/sync/glue/fake_data_type_controller.cc index b0316bfa..df36a19 100644 --- a/chrome/browser/sync/glue/fake_data_type_controller.cc +++ b/chrome/browser/sync/glue/fake_data_type_controller.cc @@ -28,8 +28,8 @@ void FakeDataTypeController::LoadModels( } if (model_load_delayed_ == false) { - model_load_callback.Run(type(), syncer::SyncError()); state_ = MODEL_LOADED; + model_load_callback.Run(type(), syncer::SyncError()); } else { model_load_callback_ = model_load_callback; state_ = MODEL_STARTING; diff --git a/chrome/browser/sync/glue/model_association_manager.cc b/chrome/browser/sync/glue/model_association_manager.cc index 0e435e8..43016d3 100644 --- a/chrome/browser/sync/glue/model_association_manager.cc +++ b/chrome/browser/sync/glue/model_association_manager.cc @@ -79,12 +79,10 @@ class SortComparator : public std::binary_function& - debug_info_listener, const DataTypeController::TypeMap* controllers, ModelAssociationResultProcessor* processor) : state_(IDLE), currently_associating_(NULL), controllers_(controllers), result_processor_(processor), - debug_info_listener_(debug_info_listener), weak_ptr_factory_(this) { // Ensure all data type controllers are stopped. @@ -148,7 +142,7 @@ void ModelAssociationManager::Initialize(syncer::ModelTypeSet desired_types) { needs_start_.clear(); needs_stop_.clear(); failed_datatypes_info_.clear(); - desired_types_ = desired_types; + associating_types_.Clear(); state_ = INITIALIZED_TO_CONFIGURE; DVLOG(1) << "ModelAssociationManager: Initializing"; @@ -169,13 +163,6 @@ void ModelAssociationManager::Initialize(syncer::ModelTypeSet desired_types) { waiting_to_associate_.clear(); currently_associating_ = NULL; - // We need to calculate our |needs_start_| and |needs_stop_| list. - GetControllersNeedingStart(&needs_start_); - // Sort these according to kStartOrder. - std::sort(needs_start_.begin(), - needs_start_.end(), - SortComparator(&start_order_)); - // Add any data type controllers into that needs_stop_ list that are // currently MODEL_STARTING, ASSOCIATING, RUNNING or DISABLED. for (DataTypeController::TypeMap::const_iterator it = controllers_->begin(); @@ -196,23 +183,25 @@ void ModelAssociationManager::Initialize(syncer::ModelTypeSet desired_types) { SortComparator(&start_order_)); } -void ModelAssociationManager::SetFirstSyncTypesAndDownloadTime( - const syncer::ModelTypeSet& types, const base::TimeDelta& time) { - DCHECK_EQ(state_, INITIALIZED_TO_CONFIGURE); - first_sync_types_ = types; - first_sync_download_time_ = time; -} - -void ModelAssociationManager::StartAssociationAsync() { - DCHECK_EQ(state_, INITIALIZED_TO_CONFIGURE); +void ModelAssociationManager::StartAssociationAsync( + const syncer::ModelTypeSet& types_to_associate) { + DCHECK(state_ == INITIALIZED_TO_CONFIGURE || state_ == IDLE); state_ = CONFIGURING; + + // Calculate |needs_start_| list. + associating_types_ = types_to_associate; + GetControllersNeedingStart(&needs_start_); + // Sort these according to kStartOrder. + std::sort(needs_start_.begin(), + needs_start_.end(), + SortComparator(&start_order_)); + DVLOG(1) << "ModelAssociationManager: Going to start model association"; association_start_time_ = base::Time::Now(); LoadModelForNextType(); } void ModelAssociationManager::ResetForReconfiguration() { - DCHECK_EQ(state_, INITIALIZED_TO_CONFIGURE); state_ = IDLE; DVLOG(1) << "ModelAssociationManager: Reseting for reconfiguration"; needs_start_.clear(); @@ -276,7 +265,7 @@ void ModelAssociationManager::Stop() { if (need_to_call_model_association_done) { DVLOG(1) << "ModelAssociationManager: Calling OnModelAssociationDone"; DataTypeManager::ConfigureResult result(DataTypeManager::ABORTED, - desired_types_, + associating_types_, failed_datatypes_info_, syncer::ModelTypeSet()); result_processor_->OnModelAssociationDone(result); @@ -291,7 +280,7 @@ bool ModelAssociationManager::GetControllersNeedingStart( // Add any data type controllers into the needs_start_ list that are // currently NOT_RUNNING or STOPPING. bool found_any = false; - for (ModelTypeSet::Iterator it = desired_types_.First(); + for (ModelTypeSet::Iterator it = associating_types_.First(); it.Good(); it.Inc()) { DataTypeController::TypeMap::const_iterator dtc = controllers_->find(it.Get()); @@ -355,11 +344,7 @@ void ModelAssociationManager::TypeStartCallback( // occurred. if ((DataTypeController::IsSuccessfulResult(start_result) || start_result == DataTypeController::ASSOCIATION_FAILED) && - debug_info_listener_.IsInitialized() && syncer::ProtocolTypes().Has(local_merge_result.model_type())) { - base::TimeDelta download_time_first_sync = - first_sync_types_.Has(local_merge_result.model_type()) ? - first_sync_download_time_ : base::TimeDelta(); base::TimeDelta association_wait_time = current_type_association_start_time_ - association_start_time_; base::TimeDelta association_time = @@ -367,13 +352,10 @@ void ModelAssociationManager::TypeStartCallback( syncer::DataTypeAssociationStats stats = BuildAssociationStatsFromMergeResults(local_merge_result, syncer_merge_result, - download_time_first_sync, association_wait_time, association_time); - debug_info_listener_.Call( - FROM_HERE, - &syncer::DataTypeDebugInfoListener::OnDataTypeAssociationComplete, - stats); + result_processor_->OnSingleDataTypeAssociationDone( + local_merge_result.model_type(), stats); } // If the type started normally, continue to the next type. @@ -415,7 +397,7 @@ void ModelAssociationManager::TypeStartCallback( state_ = IDLE; DataTypeManager::ConfigureResult configure_result(configure_status, - desired_types_, + associating_types_, errors, syncer::ModelTypeSet()); result_processor_->OnModelAssociationDone(configure_result); @@ -543,7 +525,7 @@ void ModelAssociationManager::StartAssociatingNextType() { DataTypeManager::ConfigureResult configure_result( DataTypeManager::CONFIGURE_BLOCKED, - desired_types_, + associating_types_, failed_datatypes_info_, syncer::ModelTypeSet()); state_ = IDLE; @@ -563,7 +545,7 @@ void ModelAssociationManager::StartAssociatingNextType() { } DataTypeManager::ConfigureResult result(configure_status, - desired_types_, + associating_types_, failed_datatypes_info_, GetTypesWaitingToLoad()); result_processor_->OnModelAssociationDone(result); diff --git a/chrome/browser/sync/glue/model_association_manager.h b/chrome/browser/sync/glue/model_association_manager.h index 33aad48..eff88c5 100644 --- a/chrome/browser/sync/glue/model_association_manager.h +++ b/chrome/browser/sync/glue/model_association_manager.h @@ -11,7 +11,7 @@ #include "base/timer.h" #include "chrome/browser/sync/glue/data_type_manager.h" -#include "sync/internal_api/public/data_type_debug_info_listener.h" +#include "sync/internal_api/public/data_type_association_stats.h" #include "sync/internal_api/public/util/weak_handle.h" // |ModelAssociationManager| does the heavy lifting for doing the actual model @@ -27,6 +27,9 @@ class DataTypeController; // those operations are passed back via this interface. class ModelAssociationResultProcessor { public: + virtual void OnSingleDataTypeAssociationDone( + syncer::ModelType type, + const syncer::DataTypeAssociationStats& association_stats) = 0; virtual void OnModelAssociationDone( const DataTypeManager::ConfigureResult& result) = 0; virtual ~ModelAssociationResultProcessor() {} @@ -41,32 +44,25 @@ class ModelAssociationResultProcessor { // The class that is responsible for model association. class ModelAssociationManager { public: - ModelAssociationManager( - const syncer::WeakHandle& - debug_info_listener, - const DataTypeController::TypeMap* controllers, - ModelAssociationResultProcessor* processor); + ModelAssociationManager(const DataTypeController::TypeMap* controllers, + ModelAssociationResultProcessor* processor); virtual ~ModelAssociationManager(); // Initializes the state to do the model association in future. This // should be called before communicating with sync server. A subsequent call // of Initialize is only allowed if the ModelAssociationManager has invoked - // |OnModelAssociationDone| on the |ModelAssociationResultProcessor|. + // |OnModelAssociationDone| on the |ModelAssociationResultProcessor|. After + // this call, there should be several calls to StartAssociationAsync() + // to associate subset of |desired_types|. void Initialize(syncer::ModelTypeSet desired_types); // Can be called at any time. Synchronously stops all datatypes. void Stop(); - // Called before StartAssociationAsync() to flag types that are synced - // for the first time. For those types, |download_time| is recorded in their - // association stats. - void SetFirstSyncTypesAndDownloadTime(const syncer::ModelTypeSet& types, - const base::TimeDelta& download_time); - - // Should only be called after Initialize. - // Starts the actual association. When this is completed - // |OnModelAssociationDone| would be called invoked. - void StartAssociationAsync(); + // Should only be called after Initialize to start the actual association. + // |types_to_associate| should be subset of |desired_types| in Initialize(). + // When this is completed, |OnModelAssociationDone| will be invoked. + void StartAssociationAsync(const syncer::ModelTypeSet& types_to_associate); // It is valid to call this only when we are initialized to configure // but we have not started the configuration.(i.e., |Initialize| has @@ -129,9 +125,11 @@ class ModelAssociationManager { syncer::ModelTypeSet GetTypesWaitingToLoad(); - State state_; - syncer::ModelTypeSet desired_types_; + + // Data types currently being associated. + syncer::ModelTypeSet associating_types_; + std::list failed_datatypes_info_; std::map start_order_; @@ -183,18 +181,8 @@ class ModelAssociationManager { // Timer to track and limit how long a datatype takes to model associate. base::OneShotTimer timer_; - // Sync's datatype debug info listener, which we pass model association - // statistics to. - const syncer::WeakHandle - debug_info_listener_; - base::WeakPtrFactory weak_ptr_factory_; - // Types that are synced for the first time and time spent on downloading - // their data. - syncer::ModelTypeSet first_sync_types_; - base::TimeDelta first_sync_download_time_; - DISALLOW_COPY_AND_ASSIGN(ModelAssociationManager); }; } // namespace browser_sync diff --git a/chrome/browser/sync/glue/model_association_manager_unittest.cc b/chrome/browser/sync/glue/model_association_manager_unittest.cc index 4f5a43f..2bc1d31 100644 --- a/chrome/browser/sync/glue/model_association_manager_unittest.cc +++ b/chrome/browser/sync/glue/model_association_manager_unittest.cc @@ -17,6 +17,9 @@ class MockModelAssociationResultProcessor : public: MockModelAssociationResultProcessor() {} ~MockModelAssociationResultProcessor() {} + MOCK_METHOD2(OnSingleDataTypeAssociationDone, + void(syncer::ModelType type, + const syncer::DataTypeAssociationStats& association_stats)); MOCK_METHOD1(OnModelAssociationDone, void( const DataTypeManager::ConfigureResult& result)); MOCK_METHOD0(OnTypesLoaded, void()); @@ -71,10 +74,8 @@ class SyncModelAssociationManagerTest : public testing::Test { TEST_F(SyncModelAssociationManagerTest, SimpleModelStart) { controllers_[syncer::BOOKMARKS] = new FakeDataTypeController(syncer::BOOKMARKS); - ModelAssociationManager model_association_manager( - syncer::WeakHandle(), - &controllers_, - &result_processor_); + ModelAssociationManager model_association_manager(&controllers_, + &result_processor_); syncer::ModelTypeSet types; types.Put(syncer::BOOKMARKS); DataTypeManager::ConfigureResult expected_result( @@ -87,10 +88,10 @@ TEST_F(SyncModelAssociationManagerTest, SimpleModelStart) { model_association_manager.Initialize(types); model_association_manager.StopDisabledTypes(); - model_association_manager.StartAssociationAsync(); + model_association_manager.StartAssociationAsync(types); EXPECT_EQ(GetController(controllers_, syncer::BOOKMARKS)->state(), - DataTypeController::MODEL_LOADED); + DataTypeController::ASSOCIATING); GetController(controllers_, syncer::BOOKMARKS)->FinishStart( DataTypeController::OK); } @@ -100,7 +101,6 @@ TEST_F(SyncModelAssociationManagerTest, StopModelBeforeFinish) { controllers_[syncer::BOOKMARKS] = new FakeDataTypeController(syncer::BOOKMARKS); ModelAssociationManager model_association_manager( - syncer::WeakHandle(), &controllers_, &result_processor_); @@ -118,10 +118,10 @@ TEST_F(SyncModelAssociationManagerTest, StopModelBeforeFinish) { model_association_manager.Initialize(types); model_association_manager.StopDisabledTypes(); - model_association_manager.StartAssociationAsync(); + model_association_manager.StartAssociationAsync(types); EXPECT_EQ(GetController(controllers_, syncer::BOOKMARKS)->state(), - DataTypeController::MODEL_LOADED); + DataTypeController::ASSOCIATING); model_association_manager.Stop(); EXPECT_EQ(GetController(controllers_, syncer::BOOKMARKS)->state(), DataTypeController::NOT_RUNNING); @@ -132,7 +132,6 @@ TEST_F(SyncModelAssociationManagerTest, StopAfterFinish) { controllers_[syncer::BOOKMARKS] = new FakeDataTypeController(syncer::BOOKMARKS); ModelAssociationManager model_association_manager( - syncer::WeakHandle(), &controllers_, &result_processor_); syncer::ModelTypeSet types; @@ -147,10 +146,10 @@ TEST_F(SyncModelAssociationManagerTest, StopAfterFinish) { model_association_manager.Initialize(types); model_association_manager.StopDisabledTypes(); - model_association_manager.StartAssociationAsync(); + model_association_manager.StartAssociationAsync(types); EXPECT_EQ(GetController(controllers_, syncer::BOOKMARKS)->state(), - DataTypeController::MODEL_LOADED); + DataTypeController::ASSOCIATING); GetController(controllers_, syncer::BOOKMARKS)->FinishStart( DataTypeController::OK); @@ -164,7 +163,6 @@ TEST_F(SyncModelAssociationManagerTest, TypeFailModelAssociation) { controllers_[syncer::BOOKMARKS] = new FakeDataTypeController(syncer::BOOKMARKS); ModelAssociationManager model_association_manager( - syncer::WeakHandle(), &controllers_, &result_processor_); syncer::ModelTypeSet types; @@ -182,10 +180,10 @@ TEST_F(SyncModelAssociationManagerTest, TypeFailModelAssociation) { model_association_manager.Initialize(types); model_association_manager.StopDisabledTypes(); - model_association_manager.StartAssociationAsync(); + model_association_manager.StartAssociationAsync(types); EXPECT_EQ(GetController(controllers_, syncer::BOOKMARKS)->state(), - DataTypeController::MODEL_LOADED); + DataTypeController::ASSOCIATING); GetController(controllers_, syncer::BOOKMARKS)->FinishStart( DataTypeController::ASSOCIATION_FAILED); } @@ -195,7 +193,6 @@ TEST_F(SyncModelAssociationManagerTest, TypeReturnUnrecoverableError) { controllers_[syncer::BOOKMARKS] = new FakeDataTypeController(syncer::BOOKMARKS); ModelAssociationManager model_association_manager( - syncer::WeakHandle(), &controllers_, &result_processor_); syncer::ModelTypeSet types; @@ -213,10 +210,10 @@ TEST_F(SyncModelAssociationManagerTest, TypeReturnUnrecoverableError) { model_association_manager.Initialize(types); model_association_manager.StopDisabledTypes(); - model_association_manager.StartAssociationAsync(); + model_association_manager.StartAssociationAsync(types); EXPECT_EQ(GetController(controllers_, syncer::BOOKMARKS)->state(), - DataTypeController::MODEL_LOADED); + DataTypeController::ASSOCIATING); GetController(controllers_, syncer::BOOKMARKS)->FinishStart( DataTypeController::UNRECOVERABLE_ERROR); } @@ -228,10 +225,8 @@ TEST_F(SyncModelAssociationManagerTest, InitializeAbortsLoad) { new FakeDataTypeController(syncer::THEMES); GetController(controllers_, syncer::BOOKMARKS)->SetDelayModelLoad(); - ModelAssociationManager model_association_manager( - syncer::WeakHandle(), - &controllers_, - &result_processor_); + ModelAssociationManager model_association_manager(&controllers_, + &result_processor_); syncer::ModelTypeSet types(syncer::BOOKMARKS, syncer::THEMES); syncer::ModelTypeSet expected_types_waiting_to_load; @@ -245,7 +240,7 @@ TEST_F(SyncModelAssociationManagerTest, InitializeAbortsLoad) { model_association_manager.Initialize(types); model_association_manager.StopDisabledTypes(); - model_association_manager.StartAssociationAsync(); + model_association_manager.StartAssociationAsync(types); EXPECT_CALL(result_processor_, OnModelAssociationDone(_)). WillOnce(VerifyResult(expected_result_partially_done)); @@ -285,7 +280,7 @@ TEST_F(SyncModelAssociationManagerTest, InitializeAbortsLoad) { WillOnce(VerifyResult(expected_result_done)); model_association_manager.StopDisabledTypes(); - model_association_manager.StartAssociationAsync(); + model_association_manager.StartAssociationAsync(types); GetController(controllers_, syncer::BOOKMARKS)->SimulateModelLoadFinishing(); @@ -301,10 +296,8 @@ TEST_F(SyncModelAssociationManagerTest, ModelStartWithSlowLoadingType) { controllers_[syncer::APPS] = new FakeDataTypeController(syncer::APPS); GetController(controllers_, syncer::BOOKMARKS)->SetDelayModelLoad(); - ModelAssociationManager model_association_manager( - syncer::WeakHandle(), - &controllers_, - &result_processor_); + ModelAssociationManager model_association_manager(&controllers_, + &result_processor_); syncer::ModelTypeSet types; types.Put(syncer::BOOKMARKS); types.Put(syncer::APPS); @@ -329,7 +322,7 @@ TEST_F(SyncModelAssociationManagerTest, ModelStartWithSlowLoadingType) { model_association_manager.Initialize(types); model_association_manager.StopDisabledTypes(); - model_association_manager.StartAssociationAsync(); + model_association_manager.StartAssociationAsync(types); base::OneShotTimer* timer = model_association_manager.GetTimerForTesting(); @@ -354,7 +347,7 @@ TEST_F(SyncModelAssociationManagerTest, ModelStartWithSlowLoadingType) { // Do it once more to associate bookmarks. model_association_manager.Initialize(types); model_association_manager.StopDisabledTypes(); - model_association_manager.StartAssociationAsync(); + model_association_manager.StartAssociationAsync(types); GetController(controllers_, syncer::BOOKMARKS)->SimulateModelLoadFinishing(); @@ -363,5 +356,60 @@ TEST_F(SyncModelAssociationManagerTest, ModelStartWithSlowLoadingType) { DataTypeController::OK); } +TEST_F(SyncModelAssociationManagerTest, StartMultipleTimes) { + controllers_[syncer::BOOKMARKS] = + new FakeDataTypeController(syncer::BOOKMARKS); + controllers_[syncer::APPS] = + new FakeDataTypeController(syncer::APPS); + ModelAssociationManager model_association_manager(&controllers_, + &result_processor_); + syncer::ModelTypeSet types; + types.Put(syncer::BOOKMARKS); + types.Put(syncer::APPS); + + DataTypeManager::ConfigureResult result_1st( + DataTypeManager::OK, + syncer::ModelTypeSet(syncer::BOOKMARKS), + std::list(), + syncer::ModelTypeSet()); + DataTypeManager::ConfigureResult result_2nd( + DataTypeManager::OK, + syncer::ModelTypeSet(syncer::APPS), + std::list(), + syncer::ModelTypeSet()); + EXPECT_CALL(result_processor_, OnModelAssociationDone(_)). + Times(2). + WillOnce(VerifyResult(result_1st)). + WillOnce(VerifyResult(result_2nd)); + + model_association_manager.Initialize(types); + model_association_manager.StopDisabledTypes(); + + // Start BOOKMARKS first. + model_association_manager.StartAssociationAsync( + syncer::ModelTypeSet(syncer::BOOKMARKS)); + EXPECT_EQ(GetController(controllers_, syncer::BOOKMARKS)->state(), + DataTypeController::ASSOCIATING); + EXPECT_EQ(GetController(controllers_, syncer::APPS)->state(), + DataTypeController::NOT_RUNNING); + + // Finish BOOKMARKS association. + GetController(controllers_, syncer::BOOKMARKS)->FinishStart( + DataTypeController::OK); + EXPECT_EQ(GetController(controllers_, syncer::BOOKMARKS)->state(), + DataTypeController::RUNNING); + EXPECT_EQ(GetController(controllers_, syncer::APPS)->state(), + DataTypeController::NOT_RUNNING); + + // Start APPS next. + model_association_manager.StartAssociationAsync( + syncer::ModelTypeSet(syncer::APPS)); + EXPECT_EQ(GetController(controllers_, syncer::APPS)->state(), + DataTypeController::ASSOCIATING); + GetController(controllers_, syncer::APPS)->FinishStart( + DataTypeController::OK); + EXPECT_EQ(GetController(controllers_, syncer::APPS)->state(), + DataTypeController::RUNNING); +} } // namespace browser_sync diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc index 15e7967..badc896 100644 --- a/chrome/browser/sync/glue/sync_backend_host.cc +++ b/chrome/browser/sync/glue/sync_backend_host.cc @@ -704,7 +704,7 @@ void SyncBackendHost::ConfigureDataTypes( // until they succeed or the backend is shut down. syncer::ModelTypeSet types_to_download = registrar_->ConfigureDataTypes( - GetDataTypesInState(ENABLED, config_state_map), + GetDataTypesInState(CONFIGURE_ACTIVE, config_state_map), syncer::Union(GetDataTypesInState(DISABLED, config_state_map), GetDataTypesInState(FAILED, config_state_map))); types_to_download.RemoveAll(syncer::ProxyTypes()); diff --git a/chrome/browser/sync/glue/sync_backend_host_unittest.cc b/chrome/browser/sync/glue/sync_backend_host_unittest.cc index 4474035..da3cc97 100644 --- a/chrome/browser/sync/glue/sync_backend_host_unittest.cc +++ b/chrome/browser/sync/glue/sync_backend_host_unittest.cc @@ -215,7 +215,9 @@ class SyncBackendHostTest : public testing::Test { syncer::ModelTypeSet types_to_remove) { BackendDataTypeConfigurer::DataTypeConfigStateMap config_state_map; BackendDataTypeConfigurer::SetDataTypesState( - BackendDataTypeConfigurer::ENABLED, types_to_add, &config_state_map); + BackendDataTypeConfigurer::CONFIGURE_ACTIVE, + types_to_add, + &config_state_map); BackendDataTypeConfigurer::SetDataTypesState( BackendDataTypeConfigurer::DISABLED, types_to_remove, &config_state_map); diff --git a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc index 8ffcbb0..f0ada06 100644 --- a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc @@ -49,6 +49,7 @@ #include "content/public/test/test_browser_thread.h" #include "google_apis/gaia/gaia_constants.h" #include "sync/internal_api/public/base/model_type.h" +#include "sync/internal_api/public/data_type_debug_info_listener.h" #include "sync/internal_api/public/read_node.h" #include "sync/internal_api/public/read_transaction.h" #include "sync/internal_api/public/write_node.h" @@ -473,9 +474,9 @@ class ProfileSyncServiceAutofillTest public syncer::DataTypeDebugInfoListener { public: // DataTypeDebugInfoListener implementation. - virtual void OnDataTypeAssociationComplete( - const syncer::DataTypeAssociationStats& association_stats) OVERRIDE { - association_stats_ = association_stats; + virtual void OnSingleDataTypeConfigureComplete( + const syncer::DataTypeConfigurationStats& configuration_stats) OVERRIDE { + association_stats_ = configuration_stats.association_stats; } virtual void OnConfigureComplete() OVERRIDE { // Do nothing. diff --git a/chrome/browser/sync/profile_sync_service_preference_unittest.cc b/chrome/browser/sync/profile_sync_service_preference_unittest.cc index ce02d29..ade9e40 100644 --- a/chrome/browser/sync/profile_sync_service_preference_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_preference_unittest.cc @@ -33,6 +33,7 @@ #include "sync/api/sync_data.h" #include "sync/internal_api/public/base/model_type.h" #include "sync/internal_api/public/change_record.h" +#include "sync/internal_api/public/data_type_debug_info_listener.h" #include "sync/internal_api/public/read_node.h" #include "sync/internal_api/public/read_transaction.h" #include "sync/internal_api/public/write_node.h" @@ -100,9 +101,9 @@ class ProfileSyncServicePreferenceTest } // DataTypeDebugInfoListener implementation. - virtual void OnDataTypeAssociationComplete( - const syncer::DataTypeAssociationStats& association_stats) OVERRIDE { - association_stats_ = association_stats; + virtual void OnSingleDataTypeConfigureComplete( + const syncer::DataTypeConfigurationStats& configuration_stats) OVERRIDE { + association_stats_ = configuration_stats.association_stats; } virtual void OnConfigureComplete() OVERRIDE { // Do nothing. -- cgit v1.1