diff options
24 files changed, 363 insertions, 68 deletions
diff --git a/chrome/browser/sync/glue/data_type_manager.h b/chrome/browser/sync/glue/data_type_manager.h index b346eb8..06c69e3 100644 --- a/chrome/browser/sync/glue/data_type_manager.h +++ b/chrome/browser/sync/glue/data_type_manager.h @@ -93,9 +93,6 @@ class DataTypeManager { // stopped. virtual void Stop() = 0; - // Reference to map of data type controllers. - virtual const DataTypeController::TypeMap& controllers() = 0; - // The current state of the data type manager. virtual State state() = 0; }; diff --git a/chrome/browser/sync/glue/data_type_manager_impl.cc b/chrome/browser/sync/glue/data_type_manager_impl.cc index fed4319..504f996 100644 --- a/chrome/browser/sync/glue/data_type_manager_impl.cc +++ b/chrome/browser/sync/glue/data_type_manager_impl.cc @@ -64,7 +64,7 @@ class SortComparator : public std::binary_function<DataTypeController*, } // namespace DataTypeManagerImpl::DataTypeManagerImpl(SyncBackendHost* backend, - const DataTypeController::TypeMap& controllers) + const DataTypeController::TypeMap* controllers) : backend_(backend), controllers_(controllers), state_(DataTypeManager::STOPPED), @@ -73,8 +73,8 @@ DataTypeManagerImpl::DataTypeManagerImpl(SyncBackendHost* backend, weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) { DCHECK(backend_); // Ensure all data type controllers are stopped. - for (DataTypeController::TypeMap::const_iterator it = controllers_.begin(); - it != controllers_.end(); ++it) { + for (DataTypeController::TypeMap::const_iterator it = controllers_->begin(); + it != controllers_->end(); ++it) { DCHECK_EQ(DataTypeController::NOT_RUNNING, (*it).second->state()); } @@ -92,8 +92,8 @@ bool DataTypeManagerImpl::GetControllersNeedingStart( bool found_any = false; for (TypeSet::const_iterator it = last_requested_types_.begin(); it != last_requested_types_.end(); ++it) { - DataTypeController::TypeMap::const_iterator dtc = controllers_.find(*it); - if (dtc != controllers_.end() && + DataTypeController::TypeMap::const_iterator dtc = controllers_->find(*it); + if (dtc != controllers_->end() && (dtc->second->state() == DataTypeController::NOT_RUNNING || dtc->second->state() == DataTypeController::STOPPING)) { found_any = true; @@ -163,8 +163,8 @@ void DataTypeManagerImpl::ConfigureImpl(const TypeSet& desired_types, // Add any data type controllers into that needs_stop_ list that are // currently MODEL_STARTING, ASSOCIATING, or RUNNING. needs_stop_.clear(); - for (DataTypeController::TypeMap::const_iterator it = controllers_.begin(); - it != controllers_.end(); ++it) { + for (DataTypeController::TypeMap::const_iterator it = controllers_->begin(); + it != controllers_->end(); ++it) { DataTypeController* dtc = (*it).second; if (desired_types.count(dtc->type()) == 0 && ( dtc->state() == DataTypeController::MODEL_STARTING || @@ -214,7 +214,7 @@ void DataTypeManagerImpl::Restart(sync_api::ConfigureReason reason, const syncable::ModelTypeSet& types_to_add = last_requested_types_; syncable::ModelTypeSet types_to_remove; for (DataTypeController::TypeMap::const_iterator it = - controllers_.begin(); it != controllers_.end(); ++it) { + controllers_->begin(); it != controllers_->end(); ++it) { all_types.insert(it->first); } // Check that types_to_add \subseteq all_types. @@ -406,8 +406,8 @@ void DataTypeManagerImpl::FinishStop() { DCHECK(state_== CONFIGURING || state_ == STOPPING || state_ == BLOCKED || state_ == DOWNLOAD_PENDING); // Simply call the Stop() method on all running data types. - for (DataTypeController::TypeMap::const_iterator it = controllers_.begin(); - it != controllers_.end(); ++it) { + for (DataTypeController::TypeMap::const_iterator it = controllers_->begin(); + it != controllers_->end(); ++it) { DataTypeController* dtc = (*it).second; if (dtc->state() != DataTypeController::NOT_RUNNING && dtc->state() != DataTypeController::STOPPING) { @@ -477,10 +477,6 @@ void DataTypeManagerImpl::NotifyDone(const ConfigureResult& result) { Details<const ConfigureResult>(&result)); } -const DataTypeController::TypeMap& DataTypeManagerImpl::controllers() { - return controllers_; -} - DataTypeManager::State DataTypeManagerImpl::state() { return state_; } diff --git a/chrome/browser/sync/glue/data_type_manager_impl.h b/chrome/browser/sync/glue/data_type_manager_impl.h index 28c9751..01af332 100644 --- a/chrome/browser/sync/glue/data_type_manager_impl.h +++ b/chrome/browser/sync/glue/data_type_manager_impl.h @@ -24,7 +24,7 @@ class SyncBackendHost; class DataTypeManagerImpl : public DataTypeManager { public: DataTypeManagerImpl(SyncBackendHost* backend, - const DataTypeController::TypeMap& controllers); + const DataTypeController::TypeMap* controllers); virtual ~DataTypeManagerImpl(); // DataTypeManager interface. @@ -36,7 +36,6 @@ class DataTypeManagerImpl : public DataTypeManager { sync_api::ConfigureReason reason); virtual void Stop(); - virtual const DataTypeController::TypeMap& controllers(); virtual State state(); private: @@ -77,7 +76,7 @@ class DataTypeManagerImpl : public DataTypeManager { SyncBackendHost* backend_; // Map of all data type controllers that are available for sync. // This list is determined at startup by various command line flags. - const DataTypeController::TypeMap controllers_; + const DataTypeController::TypeMap* controllers_; State state_; std::map<syncable::ModelType, int> start_order_; TypeSet last_requested_types_; 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 ff36242..2a8d15d 100644 --- a/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc +++ b/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc @@ -167,7 +167,7 @@ class DataTypeManagerImplTest : public TestingBrowserProcessTest { }; TEST_F(DataTypeManagerImplTest, NoControllers) { - DataTypeManagerImpl dtm(&backend_, controllers_); + DataTypeManagerImpl dtm(&backend_, &controllers_); SetConfigureStartExpectation(); SetConfigureDoneExpectation(DataTypeManager::OK); dtm.Configure(types_, sync_api::CONFIGURE_REASON_RECONFIGURATION); @@ -181,7 +181,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureOne) { SetStartStopExpectations(bookmark_dtc); controllers_[syncable::BOOKMARKS] = bookmark_dtc; EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _, _, _)).Times(1); - DataTypeManagerImpl dtm(&backend_, controllers_); + DataTypeManagerImpl dtm(&backend_, &controllers_); types_.insert(syncable::BOOKMARKS); SetConfigureStartExpectation(); SetConfigureDoneExpectation(DataTypeManager::OK); @@ -197,7 +197,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureOneStopWhileStarting) { DataTypeController::MODEL_STARTING); controllers_[syncable::BOOKMARKS] = bookmark_dtc; EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _, _, _)).Times(1); - DataTypeManagerImpl dtm(&backend_, controllers_); + DataTypeManagerImpl dtm(&backend_, &controllers_); types_.insert(syncable::BOOKMARKS); SetConfigureStartExpectation(); SetConfigureDoneExpectation(DataTypeManager::OK); @@ -212,7 +212,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureOneStopWhileAssociating) { SetBusyStartStopExpectations(bookmark_dtc, DataTypeController::ASSOCIATING); controllers_[syncable::BOOKMARKS] = bookmark_dtc; EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _, _, _)).Times(1); - DataTypeManagerImpl dtm(&backend_, controllers_); + DataTypeManagerImpl dtm(&backend_, &controllers_); types_.insert(syncable::BOOKMARKS); SetConfigureStartExpectation(); SetConfigureDoneExpectation(DataTypeManager::OK); @@ -232,7 +232,7 @@ TEST_F(DataTypeManagerImplTest, OneWaitingForCrypto) { controllers_[syncable::PASSWORDS] = password_dtc; EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _, _, _)).Times(1); - DataTypeManagerImpl dtm(&backend_, controllers_); + DataTypeManagerImpl dtm(&backend_, &controllers_); types_.insert(syncable::PASSWORDS); SetConfigureStartExpectation(); @@ -268,7 +268,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureOneThenAnother) { controllers_[syncable::PREFERENCES] = preference_dtc; EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _, _, _)).Times(2); - DataTypeManagerImpl dtm(&backend_, controllers_); + DataTypeManagerImpl dtm(&backend_, &controllers_); types_.insert(syncable::BOOKMARKS); SetConfigureStartExpectation(); @@ -295,7 +295,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureOneThenSwitch) { controllers_[syncable::PREFERENCES] = preference_dtc; EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _, _, _)).Times(2); - DataTypeManagerImpl dtm(&backend_, controllers_); + DataTypeManagerImpl dtm(&backend_, &controllers_); types_.insert(syncable::BOOKMARKS); SetConfigureStartExpectation(); @@ -349,7 +349,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureWhileOneInFlight) { SetStartStopExpectations(preference_dtc); controllers_[syncable::PREFERENCES] = preference_dtc; - DataTypeManagerImpl dtm(&backend_, controllers_); + DataTypeManagerImpl dtm(&backend_, &controllers_); EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _, _, _)) .WillOnce(Invoke(DoConfigureDataTypes)) .WillOnce(DoAll(Invoke(DoConfigureDataTypes), @@ -385,7 +385,7 @@ TEST_F(DataTypeManagerImplTest, OneFailingController) { WillRepeatedly(Return(DataTypeController::NOT_RUNNING)); controllers_[syncable::BOOKMARKS] = bookmark_dtc; - DataTypeManagerImpl dtm(&backend_, controllers_); + DataTypeManagerImpl dtm(&backend_, &controllers_); SetConfigureStartExpectation(); SetConfigureDoneExpectation(DataTypeManager::ASSOCIATION_FAILED); EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _, _, _)).Times(1); @@ -409,7 +409,7 @@ TEST_F(DataTypeManagerImplTest, StopWhileInFlight) { WillRepeatedly(Return(DataTypeController::NOT_RUNNING)); controllers_[syncable::PREFERENCES] = preference_dtc; - DataTypeManagerImpl dtm(&backend_, controllers_); + DataTypeManagerImpl dtm(&backend_, &controllers_); SetConfigureStartExpectation(); SetConfigureDoneExpectation(DataTypeManager::ABORTED); EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _, _, _)).Times(1); @@ -441,7 +441,7 @@ TEST_F(DataTypeManagerImplTest, SecondControllerFails) { WillRepeatedly(Return(DataTypeController::NOT_RUNNING)); controllers_[syncable::PREFERENCES] = preference_dtc; - DataTypeManagerImpl dtm(&backend_, controllers_); + DataTypeManagerImpl dtm(&backend_, &controllers_); SetConfigureStartExpectation(); SetConfigureDoneExpectation(DataTypeManager::ASSOCIATION_FAILED); EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _, _, _)).Times(1); @@ -461,7 +461,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureWhileDownloadPending) { SetStartStopExpectations(preference_dtc); controllers_[syncable::PREFERENCES] = preference_dtc; - DataTypeManagerImpl dtm(&backend_, controllers_); + DataTypeManagerImpl dtm(&backend_, &controllers_); SetConfigureStartExpectation(); SetConfigureDoneExpectation(DataTypeManager::OK); base::Callback<void(bool)> task; @@ -495,7 +495,7 @@ TEST_F(DataTypeManagerImplTest, StopWhileDownloadPending) { SetNotUsedExpectations(bookmark_dtc); controllers_[syncable::BOOKMARKS] = bookmark_dtc; - DataTypeManagerImpl dtm(&backend_, controllers_); + DataTypeManagerImpl dtm(&backend_, &controllers_); SetConfigureStartExpectation(); SetConfigureDoneExpectation(DataTypeManager::ABORTED); base::Callback<void(bool)> task; diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc index a031fde..00786e0 100644 --- a/chrome/browser/sync/glue/sync_backend_host.cc +++ b/chrome/browser/sync/glue/sync_backend_host.cc @@ -710,6 +710,14 @@ void SyncBackendHost::Core::HandleSyncCycleCompletedOnFrontendLoop( if (!to_migrate.empty()) host_->frontend_->OnMigrationNeededForTypes(to_migrate); + // Process any changes to the datatypes we're syncing. + // TODO(sync): add support for removing types. + syncable::ModelTypeSet to_add; + if (host_->initialized() && + sync_manager()->ReceivedExperimentalTypes(&to_add)) { + host_->frontend_->OnDataTypesChanged(to_add); + } + // 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. diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h index 81be5a2..5a0b0f6 100644 --- a/chrome/browser/sync/glue/sync_backend_host.h +++ b/chrome/browser/sync/glue/sync_backend_host.h @@ -94,6 +94,9 @@ class SyncFrontend { virtual void OnMigrationNeededForTypes( const syncable::ModelTypeSet& types) = 0; + // Inform the Frontend that new datatypes are available for registration. + virtual void OnDataTypesChanged(const syncable::ModelTypeSet& to_add) = 0; + protected: // Don't delete through SyncFrontend interface. virtual ~SyncFrontend() { diff --git a/chrome/browser/sync/glue/sync_backend_host_unittest.cc b/chrome/browser/sync/glue/sync_backend_host_unittest.cc index 67c7aad..821ecc0 100644 --- a/chrome/browser/sync/glue/sync_backend_host_unittest.cc +++ b/chrome/browser/sync/glue/sync_backend_host_unittest.cc @@ -42,6 +42,7 @@ class MockSyncFrontend : public SyncFrontend { MOCK_METHOD0(OnPassphraseAccepted, void()); MOCK_METHOD1(OnEncryptionComplete, void(const syncable::ModelTypeSet&)); MOCK_METHOD1(OnMigrationNeededForTypes, void(const syncable::ModelTypeSet&)); + MOCK_METHOD1(OnDataTypesChanged, void(const syncable::ModelTypeSet&)); }; } // namespace diff --git a/chrome/browser/sync/internal_api/sync_manager.cc b/chrome/browser/sync/internal_api/sync_manager.cc index c0b9cb3..dea6522 100644 --- a/chrome/browser/sync/internal_api/sync_manager.cc +++ b/chrome/browser/sync/internal_api/sync_manager.cc @@ -1968,10 +1968,25 @@ void SyncManager::RefreshEncryption() { } syncable::ModelTypeSet SyncManager::GetEncryptedDataTypes() const { - sync_api::ReadTransaction trans(FROM_HERE, GetUserShare()); + ReadTransaction trans(FROM_HERE, GetUserShare()); return GetEncryptedTypes(&trans); } +bool SyncManager::ReceivedExperimentalTypes(syncable::ModelTypeSet* to_add) + const { + ReadTransaction trans(FROM_HERE, GetUserShare()); + ReadNode node(&trans); + if (!node.InitByTagLookup(kNigoriTag)) { + VLOG(1) << "Couldn't find Nigori node."; + return false; + } + if (node.GetNigoriSpecifics().sync_tabs()) { + to_add->insert(syncable::SESSIONS); + return true; + } + return false; +} + bool SyncManager::HasUnsyncedItems() const { sync_api::ReadTransaction trans(FROM_HERE, GetUserShare()); return (trans.GetWrappedTrans()->directory()->unsynced_entity_count() != 0); diff --git a/chrome/browser/sync/internal_api/sync_manager.h b/chrome/browser/sync/internal_api/sync_manager.h index 14350a4..2dfe125 100644 --- a/chrome/browser/sync/internal_api/sync_manager.h +++ b/chrome/browser/sync/internal_api/sync_manager.h @@ -512,8 +512,15 @@ class SyncManager { // only be called after syncapi has been initialized. void RefreshEncryption(); + // Gets the set of encrypted types from the cryptographer + // Note: opens a transaction. syncable::ModelTypeSet GetEncryptedDataTypes() const; + // Reads the nigori node to determine if any experimental types should be + // enabled. + // Note: opens a transaction. + bool ReceivedExperimentalTypes(syncable::ModelTypeSet* to_add) const; + // Uses a read-only transaction to determine if the directory being synced has // any remaining unsynced items. bool HasUnsyncedItems() const; diff --git a/chrome/browser/sync/profile_sync_factory.h b/chrome/browser/sync/profile_sync_factory.h index 8befe35..3c6156c 100644 --- a/chrome/browser/sync/profile_sync_factory.h +++ b/chrome/browser/sync/profile_sync_factory.h @@ -61,7 +61,7 @@ class ProfileSyncFactory { // the caller. virtual browser_sync::DataTypeManager* CreateDataTypeManager( browser_sync::SyncBackendHost* backend, - const browser_sync::DataTypeController::TypeMap& controllers) = 0; + const browser_sync::DataTypeController::TypeMap* controllers) = 0; // Instantiates both a model associator and change processor for the // app data type. The pointers in the return struct are diff --git a/chrome/browser/sync/profile_sync_factory_impl.cc b/chrome/browser/sync/profile_sync_factory_impl.cc index cb80f2a..7d159b2 100644 --- a/chrome/browser/sync/profile_sync_factory_impl.cc +++ b/chrome/browser/sync/profile_sync_factory_impl.cc @@ -158,7 +158,7 @@ void ProfileSyncFactoryImpl::RegisterDataTypes(ProfileSyncService* pss) { DataTypeManager* ProfileSyncFactoryImpl::CreateDataTypeManager( SyncBackendHost* backend, - const DataTypeController::TypeMap& controllers) { + const DataTypeController::TypeMap* controllers) { return new DataTypeManagerImpl(backend, controllers); } diff --git a/chrome/browser/sync/profile_sync_factory_impl.h b/chrome/browser/sync/profile_sync_factory_impl.h index 2015a40..c9925bd 100644 --- a/chrome/browser/sync/profile_sync_factory_impl.h +++ b/chrome/browser/sync/profile_sync_factory_impl.h @@ -27,7 +27,7 @@ class ProfileSyncFactoryImpl : public ProfileSyncFactory { virtual browser_sync::DataTypeManager* CreateDataTypeManager( browser_sync::SyncBackendHost* backend, - const browser_sync::DataTypeController::TypeMap& controllers); + const browser_sync::DataTypeController::TypeMap* controllers); virtual SyncComponents CreateAppSyncComponents( ProfileSyncService* profile_sync_service, diff --git a/chrome/browser/sync/profile_sync_factory_mock.h b/chrome/browser/sync/profile_sync_factory_mock.h index 3fa856c..5bef3a6 100644 --- a/chrome/browser/sync/profile_sync_factory_mock.h +++ b/chrome/browser/sync/profile_sync_factory_mock.h @@ -30,7 +30,7 @@ class ProfileSyncFactoryMock : public ProfileSyncFactory { MOCK_METHOD2(CreateDataTypeManager, browser_sync::DataTypeManager*( browser_sync::SyncBackendHost*, - const browser_sync::DataTypeController::TypeMap&)); + const browser_sync::DataTypeController::TypeMap*)); MOCK_METHOD2(CreateAppSyncComponents, SyncComponents(ProfileSyncService* profile_sync_service, browser_sync::UnrecoverableErrorHandler* error_handler)); diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc index 1c4aca3..4480774 100644 --- a/chrome/browser/sync/profile_sync_service.cc +++ b/chrome/browser/sync/profile_sync_service.cc @@ -6,7 +6,6 @@ #include <stddef.h> #include <map> -#include <ostream> #include <set> #include <utility> @@ -21,6 +20,8 @@ #include "base/stringprintf.h" #include "base/task.h" #include "base/threading/thread_restrictions.h" +#include "chrome/browser/about_flags.h" +#include "chrome/browser/browser_process.h" #include "chrome/browser/net/chrome_cookie_notification_details.h" #include "chrome/browser/net/gaia/token_service.h" #include "chrome/browser/prefs/pref_service.h" @@ -29,6 +30,8 @@ #include "chrome/browser/sync/engine/configure_reason.h" #include "chrome/browser/sync/glue/change_processor.h" #include "chrome/browser/sync/glue/data_type_controller.h" +#include "chrome/browser/sync/glue/session_data_type_controller.h" +#include "chrome/browser/sync/glue/typed_url_data_type_controller.h" #include "chrome/browser/sync/glue/data_type_manager.h" #include "chrome/browser/sync/glue/session_data_type_controller.h" #include "chrome/browser/sync/internal_api/sync_manager.h" @@ -316,7 +319,6 @@ void ProfileSyncService::RegisterPreferences() { pref_service->RegisterStringPref(prefs::kEncryptionBootstrapToken, "", PrefService::UNSYNCABLE_PREF); - pref_service->RegisterBooleanPref(prefs::kSyncAutofillProfile, enable_by_default, PrefService::UNSYNCABLE_PREF); @@ -550,6 +552,41 @@ const char* ProfileSyncService::GetPrefNameForDataType( return NULL; } +// static +std::string ProfileSyncService::GetExperimentNameForDataType( + syncable::ModelType data_type) { + switch (data_type) { + case syncable::SESSIONS: + return "sync-sessions"; + case syncable::TYPED_URLS: + return "sync-typed-urls"; + default: + break; + } + NOTREACHED(); + return ""; +} + +void ProfileSyncService::RegisterNewDataType(syncable::ModelType data_type) { + if (data_type_controllers_.count(data_type) > 0) + return; + switch (data_type) { + case syncable::SESSIONS: + RegisterDataTypeController( + new browser_sync::SessionDataTypeController(factory_, + profile_, + this)); + return; + case syncable::TYPED_URLS: + RegisterDataTypeController( + new browser_sync::TypedUrlDataTypeController(factory_, profile_)); + return; + default: + break; + } + NOTREACHED(); +} + // An invariant has been violated. Transition to an error state where we try // to do as little work as possible, to avoid further corruption or crashes. void ProfileSyncService::OnUnrecoverableError( @@ -627,6 +664,60 @@ void ProfileSyncService::OnSyncCycleCompleted() { NotifyObservers(); } +// TODO(sync): eventually support removing datatypes too. +void ProfileSyncService::OnDataTypesChanged( + const syncable::ModelTypeSet& to_add) { + // We don't bother doing anything if the migrator is busy. + if (migrator_->state() != browser_sync::BackendMigrator::IDLE) { + VLOG(1) << "Dropping OnDataTypesChanged due to migrator busy."; + return; + } + + syncable::ModelTypeSet new_types; + syncable::ModelTypeSet preferred_types; + GetPreferredDataTypes(&preferred_types); + syncable::ModelTypeSet registered_types; + GetRegisteredDataTypes(®istered_types); + + for (syncable::ModelTypeSet::const_iterator iter = to_add.begin(); + iter != to_add.end(); + ++iter) { + // Received notice to enable session sync. Check if sessions are + // registered, and if not register a new datatype controller. + if (registered_types.count(*iter) == 0) { + RegisterNewDataType(*iter); + // Enable the about:flags switch for sessions so we don't have to always + // perform this reconfiguration. Once we set this, sessions will remain + // registered, so we will no longer go down this code path. + std::string experiment_name = GetExperimentNameForDataType(*iter); + if (experiment_name.empty()) + continue; + about_flags::SetExperimentEnabled(g_browser_process->local_state(), + experiment_name, + true); + + // Check if the user has "Keep Everything Synced" enabled. If so, we want + // to turn on sessions if it's not already on. Otherwise we leave it off. + // Note: if sessions are already registered, we don't turn it on. This + // covers the case where we're already in the process of reconfiguring + // to turn sessions on. + if (profile_->GetPrefs()->GetBoolean(prefs::kKeepEverythingSynced) && + preferred_types.count(*iter) == 0){ + std::string pref_name = GetPrefNameForDataType(*iter); + if (pref_name.empty()) + continue; + profile_->GetPrefs()->SetBoolean(pref_name.c_str(), true); + } + } + } + + if (!new_types.empty()) { + VLOG(1) << "Dynamically enabling new datatypes: " + << syncable::ModelTypeSetToString(new_types); + OnMigrationNeededForTypes(new_types); + } +} + void ProfileSyncService::UpdateAuthErrorState( const GoogleServiceAuthError& error) { last_auth_error_ = error; @@ -1126,7 +1217,7 @@ void ProfileSyncService::ConfigureDataTypeManager() { restart = true; data_type_manager_.reset( factory_->CreateDataTypeManager(backend_.get(), - data_type_controllers_)); + &data_type_controllers_)); registrar_.Add(this, chrome::NOTIFICATION_SYNC_CONFIGURE_START, Source<DataTypeManager>(data_type_manager_.get())); diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h index b39cacf..0af7081 100644 --- a/chrome/browser/sync/profile_sync_service.h +++ b/chrome/browser/sync/profile_sync_service.h @@ -207,6 +207,8 @@ class ProfileSyncService : public browser_sync::SyncFrontend, const syncable::ModelTypeSet& encrypted_types) OVERRIDE; virtual void OnMigrationNeededForTypes( const syncable::ModelTypeSet& types) OVERRIDE; + virtual void OnDataTypesChanged( + const syncable::ModelTypeSet& to_add) OVERRIDE; void OnClearServerDataTimeout(); @@ -551,6 +553,14 @@ class ProfileSyncService : public browser_sync::SyncFrontend, static const char* GetPrefNameForDataType(syncable::ModelType data_type); + // About-flags experiment names for datatypes that aren't enabled by default + // yet. + static std::string GetExperimentNameForDataType( + syncable::ModelType data_type); + + // Create and register a new datatype controller. + void RegisterNewDataType(syncable::ModelType data_type); + // Time at which we begin an attempt a GAIA authorization. base::TimeTicks auth_start_time_; diff --git a/chrome/browser/sync/profile_sync_service_harness.cc b/chrome/browser/sync/profile_sync_service_harness.cc index 9149723..2d0fd3e 100644 --- a/chrome/browser/sync/profile_sync_service_harness.cc +++ b/chrome/browser/sync/profile_sync_service_harness.cc @@ -874,10 +874,19 @@ bool ProfileSyncServiceHarness::WaitForTypeEncryption( bool ProfileSyncServiceHarness::IsTypeEncrypted(syncable::ModelType type) { syncable::ModelTypeSet encrypted_types; service_->GetEncryptedDataTypes(&encrypted_types); - if (encrypted_types.count(type) == 0) { - return false; - } - return true; + return (encrypted_types.count(type) != 0); +} + +bool ProfileSyncServiceHarness::IsTypeRegistered(syncable::ModelType type) { + syncable::ModelTypeSet registered_types; + service_->GetRegisteredDataTypes(®istered_types); + return (registered_types.count(type) != 0); +} + +bool ProfileSyncServiceHarness::IsTypePreferred(syncable::ModelType type) { + syncable::ModelTypeSet synced_types; + service_->GetPreferredDataTypes(&synced_types); + return (synced_types.count(type) != 0); } std::string ProfileSyncServiceHarness::GetServiceStatus() { diff --git a/chrome/browser/sync/profile_sync_service_harness.h b/chrome/browser/sync/profile_sync_service_harness.h index 1179bce..8a00cc6 100644 --- a/chrome/browser/sync/profile_sync_service_harness.h +++ b/chrome/browser/sync/profile_sync_service_harness.h @@ -157,6 +157,12 @@ class ProfileSyncServiceHarness : public ProfileSyncServiceObserver { // Check if |type| is encrypted. bool IsTypeEncrypted(syncable::ModelType type); + // Check if |type| is registered. + bool IsTypeRegistered(syncable::ModelType types); + + // Check if |type| is being synced. + bool IsTypePreferred(syncable::ModelType type); + private: friend class StateChangeTimeoutEvent; diff --git a/chrome/browser/sync/protocol/nigori_specifics.proto b/chrome/browser/sync/protocol/nigori_specifics.proto index 60f5181..41cdfb2 100644 --- a/chrome/browser/sync/protocol/nigori_specifics.proto +++ b/chrome/browser/sync/protocol/nigori_specifics.proto @@ -60,6 +60,8 @@ message NigoriSpecifics { optional bool encrypt_sessions = 20; optional bool encrypt_apps = 21; optional bool encrypt_search_engines = 22; + + optional bool sync_tabs = 23; } extend EntitySpecifics { diff --git a/chrome/browser/sync/protocol/proto_value_conversions.cc b/chrome/browser/sync/protocol/proto_value_conversions.cc index 753f568..eb583f0 100644 --- a/chrome/browser/sync/protocol/proto_value_conversions.cc +++ b/chrome/browser/sync/protocol/proto_value_conversions.cc @@ -238,6 +238,7 @@ DictionaryValue* NigoriSpecificsToValue( SET_BOOL(encrypt_sessions); SET_BOOL(encrypt_apps); SET_BOOL(encrypt_search_engines); + SET_BOOL(sync_tabs); return value; } diff --git a/chrome/test/live_sync/live_sync_test.cc b/chrome/test/live_sync/live_sync_test.cc index cd87d21..b609fc2 100644 --- a/chrome/test/live_sync/live_sync_test.cc +++ b/chrome/test/live_sync/live_sync_test.cc @@ -151,14 +151,14 @@ void LiveSyncTest::SetUp() { SetupMockGaiaResponses(); } - if (!cl->HasSwitch(switches::kSyncServiceURL) && - !cl->HasSwitch(switches::kSyncServerCommandLine)) { - // If neither a sync server URL nor a sync server command line is - // provided, start up a local python sync test server and point Chrome - // to its URL. This is the most common configuration, and the only - // one that makes sense for most developers. - server_type_ = LOCAL_PYTHON_SERVER; - } else if (cl->HasSwitch(switches::kSyncServiceURL) && + if (!cl->HasSwitch(switches::kSyncServiceURL) && + !cl->HasSwitch(switches::kSyncServerCommandLine)) { + // If neither a sync server URL nor a sync server command line is + // provided, start up a local python sync test server and point Chrome + // to its URL. This is the most common configuration, and the only + // one that makes sense for most developers. + server_type_ = LOCAL_PYTHON_SERVER; + } else if (cl->HasSwitch(switches::kSyncServiceURL) && cl->HasSwitch(switches::kSyncServerCommandLine)) { // If a sync server URL and a sync server command line are provided, // start up a local sync server by running the command line. Chrome @@ -201,20 +201,27 @@ void LiveSyncTest::TearDown() { } void LiveSyncTest::SetUpCommandLine(CommandLine* cl) { + AddTestSwitches(cl); + AddOptionalTypesToCommandLine(cl); +} + +void LiveSyncTest::AddTestSwitches(CommandLine* cl) { // TODO(rsimha): Until we implement a fake Tango server against which tests // can run, we need to set the --sync-notification-method to "p2p". if (!cl->HasSwitch(switches::kSyncNotificationMethod)) cl->AppendSwitchASCII(switches::kSyncNotificationMethod, "p2p"); - // TODO(sync): Remove this once sessions sync is enabled by default. - if (!cl->HasSwitch(switches::kEnableSyncSessions)) - cl->AppendSwitch(switches::kEnableSyncSessions); - // Disable non-essential access of external network resources. if (!cl->HasSwitch(switches::kDisableBackgroundNetworking)) cl->AppendSwitch(switches::kDisableBackgroundNetworking); } +void LiveSyncTest::AddOptionalTypesToCommandLine(CommandLine* cl) { + // TODO(sync): Remove this once sessions sync is enabled by default. + if (!cl->HasSwitch(switches::kEnableSyncSessions)) + cl->AppendSwitch(switches::kEnableSyncSessions); +} + // static Profile* LiveSyncTest::MakeProfile(const FilePath::StringType name) { FilePath path; @@ -357,7 +364,7 @@ void LiveSyncTest::ReadPasswordFile() { std::vector<std::string> tokens; std::string delimiters = "\r\n"; Tokenize(file_contents, delimiters, &tokens); - ASSERT_TRUE(tokens.size() == 2) << "Password file \"" + ASSERT_EQ(2U, tokens.size()) << "Password file \"" << password_file_.value() << "\" must contain exactly two lines of text."; username_ = tokens[0]; @@ -571,6 +578,14 @@ void LiveSyncTest::TriggerTransientError() { UTF16ToASCII(browser()->GetSelectedTabContents()->GetTitle())); } +void LiveSyncTest::TriggerSetSyncTabs() { + ASSERT_TRUE(ServerSupportsErrorTriggering()); + std::string path = "chromiumsync/synctabs"; + ui_test_utils::NavigateToURL(browser(), sync_server_.GetURL(path)); + ASSERT_EQ("Sync Tabs", + UTF16ToASCII(browser()->GetSelectedTabContents()->GetTitle())); +} + void LiveSyncTest::SetProxyConfig(net::URLRequestContextGetter* context_getter, const net::ProxyConfig& proxy_config) { base::WaitableEvent done(false, false); diff --git a/chrome/test/live_sync/live_sync_test.h b/chrome/test/live_sync/live_sync_test.h index 6e79172..f9f9b15 100644 --- a/chrome/test/live_sync/live_sync_test.h +++ b/chrome/test/live_sync/live_sync_test.h @@ -8,6 +8,9 @@ #include "chrome/test/base/in_process_browser_test.h" +#include <string> +#include <vector> + #include "base/basictypes.h" #include "base/file_util.h" #include "base/memory/scoped_ptr.h" @@ -17,9 +20,6 @@ #include "net/base/mock_host_resolver.h" #include "net/test/test_server.h" -#include <string> -#include <vector> - class CommandLine; class Profile; class ProfileSyncServiceHarness; @@ -77,13 +77,13 @@ class LiveSyncTest : public InProcessBrowserTest { // Validates command line parameters and creates a local python test server if // specified. - virtual void SetUp(); + virtual void SetUp() OVERRIDE; // Brings down local python test server if one was created. - virtual void TearDown(); + virtual void TearDown() OVERRIDE; // Sets up command line flags required for sync tests. - virtual void SetUpCommandLine(CommandLine* cl); + virtual void SetUpCommandLine(CommandLine* cl) OVERRIDE; // Used to get the number of sync clients used by a test. int num_clients() WARN_UNUSED_RESULT { return num_clients_; } @@ -161,18 +161,28 @@ class LiveSyncTest : public InProcessBrowserTest { // this state until shut down. void TriggerTransientError(); + // Triggers setting the sync_tabs field of the nigori node. + void TriggerSetSyncTabs(); + protected: + // Add custom switches needed for running the test. + virtual void AddTestSwitches(CommandLine* cl); + + // Append the command line switches to enable experimental types that aren't + // on by default yet. + virtual void AddOptionalTypesToCommandLine(CommandLine* cl); + // InProcessBrowserTest override. Destroys all the sync clients and sync // profiles created by a test. - virtual void CleanUpOnMainThread(); + virtual void CleanUpOnMainThread() OVERRIDE; // InProcessBrowserTest override. Changes behavior of the default host // resolver to avoid DNS lookup errors. - virtual void SetUpInProcessBrowserTestFixture(); + virtual void SetUpInProcessBrowserTestFixture() OVERRIDE; // InProcessBrowserTest override. Resets the host resolver its default // behavior. - virtual void TearDownInProcessBrowserTestFixture(); + virtual void TearDownInProcessBrowserTestFixture() OVERRIDE; // GAIA account used by the test case. std::string username_; diff --git a/chrome/test/live_sync/migration_errors_test.cc b/chrome/test/live_sync/migration_errors_test.cc index 6626ea2..fb124a4 100644 --- a/chrome/test/live_sync/migration_errors_test.cc +++ b/chrome/test/live_sync/migration_errors_test.cc @@ -279,3 +279,91 @@ IN_PROC_BROWSER_TEST_F(MigrationErrorsTest, MigrationHellWithNigori) { ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1))); ASSERT_TRUE(BooleanPrefMatches(prefs::kShowHomeButton)); } + +class MigrationReconfigureTest : public LiveSyncTest { + public: + MigrationReconfigureTest() : LiveSyncTest(TWO_CLIENT) {} + + virtual void SetUpCommandLine(CommandLine* cl) OVERRIDE { + AddTestSwitches(cl); + // Do not add optional datatypes. + } + + virtual ~MigrationReconfigureTest() {} + + private: + DISALLOW_COPY_AND_ASSIGN(MigrationReconfigureTest); +}; + +IN_PROC_BROWSER_TEST_F(MigrationReconfigureTest, SetSyncTabs) { + if (!ServerSupportsErrorTriggering()) { + LOG(WARNING) << "Test skipped in this server environment."; + return; + } + + ASSERT_TRUE(SetupSync()) << "SetupSync() failed."; + ASSERT_FALSE(GetClient(0)->IsTypeRegistered(syncable::SESSIONS)); + ASSERT_FALSE(GetClient(0)->IsTypePreferred(syncable::SESSIONS)); + + // Phase 1: Before migrating anything, create & sync a preference. + ASSERT_TRUE(BooleanPrefMatches(prefs::kShowHomeButton)); + ChangeBooleanPref(0, prefs::kShowHomeButton); + ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1))); + ASSERT_TRUE(BooleanPrefMatches(prefs::kShowHomeButton)); + + // Phase 2: Trigger setting the sync_tabs field. + TriggerSetSyncTabs(); + + // Phase 3: Modify a bookmark and wait for it to sync. + ASSERT_TRUE(AddURL(0, IndexedURLTitle(0), GURL(IndexedURL(0))) != NULL); + ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1))); + + // Phase 4: Verify that preferences can still be synchronized. + ASSERT_TRUE(BooleanPrefMatches(prefs::kShowHomeButton)); + ChangeBooleanPref(0, prefs::kShowHomeButton); + ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1))); + ASSERT_TRUE(BooleanPrefMatches(prefs::kShowHomeButton)); + + // Phase 5: Verify that sessions are registered and enabled. + ASSERT_TRUE(GetClient(0)->IsTypeRegistered(syncable::SESSIONS)); + ASSERT_TRUE(GetClient(0)->IsTypePreferred(syncable::SESSIONS)); +} + +IN_PROC_BROWSER_TEST_F(MigrationReconfigureTest, SetSyncTabsAndMigrate) { + if (!ServerSupportsErrorTriggering()) { + LOG(WARNING) << "Test skipped in this server environment."; + return; + } + + ASSERT_TRUE(SetupSync()) << "SetupSync() failed."; + ASSERT_FALSE(GetClient(0)->IsTypeRegistered(syncable::SESSIONS)); + ASSERT_FALSE(GetClient(0)->IsTypePreferred(syncable::SESSIONS)); + + // Phase 1: Before migrating anything, create & sync a preference. + ASSERT_TRUE(BooleanPrefMatches(prefs::kShowHomeButton)); + ChangeBooleanPref(0, prefs::kShowHomeButton); + ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1))); + ASSERT_TRUE(BooleanPrefMatches(prefs::kShowHomeButton)); + + // Phase 2: Trigger setting the sync_tabs field. + TriggerSetSyncTabs(); + + // Phase 3: Trigger a preference migration on the server. + syncable::ModelTypeSet migrate_types; + migrate_types.insert(syncable::PREFERENCES); + TriggerMigrationDoneError(migrate_types); + + // Phase 4: Modify a bookmark and wait for it to sync. + ASSERT_TRUE(AddURL(0, IndexedURLTitle(0), GURL(IndexedURL(0))) != NULL); + ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1))); + + // Phase 5: Verify that preferences can still be synchronized. + ASSERT_TRUE(BooleanPrefMatches(prefs::kShowHomeButton)); + ChangeBooleanPref(0, prefs::kShowHomeButton); + ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1))); + ASSERT_TRUE(BooleanPrefMatches(prefs::kShowHomeButton)); + + // Phase 6: Verify that sessions are registered and enabled. + ASSERT_TRUE(GetClient(0)->IsTypeRegistered(syncable::SESSIONS)); + ASSERT_TRUE(GetClient(0)->IsTypePreferred(syncable::SESSIONS)); +} diff --git a/net/tools/testserver/chromiumsync.py b/net/tools/testserver/chromiumsync.py index cc71d47b..166dff1 100755 --- a/net/tools/testserver/chromiumsync.py +++ b/net/tools/testserver/chromiumsync.py @@ -863,6 +863,23 @@ class SyncDataModel(object): entry.parent_id_string) self._entries[entry.id_string] = entry + def TriggerSyncTabs(self): + """Set the 'sync_tabs' field to this account's nigori node. + + If the field is not currently set, will write a new nigori node entry + with the field set. Else does nothing. + """ + + nigori_tag = "google_chrome_nigori" + nigori_original = self._entries.get(self._ServerTagToId(nigori_tag)) + if (nigori_original.specifics.Extensions[nigori_specifics_pb2.nigori]. + sync_tabs): + return + nigori_new = copy.deepcopy(nigori_original) + nigori_new.specifics.Extensions[nigori_specifics_pb2.nigori].sync_tabs = ( + True) + self._SaveEntry(nigori_new) + class TestServer(object): """An object to handle requests for one (and only one) Chrome Sync account. @@ -939,6 +956,13 @@ class TestServer(object): 200, '<html><title>Transient error</title><H1>Transient error</H1></html>') + def HandleSetSyncTabs(self): + """Set the 'sync_tab' field of the nigori node for this account.""" + self.account.TriggerSyncTabs() + return ( + 200, + '<html><title>Sync Tabs</title><H1>Sync Tabs</H1></html>') + def HandleCommand(self, query, raw_request): """Decode and handle a sync command from a raw input of bytes. diff --git a/net/tools/testserver/testserver.py b/net/tools/testserver/testserver.py index c079b4c5..5d27dbe 100755 --- a/net/tools/testserver/testserver.py +++ b/net/tools/testserver/testserver.py @@ -1407,7 +1407,8 @@ class SyncPageHandler(BasePageHandler): get_handlers = [self.ChromiumSyncMigrationOpHandler, self.ChromiumSyncTimeHandler, self.ChromiumSyncBirthdayErrorOpHandler, - self.ChromiumSyncTransientErrorOpHandler] + self.ChromiumSyncTransientErrorOpHandler, + self.ChromiumSyncSyncTabsOpHandler] post_handlers = [self.ChromiumSyncCommandHandler, self.ChromiumSyncTimeHandler] @@ -1495,6 +1496,18 @@ class SyncPageHandler(BasePageHandler): self.wfile.write(raw_reply) return True; + def ChromiumSyncSyncTabsOpHandler(self): + test_name = "/chromiumsync/synctabs" + if not self._ShouldHandleRequest(test_name): + return False + result, raw_reply = self.server._sync_handler.HandleSetSyncTabs() + self.send_response(result) + self.send_header('Content-Type', 'text/html') + self.send_header('Content-Length', len(raw_reply)) + self.end_headers() + self.wfile.write(raw_reply) + return True; + def MakeDataDir(): if options.data_dir: |