Sync: Added extra validation to FindLocalIdToUpdate.

sync/engine/syncer_util.cc

 // Copyright 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/syncer_util.h"
 
 #include <algorithm>
 #include <set>
 #include <string>
 #include <vector>
@@ skipping 57 matching lines @@
 using syncable::SERVER_UNIQUE_POSITION;
 using syncable::SERVER_VERSION;
 using syncable::SPECIFICS;
 using syncable::SYNCER;
 using syncable::UNIQUE_BOOKMARK_TAG;
 using syncable::UNIQUE_CLIENT_TAG;
 using syncable::UNIQUE_POSITION;
 using syncable::UNIQUE_SERVER_TAG;
 using syncable::WriteTransaction;
 
+// TODO (stanisc): crbug.com/362467: remove this function once
+// issue 362467 is fixed.
+// Validates that the local ID picked by FindLocalIdToUpdate doesn't
+// conflict with already existing item with update_id.
+void VerifyLocalIdToUpdate(syncable::BaseTransaction* trans,
+                           const syncable::Id& local_id,
+                           const syncable::Id& update_id,
+                           bool local_deleted,
+                           bool deleted_in_update) {
+  if (local_id == update_id) {
+    // ID matches, everything is good.
+    return;
+  }
+
+  // If the ID doesn't match, it means that an entry with |local_id| has been
+  // picked and an entry with |update_id| isn't supposed to exist.
+  syncable::Entry update_entry(trans, GET_BY_ID, update_id);
+  if (!update_entry.good())
+    return;
+
+  // Fail early so that the crash dump indicates which of the cases below
+  // has triggered the issue.
+  // Crash dumps don't always preserve data. The 2 separate cases below are
+  // to make it easy to see the the state of item with |update_id| in the
+  // crash dump.
+  if (update_entry.GetIsDel()) {
+    LOG(FATAL) << "VerifyLocalIdToUpdate: existing deleted entry " << update_id
+               << " conflicts with local entry " << local_id
+               << " picked by an update.\n"
+               << "Local item deleted: " << local_deleted
+               << ", deleted flag in update: " << deleted_in_update;
+  } else {
+    LOG(FATAL) << "VerifyLocalIdToUpdate: existing entry " << update_id
+               << " conflicts with local entry " << local_id
+               << " picked by an update.\n"
+               << "Local item deleted: " << local_deleted
+               << ", deleted flag in update: " << deleted_in_update;
+  }
+}
+
 syncable::Id FindLocalIdToUpdate(
     syncable::BaseTransaction* trans,
     const sync_pb::SyncEntity& update) {
   // Expected entry points of this function:
   // SyncEntity has NOT been applied to SERVER fields.
   // SyncEntity has NOT been applied to LOCAL fields.
   // DB has not yet been modified, no entries created for this update.
 
   const std::string& client_id = trans->directory()->cache_guid();
   const syncable::Id& update_id = SyncableIdFromProto(update.id_string());
@@ skipping 32 matching lines @@
             // Signal an error; drop this update on the floor.  Note that
             // we don't server delete the item, because we don't allow it to
             // exist locally at all.  So the item will remain orphaned on
             // the server, and we won't pay attention to it.
             return syncable::Id();
           }
         }
         // Target this change to the existing local entry; later,
         // we'll change the ID of the local entry to update_id
         // if needed.
+        VerifyLocalIdToUpdate(trans, local_entry.GetId(), update_id,
+                              local_entry.GetIsDel(), update.deleted());
         return local_entry.GetId();
       } else {
         // Case 3: We have a local entry with the same client tag.
         // We should change the ID of the local entry to the server entry.
         // This will result in an server ID with base version == 0, but that's
         // a legal state for an item with a client tag.  By changing the ID,
         // update will now be applied to local_entry.
         DCHECK(0 == local_entry.GetBaseVersion() ||
                CHANGES_VERSION == local_entry.GetBaseVersion());
+        VerifyLocalIdToUpdate(trans, local_entry.GetId(), update_id,
+                              local_entry.GetIsDel(), update.deleted());
         return local_entry.GetId();
       }
     }
   } else if (update.has_originator_cache_guid() &&
       update.originator_cache_guid() == client_id) {
     // If a commit succeeds, but the response does not come back fast enough
     // then the syncer might assume that it was never committed.
     // The server will track the client that sent up the original commit and
     // return this in a get updates response. When this matches a local
     // uncommitted item, we must mutate our local item and version to pick up
@@ skipping 26 matching lines @@
       // An entry should never be version 0 and SYNCED.
       DCHECK(local_entry.GetIsUnsynced());
 
       // Just a quick sanity check.
       DCHECK(!local_entry.GetId().ServerKnows());
 
       DVLOG(1) << "Reuniting lost commit response IDs. server id: "
                << update_id << " local id: " << local_entry.GetId()
                << " new version: " << new_version;
 
+      VerifyLocalIdToUpdate(trans, local_entry.GetId(), update_id,
+                            local_entry.GetIsDel(), update.deleted());
       return local_entry.GetId();
     }
   } else if (update.has_server_defined_unique_tag() &&
              !update.server_defined_unique_tag().empty()) {
     // The client creates type root folders with a local ID on demand when a
     // progress marker for the given type is initially set.
     // The server might also attempt to send a type root folder for the same
     // type (during the transition period until support for root folders is
     // removed for new client versions).
     // TODO(stanisc): crbug.com/438313: remove this once the transition to
     // implicit root folders on the server is done.
     syncable::Entry local_entry(trans, syncable::GET_BY_SERVER_TAG,
                                 update.server_defined_unique_tag());
     if (local_entry.good() && !local_entry.GetId().ServerKnows()) {
       DCHECK(local_entry.GetId() != update_id);
+      VerifyLocalIdToUpdate(trans, local_entry.GetId(), update_id,
+                            local_entry.GetIsDel(), update.deleted());
       return local_entry.GetId();
     }
   }
 
   // Fallback: target an entry having the server ID, creating one if needed.
   return update_id;
 }
 
 UpdateAttemptResponse AttemptToUpdateEntry(
     syncable::WriteTransaction* const trans,
@@ skipping 459 matching lines @@
   if (update.version() < target->GetServerVersion()) {
     LOG(WARNING) << "Update older than current server version for "
                  << *target << " Update:"
                  << SyncerProtoUtil::SyncEntityDebugString(update);
     return VERIFY_SUCCESS;  // Expected in new sync protocol.
   }
   return VERIFY_UNDECIDED;
 }
 
 }  // namespace syncer