3 files changed, 61 insertions, 8 deletions

diff --git a/chrome/browser/bookmarks/bookmark_codec.cc b/chrome/browser/bookmarks/bookmark_codec.cc
index ebf48db..401bbd2 100644
--- a/chrome/browser/bookmarks/bookmark_codec.cc
+++ b/chrome/browser/bookmarks/bookmark_codec.cc
@@ -35,7 +35,7 @@ static const int kCurrentVersion = 1;
 
 BookmarkCodec::BookmarkCodec()
     : ids_reassigned_(false),
-      ids_missing_(false),
+      ids_valid_(true),
       maximum_id_(0) {
 }
 
@@ -66,15 +66,17 @@ bool BookmarkCodec::Decode(BookmarkNode* bb_node,
                            BookmarkNode* other_folder_node,
                            int64* max_id,
                            const Value& value) {
+  ids_.clear();
   ids_reassigned_ = false;
-  ids_missing_ = false;
+  ids_valid_ = true;
   maximum_id_ = 0;
   stored_checksum_.clear();
   InitializeChecksum();
   bool success = DecodeHelper(bb_node, other_folder_node, value);
   FinalizeChecksum();
-  // If either the checksums differ or some IDs were missing, reassign IDs.
-  if (ids_missing_ || computed_checksum() != stored_checksum())
+  // If either the checksums differ or some IDs were missing/not unique,
+  // reassign IDs.
+  if (!ids_valid_ || computed_checksum() != stored_checksum())
     ReassignIDs(bb_node, other_folder_node);
   *max_id = maximum_id_ + 1;
   return success;
@@ -189,8 +191,15 @@ bool BookmarkCodec::DecodeNode(const DictionaryValue& value,
 
   std::string id_string;
   int64 id = 0;
-  if (!value.GetString(kIdKey, &id_string) || !StringToInt64(id_string, &id))
-    ids_missing_ = true;
+  if (ids_valid_) {
+    if (!value.GetString(kIdKey, &id_string) ||
+        !StringToInt64(id_string, &id) ||
+        ids_.count(id) != 0) {
+      ids_valid_ = false;
+    } else {
+      ids_.insert(id);
+    }
+  }
 
   maximum_id_ = std::max(maximum_id_, id);
 
diff --git a/chrome/browser/bookmarks/bookmark_codec.h b/chrome/browser/bookmarks/bookmark_codec.h
index 25e9cb0..257a80f 100644
--- a/chrome/browser/bookmarks/bookmark_codec.h
+++ b/chrome/browser/bookmarks/bookmark_codec.h
@@ -9,6 +9,7 @@
 #ifndef CHROME_BROWSER_BOOKMARKS_BOOKMARK_CODEC_H_
 #define CHROME_BROWSER_BOOKMARKS_BOOKMARK_CODEC_H_
 
+#include <set>
 #include <string>
 
 #include "base/basictypes.h"
@@ -137,8 +138,13 @@ class BookmarkCodec {
   // Whether or not IDs were reassigned by the codec.
   bool ids_reassigned_;
 
-  // Whether or not IDs were missing for some bookmark nodes during decoding.
-  bool ids_missing_;
+  // Whether or not IDs are valid. This is initially true, but set to false
+  // if an id is missing or not unique.
+  bool ids_valid_;
+
+  // Contains the id of each of the nodes found in the file. Used to determine
+  // if we have duplicates.
+  std::set<int64> ids_;
 
   // MD5 context used to compute MD5 hash of all bookmark data.
   MD5Context md5_context_;
diff --git a/chrome/browser/bookmarks/bookmark_model_unittest.cc b/chrome/browser/bookmarks/bookmark_model_unittest.cc
index 047ac25..8ae04ad 100644
--- a/chrome/browser/bookmarks/bookmark_model_unittest.cc
+++ b/chrome/browser/bookmarks/bookmark_model_unittest.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <set>
+
 #include "app/tree_node_iterator.h"
 #include "app/tree_node_model.h"
 #include "base/hash_tables.h"
@@ -764,6 +766,32 @@ class BookmarkModelTestWithProfile2 : public BookmarkModelTestWithProfile {
 
     ASSERT_TRUE(bb_model_->IsBookmarked(GURL("http://www.google.com")));
   }
+
+  void VerifyUniqueIDs() {
+    std::set<int64> ids;
+    bool has_unique = true;
+    VerifyUniqueIDImpl(bb_model_->GetBookmarkBarNode(), &ids, &has_unique);
+    VerifyUniqueIDImpl(bb_model_->other_node(), &ids, &has_unique);
+    ASSERT_TRUE(has_unique);
+  }
+
+ private:
+  void VerifyUniqueIDImpl(const BookmarkNode* node,
+                          std::set<int64>* ids,
+                          bool* has_unique) {
+    if (!*has_unique)
+      return;
+    if (ids->count(node->id()) != 0) {
+      *has_unique = false;
+      return;
+    }
+    ids->insert(node->id());
+    for (int i = 0; i < node->GetChildCount(); ++i) {
+      VerifyUniqueIDImpl(node->GetChild(i), ids, has_unique);
+      if (!*has_unique)
+        return;
+    }
+  }
 };
 
 // Tests migrating bookmarks from db into file. This copies an old history db
@@ -793,6 +821,11 @@ TEST_F(BookmarkModelTestWithProfile2, MigrateFromDBToFileTest) {
   if (HasFatalFailure())
     return;
 
+  // Make sure the ids are unique.
+  VerifyUniqueIDs();
+  if (HasFatalFailure())
+    return;
+
   // Create again. This time we shouldn't load from history at all.
   profile_->CreateBookmarkModel(false);
   BlockTillBookmarkModelLoaded();
@@ -802,12 +835,17 @@ TEST_F(BookmarkModelTestWithProfile2, MigrateFromDBToFileTest) {
   if (HasFatalFailure())
     return;
 
+  VerifyUniqueIDs();
+  if (HasFatalFailure())
+    return;
+
   // Recreate the history service (with a clean db). Do this just to make sure
   // we're loading correctly from the bookmarks file.
   profile_->CreateHistoryService(true);
   profile_->CreateBookmarkModel(false);
   BlockTillBookmarkModelLoaded();
   VerifyExpectedState();
+  VerifyUniqueIDs();
 }
 
 // Simple test that removes a bookmark. This test exercises the code paths in