summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--chrome/browser/sync/backend_migrator.cc18
-rw-r--r--chrome/browser/sync/backend_migrator_unittest.cc48
-rw-r--r--chrome/browser/sync/glue/data_type_manager.h4
-rw-r--r--chrome/browser/sync/glue/data_type_manager_impl.cc14
-rw-r--r--chrome/browser/sync/glue/data_type_manager_impl.h4
-rw-r--r--chrome/browser/sync/glue/data_type_manager_impl_unittest.cc154
-rw-r--r--chrome/browser/sync/glue/data_type_manager_mock.h3
-rw-r--r--chrome/browser/sync/glue/sync_backend_host.cc2
-rw-r--r--chrome/browser/sync/glue/sync_backend_host_unittest.cc24
-rw-r--r--chrome/browser/sync/sync_prefs.cc14
-rw-r--r--sync/engine/apply_control_data_updates.cc138
-rw-r--r--sync/engine/apply_control_data_updates.h23
-rw-r--r--sync/engine/apply_control_data_updates_unittest.cc347
-rw-r--r--sync/engine/apply_updates_command.cc8
-rw-r--r--sync/engine/apply_updates_command_unittest.cc363
-rw-r--r--sync/engine/conflict_resolver.cc100
-rw-r--r--sync/engine/conflict_resolver.h4
-rw-r--r--sync/engine/conflict_util.cc51
-rw-r--r--sync/engine/conflict_util.h31
-rw-r--r--sync/engine/syncer.cc4
-rw-r--r--sync/engine/syncer_util.cc48
-rw-r--r--sync/internal_api/public/base/model_type.h37
-rw-r--r--sync/internal_api/public/sync_encryption_handler.cc4
-rw-r--r--sync/internal_api/sync_encryption_handler_impl.cc18
-rw-r--r--sync/internal_api/sync_encryption_handler_impl_unittest.cc61
-rw-r--r--sync/internal_api/sync_manager_impl_unittest.cc18
-rw-r--r--sync/sync.gyp9
-rw-r--r--sync/syncable/model_type.cc28
-rw-r--r--sync/syncable/nigori_util.cc6
29 files changed, 910 insertions, 673 deletions
diff --git a/chrome/browser/sync/backend_migrator.cc b/chrome/browser/sync/backend_migrator.cc
index 72e711c..e6c5002 100644
--- a/chrome/browser/sync/backend_migrator.cc
+++ b/chrome/browser/sync/backend_migrator.cc
@@ -101,22 +101,10 @@ bool BackendMigrator::TryStart() {
void BackendMigrator::RestartMigration() {
// We'll now disable any running types that need to be migrated.
ChangeState(DISABLING_TYPES);
- const ModelTypeSet full_set = service_->GetPreferredDataTypes();
- const ModelTypeSet difference = Difference(full_set, to_migrate_);
- bool configure_with_nigori = !to_migrate_.Has(syncer::NIGORI);
SDVLOG(1) << "BackendMigrator disabling types "
- << ModelTypeSetToString(to_migrate_) << "; configuring "
- << ModelTypeSetToString(difference)
- << (configure_with_nigori ? " with nigori" : " without nigori");
-
- // Add nigori for config or not based upon if the server told us to migrate
- // nigori or not.
- if (configure_with_nigori) {
- manager_->Configure(difference, syncer::CONFIGURE_REASON_MIGRATION);
- } else {
- manager_->ConfigureWithoutNigori(difference,
- syncer::CONFIGURE_REASON_MIGRATION);
- }
+ << ModelTypeSetToString(to_migrate_);
+
+ manager_->PurgeForMigration(to_migrate_, syncer::CONFIGURE_REASON_MIGRATION);
}
void BackendMigrator::OnConfigureDone(
diff --git a/chrome/browser/sync/backend_migrator_unittest.cc b/chrome/browser/sync/backend_migrator_unittest.cc
index 7331072..bc28fcb 100644
--- a/chrome/browser/sync/backend_migrator_unittest.cc
+++ b/chrome/browser/sync/backend_migrator_unittest.cc
@@ -130,8 +130,12 @@ TEST_F(SyncBackendMigratorTest, Sanity) {
EXPECT_CALL(*manager(), state())
.WillOnce(Return(DataTypeManager::CONFIGURED));
- EXPECT_CALL(*manager(), Configure(_, syncer::CONFIGURE_REASON_MIGRATION))
- .Times(2);
+ EXPECT_CALL(
+ *manager(),
+ PurgeForMigration(_, syncer::CONFIGURE_REASON_MIGRATION)).Times(1);
+ EXPECT_CALL(
+ *manager(),
+ Configure(_, syncer::CONFIGURE_REASON_MIGRATION)).Times(1);
migrator()->MigrateTypes(to_migrate);
EXPECT_EQ(BackendMigrator::DISABLING_TYPES, migrator()->state());
@@ -158,7 +162,7 @@ TEST_F(SyncBackendMigratorTest, MigrateNigori) {
EXPECT_CALL(*manager(), state())
.WillOnce(Return(DataTypeManager::CONFIGURED));
- EXPECT_CALL(*manager(), ConfigureWithoutNigori(_,
+ EXPECT_CALL(*manager(), PurgeForMigration(_,
syncer::CONFIGURE_REASON_MIGRATION));
migrator()->MigrateTypes(to_migrate);
@@ -189,7 +193,8 @@ TEST_F(SyncBackendMigratorTest, WaitToStart) {
Mock::VerifyAndClearExpectations(manager());
EXPECT_CALL(*manager(), state())
.WillOnce(Return(DataTypeManager::CONFIGURED));
- EXPECT_CALL(*manager(), Configure(_, syncer::CONFIGURE_REASON_MIGRATION));
+ EXPECT_CALL(*manager(),
+ PurgeForMigration(_, syncer::CONFIGURE_REASON_MIGRATION));
SetUnsyncedTypes(syncer::ModelTypeSet());
SendConfigureDone(DataTypeManager::OK, syncer::ModelTypeSet());
@@ -208,8 +213,9 @@ TEST_F(SyncBackendMigratorTest, RestartMigration) {
EXPECT_CALL(*manager(), state())
.WillOnce(Return(DataTypeManager::CONFIGURED));
- EXPECT_CALL(*manager(), Configure(_, syncer::CONFIGURE_REASON_MIGRATION))
- .Times(2);
+ EXPECT_CALL(
+ *manager(),
+ PurgeForMigration(_, syncer::CONFIGURE_REASON_MIGRATION)).Times(2);
migrator()->MigrateTypes(to_migrate1);
EXPECT_EQ(BackendMigrator::DISABLING_TYPES, migrator()->state());
@@ -219,8 +225,11 @@ TEST_F(SyncBackendMigratorTest, RestartMigration) {
Difference(preferred_types(), to_migrate1);
Mock::VerifyAndClearExpectations(manager());
+ EXPECT_CALL(
+ *manager(),
+ PurgeForMigration(_, syncer::CONFIGURE_REASON_MIGRATION)).Times(1);
EXPECT_CALL(*manager(), Configure(_, syncer::CONFIGURE_REASON_MIGRATION))
- .Times(2);
+ .Times(1);
SetUnsyncedTypes(to_migrate1);
SendConfigureDone(DataTypeManager::OK, difference1);
EXPECT_EQ(BackendMigrator::DISABLING_TYPES, migrator()->state());
@@ -241,13 +250,13 @@ TEST_F(SyncBackendMigratorTest, InterruptedWhileDisablingTypes) {
EXPECT_CALL(*manager(), state())
.WillOnce(Return(DataTypeManager::CONFIGURED));
- EXPECT_CALL(*manager(), Configure(HasModelTypes(difference),
+ EXPECT_CALL(*manager(), PurgeForMigration(HasModelTypes(to_migrate),
syncer::CONFIGURE_REASON_MIGRATION));
migrator()->MigrateTypes(to_migrate);
EXPECT_EQ(BackendMigrator::DISABLING_TYPES, migrator()->state());
Mock::VerifyAndClearExpectations(manager());
- EXPECT_CALL(*manager(), Configure(HasModelTypes(difference),
+ EXPECT_CALL(*manager(), PurgeForMigration(HasModelTypes(to_migrate),
syncer::CONFIGURE_REASON_MIGRATION));
SetUnsyncedTypes(syncer::ModelTypeSet());
SendConfigureDone(DataTypeManager::OK, preferred_types());
@@ -266,8 +275,12 @@ TEST_F(SyncBackendMigratorTest, WaitingForPurge) {
EXPECT_CALL(*manager(), state())
.WillOnce(Return(DataTypeManager::CONFIGURED));
- EXPECT_CALL(*manager(), Configure(_, syncer::CONFIGURE_REASON_MIGRATION))
- .Times(2);
+ EXPECT_CALL(
+ *manager(),
+ PurgeForMigration(_, syncer::CONFIGURE_REASON_MIGRATION)).Times(1);
+ EXPECT_CALL(
+ *manager(),
+ Configure(_, syncer::CONFIGURE_REASON_MIGRATION)).Times(1);
migrator()->MigrateTypes(to_migrate);
EXPECT_EQ(BackendMigrator::DISABLING_TYPES, migrator()->state());
@@ -292,8 +305,12 @@ TEST_F(SyncBackendMigratorTest, MigratedTypeDisabledByUserDuringMigration) {
EXPECT_CALL(*manager(), state())
.WillOnce(Return(DataTypeManager::CONFIGURED));
- EXPECT_CALL(*manager(), Configure(_, syncer::CONFIGURE_REASON_MIGRATION))
- .Times(2);
+ EXPECT_CALL(
+ *manager(),
+ PurgeForMigration(_, syncer::CONFIGURE_REASON_MIGRATION)).Times(1);
+ EXPECT_CALL(
+ *manager(),
+ Configure(_, syncer::CONFIGURE_REASON_MIGRATION)).Times(1);
migrator()->MigrateTypes(to_migrate);
RemovePreferredType(syncer::PREFERENCES);
@@ -311,8 +328,9 @@ TEST_F(SyncBackendMigratorTest, ConfigureFailure) {
EXPECT_CALL(*manager(), state())
.WillOnce(Return(DataTypeManager::CONFIGURED));
- EXPECT_CALL(*manager(), Configure(_, syncer::CONFIGURE_REASON_MIGRATION))
- .Times(1);
+ EXPECT_CALL(
+ *manager(),
+ PurgeForMigration(_, syncer::CONFIGURE_REASON_MIGRATION)).Times(1);
migrator()->MigrateTypes(to_migrate);
SetUnsyncedTypes(syncer::ModelTypeSet());
SendConfigureDone(DataTypeManager::ABORTED, syncer::ModelTypeSet());
diff --git a/chrome/browser/sync/glue/data_type_manager.h b/chrome/browser/sync/glue/data_type_manager.h
index 0351088..18db082 100644
--- a/chrome/browser/sync/glue/data_type_manager.h
+++ b/chrome/browser/sync/glue/data_type_manager.h
@@ -93,8 +93,8 @@ class DataTypeManager {
virtual void Configure(TypeSet desired_types,
syncer::ConfigureReason reason) = 0;
- virtual void ConfigureWithoutNigori(TypeSet desired_types,
- syncer::ConfigureReason reason) = 0;
+ virtual void PurgeForMigration(TypeSet undesired_types,
+ syncer::ConfigureReason reason) = 0;
// Synchronously stops all registered data types. If called after
// Configure() is called but before it finishes, it will abort the
diff --git a/chrome/browser/sync/glue/data_type_manager_impl.cc b/chrome/browser/sync/glue/data_type_manager_impl.cc
index d9e0bb4..6c81668 100644
--- a/chrome/browser/sync/glue/data_type_manager_impl.cc
+++ b/chrome/browser/sync/glue/data_type_manager_impl.cc
@@ -46,15 +46,15 @@ DataTypeManagerImpl::~DataTypeManagerImpl() {}
void DataTypeManagerImpl::Configure(TypeSet desired_types,
syncer::ConfigureReason reason) {
- desired_types.Put(syncer::NIGORI);
+ desired_types.PutAll(syncer::ControlTypes());
ConfigureImpl(desired_types, reason);
}
-void DataTypeManagerImpl::ConfigureWithoutNigori(
- TypeSet desired_types,
+void DataTypeManagerImpl::PurgeForMigration(
+ TypeSet undesired_types,
syncer::ConfigureReason reason) {
- DCHECK(!desired_types.Has(syncer::NIGORI));
- ConfigureImpl(desired_types, reason);
+ TypeSet remainder = Difference(last_requested_types_, undesired_types);
+ ConfigureImpl(remainder, reason);
}
void DataTypeManagerImpl::ConfigureImpl(
@@ -114,8 +114,8 @@ void DataTypeManagerImpl::Restart(syncer::ConfigureReason reason) {
controllers_->begin(); it != controllers_->end(); ++it) {
all_types.Put(it->first);
}
- // NIGORI has no controller. We must add it manually.
- all_types.Put(syncer::NIGORI);
+ // These have no controller. We must add them manually.
+ all_types.PutAll(syncer::ControlTypes());
const syncer::ModelTypeSet types_to_add = last_requested_types_;
// Check that types_to_add \subseteq all_types.
DCHECK(all_types.HasAll(types_to_add));
diff --git a/chrome/browser/sync/glue/data_type_manager_impl.h b/chrome/browser/sync/glue/data_type_manager_impl.h
index da417c0..9f38a45 100644
--- a/chrome/browser/sync/glue/data_type_manager_impl.h
+++ b/chrome/browser/sync/glue/data_type_manager_impl.h
@@ -36,8 +36,8 @@ class DataTypeManagerImpl : public DataTypeManager,
syncer::ConfigureReason reason) OVERRIDE;
// Needed only for backend migration.
- virtual void ConfigureWithoutNigori(
- TypeSet desired_types,
+ virtual void PurgeForMigration(
+ TypeSet undesired_types,
syncer::ConfigureReason reason) OVERRIDE;
virtual void Stop() OVERRIDE;
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 f15f7dd..c0a5142 100644
--- a/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc
+++ b/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc
@@ -26,15 +26,11 @@ using syncer::BOOKMARKS;
using syncer::APPS;
using syncer::PASSWORDS;
using syncer::PREFERENCES;
+using syncer::NIGORI;
using testing::_;
using testing::Mock;
using testing::ResultOf;
-enum NigoriState {
- WITH_NIGORI,
- WITHOUT_NIGORI
-};
-
// Fake BackendDataTypeConfigurer implementation that simply stores away the
// callback passed into ConfigureDataTypes.
class FakeBackendDataTypeConfigurer : public BackendDataTypeConfigurer {
@@ -80,8 +76,7 @@ DataTypeManager::ConfigureStatus GetStatus(
// 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::TestWithParam<NigoriState> {
+class SyncDataTypeManagerImplTest : public testing::Test {
public:
SyncDataTypeManagerImplTest()
: ui_thread_(content::BrowserThread::UI, &ui_loop_) {}
@@ -93,11 +88,6 @@ class SyncDataTypeManagerImplTest
virtual void SetUp() {
}
- // A clearer name for the param accessor.
- NigoriState GetNigoriState() {
- return GetParam();
- }
-
void SetConfigureStartExpectation() {
EXPECT_CALL(observer_, OnConfigureStart());
}
@@ -113,13 +103,7 @@ class SyncDataTypeManagerImplTest
// Configure the given DTM with the given desired types.
void Configure(DataTypeManagerImpl* dtm,
const DataTypeManager::TypeSet& desired_types) {
- const syncer::ConfigureReason kReason =
- syncer::CONFIGURE_REASON_RECONFIGURATION;
- if (GetNigoriState() == WITH_NIGORI) {
- dtm->Configure(desired_types, kReason);
- } else {
- dtm->ConfigureWithoutNigori(desired_types, kReason);
- }
+ dtm->Configure(desired_types, syncer::CONFIGURE_REASON_RECONFIGURATION);
}
// Finish downloading for the given DTM. Should be done only after
@@ -159,7 +143,7 @@ class SyncDataTypeManagerImplTest
// Set up a DTM with no controllers, configure it, finish downloading,
// and then stop it.
-TEST_P(SyncDataTypeManagerImplTest, NoControllers) {
+TEST_F(SyncDataTypeManagerImplTest, NoControllers) {
DataTypeManagerImpl dtm(&configurer_, &controllers_, &observer_);
SetConfigureStartExpectation();
SetConfigureDoneExpectation(DataTypeManager::OK);
@@ -176,7 +160,7 @@ TEST_P(SyncDataTypeManagerImplTest, NoControllers) {
// Set up a DTM with a single controller, configure it, finish
// downloading, finish starting the controller, and then stop the DTM.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureOne) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureOne) {
AddController(BOOKMARKS);
DataTypeManagerImpl dtm(&configurer_, &controllers_, &observer_);
@@ -198,7 +182,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureOne) {
// Set up a DTM with 2 controllers. configure it. One of them finishes loading
// after the timeout. Make sure eventually all are configured.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureSlowLoadingType) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureSlowLoadingType) {
AddController(BOOKMARKS);
AddController(APPS);
@@ -245,7 +229,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureSlowLoadingType) {
// Set up a DTM with a single controller, configure it, but stop it
// before finishing the download. It should still be safe to run the
// download callback even after the DTM is stopped and destroyed.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureOneStopWhileDownloadPending) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureOneStopWhileDownloadPending) {
AddController(BOOKMARKS);
{
@@ -267,7 +251,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureOneStopWhileDownloadPending) {
// downloading, but stop the DTM before the controller finishes
// starting up. It should still be safe to finish starting up the
// controller even after the DTM is stopped and destroyed.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureOneStopWhileStartingModel) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureOneStopWhileStartingModel) {
AddController(BOOKMARKS);
{
@@ -293,7 +277,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureOneStopWhileStartingModel) {
// the controller finishes starting up. It should still be safe to
// finish starting up the controller even after the DTM is stopped and
// destroyed.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureOneStopWhileAssociating) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureOneStopWhileAssociating) {
AddController(BOOKMARKS);
{
DataTypeManagerImpl dtm(&configurer_, &controllers_, &observer_);
@@ -322,7 +306,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureOneStopWhileAssociating) {
// 5) Finish the download for step 4.
// 6) Finish starting the controller successfully.
// 7) Stop the DTM.
-TEST_P(SyncDataTypeManagerImplTest, OneWaitingForCrypto) {
+TEST_F(SyncDataTypeManagerImplTest, OneWaitingForCrypto) {
AddController(PASSWORDS);
DataTypeManagerImpl dtm(&configurer_, &controllers_, &observer_);
@@ -372,7 +356,7 @@ TEST_P(SyncDataTypeManagerImplTest, OneWaitingForCrypto) {
// 5) Finish the download for step 4.
// 6) Finish starting the second controller.
// 7) Stop the DTM.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureOneThenBoth) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureOneThenBoth) {
AddController(BOOKMARKS);
AddController(PREFERENCES);
@@ -422,7 +406,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureOneThenBoth) {
// 5) Finish the download for step 4.
// 6) Finish starting the second controller.
// 7) Stop the DTM.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureOneThenSwitch) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureOneThenSwitch) {
AddController(BOOKMARKS);
AddController(PREFERENCES);
@@ -472,7 +456,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureOneThenSwitch) {
// 5) Finish the download for step 3.
// 6) Finish starting the second controller.
// 7) Stop the DTM.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureWhileOneInFlight) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureWhileOneInFlight) {
AddController(BOOKMARKS);
AddController(PREFERENCES);
@@ -518,7 +502,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureWhileOneInFlight) {
// Set up a DTM with one controller. Then configure, finish
// downloading, and start the controller with an unrecoverable error.
// The unrecoverable error should cause the DTM to stop.
-TEST_P(SyncDataTypeManagerImplTest, OneFailingController) {
+TEST_F(SyncDataTypeManagerImplTest, OneFailingController) {
AddController(BOOKMARKS);
DataTypeManagerImpl dtm(&configurer_, &controllers_, &observer_);
@@ -544,7 +528,7 @@ TEST_P(SyncDataTypeManagerImplTest, OneFailingController) {
// 4) Finish starting the second controller with an unrecoverable error.
//
// The failure from step 4 should cause the DTM to stop.
-TEST_P(SyncDataTypeManagerImplTest, SecondControllerFails) {
+TEST_F(SyncDataTypeManagerImplTest, SecondControllerFails) {
AddController(BOOKMARKS);
AddController(PREFERENCES);
@@ -582,7 +566,7 @@ TEST_P(SyncDataTypeManagerImplTest, SecondControllerFails) {
//
// TODO(akalin): Check that the data type that failed association is
// recorded in the CONFIGURE_DONE notification.
-TEST_P(SyncDataTypeManagerImplTest, OneControllerFailsAssociation) {
+TEST_F(SyncDataTypeManagerImplTest, OneControllerFailsAssociation) {
AddController(BOOKMARKS);
AddController(PREFERENCES);
@@ -620,7 +604,7 @@ TEST_P(SyncDataTypeManagerImplTest, OneControllerFailsAssociation) {
// 4) Finish the download for step 2.
// 5) Finish starting both controllers.
// 6) Stop the DTM.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureWhileDownloadPending) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureWhileDownloadPending) {
AddController(BOOKMARKS);
AddController(PREFERENCES);
@@ -672,7 +656,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureWhileDownloadPending) {
//
// The failure from step 3 should be ignored since there's a
// reconfigure pending from step 2.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureWhileDownloadPendingWithFailure) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureWhileDownloadPendingWithFailure) {
AddController(BOOKMARKS);
AddController(PREFERENCES);
@@ -713,12 +697,102 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureWhileDownloadPendingWithFailure) {
EXPECT_EQ(DataTypeManager::STOPPED, dtm.state());
}
-INSTANTIATE_TEST_CASE_P(
- WithoutNigori, SyncDataTypeManagerImplTest,
- ::testing::Values(WITHOUT_NIGORI));
+// Tests a Purge then Configure. This is similar to the sequence of
+// operations that would be invoked by the BackendMigrator.
+TEST_F(SyncDataTypeManagerImplTest, MigrateAll) {
+ AddController(BOOKMARKS);
+
+ DataTypeManagerImpl dtm(&configurer_, &controllers_, &observer_);
+ SetConfigureStartExpectation();
+ SetConfigureDoneExpectation(DataTypeManager::OK);
+
+ // Initial setup.
+ Configure(&dtm, ModelTypeSet(BOOKMARKS));
+ FinishDownload(dtm, ModelTypeSet());
+ GetController(BOOKMARKS)->FinishStart(DataTypeController::OK);
+
+ // We've now configured bookmarks and (implicitly) the control types.
+ EXPECT_EQ(DataTypeManager::CONFIGURED, dtm.state());
+ Mock::VerifyAndClearExpectations(&observer_);
+
+ // Pretend we were told to migrate all types.
+ ModelTypeSet to_migrate;
+ to_migrate.Put(BOOKMARKS);
+ to_migrate.PutAll(syncer::ControlTypes());
-INSTANTIATE_TEST_CASE_P(
- WithNigori, SyncDataTypeManagerImplTest,
- ::testing::Values(WITH_NIGORI));
+ SetConfigureStartExpectation();
+ SetConfigureDoneExpectation(DataTypeManager::OK);
+ dtm.PurgeForMigration(to_migrate,
+ syncer::CONFIGURE_REASON_MIGRATION);
+ EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm.state());
+
+ // The DTM will call ConfigureDataTypes(), even though it is unnecessary.
+ FinishDownload(dtm, ModelTypeSet());
+ EXPECT_EQ(DataTypeManager::CONFIGURED, dtm.state());
+ Mock::VerifyAndClearExpectations(&observer_);
+
+ // Re-enable the migrated types.
+ SetConfigureStartExpectation();
+ SetConfigureDoneExpectation(DataTypeManager::OK);
+ Configure(&dtm, to_migrate);
+ FinishDownload(dtm, ModelTypeSet());
+ GetController(BOOKMARKS)->FinishStart(DataTypeController::OK);
+ EXPECT_EQ(DataTypeManager::CONFIGURED, dtm.state());
+}
+
+// Test receipt of a Configure request while a purge is in flight.
+TEST_F(SyncDataTypeManagerImplTest, ConfigureDuringPurge) {
+ AddController(BOOKMARKS);
+ AddController(PREFERENCES);
+
+ DataTypeManagerImpl dtm(&configurer_, &controllers_, &observer_);
+
+ // Initial configure.
+ SetConfigureStartExpectation();
+ SetConfigureDoneExpectation(DataTypeManager::OK);
+ Configure(&dtm, ModelTypeSet(BOOKMARKS));
+ FinishDownload(dtm, ModelTypeSet());
+ GetController(BOOKMARKS)->FinishStart(DataTypeController::OK);
+ EXPECT_EQ(DataTypeManager::CONFIGURED, dtm.state());
+ Mock::VerifyAndClearExpectations(&observer_);
+
+ // Purge the Nigori type.
+ SetConfigureStartExpectation();
+ dtm.PurgeForMigration(ModelTypeSet(NIGORI),
+ syncer::CONFIGURE_REASON_MIGRATION);
+ EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm.state());
+ Mock::VerifyAndClearExpectations(&observer_);
+
+ // Before the backend configuration completes, ask for a different
+ // set of types. This request asks for
+ // - BOOKMARKS: which is redundant because it was already enabled,
+ // - PREFERENCES: which is new and will need to be downloaded, and
+ // - NIGORI: (added implicitly because it is a control type) which
+ // the DTM is part-way through purging.
+ SetConfigureBlockedExpectation();
+ Configure(&dtm, ModelTypeSet(BOOKMARKS, PREFERENCES));
+ EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm.state());
+
+ // Invoke the callback we've been waiting for since we asked to purge NIGORI.
+ FinishDownload(dtm, ModelTypeSet());
+ EXPECT_EQ(DataTypeManager::BLOCKED, dtm.state());
+ Mock::VerifyAndClearExpectations(&observer_);
+
+ SetConfigureDoneExpectation(DataTypeManager::OK);
+ // Pump the loop to run the posted DTMI::ConfigureImpl() task from
+ // DTMI::ProcessReconfigure().
+ ui_loop_.RunAllPending();
+ EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm.state());
+
+ // Now invoke the callback for the second configure request.
+ FinishDownload(dtm, ModelTypeSet());
+ EXPECT_EQ(DataTypeManager::CONFIGURING, dtm.state());
+
+ // Start the preferences controller. We don't need to start controller for
+ // the NIGORI because it has none. We don't need to start the controller for
+ // the BOOKMARKS because it was never stopped.
+ GetController(PREFERENCES)->FinishStart(DataTypeController::OK);
+ EXPECT_EQ(DataTypeManager::CONFIGURED, dtm.state());
+}
} // namespace browser_sync
diff --git a/chrome/browser/sync/glue/data_type_manager_mock.h b/chrome/browser/sync/glue/data_type_manager_mock.h
index 5f6d192..a1447b3 100644
--- a/chrome/browser/sync/glue/data_type_manager_mock.h
+++ b/chrome/browser/sync/glue/data_type_manager_mock.h
@@ -18,8 +18,7 @@ class DataTypeManagerMock : public DataTypeManager {
virtual ~DataTypeManagerMock();
MOCK_METHOD2(Configure, void(TypeSet, syncer::ConfigureReason));
- MOCK_METHOD2(ConfigureWithoutNigori,
- void(TypeSet, syncer::ConfigureReason));
+ MOCK_METHOD2(PurgeForMigration, void(TypeSet, syncer::ConfigureReason));
MOCK_METHOD0(Stop, void());
MOCK_METHOD0(controllers, const DataTypeController::TypeMap&());
MOCK_CONST_METHOD0(state, State());
diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc
index 579a40c..7d1711d 100644
--- a/chrome/browser/sync/glue/sync_backend_host.cc
+++ b/chrome/browser/sync/glue/sync_backend_host.cc
@@ -1349,7 +1349,7 @@ void SyncBackendHost::HandleInitializationCompletedOnFrontendLoop(
initialization_state_ = DOWNLOADING_NIGORI;
ConfigureDataTypes(
syncer::CONFIGURE_REASON_NEW_CLIENT,
- syncer::ModelTypeSet(syncer::NIGORI),
+ syncer::ModelTypeSet(syncer::ControlTypes()),
syncer::ModelTypeSet(),
// Calls back into this function.
base::Bind(
diff --git a/chrome/browser/sync/glue/sync_backend_host_unittest.cc b/chrome/browser/sync/glue/sync_backend_host_unittest.cc
index ec96c74..5a4ae88 100644
--- a/chrome/browser/sync/glue/sync_backend_host_unittest.cc
+++ b/chrome/browser/sync/glue/sync_backend_host_unittest.cc
@@ -204,7 +204,7 @@ class SyncBackendHostTest : public testing::Test {
// Synchronously configures the backend's datatypes.
void ConfigureDataTypes(syncer::ModelTypeSet types_to_add,
syncer::ModelTypeSet types_to_remove) {
- types_to_add.Put(syncer::NIGORI);
+ types_to_add.PutAll(syncer::ControlTypes());
backend_->ConfigureDataTypes(
syncer::CONFIGURE_REASON_RECONFIGURATION,
types_to_add,
@@ -247,28 +247,28 @@ class SyncBackendHostTest : public testing::Test {
TEST_F(SyncBackendHostTest, InitShutdown) {
InitializeBackend();
EXPECT_TRUE(fake_manager_->GetAndResetDownloadedTypes().Equals(
- syncer::ModelTypeSet(syncer::NIGORI)));
+ syncer::ControlTypes()));
EXPECT_TRUE(fake_manager_->InitialSyncEndedTypes().Equals(
- syncer::ModelTypeSet(syncer::NIGORI)));
+ syncer::ControlTypes()));
EXPECT_TRUE(fake_manager_->GetTypesWithEmptyProgressMarkerToken(
- syncer::ModelTypeSet(syncer::NIGORI)).Empty());
+ syncer::ControlTypes()).Empty());
}
// Test first time sync scenario. All types should be properly configured.
TEST_F(SyncBackendHostTest, FirstTimeSync) {
InitializeBackend();
EXPECT_TRUE(fake_manager_->GetAndResetDownloadedTypes().Equals(
- syncer::ModelTypeSet(syncer::NIGORI)));
+ syncer::ControlTypes()));
EXPECT_TRUE(fake_manager_->InitialSyncEndedTypes().Equals(
- syncer::ModelTypeSet(syncer::NIGORI)));
+ syncer::ControlTypes()));
EXPECT_TRUE(fake_manager_->GetTypesWithEmptyProgressMarkerToken(
- syncer::ModelTypeSet(syncer::NIGORI)).Empty());
+ syncer::ControlTypes()).Empty());
ConfigureDataTypes(enabled_types_,
Difference(syncer::ModelTypeSet::All(),
enabled_types_));
EXPECT_TRUE(fake_manager_->GetAndResetDownloadedTypes().HasAll(
- enabled_types_));
+ Difference(enabled_types_, syncer::ControlTypes())));
EXPECT_TRUE(fake_manager_->InitialSyncEndedTypes().Equals(enabled_types_));
EXPECT_TRUE(fake_manager_->GetAndResetEnabledTypes().Equals(enabled_types_));
EXPECT_TRUE(fake_manager_->GetTypesWithEmptyProgressMarkerToken(
@@ -348,12 +348,12 @@ TEST_F(SyncBackendHostTest, LostDB) {
// left untouched.
InitializeBackend();
EXPECT_TRUE(fake_manager_->GetAndResetDownloadedTypes().Equals(
- syncer::ModelTypeSet(syncer::NIGORI)));
+ syncer::ModelTypeSet(syncer::ControlTypes())));
EXPECT_TRUE(fake_manager_->InitialSyncEndedTypes().Equals(
- syncer::ModelTypeSet(syncer::NIGORI)));
+ syncer::ModelTypeSet(syncer::ControlTypes())));
EXPECT_TRUE(fake_manager_->GetTypesWithEmptyProgressMarkerToken(
enabled_types_).Equals(
- Difference(enabled_types_, syncer::ModelTypeSet(syncer::NIGORI))));
+ Difference(enabled_types_, syncer::ControlTypes())));
// The database was empty, so any cleaning is entirely optional. We want to
// reset this value before running the next part of the test, though.
@@ -364,7 +364,7 @@ TEST_F(SyncBackendHostTest, LostDB) {
Difference(syncer::ModelTypeSet::All(),
enabled_types_));
EXPECT_TRUE(fake_manager_->GetAndResetDownloadedTypes().HasAll(
- enabled_types_));
+ Difference(enabled_types_, syncer::ControlTypes())));
EXPECT_TRUE(Intersection(fake_manager_->GetAndResetCleanedTypes(),
enabled_types_).Empty());
EXPECT_TRUE(fake_manager_->InitialSyncEndedTypes().Equals(enabled_types_));
diff --git a/chrome/browser/sync/sync_prefs.cc b/chrome/browser/sync/sync_prefs.cc
index 2ba2dd9..1b7a92f 100644
--- a/chrome/browser/sync/sync_prefs.cc
+++ b/chrome/browser/sync/sync_prefs.cc
@@ -336,15 +336,15 @@ void SyncPrefs::RegisterPreferences() {
enable_by_default,
PrefService::UNSYNCABLE_PREF);
+ syncer::ModelTypeSet user_types = syncer::UserTypes();
+
// Treat bookmarks specially.
RegisterDataTypePreferredPref(syncer::BOOKMARKS, true);
- for (int i = syncer::PREFERENCES; i < syncer::MODEL_TYPE_COUNT; ++i) {
- const syncer::ModelType type = syncer::ModelTypeFromInt(i);
- // Also treat nigori specially.
- if (type == syncer::NIGORI) {
- continue;
- }
- RegisterDataTypePreferredPref(type, enable_by_default);
+ user_types.Remove(syncer::BOOKMARKS);
+
+ for (syncer::ModelTypeSet::Iterator it = user_types.First();
+ it.Good(); it.Inc()) {
+ RegisterDataTypePreferredPref(it.Get(), enable_by_default);
}
pref_service_->RegisterBooleanPref(prefs::kSyncManaged,
diff --git a/sync/engine/apply_control_data_updates.cc b/sync/engine/apply_control_data_updates.cc
new file mode 100644
index 0000000..2aa7dd4
--- /dev/null
+++ b/sync/engine/apply_control_data_updates.cc
@@ -0,0 +1,138 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "sync/engine/apply_control_data_updates.h"
+
+#include "base/metrics/histogram.h"
+#include "sync/engine/conflict_resolver.h"
+#include "sync/engine/conflict_util.h"
+#include "sync/engine/syncer_util.h"
+#include "sync/syncable/directory.h"
+#include "sync/syncable/mutable_entry.h"
+#include "sync/syncable/nigori_handler.h"
+#include "sync/syncable/nigori_util.h"
+#include "sync/syncable/write_transaction.h"
+#include "sync/util/cryptographer.h"
+
+namespace syncer {
+
+using syncable::GET_BY_SERVER_TAG;
+using syncable::IS_UNAPPLIED_UPDATE;
+using syncable::IS_UNSYNCED;
+using syncable::SERVER_SPECIFICS;
+using syncable::SPECIFICS;
+using syncable::SYNCER;
+
+void ApplyControlDataUpdates(syncable::Directory* dir) {
+ syncable::WriteTransaction trans(FROM_HERE, SYNCER, dir);
+
+ if (ApplyNigoriUpdates(&trans, dir->GetCryptographer(&trans))) {
+ dir->set_initial_sync_ended_for_type(NIGORI, true);
+ }
+}
+
+// Update the sync encryption handler with the server's nigori node.
+//
+// If we have a locally modified nigori node, we merge them manually. This
+// handles the case where two clients both set a different passphrase. The
+// second client to attempt to commit will go into a state of having pending
+// keys, unioned the set of encrypted types, and eventually re-encrypt
+// everything with the passphrase of the first client and commit the set of
+// merged encryption keys. Until the second client provides the pending
+// passphrase, the cryptographer will preserve the encryption keys based on the
+// local passphrase, while the nigori node will preserve the server encryption
+// keys.
+bool ApplyNigoriUpdates(syncable::WriteTransaction* trans,
+ Cryptographer* cryptographer) {
+ syncable::MutableEntry nigori_node(trans, GET_BY_SERVER_TAG,
+ ModelTypeToRootTag(NIGORI));
+
+ // Mainly for unit tests. We should have a Nigori node by this point.
+ if (!nigori_node.good()) {
+ return false;
+ }
+
+ if (!nigori_node.Get(IS_UNAPPLIED_UPDATE)) {
+ return true;
+ }
+
+ const sync_pb::NigoriSpecifics& nigori =
+ nigori_node.Get(SERVER_SPECIFICS).nigori();
+ trans->directory()->GetNigoriHandler()->ApplyNigoriUpdate(nigori, trans);
+
+ // Make sure any unsynced changes are properly encrypted as necessary.
+ // We only perform this if the cryptographer is ready. If not, these are
+ // re-encrypted at SetDecryptionPassphrase time (via ReEncryptEverything).
+ // This logic covers the case where the nigori update marked new datatypes
+ // for encryption, but didn't change the passphrase.
+ if (cryptographer->is_ready()) {
+ // Note that we don't bother to encrypt any data for which IS_UNSYNCED
+ // == false here. The machine that turned on encryption should know about
+ // and re-encrypt all synced data. It's possible it could get interrupted
+ // during this process, but we currently reencrypt everything at startup
+ // as well, so as soon as a client is restarted with this datatype marked
+ // for encryption, all the data should be updated as necessary.
+
+ // If this fails, something is wrong with the cryptographer, but there's
+ // nothing we can do about it here.
+ DVLOG(1) << "Received new nigori, encrypting unsynced changes.";
+ syncable::ProcessUnsyncedChangesForEncryption(trans);
+ }
+
+ if (!nigori_node.Get(IS_UNSYNCED)) { // Update only.
+ UpdateLocalDataFromServerData(trans, &nigori_node);
+ } else { // Conflict.
+ // Create a new set of specifics based on the server specifics (which
+ // preserves their encryption keys).
+ sync_pb::EntitySpecifics specifics = nigori_node.Get(SERVER_SPECIFICS);
+ sync_pb::NigoriSpecifics* server_nigori = specifics.mutable_nigori();
+ // Store the merged set of encrypted types.
+ // (NigoriHandler::ApplyNigoriUpdate(..) will have merged the local types
+ // already).
+ trans->directory()->GetNigoriHandler()->UpdateNigoriFromEncryptedTypes(
+ server_nigori,
+ trans);
+ // The cryptographer has the both the local and remote encryption keys
+ // (added at NigoriHandler::ApplyNigoriUpdate(..) time).
+ // If the cryptographer is ready, then it already merged both sets of keys
+ // and we can store them back in. In that case, the remote key was already
+ // part of the local keybag, so we preserve the local key as the default
+ // (including whether it's an explicit key).
+ // If the cryptographer is not ready, then the user will have to provide
+ // the passphrase to decrypt the pending keys. When they do so, the
+ // SetDecryptionPassphrase code will act based on whether the server
+ // update has an explicit passphrase or not.
+ // - If the server had an explicit passphrase, that explicit passphrase
+ // will be preserved as the default encryption key.
+ // - If the server did not have an explicit passphrase, we assume the
+ // local passphrase is the most up to date and preserve the local
+ // default encryption key marked as an implicit passphrase.
+ // This works fine except for the case where we had locally set an
+ // explicit passphrase. In that case the nigori node will have the default
+ // key based on the local explicit passphassphrase, but will not have it
+ // marked as explicit. To fix this we'd have to track whether we have a
+ // explicit passphrase or not separate from the nigori, which would
+ // introduce even more complexity, so we leave it up to the user to reset
+ // that passphrase as an explicit one via settings. The goal here is to
+ // ensure both sets of encryption keys are preserved.
+ if (cryptographer->is_ready()) {
+ cryptographer->GetKeys(server_nigori->mutable_encrypted());
+ server_nigori->set_using_explicit_passphrase(
+ nigori_node.Get(SPECIFICS).nigori().using_explicit_passphrase());
+ }
+ nigori_node.Put(SPECIFICS, specifics);
+ DVLOG(1) << "Resolving simple conflict, merging nigori nodes: "
+ << nigori_node;
+
+ OverwriteServerChanges(&nigori_node);
+
+ UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
+ ConflictResolver::NIGORI_MERGE,
+ ConflictResolver::CONFLICT_RESOLUTION_SIZE);
+ }
+
+ return true;
+}
+
+} // namespace syncer
diff --git a/sync/engine/apply_control_data_updates.h b/sync/engine/apply_control_data_updates.h
new file mode 100644
index 0000000..efe2cc8
--- /dev/null
+++ b/sync/engine/apply_control_data_updates.h
@@ -0,0 +1,23 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SYNC_ENGINE_APPLY_CONTROL_DATA_UPDATES_H_
+#define SYNC_ENGINE_APPLY_CONTROL_DATA_UPDATES_H_
+
+namespace syncer {
+
+class Cryptographer;
+
+namespace syncable {
+class Directory;
+class WriteTransaction;
+}
+
+void ApplyControlDataUpdates(syncable::Directory* dir);
+bool ApplyNigoriUpdates(syncable::WriteTransaction* trans,
+ Cryptographer* cryptographer);
+
+} // namespace syncer
+
+#endif // SYNC_ENGINE_APPLY_CONTROL_DATA_UPDATES_H_
diff --git a/sync/engine/apply_control_data_updates_unittest.cc b/sync/engine/apply_control_data_updates_unittest.cc
new file mode 100644
index 0000000..dbfc98d
--- /dev/null
+++ b/sync/engine/apply_control_data_updates_unittest.cc
@@ -0,0 +1,347 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/format_macros.h"
+#include "base/location.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/stringprintf.h"
+#include "sync/engine/apply_control_data_updates.h"
+#include "sync/engine/syncer.h"
+#include "sync/engine/syncer_util.h"
+#include "sync/internal_api/public/test/test_entry_factory.h"
+#include "sync/protocol/nigori_specifics.pb.h"
+#include "sync/syncable/mutable_entry.h"
+#include "sync/syncable/nigori_util.h"
+#include "sync/syncable/read_transaction.h"
+#include "sync/syncable/syncable_util.h"
+#include "sync/syncable/write_transaction.h"
+#include "sync/test/engine/fake_model_worker.h"
+#include "sync/test/engine/syncer_command_test.h"
+#include "sync/test/engine/test_id_factory.h"
+#include "sync/test/fake_sync_encryption_handler.h"
+#include "sync/util/cryptographer.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace syncer {
+
+using syncable::MutableEntry;
+using syncable::UNITTEST;
+using syncable::Id;
+
+class ApplyControlDataUpdatesTest : public SyncerCommandTest {
+ public:
+ protected:
+ ApplyControlDataUpdatesTest() {}
+ virtual ~ApplyControlDataUpdatesTest() {}
+
+ virtual void SetUp() {
+ workers()->clear();
+ mutable_routing_info()->clear();
+ workers()->push_back(make_scoped_refptr(new FakeModelWorker(GROUP_UI)));
+ workers()->push_back(
+ make_scoped_refptr(new FakeModelWorker(GROUP_PASSWORD)));
+ (*mutable_routing_info())[BOOKMARKS] = GROUP_UI;
+ (*mutable_routing_info())[PASSWORDS] = GROUP_PASSWORD;
+ (*mutable_routing_info())[NIGORI] = GROUP_PASSIVE;
+ SyncerCommandTest::SetUp();
+ entry_factory_.reset(new TestEntryFactory(directory()));
+
+ syncable::ReadTransaction trans(FROM_HERE, directory());
+ }
+
+ TestIdFactory id_factory_;
+ scoped_ptr<TestEntryFactory> entry_factory_;
+ private:
+ DISALLOW_COPY_AND_ASSIGN(ApplyControlDataUpdatesTest);
+};
+
+TEST_F(ApplyControlDataUpdatesTest, NigoriUpdate) {
+ // Storing the cryptographer separately is bad, but for this test we
+ // know it's safe.
+ Cryptographer* cryptographer;
+ ModelTypeSet encrypted_types;
+ encrypted_types.Put(PASSWORDS);
+
+ // We start with initial_sync_ended == false. This is wrong, since would have
+ // no nigori node if that were the case. Howerver, it makes it easier to
+ // verify that ApplyControlDataUpdates sets initial_sync_ended correctly.
+ EXPECT_FALSE(directory()->initial_sync_ended_types().Has(NIGORI));
+
+ {
+ syncable::ReadTransaction trans(FROM_HERE, directory());
+ cryptographer = directory()->GetCryptographer(&trans);
+ EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
+ .Equals(encrypted_types));
+ }
+
+ // Nigori node updates should update the Cryptographer.
+ Cryptographer other_cryptographer(cryptographer->encryptor());
+ KeyParams params = {"localhost", "dummy", "foobar"};
+ other_cryptographer.AddKey(params);
+
+ sync_pb::EntitySpecifics specifics;
+ sync_pb::NigoriSpecifics* nigori = specifics.mutable_nigori();
+ other_cryptographer.GetKeys(nigori->mutable_encrypted());
+ nigori->set_encrypt_everything(true);
+ entry_factory_->CreateUnappliedNewItem(
+ ModelTypeToRootTag(NIGORI), specifics, true);
+ EXPECT_FALSE(cryptographer->has_pending_keys());
+
+ ApplyControlDataUpdates(directory());
+
+ EXPECT_FALSE(cryptographer->is_ready());
+ EXPECT_TRUE(cryptographer->has_pending_keys());
+ {
+ syncable::ReadTransaction trans(FROM_HERE, directory());
+ EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
+ .Equals(ModelTypeSet::All()));
+ EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI));
+ }
+}
+
+TEST_F(ApplyControlDataUpdatesTest, NigoriUpdateForDisabledTypes) {
+ // Storing the cryptographer separately is bad, but for this test we
+ // know it's safe.
+ Cryptographer* cryptographer;
+ ModelTypeSet encrypted_types;
+ encrypted_types.Put(PASSWORDS);
+
+ // We start with initial_sync_ended == false. This is wrong, since would have
+ // no nigori node if that were the case. Howerver, it makes it easier to
+ // verify that ApplyControlDataUpdates sets initial_sync_ended correctly.
+ EXPECT_FALSE(directory()->initial_sync_ended_types().Has(NIGORI));
+
+ {
+ syncable::ReadTransaction trans(FROM_HERE, directory());
+ cryptographer = directory()->GetCryptographer(&trans);
+ EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
+ .Equals(encrypted_types));
+ }
+
+ // Nigori node updates should update the Cryptographer.
+ Cryptographer other_cryptographer(cryptographer->encryptor());
+ KeyParams params = {"localhost", "dummy", "foobar"};
+ other_cryptographer.AddKey(params);
+
+ sync_pb::EntitySpecifics specifics;
+ sync_pb::NigoriSpecifics* nigori = specifics.mutable_nigori();
+ other_cryptographer.GetKeys(nigori->mutable_encrypted());
+ nigori->set_encrypt_everything(true);
+ entry_factory_->CreateUnappliedNewItem(
+ ModelTypeToRootTag(NIGORI), specifics, true);
+ EXPECT_FALSE(cryptographer->has_pending_keys());
+
+ ApplyControlDataUpdates(directory());
+
+ EXPECT_FALSE(cryptographer->is_ready());
+ EXPECT_TRUE(cryptographer->has_pending_keys());
+ {
+ syncable::ReadTransaction trans(FROM_HERE, directory());
+ EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
+ .Equals(ModelTypeSet::All()));
+ EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI));
+ }
+}
+
+// Create some local unsynced and unencrypted data. Apply a nigori update that
+// turns on encryption for the unsynced data. Ensure we properly encrypt the
+// data as part of the nigori update. Apply another nigori update with no
+// changes. Ensure we ignore already-encrypted unsynced data and that nothing
+// breaks.
+TEST_F(ApplyControlDataUpdatesTest, EncryptUnsyncedChanges) {
+ // Storing the cryptographer separately is bad, but for this test we
+ // know it's safe.
+ Cryptographer* cryptographer;
+ ModelTypeSet encrypted_types;
+ encrypted_types.Put(PASSWORDS);
+ {
+ syncable::ReadTransaction trans(FROM_HERE, directory());
+ cryptographer = directory()->GetCryptographer(&trans);
+ EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
+ .Equals(encrypted_types));
+
+ // With default encrypted_types, this should be true.
+ EXPECT_TRUE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types));
+
+ Syncer::UnsyncedMetaHandles handles;
+ GetUnsyncedEntries(&trans, &handles);
+ EXPECT_TRUE(handles.empty());
+ }
+
+ // Create unsynced bookmarks without encryption.
+ // First item is a folder
+ Id folder_id = id_factory_.NewLocalId();
+ entry_factory_->CreateUnsyncedItem(folder_id, id_factory_.root(), "folder",
+ true, BOOKMARKS, NULL);
+ // Next five items are children of the folder
+ size_t i;
+ size_t batch_s = 5;
+ for (i = 0; i < batch_s; ++i) {
+ entry_factory_->CreateUnsyncedItem(id_factory_.NewLocalId(), folder_id,
+ base::StringPrintf("Item %"PRIuS"", i),
+ false, BOOKMARKS, NULL);
+ }
+ // Next five items are children of the root.
+ for (; i < 2*batch_s; ++i) {
+ entry_factory_->CreateUnsyncedItem(
+ id_factory_.NewLocalId(), id_factory_.root(),
+ base::StringPrintf("Item %"PRIuS"", i), false,
+ BOOKMARKS, NULL);
+ }
+
+ KeyParams params = {"localhost", "dummy", "foobar"};
+ cryptographer->AddKey(params);
+ sync_pb::EntitySpecifics specifics;
+ sync_pb::NigoriSpecifics* nigori = specifics.mutable_nigori();
+ cryptographer->GetKeys(nigori->mutable_encrypted());
+ nigori->set_encrypt_everything(true);
+ encrypted_types.Put(BOOKMARKS);
+ entry_factory_->CreateUnappliedNewItem(
+ ModelTypeToRootTag(NIGORI), specifics, true);
+ EXPECT_FALSE(cryptographer->has_pending_keys());
+ EXPECT_TRUE(cryptographer->is_ready());
+
+ {
+ // Ensure we have unsynced nodes that aren't properly encrypted.
+ syncable::ReadTransaction trans(FROM_HERE, directory());
+ EXPECT_FALSE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types));
+
+ Syncer::UnsyncedMetaHandles handles;
+ GetUnsyncedEntries(&trans, &handles);
+ EXPECT_EQ(2*batch_s+1, handles.size());
+ }
+
+ ApplyControlDataUpdates(directory());
+
+ EXPECT_FALSE(cryptographer->has_pending_keys());
+ EXPECT_TRUE(cryptographer->is_ready());
+ {
+ syncable::ReadTransaction trans(FROM_HERE, directory());
+
+ // If ProcessUnsyncedChangesForEncryption worked, all our unsynced changes
+ // should be encrypted now.
+ EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
+ .Equals(ModelTypeSet::All()));
+ EXPECT_TRUE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types));
+
+ Syncer::UnsyncedMetaHandles handles;
+ GetUnsyncedEntries(&trans, &handles);
+ EXPECT_EQ(2*batch_s+1, handles.size());
+ }
+
+ // Simulate another nigori update that doesn't change anything.
+ {
+ syncable::WriteTransaction trans(FROM_HERE, UNITTEST, directory());
+ MutableEntry entry(&trans, syncable::GET_BY_SERVER_TAG,
+ ModelTypeToRootTag(NIGORI));
+ ASSERT_TRUE(entry.good());
+ entry.Put(syncable::SERVER_VERSION, entry_factory_->GetNextRevision());
+ entry.Put(syncable::IS_UNAPPLIED_UPDATE, true);
+ }
+
+ ApplyControlDataUpdates(directory());
+
+ EXPECT_FALSE(cryptographer->has_pending_keys());
+ EXPECT_TRUE(cryptographer->is_ready());
+ {
+ syncable::ReadTransaction trans(FROM_HERE, directory());
+
+ // All our changes should still be encrypted.
+ EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
+ .Equals(ModelTypeSet::All()));
+ EXPECT_TRUE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types));
+
+ Syncer::UnsyncedMetaHandles handles;
+ GetUnsyncedEntries(&trans, &handles);
+ EXPECT_EQ(2*batch_s+1, handles.size());
+ }
+}
+
+TEST_F(ApplyControlDataUpdatesTest, CannotEncryptUnsyncedChanges) {
+ // Storing the cryptographer separately is bad, but for this test we
+ // know it's safe.
+ Cryptographer* cryptographer;
+ ModelTypeSet encrypted_types;
+ encrypted_types.Put(PASSWORDS);
+ {
+ syncable::ReadTransaction trans(FROM_HERE, directory());
+ cryptographer = directory()->GetCryptographer(&trans);
+ EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
+ .Equals(encrypted_types));
+
+ // With default encrypted_types, this should be true.
+ EXPECT_TRUE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types));
+
+ Syncer::UnsyncedMetaHandles handles;
+ GetUnsyncedEntries(&trans, &handles);
+ EXPECT_TRUE(handles.empty());
+ }
+
+ // Create unsynced bookmarks without encryption.
+ // First item is a folder
+ Id folder_id = id_factory_.NewLocalId();
+ entry_factory_->CreateUnsyncedItem(
+ folder_id, id_factory_.root(), "folder", true,
+ BOOKMARKS, NULL);
+ // Next five items are children of the folder
+ size_t i;
+ size_t batch_s = 5;
+ for (i = 0; i < batch_s; ++i) {
+ entry_factory_->CreateUnsyncedItem(id_factory_.NewLocalId(), folder_id,
+ base::StringPrintf("Item %"PRIuS"", i),
+ false, BOOKMARKS, NULL);
+ }
+ // Next five items are children of the root.
+ for (; i < 2*batch_s; ++i) {
+ entry_factory_->CreateUnsyncedItem(
+ id_factory_.NewLocalId(), id_factory_.root(),
+ base::StringPrintf("Item %"PRIuS"", i), false,
+ BOOKMARKS, NULL);
+ }
+
+ // We encrypt with new keys, triggering the local cryptographer to be unready
+ // and unable to decrypt data (once updated).
+ Cryptographer other_cryptographer(cryptographer->encryptor());
+ KeyParams params = {"localhost", "dummy", "foobar"};
+ other_cryptographer.AddKey(params);
+ sync_pb::EntitySpecifics specifics;
+ sync_pb::NigoriSpecifics* nigori = specifics.mutable_nigori();
+ other_cryptographer.GetKeys(nigori->mutable_encrypted());
+ nigori->set_encrypt_everything(true);
+ encrypted_types.Put(BOOKMARKS);
+ entry_factory_->CreateUnappliedNewItem(
+ ModelTypeToRootTag(NIGORI), specifics, true);
+ EXPECT_FALSE(cryptographer->has_pending_keys());
+
+ {
+ // Ensure we have unsynced nodes that aren't properly encrypted.
+ syncable::ReadTransaction trans(FROM_HERE, directory());
+ EXPECT_FALSE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types));
+ Syncer::UnsyncedMetaHandles handles;
+ GetUnsyncedEntries(&trans, &handles);
+ EXPECT_EQ(2*batch_s+1, handles.size());
+ }
+
+ ApplyControlDataUpdates(directory());
+
+ EXPECT_FALSE(cryptographer->is_ready());
+ EXPECT_TRUE(cryptographer->has_pending_keys());
+ {
+ syncable::ReadTransaction trans(FROM_HERE, directory());
+
+ // Since we have pending keys, we would have failed to encrypt, but the
+ // cryptographer should be updated.
+ EXPECT_FALSE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types));
+ EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
+ .Equals(ModelTypeSet::All()));
+ EXPECT_FALSE(cryptographer->is_ready());
+ EXPECT_TRUE(cryptographer->has_pending_keys());
+
+ Syncer::UnsyncedMetaHandles handles;
+ GetUnsyncedEntries(&trans, &handles);
+ EXPECT_EQ(2*batch_s+1, handles.size());
+ }
+}
+
+} // namespace syncer
diff --git a/sync/engine/apply_updates_command.cc b/sync/engine/apply_updates_command.cc
index da8a1cc..7d4e72e 100644
--- a/sync/engine/apply_updates_command.cc
+++ b/sync/engine/apply_updates_command.cc
@@ -57,6 +57,10 @@ SyncerError ApplyUpdatesCommand::ModelChangingExecuteImpl(
}
}
+ // Don't process control type updates here. They will be handled elsewhere.
+ FullModelTypeSet control_types = ToFullModelTypeSet(ControlTypes());
+ server_type_restriction.RemoveAll(control_types);
+
std::vector<int64> handles;
dir->GetUnappliedUpdateMetaHandles(
&trans, server_type_restriction, &handles);
@@ -77,6 +81,10 @@ SyncerError ApplyUpdatesCommand::ModelChangingExecuteImpl(
if (status.ServerSaysNothingMoreToDownload()) {
for (ModelTypeSet::Iterator it =
status.updates_request_types().First(); it.Good(); it.Inc()) {
+ // Don't set the flag for control types. We didn't process them here.
+ if (IsControlType(it.Get()))
+ continue;
+
// This gets persisted to the directory's backing store.
dir->set_initial_sync_ended_for_type(it.Get(), true);
}
diff --git a/sync/engine/apply_updates_command_unittest.cc b/sync/engine/apply_updates_command_unittest.cc
index 5e14c59..eee3edb 100644
--- a/sync/engine/apply_updates_command_unittest.cc
+++ b/sync/engine/apply_updates_command_unittest.cc
@@ -4,7 +4,6 @@
#include <string>
-#include "base/format_macros.h"
#include "base/location.h"
#include "base/memory/scoped_ptr.h"
#include "base/stringprintf.h"
@@ -13,9 +12,7 @@
#include "sync/internal_api/public/test/test_entry_factory.h"
#include "sync/protocol/bookmark_specifics.pb.h"
#include "sync/protocol/password_specifics.pb.h"
-#include "sync/sessions/sync_session.h"
#include "sync/syncable/mutable_entry.h"
-#include "sync/syncable/nigori_util.h"
#include "sync/syncable/read_transaction.h"
#include "sync/syncable/syncable_id.h"
#include "sync/syncable/syncable_util.h"
@@ -23,14 +20,12 @@
#include "sync/test/engine/fake_model_worker.h"
#include "sync/test/engine/syncer_command_test.h"
#include "sync/test/engine/test_id_factory.h"
-#include "sync/test/fake_encryptor.h"
#include "sync/test/fake_sync_encryption_handler.h"
#include "sync/util/cryptographer.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace syncer {
-using sessions::SyncSession;
using std::string;
using syncable::Id;
using syncable::MutableEntry;
@@ -71,7 +66,6 @@ class ApplyUpdatesCommandTest : public SyncerCommandTest {
DISALLOW_COPY_AND_ASSIGN(ApplyUpdatesCommandTest);
ApplyUpdatesCommand apply_updates_command_;
- TestIdFactory id_factory_;
scoped_ptr<TestEntryFactory> entry_factory_;
};
@@ -168,7 +162,7 @@ TEST_F(ApplyUpdatesCommandTest, HierarchyAndSimpleConflict) {
ASSERT_TRUE(entry.good());
entry.Put(syncable::SERVER_PARENT_ID,
- id_factory_.MakeServer("bogus_parent"));
+ TestIdFactory::MakeServer("bogus_parent"));
}
ExpectGroupToChange(apply_updates_command_, GROUP_UI);
@@ -205,12 +199,12 @@ TEST_F(ApplyUpdatesCommandTest, HierarchyConflictDirectoryLoop) {
// Re-parent from root to "Y"
entry.Put(syncable::SERVER_VERSION, entry_factory_->GetNextRevision());
entry.Put(syncable::IS_UNAPPLIED_UPDATE, true);
- entry.Put(syncable::SERVER_PARENT_ID, id_factory_.MakeServer("Y"));
+ entry.Put(syncable::SERVER_PARENT_ID, TestIdFactory::MakeServer("Y"));
}
// Item 'Y' is child of 'X'.
entry_factory_->CreateUnsyncedItem(
- id_factory_.MakeServer("Y"), id_factory_.MakeServer("X"), "Y", true,
+ TestIdFactory::MakeServer("Y"), TestIdFactory::MakeServer("X"), "Y", true,
BOOKMARKS, NULL);
// If the server's update were applied, we would have X be a child of Y, and Y
@@ -240,7 +234,7 @@ TEST_F(ApplyUpdatesCommandTest, HierarchyConflictDeletedParent) {
// Create a locally deleted parent item.
int64 parent_handle;
entry_factory_->CreateUnsyncedItem(
- Id::CreateFromServerId("parent"), id_factory_.root(),
+ Id::CreateFromServerId("parent"), TestIdFactory::root(),
"parent", true, BOOKMARKS, &parent_handle);
{
WriteTransaction trans(FROM_HERE, UNITTEST, directory());
@@ -286,13 +280,13 @@ TEST_F(ApplyUpdatesCommandTest, HierarchyConflictDeleteNonEmptyDirectory) {
// Delete it on the server.
entry.Put(syncable::SERVER_VERSION, entry_factory_->GetNextRevision());
entry.Put(syncable::IS_UNAPPLIED_UPDATE, true);
- entry.Put(syncable::SERVER_PARENT_ID, id_factory_.root());
+ entry.Put(syncable::SERVER_PARENT_ID, TestIdFactory::root());
entry.Put(syncable::SERVER_IS_DEL, true);
}
// Create a local child of the server-deleted directory.
entry_factory_->CreateUnsyncedItem(
- id_factory_.MakeServer("child"), id_factory_.MakeServer("parent"),
+ TestIdFactory::MakeServer("child"), TestIdFactory::MakeServer("parent"),
"child", false, BOOKMARKS, NULL);
// The server's request to delete the directory must be ignored, otherwise our
@@ -515,349 +509,4 @@ TEST_F(ApplyUpdatesCommandTest, SomeUndecryptablePassword) {
}
}
-TEST_F(ApplyUpdatesCommandTest, NigoriUpdate) {
- // Storing the cryptographer separately is bad, but for this test we
- // know it's safe.
- Cryptographer* cryptographer;
- ModelTypeSet encrypted_types;
- encrypted_types.Put(PASSWORDS);
- encrypted_types.Put(NIGORI);
- {
- syncable::ReadTransaction trans(FROM_HERE, directory());
- cryptographer = directory()->GetCryptographer(&trans);
- EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
- .Equals(encrypted_types));
- }
-
- // Nigori node updates should update the Cryptographer.
- Cryptographer other_cryptographer(cryptographer->encryptor());
- KeyParams params = {"localhost", "dummy", "foobar"};
- other_cryptographer.AddKey(params);
-
- sync_pb::EntitySpecifics specifics;
- sync_pb::NigoriSpecifics* nigori = specifics.mutable_nigori();
- other_cryptographer.GetKeys(nigori->mutable_encrypted());
- nigori->set_encrypt_everything(true);
- entry_factory_->CreateUnappliedNewItem(
- ModelTypeToRootTag(NIGORI), specifics, true);
- EXPECT_FALSE(cryptographer->has_pending_keys());
-
- ExpectGroupToChange(apply_updates_command_, GROUP_PASSIVE);
- apply_updates_command_.ExecuteImpl(session());
-
- sessions::StatusController* status = session()->mutable_status_controller();
- sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
- ASSERT_TRUE(status->update_progress());
- EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize())
- << "All updates should have been attempted";
- ASSERT_TRUE(status->conflict_progress());
- EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize())
- << "The nigori update shouldn't be in conflict";
- EXPECT_EQ(1, status->update_progress()->SuccessfullyAppliedUpdateCount())
- << "The nigori update should be applied";
-
- EXPECT_FALSE(cryptographer->is_ready());
- EXPECT_TRUE(cryptographer->has_pending_keys());
- {
- syncable::ReadTransaction trans(FROM_HERE, directory());
- EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
- .Equals(ModelTypeSet::All()));
- }
-}
-
-TEST_F(ApplyUpdatesCommandTest, NigoriUpdateForDisabledTypes) {
- // Storing the cryptographer separately is bad, but for this test we
- // know it's safe.
- Cryptographer* cryptographer;
- ModelTypeSet encrypted_types;
- encrypted_types.Put(PASSWORDS);
- encrypted_types.Put(NIGORI);
- {
- syncable::ReadTransaction trans(FROM_HERE, directory());
- cryptographer = directory()->GetCryptographer(&trans);
- EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
- .Equals(encrypted_types));
- }
-
- // Nigori node updates should update the Cryptographer.
- Cryptographer other_cryptographer(cryptographer->encryptor());
- KeyParams params = {"localhost", "dummy", "foobar"};
- other_cryptographer.AddKey(params);
-
- sync_pb::EntitySpecifics specifics;
- sync_pb::NigoriSpecifics* nigori = specifics.mutable_nigori();
- other_cryptographer.GetKeys(nigori->mutable_encrypted());
- nigori->set_encrypt_everything(true);
- entry_factory_->CreateUnappliedNewItem(
- ModelTypeToRootTag(NIGORI), specifics, true);
- EXPECT_FALSE(cryptographer->has_pending_keys());
-
- ExpectGroupToChange(apply_updates_command_, GROUP_PASSIVE);
- apply_updates_command_.ExecuteImpl(session());
-
- sessions::StatusController* status = session()->mutable_status_controller();
- sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
- ASSERT_TRUE(status->update_progress());
- EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize())
- << "All updates should have been attempted";
- ASSERT_TRUE(status->conflict_progress());
- EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize())
- << "The nigori update shouldn't be in conflict";
- EXPECT_EQ(1, status->update_progress()->SuccessfullyAppliedUpdateCount())
- << "The nigori update should be applied";
-
- EXPECT_FALSE(cryptographer->is_ready());
- EXPECT_TRUE(cryptographer->has_pending_keys());
- {
- syncable::ReadTransaction trans(FROM_HERE, directory());
- EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
- .Equals(ModelTypeSet::All()));
- }
-}
-
-// Create some local unsynced and unencrypted data. Apply a nigori update that
-// turns on encryption for the unsynced data. Ensure we properly encrypt the
-// data as part of the nigori update. Apply another nigori update with no
-// changes. Ensure we ignore already-encrypted unsynced data and that nothing
-// breaks.
-TEST_F(ApplyUpdatesCommandTest, EncryptUnsyncedChanges) {
- // Storing the cryptographer separately is bad, but for this test we
- // know it's safe.
- Cryptographer* cryptographer;
- ModelTypeSet encrypted_types;
- encrypted_types.Put(PASSWORDS);
- encrypted_types.Put(NIGORI);
- {
- syncable::ReadTransaction trans(FROM_HERE, directory());
- cryptographer = directory()->GetCryptographer(&trans);
- EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
- .Equals(encrypted_types));
-
- // With default encrypted_types, this should be true.
- EXPECT_TRUE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types));
-
- Syncer::UnsyncedMetaHandles handles;
- GetUnsyncedEntries(&trans, &handles);
- EXPECT_TRUE(handles.empty());
- }
-
- // Create unsynced bookmarks without encryption.
- // First item is a folder
- Id folder_id = id_factory_.NewLocalId();
- entry_factory_->CreateUnsyncedItem(folder_id, id_factory_.root(), "folder",
- true, BOOKMARKS, NULL);
- // Next five items are children of the folder
- size_t i;
- size_t batch_s = 5;
- for (i = 0; i < batch_s; ++i) {
- entry_factory_->CreateUnsyncedItem(id_factory_.NewLocalId(), folder_id,
- base::StringPrintf("Item %"PRIuS"", i),
- false, BOOKMARKS, NULL);
- }
- // Next five items are children of the root.
- for (; i < 2*batch_s; ++i) {
- entry_factory_->CreateUnsyncedItem(
- id_factory_.NewLocalId(), id_factory_.root(),
- base::StringPrintf("Item %"PRIuS"", i), false,
- BOOKMARKS, NULL);
- }
-
- KeyParams params = {"localhost", "dummy", "foobar"};
- cryptographer->AddKey(params);
- sync_pb::EntitySpecifics specifics;
- sync_pb::NigoriSpecifics* nigori = specifics.mutable_nigori();
- cryptographer->GetKeys(nigori->mutable_encrypted());
- nigori->set_encrypt_everything(true);
- encrypted_types.Put(BOOKMARKS);
- entry_factory_->CreateUnappliedNewItem(
- ModelTypeToRootTag(NIGORI), specifics, true);
- EXPECT_FALSE(cryptographer->has_pending_keys());
- EXPECT_TRUE(cryptographer->is_ready());
-
- {
- // Ensure we have unsynced nodes that aren't properly encrypted.
- syncable::ReadTransaction trans(FROM_HERE, directory());
- EXPECT_FALSE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types));
-
- Syncer::UnsyncedMetaHandles handles;
- GetUnsyncedEntries(&trans, &handles);
- EXPECT_EQ(2*batch_s+1, handles.size());
- }
-
- ExpectGroupToChange(apply_updates_command_, GROUP_PASSIVE);
- apply_updates_command_.ExecuteImpl(session());
-
- {
- sessions::StatusController* status = session()->mutable_status_controller();
- sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
- ASSERT_TRUE(status->update_progress());
- EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize())
- << "All updates should have been attempted";
- ASSERT_TRUE(status->conflict_progress());
- EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize())
- << "No updates should be in conflict";
- EXPECT_EQ(0, status->conflict_progress()->EncryptionConflictingItemsSize())
- << "No updates should be in conflict";
- EXPECT_EQ(1, status->update_progress()->SuccessfullyAppliedUpdateCount())
- << "The nigori update should be applied";
- }
- EXPECT_FALSE(cryptographer->has_pending_keys());
- EXPECT_TRUE(cryptographer->is_ready());
- {
- syncable::ReadTransaction trans(FROM_HERE, directory());
-
- // If ProcessUnsyncedChangesForEncryption worked, all our unsynced changes
- // should be encrypted now.
- EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
- .Equals(ModelTypeSet::All()));
- EXPECT_TRUE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types));
-
- Syncer::UnsyncedMetaHandles handles;
- GetUnsyncedEntries(&trans, &handles);
- EXPECT_EQ(2*batch_s+1, handles.size());
- }
-
- // Simulate another nigori update that doesn't change anything.
- {
- WriteTransaction trans(FROM_HERE, UNITTEST, directory());
- MutableEntry entry(&trans, syncable::GET_BY_SERVER_TAG,
- ModelTypeToRootTag(NIGORI));
- ASSERT_TRUE(entry.good());
- entry.Put(syncable::SERVER_VERSION, entry_factory_->GetNextRevision());
- entry.Put(syncable::IS_UNAPPLIED_UPDATE, true);
- }
- ExpectGroupToChange(apply_updates_command_, GROUP_PASSIVE);
- apply_updates_command_.ExecuteImpl(session());
- {
- sessions::StatusController* status = session()->mutable_status_controller();
- sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
- ASSERT_TRUE(status->update_progress());
- EXPECT_EQ(2, status->update_progress()->AppliedUpdatesSize())
- << "All updates should have been attempted";
- ASSERT_TRUE(status->conflict_progress());
- EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize())
- << "No updates should be in conflict";
- EXPECT_EQ(0, status->conflict_progress()->EncryptionConflictingItemsSize())
- << "No updates should be in conflict";
- EXPECT_EQ(2, status->update_progress()->SuccessfullyAppliedUpdateCount())
- << "The nigori update should be applied";
- }
- EXPECT_FALSE(cryptographer->has_pending_keys());
- EXPECT_TRUE(cryptographer->is_ready());
- {
- syncable::ReadTransaction trans(FROM_HERE, directory());
-
- // All our changes should still be encrypted.
- EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
- .Equals(ModelTypeSet::All()));
- EXPECT_TRUE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types));
-
- Syncer::UnsyncedMetaHandles handles;
- GetUnsyncedEntries(&trans, &handles);
- EXPECT_EQ(2*batch_s+1, handles.size());
- }
-}
-
-TEST_F(ApplyUpdatesCommandTest, CannotEncryptUnsyncedChanges) {
- // Storing the cryptographer separately is bad, but for this test we
- // know it's safe.
- Cryptographer* cryptographer;
- ModelTypeSet encrypted_types;
- encrypted_types.Put(PASSWORDS);
- encrypted_types.Put(NIGORI);
- {
- syncable::ReadTransaction trans(FROM_HERE, directory());
- cryptographer = directory()->GetCryptographer(&trans);
- EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
- .Equals(encrypted_types));
-
- // With default encrypted_types, this should be true.
- EXPECT_TRUE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types));
-
- Syncer::UnsyncedMetaHandles handles;
- GetUnsyncedEntries(&trans, &handles);
- EXPECT_TRUE(handles.empty());
- }
-
- // Create unsynced bookmarks without encryption.
- // First item is a folder
- Id folder_id = id_factory_.NewLocalId();
- entry_factory_->CreateUnsyncedItem(
- folder_id, id_factory_.root(), "folder", true,
- BOOKMARKS, NULL);
- // Next five items are children of the folder
- size_t i;
- size_t batch_s = 5;
- for (i = 0; i < batch_s; ++i) {
- entry_factory_->CreateUnsyncedItem(id_factory_.NewLocalId(), folder_id,
- base::StringPrintf("Item %"PRIuS"", i),
- false, BOOKMARKS, NULL);
- }
- // Next five items are children of the root.
- for (; i < 2*batch_s; ++i) {
- entry_factory_->CreateUnsyncedItem(
- id_factory_.NewLocalId(), id_factory_.root(),
- base::StringPrintf("Item %"PRIuS"", i), false,
- BOOKMARKS, NULL);
- }
-
- // We encrypt with new keys, triggering the local cryptographer to be unready
- // and unable to decrypt data (once updated).
- Cryptographer other_cryptographer(cryptographer->encryptor());
- KeyParams params = {"localhost", "dummy", "foobar"};
- other_cryptographer.AddKey(params);
- sync_pb::EntitySpecifics specifics;
- sync_pb::NigoriSpecifics* nigori = specifics.mutable_nigori();
- other_cryptographer.GetKeys(nigori->mutable_encrypted());
- nigori->set_encrypt_everything(true);
- encrypted_types.Put(BOOKMARKS);
- entry_factory_->CreateUnappliedNewItem(
- ModelTypeToRootTag(NIGORI), specifics, true);
- EXPECT_FALSE(cryptographer->has_pending_keys());
-
- {
- // Ensure we have unsynced nodes that aren't properly encrypted.
- syncable::ReadTransaction trans(FROM_HERE, directory());
- EXPECT_FALSE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types));
- Syncer::UnsyncedMetaHandles handles;
- GetUnsyncedEntries(&trans, &handles);
- EXPECT_EQ(2*batch_s+1, handles.size());
- }
-
- ExpectGroupToChange(apply_updates_command_, GROUP_PASSIVE);
- apply_updates_command_.ExecuteImpl(session());
-
- sessions::StatusController* status = session()->mutable_status_controller();
- sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
- ASSERT_TRUE(status->update_progress());
- EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize())
- << "All updates should have been attempted";
- ASSERT_TRUE(status->conflict_progress());
- EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize())
- << "The unsynced changes don't trigger a blocking conflict with the "
- << "nigori update.";
- EXPECT_EQ(0, status->conflict_progress()->EncryptionConflictingItemsSize())
- << "The unsynced changes don't trigger an encryption conflict with the "
- << "nigori update.";
- EXPECT_EQ(1, status->update_progress()->SuccessfullyAppliedUpdateCount())
- << "The nigori update should be applied";
- EXPECT_FALSE(cryptographer->is_ready());
- EXPECT_TRUE(cryptographer->has_pending_keys());
- {
- syncable::ReadTransaction trans(FROM_HERE, directory());
-
- // Since we have pending keys, we would have failed to encrypt, but the
- // cryptographer should be updated.
- EXPECT_FALSE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types));
- EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
- .Equals(ModelTypeSet::All()));
- EXPECT_FALSE(cryptographer->is_ready());
- EXPECT_TRUE(cryptographer->has_pending_keys());
-
- Syncer::UnsyncedMetaHandles handles;
- GetUnsyncedEntries(&trans, &handles);
- EXPECT_EQ(2*batch_s+1, handles.size());
- }
-}
-
} // namespace syncer
diff --git a/sync/engine/conflict_resolver.cc b/sync/engine/conflict_resolver.cc
index 736e668..73c25ea 100644
--- a/sync/engine/conflict_resolver.cc
+++ b/sync/engine/conflict_resolver.cc
@@ -11,9 +11,9 @@
#include "base/location.h"
#include "base/metrics/histogram.h"
+#include "sync/engine/conflict_util.h"
#include "sync/engine/syncer.h"
#include "sync/engine/syncer_util.h"
-#include "sync/protocol/nigori_specifics.pb.h"
#include "sync/sessions/status_controller.h"
#include "sync/syncable/directory.h"
#include "sync/syncable/mutable_entry.h"
@@ -48,25 +48,6 @@ ConflictResolver::ConflictResolver() {
ConflictResolver::~ConflictResolver() {
}
-void ConflictResolver::IgnoreLocalChanges(MutableEntry* entry) {
- // An update matches local actions, merge the changes.
- // This is a little fishy because we don't actually merge them.
- // In the future we should do a 3-way merge.
- // With IS_UNSYNCED false, changes should be merged.
- entry->Put(syncable::IS_UNSYNCED, false);
-}
-
-void ConflictResolver::OverwriteServerChanges(WriteTransaction* trans,
- MutableEntry * entry) {
- // This is similar to an overwrite from the old client.
- // This is equivalent to a scenario where we got the update before we'd
- // made our local client changes.
- // TODO(chron): This is really a general property clobber. We clobber
- // the server side property. Perhaps we should actually do property merging.
- entry->Put(syncable::BASE_VERSION, entry->Get(syncable::SERVER_VERSION));
- entry->Put(syncable::IS_UNAPPLIED_UPDATE, false);
-}
-
ConflictResolver::ProcessSimpleConflictResult
ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
const Id& id,
@@ -221,70 +202,11 @@ ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
base_server_specifics_match = true;
}
- // We manually merge nigori data.
- if (entry.GetModelType() == NIGORI) {
- // Create a new set of specifics based on the server specifics (which
- // preserves their encryption keys).
- sync_pb::EntitySpecifics specifics =
- entry.Get(syncable::SERVER_SPECIFICS);
- sync_pb::NigoriSpecifics* server_nigori = specifics.mutable_nigori();
- // Store the merged set of encrypted types (cryptographer->Update(..) will
- // have merged the local types already).
- trans->directory()->GetNigoriHandler()->UpdateNigoriFromEncryptedTypes(
- server_nigori,
- trans);
- // The cryptographer has the both the local and remote encryption keys
- // (added at cryptographer->Update(..) time).
- // If the cryptographer is ready, then it already merged both sets of keys
- // and we can store them back in. In that case, the remote key was already
- // part of the local keybag, so we preserve the local key as the default
- // (including whether it's an explicit key).
- // If the cryptographer is not ready, then the user will have to provide
- // the passphrase to decrypt the pending keys. When they do so, the
- // SetDecryptionPassphrase code will act based on whether the server
- // update has an explicit passphrase or not.
- // - If the server had an explicit passphrase, that explicit passphrase
- // will be preserved as the default encryption key.
- // - If the server did not have an explicit passphrase, we assume the
- // local passphrase is the most up to date and preserve the local
- // default encryption key marked as an implicit passphrase.
- // This works fine except for the case where we had locally set an
- // explicit passphrase. In that case the nigori node will have the default
- // key based on the local explicit passphassphrase, but will not have it
- // marked as explicit. To fix this we'd have to track whether we have a
- // explicit passphrase or not separate from the nigori, which would
- // introduce even more complexity, so we leave it up to the user to
- // reset that passphrase as an explicit one via settings. The goal here
- // is to ensure both sets of encryption keys are preserved.
- if (cryptographer->is_ready()) {
- cryptographer->GetKeys(server_nigori->mutable_encrypted());
- server_nigori->set_using_explicit_passphrase(
- entry.Get(syncable::SPECIFICS).nigori().
- using_explicit_passphrase());
- }
- // We deliberately leave the server's device information. This client will
- // add its own device information on restart.
- entry.Put(syncable::SPECIFICS, specifics);
- DVLOG(1) << "Resolving simple conflict, merging nigori nodes: " << entry;
- status->increment_num_server_overwrites();
- OverwriteServerChanges(trans, &entry);
- UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
- NIGORI_MERGE,
- CONFLICT_RESOLUTION_SIZE);
- } else if (!entry_deleted && name_matches && parent_matches &&
- specifics_match && !needs_reinsertion) {
+ if (!entry_deleted && name_matches && parent_matches && specifics_match &&
+ !needs_reinsertion) {
DVLOG(1) << "Resolving simple conflict, everything matches, ignoring "
<< "changes for: " << entry;
- // This unsets both IS_UNSYNCED and IS_UNAPPLIED_UPDATE, and sets the
- // BASE_VERSION to match the SERVER_VERSION. If we didn't also unset
- // IS_UNAPPLIED_UPDATE, then we would lose unsynced positional data from
- // adjacent entries when the server update gets applied and the item is
- // re-inserted into the PREV_ID/NEXT_ID linked list. This is primarily
- // an issue because we commit after applying updates, and is most
- // commonly seen when positional changes are made while a passphrase
- // is required (and hence there will be many encryption conflicts).
- OverwriteServerChanges(trans, &entry);
- IgnoreLocalChanges(&entry);
+ IgnoreConflict(&entry);
UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
CHANGES_MATCH,
CONFLICT_RESOLUTION_SIZE);
@@ -292,12 +214,12 @@ ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
DVLOG(1) << "Resolving simple conflict, ignoring server encryption "
<< " changes for: " << entry;
status->increment_num_server_overwrites();
- OverwriteServerChanges(trans, &entry);
+ OverwriteServerChanges(&entry);
UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
IGNORE_ENCRYPTION,
CONFLICT_RESOLUTION_SIZE);
} else if (entry_deleted || !name_matches || !parent_matches) {
- OverwriteServerChanges(trans, &entry);
+ OverwriteServerChanges(&entry);
status->increment_num_server_overwrites();
DVLOG(1) << "Resolving simple conflict, overwriting server changes "
<< "for: " << entry;
@@ -341,7 +263,7 @@ ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
DCHECK_EQ(entry.Get(syncable::SERVER_VERSION), 0) << "For the server to "
"know to re-create, client-tagged items should revert to version 0 "
"when server-deleted.";
- OverwriteServerChanges(trans, &entry);
+ OverwriteServerChanges(&entry);
status->increment_num_server_overwrites();
DVLOG(1) << "Resolving simple conflict, undeleting server entry: "
<< entry;
@@ -384,6 +306,14 @@ bool ConflictResolver::ResolveConflicts(syncable::WriteTransaction* trans,
if (processed_items.count(id) > 0)
continue;
+ // We don't resolve conflicts for control types here.
+ Entry conflicting_node(trans, syncable::GET_BY_ID, id);
+ CHECK(conflicting_node.good());
+ if (IsControlType(
+ GetModelTypeFromSpecifics(conflicting_node.Get(syncable::SPECIFICS)))) {
+ continue;
+ }
+
// We have a simple conflict. In order check if positions have changed,
// we need to process conflicting predecessors before successors. Traverse
// backwards through all continuous conflicting predecessors, building a
diff --git a/sync/engine/conflict_resolver.h b/sync/engine/conflict_resolver.h
index 2efb1e9..7e5be74 100644
--- a/sync/engine/conflict_resolver.h
+++ b/sync/engine/conflict_resolver.h
@@ -65,10 +65,6 @@ class ConflictResolver {
SYNC_PROGRESS, // Progress made.
};
- void IgnoreLocalChanges(syncable::MutableEntry* entry);
- void OverwriteServerChanges(syncable::WriteTransaction* trans,
- syncable::MutableEntry* entry);
-
ProcessSimpleConflictResult ProcessSimpleConflict(
syncable::WriteTransaction* trans,
const syncable::Id& id,
diff --git a/sync/engine/conflict_util.cc b/sync/engine/conflict_util.cc
new file mode 100644
index 0000000..fbb684e
--- /dev/null
+++ b/sync/engine/conflict_util.cc
@@ -0,0 +1,51 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "sync/engine/conflict_util.h"
+
+#include "sync/syncable/mutable_entry.h"
+
+namespace syncer {
+
+using syncable::BASE_VERSION;
+using syncable::IS_UNAPPLIED_UPDATE;
+using syncable::IS_UNSYNCED;
+using syncable::SERVER_VERSION;
+
+using syncable::MutableEntry;
+
+// Allow the server's changes to take precedence.
+// This will take effect during the next ApplyUpdates step.
+void IgnoreLocalChanges(MutableEntry* entry) {
+ DCHECK(entry->Get(IS_UNSYNCED));
+ DCHECK(entry->Get(IS_UNAPPLIED_UPDATE));
+ entry->Put(IS_UNSYNCED, false);
+}
+
+// Overwrite the server with our own value.
+// We will commit our local data, overwriting the server, at the next
+// opportunity.
+void OverwriteServerChanges(MutableEntry* entry) {
+ DCHECK(entry->Get(IS_UNSYNCED));
+ DCHECK(entry->Get(IS_UNAPPLIED_UPDATE));
+ entry->Put(BASE_VERSION, entry->Get(SERVER_VERSION));
+ entry->Put(IS_UNAPPLIED_UPDATE, false);
+}
+
+// Having determined that everything matches, we ignore the non-conflict.
+void IgnoreConflict(MutableEntry* entry) {
+ // If we didn't also unset IS_UNAPPLIED_UPDATE, then we would lose unsynced
+ // positional data from adjacent entries when the server update gets applied
+ // and the item is re-inserted into the PREV_ID/NEXT_ID linked list. This is
+ // primarily an issue because we commit after applying updates, and is most
+ // commonly seen when positional changes are made while a passphrase is
+ // required (and hence there will be many encryption conflicts).
+ DCHECK(entry->Get(IS_UNSYNCED));
+ DCHECK(entry->Get(IS_UNAPPLIED_UPDATE));
+ entry->Put(BASE_VERSION, entry->Get(SERVER_VERSION));
+ entry->Put(IS_UNAPPLIED_UPDATE, false);
+ entry->Put(IS_UNSYNCED, false);
+}
+
+} // namespace syncer
diff --git a/sync/engine/conflict_util.h b/sync/engine/conflict_util.h
new file mode 100644
index 0000000..c3efa5e
--- /dev/null
+++ b/sync/engine/conflict_util.h
@@ -0,0 +1,31 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Utility functions that act on syncable::MutableEntry to resolve conflicts.
+
+#ifndef SYNC_ENGINE_CONFLICT_UTIL_H_
+#define SYNC_ENGINE_CONFLICT_UTIL_H_
+
+namespace syncable {
+class MutableEntry;
+}
+
+namespace syncer {
+
+// Marks the item as no longer requiring sync, allowing the server's version
+// to 'win' during the next update application step.
+void IgnoreLocalChanges(syncable::MutableEntry* entry);
+
+// Marks the item as no longer requiring update from server data. This will
+// cause the item to be committed to the server, overwriting the server's
+// version.
+void OverwriteServerChanges(syncable::MutableEntry* entry);
+
+// The local and server versions are identical, so unset the bits that put them
+// into a conflicting state.
+void IgnoreConflict(syncable::MutableEntry *trans);
+
+} // namespace syncer
+
+#endif // SYNC_ENGINE_CONFLICT_UTIL_H_
diff --git a/sync/engine/syncer.cc b/sync/engine/syncer.cc
index 66afdbc..d8d2ce3 100644
--- a/sync/engine/syncer.cc
+++ b/sync/engine/syncer.cc
@@ -10,6 +10,7 @@
#include "base/message_loop.h"
#include "base/time.h"
#include "build/build_config.h"
+#include "sync/engine/apply_control_data_updates.h"
#include "sync/engine/apply_updates_command.h"
#include "sync/engine/build_commit_command.h"
#include "sync/engine/commit.h"
@@ -158,6 +159,9 @@ void Syncer::SyncShare(sessions::SyncSession* session,
break;
}
case APPLY_UPDATES: {
+ // These include encryption updates that should be applied early.
+ ApplyControlDataUpdates(session->context()->directory());
+
ApplyUpdatesCommand apply_updates;
apply_updates.Execute(session);
session->SendEventNotification(SyncEngineEvent::STATUS_CHANGED);
diff --git a/sync/engine/syncer_util.cc b/sync/engine/syncer_util.cc
index 8a243df..85e1ff5 100644
--- a/sync/engine/syncer_util.cc
+++ b/sync/engine/syncer_util.cc
@@ -16,14 +16,11 @@
#include "sync/engine/syncer_types.h"
#include "sync/internal_api/public/base/model_type.h"
#include "sync/protocol/bookmark_specifics.pb.h"
-#include "sync/protocol/nigori_specifics.pb.h"
#include "sync/protocol/password_specifics.pb.h"
#include "sync/protocol/sync.pb.h"
#include "sync/syncable/directory.h"
#include "sync/syncable/entry.h"
#include "sync/syncable/mutable_entry.h"
-#include "sync/syncable/nigori_handler.h"
-#include "sync/syncable/nigori_util.h"
#include "sync/syncable/read_transaction.h"
#include "sync/syncable/syncable_changes_version.h"
#include "sync/syncable/syncable_proto_util.h"
@@ -195,51 +192,6 @@ UpdateAttemptResponse AttemptToUpdateEntry(
syncable::Id id = entry->Get(ID);
const sync_pb::EntitySpecifics& specifics = entry->Get(SERVER_SPECIFICS);
- // We intercept updates to the Nigori node, update the Cryptographer and
- // encrypt any unsynced changes here because there is no Nigori
- // ChangeProcessor. We never put the nigori node in a state of
- // conflict_encryption.
- //
- // We always update the cryptographer with the server's nigori node,
- // even if we have a locally modified nigori node (we manually merge nigori
- // data in the conflict resolver in that case). This handles the case where
- // two clients both set a different passphrase. The second client to attempt
- // to commit will go into a state of having pending keys, unioned the set of
- // encrypted types, and eventually re-encrypt everything with the passphrase
- // of the first client and commit the set of merged encryption keys. Until the
- // second client provides the pending passphrase, the cryptographer will
- // preserve the encryption keys based on the local passphrase, while the
- // nigori node will preserve the server encryption keys.
- //
- // If non-encryption changes are made to the nigori node, they will be
- // lost as part of conflict resolution. This is intended, as we place a higher
- // priority on preserving the server's passphrase change to preserving local
- // non-encryption changes. Next time the non-encryption changes are made to
- // the nigori node (e.g. on restart), they will commit without issue.
- if (specifics.has_nigori()) {
- const sync_pb::NigoriSpecifics& nigori = specifics.nigori();
- trans->directory()->GetNigoriHandler()->ApplyNigoriUpdate(nigori, trans);
-
- // Make sure any unsynced changes are properly encrypted as necessary.
- // We only perform this if the cryptographer is ready. If not, these are
- // re-encrypted at SetDecryptionPassphrase time (via ReEncryptEverything).
- // This logic covers the case where the nigori update marked new datatypes
- // for encryption, but didn't change the passphrase.
- if (cryptographer->is_ready()) {
- // Note that we don't bother to encrypt any data for which IS_UNSYNCED
- // == false here. The machine that turned on encryption should know about
- // and re-encrypt all synced data. It's possible it could get interrupted
- // during this process, but we currently reencrypt everything at startup
- // as well, so as soon as a client is restarted with this datatype marked
- // for encryption, all the data should be updated as necessary.
-
- // If this fails, something is wrong with the cryptographer, but there's
- // nothing we can do about it here.
- DVLOG(1) << "Received new nigori, encrypting unsynced changes.";
- syncable::ProcessUnsyncedChangesForEncryption(trans);
- }
- }
-
// Only apply updates that we can decrypt. If we can't decrypt the update, it
// is likely because the passphrase has not arrived yet. Because the
// passphrase may not arrive within this GetUpdates, we can't just return
diff --git a/sync/internal_api/public/base/model_type.h b/sync/internal_api/public/base/model_type.h
index 20c8ffa..8037e54 100644
--- a/sync/internal_api/public/base/model_type.h
+++ b/sync/internal_api/public/base/model_type.h
@@ -52,7 +52,8 @@ enum ModelType {
//
// A bookmark folder or a bookmark URL object.
BOOKMARKS,
- FIRST_REAL_MODEL_TYPE = BOOKMARKS, // Declared 2nd, for debugger prettiness.
+ FIRST_USER_MODEL_TYPE = BOOKMARKS, // Declared 2nd, for debugger prettiness.
+ FIRST_REAL_MODEL_TYPE = FIRST_USER_MODEL_TYPE,
// A preference folder or a preference object.
PREFERENCES,
@@ -69,8 +70,6 @@ enum ModelType {
TYPED_URLS,
// An extension folder or an extension object.
EXTENSIONS,
- // An object representing a set of Nigori keys.
- NIGORI,
// An object representing a custom search engine.
SEARCH_ENGINES,
// An object representing a browser session.
@@ -83,7 +82,14 @@ enum ModelType {
EXTENSION_SETTINGS,
// App notifications.
APP_NOTIFICATIONS,
- LAST_REAL_MODEL_TYPE = APP_NOTIFICATIONS,
+ LAST_USER_MODEL_TYPE = APP_NOTIFICATIONS,
+
+ // An object representing a set of Nigori keys.
+ NIGORI,
+ FIRST_CONTROL_MODEL_TYPE = NIGORI,
+ LAST_CONTROL_MODEL_TYPE = NIGORI,
+
+ LAST_REAL_MODEL_TYPE = LAST_CONTROL_MODEL_TYPE,
// If you are adding a new sync datatype that is exposed to the user via the
// sync preferences UI, be sure to update the list in
@@ -124,6 +130,27 @@ SYNC_EXPORT ModelType GetModelTypeFromSpecifics(
// value (sibling ordering) for this item.
bool ShouldMaintainPosition(ModelType model_type);
+// These are the user-selectable data types. Note that some of these share a
+// preference flag, so not all of them are individually user-selectable.
+ModelTypeSet UserTypes();
+
+// Returns a list of all control types.
+//
+// The control types are intended to contain metadata nodes that are essential
+// for the normal operation of the syncer. As such, they have the following
+// special properties:
+// - They are downloaded early during SyncBackend initialization.
+// - They are always enabled. Users may not disable these types.
+// - Their contents are not encrypted automatically.
+// - They support custom update application and conflict resolution logic.
+// - All change processing occurs on the sync thread (GROUP_PASSIVE).
+ModelTypeSet ControlTypes();
+
+// Returns true if this is a control type.
+//
+// See comment above for more information on what makes these types special.
+bool IsControlType(ModelType model_type);
+
// Determine a model type from the field number of its associated
// EntitySpecifics field.
ModelType GetModelTypeFromSpecificsFieldNumber(int field_number);
@@ -135,6 +162,8 @@ ModelType GetModelTypeFromSpecificsFieldNumber(int field_number);
SYNC_EXPORT int GetSpecificsFieldNumberFromModelType(
ModelType model_type);
+FullModelTypeSet ToFullModelTypeSet(ModelTypeSet in);
+
// TODO(sync): The functions below badly need some cleanup.
// Returns a pointer to a string with application lifetime that represents
diff --git a/sync/internal_api/public/sync_encryption_handler.cc b/sync/internal_api/public/sync_encryption_handler.cc
index d2b1ca2..e967600 100644
--- a/sync/internal_api/public/sync_encryption_handler.cc
+++ b/sync/internal_api/public/sync_encryption_handler.cc
@@ -14,11 +14,9 @@ SyncEncryptionHandler::~SyncEncryptionHandler() {}
// Static.
ModelTypeSet SyncEncryptionHandler::SensitiveTypes() {
- // Both of these have their own encryption schemes, but we include them
- // anyways.
+ // It has its own encryption scheme, but we include it anyway.
ModelTypeSet types;
types.Put(PASSWORDS);
- types.Put(NIGORI);
return types;
}
diff --git a/sync/internal_api/sync_encryption_handler_impl.cc b/sync/internal_api/sync_encryption_handler_impl.cc
index 82149b1..7cd5be8 100644
--- a/sync/internal_api/sync_encryption_handler_impl.cc
+++ b/sync/internal_api/sync_encryption_handler_impl.cc
@@ -375,14 +375,14 @@ void SyncEncryptionHandlerImpl::EnableEncryptEverything() {
ModelTypeSet* encrypted_types =
&UnlockVaultMutable(trans.GetWrappedTrans())->encrypted_types;
if (encrypt_everything_) {
- DCHECK(encrypted_types->Equals(ModelTypeSet::All()));
+ DCHECK(encrypted_types->Equals(UserTypes()));
return;
}
DVLOG(1) << "Enabling encrypt everything.";
encrypt_everything_ = true;
// Change |encrypted_types_| directly to avoid sending more than one
// notification.
- *encrypted_types = ModelTypeSet::All();
+ *encrypted_types = UserTypes();
FOR_EACH_OBSERVER(
Observer, observers_,
OnEncryptedTypesChanged(*encrypted_types, encrypt_everything_));
@@ -477,7 +477,7 @@ void SyncEncryptionHandlerImpl::ReEncryptEverything(
for (ModelTypeSet::Iterator iter =
UnlockVault(trans->GetWrappedTrans()).encrypted_types.First();
iter.Good(); iter.Inc()) {
- if (iter.Get() == PASSWORDS || iter.Get() == NIGORI)
+ if (iter.Get() == PASSWORDS || IsControlType(iter.Get()))
continue; // These types handle encryption differently.
ReadNode type_root(trans);
@@ -662,13 +662,13 @@ bool SyncEncryptionHandlerImpl::UpdateEncryptedTypesFromNigori(
if (nigori.encrypt_everything()) {
if (!encrypt_everything_) {
encrypt_everything_ = true;
- *encrypted_types = ModelTypeSet::All();
+ *encrypted_types = UserTypes();
DVLOG(1) << "Enabling encrypt everything via nigori node update";
FOR_EACH_OBSERVER(
Observer, observers_,
OnEncryptedTypesChanged(*encrypted_types, encrypt_everything_));
}
- DCHECK(encrypted_types->Equals(ModelTypeSet::All()));
+ DCHECK(encrypted_types->Equals(UserTypes()));
return true;
}
@@ -683,12 +683,12 @@ bool SyncEncryptionHandlerImpl::UpdateEncryptedTypesFromNigori(
!Difference(nigori_encrypted_types, SensitiveTypes()).Empty()) {
if (!encrypt_everything_) {
encrypt_everything_ = true;
- *encrypted_types = ModelTypeSet::All();
+ *encrypted_types = UserTypes();
FOR_EACH_OBSERVER(
Observer, observers_,
OnEncryptedTypesChanged(*encrypted_types, encrypt_everything_));
}
- DCHECK(encrypted_types->Equals(ModelTypeSet::All()));
+ DCHECK(encrypted_types->Equals(UserTypes()));
return false;
}
@@ -767,6 +767,10 @@ void SyncEncryptionHandlerImpl::MergeEncryptedTypes(
ModelTypeSet new_encrypted_types,
syncable::BaseTransaction* const trans) {
DCHECK(thread_checker_.CalledOnValidThread());
+
+ // Only UserTypes may be encrypted.
+ DCHECK(UserTypes().HasAll(new_encrypted_types));
+
ModelTypeSet* encrypted_types = &UnlockVaultMutable(trans)->encrypted_types;
if (!encrypted_types->HasAll(new_encrypted_types)) {
*encrypted_types = new_encrypted_types;
diff --git a/sync/internal_api/sync_encryption_handler_impl_unittest.cc b/sync/internal_api/sync_encryption_handler_impl_unittest.cc
index f1472c2..cc31a8e 100644
--- a/sync/internal_api/sync_encryption_handler_impl_unittest.cc
+++ b/sync/internal_api/sync_encryption_handler_impl_unittest.cc
@@ -155,13 +155,13 @@ TEST_F(SyncEncryptionHandlerImplTest, NigoriEncryptionTypes) {
EXPECT_CALL(*observer(),
OnEncryptedTypesChanged(
- HasModelTypes(ModelTypeSet::All()), false));
+ HasModelTypes(UserTypes()), false));
EXPECT_CALL(observer2,
OnEncryptedTypesChanged(
- HasModelTypes(ModelTypeSet::All()), false));
+ HasModelTypes(UserTypes()), false));
// Set all encrypted types
- encrypted_types = ModelTypeSet::All();
+ encrypted_types = UserTypes();
{
WriteTransaction trans(FROM_HERE, user_share());
encryption_handler()->MergeEncryptedTypes(
@@ -192,24 +192,17 @@ TEST_F(SyncEncryptionHandlerImplTest, NigoriEncryptionTypes) {
// Verify the encryption handler processes the encrypt everything field
// properly.
TEST_F(SyncEncryptionHandlerImplTest, EncryptEverythingExplicit) {
- ModelTypeSet real_types = ModelTypeSet::All();
sync_pb::NigoriSpecifics nigori;
nigori.set_encrypt_everything(true);
EXPECT_CALL(*observer(),
OnEncryptedTypesChanged(
- HasModelTypes(ModelTypeSet::All()), true));
+ HasModelTypes(UserTypes()), true));
EXPECT_FALSE(encryption_handler()->EncryptEverythingEnabled());
ModelTypeSet encrypted_types =
encryption_handler()->GetEncryptedTypesUnsafe();
- for (ModelTypeSet::Iterator iter = real_types.First();
- iter.Good(); iter.Inc()) {
- if (iter.Get() == PASSWORDS || iter.Get() == NIGORI)
- EXPECT_TRUE(encrypted_types.Has(iter.Get()));
- else
- EXPECT_FALSE(encrypted_types.Has(iter.Get()));
- }
+ EXPECT_TRUE(encrypted_types.Equals(ModelTypeSet(PASSWORDS)));
{
WriteTransaction trans(FROM_HERE, user_share());
@@ -220,10 +213,7 @@ TEST_F(SyncEncryptionHandlerImplTest, EncryptEverythingExplicit) {
EXPECT_TRUE(encryption_handler()->EncryptEverythingEnabled());
encrypted_types = encryption_handler()->GetEncryptedTypesUnsafe();
- for (ModelTypeSet::Iterator iter = real_types.First();
- iter.Good(); iter.Inc()) {
- EXPECT_TRUE(encrypted_types.Has(iter.Get()));
- }
+ EXPECT_TRUE(encrypted_types.HasAll(UserTypes()));
// Receiving the nigori node again shouldn't trigger another notification.
Mock::VerifyAndClearExpectations(observer());
@@ -238,24 +228,17 @@ TEST_F(SyncEncryptionHandlerImplTest, EncryptEverythingExplicit) {
// Verify the encryption handler can detect an implicit encrypt everything state
// (from clients that failed to write the encrypt everything field).
TEST_F(SyncEncryptionHandlerImplTest, EncryptEverythingImplicit) {
- ModelTypeSet real_types = ModelTypeSet::All();
sync_pb::NigoriSpecifics nigori;
nigori.set_encrypt_bookmarks(true); // Non-passwords = encrypt everything
EXPECT_CALL(*observer(),
OnEncryptedTypesChanged(
- HasModelTypes(ModelTypeSet::All()), true));
+ HasModelTypes(UserTypes()), true));
EXPECT_FALSE(encryption_handler()->EncryptEverythingEnabled());
ModelTypeSet encrypted_types =
encryption_handler()->GetEncryptedTypesUnsafe();
- for (ModelTypeSet::Iterator iter = real_types.First();
- iter.Good(); iter.Inc()) {
- if (iter.Get() == PASSWORDS || iter.Get() == NIGORI)
- EXPECT_TRUE(encrypted_types.Has(iter.Get()));
- else
- EXPECT_FALSE(encrypted_types.Has(iter.Get()));
- }
+ EXPECT_TRUE(encrypted_types.Equals(ModelTypeSet(PASSWORDS)));
{
WriteTransaction trans(FROM_HERE, user_share());
@@ -266,10 +249,7 @@ TEST_F(SyncEncryptionHandlerImplTest, EncryptEverythingImplicit) {
EXPECT_TRUE(encryption_handler()->EncryptEverythingEnabled());
encrypted_types = encryption_handler()->GetEncryptedTypesUnsafe();
- for (ModelTypeSet::Iterator iter = real_types.First();
- iter.Good(); iter.Inc()) {
- EXPECT_TRUE(encrypted_types.Has(iter.Get()));
- }
+ EXPECT_TRUE(encrypted_types.HasAll(UserTypes()));
// Receiving a nigori node with encrypt everything explicitly set shouldn't
// trigger another notification.
@@ -287,7 +267,6 @@ TEST_F(SyncEncryptionHandlerImplTest, EncryptEverythingImplicit) {
// as Sensitive, and that it does not consider this an implicit encrypt
// everything case.
TEST_F(SyncEncryptionHandlerImplTest, UnknownSensitiveTypes) {
- ModelTypeSet real_types = ModelTypeSet::All();
sync_pb::NigoriSpecifics nigori;
nigori.set_encrypt_everything(false);
nigori.set_encrypt_bookmarks(true);
@@ -303,13 +282,7 @@ TEST_F(SyncEncryptionHandlerImplTest, UnknownSensitiveTypes) {
EXPECT_FALSE(encryption_handler()->EncryptEverythingEnabled());
ModelTypeSet encrypted_types =
encryption_handler()->GetEncryptedTypesUnsafe();
- for (ModelTypeSet::Iterator iter = real_types.First();
- iter.Good(); iter.Inc()) {
- if (iter.Get() == PASSWORDS || iter.Get() == NIGORI)
- EXPECT_TRUE(encrypted_types.Has(iter.Get()));
- else
- EXPECT_FALSE(encrypted_types.Has(iter.Get()));
- }
+ EXPECT_TRUE(encrypted_types.Equals(ModelTypeSet(PASSWORDS)));
{
WriteTransaction trans(FROM_HERE, user_share());
@@ -320,15 +293,7 @@ TEST_F(SyncEncryptionHandlerImplTest, UnknownSensitiveTypes) {
EXPECT_FALSE(encryption_handler()->EncryptEverythingEnabled());
encrypted_types = encryption_handler()->GetEncryptedTypesUnsafe();
- for (ModelTypeSet::Iterator iter = real_types.First();
- iter.Good(); iter.Inc()) {
- if (iter.Get() == PASSWORDS ||
- iter.Get() == NIGORI ||
- iter.Get() == BOOKMARKS)
- EXPECT_TRUE(encrypted_types.Has(iter.Get()));
- else
- EXPECT_FALSE(encrypted_types.Has(iter.Get()));
- }
+ EXPECT_TRUE(encrypted_types.Equals(ModelTypeSet(BOOKMARKS, PASSWORDS)));
}
// Receive an old nigori with old encryption keys and encrypted types. We should
@@ -348,7 +313,7 @@ TEST_F(SyncEncryptionHandlerImplTest, ReceiveOldNigori) {
other_encrypted_specifics.mutable_encrypted());
sync_pb::EntitySpecifics our_encrypted_specifics;
our_encrypted_specifics.mutable_bookmark()->set_title("title2");
- ModelTypeSet encrypted_types = ModelTypeSet::All();
+ ModelTypeSet encrypted_types = UserTypes();
// Set up the current encryption state (containing both keys and encrypt
// everything).
@@ -364,7 +329,7 @@ TEST_F(SyncEncryptionHandlerImplTest, ReceiveOldNigori) {
EXPECT_CALL(*observer(), OnCryptographerStateChanged(_));
EXPECT_CALL(*observer(), OnEncryptedTypesChanged(
- HasModelTypes(ModelTypeSet::All()), true));
+ HasModelTypes(UserTypes()), true));
{
// Update the encryption handler.
WriteTransaction trans(FROM_HERE, user_share());
diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc
index ec08939..9f5538e 100644
--- a/sync/internal_api/sync_manager_impl_unittest.cc
+++ b/sync/internal_api/sync_manager_impl_unittest.cc
@@ -1424,7 +1424,7 @@ TEST_F(SyncManagerTest, EncryptDataTypesWithNoData) {
EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION));
EXPECT_CALL(encryption_observer_,
OnEncryptedTypesChanged(
- HasModelTypes(ModelTypeSet::All()), true));
+ HasModelTypes(UserTypes()), true));
EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
sync_manager_.GetEncryptionHandler()->EnableEncryptEverything();
EXPECT_TRUE(EncryptEverythingEnabledForTest());
@@ -1478,14 +1478,14 @@ TEST_F(SyncManagerTest, EncryptDataTypesWithData) {
EXPECT_CALL(encryption_observer_,
OnEncryptedTypesChanged(
- HasModelTypes(ModelTypeSet::All()), true));
+ HasModelTypes(UserTypes()), true));
EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
sync_manager_.GetEncryptionHandler()->EnableEncryptEverything();
EXPECT_TRUE(EncryptEverythingEnabledForTest());
{
ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
EXPECT_TRUE(GetEncryptedTypesWithTrans(&trans).Equals(
- ModelTypeSet::All()));
+ UserTypes()));
EXPECT_TRUE(syncable::VerifyDataTypeEncryptionForTest(
trans.GetWrappedTrans(),
BOOKMARKS,
@@ -1514,7 +1514,7 @@ TEST_F(SyncManagerTest, EncryptDataTypesWithData) {
EXPECT_TRUE(EncryptEverythingEnabledForTest());
{
ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
- EXPECT_TRUE(GetEncryptedTypesWithTrans(&trans).Equals(ModelTypeSet::All()));
+ EXPECT_TRUE(GetEncryptedTypesWithTrans(&trans).Equals(UserTypes()));
EXPECT_TRUE(syncable::VerifyDataTypeEncryptionForTest(
trans.GetWrappedTrans(),
BOOKMARKS,
@@ -2007,14 +2007,14 @@ TEST_F(SyncManagerTest, EncryptBookmarksWithLegacyData) {
EXPECT_CALL(encryption_observer_,
OnEncryptedTypesChanged(
- HasModelTypes(ModelTypeSet::All()), true));
+ HasModelTypes(UserTypes()), true));
EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
sync_manager_.GetEncryptionHandler()->EnableEncryptEverything();
EXPECT_TRUE(EncryptEverythingEnabledForTest());
{
ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
- EXPECT_TRUE(GetEncryptedTypesWithTrans(&trans).Equals(ModelTypeSet::All()));
+ EXPECT_TRUE(GetEncryptedTypesWithTrans(&trans).Equals(UserTypes()));
EXPECT_TRUE(syncable::VerifyDataTypeEncryptionForTest(
trans.GetWrappedTrans(),
BOOKMARKS,
@@ -2094,7 +2094,7 @@ TEST_F(SyncManagerTest, UpdateEntryWithEncryption) {
// Encrypt the datatatype, should set is_unsynced.
EXPECT_CALL(encryption_observer_,
OnEncryptedTypesChanged(
- HasModelTypes(ModelTypeSet::All()), true));
+ HasModelTypes(UserTypes()), true));
EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, FULL_ENCRYPTION));
@@ -2419,7 +2419,7 @@ TEST_F(SyncManagerTest, SetBookmarkTitleWithEncryption) {
// Encrypt the datatatype, should set is_unsynced.
EXPECT_CALL(encryption_observer_,
OnEncryptedTypesChanged(
- HasModelTypes(ModelTypeSet::All()), true));
+ HasModelTypes(UserTypes()), true));
EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, FULL_ENCRYPTION));
EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
@@ -2516,7 +2516,7 @@ TEST_F(SyncManagerTest, SetNonBookmarkTitleWithEncryption) {
// Encrypt the datatatype, should set is_unsynced.
EXPECT_CALL(encryption_observer_,
OnEncryptedTypesChanged(
- HasModelTypes(ModelTypeSet::All()), true));
+ HasModelTypes(UserTypes()), true));
EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, FULL_ENCRYPTION));
EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
diff --git a/sync/sync.gyp b/sync/sync.gyp
index 369605b..a5a9e80 100644
--- a/sync/sync.gyp
+++ b/sync/sync.gyp
@@ -73,6 +73,8 @@
'internal_api/public/util/weak_handle.h',
'engine/all_status.cc',
'engine/all_status.h',
+ 'engine/apply_control_data_updates.cc',
+ 'engine/apply_control_data_updates.h',
'engine/apply_updates_command.cc',
'engine/apply_updates_command.h',
'engine/backoff_delay_provider.cc',
@@ -83,6 +85,8 @@
'engine/commit.h',
'engine/conflict_resolver.cc',
'engine/conflict_resolver.h',
+ 'engine/conflict_util.cc',
+ 'engine/conflict_util.h',
'engine/download_updates_command.cc',
'engine/download_updates_command.h',
'engine/get_commit_ids_command.cc',
@@ -582,6 +586,7 @@
'internal_api/public/base/model_type_state_map_unittest.cc',
'internal_api/public/engine/model_safe_worker_unittest.cc',
'internal_api/public/util/immutable_unittest.cc',
+ 'engine/apply_control_data_updates_unittest.cc',
'engine/apply_updates_command_unittest.cc',
'engine/backoff_delay_provider_unittest.cc',
'engine/build_commit_command_unittest.cc',
@@ -590,10 +595,10 @@
'engine/process_commit_response_command_unittest.cc',
'engine/process_updates_command_unittest.cc',
'engine/resolve_conflicts_command_unittest.cc',
- 'engine/syncer_proto_util_unittest.cc',
- 'engine/syncer_unittest.cc',
'engine/sync_scheduler_unittest.cc',
'engine/sync_scheduler_whitebox_unittest.cc',
+ 'engine/syncer_proto_util_unittest.cc',
+ 'engine/syncer_unittest.cc',
'engine/throttled_data_type_tracker_unittest.cc',
'engine/traffic_recorder_unittest.cc',
'engine/verify_updates_command_unittest.cc',
diff --git a/sync/syncable/model_type.cc b/sync/syncable/model_type.cc
index 83e803c..1674206 100644
--- a/sync/syncable/model_type.cc
+++ b/sync/syncable/model_type.cc
@@ -144,6 +144,14 @@ int GetSpecificsFieldNumberFromModelType(ModelType model_type) {
return 0;
}
+FullModelTypeSet ToFullModelTypeSet(ModelTypeSet in) {
+ FullModelTypeSet out;
+ for (ModelTypeSet::Iterator i = in.First(); i.Good(); i.Inc()) {
+ out.Put(i.Get());
+ }
+ return out;
+}
+
// Note: keep this consistent with GetModelType in syncable.cc!
ModelType GetModelType(const sync_pb::SyncEntity& sync_entity) {
DCHECK(!IsRoot(sync_entity)); // Root shouldn't ever go over the wire.
@@ -226,6 +234,26 @@ bool ShouldMaintainPosition(ModelType model_type) {
return model_type == BOOKMARKS;
}
+ModelTypeSet UserTypes() {
+ ModelTypeSet set;
+ for (int i = FIRST_USER_MODEL_TYPE; i <= LAST_USER_MODEL_TYPE; ++i) {
+ set.Put(ModelTypeFromInt(i));
+ }
+ return set;
+}
+
+ModelTypeSet ControlTypes() {
+ ModelTypeSet set;
+ for (int i = FIRST_CONTROL_MODEL_TYPE; i <= LAST_CONTROL_MODEL_TYPE; ++i) {
+ set.Put(ModelTypeFromInt(i));
+ }
+ return set;
+}
+
+bool IsControlType(ModelType model_type) {
+ return ControlTypes().Has(model_type);
+}
+
const char* ModelTypeToString(ModelType model_type) {
// This is used in serialization routines as well as for displaying debug
// information. Do not attempt to change these string values unless you know
diff --git a/sync/syncable/nigori_util.cc b/sync/syncable/nigori_util.cc
index 4b59516..70cf41b 100644
--- a/sync/syncable/nigori_util.cc
+++ b/sync/syncable/nigori_util.cc
@@ -70,7 +70,7 @@ bool EntryNeedsEncryption(ModelTypeSet encrypted_types,
if (!entry.Get(UNIQUE_SERVER_TAG).empty())
return false; // We don't encrypt unique server nodes.
ModelType type = entry.GetModelType();
- if (type == PASSWORDS || type == NIGORI)
+ if (type == PASSWORDS || IsControlType(type))
return false;
// Checking NON_UNIQUE_NAME is not necessary for the correctness of encrypting
// the data, nor for determining if data is encrypted. We simply ensure it has
@@ -83,7 +83,7 @@ bool EntryNeedsEncryption(ModelTypeSet encrypted_types,
bool SpecificsNeedsEncryption(ModelTypeSet encrypted_types,
const sync_pb::EntitySpecifics& specifics) {
const ModelType type = GetModelTypeFromSpecifics(specifics);
- if (type == PASSWORDS || type == NIGORI)
+ if (type == PASSWORDS || IsControlType(type))
return false; // These types have their own encryption schemes.
if (!encrypted_types.Has(type))
return false; // This type does not require encryption
@@ -96,7 +96,7 @@ bool VerifyDataTypeEncryptionForTest(
ModelType type,
bool is_encrypted) {
Cryptographer* cryptographer = trans->directory()->GetCryptographer(trans);
- if (type == PASSWORDS || type == NIGORI) {
+ if (type == PASSWORDS || IsControlType(type)) {
NOTREACHED();
return true;
}