sync: Process 'control' data separately

sync/engine/conflict_resolver.cc

 // 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_resolver.h"
 
 #include <algorithm>
 #include <list>
 #include <map>
 #include <set>
 
 #include "base/location.h"
 #include "base/metrics/histogram.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"
 #include "sync/syncable/write_transaction.h"
 #include "sync/util/cryptographer.h"
 
 using std::list;
 using std::map;
 using std::set;
 using syncable::BaseTransaction;
@@ skipping 15 matching lines @@
 const int SYNC_CYCLES_BEFORE_ADMITTING_DEFEAT = 8;
 
 }  // namespace
 
 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,
                                         const Cryptographer* cryptographer,
                                         StatusController* status) {
   MutableEntry entry(trans, syncable::GET_BY_ID, id);
   // Must be good as the entry won't have been cleaned up.
   CHECK(entry.good());
 
   // This function can only resolve simple conflicts.  Simple conflicts have
@@ skipping 134 matching lines @@
         decrypted_base_server_specifics =
             base_server_specifics.SerializeAsString();
       } else {
         decrypted_base_server_specifics = cryptographer->DecryptToString(
             base_server_specifics.encrypted());
       }
       if (decrypted_server_specifics == decrypted_base_server_specifics)
           base_server_specifics_match = true;
     }
 
-    // We manually merge nigori data.
-    if (entry.GetModelType() == syncable::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).
-      cryptographer->UpdateNigoriFromEncryptedTypes(server_nigori);
-      // 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);
+      SyncerUtil::IgnoreConflict(&entry);
       UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
                                 CHANGES_MATCH,
                                 CONFLICT_RESOLUTION_SIZE);
     } else if (base_server_specifics_match) {
       DVLOG(1) << "Resolving simple conflict, ignoring server encryption "
                << " changes for: " << entry;
       status->increment_num_server_overwrites();
-      OverwriteServerChanges(trans, &entry);
+      SyncerUtil::OverwriteServerChanges(&entry);
       UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
                                 IGNORE_ENCRYPTION,
                                 CONFLICT_RESOLUTION_SIZE);
     } else if (entry_deleted || !name_matches || !parent_matches) {
-      OverwriteServerChanges(trans, &entry);
+      SyncerUtil::OverwriteServerChanges(&entry);
       status->increment_num_server_overwrites();
       DVLOG(1) << "Resolving simple conflict, overwriting server changes "
                << "for: " << entry;
       UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
                                 OVERWRITE_SERVER,
                                 CONFLICT_RESOLUTION_SIZE);
     } else {
       DVLOG(1) << "Resolving simple conflict, ignoring local changes for: "
                << entry;
-      IgnoreLocalChanges(&entry);
+      SyncerUtil::IgnoreLocalChanges(&entry);
       status->increment_num_local_overwrites();
       UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
                                 OVERWRITE_LOCAL,
                                 CONFLICT_RESOLUTION_SIZE);
     }
     // Now that we've resolved the conflict, clear the prev server
     // specifics.
     entry.Put(syncable::BASE_SERVER_SPECIFICS, sync_pb::EntitySpecifics());
     return SYNC_PROGRESS;
   } else {  // SERVER_IS_DEL is true
@@ skipping 13 matching lines @@
       }
     }
 
     // The entry is deleted on the server but still exists locally.
     if (!entry.Get(syncable::UNIQUE_CLIENT_TAG).empty()) {
       // If we've got a client-unique tag, we can undelete while retaining
       // our present ID.
       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);
+      SyncerUtil::OverwriteServerChanges(&entry);
       status->increment_num_server_overwrites();
       DVLOG(1) << "Resolving simple conflict, undeleting server entry: "
                << entry;
       UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
                                 OVERWRITE_SERVER,
                                 CONFLICT_RESOLUTION_SIZE);
       // Clobber the versions, just in case the above DCHECK is violated.
       entry.Put(syncable::SERVER_VERSION, 0);
       entry.Put(syncable::BASE_VERSION, 0);
     } else {
@@ skipping 57 matching lines @@
           forward_progress = true;
           break;
       }
       processed_items.insert(id);
     }
   }
   return forward_progress;
 }
 
 }  // namespace csync