Sync: Change Local IDs to GUID based to avoid ID collision

sync/syncable/directory.cc

Index: sync/syncable/directory.cc
diff --git a/sync/syncable/directory.cc b/sync/syncable/directory.cc
index 1232918f8f22dc93a34af37ab4d850d0c5807781..7e9d7476b36a469866b767a6cf5ecd91d6e5176e 100644
--- a/sync/syncable/directory.cc
+++ b/sync/syncable/directory.cc
@@ -8,6 +8,7 @@
 #include <iterator>
 
 #include "base/base64.h"
+#include "base/guid.h"
 #include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
@@ -37,8 +38,7 @@ namespace syncable {
 const base::FilePath::CharType Directory::kSyncDatabaseFilename[] =
     FILE_PATH_LITERAL("SyncData.sqlite3");
 
-Directory::PersistedKernelInfo::PersistedKernelInfo()
-    : next_id(0) {
+Directory::PersistedKernelInfo::PersistedKernelInfo() {
   ModelTypeSet protocol_types = ProtocolTypes();
   for (ModelTypeSet::Iterator iter = protocol_types.First(); iter.Good();
        iter.Inc()) {
@@ -197,15 +197,11 @@ DirOpenResult Directory::OpenImpl(
 
   DCHECK(!kernel_);
   kernel_ = new Kernel(name, info, delegate, transaction_observer);
+  kernel_->metahandles_to_purge.swap(metahandles_to_purge);
   delete_journal_.reset(new DeleteJournal(&delete_journals));
   InitializeIndices(&tmp_handles_map);
 
-  // Write back the share info to reserve some space in 'next_id'.  This will
-  // prevent local ID reuse in the case of an early crash.  See the comments in
-  // TakeSnapshotForSaveChanges() or crbug.com/142987 for more information.
-  kernel_->info_status = KERNEL_SHARE_INFO_DIRTY;
-
-  kernel_->metahandles_to_purge.swap(metahandles_to_purge);
+  // Save changes back in case there are any metahandles to purge.
   if (!SaveChanges())
     return FAILED_INITIAL_WRITE;
 
@@ -575,11 +571,6 @@ void Directory::TakeSnapshotForSaveChanges(SaveChangesSnapshot* snapshot) {
 
   // Fill kernel_info_status and kernel_info.
   snapshot->kernel_info = kernel_->persisted_info;
-  // To avoid duplicates when the process crashes, we record the next_id to be
-  // greater magnitude than could possibly be reached before the next save
-  // changes.  In other words, it's effectively impossible for the user to
-  // generate 65536 new bookmarks in 3 seconds.
-  snapshot->kernel_info.next_id -= 65536;
   snapshot->kernel_info_status = kernel_->info_status;
   // This one we reset on failure.
   kernel_->info_status = KERNEL_SHARE_INFO_VALID;
@@ -955,6 +946,7 @@ int64 Directory::GetTransactionVersion(ModelType type) const {
 void Directory::IncrementTransactionVersion(ModelType type) {
   kernel_->transaction_mutex.AssertAcquired();
   kernel_->persisted_info.transaction_version[type]++;
+  kernel_->info_status = KERNEL_SHARE_INFO_DIRTY;
 }
 
 void Directory::GetDataTypeContext(BaseTransaction* trans,
@@ -1342,14 +1334,7 @@ int64 Directory::NextMetahandle() {
 // Always returns a client ID that is the string representation of a negative

[maniscalco, 2015/05/14 15:54:20]

How about replacing this comment with a short note on what properties the
returned ID must have (must be unique, anything else?).

[stanisc, 2015/05/14 21:21:55]

Done.

 // number.
 Id Directory::NextId() {
-  int64 result;
-  {
-    ScopedKernelLock lock(this);
-    result = (kernel_->persisted_info.next_id)--;
-    kernel_->info_status = KERNEL_SHARE_INFO_DIRTY;
-  }
-  DCHECK_LT(result, 0);
-  return Id::CreateFromClientString(base::Int64ToString(result));
+  return Id::CreateFromClientString(base::GenerateGUID());
 }
 
 bool Directory::HasChildren(BaseTransaction* trans, const Id& id) {