summaryrefslogtreecommitdiffstats
path: root/sync
diff options
context:
space:
mode:
authorrlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-08-31 22:32:46 +0000
committerrlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-08-31 22:32:46 +0000
commit41b89266ea2cdc7e93ee20ffaba945bdb7680d1b (patch)
tree2353d6fc1269f72a3ebcc85e604c5a562edf87fd /sync
parent16350fc17d592a88118b97993f33d67840cc2205 (diff)
downloadchromium_src-41b89266ea2cdc7e93ee20ffaba945bdb7680d1b.zip
chromium_src-41b89266ea2cdc7e93ee20ffaba945bdb7680d1b.tar.gz
chromium_src-41b89266ea2cdc7e93ee20ffaba945bdb7680d1b.tar.bz2
sync: Introduce control data types
The Nigori node has always been treated specially. It is downloaded during early sync initialization. It has custom encryption, update application, and conflict resolution logic. It is not user-selectable, but is instead always implicitly enabled. This patch replaces a lot of if (NIGORI)' code branches and creates the infrastructure we will need to support other data types that have these properties. These types will be used to store metadata required for the normal operation of the syncer. Notable changes include: - Introduce mutually exclusive lists of control types and user types in model_type.cc. - Create an ApplyControlDataUpdates() function, which will be used to apply changes and resolve conflicts for control data types. - Add some if statements to skip processing of control data updates in the regular conflict resolution code path. - Move around some code to accomodate the above changes. Introduce apply_control_data_updates_unittest.cc and conflict_util.cc - Have SyncBackendHost download all control types during init, not just the NIGORI. - Introduce a PurgeForMigration() function on the DataTypeManagerImpl class to support migration of control data types. This replaces ConfigureWithoutNigori(), which would not have scaled well as additional control data types were addded. - Modify the encryption logic so that only user types may be set to be encrypted. The Nigori is no longer expected to be in the set of encrypted types, though it is no less encrypted than it was before this change. - Use the list of user types to help calculate the list of registered sync prefs. BUG=122825,111297 Review URL: https://chromiumcodereview.appspot.com/10832286 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@154522 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync')
-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
19 files changed, 728 insertions, 570 deletions
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;
}