sync: Improve handling of bad UniquePos (retry)

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>
 
 #include "base/base64.h"
 #include "base/location.h"
 #include "base/metrics/histogram.h"
+#include "base/rand_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "sync/engine/conflict_resolver.h"
 #include "sync/engine/syncer_proto_util.h"
 #include "sync/engine/syncer_types.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/base/unique_position.h"
 #include "sync/protocol/bookmark_specifics.pb.h"
 #include "sync/protocol/password_specifics.pb.h"
 #include "sync/protocol/sync.pb.h"
 #include "sync/syncable/directory.h"
@@ skipping 246 matching lines @@
   }
 
   UpdateLocalDataFromServerData(trans, entry);
 
   return SUCCESS;
 }
 
 std::string GetUniqueBookmarkTagFromUpdate(const sync_pb::SyncEntity& update) {
   if (!update.has_originator_cache_guid() ||
       !update.has_originator_client_item_id()) {
-    return std::string();
+    LOG(ERROR) << "Update is missing requirements for bookmark position."
+               << " This is a server bug.";
+    return UniquePosition::RandomSuffix();
   }
 
   return syncable::GenerateSyncableBookmarkHash(
       update.originator_cache_guid(), update.originator_client_item_id());
 }
 
 UniquePosition GetUpdatePosition(const sync_pb::SyncEntity& update,
                                  const std::string& suffix) {
   DCHECK(UniquePosition::IsValidSuffix(suffix));
   if (!(SyncerProtoUtil::ShouldMaintainPosition(update))) {
     return UniquePosition::CreateInvalid();
   } else if (update.has_unique_position()) {
     return UniquePosition::FromProto(update.unique_position());
   } else if (update.has_position_in_parent()) {
     return UniquePosition::FromInt64(update.position_in_parent(), suffix);
   } else {
-    return UniquePosition::CreateInvalid();
+    LOG(ERROR) << "No position information in update. This is a server bug.";
+    return UniquePosition::FromInt64(0, suffix);
   }
 }
 
 namespace {
 
 // Helper to synthesize a new-style sync_pb::EntitySpecifics for use locally,
 // when the server speaks only the old sync_pb::SyncEntity_BookmarkData-based
 // protocol.
 void UpdateBookmarkSpecifics(const std::string& singleton_tag,
                              const std::string& url,
@@ skipping 25 matching lines @@
   std::string bookmark_tag = GetUniqueBookmarkTagFromUpdate(update);
   if (UniquePosition::IsValidSuffix(bookmark_tag)) {
     local_entry->PutUniqueBookmarkTag(bookmark_tag);
   }
 
   // Update our position.
   UniquePosition update_pos =
       GetUpdatePosition(update, local_entry->GetUniqueBookmarkTag());
   if (update_pos.IsValid()) {
     local_entry->PutServerUniquePosition(update_pos);
-  } else {
-    // TODO(sync): This and other cases of unexpected input should be handled
-    // better.
-    NOTREACHED();
   }
 }
 
 }  // namespace
 
 void UpdateServerFieldsFromUpdate(
     syncable::ModelNeutralMutableEntry* target,
     const sync_pb::SyncEntity& update,
     const std::string& name) {
   if (update.deleted()) {
@@ skipping 283 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