Always persist bookmark IDs.

Remove the preference to persist IDs.
NOTE that we need to save the file the first time with IDs
since existing bookmark files won't have IDs and the file
won't be saved until something changes in the bookmark model.
So we need to explicitly save once when we assign ids for the
first time.

TEST=NONE
BUG=16068

Review URL: http://codereview.chromium.org/149310

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@20532 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 104 insertions, 146 deletions

diff --git a/chrome/browser/bookmarks/bookmark_codec.cc b/chrome/browser/bookmarks/bookmark_codec.cc
index 4e4f5ed..92f6dcf 100644
--- a/chrome/browser/bookmarks/bookmark_codec.cc
+++ b/chrome/browser/bookmarks/bookmark_codec.cc
@@ -17,7 +17,7 @@ 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;
@@ -31,7 +31,7 @@ int UniqueIDGenerator::GetUniqueID(int 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.
@@ -41,7 +41,7 @@ bool UniqueIDGenerator::IsIdAssigned(int id) const {
     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);
@@ -55,8 +55,8 @@ void UniqueIDGenerator::RecordId(int id) {
     ++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);
 }
@@ -85,11 +85,7 @@ const wchar_t* BookmarkCodec::kTypeFolder = L"folder";
 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) {
@@ -98,6 +94,7 @@ Value* BookmarkCodec::Encode(BookmarkModel* model) {
 
 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));
@@ -116,12 +113,16 @@ Value* BookmarkCodec::Encode(const BookmarkNode* bookmark_bar_node,
 
 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;
@@ -130,10 +131,8 @@ bool BookmarkCodec::Decode(BookmarkNode* bb_node,
 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,
@@ -160,7 +159,6 @@ Value* BookmarkCodec::EncodeNode(const BookmarkNode* node) {
 
 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.
@@ -231,13 +229,14 @@ 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))
diff --git a/chrome/browser/bookmarks/bookmark_codec.h b/chrome/browser/bookmarks/bookmark_codec.h
index e6292a8..2c13cef 100644
--- a/chrome/browser/bookmarks/bookmark_codec.h
+++ b/chrome/browser/bookmarks/bookmark_codec.h
@@ -22,7 +22,7 @@ class DictionaryValue;
 class ListValue;
 class Value;
 
-// Utility class to help assign unique integer IDs.
+// Utility class to help assign unique 64-bit IDs.
 class UniqueIDGenerator {
  public:
   UniqueIDGenerator();
@@ -31,25 +31,26 @@ class UniqueIDGenerator {
   // returns the id itself, otherwise generates a new unique id in a simple way
   // and returns that.
   // NOTE that if id is 0, a new unique id is returned.
-  int GetUniqueID(int id);
+  int64 GetUniqueID(int64 id);
 
   // Resets the ID generator to initial state.
   void Reset();
 
   // Returns the current maximum.
-  int current_max() const { return current_max_; }
+  int64 current_max() const { return current_max_; }
 
  private:
   // Checks if the given ID is already assigned.
-  bool IsIdAssigned(int id) const;
+  bool IsIdAssigned(int64 id) const;
 
   // Records the given ID as assigned.
-  void RecordId(int id);
+  void RecordId(int64 id);
 
   // Maximum value we have seen so far.
-  int current_max_;
+  int64 current_max_;
+
   // All IDs assigned so far.
-  scoped_ptr<std::set<int> > assigned_ids_;
+  scoped_ptr<std::set<int64> > assigned_ids_;
 
   DISALLOW_COPY_AND_ASSIGN(UniqueIDGenerator);
 };
@@ -59,14 +60,11 @@ class UniqueIDGenerator {
 
 class BookmarkCodec {
  public:
-  // Creates an instance of the codec. Encodes/decodes bookmark IDs also if
-  // persist_ids is true. The default constructor will not encode/decode IDs.
-  // During decoding, if persist_ids is true and if the IDs in the file are not
-  // unique, we will reassign IDs to make them unique. There are no guarantees
-  // on how the IDs are reassigned or about doing minimal reassignments to
-  // achieve uniqueness.
+  // Creates an instance of the codec. During decoding, if the IDs in the file
+  // are not unique, we will reassign IDs to make them unique. There are no
+  // guarantees on how the IDs are reassigned or about doing minimal
+  // reassignments to achieve uniqueness.
   BookmarkCodec();
-  explicit BookmarkCodec(bool persist_ids);
 
   // Encodes the model to a JSON value. It's up to the caller to delete the
   // returned object. This is invoked to encode the contents of the bookmark bar
@@ -88,7 +86,7 @@ class BookmarkCodec {
   // |max_node_id| is set to the max id of the nodes.
   bool Decode(BookmarkNode* bb_node,
               BookmarkNode* other_folder_node,
-              int* max_node_id,
+              int64* max_node_id,
               const Value& value);
 
   // Returns the checksum computed during last encoding/decoding call.
@@ -101,6 +99,10 @@ class BookmarkCodec {
   // user.
   const std::string& stored_checksum() const { return stored_checksum_; }
 
+  // Returns whether the IDs were reassigned during decoding. Always returns
+  // false after encoding.
+  bool ids_reassigned() const { return ids_reassigned_; }
+
   // Names of the various keys written to the Value.
   static const wchar_t* kRootsKey;
   static const wchar_t* kRootFolderNameKey;
@@ -127,7 +129,6 @@ class BookmarkCodec {
   // Helper to perform decoding.
   bool DecodeHelper(BookmarkNode* bb_node,
                     BookmarkNode* other_folder_node,
-                    int* max_id,
                     const Value& value);
 
   // Decodes the children of the specified node. Returns true on success.
@@ -160,11 +161,12 @@ class BookmarkCodec {
   void InitializeChecksum();
   void FinalizeChecksum();
 
-  // Whether to persist IDs or not.
-  bool persist_ids_;
   // Unique ID generator used during decoding.
   UniqueIDGenerator id_generator_;
 
+  // Whether or not IDs were reassigned by the codec.
+  bool ids_reassigned_;
+
   // MD5 context used to compute MD5 hash of all bookmark data.
   MD5Context md5_context_;
 
diff --git a/chrome/browser/bookmarks/bookmark_codec_unittest.cc b/chrome/browser/bookmarks/bookmark_codec_unittest.cc
index d6a3424..ed3996e 100644
--- a/chrome/browser/bookmarks/bookmark_codec_unittest.cc
+++ b/chrome/browser/bookmarks/bookmark_codec_unittest.cc
@@ -106,7 +106,7 @@ class BookmarkCodecTest : public testing::Test {
   }
 
   bool Decode(BookmarkCodec* codec, BookmarkModel* model, const Value& value) {
-    int max_id;
+    int64 max_id;
     bool result = codec->Decode(AsMutable(model->GetBookmarkBarNode()),
                                 AsMutable(model->other_node()),
                                 &max_id, value);
@@ -201,11 +201,11 @@ TEST_F(BookmarkCodecTest, ChecksumManualEditTest) {
 
 TEST_F(BookmarkCodecTest, PersistIDsTest) {
   scoped_ptr<BookmarkModel> model_to_encode(CreateTestModel3());
-  BookmarkCodec encoder(true);
+  BookmarkCodec encoder;
   scoped_ptr<Value> model_value(encoder.Encode(model_to_encode.get()));
 
   BookmarkModel decoded_model(NULL);
-  BookmarkCodec decoder(true);
+  BookmarkCodec decoder;
   ASSERT_TRUE(Decode(&decoder, &decoded_model, *model_value.get()));
   BookmarkModelTestUtils::AssertModelsEqual(model_to_encode.get(),
                                             &decoded_model,
@@ -220,11 +220,11 @@ TEST_F(BookmarkCodecTest, PersistIDsTest) {
       bookmark_bar, bookmark_bar->GetChildCount(), kGroup2Title);
   decoded_model.AddURL(group2_node, 0, kUrl4Title, GURL(kUrl4Url));
 
-  BookmarkCodec encoder2(true);
+  BookmarkCodec encoder2;
   scoped_ptr<Value> model_value2(encoder2.Encode(&decoded_model));
 
   BookmarkModel decoded_model2(NULL);
-  BookmarkCodec decoder2(true);
+  BookmarkCodec decoder2;
   ASSERT_TRUE(Decode(&decoder2, &decoded_model2, *model_value2.get()));
   BookmarkModelTestUtils::AssertModelsEqual(&decoded_model,
                                             &decoded_model2,
@@ -235,17 +235,17 @@ class UniqueIDGeneratorTest : public testing::Test {
  protected:
   void TestMixed(UniqueIDGenerator* gen) {
     // Few unique numbers.
-    for (int i = 1; i <= 5; ++i) {
+    for (int64 i = 1; i <= 5; ++i) {
       EXPECT_EQ(i, gen->GetUniqueID(i));
     }
 
     // All numbers from 1 to 5 should produce numbers 6 to 10.
-    for (int i = 1; i <= 5; ++i) {
+    for (int64 i = 1; i <= 5; ++i) {
       EXPECT_EQ(5 + i, gen->GetUniqueID(i));
     }
 
     // 10 should produce 11, then 11 should produce 12, and so on.
-    for (int i = 1; i <= 5; ++i) {
+    for (int64 i = 1; i <= 5; ++i) {
       EXPECT_EQ(10 + i, gen->GetUniqueID(9 + i));
     }
 
@@ -272,14 +272,14 @@ class UniqueIDGeneratorTest : public testing::Test {
 
 TEST_F(UniqueIDGeneratorTest, SerialNumbersTest) {
   UniqueIDGenerator gen;
-  for (int i = 1; i <= 10; ++i) {
+  for (int64 i = 1; i <= 10; ++i) {
     EXPECT_EQ(i, gen.GetUniqueID(i));
   }
 }
 
 TEST_F(UniqueIDGeneratorTest, UniqueSortedNumbersTest) {
   UniqueIDGenerator gen;
-  for (int i = 1; i <= 10; i += 2) {
+  for (int64 i = 1; i <= 10; i += 2) {
     EXPECT_EQ(i, gen.GetUniqueID(i));
   }
 }
@@ -287,7 +287,7 @@ TEST_F(UniqueIDGeneratorTest, UniqueSortedNumbersTest) {
 TEST_F(UniqueIDGeneratorTest, UniqueUnsortedConsecutiveNumbersTest) {
   UniqueIDGenerator gen;
   int numbers[] = {2, 10, 6, 3, 8, 5, 1, 7, 4, 9};
-  for (int i = 0; i < ARRAYSIZE(numbers); ++i) {
+  for (int64 i = 0; i < ARRAYSIZE(numbers); ++i) {
     EXPECT_EQ(numbers[i], gen.GetUniqueID(numbers[i]));
   }
 }
@@ -295,14 +295,14 @@ TEST_F(UniqueIDGeneratorTest, UniqueUnsortedConsecutiveNumbersTest) {
 TEST_F(UniqueIDGeneratorTest, UniqueUnsortedNumbersTest) {
   UniqueIDGenerator gen;
   int numbers[] = {20, 100, 60, 30, 80, 50, 10, 70, 40, 90};
-  for (int i = 0; i < ARRAYSIZE(numbers); ++i) {
+  for (int64 i = 0; i < ARRAYSIZE(numbers); ++i) {
     EXPECT_EQ(numbers[i], gen.GetUniqueID(numbers[i]));
   }
 }
 
 TEST_F(UniqueIDGeneratorTest, AllDuplicatesTest) {
   UniqueIDGenerator gen;
-  for (int i = 1; i <= 10; ++i) {
+  for (int64 i = 1; i <= 10; ++i) {
     EXPECT_EQ(i, gen.GetUniqueID(1));
   }
 }
@@ -314,7 +314,7 @@ TEST_F(UniqueIDGeneratorTest, MixedTest) {
 
 TEST_F(UniqueIDGeneratorTest, ResetTest) {
   UniqueIDGenerator gen;
-  for (int i = 0; i < 5; ++i) {
+  for (int64 i = 0; i < 5; ++i) {
     TestMixed(&gen);
     gen.Reset();
   }
diff --git a/chrome/browser/bookmarks/bookmark_drag_data.cc b/chrome/browser/bookmarks/bookmark_drag_data.cc
index 732d119..bf3ae05 100644
--- a/chrome/browser/bookmarks/bookmark_drag_data.cc
+++ b/chrome/browser/bookmarks/bookmark_drag_data.cc
@@ -41,7 +41,7 @@ void BookmarkDragData::Element::WriteToPickle(Pickle* pickle) const {
   pickle->WriteBool(is_url);
   pickle->WriteString(url.spec());
   pickle->WriteWString(title);
-  pickle->WriteInt(id_);
+  pickle->WriteInt64(id_);
   if (!is_url) {
     pickle->WriteSize(children.size());
     for (std::vector<Element>::const_iterator i = children.begin();
@@ -57,7 +57,7 @@ bool BookmarkDragData::Element::ReadFromPickle(Pickle* pickle,
   if (!pickle->ReadBool(iterator, &is_url) ||
       !pickle->ReadString(iterator, &url_spec) ||
       !pickle->ReadWString(iterator, &title) ||
-      !pickle->ReadInt(iterator, &id_)) {
+      !pickle->ReadInt64(iterator, &id_)) {
     return false;
   }
   url = GURL(url_spec);
diff --git a/chrome/browser/bookmarks/bookmark_drag_data.h b/chrome/browser/bookmarks/bookmark_drag_data.h
index 88cb6d4..9af3c20 100644
--- a/chrome/browser/bookmarks/bookmark_drag_data.h
+++ b/chrome/browser/bookmarks/bookmark_drag_data.h
@@ -62,7 +62,7 @@ struct BookmarkDragData {
     bool ReadFromPickle(Pickle* pickle, void** iterator);
 
     // ID of the node.
-    int id_;
+    int64 id_;
   };
 
   BookmarkDragData() { }
diff --git a/chrome/browser/bookmarks/bookmark_model.cc b/chrome/browser/bookmarks/bookmark_model.cc
index 5d87da1..9bcc8c3 100644
--- a/chrome/browser/bookmarks/bookmark_model.cc
+++ b/chrome/browser/bookmarks/bookmark_model.cc
@@ -35,12 +35,12 @@ BookmarkNode::BookmarkNode(const GURL& url)
   Initialize(0);
 }
 
-BookmarkNode::BookmarkNode(int id, const GURL& url)
+BookmarkNode::BookmarkNode(int64 id, const GURL& url)
     : url_(url){
   Initialize(id);
 }
 
-void BookmarkNode::Initialize(int id) {
+void BookmarkNode::Initialize(int64 id) {
   id_ = id;
   loaded_favicon_ = false;
   favicon_load_handle_ = 0;
@@ -77,9 +77,6 @@ void BookmarkNode::Reset(const history::StarredEntry& entry) {
 
 namespace {
 
-// Constant for persist IDs prefernece.
-const wchar_t kPrefPersistIDs[] = L"bookmarks.persist_ids";
-
 // Comparator used when sorting bookmarks. Folders are sorted first, then
   // bookmarks.
 class SortComparator : public std::binary_function<const BookmarkNode*,
@@ -110,7 +107,6 @@ class SortComparator : public std::binary_function<const BookmarkNode*,
 BookmarkModel::BookmarkModel(Profile* profile)
     : profile_(profile),
       loaded_(false),
-      persist_ids_(false),
       file_changed_(false),
       root_(GURL()),
       bookmark_bar_node_(NULL),
@@ -122,8 +118,6 @@ BookmarkModel::BookmarkModel(Profile* profile)
     // Profile is null during testing.
     DoneLoading(CreateLoadDetails());
   }
-  RegisterPreferences();
-  LoadPreferences();
 }
 
 BookmarkModel::~BookmarkModel() {
@@ -282,7 +276,7 @@ bool BookmarkModel::IsBookmarked(const GURL& url) {
   return IsBookmarkedNoLock(url);
 }
 
-const BookmarkNode* BookmarkModel::GetNodeByID(int id) {
+const BookmarkNode* BookmarkModel::GetNodeByID(int64 id) {
   // TODO(sky): TreeNode needs a method that visits all nodes using a predicate.
   return GetNodeByID(&root_, id);
 }
@@ -408,19 +402,6 @@ void BookmarkModel::ClearStore() {
   store_ = NULL;
 }
 
-void BookmarkModel::SetPersistIDs(bool value) {
-  if (value == persist_ids_)
-    return;
-  persist_ids_ = value;
-  if (profile_) {
-    PrefService* pref_service = profile_->GetPrefs();
-    pref_service->SetBoolean(kPrefPersistIDs, persist_ids_);
-  }
-  // Need to save the bookmark data if the value of persist IDs changes.
-  if (store_.get())
-    store_->ScheduleSave();
-}
-
 bool BookmarkModel::IsBookmarkedNoLock(const GURL& url) {
   BookmarkNode tmp_node(url);
   return (nodes_ordered_by_url_set_.find(&tmp_node) !=
@@ -477,6 +458,15 @@ void BookmarkModel::DoneLoading(
   next_node_id_ = details->max_id();
   if (details->computed_checksum() != details->stored_checksum())
     SetFileChanged();
+  if (details->computed_checksum() != details->stored_checksum() ||
+      details->ids_reassigned()) {
+    // If bookmarks file changed externally, the IDs may have changed
+    // externally. In that case, the decoder may have reassigned IDs to make
+    // them unique. So when the file has changed externally, we should save the
+    // bookmarks file to persist new IDs.
+    if (store_.get())
+      store_->ScheduleSave();
+  }
   index_.reset(details->index());
   details->release();
 
@@ -587,7 +577,7 @@ void BookmarkModel::BlockTillLoaded() {
 }
 
 const BookmarkNode* BookmarkModel::GetNodeByID(const BookmarkNode* node,
-                                               int id) {
+                                               int64 id) {
   if (node->id() == id)
     return node;
 
@@ -725,18 +715,12 @@ void BookmarkModel::PopulateNodesByURL(BookmarkNode* node) {
     PopulateNodesByURL(node->GetChild(i));
 }
 
-int BookmarkModel::generate_next_node_id() {
+int64 BookmarkModel::generate_next_node_id() {
   return next_node_id_++;
 }
 
 void BookmarkModel::SetFileChanged() {
   file_changed_ = true;
-  // If bookmarks file changed externally, the IDs may have changed externally.
-  // in that case, the decoder may have reassigned IDs to make them unique.
-  // So when the file has changed externally and IDs are persisted, we should
-  // save the bookmarks file to persist new IDs.
-  if (persist_ids_ && store_.get())
-    store_->ScheduleSave();
 }
 
 BookmarkStorage::LoadDetails* BookmarkModel::CreateLoadDetails() {
@@ -745,18 +729,3 @@ BookmarkStorage::LoadDetails* BookmarkModel::CreateLoadDetails() {
   return new BookmarkStorage::LoadDetails(
       bb_node, other_folder_node, new BookmarkIndex(), next_node_id_);
 }
-
-void BookmarkModel::RegisterPreferences() {
-  if (!profile_)
-    return;
-  PrefService* pref_service = profile_->GetPrefs();
-  if (!pref_service->IsPrefRegistered(kPrefPersistIDs))
-    pref_service->RegisterBooleanPref(kPrefPersistIDs, false);
-}
-
-void BookmarkModel::LoadPreferences() {
-  if (!profile_)
-    return;
-  PrefService* pref_service = profile_->GetPrefs();
-  persist_ids_ = pref_service->GetBoolean(kPrefPersistIDs);
-}
diff --git a/chrome/browser/bookmarks/bookmark_model.h b/chrome/browser/bookmarks/bookmark_model.h
index 9726624..c8ba083 100644
--- a/chrome/browser/bookmarks/bookmark_model.h
+++ b/chrome/browser/bookmarks/bookmark_model.h
@@ -53,19 +53,18 @@ class BookmarkNode : public TreeNode<BookmarkNode> {
   // Creates a new node with the specified url and id of 0
   explicit BookmarkNode(const GURL& url);
   // Creates a new node with the specified url and id.
-  BookmarkNode(int id, const GURL& url);
+  BookmarkNode(int64 id, const GURL& url);
   virtual ~BookmarkNode() {}
 
   // Returns the URL.
   const GURL& GetURL() const { return url_; }
 
   // Returns a unique id for this node.
-  //
-  // NOTE: this id is only unique for the session and NOT unique across
-  // sessions. Don't persist it!
-  int id() const { return id_; }
+  // For bookmark nodes that are managed by the bookmark model, the IDs are
+  // persisted across sessions.
+  int64 id() const { return id_; }
   // Sets the id to the given value.
-  void set_id(int id) { id_ = id; }
+  void set_id(int64 id) { id_ = id; }
 
   // Returns the type of this node.
   BookmarkNode::Type GetType() const { return type_; }
@@ -126,10 +125,10 @@ class BookmarkNode : public TreeNode<BookmarkNode> {
 
  private:
   // helper to initialize various fields during construction.
-  void Initialize(int id);
+  void Initialize(int64 id);
 
   // Unique identifier for this node.
-  int id_;
+  int64 id_;
 
   // Whether the favicon has been loaded.
   bool loaded_favicon_;
@@ -292,7 +291,7 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
 
   // Returns the node with the specified id, or NULL if there is no node with
   // the specified id.
-  const BookmarkNode* GetNodeByID(int id);
+  const BookmarkNode* GetNodeByID(int64 id);
 
   // Adds a new group node at the specified position.
   const BookmarkNode* AddGroup(const BookmarkNode* parent,
@@ -355,10 +354,6 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
   // testing.
   void ClearStore();
 
-  // Sets/returns whether or not bookmark IDs are persisted or not.
-  bool PersistIDs() const { return persist_ids_; }
-  void SetPersistIDs(bool value);
-
   // Returns whether the bookmarks file changed externally.
   bool file_changed() const { return file_changed_; }
 
@@ -404,7 +399,7 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
                         bool was_bookmarked);
 
   // Implementation of GetNodeByID.
-  const BookmarkNode* GetNodeByID(const BookmarkNode* node, int id);
+  const BookmarkNode* GetNodeByID(const BookmarkNode* node, int64 id);
 
   // Returns true if the parent and index are valid.
   bool IsValidIndex(const BookmarkNode* parent, int index, bool allow_end);
@@ -444,13 +439,12 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
                        const NotificationDetails& details);
 
   // Generates and returns the next node ID.
-  int generate_next_node_id();
+  int64 generate_next_node_id();
 
   // Sets the maximum node ID to the given value.
   // This is used by BookmarkCodec to report the maximum ID after it's done
-  // decoding since during decoding codec can assign IDs to nodes if IDs are
-  // persisted.
-  void set_next_node_id(int id) { next_node_id_ = id; }
+  // decoding since during decoding codec assigns node IDs.
+  void set_next_node_id(int64 id) { next_node_id_ = id; }
 
   // Records that the bookmarks file was changed externally.
   void SetFileChanged();
@@ -459,11 +453,6 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
   // the returned object.
   BookmarkStorage::LoadDetails* CreateLoadDetails();
 
-  // Registers bookmarks related prefs.
-  void RegisterPreferences();
-  // Loads bookmark related preferences.
-  void LoadPreferences();
-
   NotificationRegistrar registrar_;
 
   Profile* profile_;
@@ -471,9 +460,6 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
   // Whether the initial set of data has been loaded.
   bool loaded_;
 
-  // Whether to persist bookmark IDs.
-  bool persist_ids_;
-
   // Whether the bookmarks file was changed externally. This is set after
   // loading is complete and once set the value never changes.
   bool file_changed_;
@@ -486,7 +472,7 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
   BookmarkNode* other_node_;
 
   // The maximum ID assigned to the bookmark nodes in the model.
-  int next_node_id_;
+  int64 next_node_id_;
 
   // The observers.
   ObserverList<BookmarkModelObserver> observers_;
diff --git a/chrome/browser/bookmarks/bookmark_model_unittest.cc b/chrome/browser/bookmarks/bookmark_model_unittest.cc
index 05d0843..627a058 100644
--- a/chrome/browser/bookmarks/bookmark_model_unittest.cc
+++ b/chrome/browser/bookmarks/bookmark_model_unittest.cc
@@ -593,7 +593,7 @@ class BookmarkModelTestWithProfile : public testing::Test,
 
   void VerifyNoDuplicateIDs(BookmarkModel* model) {
     TreeNodeIterator<const BookmarkNode> it(model->root_node());
-    base::hash_set<int> ids;
+    base::hash_set<int64> ids;
     while (it.has_next())
       ASSERT_TRUE(ids.insert(it.Next()->id()).second);
   }
diff --git a/chrome/browser/bookmarks/bookmark_storage.cc b/chrome/browser/bookmarks/bookmark_storage.cc
index a530158..d7dbe19 100644
--- a/chrome/browser/bookmarks/bookmark_storage.cc
+++ b/chrome/browser/bookmarks/bookmark_storage.cc
@@ -68,13 +68,11 @@ class BookmarkStorage::LoadTask : public Task {
   LoadTask(const FilePath& path,
            MessageLoop* loop,
            BookmarkStorage* storage,
-           LoadDetails* details,
-           bool persist_ids)
+           LoadDetails* details)
       : path_(path),
         loop_(loop),
         storage_(storage),
-        details_(details),
-        persist_ids_(persist_ids) {
+        details_(details) {
   }
 
   virtual void Run() {
@@ -86,14 +84,15 @@ class BookmarkStorage::LoadTask : public Task {
       if (root.get()) {
         // Building the index cane take a while, so we do it on the background
         // thread.
-        int max_node_id = 0;
-        BookmarkCodec codec(persist_ids_);
+        int64 max_node_id = 0;
+        BookmarkCodec codec;
         TimeTicks start_time = TimeTicks::Now();
         codec.Decode(details_->bb_node(), details_->other_folder_node(),
                      &max_node_id, *root.get());
         details_->set_max_id(std::max(max_node_id, details_->max_id()));
         details_->set_computed_checksum(codec.computed_checksum());
         details_->set_stored_checksum(codec.stored_checksum());
+        details_->set_ids_reassigned(codec.ids_reassigned());
         UMA_HISTOGRAM_TIMES("Bookmarks.DecodeTime",
                             TimeTicks::Now() - start_time);
 
@@ -129,7 +128,6 @@ class BookmarkStorage::LoadTask : public Task {
   MessageLoop* loop_;
   scoped_refptr<BookmarkStorage> storage_;
   LoadDetails* details_;
-  bool persist_ids_;
 
   DISALLOW_COPY_AND_ASSIGN(LoadTask);
 };
@@ -164,8 +162,7 @@ void BookmarkStorage::DoLoadBookmarks(const FilePath& path) {
   Task* task = new LoadTask(path,
                             backend_thread() ? MessageLoop::current() : NULL,
                             this,
-                            details_.get(),
-                            model_->PersistIDs());
+                            details_.get());
   RunTaskOnBackendThread(task);
 }
 
@@ -211,7 +208,7 @@ void BookmarkStorage::BookmarkModelDeleted() {
 }
 
 bool BookmarkStorage::SerializeData(std::string* output) {
-  BookmarkCodec codec(model_->PersistIDs());
+  BookmarkCodec codec;
   scoped_ptr<Value> value(codec.Encode(model_));
   JSONStringValueSerializer serializer(output);
   serializer.set_pretty_print(true);
diff --git a/chrome/browser/bookmarks/bookmark_storage.h b/chrome/browser/bookmarks/bookmark_storage.h
index cc18b60..0ad633e 100644
--- a/chrome/browser/bookmarks/bookmark_storage.h
+++ b/chrome/browser/bookmarks/bookmark_storage.h
@@ -44,7 +44,7 @@ class BookmarkStorage : public NotificationObserver,
     LoadDetails(BookmarkNode* bb_node,
                 BookmarkNode* other_folder_node,
                 BookmarkIndex* index,
-                int max_id)
+                int64 max_id)
         : bb_node_(bb_node),
           other_folder_node_(other_folder_node),
           index_(index),
@@ -62,8 +62,8 @@ class BookmarkStorage : public NotificationObserver,
     BookmarkIndex* index() { return index_.get(); }
 
     // Max id of the nodes.
-    void set_max_id(int max_id) { max_id_ = max_id; }
-    int max_id() const { return max_id_; }
+    void set_max_id(int64 max_id) { max_id_ = max_id; }
+    int64 max_id() const { return max_id_; }
 
     // Computed checksum.
     void set_computed_checksum(const std::string& value) {
@@ -77,13 +77,18 @@ class BookmarkStorage : public NotificationObserver,
     }
     const std::string& stored_checksum() const { return stored_checksum_; }
 
+    // Whether ids were reassigned.
+    void set_ids_reassigned(bool value) { ids_reassigned_ = value; }
+    bool ids_reassigned() const { return ids_reassigned_; }
+
    private:
     scoped_ptr<BookmarkNode> bb_node_;
     scoped_ptr<BookmarkNode> other_folder_node_;
     scoped_ptr<BookmarkIndex> index_;
-    int max_id_;
+    int64 max_id_;
     std::string computed_checksum_;
     std::string stored_checksum_;
+    bool ids_reassigned_;
 
     DISALLOW_COPY_AND_ASSIGN(LoadDetails);
   };