[Sync] Implement encryption-aware conflict resolution.

chrome/browser/sync/engine/process_updates_command.cc

Index: chrome/browser/sync/engine/process_updates_command.cc
diff --git a/chrome/browser/sync/engine/process_updates_command.cc b/chrome/browser/sync/engine/process_updates_command.cc
index 8ee7d7b41ecbc6caafb2edf7efcc317b510428ac..2b11c3c5e5da9d44e756e6286c83d5bcd099806f 100644
--- a/chrome/browser/sync/engine/process_updates_command.cc
+++ b/chrome/browser/sync/engine/process_updates_command.cc
@@ -15,6 +15,7 @@
 #include "chrome/browser/sync/sessions/sync_session.h"
 #include "chrome/browser/sync/syncable/directory_manager.h"
 #include "chrome/browser/sync/syncable/syncable.h"
+#include "chrome/browser/sync/util/cryptographer.h"
 
 using std::vector;
 
@@ -68,7 +69,9 @@ void ProcessUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) {
 
     if (it->first != VERIFY_SUCCESS && it->first != VERIFY_UNDELETE)
       continue;
-    switch (ProcessUpdate(dir, update, &trans)) {
+    switch (ProcessUpdate(dir, update,
+        session->context()->directory_manager()->GetCryptographer(&trans),
+        &trans)) {
       case SUCCESS_PROCESSED:
       case SUCCESS_STORED:
         break;
@@ -105,6 +108,7 @@ bool ReverifyEntry(syncable::WriteTransaction* trans, const SyncEntity& entry,
 ServerUpdateProcessingResult ProcessUpdatesCommand::ProcessUpdate(
     const syncable::ScopedDirLookup& dir,
     const sync_pb::SyncEntity& proto_update,
+    const Cryptographer* cryptographer,
     syncable::WriteTransaction* const trans) {
 
   const SyncEntity& update = *static_cast<const SyncEntity*>(&proto_update);
@@ -158,6 +162,39 @@ ServerUpdateProcessingResult ProcessUpdatesCommand::ProcessUpdate(
     target_entry.Put(syncable::IS_UNAPPLIED_UPDATE, true);
   }
 
+  // If this is a newly received undecryptable update, and the only thing that
+  // has changed are the specifics, store the original decryptable specifics,
+  // (on which any current or future local changes are based) before we
+  // overwrite SERVER_SPECIFICS.
+  // MTIME, CTIME, and NON_UNIQUE_NAME are not enforced.
+  if (!update.deleted() && !target_entry.Get(syncable::SERVER_IS_DEL) &&
+      (update.parent_id() == target_entry.Get(syncable::SERVER_PARENT_ID)) &&
+      (update.position_in_parent() ==
+          target_entry.Get(syncable::SERVER_POSITION_IN_PARENT)) &&
+      update.has_specifics() && update.specifics().has_encrypted() &&
+      !cryptographer->CanDecrypt(update.specifics().encrypted())) {

[tim (not reviewing), 2011/12/15 20:55:11]

holy crap, complicated

[Nicolas Zea, 2011/12/16 19:25:14]

Yeah :-/ Is there a better way of detecting when only the specifics have changed
and are encrypted? (disregarding mtime, ctime, and non_unique_name)

+    sync_pb::EntitySpecifics prev_specifics =
+        target_entry.Get(syncable::SERVER_SPECIFICS);
+    // We only store the old specifics if they were decryptable and applied and
+    // there is no PREV_SERVER_SPECIFICS already. Else do nothing.
+    if (!target_entry.Get(syncable::IS_UNAPPLIED_UPDATE) &&
+        !syncable::IsRealDataType(syncable::GetModelTypeFromSpecifics(
+            target_entry.Get(syncable::PREV_SERVER_SPECIFICS))) &&
+        (!prev_specifics.has_encrypted() ||
+         cryptographer->CanDecrypt(prev_specifics.encrypted()))) {
+      DVLOG(2) << "Storing previous server specifcs: "
+               << prev_specifics.SerializeAsString();
+      target_entry.Put(syncable::PREV_SERVER_SPECIFICS, prev_specifics);

[tim (not reviewing), 2011/12/15 20:55:11]

I don't really like PREV_SERVER_SPECIFICS, it's easy to confuse it with PREV as
in PREV_ID.

BASE_SERVER_SPECIFICS? ORIGINAL_

[Nicolas Zea, 2011/12/22 18:35:28]

Done.

+    }
+  } else if (target_entry.Get(syncable::PREV_SERVER_SPECIFICS).
+                 IsInitialized()) {

[tim (not reviewing), 2011/12/15 20:55:11]

indent

[Nicolas Zea, 2011/12/22 18:35:28]

Done.

+    // We have a PREV_SERVER_SPECIFICS, but a subsequent non-encryption-only

[tim (not reviewing), 2011/12/15 20:55:11]

how do we know it is non-encryption only?

[Nicolas Zea, 2011/12/16 19:25:14]

You're right, the comment is inaccurate. This should read non-specifics-only
change. Whether it's an encryption only change or not is unknown until we
actually attempt to resolve conflicts.

+    // change arrived, invalidating PREV_SERVER_SPECIFICS. Clear it, as it's
+    // no longer safe to assume only the specifics changed.
+    target_entry.Put(syncable::PREV_SERVER_SPECIFICS,
+                     sync_pb::EntitySpecifics());
+  }
+
   SyncerUtil::UpdateServerFieldsFromUpdate(&target_entry, update, name);
 
   return SUCCESS_PROCESSED;