diff options
Diffstat (limited to 'sync/internal_api')
-rw-r--r-- | sync/internal_api/public/sync_manager.h | 49 | ||||
-rw-r--r-- | sync/internal_api/sync_manager.cc | 158 | ||||
-rw-r--r-- | sync/internal_api/syncapi_unittest.cc | 126 |
3 files changed, 71 insertions, 262 deletions
diff --git a/sync/internal_api/public/sync_manager.h b/sync/internal_api/public/sync_manager.h index d31888c6..75e55aa 100644 --- a/sync/internal_api/public/sync_manager.h +++ b/sync/internal_api/public/sync_manager.h @@ -31,7 +31,6 @@ struct Experiments; class ExtensionsActivityMonitor; class JsBackend; class JsEventHandler; -class SyncScheduler; namespace sessions { class SyncSessionSnapshot; @@ -402,16 +401,6 @@ class SyncManager { // Returns the set of types for which we have stored some sync data. syncer::ModelTypeSet InitialSyncEndedTypes(); - // Returns those types within |types| that have an empty progress marker - // token. - syncer::ModelTypeSet GetTypesWithEmptyProgressMarkerToken( - syncer::ModelTypeSet types); - - // Purge from the directory those types with non-empty progress markers - // but without initial synced ended set. - // Returns false if an error occurred, true otherwise. - bool PurgePartiallySyncedTypes(); - // Update tokens that we're using in Sync. Email must stay the same. void UpdateCredentials(const SyncCredentials& credentials); @@ -439,20 +428,20 @@ class SyncManager { // error to call this when we don't have pending keys. void SetDecryptionPassphrase(const std::string& passphrase); - // Switches the mode of operation to CONFIGURATION_MODE and performs - // any configuration tasks needed as determined by the params. Once complete, - // syncer will remain in CONFIGURATION_MODE until StartSyncingNormally is - // called. - // |ready_task| is invoked when the configuration completes. - // |retry_task| is invoked if the configuration job could not immediately - // execute. |ready_task| will still be called when it eventually - // does finish. - void ConfigureSyncer( - ConfigureReason reason, - const syncer::ModelTypeSet& types_to_config, - const syncer::ModelSafeRoutingInfo& new_routing_info, - const base::Closure& ready_task, - const base::Closure& retry_task); + // Puts the SyncScheduler into a mode where no normal nudge or poll traffic + // will occur, but calls to RequestConfig will be supported. If |callback| + // is provided, it will be invoked (from the internal SyncScheduler) when + // the thread has changed to configuration mode. + void StartConfigurationMode(const base::Closure& callback); + + // Switches the mode of operation to CONFIGURATION_MODE and + // schedules a config task to fetch updates for |types|. + void RequestConfig(const syncer::ModelSafeRoutingInfo& routing_info, + const syncer::ModelTypeSet& types, + syncer::ConfigureReason reason); + + void RequestCleanupDisabledTypes( + const syncer::ModelSafeRoutingInfo& routing_info); // Adds a listener to be notified of sync events. // NOTE: It is OK (in fact, it's probably a good idea) to call this before @@ -549,17 +538,11 @@ class SyncManager { static const FilePath::CharType kSyncDatabaseFilename[]; private: - friend class SyncManagerTest; FRIEND_TEST_ALL_PREFIXES(SyncManagerTest, NudgeDelayTest); // For unit tests. base::TimeDelta GetNudgeDelayTimeDelta(const syncer::ModelType& model_type); - // Set the internal scheduler for testing purposes. - // TODO(sync): Use dependency injection instead. crbug.com/133061 - void SetSyncSchedulerForTest( - scoped_ptr<syncer::SyncScheduler> scheduler); - base::ThreadChecker thread_checker_; // An opaque pointer to the nested private class. @@ -570,6 +553,10 @@ class SyncManager { bool InitialSyncEndedForTypes(syncer::ModelTypeSet types, UserShare* share); +syncer::ModelTypeSet GetTypesWithEmptyProgressMarkerToken( + syncer::ModelTypeSet types, + syncer::UserShare* share); + const char* ConnectionStatusToString(ConnectionStatus status); // Returns the string representation of a PassphraseRequiredReason value. diff --git a/sync/internal_api/sync_manager.cc b/sync/internal_api/sync_manager.cc index e8ba9dd..c8a293e 100644 --- a/sync/internal_api/sync_manager.cc +++ b/sync/internal_api/sync_manager.cc @@ -197,11 +197,6 @@ class SyncManager::SyncInternal // went wrong. bool SignIn(const SyncCredentials& credentials); - // Purge from the directory those types with non-empty progress markers - // but without initial synced ended set. - // Returns false if an error occurred, true otherwise. - bool PurgePartiallySyncedTypes(); - // Update tokens that we're using in Sync. Email must stay the same. void UpdateCredentials(const SyncCredentials& credentials); @@ -362,21 +357,6 @@ class SyncManager::SyncInternal // Called only by our NetworkChangeNotifier. virtual void OnIPAddressChanged() OVERRIDE; - ModelTypeSet GetTypesWithEmptyProgressMarkerToken(ModelTypeSet types) { - DCHECK(initialized_); - syncer::ModelTypeSet result; - for (syncer::ModelTypeSet::Iterator i = types.First(); - i.Good(); i.Inc()) { - sync_pb::DataTypeProgressMarker marker; - directory()->GetDownloadProgress(i.Get(), &marker); - - if (marker.token().empty()) - result.Put(i.Get()); - - } - return result; - } - syncer::ModelTypeSet InitialSyncEndedTypes() { DCHECK(initialized_); return directory()->initial_sync_ended_types(); @@ -396,8 +376,6 @@ class SyncManager::SyncInternal const std::string& name, const JsArgList& args, const WeakHandle<JsReplyHandler>& reply_handler) OVERRIDE; - void SetSyncSchedulerForTest(scoped_ptr<SyncScheduler> scheduler); - private: struct NotificationInfo { int total_count; @@ -771,15 +749,6 @@ syncer::ModelTypeSet SyncManager::InitialSyncEndedTypes() { return data_->InitialSyncEndedTypes(); } -syncer::ModelTypeSet SyncManager::GetTypesWithEmptyProgressMarkerToken( - syncer::ModelTypeSet types) { - return data_->GetTypesWithEmptyProgressMarkerToken(types); -} - -bool SyncManager::PurgePartiallySyncedTypes() { - return data_->PurgePartiallySyncedTypes(); -} - void SyncManager::StartSyncingNormally( const syncer::ModelSafeRoutingInfo& routing_info) { DCHECK(thread_checker_.CalledOnValidThread()); @@ -824,39 +793,40 @@ bool SyncManager::IsUsingExplicitPassphrase() { return data_ && data_->IsUsingExplicitPassphrase(); } -void SyncManager::ConfigureSyncer( - ConfigureReason reason, - const syncer::ModelTypeSet& types_to_config, - const syncer::ModelSafeRoutingInfo& new_routing_info, - const base::Closure& ready_task, - const base::Closure& retry_task) { +void SyncManager::RequestCleanupDisabledTypes( + const syncer::ModelSafeRoutingInfo& routing_info) { DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(!ready_task.is_null()); - DCHECK(!retry_task.is_null()); - - // TODO(zea): set this based on whether cryptographer has keystore - // encryption key or not (requires opening a transaction). crbug.com/129665. - ConfigurationParams::KeystoreKeyStatus keystore_key_status = - ConfigurationParams::KEYSTORE_KEY_UNNECESSARY; - - ConfigurationParams params(GetSourceFromReason(reason), - types_to_config, - new_routing_info, - keystore_key_status, - ready_task); + if (data_->scheduler()) { + data_->session_context()->set_routing_info(routing_info); + data_->scheduler()->CleanupDisabledTypes(); + } +} +void SyncManager::RequestConfig( + const syncer::ModelSafeRoutingInfo& routing_info, + const ModelTypeSet& types, ConfigureReason reason) { + DCHECK(thread_checker_.CalledOnValidThread()); if (!data_->scheduler()) { LOG(INFO) - << "SyncManager::ConfigureSyncer: could not configure because " - << "scheduler is null"; - params.ready_task.Run(); + << "SyncManager::RequestConfig: bailing out because scheduler is " + << "null"; return; } + StartConfigurationMode(base::Closure()); + data_->session_context()->set_routing_info(routing_info); + data_->scheduler()->ScheduleConfiguration(types, GetSourceFromReason(reason)); +} - data_->scheduler()->Start(syncer::SyncScheduler::CONFIGURATION_MODE); - if (!data_->scheduler()->ScheduleConfiguration(params)) - retry_task.Run(); - +void SyncManager::StartConfigurationMode(const base::Closure& callback) { + DCHECK(thread_checker_.CalledOnValidThread()); + if (!data_->scheduler()) { + LOG(INFO) + << "SyncManager::StartConfigurationMode: could not start " + << "configuration mode because because scheduler is null"; + return; + } + data_->scheduler()->Start( + syncer::SyncScheduler::CONFIGURATION_MODE, callback); } bool SyncManager::SyncInternal::Init( @@ -953,29 +923,16 @@ bool SyncManager::SyncInternal::Init( scheduler_.reset(new SyncScheduler(name_, session_context(), new Syncer())); } - bool success = SignIn(credentials); + bool signed_in = SignIn(credentials); - if (success) { + if (signed_in) { if (scheduler()) { - scheduler()->Start(syncer::SyncScheduler::CONFIGURATION_MODE); + scheduler()->Start( + syncer::SyncScheduler::CONFIGURATION_MODE, base::Closure()); } initialized_ = true; - // Unapplied datatypes (those that do not have initial sync ended set) get - // re-downloaded during any configuration. But, it's possible for a datatype - // to have a progress marker but not have initial sync ended yet, making - // it a candidate for migration. This is a problem, as the DataTypeManager - // does not support a migration while it's already in the middle of a - // configuration. As a result, any partially synced datatype can stall the - // DTM, waiting for the configuration to complete, which it never will due - // to the migration error. In addition, a partially synced nigori will - // trigger the migration logic before the backend is initialized, resulting - // in crashes. We therefore detect and purge any partially synced types as - // part of initialization. - if (!PurgePartiallySyncedTypes()) - success = false; - // Cryptographer should only be accessed while holding a // transaction. Grabbing the user share for the transaction // checks the initialization state, so this must come after @@ -993,14 +950,14 @@ bool SyncManager::SyncInternal::Init( FOR_EACH_OBSERVER(SyncManager::Observer, observers_, OnInitializationComplete( MakeWeakHandle(weak_ptr_factory_.GetWeakPtr()), - success)); + signed_in)); - if (!success && testing_mode_ == NON_TEST) + if (!signed_in && testing_mode_ == NON_TEST) return false; sync_notifier_->AddObserver(this); - return success; + return signed_in; } void SyncManager::SyncInternal::UpdateCryptographerAndNigori( @@ -1145,13 +1102,9 @@ void SyncManager::SyncInternal::NotifyCryptographerState( void SyncManager::SyncInternal::StartSyncingNormally( const syncer::ModelSafeRoutingInfo& routing_info) { // Start the sync scheduler. - if (scheduler()) { // NULL during certain unittests. - // TODO(sync): We always want the newest set of routes when we switch back - // to normal mode. Figure out how to enforce set_routing_info is always - // appropriately set and that it's only modified when switching to normal - // mode. + if (scheduler()) { // NULL during certain unittests. session_context()->set_routing_info(routing_info); - scheduler()->Start(SyncScheduler::NORMAL_MODE); + scheduler()->Start(SyncScheduler::NORMAL_MODE, base::Closure()); } } @@ -1205,20 +1158,6 @@ bool SyncManager::SyncInternal::SignIn(const SyncCredentials& credentials) { return true; } -bool SyncManager::SyncInternal::PurgePartiallySyncedTypes() { - syncer::ModelTypeSet partially_synced_types = - syncer::ModelTypeSet::All(); - partially_synced_types.RemoveAll(InitialSyncEndedTypes()); - partially_synced_types.RemoveAll(GetTypesWithEmptyProgressMarkerToken( - syncer::ModelTypeSet::All())); - - UMA_HISTOGRAM_COUNTS("Sync.PartiallySyncedTypes", - partially_synced_types.Size()); - if (partially_synced_types.Empty()) - return true; - return directory()->PurgeEntriesWithTypeIn(partially_synced_types); -} - void SyncManager::SyncInternal::UpdateCredentials( const SyncCredentials& credentials) { DCHECK(thread_checker_.CalledOnValidThread()); @@ -2401,11 +2340,6 @@ void SyncManager::SyncInternal::RemoveObserver( observers_.RemoveObserver(observer); } -void SyncManager::SyncInternal::SetSyncSchedulerForTest( - scoped_ptr<SyncScheduler> sync_scheduler) { - scheduler_ = sync_scheduler.Pass(); -} - SyncStatus SyncManager::GetDetailedStatus() const { return data_->GetStatus(); } @@ -2436,10 +2370,6 @@ TimeDelta SyncManager::GetNudgeDelayTimeDelta( return data_->GetNudgeDelayTimeDelta(model_type); } -void SyncManager::SetSyncSchedulerForTest(scoped_ptr<SyncScheduler> scheduler) { - data_->SetSyncSchedulerForTest(scheduler.Pass()); -} - syncer::ModelTypeSet SyncManager::GetEncryptedDataTypesForTest() const { ReadTransaction trans(FROM_HERE, GetUserShare()); return GetEncryptedTypes(&trans); @@ -2529,4 +2459,20 @@ bool InitialSyncEndedForTypes(syncer::ModelTypeSet types, return true; } +syncer::ModelTypeSet GetTypesWithEmptyProgressMarkerToken( + syncer::ModelTypeSet types, + syncer::UserShare* share) { + syncer::ModelTypeSet result; + for (syncer::ModelTypeSet::Iterator i = types.First(); + i.Good(); i.Inc()) { + sync_pb::DataTypeProgressMarker marker; + share->directory->GetDownloadProgress(i.Get(), &marker); + + if (marker.token().empty()) + result.Put(i.Get()); + + } + return result; +} + } // namespace syncer diff --git a/sync/internal_api/syncapi_unittest.cc b/sync/internal_api/syncapi_unittest.cc index b22b585..6fff8c5 100644 --- a/sync/internal_api/syncapi_unittest.cc +++ b/sync/internal_api/syncapi_unittest.cc @@ -24,7 +24,6 @@ #include "base/utf_string_conversions.h" #include "base/values.h" #include "sync/internal_api/public/base/model_type_test_util.h" -#include "sync/engine/sync_scheduler.h" #include "sync/internal_api/public/change_record.h" #include "sync/internal_api/public/engine/model_safe_worker.h" #include "sync/internal_api/public/engine/polling_constants.h" @@ -58,7 +57,6 @@ #include "sync/syncable/nigori_util.h" #include "sync/syncable/syncable_id.h" #include "sync/syncable/write_transaction.h" -#include "sync/test/callback_counter.h" #include "sync/test/fake_encryptor.h" #include "sync/test/fake_extensions_activity_monitor.h" #include "sync/util/cryptographer.h" @@ -72,10 +70,8 @@ using base::ExpectDictStringValue; using testing::_; using testing::AnyNumber; using testing::AtLeast; -using testing::DoAll; using testing::InSequence; using testing::Invoke; -using testing::Return; using testing::SaveArg; using testing::StrictMock; @@ -910,10 +906,6 @@ class SyncManagerTest : public testing::Test, return true; } - void SetScheduler(scoped_ptr<SyncScheduler> scheduler) { - sync_manager_.SetSyncSchedulerForTest(scheduler.Pass()); - } - private: // Needed by |sync_manager_|. MessageLoop message_loop_; @@ -2493,120 +2485,4 @@ TEST_F(SyncManagerTest, SetPreviouslyEncryptedSpecifics) { } } -class MockSyncScheduler : public SyncScheduler { - public: - MockSyncScheduler() : SyncScheduler("name", NULL, NULL) {} - virtual ~MockSyncScheduler() {} - - MOCK_METHOD1(Start, void(SyncScheduler::Mode)); - MOCK_METHOD1(ScheduleConfiguration, bool(const ConfigurationParams&)); -}; - -// Test that the configuration params are properly created and sent to -// ScheduleConfigure. No callback should be invoked. -TEST_F(SyncManagerTest, BasicConfiguration) { - ConfigureReason reason = CONFIGURE_REASON_RECONFIGURATION; - syncer::ModelTypeSet types_to_download(syncer::BOOKMARKS, - syncer::PREFERENCES); - syncer::ModelSafeRoutingInfo new_routing_info; - GetModelSafeRoutingInfo(&new_routing_info); - - scoped_ptr<MockSyncScheduler> scheduler(new MockSyncScheduler()); - ConfigurationParams params; - EXPECT_CALL(*scheduler, Start(SyncScheduler::CONFIGURATION_MODE)); - EXPECT_CALL(*scheduler, ScheduleConfiguration(_)). - WillOnce(DoAll(SaveArg<0>(¶ms), Return(true))); - SetScheduler(scheduler.PassAs<SyncScheduler>()); - - CallbackCounter ready_task_counter, retry_task_counter; - sync_manager_.ConfigureSyncer( - reason, - types_to_download, - new_routing_info, - base::Bind(&CallbackCounter::Callback, - base::Unretained(&ready_task_counter)), - base::Bind(&CallbackCounter::Callback, - base::Unretained(&retry_task_counter))); - EXPECT_EQ(0, ready_task_counter.times_called()); - EXPECT_EQ(0, retry_task_counter.times_called()); - EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::RECONFIGURATION, - params.source); - EXPECT_TRUE(types_to_download.Equals(params.types_to_download)); - EXPECT_EQ(new_routing_info, params.routing_info); -} - -// Test that the retry callback is invoked on configuration failure. -TEST_F(SyncManagerTest, ConfigurationRetry) { - ConfigureReason reason = CONFIGURE_REASON_RECONFIGURATION; - syncer::ModelTypeSet types_to_download(syncer::BOOKMARKS, - syncer::PREFERENCES); - syncer::ModelSafeRoutingInfo new_routing_info; - GetModelSafeRoutingInfo(&new_routing_info); - - scoped_ptr<MockSyncScheduler> scheduler(new MockSyncScheduler()); - ConfigurationParams params; - EXPECT_CALL(*scheduler, Start(SyncScheduler::CONFIGURATION_MODE)); - EXPECT_CALL(*scheduler, ScheduleConfiguration(_)). - WillOnce(DoAll(SaveArg<0>(¶ms), Return(false))); - SetScheduler(scheduler.PassAs<SyncScheduler>()); - - CallbackCounter ready_task_counter, retry_task_counter; - sync_manager_.ConfigureSyncer( - reason, - types_to_download, - new_routing_info, - base::Bind(&CallbackCounter::Callback, - base::Unretained(&ready_task_counter)), - base::Bind(&CallbackCounter::Callback, - base::Unretained(&retry_task_counter))); - EXPECT_EQ(0, ready_task_counter.times_called()); - EXPECT_EQ(1, retry_task_counter.times_called()); - EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::RECONFIGURATION, - params.source); - EXPECT_TRUE(types_to_download.Equals(params.types_to_download)); - EXPECT_EQ(new_routing_info, params.routing_info); -} - -// Test that PurgePartiallySyncedTypes purges only those types that don't -// have empty progress marker and don't have initial sync ended set. -TEST_F(SyncManagerTest, PurgePartiallySyncedTypes) { - UserShare* share = sync_manager_.GetUserShare(); - - // Set Nigori and Bookmarks to be partial types. - sync_pb::DataTypeProgressMarker nigori_marker; - nigori_marker.set_data_type_id( - GetSpecificsFieldNumberFromModelType(NIGORI)); - nigori_marker.set_token("token"); - sync_pb::DataTypeProgressMarker bookmark_marker; - bookmark_marker.set_data_type_id( - GetSpecificsFieldNumberFromModelType(BOOKMARKS)); - bookmark_marker.set_token("token"); - share->directory->SetDownloadProgress(NIGORI, nigori_marker); - share->directory->SetDownloadProgress(BOOKMARKS, bookmark_marker); - - // Set Preferences to be a full type. - sync_pb::DataTypeProgressMarker pref_marker; - pref_marker.set_data_type_id( - GetSpecificsFieldNumberFromModelType(PREFERENCES)); - pref_marker.set_token("token"); - share->directory->SetDownloadProgress(PREFERENCES, pref_marker); - share->directory->set_initial_sync_ended_for_type(PREFERENCES, true); - - ModelTypeSet partial_types = - sync_manager_.GetTypesWithEmptyProgressMarkerToken(ModelTypeSet::All()); - EXPECT_FALSE(partial_types.Has(NIGORI)); - EXPECT_FALSE(partial_types.Has(BOOKMARKS)); - EXPECT_FALSE(partial_types.Has(PREFERENCES)); - - EXPECT_TRUE(sync_manager_.PurgePartiallySyncedTypes()); - - // Ensure only bookmarks and nigori lost their progress marker. Preferences - // should still have it. - partial_types = - sync_manager_.GetTypesWithEmptyProgressMarkerToken(ModelTypeSet::All()); - EXPECT_TRUE(partial_types.Has(NIGORI)); - EXPECT_TRUE(partial_types.Has(BOOKMARKS)); - EXPECT_FALSE(partial_types.Has(PREFERENCES)); -} - -} // namespace +} // namespace syncer |