Try the original CL "Always persist bookmark IDs" again with the fix to...

chrome/browser/bookmarks/bookmark_codec.cc

 // Copyright (c) 2006-2008 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 "chrome/browser/bookmarks/bookmark_codec.h"
 
 #include "app/l10n_util.h"
 #include "base/string_util.h"
 #include "base/values.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "googleurl/src/gurl.h"
 #include "grit/generated_resources.h"
 
 using base::Time;
 
 UniqueIDGenerator::UniqueIDGenerator() {
   Reset();
 }
 
-int UniqueIDGenerator::GetUniqueID(int id) {
+int64 UniqueIDGenerator::GetUniqueID(int64 id) {
   // If the given ID is already assigned, generate new ID.
   if (IsIdAssigned(id))
     id = current_max_ + 1;
 
   // Record the new ID as assigned.
   RecordId(id);
 
   if (id > current_max_)
     current_max_ = id;
 
   return id;
 }
 
-bool UniqueIDGenerator::IsIdAssigned(int id) const {
+bool UniqueIDGenerator::IsIdAssigned(int64 id) const {
   // If the set is already instantiated, then use the set to determine if the
   // given ID is assigned. Otherwise use the current maximum to determine if the
   // given ID is assigned.
   if (assigned_ids_.get())
     return assigned_ids_->find(id) != assigned_ids_->end();
   else
     return id <= current_max_;
 }
 
-void UniqueIDGenerator::RecordId(int id) {
+void UniqueIDGenerator::RecordId(int64 id) {
   // If the set is instantiated, then use the set.
   if (assigned_ids_.get()) {
     assigned_ids_->insert(id);
     return;
   }
 
   // The set is not yet instantiated. If the ID is current_max_ + 1, then just
   // update the current_max_. Otherwise, instantiate the set and add all IDs
   // from 0 to current_max_ to it.
   if (id == current_max_ + 1) {
     ++current_max_;
     return;
   }
-  assigned_ids_.reset(new std::set<int>);
-  for (int i = 0; i <= current_max_; ++i)
+  assigned_ids_.reset(new std::set<int64>);
+  for (int64 i = 0; i <= current_max_; ++i)
     assigned_ids_->insert(i);
   assigned_ids_->insert(id);
 }
 
 void UniqueIDGenerator::Reset() {
   current_max_ = 0;
   assigned_ids_.reset(NULL);
 }
 
 const wchar_t* BookmarkCodec::kRootsKey = L"roots";
 const wchar_t* BookmarkCodec::kRootFolderNameKey = L"bookmark_bar";
 const wchar_t* BookmarkCodec::kOtherBookmarkFolderNameKey = L"other";
 const wchar_t* BookmarkCodec::kVersionKey = L"version";
 const wchar_t* BookmarkCodec::kChecksumKey = L"checksum";
 const wchar_t* BookmarkCodec::kIdKey = L"id";
 const wchar_t* BookmarkCodec::kTypeKey = L"type";
 const wchar_t* BookmarkCodec::kNameKey = L"name";
 const wchar_t* BookmarkCodec::kDateAddedKey = L"date_added";
 const wchar_t* BookmarkCodec::kURLKey = L"url";
 const wchar_t* BookmarkCodec::kDateModifiedKey = L"date_modified";
 const wchar_t* BookmarkCodec::kChildrenKey = L"children";
 const wchar_t* BookmarkCodec::kTypeURL = L"url";
 const wchar_t* BookmarkCodec::kTypeFolder = L"folder";
 
 // Current version of the file.
 static const int kCurrentVersion = 1;
 
 BookmarkCodec::BookmarkCodec()
-    : persist_ids_(false) {
-}
-
-BookmarkCodec::BookmarkCodec(bool persist_ids)
-    : persist_ids_(persist_ids) {
+    : ids_reassigned_(false) {
 }
 
 Value* BookmarkCodec::Encode(BookmarkModel* model) {
   return Encode(model->GetBookmarkBarNode(), model->other_node());
 }
 
 Value* BookmarkCodec::Encode(const BookmarkNode* bookmark_bar_node,
                              const BookmarkNode* other_folder_node) {
+  ids_reassigned_ = false;
   InitializeChecksum();
   DictionaryValue* roots = new DictionaryValue();
   roots->Set(kRootFolderNameKey, EncodeNode(bookmark_bar_node));
   roots->Set(kOtherBookmarkFolderNameKey, EncodeNode(other_folder_node));
 
   DictionaryValue* main = new DictionaryValue();
   main->SetInteger(kVersionKey, kCurrentVersion);
   FinalizeChecksum();
   // We are going to store the computed checksum. So set stored checksum to be
   // the same as computed checksum.
   stored_checksum_ = computed_checksum_;
   main->Set(kChecksumKey, Value::CreateStringValue(computed_checksum_));
   main->Set(kRootsKey, roots);
   return main;
 }
 
 bool BookmarkCodec::Decode(BookmarkNode* bb_node,
                            BookmarkNode* other_folder_node,
-                           int* max_id,
+                           int64* max_id,
                            const Value& value) {
+  // TODO(munjal): Instead of paying the price of ID generator in al lcases, we
+  // should only pay the price if we detect the file has changed and do a second
+  // pass to reassign IDs. See issue 16357.
   id_generator_.Reset();
+  ids_reassigned_ = false;
   stored_checksum_.clear();
   InitializeChecksum();
-  bool success = DecodeHelper(bb_node, other_folder_node, max_id, value);
+  bool success = DecodeHelper(bb_node, other_folder_node, value);
   FinalizeChecksum();
   *max_id = id_generator_.current_max() + 1;
   return success;
 }
 
 Value* BookmarkCodec::EncodeNode(const BookmarkNode* node) {
   DictionaryValue* value = new DictionaryValue();
   std::string id;
-  if (persist_ids_) {
-    id = IntToString(node->id());
-    value->SetString(kIdKey, id);
-  }
+  id = Int64ToString(node->id());
+  value->SetString(kIdKey, id);
   const std::wstring& title = node->GetTitle();
   value->SetString(kNameKey, title);
   value->SetString(kDateAddedKey,
                    Int64ToWString(node->date_added().ToInternalValue()));
   if (node->GetType() == BookmarkNode::URL) {
     value->SetString(kTypeKey, kTypeURL);
     std::wstring url = UTF8ToWide(node->GetURL().possibly_invalid_spec());
     value->SetString(kURLKey, url);
     UpdateChecksumWithUrlNode(id, title, url);
   } else {
     value->SetString(kTypeKey, kTypeFolder);
     value->SetString(kDateModifiedKey,
                      Int64ToWString(node->date_group_modified().
                                     ToInternalValue()));
     UpdateChecksumWithFolderNode(id, title);
 
     ListValue* child_values = new ListValue();
     value->Set(kChildrenKey, child_values);
     for (int i = 0; i < node->GetChildCount(); ++i)
       child_values->Append(EncodeNode(node->GetChild(i)));
   }
   return value;
 }
 
 bool BookmarkCodec::DecodeHelper(BookmarkNode* bb_node,
                                  BookmarkNode* other_folder_node,
-                                 int* max_id,
                                  const Value& value) {
   if (value.GetType() != Value::TYPE_DICTIONARY)
     return false;  // Unexpected type.
 
   const DictionaryValue& d_value = static_cast<const DictionaryValue&>(value);
 
   int version;
   if (!d_value.GetInteger(kVersionKey, &version) || version != kCurrentVersion)
     return false;  // Unknown version.
 
@@ skipping 50 matching lines @@
 
     DecodeNode(*static_cast<DictionaryValue*>(child_value), parent, NULL);
   }
   return true;
 }
 
 bool BookmarkCodec::DecodeNode(const DictionaryValue& value,
                                BookmarkNode* parent,
                                BookmarkNode* node) {
   std::string id_string;
-  int id = 0;
-  if (persist_ids_) {
-    if (value.GetString(kIdKey, &id_string))
-      if (!StringToInt(id_string, &id))
-        return false;
-  }
-  id = id_generator_.GetUniqueID(id);
+  int64 id = 0;
+  if (value.GetString(kIdKey, &id_string))
+    if (!StringToInt64(id_string, &id))
+      return false;
+  int64 new_id = id_generator_.GetUniqueID(id);
+  if (new_id != id)
+    ids_reassigned_ = true;
+  id = new_id;
 
   std::wstring title;
   if (!value.GetString(kNameKey, &title))
     title = std::wstring();
 
   std::wstring date_added_string;
   if (!value.GetString(kDateAddedKey, &date_added_string))
     date_added_string = Int64ToWString(Time::Now().ToInternalValue());
 
   std::wstring type_string;
@@ skipping 82 matching lines @@
 
 void BookmarkCodec::InitializeChecksum() {
   MD5Init(&md5_context_);
 }
 
 void BookmarkCodec::FinalizeChecksum() {
   MD5Digest digest;
   MD5Final(&digest, &md5_context_);
   computed_checksum_ = MD5DigestToBase16(digest);
 }