Revert "Revert 84829 - Initial implementation of "Synced Bookmarks" folder."

chrome/browser/bookmarks/bookmark_codec_unittest.cc

 // Copyright (c) 2011 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 "base/command_line.h"
+#include "base/file_path.h"
+#include "base/file_util.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/path_service.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.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 "chrome/common/chrome_paths.h"
+#include "chrome/common/chrome_switches.h"
+#include "content/common/json_value_serializer.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
 const char kUrl1Title[] = "url1";
 const char kUrl1Url[] = "http://www.url1.com";
 const char kUrl2Title[] = "url2";
 const char kUrl2Url[] = "http://www.url2.com";
 const char kUrl3Title[] = "url3";
 const char kUrl3Url[] = "http://www.url3.com";
@@ skipping 81 matching lines @@
     EXPECT_EQ(computed_checksum, stored_checksum);
 
     *checksum = computed_checksum;
     return value.release();
   }
 
   bool Decode(BookmarkCodec* codec, BookmarkModel* model, const Value& value) {
     int64 max_id;
     bool result = codec->Decode(AsMutable(model->GetBookmarkBarNode()),
                                 AsMutable(model->other_node()),
+                                AsMutable(model->synced_node()),
                                 &max_id, value);
     model->set_next_node_id(max_id);
     return result;
   }
 
   BookmarkModel* DecodeHelper(const Value& value,
                               const std::string& expected_stored_checksum,
                               std::string* computed_checksum,
                               bool expected_changes) {
     BookmarkCodec decoder;
@@ skipping 30 matching lines @@
     EXPECT_TRUE(assigned_ids->find(node_id) == assigned_ids->end());
     assigned_ids->insert(node_id);
     for (int i = 0; i < node->child_count(); ++i)
       CheckIDs(node->GetChild(i), assigned_ids);
   }
 
   void ExpectIDsUnique(BookmarkModel* model) {
     std::set<int64> assigned_ids;
     CheckIDs(model->GetBookmarkBarNode(), &assigned_ids);
     CheckIDs(model->other_node(), &assigned_ids);
+    CheckIDs(model->synced_node(), &assigned_ids);
   }
 };
 
 TEST_F(BookmarkCodecTest, ChecksumEncodeDecodeTest) {
   scoped_ptr<BookmarkModel> model_to_encode(CreateTestModel1());
   std::string enc_checksum;
   scoped_ptr<Value> value(EncodeHelper(model_to_encode.get(), &enc_checksum));
 
   EXPECT_TRUE(value.get() != NULL);
 
@@ skipping 106 matching lines @@
   BookmarkCodec encoder2;
   scoped_ptr<Value> model_value2(encoder2.Encode(&decoded_model));
 
   BookmarkModel decoded_model2(NULL);
   BookmarkCodec decoder2;
   ASSERT_TRUE(Decode(&decoder2, &decoded_model2, *model_value2.get()));
   BookmarkModelTestUtils::AssertModelsEqual(&decoded_model,
                                             &decoded_model2,
                                             true);
 }
+
+TEST_F(BookmarkCodecTest, CanDecodeModelWithoutSyncedBookmarks) {
+  FilePath test_data_directory;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_data_directory));
+  FilePath test_file = test_data_directory.AppendASCII(
+      "bookmarks/model_without_sync.json");
+  ASSERT_TRUE(file_util::PathExists(test_file));
+
+  JSONFileValueSerializer serializer(test_file);
+  scoped_ptr<Value> root(serializer.Deserialize(NULL, NULL));
+
+  BookmarkModel decoded_model(NULL);
+  BookmarkCodec decoder;
+  ASSERT_TRUE(Decode(&decoder, &decoded_model, *root.get()));
+  ExpectIDsUnique(&decoded_model);

[Yaron, 2011/05/12 20:17:46]

My test is failing on this assertion. I thought that ids should be unique
throughout the tree and this should be a valid check. The problem is that the
synced bookmarks folder is given id 3, but that already exists in the serialized
tree. This may not be a problem in practice (you wouldn't make edits to an
immutable folder, and lookups for the permanent folders are done with special
accessors) but seems bad to leave around. Should I handle this by reassigning a
decoded id of 3 with a new id or only construct the synced node after decoding?

[sky, 2011/05/12 21:14:33]

Good thing I had you write this test. I've added a comment where the bug is.

+
+  const BookmarkNode* bbn = decoded_model.GetBookmarkBarNode();
+  ASSERT_EQ(1, bbn->child_count());
+
+  const BookmarkNode* child = bbn->GetChild(0);
+  EXPECT_EQ(BookmarkNode::FOLDER, child->type());
+  EXPECT_EQ(ASCIIToUTF16("Folder A"), child->GetTitle());
+  ASSERT_EQ(1, child->child_count());
+
+  child = child->GetChild(0);
+  EXPECT_EQ(BookmarkNode::URL, child->type());
+  EXPECT_EQ(ASCIIToUTF16("Bookmark Manager"), child->GetTitle());
+
+  const BookmarkNode* other = decoded_model.other_node();
+  ASSERT_EQ(1, other->child_count());
+
+  child = other->GetChild(0);
+  EXPECT_EQ(BookmarkNode::FOLDER, child->type());
+  EXPECT_EQ(ASCIIToUTF16("Folder B"), child->GetTitle());
+  ASSERT_EQ(1, child->child_count());
+
+  child = child->GetChild(0);
+  EXPECT_EQ(BookmarkNode::URL, child->type());
+  EXPECT_EQ(ASCIIToUTF16("Get started with Google Chrome"), child->GetTitle());
+
+  ASSERT_TRUE(decoded_model.synced_node() != NULL);
+}