diff options
author | kkania@chromium.org <kkania@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-06-18 20:36:03 +0000 |
---|---|---|
committer | kkania@chromium.org <kkania@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-06-18 20:36:03 +0000 |
commit | b07bbdd73a0bbe6d8e1423a8a003de8ee90c4c7b (patch) | |
tree | ac6566a55cd975b331e6382a744fcf8149b17c8e /chrome/browser/sync | |
parent | 2a46e14ef064c6556f077cf214759adc4b235f04 (diff) | |
download | chromium_src-b07bbdd73a0bbe6d8e1423a8a003de8ee90c4c7b.zip chromium_src-b07bbdd73a0bbe6d8e1423a8a003de8ee90c4c7b.tar.gz chromium_src-b07bbdd73a0bbe6d8e1423a8a003de8ee90c4c7b.tar.bz2 |
Revert 142794 - Revert 142517 - [Sync] Refactor sync configuration logic.
We remove all the pending download/configure state in SBH, in addition
to the split transaction nature of configurations themselves. This allows
us to have a single SyncScheduler::ScheduleConfiguration command that is
both synchronous (assuming it doesn't fail) and can handle
CleanupDisabledTypes and GetKey commands.
This also now keys which datatypes need downloading by checking the
initial sync ended bits directly. This allows us to recover from a new
sync db gracefully.
BUG=129665,133061,129825
TEST=unit/integration tests
Review URL: https://chromiumcodereview.appspot.com/10483015
TBR=zea@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10536194
TBR=zea@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10581005
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@142798 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/sync')
-rw-r--r-- | chrome/browser/sync/glue/backend_data_type_configurer.h | 8 | ||||
-rw-r--r-- | chrome/browser/sync/glue/data_type_manager_impl_unittest.cc | 8 | ||||
-rw-r--r-- | chrome/browser/sync/glue/sync_backend_host.cc | 409 | ||||
-rw-r--r-- | chrome/browser/sync/glue/sync_backend_host.h | 78 | ||||
-rw-r--r-- | chrome/browser/sync/test_profile_sync_service.cc | 38 | ||||
-rw-r--r-- | chrome/browser/sync/test_profile_sync_service.h | 11 |
6 files changed, 228 insertions, 324 deletions
diff --git a/chrome/browser/sync/glue/backend_data_type_configurer.h b/chrome/browser/sync/glue/backend_data_type_configurer.h index b77ad51..e7bf098 100644 --- a/chrome/browser/sync/glue/backend_data_type_configurer.h +++ b/chrome/browser/sync/glue/backend_data_type_configurer.h @@ -34,11 +34,11 @@ class BackendDataTypeConfigurer { // Nigori. virtual void ConfigureDataTypes( sync_api::ConfigureReason reason, - syncable::ModelTypeSet types_to_add, - syncable::ModelTypeSet types_to_remove, + const syncable::ModelTypeSet& types_to_add, + const syncable::ModelTypeSet& types_to_remove, NigoriState nigori_state, - base::Callback<void(syncable::ModelTypeSet)> ready_task, - base::Callback<void()> retry_callback) = 0; + const base::Callback<void(syncable::ModelTypeSet)>& ready_task, + const base::Callback<void()>& retry_callback) = 0; protected: virtual ~BackendDataTypeConfigurer() {} 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 491f0c3..440ce24 100644 --- a/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc +++ b/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc @@ -42,11 +42,11 @@ class FakeBackendDataTypeConfigurer : public BackendDataTypeConfigurer { virtual void ConfigureDataTypes( sync_api::ConfigureReason reason, - ModelTypeSet types_to_add, - ModelTypeSet types_to_remove, + const ModelTypeSet& types_to_add, + const ModelTypeSet& types_to_remove, NigoriState nigori_state, - base::Callback<void(ModelTypeSet)> ready_task, - base::Callback<void()> retry_callback) OVERRIDE { + const base::Callback<void(ModelTypeSet)>& ready_task, + const base::Callback<void()>& retry_callback) OVERRIDE { last_nigori_state_ = nigori_state; last_ready_task_ = ready_task; } diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc index c4c6e31..58e7a37 100644 --- a/chrome/browser/sync/glue/sync_backend_host.cc +++ b/chrome/browser/sync/glue/sync_backend_host.cc @@ -131,10 +131,6 @@ class SyncBackendHost::Core // Called to clear server data. void DoRequestClearServerData(); - // Called to cleanup disabled types. - void DoRequestCleanupDisabledTypes( - const browser_sync::ModelSafeRoutingInfo& routing_info); - // Called to set the passphrase for encryption. void DoSetEncryptionPassphrase(const std::string& passphrase, bool is_explicit); @@ -164,14 +160,18 @@ class SyncBackendHost::Core void DoStopSyncManagerForShutdown(const base::Closure& closure); void DoShutdown(bool stopping_sync); - virtual void DoRequestConfig( - const browser_sync::ModelSafeRoutingInfo& routing_info, + // Configuration methods that must execute on sync loop. + void DoConfigureSyncer( + sync_api::ConfigureReason reason, syncable::ModelTypeSet types_to_config, - sync_api::ConfigureReason reason); - - // Start the configuration mode. |callback| is called on the sync - // thread. - virtual void DoStartConfiguration(const base::Closure& callback); + const browser_sync::ModelSafeRoutingInfo routing_info, + const base::Callback<void(syncable::ModelTypeSet)>& ready_task, + const base::Closure& retry_callback); + void DoFinishConfigureDataTypes( + syncable::ModelTypeSet types_to_config, + const base::Callback<void(syncable::ModelTypeSet)>& ready_task); + void DoRetryConfiguration( + const base::Closure& retry_callback); // Set the base request context to use when making HTTP calls. // This method will add a reference to the context to persist it @@ -184,9 +184,6 @@ class SyncBackendHost::Core // sync databases), as well as shutdown when you're no longer syncing. void DeleteSyncDataFolder(); - // A callback from the SyncerThread when it is safe to continue config. - void FinishConfigureDataTypes(); - private: friend class base::RefCountedThreadSafe<SyncBackendHost::Core>; friend class SyncBackendHostForProfileSyncTest; @@ -556,60 +553,79 @@ void SyncBackendHost::Shutdown(bool sync_disabled) { core_ = NULL; // Releases reference to core_. } +// TODO(sync): remove |disabled_types| completely, as it's just the set of +// registered types - the set of desired types. void SyncBackendHost::ConfigureDataTypes( sync_api::ConfigureReason reason, - syncable::ModelTypeSet types_to_add, - syncable::ModelTypeSet types_to_remove, + const syncable::ModelTypeSet& desired_types, + const syncable::ModelTypeSet& disabled_types, NigoriState nigori_state, - base::Callback<void(syncable::ModelTypeSet)> ready_task, - base::Callback<void()> retry_callback) { - syncable::ModelTypeSet types_to_add_with_nigori = types_to_add; - syncable::ModelTypeSet types_to_remove_with_nigori = types_to_remove; + const base::Callback<void(syncable::ModelTypeSet)>& ready_task, + const base::Callback<void()>& retry_callback) { + syncable::ModelTypeSet desired_types_with_nigori = desired_types; + syncable::ModelTypeSet disabled_types_with_nigori = disabled_types; if (nigori_state == WITH_NIGORI) { - types_to_add_with_nigori.Put(syncable::NIGORI); - types_to_remove_with_nigori.Remove(syncable::NIGORI); + desired_types_with_nigori.Put(syncable::NIGORI); + disabled_types_with_nigori.Remove(syncable::NIGORI); } else { - types_to_add_with_nigori.Remove(syncable::NIGORI); - types_to_remove_with_nigori.Put(syncable::NIGORI); + desired_types_with_nigori.Remove(syncable::NIGORI); + disabled_types_with_nigori.Put(syncable::NIGORI); } - // Only one configure is allowed at a time. - DCHECK(!pending_config_mode_state_.get()); - DCHECK(!pending_download_state_.get()); + // Only one configure is allowed at a time (DataTypeManager handles user + // changes that happen while the syncer is reconfiguraing, and will only + // trigger another call to ConfigureDataTypes once the current reconfiguration + // completes). DCHECK_GT(initialization_state_, NOT_INITIALIZED); - pending_config_mode_state_.reset(new PendingConfigureDataTypesState()); - pending_config_mode_state_->ready_task = ready_task; - pending_config_mode_state_->types_to_add = types_to_add_with_nigori; - pending_config_mode_state_->added_types = - registrar_->ConfigureDataTypes(types_to_add_with_nigori, - types_to_remove_with_nigori); - pending_config_mode_state_->reason = reason; - pending_config_mode_state_->retry_callback = retry_callback; - - // Cleanup disabled types before starting configuration so that - // callers can assume that the data types are cleaned up once - // configuration is done. - if (!types_to_remove_with_nigori.Empty()) { - ModelSafeRoutingInfo routing_info; - registrar_->GetModelSafeRoutingInfo(&routing_info); - sync_thread_.message_loop()->PostTask( - FROM_HERE, - base::Bind(&SyncBackendHost::Core::DoRequestCleanupDisabledTypes, - core_.get(), - routing_info)); - } + // enabled_types \setunion disabled_types gives the set of all registered + // types (i.e. types with datatype controllers registered). + ModelSafeRoutingInfo routing_info; + registrar_->ConfigureDataTypes(desired_types_with_nigori, + disabled_types_with_nigori); + registrar_->GetModelSafeRoutingInfo(&routing_info); - StartConfiguration( - base::Bind(&SyncBackendHost::Core::FinishConfigureDataTypes, - core_.get())); -} + // |enabled_types| is desired_types_with_nigori \setintersect types with + // model safe workers (e.g. the password store may fail to start up, in which + // case we have no model safe worker for it and should not enable it). + // TODO(sync): This shouldn't be necessary. We should do whatever filtering + // we need beforehand as part of ProfileSyncComponentsFactory's + // RegisterDataTypes. + const syncable::ModelTypeSet enabled_types = + GetRoutingInfoTypes(routing_info); -void SyncBackendHost::StartConfiguration(const base::Closure& callback) { - // Put syncer in the config mode. DTM will put us in normal mode once it is - // done. This is to ensure we dont do a normal sync when we are doing model - // association. - sync_thread_.message_loop()->PostTask(FROM_HERE, base::Bind( - &SyncBackendHost::Core::DoStartConfiguration, core_.get(), callback)); + // Figure out which types need to actually be downloaded. We pass those on + // to the syncer while it's in configuration mode so that they can be + // downloaded before we perform association. Once we switch to normal mode + // downloads will get applied normally and hit the datatype's change + // processor. + // A datatype is in need of downloading if any of the following are true: + // 1. it's enabled and initial_sync_ended is false (initial_sync_ended is + // set after applying updates, and hence is a more conservative measure + // than having a non-empty progress marker, which is set during + // StoreTimestamps). + // 2. the type is NIGORI, and any other datatype is being downloaded (nigori + // is always included if we download a datatype). + // TODO(sync): consider moving this logic onto the sync thread (perhaps + // as part of SyncManager::ConfigureSyncer). + syncable::ModelTypeSet initial_sync_ended_types = + core_->sync_manager()->InitialSyncEndedTypes(); + initial_sync_ended_types.RetainAll(enabled_types); + syncable::ModelTypeSet types_to_config = + Difference(enabled_types, initial_sync_ended_types); + if (!types_to_config.Empty() && enabled_types.Has(syncable::NIGORI)) + types_to_config.Put(syncable::NIGORI); + + SDVLOG(1) << "Types " + << syncable::ModelTypeSetToString(types_to_config) + << " added; calling DoConfigureSyncer"; + // TODO(zea): figure out how to bypass this call if no types are being + // configured and GetKey is not needed. For now we rely on determining the + // need for GetKey as part of the SyncManager::ConfigureSyncer logic. + RequestConfigureSyncer(reason, + types_to_config, + routing_info, + ready_task, + retry_callback); } void SyncBackendHost::EnableEncryptEverything() { @@ -687,157 +703,47 @@ void SyncBackendHost::InitCore(const DoInitializeOptions& options) { base::Bind(&SyncBackendHost::Core::DoInitialize, core_.get(), options)); } -void SyncBackendHost::HandleSyncCycleCompletedOnFrontendLoop( - const SyncSessionSnapshot& snapshot) { - if (!frontend_) - return; - DCHECK_EQ(MessageLoop::current(), frontend_loop_); - - last_snapshot_ = snapshot; - - SDVLOG(1) << "Got snapshot " << snapshot.ToString(); - - const syncable::ModelTypeSet to_migrate = - snapshot.syncer_status().types_needing_local_migration; - if (!to_migrate.Empty()) - frontend_->OnMigrationNeededForTypes(to_migrate); - - // Process any changes to the datatypes we're syncing. - // TODO(sync): add support for removing types. - if (initialized()) - AddExperimentalTypes(); - - // If we are waiting for a configuration change, check here to see - // if this sync cycle has initialized all of the types we've been - // waiting for. - if (pending_download_state_.get()) { - const syncable::ModelTypeSet types_to_add = - pending_download_state_->types_to_add; - const syncable::ModelTypeSet added_types = - pending_download_state_->added_types; - DCHECK(types_to_add.HasAll(added_types)); - const syncable::ModelTypeSet initial_sync_ended = - snapshot.initial_sync_ended(); - const syncable::ModelTypeSet failed_configuration_types = - Difference(added_types, initial_sync_ended); - SDVLOG(1) - << "Added types: " - << syncable::ModelTypeSetToString(added_types) - << ", configured types: " - << syncable::ModelTypeSetToString(initial_sync_ended) - << ", failed configuration types: " - << syncable::ModelTypeSetToString(failed_configuration_types); - - if (!failed_configuration_types.Empty() && - snapshot.retry_scheduled()) { - // Inform the caller that download failed but we are retrying. - if (!pending_download_state_->retry_in_progress) { - pending_download_state_->retry_callback.Run(); - pending_download_state_->retry_in_progress = true; - } - // Nothing more to do. - return; - } - - scoped_ptr<PendingConfigureDataTypesState> state( - pending_download_state_.release()); - state->ready_task.Run(failed_configuration_types); - - // Syncer did not report an error but did not download everything - // we requested either. So abort. The caller of the config will cleanup. - if (!failed_configuration_types.Empty()) - return; - } - - if (initialized()) - frontend_->OnSyncCycleCompleted(); -} - -void SyncBackendHost::FinishConfigureDataTypesOnFrontendLoop() { - DCHECK_EQ(MessageLoop::current(), frontend_loop_); - // Nudge the syncer. This is necessary for both datatype addition/deletion. - // - // Deletions need a nudge in order to ensure the deletion occurs in a timely - // manner (see issue 56416). - // - // In the case of additions, on the next sync cycle, the syncer should - // notice that the routing info has changed and start the process of - // downloading updates for newly added data types. Once this is - // complete, the configure_state_.ready_task_ is run via an - // OnInitializationComplete notification. - - SDVLOG(1) << "Syncer in config mode. SBH executing " - << "FinishConfigureDataTypesOnFrontendLoop"; - - - ModelSafeRoutingInfo routing_info; - registrar_->GetModelSafeRoutingInfo(&routing_info); - const syncable::ModelTypeSet enabled_types = - GetRoutingInfoTypes(routing_info); +void SyncBackendHost::RequestConfigureSyncer( + sync_api::ConfigureReason reason, + syncable::ModelTypeSet types_to_config, + const browser_sync::ModelSafeRoutingInfo& routing_info, + const base::Callback<void(syncable::ModelTypeSet)>& ready_task, + const base::Closure& retry_callback) { + sync_thread_.message_loop()->PostTask(FROM_HERE, + base::Bind(&SyncBackendHost::Core::DoConfigureSyncer, + core_.get(), + reason, + types_to_config, + routing_info, + ready_task, + retry_callback)); +} + +void SyncBackendHost::FinishConfigureDataTypesOnFrontendLoop( + const syncable::ModelTypeSet& types_to_configure, + const syncable::ModelTypeSet& configured_types, + const base::Callback<void(syncable::ModelTypeSet)>& ready_task) { + const syncable::ModelTypeSet failed_configuration_types = + Difference(types_to_configure, configured_types); + SDVLOG(1) + << "Added types: " + << syncable::ModelTypeSetToString(types_to_configure) + << ", configured types: " + << syncable::ModelTypeSetToString(configured_types) + << ", failed configuration types: " + << syncable::ModelTypeSetToString(failed_configuration_types); // Update |chrome_sync_notification_bridge_|'s enabled types here as it has // to happen on the UI thread. - chrome_sync_notification_bridge_.UpdateEnabledTypes(enabled_types); - - if (pending_config_mode_state_->added_types.Empty() && - !core_->sync_manager()->InitialSyncEndedTypes().HasAll(enabled_types)) { - - // TODO(tim): Log / UMA / count this somehow? - // Add only the types with empty progress markers. Note: it is possible - // that some types have their initial_sync_ended be false but with non - // empty progress marker. Which is ok as the rest of the changes would - // be downloaded on a regular nudge and initial_sync_ended should be set - // to true. However this is a very corner case. So it is not explicitly - // handled. - pending_config_mode_state_->added_types = - sync_api::GetTypesWithEmptyProgressMarkerToken(enabled_types, - GetUserShare()); - } - - // If we've added types, we always want to request a nudge/config (even if - // the initial sync is ended), in case we could not decrypt the data. - if (pending_config_mode_state_->added_types.Empty()) { - SDVLOG(1) << "No new types added; calling ready_task directly"; - // No new types - just notify the caller that the types are available. - const syncable::ModelTypeSet failed_configuration_types; - pending_config_mode_state_->ready_task.Run(failed_configuration_types); - } else { - pending_download_state_.reset(pending_config_mode_state_.release()); - - // Always configure nigori if it's enabled. - syncable::ModelTypeSet types_to_config = - pending_download_state_->added_types; - if (IsNigoriEnabled()) { - // Note: Nigori is the only type that gets added with a nonempty - // progress marker during config. If the server returns a migration - // error then we will go into unrecoverable error. We dont handle it - // explicitly because server might help us out here by not sending a - // migraiton error for nigori during config. - types_to_config.Put(syncable::NIGORI); - } - SDVLOG(1) << "Types " - << syncable::ModelTypeSetToString(types_to_config) - << " added; calling DoRequestConfig"; - ModelSafeRoutingInfo routing_info; - registrar_->GetModelSafeRoutingInfo(&routing_info); - sync_thread_.message_loop()->PostTask(FROM_HERE, - base::Bind(&SyncBackendHost::Core::DoRequestConfig, - core_.get(), - routing_info, - types_to_config, - pending_download_state_->reason)); - } - - pending_config_mode_state_.reset(); + chrome_sync_notification_bridge_.UpdateEnabledTypes(configured_types); // Notify SyncManager (especially the notification listener) about new types. sync_thread_.message_loop()->PostTask(FROM_HERE, base::Bind(&SyncBackendHost::Core::DoUpdateEnabledTypes, core_.get(), - enabled_types)); -} + configured_types)); -bool SyncBackendHost::IsDownloadingNigoriForTest() const { - return initialization_state_ == DOWNLOADING_NIGORI; + if (!ready_task.is_null()) + ready_task.Run(failed_configuration_types); } SyncBackendHost::DoInitializeOptions::DoInitializeOptions( @@ -894,14 +800,6 @@ SyncBackendHost::Core::~Core() { DCHECK(!sync_loop_); } -SyncBackendHost::PendingConfigureDataTypesState:: -PendingConfigureDataTypesState() - : reason(sync_api::CONFIGURE_REASON_UNKNOWN), - retry_in_progress(false) {} - -SyncBackendHost::PendingConfigureDataTypesState:: -~PendingConfigureDataTypesState() {} - void SyncBackendHost::Core::OnSyncCycleCompleted( const SyncSessionSnapshot& snapshot) { if (!sync_loop_) @@ -1153,12 +1051,6 @@ void SyncBackendHost::Core::DoRequestClearServerData() { sync_manager_->RequestClearServerData(); } -void SyncBackendHost::Core::DoRequestCleanupDisabledTypes( - const browser_sync::ModelSafeRoutingInfo& routing_info) { - DCHECK_EQ(MessageLoop::current(), sync_loop_); - sync_manager_->RequestCleanupDisabledTypes(routing_info); -} - void SyncBackendHost::Core::DoSetEncryptionPassphrase( const std::string& passphrase, bool is_explicit) { @@ -1210,18 +1102,46 @@ void SyncBackendHost::Core::DoShutdown(bool sync_disabled) { host_.Reset(); } -void SyncBackendHost::Core::DoRequestConfig( - const browser_sync::ModelSafeRoutingInfo& routing_info, +void SyncBackendHost::Core::DoConfigureSyncer( + sync_api::ConfigureReason reason, syncable::ModelTypeSet types_to_config, - sync_api::ConfigureReason reason) { + const browser_sync::ModelSafeRoutingInfo routing_info, + const base::Callback<void(syncable::ModelTypeSet)>& ready_task, + const base::Closure& retry_callback) { DCHECK_EQ(MessageLoop::current(), sync_loop_); - sync_manager_->RequestConfig(routing_info, types_to_config, reason); + sync_manager_->ConfigureSyncer( + reason, + types_to_config, + routing_info, + base::Bind(&SyncBackendHost::Core::DoFinishConfigureDataTypes, + this, + types_to_config, + ready_task), + base::Bind(&SyncBackendHost::Core::DoRetryConfiguration, + this, + retry_callback)); } -void SyncBackendHost::Core::DoStartConfiguration( - const base::Closure& callback) { +void SyncBackendHost::Core::DoFinishConfigureDataTypes( + syncable::ModelTypeSet types_to_config, + const base::Callback<void(syncable::ModelTypeSet)>& ready_task) { + DCHECK_EQ(MessageLoop::current(), sync_loop_); + syncable::ModelTypeSet configured_types = + sync_manager_->InitialSyncEndedTypes(); + configured_types.RetainAll(types_to_config); + host_.Call(FROM_HERE, + &SyncBackendHost::FinishConfigureDataTypesOnFrontendLoop, + types_to_config, + configured_types, + ready_task); +} + +void SyncBackendHost::Core::DoRetryConfiguration( + const base::Closure& retry_callback) { DCHECK_EQ(MessageLoop::current(), sync_loop_); - sync_manager_->StartConfigurationMode(callback); + host_.Call(FROM_HERE, + &SyncBackendHost::RetryConfigurationOnFrontendLoop, + retry_callback); } void SyncBackendHost::Core::DeleteSyncDataFolder() { @@ -1232,13 +1152,6 @@ void SyncBackendHost::Core::DeleteSyncDataFolder() { } } -void SyncBackendHost::Core::FinishConfigureDataTypes() { - DCHECK_EQ(MessageLoop::current(), sync_loop_); - host_.Call( - FROM_HERE, - &SyncBackendHost::FinishConfigureDataTypesOnFrontendLoop); -} - void SyncBackendHost::Core::StartSavingChanges() { // We may already be shut down. if (!sync_loop_) @@ -1289,14 +1202,6 @@ void SyncBackendHost::HandleInitializationCompletedOnFrontendLoop( return; } - // If setup has completed, start off in DOWNLOADING_NIGORI so that - // we start off by refreshing nigori. - CHECK(sync_prefs_.get()); - if (sync_prefs_->HasSyncSetupCompleted() && - initialization_state_ < DOWNLOADING_NIGORI) { - initialization_state_ = DOWNLOADING_NIGORI; - } - // Run initialization state machine. switch (initialization_state_) { case NOT_INITIALIZED: @@ -1337,6 +1242,36 @@ void SyncBackendHost::HandleInitializationCompletedOnFrontendLoop( } } +void SyncBackendHost::HandleSyncCycleCompletedOnFrontendLoop( + const SyncSessionSnapshot& snapshot) { + if (!frontend_) + return; + DCHECK_EQ(MessageLoop::current(), frontend_loop_); + + last_snapshot_ = snapshot; + + SDVLOG(1) << "Got snapshot " << snapshot.ToString(); + + const syncable::ModelTypeSet to_migrate = + snapshot.syncer_status().types_needing_local_migration; + if (!to_migrate.Empty()) + frontend_->OnMigrationNeededForTypes(to_migrate); + + // Process any changes to the datatypes we're syncing. + // TODO(sync): add support for removing types. + if (initialized()) + AddExperimentalTypes(); + + if (initialized()) + frontend_->OnSyncCycleCompleted(); +} + +void SyncBackendHost::RetryConfigurationOnFrontendLoop( + const base::Closure& retry_callback) { + SDVLOG(1) << "Failed to complete configuration, informing of retry."; + retry_callback.Run(); +} + void SyncBackendHost::PersistEncryptionBootstrapToken( const std::string& token) { CHECK(sync_prefs_.get()); diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h index 5571207..5cde824 100644 --- a/chrome/browser/sync/glue/sync_backend_host.h +++ b/chrome/browser/sync/glue/sync_backend_host.h @@ -226,15 +226,11 @@ class SyncBackendHost : public BackendDataTypeConfigurer { // is non-empty, then an error was encountered). virtual void ConfigureDataTypes( sync_api::ConfigureReason reason, - syncable::ModelTypeSet types_to_add, - syncable::ModelTypeSet types_to_remove, + const syncable::ModelTypeSet& desired_types, + const syncable::ModelTypeSet& disabled_types, NigoriState nigori_state, - base::Callback<void(syncable::ModelTypeSet)> ready_task, - base::Callback<void()> retry_callback) OVERRIDE; - - // Makes an asynchronous call to syncer to switch to config mode. When done - // syncer will call us back on FinishConfigureDataTypes. - virtual void StartConfiguration(const base::Closure& callback); + const base::Callback<void(syncable::ModelTypeSet)>& ready_task, + const base::Callback<void()>& retry_callback) OVERRIDE; // Turns on encryption of all present and future sync data. virtual void EnableEncryptEverything(); @@ -333,16 +329,20 @@ class SyncBackendHost : public BackendDataTypeConfigurer { // Allows tests to perform alternate core initialization work. virtual void InitCore(const DoInitializeOptions& options); - // Called from Core::OnSyncCycleCompleted to handle updating frontend - // thread components. - void HandleSyncCycleCompletedOnFrontendLoop( - const sessions::SyncSessionSnapshot& snapshot); - - // Called to finish the job of ConfigureDataTypes once the syncer is in - // configuration mode. - void FinishConfigureDataTypesOnFrontendLoop(); + // Request the syncer to reconfigure with the specfied params. + // Virtual for testing. + virtual void RequestConfigureSyncer( + sync_api::ConfigureReason reason, + syncable::ModelTypeSet types_to_config, + const browser_sync::ModelSafeRoutingInfo& routing_info, + const base::Callback<void(syncable::ModelTypeSet)>& ready_task, + const base::Closure& retry_callback); - bool IsDownloadingNigoriForTest() const; + // Called when the syncer has finished performing a configuration. + void FinishConfigureDataTypesOnFrontendLoop( + const syncable::ModelTypeSet& types_to_configure, + const syncable::ModelTypeSet& configured_types, + const base::Callback<void(syncable::ModelTypeSet)>& ready_task); private: // The real guts of SyncBackendHost, to keep the public client API clean. @@ -365,47 +365,32 @@ class SyncBackendHost : public BackendDataTypeConfigurer { INITIALIZED, // Initialization is complete. }; - struct PendingConfigureDataTypesState { - PendingConfigureDataTypesState(); - ~PendingConfigureDataTypesState(); - - // The ready_task will be run when configuration is done with the - // set of all types that failed configuration (i.e., if its - // argument is non-empty, then an error was encountered). - base::Callback<void(syncable::ModelTypeSet)> ready_task; - - // The retry callback will be run when the download failed due to a - // transient error. This is to notify DTM so it can apropriately inform - // the UI. Note: The retry_callback will be run only once and after - // that we will not notify DTM until the sync is successful or in a - // permanent error state. - base::Callback<void()> retry_callback; - - // The set of types that we are waiting to be initially synced in a - // configuration cycle. - syncable::ModelTypeSet types_to_add; - - // Additional details about which types were added. - syncable::ModelTypeSet added_types; - sync_api::ConfigureReason reason; - bool retry_in_progress; - }; - // Checks if we have received a notice to turn on experimental datatypes // (via the nigori node) and informs the frontend if that is the case. // Note: it is illegal to call this before the backend is initialized. void AddExperimentalTypes(); // Downloading of nigori failed and will be retried. - virtual void OnNigoriDownloadRetry(); + void OnNigoriDownloadRetry(); // InitializationComplete passes through the SyncBackendHost to forward // on to |frontend_|, and so that tests can intercept here if they need to // set up initial conditions. - virtual void HandleInitializationCompletedOnFrontendLoop( + void HandleInitializationCompletedOnFrontendLoop( const WeakHandle<JsBackend>& js_backend, bool success); + // Called from Core::OnSyncCycleCompleted to handle updating frontend + // thread components. + void HandleSyncCycleCompletedOnFrontendLoop( + const sessions::SyncSessionSnapshot& snapshot); + + // Called when the syncer failed to perform a configuration and will + // eventually retry. FinishingConfigurationOnFrontendLoop(..) will be called + // on successful completion. + void RetryConfigurationOnFrontendLoop( + const base::Closure& retry_callback); + // Helpers to persist a token that can be used to bootstrap sync encryption // across browser restart to avoid requiring the user to re-enter their // passphrase. |token| must be valid UTF-8 as we use the PrefService for @@ -509,9 +494,6 @@ class SyncBackendHost : public BackendDataTypeConfigurer { // The frontend which we serve (and are owned by). SyncFrontend* frontend_; - scoped_ptr<PendingConfigureDataTypesState> pending_download_state_; - scoped_ptr<PendingConfigureDataTypesState> pending_config_mode_state_; - // We cache the cryptographer's pending keys whenever NotifyPassphraseRequired // is called. This way, before the UI calls SetDecryptionPassphrase on the // syncer, it can avoid the overhead of an asynchronous decryption call and diff --git a/chrome/browser/sync/test_profile_sync_service.cc b/chrome/browser/sync/test_profile_sync_service.cc index 2eb2872..9c0f3cf 100644 --- a/chrome/browser/sync/test_profile_sync_service.cc +++ b/chrome/browser/sync/test_profile_sync_service.cc @@ -44,19 +44,6 @@ SyncBackendHostForProfileSyncTest::SyncBackendHostForProfileSyncTest( SyncBackendHostForProfileSyncTest::~SyncBackendHostForProfileSyncTest() {} -void SyncBackendHostForProfileSyncTest:: - SimulateSyncCycleCompletedInitialSyncEnded( - const tracked_objects::Location& location) { - syncable::ModelTypeSet sync_ended; - if (!fail_initial_download_) - sync_ended = syncable::ModelTypeSet::All(); - syncable::ModelTypePayloadMap download_progress_markers; - HandleSyncCycleCompletedOnFrontendLoop(SyncSessionSnapshot( - SyncerStatus(), ErrorCounters(), 0, false, - sync_ended, download_progress_markers, false, false, 0, 0, 0, 0, - SyncSourceInfo(), false, 0, base::Time::Now(), false)); -} - namespace { sync_api::HttpPostProviderFactory* MakeTestHttpBridgeFactory() { @@ -85,20 +72,19 @@ void SyncBackendHostForProfileSyncTest::InitCore( } } -void SyncBackendHostForProfileSyncTest::StartConfiguration( - const base::Closure& callback) { - SyncBackendHost::FinishConfigureDataTypesOnFrontendLoop(); - if (IsDownloadingNigoriForTest()) { - syncable::ModelTypeSet sync_ended; +void SyncBackendHostForProfileSyncTest::RequestConfigureSyncer( + sync_api::ConfigureReason reason, + syncable::ModelTypeSet types_to_config, + const browser_sync::ModelSafeRoutingInfo& routing_info, + const base::Callback<void(syncable::ModelTypeSet)>& ready_task, + const base::Closure& retry_callback) { + syncable::ModelTypeSet sync_ended; + if (!fail_initial_download_) + sync_ended.PutAll(types_to_config); - if (!fail_initial_download_) - sync_ended.Put(syncable::NIGORI); - syncable::ModelTypePayloadMap download_progress_markers; - HandleSyncCycleCompletedOnFrontendLoop(SyncSessionSnapshot( - SyncerStatus(), ErrorCounters(), 0, false, - sync_ended, download_progress_markers, false, false, 0, 0, 0, 0, - SyncSourceInfo(), false, 0, base::Time::Now(), false)); - } + FinishConfigureDataTypesOnFrontendLoop(types_to_config, + sync_ended, + ready_task); } void SyncBackendHostForProfileSyncTest::SetHistoryServiceExpectations( diff --git a/chrome/browser/sync/test_profile_sync_service.h b/chrome/browser/sync/test_profile_sync_service.h index 1227e5f..efec408 100644 --- a/chrome/browser/sync/test_profile_sync_service.h +++ b/chrome/browser/sync/test_profile_sync_service.h @@ -45,11 +45,12 @@ class SyncBackendHostForProfileSyncTest : public SyncBackendHost { MOCK_METHOD1(RequestNudge, void(const tracked_objects::Location&)); - // Called when a nudge comes in. - void SimulateSyncCycleCompletedInitialSyncEnded( - const tracked_objects::Location&); - - virtual void StartConfiguration(const base::Closure& callback) OVERRIDE; + virtual void RequestConfigureSyncer( + sync_api::ConfigureReason reason, + syncable::ModelTypeSet types_to_config, + const browser_sync::ModelSafeRoutingInfo& routing_info, + const base::Callback<void(syncable::ModelTypeSet)>& ready_task, + const base::Closure& retry_callback) OVERRIDE; static void SetHistoryServiceExpectations(ProfileMock* profile); |