diff options
31 files changed, 646 insertions, 162 deletions
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index 564898d..d2487f6 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -5911,6 +5911,9 @@ Keep your key file in a safe place. You will need it to create new versions of y <message name="IDS_BOOMARK_BAR_FOLDER_NAME" desc="Name shown in the tree for the bookmarks bar folder"> Bookmarks bar </message> + <message name="IDS_BOOMARK_BAR_SYNCED_FOLDER_NAME" desc="Name shown in the tree for the synced bookmarks folder"> + Synced bookmarks + </message> <message name="IDS_BOOMARK_BAR_OTHER_FOLDER_NAME" desc="Name shown in the tree for the other bookmarks folder"> Other bookmarks </message> @@ -5919,6 +5922,9 @@ Keep your key file in a safe place. You will need it to create new versions of y <message name="IDS_BOOMARK_BAR_FOLDER_NAME" desc="In Title Case: Name shown in the tree for the bookmarks bar folder"> Bookmarks Bar </message> + <message name="IDS_BOOMARK_BAR_SYNCED_FOLDER_NAME" desc="In Title Case: Name shown in the tree for the synced bookmarks folder"> + Synced Bookmarks + </message> <message name="IDS_BOOMARK_BAR_OTHER_FOLDER_NAME" desc="In Title Case: Name shown in the tree for the other bookmarks folder"> Other Bookmarks </message> diff --git a/chrome/browser/bookmarks/bookmark_codec.cc b/chrome/browser/bookmarks/bookmark_codec.cc index dd145b3..56b0c25 100644 --- a/chrome/browser/bookmarks/bookmark_codec.cc +++ b/chrome/browser/bookmarks/bookmark_codec.cc @@ -19,6 +19,7 @@ using base::Time; const char* BookmarkCodec::kRootsKey = "roots"; const char* BookmarkCodec::kRootFolderNameKey = "bookmark_bar"; const char* BookmarkCodec::kOtherBookmarkFolderNameKey = "other"; +const char* BookmarkCodec::kSyncedBookmarkFolderNameKey = "synced"; const char* BookmarkCodec::kVersionKey = "version"; const char* BookmarkCodec::kChecksumKey = "checksum"; const char* BookmarkCodec::kIdKey = "id"; @@ -43,16 +44,19 @@ BookmarkCodec::BookmarkCodec() BookmarkCodec::~BookmarkCodec() {} Value* BookmarkCodec::Encode(BookmarkModel* model) { - return Encode(model->GetBookmarkBarNode(), model->other_node()); + return Encode(model->GetBookmarkBarNode(), model->other_node(), + model->synced_node()); } Value* BookmarkCodec::Encode(const BookmarkNode* bookmark_bar_node, - const BookmarkNode* other_folder_node) { + const BookmarkNode* other_folder_node, + const BookmarkNode* synced_folder_node) { ids_reassigned_ = false; InitializeChecksum(); DictionaryValue* roots = new DictionaryValue(); roots->Set(kRootFolderNameKey, EncodeNode(bookmark_bar_node)); roots->Set(kOtherBookmarkFolderNameKey, EncodeNode(other_folder_node)); + roots->Set(kSyncedBookmarkFolderNameKey, EncodeNode(synced_folder_node)); DictionaryValue* main = new DictionaryValue(); main->SetInteger(kVersionKey, kCurrentVersion); @@ -67,6 +71,7 @@ Value* BookmarkCodec::Encode(const BookmarkNode* bookmark_bar_node, bool BookmarkCodec::Decode(BookmarkNode* bb_node, BookmarkNode* other_folder_node, + BookmarkNode* synced_folder_node, int64* max_id, const Value& value) { ids_.clear(); @@ -75,12 +80,13 @@ bool BookmarkCodec::Decode(BookmarkNode* bb_node, maximum_id_ = 0; stored_checksum_.clear(); InitializeChecksum(); - bool success = DecodeHelper(bb_node, other_folder_node, value); + bool success = DecodeHelper(bb_node, other_folder_node, synced_folder_node, + value); FinalizeChecksum(); // 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); + ReassignIDs(bb_node, other_folder_node, synced_folder_node); *max_id = maximum_id_ + 1; return success; } @@ -115,6 +121,7 @@ Value* BookmarkCodec::EncodeNode(const BookmarkNode* node) { bool BookmarkCodec::DecodeHelper(BookmarkNode* bb_node, BookmarkNode* other_folder_node, + BookmarkNode* synced_folder_node, const Value& value) { if (value.GetType() != Value::TYPE_DICTIONARY) return false; // Unexpected type. @@ -147,21 +154,45 @@ bool BookmarkCodec::DecodeHelper(BookmarkNode* bb_node, if (!roots_d_value->Get(kRootFolderNameKey, &root_folder_value) || root_folder_value->GetType() != Value::TYPE_DICTIONARY || !roots_d_value->Get(kOtherBookmarkFolderNameKey, &other_folder_value) || - other_folder_value->GetType() != Value::TYPE_DICTIONARY) - return false; // Invalid type for root folder and/or other folder. - + other_folder_value->GetType() != Value::TYPE_DICTIONARY) { + return false; // Invalid type for root folder and/or other + // folder. + } DecodeNode(*static_cast<DictionaryValue*>(root_folder_value), NULL, bb_node); DecodeNode(*static_cast<DictionaryValue*>(other_folder_value), NULL, other_folder_node); + + // Fail silently if we can't deserialize synced bookmarks. We can't require + // them to exist in order to be backwards-compatible with older versions of + // chrome. + Value* synced_folder_value; + if (roots_d_value->Get(kSyncedBookmarkFolderNameKey, &synced_folder_value) && + synced_folder_value->GetType() == Value::TYPE_DICTIONARY) { + DecodeNode(*static_cast<DictionaryValue*>(synced_folder_value), NULL, + synced_folder_node); + } else { + // If we didn't find the synced folder, we're almost guaranteed to have a + // duplicate id when we add the synced folder. Consequently, if we don't + // intend to reassign ids in the future (ids_valid_ is still true), then at + // least reassign the synced bookmarks to avoid it colliding with anything + // else. + if (ids_valid_) { + ReassignIDsHelper(synced_folder_node); + } + } + // Need to reset the type as decoding resets the type to FOLDER. Similarly // we need to reset the title as the title is persisted and restored from // the file. bb_node->set_type(BookmarkNode::BOOKMARK_BAR); other_folder_node->set_type(BookmarkNode::OTHER_NODE); + synced_folder_node->set_type(BookmarkNode::SYNCED); bb_node->set_title(l10n_util::GetStringUTF16(IDS_BOOMARK_BAR_FOLDER_NAME)); other_folder_node->set_title( l10n_util::GetStringUTF16(IDS_BOOMARK_BAR_OTHER_FOLDER_NAME)); + synced_folder_node->set_title( + l10n_util::GetStringUTF16(IDS_BOOMARK_BAR_SYNCED_FOLDER_NAME)); return true; } @@ -289,10 +320,12 @@ bool BookmarkCodec::DecodeNode(const DictionaryValue& value, } void BookmarkCodec::ReassignIDs(BookmarkNode* bb_node, - BookmarkNode* other_node) { + BookmarkNode* other_node, + BookmarkNode* synced_node) { maximum_id_ = 0; ReassignIDsHelper(bb_node); ReassignIDsHelper(other_node); + ReassignIDsHelper(synced_node); ids_reassigned_ = true; } diff --git a/chrome/browser/bookmarks/bookmark_codec.h b/chrome/browser/bookmarks/bookmark_codec.h index fe57d1f..2d29df8 100644 --- a/chrome/browser/bookmarks/bookmark_codec.h +++ b/chrome/browser/bookmarks/bookmark_codec.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// 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. @@ -46,7 +46,8 @@ class BookmarkCodec { // This method is public for use by StarredURLDatabase in migrating the // bookmarks out of the database. Value* Encode(const BookmarkNode* bookmark_bar_node, - const BookmarkNode* other_folder_node); + const BookmarkNode* other_folder_node, + const BookmarkNode* synced_folder_node); // Decodes the previously encoded value to the specified nodes as well as // setting |max_node_id| to the greatest node id. Returns true on success, @@ -55,6 +56,7 @@ class BookmarkCodec { // |max_node_id| is set to the max id of the nodes. bool Decode(BookmarkNode* bb_node, BookmarkNode* other_folder_node, + BookmarkNode* synced_folder_node, int64* max_node_id, const Value& value); @@ -76,6 +78,7 @@ class BookmarkCodec { static const char* kRootsKey; static const char* kRootFolderNameKey; static const char* kOtherBookmarkFolderNameKey; + static const char* kSyncedBookmarkFolderNameKey; static const char* kVersionKey; static const char* kChecksumKey; static const char* kIdKey; @@ -98,6 +101,7 @@ class BookmarkCodec { // Helper to perform decoding. bool DecodeHelper(BookmarkNode* bb_node, BookmarkNode* other_folder_node, + BookmarkNode* synced_folder_node, const Value& value); // Decodes the children of the specified node. Returns true on success. @@ -105,7 +109,9 @@ class BookmarkCodec { BookmarkNode* parent); // Reassigns bookmark IDs for all nodes. - void ReassignIDs(BookmarkNode* bb_node, BookmarkNode* other_node); + void ReassignIDs(BookmarkNode* bb_node, + BookmarkNode* other_node, + BookmarkNode* synced_node); // Helper to recursively reassign IDs. void ReassignIDsHelper(BookmarkNode* node); diff --git a/chrome/browser/bookmarks/bookmark_codec_unittest.cc b/chrome/browser/bookmarks/bookmark_codec_unittest.cc index 09b9bd4..7358188 100644 --- a/chrome/browser/bookmarks/bookmark_codec_unittest.cc +++ b/chrome/browser/bookmarks/bookmark_codec_unittest.cc @@ -2,7 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#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" @@ -10,6 +13,8 @@ #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 "content/common/json_value_serializer.h" #include "testing/gtest/include/gtest/gtest.h" namespace { @@ -111,6 +116,7 @@ class BookmarkCodecTest : public testing::Test { 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; @@ -161,6 +167,7 @@ class BookmarkCodecTest : public testing::Test { std::set<int64> assigned_ids; CheckIDs(model->GetBookmarkBarNode(), &assigned_ids); CheckIDs(model->other_node(), &assigned_ids); + CheckIDs(model->synced_node(), &assigned_ids); } }; @@ -287,3 +294,45 @@ TEST_F(BookmarkCodecTest, PersistIDsTest) { &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); + + 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); +} diff --git a/chrome/browser/bookmarks/bookmark_html_writer.cc b/chrome/browser/bookmarks/bookmark_html_writer.cc index faaec66..85c48ed 100644 --- a/chrome/browser/bookmarks/bookmark_html_writer.cc +++ b/chrome/browser/bookmarks/bookmark_html_writer.cc @@ -114,12 +114,16 @@ class Writer : public Task { DictionaryValue* roots_d_value = static_cast<DictionaryValue*>(roots); Value* root_folder_value; Value* other_folder_value; + Value* synced_folder_value; if (!roots_d_value->Get(BookmarkCodec::kRootFolderNameKey, &root_folder_value) || root_folder_value->GetType() != Value::TYPE_DICTIONARY || !roots_d_value->Get(BookmarkCodec::kOtherBookmarkFolderNameKey, &other_folder_value) || - other_folder_value->GetType() != Value::TYPE_DICTIONARY) { + other_folder_value->GetType() != Value::TYPE_DICTIONARY || + !roots_d_value->Get(BookmarkCodec::kSyncedBookmarkFolderNameKey, + &synced_folder_value) || + synced_folder_value->GetType() != Value::TYPE_DICTIONARY) { NOTREACHED(); return; // Invalid type for root folder and/or other folder. } @@ -129,7 +133,9 @@ class Writer : public Task { if (!WriteNode(*static_cast<DictionaryValue*>(root_folder_value), BookmarkNode::BOOKMARK_BAR) || !WriteNode(*static_cast<DictionaryValue*>(other_folder_value), - BookmarkNode::OTHER_NODE)) { + BookmarkNode::OTHER_NODE) || + !WriteNode(*static_cast<DictionaryValue*>(synced_folder_value), + BookmarkNode::SYNCED)) { return; } @@ -286,10 +292,11 @@ class Writer : public Task { NOTREACHED(); return false; } - if (folder_type != BookmarkNode::OTHER_NODE) { - // The other folder name is not written out. This gives the effect of - // making the contents of the 'other folder' be a sibling to the bookmark - // bar folder. + if (folder_type != BookmarkNode::OTHER_NODE && + folder_type != BookmarkNode::SYNCED) { + // The other/synced folder name are not written out. This gives the effect + // of making the contents of the 'other folder' be a sibling to the + // bookmark bar folder. if (!WriteIndent() || !Write(kFolderStart) || !WriteTime(date_added_string) || @@ -329,7 +336,8 @@ class Writer : public Task { return false; } } - if (folder_type != BookmarkNode::OTHER_NODE) { + if (folder_type != BookmarkNode::OTHER_NODE && + folder_type != BookmarkNode::SYNCED) { // Close out the folder. DecrementIndent(); if (!WriteIndent() || @@ -383,6 +391,7 @@ BookmarkFaviconFetcher::~BookmarkFaviconFetcher() { void BookmarkFaviconFetcher::ExportBookmarks() { ExtractUrls(profile_->GetBookmarkModel()->GetBookmarkBarNode()); ExtractUrls(profile_->GetBookmarkModel()->other_node()); + ExtractUrls(profile_->GetBookmarkModel()->synced_node()); if (!bookmark_urls_.empty()) { FetchNextFavicon(); } else { diff --git a/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc b/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc index 2f80465..3b24b3c 100644 --- a/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc +++ b/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc @@ -167,6 +167,8 @@ TEST_F(BookmarkHTMLWriterTest, Test) { // F3 // F4 // url1 + // Synced + // url1 string16 f1_title = ASCIIToUTF16("F\"&;<1\""); string16 f2_title = ASCIIToUTF16("F2"); string16 f3_title = ASCIIToUTF16("F 3"); @@ -204,6 +206,7 @@ TEST_F(BookmarkHTMLWriterTest, Test) { model->AddURLWithCreationTime(f4, 0, url1_title, url1, t1); model->AddURLWithCreationTime(model->GetBookmarkBarNode(), 2, url4_title, url4, t4); + model->AddURLWithCreationTime(model->synced_node(), 0, url1_title, url1, t1); // Write to a temp file. BookmarksObserver observer(&message_loop); @@ -226,8 +229,8 @@ TEST_F(BookmarkHTMLWriterTest, Test) { NULL, &favicons); - // Check loaded favicon (url1 is represents by 3 separate bookmarks). - EXPECT_EQ(3U, favicons.size()); + // Check loaded favicon (url1 is represented by 4 separate bookmarks). + EXPECT_EQ(4U, favicons.size()); for (size_t i = 0; i < favicons.size(); i++) { if (url1_favicon == favicons[i].favicon_url) { EXPECT_EQ(1U, favicons[i].urls.size()); @@ -239,7 +242,7 @@ TEST_F(BookmarkHTMLWriterTest, Test) { } // Verify we got back what we wrote. - ASSERT_EQ(7U, parsed_bookmarks.size()); + ASSERT_EQ(8U, parsed_bookmarks.size()); // Windows and ChromeOS builds use Sentence case. string16 bookmark_folder_name = l10n_util::GetStringUTF16(IDS_BOOMARK_BAR_FOLDER_NAME); @@ -257,4 +260,6 @@ TEST_F(BookmarkHTMLWriterTest, Test) { string16(), string16(), string16()); AssertBookmarkEntryEquals(parsed_bookmarks[6], false, url1, url1_title, t1, f3_title, f4_title, string16()); + AssertBookmarkEntryEquals(parsed_bookmarks[7], false, url1, url1_title, t1, + string16(), string16(), string16()); } diff --git a/chrome/browser/bookmarks/bookmark_model.cc b/chrome/browser/bookmarks/bookmark_model.cc index a48ecdf..7f74bc9 100644 --- a/chrome/browser/bookmarks/bookmark_model.cc +++ b/chrome/browser/bookmarks/bookmark_model.cc @@ -8,6 +8,7 @@ #include <functional> #include "base/callback.h" +#include "base/command_line.h" #include "base/memory/scoped_vector.h" #include "build/build_config.h" #include "chrome/browser/bookmarks/bookmark_index.h" @@ -16,6 +17,7 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/history/history_notifications.h" #include "chrome/browser/profiles/profile.h" +#include "chrome/common/chrome_switches.h" #include "content/common/notification_service.h" #include "grit/generated_resources.h" #include "ui/base/l10n/l10n_util.h" @@ -61,6 +63,18 @@ void BookmarkNode::InvalidateFavicon() { favicon_ = SkBitmap(); } +bool BookmarkNode::IsVisible() const { + // The synced bookmark folder is invisible if the flag isn't set and there are + // no bookmarks under it. + if (type_ != BookmarkNode::SYNCED || + CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableSyncedBookmarksFolder) || + child_count() > 0) { + return true; + } + return false; +} + void BookmarkNode::Reset(const history::StarredEntry& entry) { DCHECK(entry.type != history::StarredEntry::URL || entry.url == url_); @@ -75,6 +89,9 @@ void BookmarkNode::Reset(const history::StarredEntry& entry) { case history::StarredEntry::BOOKMARK_BAR: type_ = BookmarkNode::BOOKMARK_BAR; break; + case history::StarredEntry::SYNCED: + type_ = BookmarkNode::SYNCED; + break; case history::StarredEntry::OTHER: type_ = BookmarkNode::OTHER_NODE; break; @@ -124,6 +141,7 @@ BookmarkModel::BookmarkModel(Profile* profile) root_(GURL()), bookmark_bar_node_(NULL), other_node_(NULL), + synced_node_(NULL), next_node_id_(1), observers_(ObserverList<BookmarkModelObserver>::NOTIFY_EXISTING_ONLY), loaded_signal_(TRUE, FALSE) { @@ -259,7 +277,7 @@ void BookmarkModel::SetTitle(const BookmarkNode* node, const string16& title) { if (node->GetTitle() == title) return; - if (node == bookmark_bar_node_ || node == other_node_) { + if (is_permanent_node(node)) { NOTREACHED(); return; } @@ -568,12 +586,14 @@ void BookmarkModel::DoneLoading( } bookmark_bar_node_ = details->release_bb_node(); other_node_ = details->release_other_folder_node(); + synced_node_ = details->release_synced_folder_node(); index_.reset(details->release_index()); // WARNING: order is important here, various places assume bookmark bar then // other node. root_.Add(bookmark_bar_node_, 0); root_.Add(other_node_, 1); + root_.Add(synced_node_, 2); { base::AutoLock url_lock(url_lock_); @@ -719,14 +739,24 @@ BookmarkNode* BookmarkModel::CreateOtherBookmarksNode() { return CreateRootNodeFromStarredEntry(entry); } +BookmarkNode* BookmarkModel::CreateSyncedBookmarksNode() { + history::StarredEntry entry; + entry.type = history::StarredEntry::SYNCED; + return CreateRootNodeFromStarredEntry(entry); +} + BookmarkNode* BookmarkModel::CreateRootNodeFromStarredEntry( const history::StarredEntry& entry) { DCHECK(entry.type == history::StarredEntry::BOOKMARK_BAR || - entry.type == history::StarredEntry::OTHER); + entry.type == history::StarredEntry::OTHER || + entry.type == history::StarredEntry::SYNCED); BookmarkNode* node = new BookmarkNode(generate_next_node_id(), GURL()); node->Reset(entry); if (entry.type == history::StarredEntry::BOOKMARK_BAR) { node->set_title(l10n_util::GetStringUTF16(IDS_BOOMARK_BAR_FOLDER_NAME)); + } else if (entry.type == history::StarredEntry::SYNCED) { + node->set_title(l10n_util::GetStringUTF16( + IDS_BOOMARK_BAR_SYNCED_FOLDER_NAME)); } else { node->set_title( l10n_util::GetStringUTF16(IDS_BOOMARK_BAR_OTHER_FOLDER_NAME)); @@ -826,6 +856,8 @@ void BookmarkModel::SetFileChanged() { BookmarkLoadDetails* BookmarkModel::CreateLoadDetails() { BookmarkNode* bb_node = CreateBookmarkNode(); BookmarkNode* other_folder_node = CreateOtherBookmarksNode(); + BookmarkNode* synced_folder_node = CreateSyncedBookmarksNode(); return new BookmarkLoadDetails( - bb_node, other_folder_node, new BookmarkIndex(profile()), next_node_id_); + bb_node, other_folder_node, synced_folder_node, + new BookmarkIndex(profile()), next_node_id_); } diff --git a/chrome/browser/bookmarks/bookmark_model.h b/chrome/browser/bookmarks/bookmark_model.h index 0e4ada68..b3c8c8d 100644 --- a/chrome/browser/bookmarks/bookmark_model.h +++ b/chrome/browser/bookmarks/bookmark_model.h @@ -50,7 +50,8 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode> { URL, FOLDER, BOOKMARK_BAR, - OTHER_NODE + OTHER_NODE, + SYNCED }; // Creates a new node with the specified url and id of 0 explicit BookmarkNode(const GURL& url); @@ -109,6 +110,15 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode> { bool is_favicon_loaded() const { return loaded_favicon_; } void set_favicon_loaded(bool value) { loaded_favicon_ = value; } + // Accessor method for controlling the visibility of a bookmark node/sub-tree. + // Note that visibility is not propagated down the tree hierarchy so if a + // parent node is marked as invisible, a child node may return "Visible". This + // function is primarily useful when traversing the model to generate a UI + // representation but we may want to suppress some nodes. + // TODO(yfriedman): Remove this when enable-synced-bookmarks-folder is + // no longer a command line flag. + bool IsVisible() const; + HistoryService::Handle favicon_load_handle() const { return favicon_load_handle_; } @@ -193,6 +203,9 @@ class BookmarkModel : public NotificationObserver, public BookmarkService { // Returns the 'other' node. This is NULL until loaded. const BookmarkNode* other_node() { return other_node_; } + // Returns the 'synced' node. This is NULL until loaded. + const BookmarkNode* synced_node() { return synced_node_; } + // Returns the parent the last node was added to. This never returns NULL // (as long as the model is loaded). const BookmarkNode* GetParentForNewNodes(); @@ -311,6 +324,9 @@ class BookmarkModel : public NotificationObserver, public BookmarkService { bool is_bookmark_bar_node(const BookmarkNode* node) const { return node == bookmark_bar_node_; } + bool is_synced_bookmarks_node(const BookmarkNode* node) const { + return node == synced_node_; + } bool is_other_bookmarks_node(const BookmarkNode* node) const { return node == other_node_; } @@ -319,7 +335,8 @@ class BookmarkModel : public NotificationObserver, public BookmarkService { bool is_permanent_node(const BookmarkNode* node) const { return is_root(node) || is_bookmark_bar_node(node) || - is_other_bookmarks_node(node); + is_other_bookmarks_node(node) || + is_synced_bookmarks_node(node); } // Sets the store to NULL, making it so the BookmarkModel does not persist @@ -380,10 +397,11 @@ class BookmarkModel : public NotificationObserver, public BookmarkService { // Returns true if the parent and index are valid. bool IsValidIndex(const BookmarkNode* parent, int index, bool allow_end); - // Creates the bookmark bar/other nodes. These call into + // Creates the bookmark bar/synced/other nodes. These call into // CreateRootNodeFromStarredEntry. BookmarkNode* CreateBookmarkNode(); BookmarkNode* CreateOtherBookmarksNode(); + BookmarkNode* CreateSyncedBookmarksNode(); // Creates a root node (either the bookmark bar node or other node) from the // specified starred entry. @@ -439,6 +457,7 @@ class BookmarkModel : public NotificationObserver, public BookmarkService { BookmarkNode* bookmark_bar_node_; BookmarkNode* other_node_; + BookmarkNode* synced_node_; // The maximum ID assigned to the bookmark nodes in the model. int64 next_node_id_; diff --git a/chrome/browser/bookmarks/bookmark_model_test_utils.cc b/chrome/browser/bookmarks/bookmark_model_test_utils.cc index 7286a9e..c2894fb 100644 --- a/chrome/browser/bookmarks/bookmark_model_test_utils.cc +++ b/chrome/browser/bookmarks/bookmark_model_test_utils.cc @@ -39,5 +39,7 @@ void BookmarkModelTestUtils::AssertModelsEqual(BookmarkModel* expected, AssertNodesEqual(expected->other_node(), actual->other_node(), check_ids); + AssertNodesEqual(expected->synced_node(), + actual->synced_node(), + check_ids); } - diff --git a/chrome/browser/bookmarks/bookmark_model_unittest.cc b/chrome/browser/bookmarks/bookmark_model_unittest.cc index aec548e..f87445e 100644 --- a/chrome/browser/bookmarks/bookmark_model_unittest.cc +++ b/chrome/browser/bookmarks/bookmark_model_unittest.cc @@ -6,6 +6,7 @@ #include <string> #include "base/base_paths.h" +#include "base/command_line.h" #include "base/file_util.h" #include "base/hash_tables.h" #include "base/path_service.h" @@ -20,6 +21,7 @@ #include "chrome/browser/history/history_notifications.h" #include "chrome/common/chrome_constants.h" #include "chrome/common/chrome_paths.h" +#include "chrome/common/chrome_switches.h" #include "chrome/test/model_test_utils.h" #include "chrome/test/testing_browser_process_test.h" #include "chrome/test/testing_profile.h" @@ -83,11 +85,16 @@ class BookmarkModelTest : public TestingBrowserProcessTest, int index2; }; - BookmarkModelTest() : model(NULL) { - model.AddObserver(this); - ClearCounts(); + BookmarkModelTest() : + model_(NULL), + original_command_line_(*CommandLine::ForCurrentProcess()) { + model_.AddObserver(this); + ClearCounts(); } + virtual void TearDown() { + *CommandLine::ForCurrentProcess() = original_command_line_; + } void Loaded(BookmarkModel* model) OVERRIDE { // We never load from the db, so that this should never get invoked. @@ -153,7 +160,9 @@ class BookmarkModelTest : public TestingBrowserProcessTest, ASSERT_EQ(reordered_count, reordered_count_); } - BookmarkModel model; + BookmarkModel model_; + + CommandLine original_command_line_; int moved_count; @@ -169,25 +178,32 @@ class BookmarkModelTest : public TestingBrowserProcessTest, }; TEST_F(BookmarkModelTest, InitialState) { - const BookmarkNode* bb_node = model.GetBookmarkBarNode(); + const BookmarkNode* bb_node = model_.GetBookmarkBarNode(); ASSERT_TRUE(bb_node != NULL); EXPECT_EQ(0, bb_node->child_count()); EXPECT_EQ(BookmarkNode::BOOKMARK_BAR, bb_node->type()); - const BookmarkNode* other_node = model.other_node(); + const BookmarkNode* other_node = model_.other_node(); ASSERT_TRUE(other_node != NULL); EXPECT_EQ(0, other_node->child_count()); EXPECT_EQ(BookmarkNode::OTHER_NODE, other_node->type()); + const BookmarkNode* synced_node = model_.synced_node(); + ASSERT_TRUE(synced_node != NULL); + EXPECT_EQ(0, synced_node->child_count()); + EXPECT_EQ(BookmarkNode::SYNCED, synced_node->type()); + EXPECT_TRUE(bb_node->id() != other_node->id()); + EXPECT_TRUE(bb_node->id() != synced_node->id()); + EXPECT_TRUE(other_node->id() != synced_node->id()); } TEST_F(BookmarkModelTest, AddURL) { - const BookmarkNode* root = model.GetBookmarkBarNode(); + const BookmarkNode* root = model_.GetBookmarkBarNode(); const string16 title(ASCIIToUTF16("foo")); const GURL url("http://foo.com"); - const BookmarkNode* new_node = model.AddURL(root, 0, title, url); + const BookmarkNode* new_node = model_.AddURL(root, 0, title, url); AssertObserverCount(1, 0, 0, 0, 0); observer_details.AssertEquals(root, NULL, 0, -1); @@ -195,17 +211,38 @@ TEST_F(BookmarkModelTest, AddURL) { ASSERT_EQ(title, new_node->GetTitle()); ASSERT_TRUE(url == new_node->GetURL()); ASSERT_EQ(BookmarkNode::URL, new_node->type()); - ASSERT_TRUE(new_node == model.GetMostRecentlyAddedNodeForURL(url)); + ASSERT_TRUE(new_node == model_.GetMostRecentlyAddedNodeForURL(url)); EXPECT_TRUE(new_node->id() != root->id() && - new_node->id() != model.other_node()->id()); + new_node->id() != model_.other_node()->id() && + new_node->id() != model_.synced_node()->id()); +} + +TEST_F(BookmarkModelTest, AddURLToSyncedBookmarks) { + const BookmarkNode* root = model_.synced_node(); + const string16 title(ASCIIToUTF16("foo")); + const GURL url("http://foo.com"); + + const BookmarkNode* new_node = model_.AddURL(root, 0, title, url); + AssertObserverCount(1, 0, 0, 0, 0); + observer_details.AssertEquals(root, NULL, 0, -1); + + ASSERT_EQ(1, root->child_count()); + ASSERT_EQ(title, new_node->GetTitle()); + ASSERT_TRUE(url == new_node->GetURL()); + ASSERT_EQ(BookmarkNode::URL, new_node->type()); + ASSERT_TRUE(new_node == model_.GetMostRecentlyAddedNodeForURL(url)); + + EXPECT_TRUE(new_node->id() != root->id() && + new_node->id() != model_.other_node()->id() && + new_node->id() != model_.synced_node()->id()); } TEST_F(BookmarkModelTest, AddFolder) { - const BookmarkNode* root = model.GetBookmarkBarNode(); + const BookmarkNode* root = model_.GetBookmarkBarNode(); const string16 title(ASCIIToUTF16("foo")); - const BookmarkNode* new_node = model.AddFolder(root, 0, title); + const BookmarkNode* new_node = model_.AddFolder(root, 0, title); AssertObserverCount(1, 0, 0, 0, 0); observer_details.AssertEquals(root, NULL, 0, -1); @@ -214,93 +251,94 @@ TEST_F(BookmarkModelTest, AddFolder) { ASSERT_EQ(BookmarkNode::FOLDER, new_node->type()); EXPECT_TRUE(new_node->id() != root->id() && - new_node->id() != model.other_node()->id()); + new_node->id() != model_.other_node()->id() && + new_node->id() != model_.synced_node()->id()); // Add another folder, just to make sure folder_ids are incremented correctly. ClearCounts(); - model.AddFolder(root, 0, title); + model_.AddFolder(root, 0, title); AssertObserverCount(1, 0, 0, 0, 0); observer_details.AssertEquals(root, NULL, 0, -1); } TEST_F(BookmarkModelTest, RemoveURL) { - const BookmarkNode* root = model.GetBookmarkBarNode(); + const BookmarkNode* root = model_.GetBookmarkBarNode(); const string16 title(ASCIIToUTF16("foo")); const GURL url("http://foo.com"); - model.AddURL(root, 0, title, url); + model_.AddURL(root, 0, title, url); ClearCounts(); - model.Remove(root, 0); + model_.Remove(root, 0); ASSERT_EQ(0, root->child_count()); AssertObserverCount(0, 0, 1, 0, 0); observer_details.AssertEquals(root, NULL, 0, -1); // Make sure there is no mapping for the URL. - ASSERT_TRUE(model.GetMostRecentlyAddedNodeForURL(url) == NULL); + ASSERT_TRUE(model_.GetMostRecentlyAddedNodeForURL(url) == NULL); } TEST_F(BookmarkModelTest, RemoveFolder) { - const BookmarkNode* root = model.GetBookmarkBarNode(); - const BookmarkNode* folder = model.AddFolder(root, 0, ASCIIToUTF16("foo")); + const BookmarkNode* root = model_.GetBookmarkBarNode(); + const BookmarkNode* folder = model_.AddFolder(root, 0, ASCIIToUTF16("foo")); ClearCounts(); // Add a URL as a child. const string16 title(ASCIIToUTF16("foo")); const GURL url("http://foo.com"); - model.AddURL(folder, 0, title, url); + model_.AddURL(folder, 0, title, url); ClearCounts(); // Now remove the folder. - model.Remove(root, 0); + model_.Remove(root, 0); ASSERT_EQ(0, root->child_count()); AssertObserverCount(0, 0, 1, 0, 0); observer_details.AssertEquals(root, NULL, 0, -1); // Make sure there is no mapping for the URL. - ASSERT_TRUE(model.GetMostRecentlyAddedNodeForURL(url) == NULL); + ASSERT_TRUE(model_.GetMostRecentlyAddedNodeForURL(url) == NULL); } TEST_F(BookmarkModelTest, SetTitle) { - const BookmarkNode* root = model.GetBookmarkBarNode(); + const BookmarkNode* root = model_.GetBookmarkBarNode(); string16 title(ASCIIToUTF16("foo")); const GURL url("http://foo.com"); - const BookmarkNode* node = model.AddURL(root, 0, title, url); + const BookmarkNode* node = model_.AddURL(root, 0, title, url); ClearCounts(); title = ASCIIToUTF16("foo2"); - model.SetTitle(node, title); + model_.SetTitle(node, title); AssertObserverCount(0, 0, 0, 1, 0); observer_details.AssertEquals(node, NULL, -1, -1); EXPECT_EQ(title, node->GetTitle()); } TEST_F(BookmarkModelTest, SetURL) { - const BookmarkNode* root = model.GetBookmarkBarNode(); + const BookmarkNode* root = model_.GetBookmarkBarNode(); const string16 title(ASCIIToUTF16("foo")); GURL url("http://foo.com"); - const BookmarkNode* node = model.AddURL(root, 0, title, url); + const BookmarkNode* node = model_.AddURL(root, 0, title, url); ClearCounts(); url = GURL("http://foo2.com"); - model.SetURL(node, url); + model_.SetURL(node, url); AssertObserverCount(0, 0, 0, 1, 0); observer_details.AssertEquals(node, NULL, -1, -1); EXPECT_EQ(url, node->GetURL()); } TEST_F(BookmarkModelTest, Move) { - const BookmarkNode* root = model.GetBookmarkBarNode(); + const BookmarkNode* root = model_.GetBookmarkBarNode(); const string16 title(ASCIIToUTF16("foo")); const GURL url("http://foo.com"); - const BookmarkNode* node = model.AddURL(root, 0, title, url); - const BookmarkNode* folder1 = model.AddFolder(root, 0, ASCIIToUTF16("foo")); + const BookmarkNode* node = model_.AddURL(root, 0, title, url); + const BookmarkNode* folder1 = model_.AddFolder(root, 0, ASCIIToUTF16("foo")); ClearCounts(); - model.Move(node, folder1, 0); + model_.Move(node, folder1, 0); AssertObserverCount(0, 1, 0, 0, 0); observer_details.AssertEquals(root, folder1, 1, 0); @@ -312,17 +350,17 @@ TEST_F(BookmarkModelTest, Move) { // And remove the folder. ClearCounts(); - model.Remove(root, 0); + model_.Remove(root, 0); AssertObserverCount(0, 0, 1, 0, 0); observer_details.AssertEquals(root, NULL, 0, -1); - EXPECT_TRUE(model.GetMostRecentlyAddedNodeForURL(url) == NULL); + EXPECT_TRUE(model_.GetMostRecentlyAddedNodeForURL(url) == NULL); EXPECT_EQ(0, root->child_count()); } TEST_F(BookmarkModelTest, Copy) { - const BookmarkNode* root = model.GetBookmarkBarNode(); + const BookmarkNode* root = model_.GetBookmarkBarNode(); static const std::string model_string("a 1:[ b c ] d 2:[ e f g ] h "); - model_test_utils::AddNodesFromModelString(model, root, model_string); + model_test_utils::AddNodesFromModelString(model_, root, model_string); // Validate initial model. std::string actualModelString = model_test_utils::ModelStringFromNode(root); @@ -331,42 +369,42 @@ TEST_F(BookmarkModelTest, Copy) { // Copy 'd' to be after '1:b': URL item from bar to folder. const BookmarkNode* nodeToCopy = root->GetChild(2); const BookmarkNode* destination = root->GetChild(1); - model.Copy(nodeToCopy, destination, 1); + model_.Copy(nodeToCopy, destination, 1); actualModelString = model_test_utils::ModelStringFromNode(root); EXPECT_EQ("a 1:[ b d c ] d 2:[ e f g ] h ", actualModelString); // Copy '1:d' to be after 'a': URL item from folder to bar. const BookmarkNode* folder = root->GetChild(1); nodeToCopy = folder->GetChild(1); - model.Copy(nodeToCopy, root, 1); + model_.Copy(nodeToCopy, root, 1); actualModelString = model_test_utils::ModelStringFromNode(root); EXPECT_EQ("a d 1:[ b d c ] d 2:[ e f g ] h ", actualModelString); // Copy '1' to be after '2:e': Folder from bar to folder. nodeToCopy = root->GetChild(2); destination = root->GetChild(4); - model.Copy(nodeToCopy, destination, 1); + model_.Copy(nodeToCopy, destination, 1); actualModelString = model_test_utils::ModelStringFromNode(root); EXPECT_EQ("a d 1:[ b d c ] d 2:[ e 1:[ b d c ] f g ] h ", actualModelString); // Copy '2:1' to be after '2:f': Folder within same folder. folder = root->GetChild(4); nodeToCopy = folder->GetChild(1); - model.Copy(nodeToCopy, folder, 3); + model_.Copy(nodeToCopy, folder, 3); actualModelString = model_test_utils::ModelStringFromNode(root); EXPECT_EQ("a d 1:[ b d c ] d 2:[ e 1:[ b d c ] f 1:[ b d c ] g ] h ", actualModelString); // Copy first 'd' to be after 'h': URL item within the bar. nodeToCopy = root->GetChild(1); - model.Copy(nodeToCopy, root, 6); + model_.Copy(nodeToCopy, root, 6); actualModelString = model_test_utils::ModelStringFromNode(root); EXPECT_EQ("a d 1:[ b d c ] d 2:[ e 1:[ b d c ] f 1:[ b d c ] g ] h d ", actualModelString); // Copy '2' to be after 'a': Folder within the bar. nodeToCopy = root->GetChild(4); - model.Copy(nodeToCopy, root, 1); + model_.Copy(nodeToCopy, root, 1); actualModelString = model_test_utils::ModelStringFromNode(root); EXPECT_EQ("a 2:[ e 1:[ b d c ] f 1:[ b d c ] g ] d 1:[ b d c ] " "d 2:[ e 1:[ b d c ] f 1:[ b d c ] g ] h d ", @@ -375,34 +413,45 @@ TEST_F(BookmarkModelTest, Copy) { // Tests that adding a URL to a folder updates the last modified time. TEST_F(BookmarkModelTest, ParentForNewNodes) { - ASSERT_EQ(model.GetBookmarkBarNode(), model.GetParentForNewNodes()); + ASSERT_EQ(model_.GetBookmarkBarNode(), model_.GetParentForNewNodes()); + + const string16 title(ASCIIToUTF16("foo")); + const GURL url("http://foo.com"); + + model_.AddURL(model_.other_node(), 0, title, url); + ASSERT_EQ(model_.other_node(), model_.GetParentForNewNodes()); +} + +// Tests that adding a URL to a folder updates the last modified time. +TEST_F(BookmarkModelTest, ParentForNewSyncedNodes) { + ASSERT_EQ(model_.GetBookmarkBarNode(), model_.GetParentForNewNodes()); const string16 title(ASCIIToUTF16("foo")); const GURL url("http://foo.com"); - model.AddURL(model.other_node(), 0, title, url); - ASSERT_EQ(model.other_node(), model.GetParentForNewNodes()); + model_.AddURL(model_.synced_node(), 0, title, url); + ASSERT_EQ(model_.synced_node(), model_.GetParentForNewNodes()); } // Make sure recently modified stays in sync when adding a URL. TEST_F(BookmarkModelTest, MostRecentlyModifiedFolders) { // Add a folder. - const BookmarkNode* folder = model.AddFolder(model.other_node(), 0, - ASCIIToUTF16("foo")); + const BookmarkNode* folder = model_.AddFolder(model_.other_node(), 0, + ASCIIToUTF16("foo")); // Add a URL to it. - model.AddURL(folder, 0, ASCIIToUTF16("blah"), GURL("http://foo.com")); + model_.AddURL(folder, 0, ASCIIToUTF16("blah"), GURL("http://foo.com")); // Make sure folder is in the most recently modified. std::vector<const BookmarkNode*> most_recent_folders = - bookmark_utils::GetMostRecentlyModifiedFolders(&model, 1); + bookmark_utils::GetMostRecentlyModifiedFolders(&model_, 1); ASSERT_EQ(1U, most_recent_folders.size()); ASSERT_EQ(folder, most_recent_folders[0]); // Nuke the folder and do another fetch, making sure folder isn't in the // returned list. - model.Remove(folder->parent(), 0); + model_.Remove(folder->parent(), 0); most_recent_folders = - bookmark_utils::GetMostRecentlyModifiedFolders(&model, 1); + bookmark_utils::GetMostRecentlyModifiedFolders(&model_, 1); ASSERT_EQ(1U, most_recent_folders.size()); ASSERT_TRUE(most_recent_folders[0] != folder); } @@ -412,19 +461,19 @@ TEST_F(BookmarkModelTest, MostRecentlyAddedEntries) { // Add a couple of nodes such that the following holds for the time of the // nodes: n1 > n2 > n3 > n4. Time base_time = Time::Now(); - BookmarkNode* n1 = AsMutable(model.AddURL(model.GetBookmarkBarNode(), + BookmarkNode* n1 = AsMutable(model_.AddURL(model_.GetBookmarkBarNode(), 0, ASCIIToUTF16("blah"), GURL("http://foo.com/0"))); - BookmarkNode* n2 = AsMutable(model.AddURL(model.GetBookmarkBarNode(), + BookmarkNode* n2 = AsMutable(model_.AddURL(model_.GetBookmarkBarNode(), 1, ASCIIToUTF16("blah"), GURL("http://foo.com/1"))); - BookmarkNode* n3 = AsMutable(model.AddURL(model.GetBookmarkBarNode(), + BookmarkNode* n3 = AsMutable(model_.AddURL(model_.GetBookmarkBarNode(), 2, ASCIIToUTF16("blah"), GURL("http://foo.com/2"))); - BookmarkNode* n4 = AsMutable(model.AddURL(model.GetBookmarkBarNode(), + BookmarkNode* n4 = AsMutable(model_.AddURL(model_.GetBookmarkBarNode(), 3, ASCIIToUTF16("blah"), GURL("http://foo.com/3"))); @@ -435,7 +484,7 @@ TEST_F(BookmarkModelTest, MostRecentlyAddedEntries) { // Make sure order is honored. std::vector<const BookmarkNode*> recently_added; - bookmark_utils::GetMostRecentlyAddedEntries(&model, 2, &recently_added); + bookmark_utils::GetMostRecentlyAddedEntries(&model_, 2, &recently_added); ASSERT_EQ(2U, recently_added.size()); ASSERT_TRUE(n1 == recently_added[0]); ASSERT_TRUE(n2 == recently_added[1]); @@ -443,7 +492,7 @@ TEST_F(BookmarkModelTest, MostRecentlyAddedEntries) { // swap 1 and 2, then check again. recently_added.clear(); SwapDateAdded(n1, n2); - bookmark_utils::GetMostRecentlyAddedEntries(&model, 4, &recently_added); + bookmark_utils::GetMostRecentlyAddedEntries(&model_, 4, &recently_added); ASSERT_EQ(4U, recently_added.size()); ASSERT_TRUE(n2 == recently_added[0]); ASSERT_TRUE(n1 == recently_added[1]); @@ -457,38 +506,38 @@ TEST_F(BookmarkModelTest, GetMostRecentlyAddedNodeForURL) { // nodes: n1 > n2 Time base_time = Time::Now(); const GURL url("http://foo.com/0"); - BookmarkNode* n1 = AsMutable(model.AddURL( - model.GetBookmarkBarNode(), 0, ASCIIToUTF16("blah"), url)); - BookmarkNode* n2 = AsMutable(model.AddURL( - model.GetBookmarkBarNode(), 1, ASCIIToUTF16("blah"), url)); + BookmarkNode* n1 = AsMutable(model_.AddURL( + model_.GetBookmarkBarNode(), 0, ASCIIToUTF16("blah"), url)); + BookmarkNode* n2 = AsMutable(model_.AddURL( + model_.GetBookmarkBarNode(), 1, ASCIIToUTF16("blah"), url)); n1->set_date_added(base_time + TimeDelta::FromDays(4)); n2->set_date_added(base_time + TimeDelta::FromDays(3)); // Make sure order is honored. - ASSERT_EQ(n1, model.GetMostRecentlyAddedNodeForURL(url)); + ASSERT_EQ(n1, model_.GetMostRecentlyAddedNodeForURL(url)); // swap 1 and 2, then check again. SwapDateAdded(n1, n2); - ASSERT_EQ(n2, model.GetMostRecentlyAddedNodeForURL(url)); + ASSERT_EQ(n2, model_.GetMostRecentlyAddedNodeForURL(url)); } // Makes sure GetBookmarks removes duplicates. TEST_F(BookmarkModelTest, GetBookmarksWithDups) { const GURL url("http://foo.com/0"); - model.AddURL(model.GetBookmarkBarNode(), 0, ASCIIToUTF16("blah"), url); - model.AddURL(model.GetBookmarkBarNode(), 1, ASCIIToUTF16("blah"), url); + model_.AddURL(model_.GetBookmarkBarNode(), 0, ASCIIToUTF16("blah"), url); + model_.AddURL(model_.GetBookmarkBarNode(), 1, ASCIIToUTF16("blah"), url); std::vector<GURL> urls; - model.GetBookmarks(&urls); + model_.GetBookmarks(&urls); EXPECT_EQ(1U, urls.size()); ASSERT_TRUE(urls[0] == url); } TEST_F(BookmarkModelTest, HasBookmarks) { const GURL url("http://foo.com/"); - model.AddURL(model.GetBookmarkBarNode(), 0, ASCIIToUTF16("bar"), url); + model_.AddURL(model_.GetBookmarkBarNode(), 0, ASCIIToUTF16("bar"), url); - EXPECT_TRUE(model.HasBookmarks()); + EXPECT_TRUE(model_.HasBookmarks()); } namespace { @@ -528,8 +577,8 @@ class StarredListener : public NotificationObserver { TEST_F(BookmarkModelTest, NotifyURLsStarred) { StarredListener listener; const GURL url("http://foo.com/0"); - const BookmarkNode* n1 = model.AddURL( - model.GetBookmarkBarNode(), 0, ASCIIToUTF16("blah"), url); + const BookmarkNode* n1 = model_.AddURL( + model_.GetBookmarkBarNode(), 0, ASCIIToUTF16("blah"), url); // Starred notification should be sent. EXPECT_EQ(1, listener.notification_count_); @@ -541,23 +590,23 @@ TEST_F(BookmarkModelTest, NotifyURLsStarred) { // Add another bookmark for the same URL. This should not send any // notification. - const BookmarkNode* n2 = model.AddURL( - model.GetBookmarkBarNode(), 1, ASCIIToUTF16("blah"), url); + const BookmarkNode* n2 = model_.AddURL( + model_.GetBookmarkBarNode(), 1, ASCIIToUTF16("blah"), url); EXPECT_EQ(0, listener.notification_count_); // Remove n2. - model.Remove(n2->parent(), 1); + model_.Remove(n2->parent(), 1); n2 = NULL; // Shouldn't have received any notification as n1 still exists with the same // URL. EXPECT_EQ(0, listener.notification_count_); - EXPECT_TRUE(model.GetMostRecentlyAddedNodeForURL(url) == n1); + EXPECT_TRUE(model_.GetMostRecentlyAddedNodeForURL(url) == n1); // Remove n1. - model.Remove(n1->parent(), 0); + model_.Remove(n1->parent(), 0); // Now we should get the notification. EXPECT_EQ(1, listener.notification_count_); @@ -762,6 +811,8 @@ TEST_F(BookmarkModelTestWithProfile, CreateAndRestore) { const std::string bbn_contents; // Structure of the children of the other node. const std::string other_contents; + // Structure of the children of the synced node. + const std::string synced_contents; } data[] = { // See PopulateNodeFromString for a description of these strings. { "", "" }, @@ -789,11 +840,16 @@ TEST_F(BookmarkModelTestWithProfile, CreateAndRestore) { PopulateNodeFromString(data[i].other_contents, &other); PopulateBookmarkNode(&other, bb_model_, bb_model_->other_node()); + TestNode synced; + PopulateNodeFromString(data[i].synced_contents, &synced); + PopulateBookmarkNode(&synced, bb_model_, bb_model_->synced_node()); + profile_->CreateBookmarkModel(false); BlockTillBookmarkModelLoaded(); VerifyModelMatchesNode(&bbn, bb_model_->GetBookmarkBarNode()); VerifyModelMatchesNode(&other, bb_model_->other_node()); + VerifyModelMatchesNode(&synced, bb_model_->synced_node()); VerifyNoDuplicateIDs(bb_model_); } } @@ -863,6 +919,9 @@ class BookmarkModelTestWithProfile2 : public BookmarkModelTestWithProfile { ASSERT_TRUE(child->GetURL() == GURL("http://www.google.com/intl/en/about.html")); + parent = bb_model_->synced_node(); + ASSERT_EQ(0, parent->child_count()); + ASSERT_TRUE(bb_model_->IsBookmarked(GURL("http://www.google.com"))); } @@ -972,8 +1031,8 @@ TEST_F(BookmarkModelTest, Sort) { // 'C' and 'a' are folders. TestNode bbn; PopulateNodeFromString("B [ a ] d [ a ]", &bbn); - const BookmarkNode* parent = model.GetBookmarkBarNode(); - PopulateBookmarkNode(&bbn, &model, parent); + const BookmarkNode* parent = model_.GetBookmarkBarNode(); + PopulateBookmarkNode(&bbn, &model_, parent); BookmarkNode* child1 = AsMutable(parent->GetChild(1)); child1->set_title(ASCIIToUTF16("a")); @@ -985,7 +1044,7 @@ TEST_F(BookmarkModelTest, Sort) { ClearCounts(); // Sort the children of the bookmark bar node. - model.SortChildren(parent); + model_.SortChildren(parent); // Make sure we were notified. AssertObserverCount(0, 0, 0, 0, 1); @@ -997,3 +1056,32 @@ TEST_F(BookmarkModelTest, Sort) { EXPECT_EQ(parent->GetChild(2)->GetTitle(), ASCIIToUTF16("B")); EXPECT_EQ(parent->GetChild(3)->GetTitle(), ASCIIToUTF16("d")); } + +TEST_F(BookmarkModelTest, NodeVisibility) { + EXPECT_TRUE(model_.GetBookmarkBarNode()->IsVisible()); + EXPECT_TRUE(model_.other_node()->IsVisible()); + // Sync node invisible by default + EXPECT_FALSE(model_.synced_node()->IsVisible()); + + // Arbitrary node should be visible + TestNode bbn; + PopulateNodeFromString("B", &bbn); + const BookmarkNode* parent = model_.GetBookmarkBarNode(); + PopulateBookmarkNode(&bbn, &model_, parent); + EXPECT_TRUE(parent->GetChild(0)->IsVisible()); +} + +TEST_F(BookmarkModelTest, SyncNodeVisibileIfFlagSet) { + CommandLine::ForCurrentProcess()->AppendSwitch( + switches::kEnableSyncedBookmarksFolder); + EXPECT_TRUE(model_.synced_node()->IsVisible()); +} + +TEST_F(BookmarkModelTest, SyncNodeVisibileWithChildren) { + const BookmarkNode* root = model_.synced_node(); + const string16 title(ASCIIToUTF16("foo")); + const GURL url("http://foo.com"); + + model_.AddURL(root, 0, title, url); + EXPECT_TRUE(model_.synced_node()->IsVisible()); +} diff --git a/chrome/browser/bookmarks/bookmark_storage.cc b/chrome/browser/bookmarks/bookmark_storage.cc index 2bb6305..050ee115 100644 --- a/chrome/browser/bookmarks/bookmark_storage.cc +++ b/chrome/browser/bookmarks/bookmark_storage.cc @@ -69,7 +69,7 @@ class BookmarkStorage::LoadTask : public Task { BookmarkCodec codec; TimeTicks start_time = TimeTicks::Now(); codec.Decode(details_->bb_node(), details_->other_folder_node(), - &max_node_id, *root.get()); + details_->synced_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()); @@ -80,6 +80,7 @@ class BookmarkStorage::LoadTask : public Task { start_time = TimeTicks::Now(); AddBookmarksToIndex(details_->bb_node()); AddBookmarksToIndex(details_->other_folder_node()); + AddBookmarksToIndex(details_->synced_folder_node()); UMA_HISTOGRAM_TIMES("Bookmarks.CreateBookmarkIndexTime", TimeTicks::Now() - start_time); } @@ -115,10 +116,12 @@ class BookmarkStorage::LoadTask : public Task { BookmarkLoadDetails::BookmarkLoadDetails(BookmarkNode* bb_node, BookmarkNode* other_folder_node, + BookmarkNode* synced_folder_node, BookmarkIndex* index, int64 max_id) : bb_node_(bb_node), other_folder_node_(other_folder_node), + synced_folder_node_(synced_folder_node), index_(index), max_id_(max_id), ids_reassigned_(false) { diff --git a/chrome/browser/bookmarks/bookmark_storage.h b/chrome/browser/bookmarks/bookmark_storage.h index 05e1392..19ec586 100644 --- a/chrome/browser/bookmarks/bookmark_storage.h +++ b/chrome/browser/bookmarks/bookmark_storage.h @@ -30,12 +30,17 @@ class BookmarkLoadDetails { public: BookmarkLoadDetails(BookmarkNode* bb_node, BookmarkNode* other_folder_node, + BookmarkNode* synced_folder_node, BookmarkIndex* index, int64 max_id); ~BookmarkLoadDetails(); BookmarkNode* bb_node() { return bb_node_.get(); } BookmarkNode* release_bb_node() { return bb_node_.release(); } + BookmarkNode* synced_folder_node() { return synced_folder_node_.get(); } + BookmarkNode* release_synced_folder_node() { + return synced_folder_node_.release(); + } BookmarkNode* other_folder_node() { return other_folder_node_.get(); } BookmarkNode* release_other_folder_node() { return other_folder_node_.release(); @@ -66,6 +71,7 @@ class BookmarkLoadDetails { private: scoped_ptr<BookmarkNode> bb_node_; scoped_ptr<BookmarkNode> other_folder_node_; + scoped_ptr<BookmarkNode> synced_folder_node_; scoped_ptr<BookmarkIndex> index_; int64 max_id_; std::string computed_checksum_; diff --git a/chrome/browser/bookmarks/bookmark_utils.cc b/chrome/browser/bookmarks/bookmark_utils.cc index 8ad306d..8206cfd 100644 --- a/chrome/browser/bookmarks/bookmark_utils.cc +++ b/chrome/browser/bookmarks/bookmark_utils.cc @@ -479,6 +479,12 @@ std::vector<const BookmarkNode*> GetMostRecentlyModifiedFolders( find(nodes.begin(), nodes.end(), model->other_node()) == nodes.end()) { nodes.push_back(model->other_node()); } + + if (nodes.size() < max_count && model->synced_node()->IsVisible() && + find(nodes.begin(), nodes.end(), + model->synced_node()) == nodes.end()) { + nodes.push_back(model->synced_node()); + } } return nodes; } diff --git a/chrome/browser/bookmarks/recently_used_folders_combo_model.cc b/chrome/browser/bookmarks/recently_used_folders_combo_model.cc index 3bc18e2..0bc1b7d 100644 --- a/chrome/browser/bookmarks/recently_used_folders_combo_model.cc +++ b/chrome/browser/bookmarks/recently_used_folders_combo_model.cc @@ -26,12 +26,12 @@ RecentlyUsedFoldersComboModel::RecentlyUsedFoldersComboModel( // We special case the placement of these, so remove them from the list, then // fix up the order. RemoveNode(model->GetBookmarkBarNode()); + RemoveNode(model->synced_node()); RemoveNode(model->other_node()); RemoveNode(node->parent()); // Make the parent the first item, unless it's the bookmark bar or other node. - if (node->parent() != model->GetBookmarkBarNode() && - node->parent() != model->other_node()) { + if (!model->is_permanent_node(node)) { nodes_.insert(nodes_.begin(), node->parent()); } @@ -42,6 +42,9 @@ RecentlyUsedFoldersComboModel::RecentlyUsedFoldersComboModel( // And put the bookmark bar and other nodes at the end of the list. nodes_.push_back(model->GetBookmarkBarNode()); nodes_.push_back(model->other_node()); + if (model->synced_node()->IsVisible()) { + nodes_.push_back(model->synced_node()); + } std::vector<const BookmarkNode*>::iterator it = std::find(nodes_.begin(), nodes_.end(), diff --git a/chrome/browser/extensions/extension_bookmark_helpers.cc b/chrome/browser/extensions/extension_bookmark_helpers.cc index a66839c8..36b6a73 100644 --- a/chrome/browser/extensions/extension_bookmark_helpers.cc +++ b/chrome/browser/extensions/extension_bookmark_helpers.cc @@ -50,7 +50,7 @@ DictionaryValue* GetNodeDictionary(const BookmarkNode* node, ListValue* children = new ListValue(); for (int i = 0; i < childCount; ++i) { const BookmarkNode* child = node->GetChild(i); - if (!only_folders || child->is_folder()) { + if (child->IsVisible() && (!only_folders || child->is_folder())) { DictionaryValue* dict = GetNodeDictionary(child, true, only_folders); children->Append(dict); } @@ -64,8 +64,10 @@ void AddNode(const BookmarkNode* node, ListValue* list, bool recurse, bool only_folders) { - DictionaryValue* dict = GetNodeDictionary(node, recurse, only_folders); - list->Append(dict); + if (node->IsVisible()) { + DictionaryValue* dict = GetNodeDictionary(node, recurse, only_folders); + list->Append(dict); + } } // Add a JSON representation of |node| to the JSON |list|. @@ -76,8 +78,8 @@ void AddNode(const BookmarkNode* node, } void AddNodeFoldersOnly(const BookmarkNode* node, - ListValue* list, - bool recurse) { + ListValue* list, + bool recurse) { return AddNode(node, list, recurse, true); } @@ -92,6 +94,7 @@ bool RemoveNode(BookmarkModel* model, } if (node == model->root_node() || node == model->other_node() || + node == model->synced_node() || node == model->GetBookmarkBarNode()) { *error = keys::kModifySpecialError; return false; diff --git a/chrome/browser/extensions/extension_bookmarks_module.cc b/chrome/browser/extensions/extension_bookmarks_module.cc index 8c31a72..f17ab59 100644 --- a/chrome/browser/extensions/extension_bookmarks_module.cc +++ b/chrome/browser/extensions/extension_bookmarks_module.cc @@ -512,9 +512,7 @@ bool MoveBookmarkFunction::RunImpl() { error_ = keys::kNoNodeError; return false; } - if (node == model->root_node() || - node == model->other_node() || - node == model->GetBookmarkBarNode()) { + if (model->is_permanent_node(node)) { error_ = keys::kModifySpecialError; return false; } @@ -606,9 +604,7 @@ bool UpdateBookmarkFunction::RunImpl() { error_ = keys::kNoNodeError; return false; } - if (node == model->root_node() || - node == model->other_node() || - node == model->GetBookmarkBarNode()) { + if (model->is_permanent_node(node)) { error_ = keys::kModifySpecialError; return false; } diff --git a/chrome/browser/history/history_types.h b/chrome/browser/history/history_types.h index 6d4925b..4d7484c 100644 --- a/chrome/browser/history/history_types.h +++ b/chrome/browser/history/history_types.h @@ -291,7 +291,10 @@ struct StarredEntry { USER_FOLDER, // The "other bookmarks" folder that holds uncategorized bookmarks. - OTHER + OTHER, + + // The synced folder. + SYNCED, }; StarredEntry(); diff --git a/chrome/browser/history/starred_url_database.cc b/chrome/browser/history/starred_url_database.cc index 5336e07..0fdf3b1 100644 --- a/chrome/browser/history/starred_url_database.cc +++ b/chrome/browser/history/starred_url_database.cc @@ -24,7 +24,9 @@ // id Unique identifier (primary key) for the entry. // type Type of entry, if 0 this corresponds to a URL, 1 for // a system folder, 2 for a user created folder, 3 for -// other. +// other. Note that 4 (synced) will never appear in the +// database because it was added after this storage was +// deprecated. // url_id ID of the url, only valid if type == 0 // group_id ID of the folder, only valid if type != 0. This id comes // from the UI and is NOT the same as id. @@ -545,6 +547,12 @@ bool StarredURLDatabase::MigrateBookmarksToFileImpl(const FilePath& path) { entry.type = history::StarredEntry::OTHER; BookmarkNode other_node(0, GURL()); other_node.Reset(entry); + // NOTE(yfriedman): We don't do anything with the synced star node because it + // won't ever exist in the starred node DB. We only need to create it to pass + // to "encode". + entry.type = history::StarredEntry::SYNCED; + BookmarkNode synced_node(0, GURL()); + synced_node.Reset(entry); std::map<history::UIStarID, history::StarID> folder_id_to_id_map; typedef std::map<history::StarID, BookmarkNode*> IDToNodeMap; @@ -579,6 +587,7 @@ bool StarredURLDatabase::MigrateBookmarksToFileImpl(const FilePath& path) { i != entries.end(); ++i) { if (!i->parent_folder_id) { DCHECK(i->type == history::StarredEntry::BOOKMARK_BAR || + i->type == history::StarredEntry::SYNCED || i->type == history::StarredEntry::OTHER); // Only entries with no parent should be the bookmark bar and other // bookmarks folders. @@ -616,7 +625,7 @@ bool StarredURLDatabase::MigrateBookmarksToFileImpl(const FilePath& path) { // Save to file. BookmarkCodec encoder; scoped_ptr<Value> encoded_bookmarks( - encoder.Encode(&bookmark_bar_node, &other_node)); + encoder.Encode(&bookmark_bar_node, &other_node, &synced_node)); std::string content; base::JSONWriter::Write(encoded_bookmarks.get(), true, &content); diff --git a/chrome/browser/sync/engine/download_updates_command.cc b/chrome/browser/sync/engine/download_updates_command.cc index e05da4e..7677989 100644 --- a/chrome/browser/sync/engine/download_updates_command.cc +++ b/chrome/browser/sync/engine/download_updates_command.cc @@ -6,11 +6,13 @@ #include <string> +#include "base/command_line.h" #include "chrome/browser/sync/engine/syncer.h" #include "chrome/browser/sync/engine/syncer_proto_util.h" #include "chrome/browser/sync/engine/syncproto.h" #include "chrome/browser/sync/syncable/directory_manager.h" #include "chrome/browser/sync/syncable/model_type_payload_map.h" +#include "chrome/common/chrome_switches.h" using syncable::ScopedDirLookup; @@ -33,6 +35,10 @@ void DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) { ClientToServerMessage::GET_UPDATES); GetUpdatesMessage* get_updates = client_to_server_message.mutable_get_updates(); + if (CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableSyncedBookmarksFolder)) { + get_updates->set_include_syncable_bookmarks(true); + } ScopedDirLookup dir(session->context()->directory_manager(), session->context()->account_name()); diff --git a/chrome/browser/sync/glue/bookmark_change_processor.cc b/chrome/browser/sync/glue/bookmark_change_processor.cc index ff19f61..bd0a82b 100644 --- a/chrome/browser/sync/glue/bookmark_change_processor.cc +++ b/chrome/browser/sync/glue/bookmark_change_processor.cc @@ -188,10 +188,10 @@ void BookmarkChangeProcessor::BookmarkNodeRemoved(BookmarkModel* model, } void BookmarkChangeProcessor::BookmarkNodeChanged(BookmarkModel* model, - const BookmarkNode* node) { + const BookmarkNode* node) { DCHECK(running()); // We shouldn't see changes to the top-level nodes. - if (node == model->GetBookmarkBarNode() || node == model->other_node()) { + if (model->is_permanent_node(node)) { NOTREACHED() << "Saw update to permanent node!"; return; } @@ -225,7 +225,7 @@ void BookmarkChangeProcessor::BookmarkNodeMoved(BookmarkModel* model, DCHECK(running()); const BookmarkNode* child = new_parent->GetChild(new_index); // We shouldn't see changes to the top-level nodes. - if (child == model->GetBookmarkBarNode() || child == model->other_node()) { + if (model->is_permanent_node(child)) { NOTREACHED() << "Saw update to permanent node!"; return; } @@ -385,7 +385,7 @@ void BookmarkChangeProcessor::ApplyChangesFromSyncModel( model_associator_->GetChromeNodeFromSyncId(changes[i].id); // Ignore changes to the permanent top-level nodes. We only care about // their children. - if ((dst == model->GetBookmarkBarNode()) || (dst == model->other_node())) + if (model->is_permanent_node(dst)) continue; if (changes[i].action == sync_api::SyncManager::ChangeRecord::ACTION_DELETE) { @@ -393,7 +393,6 @@ void BookmarkChangeProcessor::ApplyChangesFromSyncModel( DCHECK(i == 0 || changes[i-1].action == changes[i].action); // Children of a deleted node should not be deleted; they may be // reparented by a later change record. Move them to a temporary place. - DCHECK(dst) << "Could not find node to be deleted"; if (!dst) // Can't do anything if we can't find the chrome node. continue; const BookmarkNode* parent = dst->parent(); diff --git a/chrome/browser/sync/glue/bookmark_model_associator.cc b/chrome/browser/sync/glue/bookmark_model_associator.cc index c839501..af7c752 100644 --- a/chrome/browser/sync/glue/bookmark_model_associator.cc +++ b/chrome/browser/sync/glue/bookmark_model_associator.cc @@ -6,6 +6,7 @@ #include <stack> +#include "base/command_line.h" #include "base/hash_tables.h" #include "base/message_loop.h" #include "base/task.h" @@ -17,6 +18,7 @@ #include "chrome/browser/sync/syncable/autofill_migration.h" #include "chrome/browser/sync/syncable/nigori_util.h" #include "chrome/browser/sync/util/cryptographer.h" +#include "chrome/common/chrome_switches.h" #include "content/browser/browser_thread.h" namespace browser_sync { @@ -40,6 +42,7 @@ namespace browser_sync { // TODO(ncarter): Pull these tags from an external protocol specification // rather than hardcoding them here. static const char kBookmarkBarTag[] = "bookmark_bar"; +static const char kSyncedBookmarksTag[] = "synced_bookmarks"; static const char kOtherBookmarksTag[] = "other_bookmarks"; // Bookmark comparer for map of bookmark nodes. @@ -232,6 +235,7 @@ void BookmarkModelAssociator::Disassociate(int64 sync_id) { bool BookmarkModelAssociator::SyncModelHasUserCreatedNodes(bool* has_nodes) { DCHECK(has_nodes); *has_nodes = false; + bool has_synced_folder = true; int64 bookmark_bar_sync_id; if (!GetSyncIdForTaggedNode(kBookmarkBarTag, &bookmark_bar_sync_id)) { return false; @@ -240,6 +244,10 @@ bool BookmarkModelAssociator::SyncModelHasUserCreatedNodes(bool* has_nodes) { if (!GetSyncIdForTaggedNode(kOtherBookmarksTag, &other_bookmarks_sync_id)) { return false; } + int64 synced_bookmarks_sync_id; + if (!GetSyncIdForTaggedNode(kSyncedBookmarksTag, &synced_bookmarks_sync_id)) { + has_synced_folder = false; + } sync_api::ReadTransaction trans(user_share_); @@ -253,10 +261,18 @@ bool BookmarkModelAssociator::SyncModelHasUserCreatedNodes(bool* has_nodes) { return false; } + sync_api::ReadNode synced_bookmarks_node(&trans); + if (has_synced_folder && + !synced_bookmarks_node.InitByIdLookup(synced_bookmarks_sync_id)) { + return false; + } + // Sync model has user created nodes if either one of the permanent nodes // has children. *has_nodes = bookmark_bar_node.GetFirstChildId() != sync_api::kInvalidId || - other_bookmarks_node.GetFirstChildId() != sync_api::kInvalidId; + other_bookmarks_node.GetFirstChildId() != sync_api::kInvalidId || + (has_synced_folder && + synced_bookmarks_node.GetFirstChildId() != sync_api::kInvalidId); return true; } @@ -346,14 +362,32 @@ bool BookmarkModelAssociator::BuildAssociations() { << "are running against an out-of-date server?"; return false; } + if (!AssociateTaggedPermanentNode(bookmark_model_->synced_node(), + kSyncedBookmarksTag) && + // We only need to ensure that the "synced bookmarks" folder exists on the + // server if the command line flag is set. + CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableSyncedBookmarksFolder)) { + LOG(ERROR) << "Server did not create top-level synced nodes. Possibly " + << "we are running against an out-of-date server?"; + return false; + } int64 bookmark_bar_sync_id = GetSyncIdFromChromeId( bookmark_model_->GetBookmarkBarNode()->id()); - DCHECK(bookmark_bar_sync_id != sync_api::kInvalidId); + DCHECK_NE(bookmark_bar_sync_id, sync_api::kInvalidId); int64 other_bookmarks_sync_id = GetSyncIdFromChromeId( bookmark_model_->other_node()->id()); - DCHECK(other_bookmarks_sync_id != sync_api::kInvalidId); + DCHECK_NE(other_bookmarks_sync_id, sync_api::kInvalidId); + int64 synced_bookmarks_sync_id = GetSyncIdFromChromeId( + bookmark_model_->synced_node()->id()); + if (CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableSyncedBookmarksFolder)) { + DCHECK_NE(synced_bookmarks_sync_id, sync_api::kInvalidId); + } std::stack<int64> dfs_stack; + if (synced_bookmarks_sync_id != sync_api::kInvalidId) + dfs_stack.push(synced_bookmarks_sync_id); dfs_stack.push(other_bookmarks_sync_id); dfs_stack.push(bookmark_bar_sync_id); @@ -486,14 +520,25 @@ bool BookmarkModelAssociator::LoadAssociations() { // We should always be able to find the permanent nodes. return false; } + int64 synced_bookmarks_id = -1; + if (!GetSyncIdForTaggedNode(kSyncedBookmarksTag, &synced_bookmarks_id) && + CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableSyncedBookmarksFolder)) { + + // We should always be able to find the permanent nodes. + return false; + } // Build a bookmark node ID index since we are going to repeatedly search for // bookmark nodes by their IDs. BookmarkNodeIdIndex id_index; id_index.AddAll(bookmark_model_->GetBookmarkBarNode()); id_index.AddAll(bookmark_model_->other_node()); + id_index.AddAll(bookmark_model_->synced_node()); std::stack<int64> dfs_stack; + if (synced_bookmarks_id != -1) + dfs_stack.push(synced_bookmarks_id); dfs_stack.push(other_bookmarks_id); dfs_stack.push(bookmark_bar_id); @@ -522,6 +567,7 @@ bool BookmarkModelAssociator::LoadAssociations() { // Don't try to call NodesMatch on permanent nodes like bookmark bar and // other bookmarks. They are not expected to match. if (node != bookmark_model_->GetBookmarkBarNode() && + node != bookmark_model_->synced_node() && node != bookmark_model_->other_node() && !NodesMatch(node, &sync_parent)) return false; diff --git a/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc b/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc index ee82421..39e6d74 100644 --- a/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc @@ -9,6 +9,7 @@ #include <stack> #include <vector> +#include "base/command_line.h" #include "base/file_path.h" #include "base/file_util.h" #include "base/memory/scoped_ptr.h" @@ -23,6 +24,7 @@ #include "chrome/browser/sync/glue/bookmark_change_processor.h" #include "chrome/browser/sync/glue/bookmark_model_associator.h" #include "chrome/browser/sync/syncable/directory_manager.h" +#include "chrome/common/chrome_switches.h" #include "chrome/test/sync/engine/test_id_factory.h" #include "chrome/test/sync/engine/test_user_share.h" #include "chrome/test/testing_profile.h" @@ -281,7 +283,8 @@ class ProfileSyncServiceBookmarkTest : public testing::Test { ProfileSyncServiceBookmarkTest() : ui_thread_(BrowserThread::UI, &message_loop_), file_thread_(BrowserThread::FILE, &message_loop_), - model_(NULL) { + model_(NULL), + original_command_line_(*CommandLine::ForCurrentProcess()) { } virtual ~ProfileSyncServiceBookmarkTest() { @@ -291,10 +294,13 @@ class ProfileSyncServiceBookmarkTest : public testing::Test { virtual void SetUp() { test_user_share_.SetUp(); + CommandLine::ForCurrentProcess()->AppendSwitch( + switches::kEnableSyncedBookmarksFolder); } virtual void TearDown() { test_user_share_.TearDown(); + *CommandLine::ForCurrentProcess() = original_command_line_; } // Load (or re-load) the bookmark model. |load| controls use of the @@ -359,8 +365,7 @@ class ProfileSyncServiceBookmarkTest : public testing::Test { sync_api::ReadNode gnode(trans); ASSERT_TRUE(InitSyncNodeFromChromeNode(bnode, &gnode)); // Non-root node titles and parents must match. - if (bnode != model_->GetBookmarkBarNode() && - bnode != model_->other_node()) { + if (!model_->is_permanent_node(bnode)) { EXPECT_EQ(bnode->GetTitle(), WideToUTF16Hack(gnode.GetTitle())); EXPECT_EQ( model_associator_->GetChromeNodeFromSyncId(gnode.GetParentId()), @@ -459,6 +464,7 @@ class ProfileSyncServiceBookmarkTest : public testing::Test { const BookmarkNode* root = model_->root_node(); EXPECT_EQ(root->GetIndexOf(model_->GetBookmarkBarNode()), 0); EXPECT_EQ(root->GetIndexOf(model_->other_node()), 1); + EXPECT_EQ(root->GetIndexOf(model_->synced_node()), 2); std::stack<int64> stack; stack.push(bookmark_bar_id()); @@ -481,6 +487,11 @@ class ProfileSyncServiceBookmarkTest : public testing::Test { ExpectModelMatch(&trans); } + int64 synced_bookmarks_id() { + return + model_associator_->GetSyncIdFromChromeId(model_->synced_node()->id()); + } + int64 other_bookmarks_id() { return model_associator_->GetSyncIdFromChromeId(model_->other_node()->id()); @@ -503,6 +514,7 @@ class ProfileSyncServiceBookmarkTest : public testing::Test { protected: BookmarkModel* model_; + CommandLine original_command_line_; TestUserShare test_user_share_; scoped_ptr<BookmarkChangeProcessor> change_processor_; StrictMock<MockUnrecoverableErrorHandler> mock_unrecoverable_error_handler_; @@ -514,6 +526,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, InitialState) { EXPECT_TRUE(other_bookmarks_id()); EXPECT_TRUE(bookmark_bar_id()); + EXPECT_TRUE(synced_bookmarks_id()); ExpectModelMatch(); } @@ -540,6 +553,11 @@ TEST_F(ProfileSyncServiceBookmarkTest, BookmarkModelOperations) { folder, 1, ASCIIToUTF16("Airplanes"), GURL("http://www.easyjet.com/")); ExpectSyncerNodeMatching(url2); ExpectModelMatch(); + // Test addition. + const BookmarkNode* synced_folder = + model_->AddFolder(model_->synced_node(), 0, ASCIIToUTF16("pie")); + ExpectSyncerNodeMatching(synced_folder); + ExpectModelMatch(); // Test modification. model_->SetTitle(url2, ASCIIToUTF16("EasyJet")); @@ -556,6 +574,8 @@ TEST_F(ProfileSyncServiceBookmarkTest, BookmarkModelOperations) { ExpectModelMatch(); model_->Copy(url2, model_->GetBookmarkBarNode(), 0); ExpectModelMatch(); + model_->SetTitle(synced_folder, ASCIIToUTF16("strawberry")); + ExpectModelMatch(); // Test deletion. // Delete a single item. @@ -565,6 +585,8 @@ TEST_F(ProfileSyncServiceBookmarkTest, BookmarkModelOperations) { model_->Remove(folder2->parent(), folder2->parent()->GetIndexOf(folder2)); ExpectModelMatch(); + model_->Remove(model_->synced_node(), 0); + ExpectModelMatch(); } TEST_F(ProfileSyncServiceBookmarkTest, ServerChangeProcessing) { @@ -591,6 +613,8 @@ TEST_F(ProfileSyncServiceBookmarkTest, ServerChangeProcessing) { "scrollbars=0,status=0,toolbar=0,width=300," \ "height=300,resizable');});"); adds.AddURL(L"", javascript_url, other_bookmarks_id(), 0); + int64 u6 = adds.AddURL(L"Sync1", "http://www.syncable.edu/", + synced_bookmarks_id(), 0); std::vector<sync_api::SyncManager::ChangeRecord>::const_iterator it; // The bookmark model shouldn't yet have seen any of the nodes of |adds|. @@ -619,6 +643,8 @@ TEST_F(ProfileSyncServiceBookmarkTest, ServerChangeProcessing) { // Then add u3 after f1. int64 u3_old_parent = mods.ModifyPosition(u3, f2, f1); + std::wstring u6_old_title = mods.ModifyTitle(u6, L"Synced Folder A"); + // Test that the property changes have not yet taken effect. ExpectBrowserNodeTitle(u2, u2_old_title); /* ExpectBrowserNodeURL(u2, u2_old_url); */ @@ -629,6 +655,8 @@ TEST_F(ProfileSyncServiceBookmarkTest, ServerChangeProcessing) { ExpectBrowserNodeParent(u3, u3_old_parent); + ExpectBrowserNodeTitle(u6, u6_old_title); + // Apply the changes. mods.ApplyPendingChanges(change_processor_.get()); @@ -641,6 +669,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, ServerChangeProcessing) { FakeServerChange dels(&trans); dels.Delete(u2); dels.Delete(u3); + dels.Delete(u6); ExpectBrowserNodeKnown(u2); ExpectBrowserNodeKnown(u3); @@ -649,6 +678,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, ServerChangeProcessing) { ExpectBrowserNodeUnknown(u2); ExpectBrowserNodeUnknown(u3); + ExpectBrowserNodeUnknown(u6); ExpectModelMatch(&trans); } @@ -935,23 +965,31 @@ namespace { // | |-- f2u3, http://www.f2u3.com/ // | +-- f2u1, http://www.f2u1.com/ // +-- Other bookmarks -// |-- f3 -// | |-- f3u4, http://www.f3u4.com/ -// | |-- f3u2, http://www.f3u2.com/ -// | |-- f3u3, http://www.f3u3.com/ -// | +-- f3u1, http://www.f3u1.com/ -// |-- u4, http://www.u4.com/ -// |-- u3, http://www.u3.com/ -// --- f4 -// | |-- f4u1, http://www.f4u1.com/ -// | |-- f4u2, http://www.f4u2.com/ -// | |-- f4u3, http://www.f4u3.com/ -// | +-- f4u4, http://www.f4u4.com/ -// |-- dup -// | +-- dupu1, http://www.dupu1.com/ -// +-- dup -// +-- dupu2, http://www.dupu1.com/ -// +// | |-- f3 +// | | |-- f3u4, http://www.f3u4.com/ +// | | |-- f3u2, http://www.f3u2.com/ +// | | |-- f3u3, http://www.f3u3.com/ +// | | +-- f3u1, http://www.f3u1.com/ +// | |-- u4, http://www.u4.com/ +// | |-- u3, http://www.u3.com/ +// | --- f4 +// | | |-- f4u1, http://www.f4u1.com/ +// | | |-- f4u2, http://www.f4u2.com/ +// | | |-- f4u3, http://www.f4u3.com/ +// | | +-- f4u4, http://www.f4u4.com/ +// | |-- dup +// | | +-- dupu1, http://www.dupu1.com/ +// | +-- dup +// | +-- dupu2, http://www.dupu1.com/ +// | +// +-- Synced bookmarks +// |-- f5 +// | |-- f5u1, http://www.f5u1.com/ +// |-- f6 +// | |-- f6u1, http://www.f6u1.com/ +// | |-- f6u2, http://www.f6u2.com/ +// +-- u5, http://www.u5.com/ + static TestData kBookmarkBarChildren[] = { { L"u2", "http://www.u2.com/" }, { L"f1", NULL }, @@ -998,6 +1036,20 @@ static TestData kDup2Children[] = { { L"dupu2", "http://www.dupu2.com/" }, }; +static TestData kSyncedBookmarkChildren[] = { + { L"f5", NULL }, + { L"f6", NULL }, + { L"u5", "http://www.u5.com/" }, +}; +static TestData kF5Children[] = { + { L"f5u1", "http://www.f5u1.com/" }, + { L"f5u2", "http://www.f5u2.com/" }, +}; +static TestData kF6Children[] = { + { L"f6u1", "http://www.f6u1.com/" }, + { L"f6u2", "http://www.f6u2.com/" }, +}; + } // anonymous namespace. void ProfileSyncServiceBookmarkTestWithData::PopulateFromTestData( @@ -1065,6 +1117,17 @@ void ProfileSyncServiceBookmarkTestWithData::WriteTestDataToBookmarkModel() { dup_node = other_bookmarks_node->GetChild(5); PopulateFromTestData(dup_node, kDup2Children, arraysize(kDup2Children)); + const BookmarkNode* synced_bookmarks_node = model_->synced_node(); + PopulateFromTestData(synced_bookmarks_node, + kSyncedBookmarkChildren, + arraysize(kSyncedBookmarkChildren)); + + ASSERT_GE(synced_bookmarks_node->child_count(), 3); + const BookmarkNode* f5_node = synced_bookmarks_node->GetChild(0); + PopulateFromTestData(f5_node, kF5Children, arraysize(kF5Children)); + const BookmarkNode* f6_node = synced_bookmarks_node->GetChild(1); + PopulateFromTestData(f6_node, kF6Children, arraysize(kF6Children)); + ExpectBookmarkModelMatchesTestData(); } @@ -1095,6 +1158,18 @@ void ProfileSyncServiceBookmarkTestWithData:: CompareWithTestData(dup_node, kDup1Children, arraysize(kDup1Children)); dup_node = other_bookmarks_node->GetChild(5); CompareWithTestData(dup_node, kDup2Children, arraysize(kDup2Children)); + + const BookmarkNode* synced_bookmarks_node = model_->synced_node(); + CompareWithTestData(synced_bookmarks_node, + kSyncedBookmarkChildren, + arraysize(kSyncedBookmarkChildren)); + + ASSERT_GE(synced_bookmarks_node->child_count(), 3); + const BookmarkNode* f5_node = synced_bookmarks_node->GetChild(0); + CompareWithTestData(f5_node, kF5Children, arraysize(kF5Children)); + const BookmarkNode* f6_node = synced_bookmarks_node->GetChild(1); + CompareWithTestData(f6_node, kF6Children, arraysize(kF6Children)); + } // Tests persistence of the profile sync service by unloading the @@ -1161,6 +1236,7 @@ TEST_F(ProfileSyncServiceBookmarkTestWithData, MergeWithEmptyBookmarkModel) { LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE); EXPECT_EQ(model_->GetBookmarkBarNode()->child_count(), 0); EXPECT_EQ(model_->other_node()->child_count(), 0); + EXPECT_EQ(model_->synced_node()->child_count(), 0); // Now restart the sync service. Starting it should populate the bookmark // model -- test for consistency. diff --git a/chrome/browser/sync/protocol/sync.proto b/chrome/browser/sync/protocol/sync.proto index 32a8833..be947ad 100644 --- a/chrome/browser/sync/protocol/sync.proto +++ b/chrome/browser/sync/protocol/sync.proto @@ -387,6 +387,9 @@ message GetUpdatesMessage { // containing GetUpdatesStreamingResponse. These ClientToServerResponses are // delimited by a length prefix, which is encoded as a varint. optional bool streaming = 7 [default = false]; + + // Whether to request the syncable_bookmarks permanent item. + optional bool include_syncable_bookmarks = 1000 [default = false]; }; message AuthenticateMessage { diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm index 24bfc90..f5c4a3f 100644 --- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm +++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm @@ -539,6 +539,7 @@ void RecordAppLaunch(Profile* profile, GURL url) { // Don't allow edit/delete of the bar node, or of "Other Bookmarks" if ((node == nil) || (node == bookmarkModel_->other_node()) || + (node == bookmarkModel_->synced_node()) || (node == bookmarkModel_->GetBookmarkBarNode())) return NO; return YES; @@ -800,6 +801,7 @@ void RecordAppLaunch(Profile* profile, GURL url) { BookmarkNode::Type type = senderNode->type(); if (type == BookmarkNode::BOOKMARK_BAR || type == BookmarkNode::OTHER_NODE || + type == BookmarkNode::SYNCED || type == BookmarkNode::FOLDER) { parent = senderNode; newIndex = parent->child_count(); diff --git a/chrome/browser/ui/gtk/bookmarks/bookmark_editor_gtk_unittest.cc b/chrome/browser/ui/gtk/bookmarks/bookmark_editor_gtk_unittest.cc index 318f246..2efa80a 100644 --- a/chrome/browser/ui/gtk/bookmarks/bookmark_editor_gtk_unittest.cc +++ b/chrome/browser/ui/gtk/bookmarks/bookmark_editor_gtk_unittest.cc @@ -71,6 +71,8 @@ class BookmarkEditorGtkTest : public testing::Test { // oa // OF1 // of1a + // synced node + // sa void AddTestData() { std::string test_base = base_path(); @@ -89,6 +91,10 @@ class BookmarkEditorGtkTest : public testing::Test { const BookmarkNode* of1 = model_->AddFolder(model_->other_node(), 1, ASCIIToUTF16("OF1")); model_->AddURL(of1, 0, ASCIIToUTF16("of1a"), GURL(test_base + "of1a")); + + // Children of the synced node. + model_->AddURL(model_->synced_node(), 0, ASCIIToUTF16("sa"), + GURL(test_base + "sa")); } }; @@ -98,15 +104,22 @@ TEST_F(BookmarkEditorGtkTest, ModelsMatch) { BookmarkEditor::EditDetails(), BookmarkEditor::SHOW_TREE); - // The root should have two children, one for the bookmark bar node, - // the other for the 'other bookmarks' folder. + // The root should have two or three children, one for the bookmark bar node, + // another for the 'other bookmarks' folder, and depending on the visib GtkTreeModel* store = GTK_TREE_MODEL(editor.tree_store_); GtkTreeIter toplevel; ASSERT_TRUE(gtk_tree_model_get_iter_first(store, &toplevel)); GtkTreeIter bookmark_bar_node = toplevel; ASSERT_TRUE(gtk_tree_model_iter_next(store, &toplevel)); GtkTreeIter other_node = toplevel; - ASSERT_FALSE(gtk_tree_model_iter_next(store, &toplevel)); + if (model_->synced_node()->IsVisible()) { + // If we have a synced node, then the iterator should find one element after + // "other bookmarks" + ASSERT_TRUE(gtk_tree_model_iter_next(store, &toplevel)); + ASSERT_FALSE(gtk_tree_model_iter_next(store, &toplevel)); + } else { + ASSERT_FALSE(gtk_tree_model_iter_next(store, &toplevel)); + } // The bookmark bar should have 2 nodes: folder F1 and F2. GtkTreeIter f1_iter; diff --git a/chrome/browser/ui/gtk/bookmarks/bookmark_tree_model.cc b/chrome/browser/ui/gtk/bookmarks/bookmark_tree_model.cc index cc2284f..df1d016 100644 --- a/chrome/browser/ui/gtk/bookmarks/bookmark_tree_model.cc +++ b/chrome/browser/ui/gtk/bookmarks/bookmark_tree_model.cc @@ -104,8 +104,10 @@ void AddToTreeStore(BookmarkModel* model, int64 selected_id, GtkTreeStore* store, GtkTreeIter* selected_iter) { const BookmarkNode* root_node = model->root_node(); for (int i = 0; i < root_node->child_count(); ++i) { - AddToTreeStoreAt(root_node->GetChild(i), selected_id, store, selected_iter, - NULL); + const BookmarkNode* child = root_node->GetChild(i); + if (child->IsVisible()) { + AddToTreeStoreAt(child, selected_id, store, selected_iter, NULL); + } } } diff --git a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc index 4690ce0..9667acc 100644 --- a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc +++ b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc @@ -485,9 +485,12 @@ BookmarkEditorView::EditorNode* BookmarkEditorView::CreateRootNode() { EditorNode* root_node = new EditorNode(std::wstring(), 0); const BookmarkNode* bb_root_node = bb_model_->root_node(); CreateNodes(bb_root_node, root_node); - DCHECK(root_node->child_count() == 2); + DCHECK(root_node->child_count() >= 2 && root_node->child_count() <= 3); DCHECK(bb_root_node->GetChild(0)->type() == BookmarkNode::BOOKMARK_BAR); DCHECK(bb_root_node->GetChild(1)->type() == BookmarkNode::OTHER_NODE); + if (root_node->child_count() == 3) { + DCHECK(bb_root_node->GetChild(2)->type() == BookmarkNode::SYNCED); + } return root_node; } @@ -495,7 +498,7 @@ void BookmarkEditorView::CreateNodes(const BookmarkNode* bb_node, BookmarkEditorView::EditorNode* b_node) { for (int i = 0; i < bb_node->child_count(); ++i) { const BookmarkNode* child_bb_node = bb_node->GetChild(i); - if (child_bb_node->is_folder()) { + if (child_bb_node->IsVisible() && child_bb_node->is_folder()) { EditorNode* new_b_node = new EditorNode(WideToUTF16(child_bb_node->GetTitle()), child_bb_node->id()); diff --git a/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc b/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc index 042dc12..b059708 100644 --- a/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc +++ b/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc @@ -132,9 +132,13 @@ TEST_F(BookmarkEditorViewTest, ModelsMatch) { CreateEditor(profile_.get(), NULL, BookmarkEditor::EditDetails(), BookmarkEditorView::SHOW_TREE); BookmarkEditorView::EditorNode* editor_root = editor_tree_model()->GetRoot(); - // The root should have two children, one for the bookmark bar node, - // the other for the 'other bookmarks' folder. - ASSERT_EQ(2, editor_root->child_count()); + // The root should have two or three children: bookmark bar, other bookmarks + // and conditionally synced bookmarks. + if (model_->synced_node()->IsVisible()) { + ASSERT_EQ(3, editor_root->child_count()); + } else { + ASSERT_EQ(2, editor_root->child_count()); + } BookmarkEditorView::EditorNode* bb_node = editor_root->GetChild(0); // The root should have 2 nodes: folder F1 and F2. diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc index 059f378..16765cd 100644 --- a/chrome/common/chrome_switches.cc +++ b/chrome/common/chrome_switches.cc @@ -514,6 +514,9 @@ const char kEnableTabGroupsContextMenu[] = "enable-tab-groups-context-menu"; // Enable syncing browser typed urls. const char kEnableSyncTypedUrls[] = "enable-sync-typed-urls"; +// Enable syncing browser typed urls. +const char kEnableSyncedBookmarksFolder[] = "enable-synced-bookmarks-folder"; + // Enable use of experimental TCP sockets API for sending data in the // SYN packet. const char kEnableTcpFastOpen[] = "enable-tcp-fastopen"; diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h index b8626ae..9d51607 100644 --- a/chrome/common/chrome_switches.h +++ b/chrome/common/chrome_switches.h @@ -153,6 +153,7 @@ extern const char kEnableSyncEncryption[]; extern const char kEnableSyncPreferences[]; extern const char kEnableSyncSessions[]; extern const char kEnableSyncTypedUrls[]; +extern const char kEnableSyncedBookmarksFolder[]; extern const char kEnableTabGroupsContextMenu[]; extern const char kEnableTcpFastOpen[]; extern const char kEnableTopSites[]; diff --git a/chrome/test/data/bookmarks/model_without_sync.json b/chrome/test/data/bookmarks/model_without_sync.json new file mode 100644 index 0000000..517f834 --- /dev/null +++ b/chrome/test/data/bookmarks/model_without_sync.json @@ -0,0 +1,48 @@ +{ + "checksum": "47c64556f92189712e9ba2ae0b485d62", + "roots": { + "bookmark_bar": { + "children": [ { + "children": [ { + "date_added": "12949698950422234", + "id": "5", + "name": "Bookmark Manager", + "type": "url", + "url": "chrome-extension://eemcgdkfndhakfknompkggombfjjjeno/main.html#3" + } ], + "date_added": "12949698938004572", + "date_modified": "12949698958396466", + "id": "3", + "name": "Folder A", + "type": "folder" + } ], + "date_added": "0", + "date_modified": "0", + "id": "1", + "name": "Bookmarks Bar", + "type": "folder" + }, + "other": { + "children": [ { + "children": [ { + "date_added": "12949698958396466", + "id": "6", + "name": "Get started with Google Chrome", + "type": "url", + "url": "http://tools.google.com/chrome/intl/en/welcome.html" + } ], + "date_added": "12949698944107898", + "date_modified": "12949698961627573", + "id": "4", + "name": "Folder B", + "type": "folder" + } ], + "date_added": "0", + "date_modified": "0", + "id": "2", + "name": "Other Bookmarks", + "type": "folder" + } + }, + "version": 1 +} |