Implement ID persistence for bookmarks:

- Bookmark codec now takes in a ctor argument persist_ids
- If it's true, it will serialize IDs of bookmarks when encoding, and
  deserialize already serialized IDs (if present) when decoding.
- During decoding, unique-ify the IDs if they are not unique.
- Add unit tests for all new code.


Coming up in a separate changelist:
- Move ID generation logic to bookmark model, and make it non-static.

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

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

1 files changed, 161 insertions, 11 deletions

diff --git a/chrome/browser/bookmarks/bookmark_codec_unittest.cc b/chrome/browser/bookmarks/bookmark_codec_unittest.cc
index 59e0af5..1b7a4d3 100644
--- a/chrome/browser/bookmarks/bookmark_codec_unittest.cc
+++ b/chrome/browser/bookmarks/bookmark_codec_unittest.cc
@@ -5,26 +5,49 @@
 #include "base/scoped_ptr.h"
 #include "base/string_util.h"
 #include "base/values.h"
-#include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/bookmarks/bookmark_codec.h"
+#include "chrome/browser/bookmarks/bookmark_model.h"
+#include "chrome/browser/bookmarks/bookmark_model_test_utils.h"
 #include "chrome/browser/bookmarks/bookmark_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace {
+
+const wchar_t kUrl1Title[] = L"url1";
+const wchar_t kUrl1Url[] = L"http://www.url1.com";
+const wchar_t kUrl2Title[] = L"url2";
+const wchar_t kUrl2Url[] = L"http://www.url2.com";
+const wchar_t kUrl3Title[] = L"url3";
+const wchar_t kUrl3Url[] = L"http://www.url3.com";
+const wchar_t kUrl4Title[] = L"url4";
+const wchar_t kUrl4Url[] = L"http://www.url4.com";
+const wchar_t kGroup1Title[] = L"group1";
+const wchar_t kGroup2Title[] = L"group2";
+
+}
+
 class BookmarkCodecTest : public testing::Test {
  protected:
   // Helpers to create bookmark models with different data.
-  // Callers own the returned instances.
-  BookmarkModel* CreateModelWithOneUrl() {
+  BookmarkModel* CreateTestModel1() {
     scoped_ptr<BookmarkModel> model(new BookmarkModel(NULL));
     BookmarkNode* bookmark_bar = model->GetBookmarkBarNode();
-    model->AddURL(bookmark_bar, 0, L"foo", GURL(L"http://www.foo.com"));
+    model->AddURL(bookmark_bar, 0, kUrl1Title, GURL(kUrl1Url));
     return model.release();
   }
-  BookmarkModel* CreateModelWithTwoUrls() {
+  BookmarkModel* CreateTestModel2() {
     scoped_ptr<BookmarkModel> model(new BookmarkModel(NULL));
     BookmarkNode* bookmark_bar = model->GetBookmarkBarNode();
-    model->AddURL(bookmark_bar, 0, L"foo", GURL(L"http://www.foo.com"));
-    model->AddURL(bookmark_bar, 1, L"bar", GURL(L"http://www.bar.com"));
+    model->AddURL(bookmark_bar, 0, kUrl1Title, GURL(kUrl1Url));
+    model->AddURL(bookmark_bar, 1, kUrl2Title, GURL(kUrl2Url));
+    return model.release();
+  }
+  BookmarkModel* CreateTestModel3() {
+    scoped_ptr<BookmarkModel> model(new BookmarkModel(NULL));
+    BookmarkNode* bookmark_bar = model->GetBookmarkBarNode();
+    model->AddURL(bookmark_bar, 0, kUrl1Title, GURL(kUrl1Url));
+    BookmarkNode* group1 = model->AddGroup(bookmark_bar, 1, kGroup1Title);
+    model->AddURL(group1, 0, kUrl2Title, GURL(kUrl2Url));
     return model.release();
   }
 
@@ -111,7 +134,7 @@ class BookmarkCodecTest : public testing::Test {
 };
 
 TEST_F(BookmarkCodecTest, ChecksumEncodeDecodeTest) {
-  scoped_ptr<BookmarkModel> model_to_encode(CreateModelWithOneUrl());
+  scoped_ptr<BookmarkModel> model_to_encode(CreateTestModel1());
   std::string enc_checksum;
   scoped_ptr<Value> value(EncodeHelper(model_to_encode.get(), &enc_checksum));
 
@@ -125,12 +148,12 @@ TEST_F(BookmarkCodecTest, ChecksumEncodeDecodeTest) {
 TEST_F(BookmarkCodecTest, ChecksumEncodeIdenticalModelsTest) {
   // Encode two identical models and make sure the check-sums are same as long
   // as the data is the same.
-  scoped_ptr<BookmarkModel> model1(CreateModelWithOneUrl());
+  scoped_ptr<BookmarkModel> model1(CreateTestModel1());
   std::string enc_checksum1;
   scoped_ptr<Value> value1(EncodeHelper(model1.get(), &enc_checksum1));
   EXPECT_TRUE(value1.get() != NULL);
 
-  scoped_ptr<BookmarkModel> model2(CreateModelWithOneUrl());
+  scoped_ptr<BookmarkModel> model2(CreateTestModel1());
   std::string enc_checksum2;
   scoped_ptr<Value> value2(EncodeHelper(model2.get(), &enc_checksum2));
   EXPECT_TRUE(value2.get() != NULL);
@@ -139,7 +162,7 @@ TEST_F(BookmarkCodecTest, ChecksumEncodeIdenticalModelsTest) {
 }
 
 TEST_F(BookmarkCodecTest, ChecksumManualEditTest) {
-  scoped_ptr<BookmarkModel> model_to_encode(CreateModelWithOneUrl());
+  scoped_ptr<BookmarkModel> model_to_encode(CreateTestModel1());
   std::string enc_checksum;
   scoped_ptr<Value> value(EncodeHelper(model_to_encode.get(), &enc_checksum));
 
@@ -161,3 +184,130 @@ TEST_F(BookmarkCodecTest, ChecksumManualEditTest) {
   scoped_ptr<BookmarkModel> decoded_model2(DecodeHelper(
       *value.get(), enc_checksum, &dec_checksum, false));
 }
+
+TEST_F(BookmarkCodecTest, PersistIDsTest) {
+  scoped_ptr<BookmarkModel> model_to_encode(CreateTestModel3());
+  BookmarkCodec encoder(true);
+  scoped_ptr<Value> model_value(encoder.Encode(model_to_encode.get()));
+
+  // Set the next id to 1 to simulate fresh Chrome start.
+  BookmarkNode::SetNextId(1);
+
+  BookmarkModel decoded_model(NULL);
+  BookmarkCodec decoder(true);
+  ASSERT_TRUE(decoder.Decode(&decoded_model, *model_value.get()));
+  BookmarkModelTestUtils::AssertModelsEqual(model_to_encode.get(),
+                                            &decoded_model,
+                                            true);
+
+  // Add a couple of more items to the decoded bookmark model and make sure
+  // ID persistence is working properly.
+  BookmarkNode* bookmark_bar = decoded_model.GetBookmarkBarNode();
+  decoded_model.AddURL(
+      bookmark_bar, bookmark_bar->GetChildCount(), kUrl3Title, GURL(kUrl3Url));
+  BookmarkNode* group2_node = decoded_model.AddGroup(
+      bookmark_bar, bookmark_bar->GetChildCount(), kGroup2Title);
+  decoded_model.AddURL(group2_node, 0, kUrl4Title, GURL(kUrl4Url));
+
+  BookmarkCodec encoder2(true);
+  scoped_ptr<Value> model_value2(encoder2.Encode(&decoded_model));
+
+  // Set the next id to 1 to simulate fresh Chrome start.
+  BookmarkNode::SetNextId(1);
+
+  BookmarkModel decoded_model2(NULL);
+  BookmarkCodec decoder2(true);
+  ASSERT_TRUE(decoder2.Decode(&decoded_model2, *model_value2.get()));
+  BookmarkModelTestUtils::AssertModelsEqual(&decoded_model,
+                                            &decoded_model2,
+                                            true);
+}
+
+class UniqueIDGeneratorTest : public testing::Test {
+ protected:
+  void TestMixed(UniqueIDGenerator* gen) {
+    // Few unique numbers.
+    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 (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 (int i = 1; i <= 5; ++i) {
+      EXPECT_EQ(10 + i, gen->GetUniqueID(9 + i));
+    }
+
+    // Any numbers between 1 and 15 should produce a new numbers in sequence.
+    EXPECT_EQ(16, gen->GetUniqueID(10));
+    EXPECT_EQ(17, gen->GetUniqueID(2));
+    EXPECT_EQ(18, gen->GetUniqueID(14));
+    EXPECT_EQ(19, gen->GetUniqueID(7));
+    EXPECT_EQ(20, gen->GetUniqueID(4));
+
+    // Numbers not yet generated should work.
+    EXPECT_EQ(100, gen->GetUniqueID(100));
+    EXPECT_EQ(21, gen->GetUniqueID(21));
+    EXPECT_EQ(200, gen->GetUniqueID(200));
+
+    // Now any existing number should produce numbers starting from 201.
+    EXPECT_EQ(201, gen->GetUniqueID(1));
+    EXPECT_EQ(202, gen->GetUniqueID(20));
+    EXPECT_EQ(203, gen->GetUniqueID(21));
+    EXPECT_EQ(204, gen->GetUniqueID(100));
+    EXPECT_EQ(205, gen->GetUniqueID(200));
+  }
+};
+
+TEST_F(UniqueIDGeneratorTest, SerialNumbersTest) {
+  UniqueIDGenerator gen;
+  for (int i = 1; i <= 10; ++i) {
+    EXPECT_EQ(i, gen.GetUniqueID(i));
+  }
+}
+
+TEST_F(UniqueIDGeneratorTest, UniquSortedNumbersTest) {
+  UniqueIDGenerator gen;
+  for (int i = 1; i <= 10; i += 2) {
+    EXPECT_EQ(i, gen.GetUniqueID(i));
+  }
+}
+
+TEST_F(UniqueIDGeneratorTest, UniquUnsortedConsecutiveNumbersTest) {
+  UniqueIDGenerator gen;
+  int numbers[] = {2, 10, 6, 3, 8, 5, 1, 7, 4, 9};
+  for (int i = 0; i < ARRAYSIZE(numbers); ++i) {
+    EXPECT_EQ(numbers[i], gen.GetUniqueID(numbers[i]));
+  }
+}
+
+TEST_F(UniqueIDGeneratorTest, UniquUnsortedNumbersTest) {
+  UniqueIDGenerator gen;
+  int numbers[] = {20, 100, 60, 30, 80, 50, 10, 70, 40, 90};
+  for (int 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) {
+    EXPECT_EQ(i, gen.GetUniqueID(1));
+  }
+}
+
+TEST_F(UniqueIDGeneratorTest, MixedTest) {
+  UniqueIDGenerator gen;
+  TestMixed(&gen);
+}
+
+TEST_F(UniqueIDGeneratorTest, ResetTest) {
+  UniqueIDGenerator gen;
+  for (int i = 0; i < 5; ++i) {
+    TestMixed(&gen);
+    gen.Reset();
+  }
+}