Revert "Always persist bookmark IDs."

This reverts commit r20532 because valgrind was complaining
about uninitialized memory:
http://build.chromium.org/buildbot/waterfall/builders/Chromium%20Linux%20(valgrind)/builds/697/steps/valgrind%20test:%20unit/logs/stdio

TBR=munjal

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


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

10 files changed, 146 insertions, 104 deletions

diff --git a/chrome/browser/bookmarks/bookmark_codec.cc b/chrome/browser/bookmarks/bookmark_codec.cc
index 92f6dcf..4e4f5ed 100644
--- a/chrome/browser/bookmarks/bookmark_codec.cc
+++ b/chrome/browser/bookmarks/bookmark_codec.cc
@@ -17,7 +17,7 @@ UniqueIDGenerator::UniqueIDGenerator() {
   Reset();
 }
 
-int64 UniqueIDGenerator::GetUniqueID(int64 id) {
+int UniqueIDGenerator::GetUniqueID(int id) {
   // If the given ID is already assigned, generate new ID.
   if (IsIdAssigned(id))
     id = current_max_ + 1;
@@ -31,7 +31,7 @@ int64 UniqueIDGenerator::GetUniqueID(int64 id) {
   return id;
 }
 
-bool UniqueIDGenerator::IsIdAssigned(int64 id) const {
+bool UniqueIDGenerator::IsIdAssigned(int 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(int64 id) const {
     return id <= current_max_;
 }
 
-void UniqueIDGenerator::RecordId(int64 id) {
+void UniqueIDGenerator::RecordId(int 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(int64 id) {
     ++current_max_;
     return;
   }
-  assigned_ids_.reset(new std::set<int64>);
-  for (int64 i = 0; i <= current_max_; ++i)
+  assigned_ids_.reset(new std::set<int>);
+  for (int i = 0; i <= current_max_; ++i)
     assigned_ids_->insert(i);
   assigned_ids_->insert(id);
 }
@@ -85,7 +85,11 @@ const wchar_t* BookmarkCodec::kTypeFolder = L"folder";
 static const int kCurrentVersion = 1;
 
 BookmarkCodec::BookmarkCodec()
-    : ids_reassigned_(false) {
+    : persist_ids_(false) {
+}
+
+BookmarkCodec::BookmarkCodec(bool persist_ids)
+    : persist_ids_(persist_ids) {
 }
 
 Value* BookmarkCodec::Encode(BookmarkModel* model) {
@@ -94,7 +98,6 @@ 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));
@@ -113,16 +116,12 @@ Value* BookmarkCodec::Encode(const BookmarkNode* bookmark_bar_node,
 
 bool BookmarkCodec::Decode(BookmarkNode* bb_node,
                            BookmarkNode* other_folder_node,
-                           int64* max_id,
+                           int* 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, value);
+  bool success = DecodeHelper(bb_node, other_folder_node, max_id, value);
   FinalizeChecksum();
   *max_id = id_generator_.current_max() + 1;
   return success;
@@ -131,8 +130,10 @@ bool BookmarkCodec::Decode(BookmarkNode* bb_node,
 Value* BookmarkCodec::EncodeNode(const BookmarkNode* node) {
   DictionaryValue* value = new DictionaryValue();
   std::string id;
-  id = Int64ToString(node->id());
-  value->SetString(kIdKey, id);
+  if (persist_ids_) {
+    id = IntToString(node->id());
+    value->SetString(kIdKey, id);
+  }
   const std::wstring& title = node->GetTitle();
   value->SetString(kNameKey, title);
   value->SetString(kDateAddedKey,
@@ -159,6 +160,7 @@ 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.
@@ -229,14 +231,13 @@ bool BookmarkCodec::DecodeNode(const DictionaryValue& value,
                                BookmarkNode* parent,
                                BookmarkNode* node) {
   std::string id_string;
-  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;
+  int id = 0;
+  if (persist_ids_) {
+    if (value.GetString(kIdKey, &id_string))
+      if (!StringToInt(id_string, &id))
+        return false;
+  }
+  id = id_generator_.GetUniqueID(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 2c13cef..e6292a8 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 64-bit IDs.
+// Utility class to help assign unique integer IDs.
 class UniqueIDGenerator {
  public:
   UniqueIDGenerator();
@@ -31,26 +31,25 @@ 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.
-  int64 GetUniqueID(int64 id);
+  int GetUniqueID(int id);
 
   // Resets the ID generator to initial state.
   void Reset();
 
   // Returns the current maximum.
-  int64 current_max() const { return current_max_; }
+  int current_max() const { return current_max_; }
 
  private:
   // Checks if the given ID is already assigned.
-  bool IsIdAssigned(int64 id) const;
+  bool IsIdAssigned(int id) const;
 
   // Records the given ID as assigned.
-  void RecordId(int64 id);
+  void RecordId(int id);
 
   // Maximum value we have seen so far.
-  int64 current_max_;
-
+  int current_max_;
   // All IDs assigned so far.
-  scoped_ptr<std::set<int64> > assigned_ids_;
+  scoped_ptr<std::set<int> > assigned_ids_;
 
   DISALLOW_COPY_AND_ASSIGN(UniqueIDGenerator);
 };
@@ -60,11 +59,14 @@ class UniqueIDGenerator {
 
 class BookmarkCodec {
  public:
-  // 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.
+  // 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.
   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
@@ -86,7 +88,7 @@ class BookmarkCodec {
   // |max_node_id| is set to the max id of the nodes.
   bool Decode(BookmarkNode* bb_node,
               BookmarkNode* other_folder_node,
-              int64* max_node_id,
+              int* max_node_id,
               const Value& value);
 
   // Returns the checksum computed during last encoding/decoding call.
@@ -99,10 +101,6 @@ 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;
@@ -129,6 +127,7 @@ 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.
@@ -161,12 +160,11 @@ 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 ed3996e..d6a3424 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) {
-    int64 max_id;
+    int 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;
+  BookmarkCodec encoder(true);
   scoped_ptr<Value> model_value(encoder.Encode(model_to_encode.get()));
 
   BookmarkModel decoded_model(NULL);
-  BookmarkCodec decoder;
+  BookmarkCodec decoder(true);
   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;
+  BookmarkCodec encoder2(true);
   scoped_ptr<Value> model_value2(encoder2.Encode(&decoded_model));
 
   BookmarkModel decoded_model2(NULL);
-  BookmarkCodec decoder2;
+  BookmarkCodec decoder2(true);
   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 (int64 i = 1; i <= 5; ++i) {
+    for (int i = 1; i <= 5; ++i) {
       EXPECT_EQ(i, gen->GetUniqueID(i));
     }
 
     // All numbers from 1 to 5 should produce numbers 6 to 10.
-    for (int64 i = 1; i <= 5; ++i) {
+    for (int 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 (int64 i = 1; i <= 5; ++i) {
+    for (int 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 (int64 i = 1; i <= 10; ++i) {
+  for (int i = 1; i <= 10; ++i) {
     EXPECT_EQ(i, gen.GetUniqueID(i));
   }
 }
 
 TEST_F(UniqueIDGeneratorTest, UniqueSortedNumbersTest) {
   UniqueIDGenerator gen;
-  for (int64 i = 1; i <= 10; i += 2) {
+  for (int 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 (int64 i = 0; i < ARRAYSIZE(numbers); ++i) {
+  for (int 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 (int64 i = 0; i < ARRAYSIZE(numbers); ++i) {
+  for (int i = 0; i < ARRAYSIZE(numbers); ++i) {
     EXPECT_EQ(numbers[i], gen.GetUniqueID(numbers[i]));
   }
 }
 
 TEST_F(UniqueIDGeneratorTest, AllDuplicatesTest) {
   UniqueIDGenerator gen;
-  for (int64 i = 1; i <= 10; ++i) {
+  for (int 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 (int64 i = 0; i < 5; ++i) {
+  for (int 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 bf3ae05..732d119 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->WriteInt64(id_);
+  pickle->WriteInt(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->ReadInt64(iterator, &id_)) {
+      !pickle->ReadInt(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 9af3c20..88cb6d4 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.
-    int64 id_;
+    int id_;
   };
 
   BookmarkDragData() { }
diff --git a/chrome/browser/bookmarks/bookmark_model.cc b/chrome/browser/bookmarks/bookmark_model.cc
index 9bcc8c3..5d87da1 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(int64 id, const GURL& url)
+BookmarkNode::BookmarkNode(int id, const GURL& url)
     : url_(url){
   Initialize(id);
 }
 
-void BookmarkNode::Initialize(int64 id) {
+void BookmarkNode::Initialize(int id) {
   id_ = id;
   loaded_favicon_ = false;
   favicon_load_handle_ = 0;
@@ -77,6 +77,9 @@ 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*,
@@ -107,6 +110,7 @@ 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),
@@ -118,6 +122,8 @@ BookmarkModel::BookmarkModel(Profile* profile)
     // Profile is null during testing.
     DoneLoading(CreateLoadDetails());
   }
+  RegisterPreferences();
+  LoadPreferences();
 }
 
 BookmarkModel::~BookmarkModel() {
@@ -276,7 +282,7 @@ bool BookmarkModel::IsBookmarked(const GURL& url) {
   return IsBookmarkedNoLock(url);
 }
 
-const BookmarkNode* BookmarkModel::GetNodeByID(int64 id) {
+const BookmarkNode* BookmarkModel::GetNodeByID(int id) {
   // TODO(sky): TreeNode needs a method that visits all nodes using a predicate.
   return GetNodeByID(&root_, id);
 }
@@ -402,6 +408,19 @@ 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) !=
@@ -458,15 +477,6 @@ 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();
 
@@ -577,7 +587,7 @@ void BookmarkModel::BlockTillLoaded() {
 }
 
 const BookmarkNode* BookmarkModel::GetNodeByID(const BookmarkNode* node,
-                                               int64 id) {
+                                               int id) {
   if (node->id() == id)
     return node;
 
@@ -715,12 +725,18 @@ void BookmarkModel::PopulateNodesByURL(BookmarkNode* node) {
     PopulateNodesByURL(node->GetChild(i));
 }
 
-int64 BookmarkModel::generate_next_node_id() {
+int 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() {
@@ -729,3 +745,18 @@ 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 c8ba083..9726624 100644
--- a/chrome/browser/bookmarks/bookmark_model.h
+++ b/chrome/browser/bookmarks/bookmark_model.h
@@ -53,18 +53,19 @@ 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(int64 id, const GURL& url);
+  BookmarkNode(int id, const GURL& url);
   virtual ~BookmarkNode() {}
 
   // Returns the URL.
   const GURL& GetURL() const { return url_; }
 
   // Returns a unique id for this node.
-  // For bookmark nodes that are managed by the bookmark model, the IDs are
-  // persisted across sessions.
-  int64 id() const { return id_; }
+  //
+  // NOTE: this id is only unique for the session and NOT unique across
+  // sessions. Don't persist it!
+  int id() const { return id_; }
   // Sets the id to the given value.
-  void set_id(int64 id) { id_ = id; }
+  void set_id(int id) { id_ = id; }
 
   // Returns the type of this node.
   BookmarkNode::Type GetType() const { return type_; }
@@ -125,10 +126,10 @@ class BookmarkNode : public TreeNode<BookmarkNode> {
 
  private:
   // helper to initialize various fields during construction.
-  void Initialize(int64 id);
+  void Initialize(int id);
 
   // Unique identifier for this node.
-  int64 id_;
+  int id_;
 
   // Whether the favicon has been loaded.
   bool loaded_favicon_;
@@ -291,7 +292,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(int64 id);
+  const BookmarkNode* GetNodeByID(int id);
 
   // Adds a new group node at the specified position.
   const BookmarkNode* AddGroup(const BookmarkNode* parent,
@@ -354,6 +355,10 @@ 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_; }
 
@@ -399,7 +404,7 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
                         bool was_bookmarked);
 
   // Implementation of GetNodeByID.
-  const BookmarkNode* GetNodeByID(const BookmarkNode* node, int64 id);
+  const BookmarkNode* GetNodeByID(const BookmarkNode* node, int id);
 
   // Returns true if the parent and index are valid.
   bool IsValidIndex(const BookmarkNode* parent, int index, bool allow_end);
@@ -439,12 +444,13 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
                        const NotificationDetails& details);
 
   // Generates and returns the next node ID.
-  int64 generate_next_node_id();
+  int 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 assigns node IDs.
-  void set_next_node_id(int64 id) { next_node_id_ = id; }
+  // 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; }
 
   // Records that the bookmarks file was changed externally.
   void SetFileChanged();
@@ -453,6 +459,11 @@ 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_;
@@ -460,6 +471,9 @@ 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_;
@@ -472,7 +486,7 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
   BookmarkNode* other_node_;
 
   // The maximum ID assigned to the bookmark nodes in the model.
-  int64 next_node_id_;
+  int 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 627a058..05d0843 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<int64> ids;
+    base::hash_set<int> 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 d7dbe19..a530158 100644
--- a/chrome/browser/bookmarks/bookmark_storage.cc
+++ b/chrome/browser/bookmarks/bookmark_storage.cc
@@ -68,11 +68,13 @@ class BookmarkStorage::LoadTask : public Task {
   LoadTask(const FilePath& path,
            MessageLoop* loop,
            BookmarkStorage* storage,
-           LoadDetails* details)
+           LoadDetails* details,
+           bool persist_ids)
       : path_(path),
         loop_(loop),
         storage_(storage),
-        details_(details) {
+        details_(details),
+        persist_ids_(persist_ids) {
   }
 
   virtual void Run() {
@@ -84,15 +86,14 @@ class BookmarkStorage::LoadTask : public Task {
       if (root.get()) {
         // Building the index cane take a while, so we do it on the background
         // thread.
-        int64 max_node_id = 0;
-        BookmarkCodec codec;
+        int max_node_id = 0;
+        BookmarkCodec codec(persist_ids_);
         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);
 
@@ -128,6 +129,7 @@ class BookmarkStorage::LoadTask : public Task {
   MessageLoop* loop_;
   scoped_refptr<BookmarkStorage> storage_;
   LoadDetails* details_;
+  bool persist_ids_;
 
   DISALLOW_COPY_AND_ASSIGN(LoadTask);
 };
@@ -162,7 +164,8 @@ void BookmarkStorage::DoLoadBookmarks(const FilePath& path) {
   Task* task = new LoadTask(path,
                             backend_thread() ? MessageLoop::current() : NULL,
                             this,
-                            details_.get());
+                            details_.get(),
+                            model_->PersistIDs());
   RunTaskOnBackendThread(task);
 }
 
@@ -208,7 +211,7 @@ void BookmarkStorage::BookmarkModelDeleted() {
 }
 
 bool BookmarkStorage::SerializeData(std::string* output) {
-  BookmarkCodec codec;
+  BookmarkCodec codec(model_->PersistIDs());
   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 0ad633e..cc18b60 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,
-                int64 max_id)
+                int 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(int64 max_id) { max_id_ = max_id; }
-    int64 max_id() const { return max_id_; }
+    void set_max_id(int max_id) { max_id_ = max_id; }
+    int max_id() const { return max_id_; }
 
     // Computed checksum.
     void set_computed_checksum(const std::string& value) {
@@ -77,18 +77,13 @@ 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_;
-    int64 max_id_;
+    int max_id_;
     std::string computed_checksum_;
     std::string stored_checksum_;
-    bool ids_reassigned_;
 
     DISALLOW_COPY_AND_ASSIGN(LoadDetails);
   };