diff options
Diffstat (limited to 'chrome/browser')
24 files changed, 487 insertions, 132 deletions
diff --git a/chrome/browser/bookmarks/bookmark_codec.cc b/chrome/browser/bookmarks/bookmark_codec.cc index dd145b3..dee4f84 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. @@ -144,24 +151,34 @@ bool BookmarkCodec::DecodeHelper(BookmarkNode* bb_node, 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(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 || + !roots_d_value->Get(kSyncedBookmarkFolderNameKey, &synced_folder_value) || + synced_folder_value->GetType() != Value::TYPE_DICTIONARY) { + return false; // Invalid type for root folder and/or synced 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); + DecodeNode(*static_cast<DictionaryValue*>(synced_folder_value), NULL, + 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 +306,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..4569fb5 100644 --- a/chrome/browser/bookmarks/bookmark_codec_unittest.cc +++ b/chrome/browser/bookmarks/bookmark_codec_unittest.cc @@ -111,6 +111,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 +162,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); } }; 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 f203a04..0acb593 100644 --- a/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc +++ b/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc @@ -171,6 +171,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"); @@ -208,6 +210,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); @@ -232,8 +235,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()); @@ -245,7 +248,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); @@ -263,4 +266,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 b372855..e3de7c2 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" @@ -75,6 +77,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 +129,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 +265,8 @@ void BookmarkModel::SetTitle(const BookmarkNode* node, const string16& title) { if (node->GetTitle() == title) return; - if (node == bookmark_bar_node_ || node == other_node_) { + if (node == bookmark_bar_node_ || node == other_node_ || + node == synced_node_) { NOTREACHED(); return; } @@ -568,12 +575,17 @@ 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); + if (CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableSyncedBookmarksFolder)) { + root_.Add(synced_node_, 2); + } { base::AutoLock url_lock(url_lock_); @@ -719,14 +731,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 +848,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..8767645 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); @@ -193,6 +194,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 +315,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 +326,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 +388,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 +448,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_unittest.cc b/chrome/browser/bookmarks/bookmark_model_unittest.cc index 5d1a0a3..5d0aa5f 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,12 +85,14 @@ class BookmarkModelTest : public TestingBrowserProcessTest, int index2; }; - BookmarkModelTest() : model(NULL) { - model.AddObserver(this); + BookmarkModelTest() { + CommandLine::ForCurrentProcess()->AppendSwitch( + switches::kEnableSyncedBookmarksFolder); + model_.reset(new BookmarkModel(NULL)); + model_->AddObserver(this); ClearCounts(); } - void Loaded(BookmarkModel* model) { // We never load from the db, so that this should never get invoked. NOTREACHED(); @@ -152,7 +156,7 @@ class BookmarkModelTest : public TestingBrowserProcessTest, ASSERT_EQ(reordered_count, reordered_count_); } - BookmarkModel model; + scoped_ptr<BookmarkModel> model_; int moved_count; @@ -168,25 +172,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); @@ -194,17 +205,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); @@ -213,93 +245,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); @@ -311,17 +344,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); @@ -330,42 +363,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 ", @@ -374,34 +407,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_.get(), 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_.get(), 1); ASSERT_EQ(1U, most_recent_folders.size()); ASSERT_TRUE(most_recent_folders[0] != folder); } @@ -411,19 +455,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"))); @@ -434,7 +478,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_.get(), 2, &recently_added); ASSERT_EQ(2U, recently_added.size()); ASSERT_TRUE(n1 == recently_added[0]); ASSERT_TRUE(n2 == recently_added[1]); @@ -442,7 +486,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_.get(), 4, &recently_added); ASSERT_EQ(4U, recently_added.size()); ASSERT_TRUE(n2 == recently_added[0]); ASSERT_TRUE(n1 == recently_added[1]); @@ -456,38 +500,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 { @@ -527,8 +571,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_); @@ -540,23 +584,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_); @@ -760,6 +804,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. { "", "" }, @@ -787,11 +833,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_); } } @@ -861,6 +912,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"))); } @@ -970,8 +1024,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_.get(), parent); BookmarkNode* child1 = AsMutable(parent->GetChild(1)); child1->set_title(ASCIIToUTF16("a")); @@ -983,7 +1037,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); 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 cf1d4b1..847dafd 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 22a2d21..aed5d4b 100644 --- a/chrome/browser/bookmarks/bookmark_utils.cc +++ b/chrome/browser/bookmarks/bookmark_utils.cc @@ -479,6 +479,17 @@ std::vector<const BookmarkNode*> GetMostRecentlyModifiedFolders( if (nodes.size() < max_count && find(nodes.begin(), nodes.end(), model->other_node()) == nodes.end()) { nodes.push_back(model->other_node()); + + if (nodes.size() < max_count && + find(nodes.begin(), nodes.end(), + model->synced_node()) == nodes.end()) { + nodes.push_back(model->synced_node()); + if (nodes.size() < max_count && + std::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..c625d2d 100644 --- a/chrome/browser/bookmarks/recently_used_folders_combo_model.cc +++ b/chrome/browser/bookmarks/recently_used_folders_combo_model.cc @@ -4,7 +4,9 @@ #include "chrome/browser/bookmarks/recently_used_folders_combo_model.h" +#include "base/command_line.h" #include "chrome/browser/bookmarks/bookmark_utils.h" +#include "chrome/common/chrome_switches.h" #include "grit/generated_resources.h" #include "ui/base/l10n/l10n_util.h" @@ -26,11 +28,13 @@ 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->synced_node() && node->parent() != model->other_node()) { nodes_.insert(nodes_.begin(), node->parent()); } @@ -42,6 +46,10 @@ 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 (CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableSyncedBookmarksFolder)) { + 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..92e5519 100644 --- a/chrome/browser/extensions/extension_bookmark_helpers.cc +++ b/chrome/browser/extensions/extension_bookmark_helpers.cc @@ -92,6 +92,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/history/history_types.h b/chrome/browser/history/history_types.h index 44c422e..b09e5b2 100644 --- a/chrome/browser/history/history_types.h +++ b/chrome/browser/history/history_types.h @@ -282,6 +282,9 @@ struct StarredEntry { // User created folder. USER_FOLDER, + // The synced folder. + SYNCED, + // The "other bookmarks" folder that holds uncategorized bookmarks. OTHER }; diff --git a/chrome/browser/history/starred_url_database.cc b/chrome/browser/history/starred_url_database.cc index 5336e07..d0cc85c 100644 --- a/chrome/browser/history/starred_url_database.cc +++ b/chrome/browser/history/starred_url_database.cc @@ -66,6 +66,9 @@ void FillInStarredEntry(const sql::Statement& s, StarredEntry* entry) { case 3: entry->type = history::StarredEntry::OTHER; break; + case 4: + entry->type = history::StarredEntry::SYNCED; + break; default: NOTREACHED(); break; @@ -213,6 +216,9 @@ StarID StarredURLDatabase::CreateStarredEntryRow(URLID url_id, case history::StarredEntry::OTHER: statement.BindInt(0, 3); break; + case history::StarredEntry::SYNCED: + statement.BindInt(0, 4); + break; default: NOTREACHED(); } @@ -435,6 +441,29 @@ bool StarredURLDatabase::EnsureStarredIntegrityImpl( return false; } + // Make sure the synced node exists. + StarredNode* synced_node = GetNodeByType(*roots, StarredEntry::SYNCED); + if (!synced_node) { + LOG(WARNING) << "No bookmark synced folder in database"; + StarredEntry entry; + entry.folder_id = GetMaxFolderID() + 1; + // TODO (yfriedman): Is this index right? + if (entry.folder_id == 1) { + NOTREACHED() << "Unable to get new id for synced bookmarks folder"; + return false; + } + entry.id = CreateStarredEntryRow( + 0, entry.folder_id, 0, UTF8ToUTF16("synced"), base::Time::Now(), 0, + history::StarredEntry::SYNCED); + if (!entry.id) { + NOTREACHED() << "Unable to create synced bookmarks folder"; + return false; + } + entry.type = StarredEntry::SYNCED; + StarredNode* synced_node = new StarredNode(entry); + roots->insert(synced_node); + } + // Make sure the other node exists. StarredNode* other_node = GetNodeByType(*roots, StarredEntry::OTHER); if (!other_node) { @@ -545,11 +574,17 @@ bool StarredURLDatabase::MigrateBookmarksToFileImpl(const FilePath& path) { entry.type = history::StarredEntry::OTHER; BookmarkNode other_node(0, GURL()); other_node.Reset(entry); + 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; IDToNodeMap id_to_node_map; + history::UIStarID synced_folder_folder_id = 0; + history::StarID synced_folder_id = 0; + history::UIStarID other_folder_folder_id = 0; history::StarID other_folder_id = 0; @@ -562,6 +597,10 @@ bool StarredURLDatabase::MigrateBookmarksToFileImpl(const FilePath& path) { other_folder_id = i->id; other_folder_folder_id = i->folder_id; } + if (i->type == history::StarredEntry::SYNCED) { + synced_folder_id = i->id; + synced_folder_folder_id = i->folder_id; + } } } @@ -573,12 +612,17 @@ bool StarredURLDatabase::MigrateBookmarksToFileImpl(const FilePath& path) { id_to_node_map[other_folder_id] = &other_node; folder_id_to_id_map[other_folder_folder_id] = other_folder_id; } + if (synced_folder_folder_id) { + id_to_node_map[synced_folder_id] = &synced_node; + folder_id_to_id_map[synced_folder_folder_id] = synced_folder_id; + } // Iterate through the entries again creating the nodes. for (std::vector<history::StarredEntry>::iterator i = entries.begin(); 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 +660,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 a82291b..8d61778 100644 --- a/chrome/browser/sync/glue/bookmark_change_processor.cc +++ b/chrome/browser/sync/glue/bookmark_change_processor.cc @@ -191,7 +191,8 @@ void BookmarkChangeProcessor::BookmarkNodeChanged(BookmarkModel* model, const BookmarkNode* node) { DCHECK(running()); // We shouldn't see changes to the top-level nodes. - if (node == model->GetBookmarkBarNode() || node == model->other_node()) { + if (node == model->GetBookmarkBarNode() || node == model->other_node() || + node == model->synced_node()) { NOTREACHED() << "Saw update to permanent node!"; return; } @@ -225,7 +226,8 @@ 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 (child == model->GetBookmarkBarNode() || child == model->other_node() || + child == model->synced_node()) { NOTREACHED() << "Saw update to permanent node!"; return; } @@ -384,7 +386,8 @@ 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 ((dst == model->GetBookmarkBarNode()) || (dst==model->other_node()) || + (dst == model->synced_node())) continue; if (changes[i].action == sync_api::SyncManager::ChangeRecord::ACTION_DELETE) { diff --git a/chrome/browser/sync/glue/bookmark_model_associator.cc b/chrome/browser/sync/glue/bookmark_model_associator.cc index c839501..c01829b 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; } + // We only need to ensure that the "synced bookmarks" folder exists on the + // server if the command line flag is set. + if (CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableSyncedBookmarksFolder) && + !AssociateTaggedPermanentNode(bookmark_model_->synced_node(), + kSyncedBookmarksTag)) { + 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); int64 other_bookmarks_sync_id = GetSyncIdFromChromeId( bookmark_model_->other_node()->id()); DCHECK(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(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,24 @@ bool BookmarkModelAssociator::LoadAssociations() { // We should always be able to find the permanent nodes. return false; } + int64 synced_bookmarks_id = -1; + if (CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableSyncedBookmarksFolder) && + !GetSyncIdForTaggedNode(kSyncedBookmarksTag, &synced_bookmarks_id)) { + // 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 +566,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..5f6230e 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" @@ -291,6 +293,8 @@ class ProfileSyncServiceBookmarkTest : public testing::Test { virtual void SetUp() { test_user_share_.SetUp(); + CommandLine::ForCurrentProcess()->AppendSwitch( + switches::kEnableSyncedBookmarksFolder); } virtual void TearDown() { @@ -360,6 +364,7 @@ class ProfileSyncServiceBookmarkTest : public testing::Test { ASSERT_TRUE(InitSyncNodeFromChromeNode(bnode, &gnode)); // Non-root node titles and parents must match. if (bnode != model_->GetBookmarkBarNode() && + bnode != model_->synced_node() && bnode != model_->other_node()) { EXPECT_EQ(bnode->GetTitle(), WideToUTF16Hack(gnode.GetTitle())); EXPECT_EQ( @@ -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()); @@ -514,6 +525,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, InitialState) { EXPECT_TRUE(other_bookmarks_id()); EXPECT_TRUE(bookmark_bar_id()); + EXPECT_TRUE(synced_bookmarks_id()); ExpectModelMatch(); } @@ -540,6 +552,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 +573,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 +584,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 +612,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 +642,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 +654,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 +668,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, ServerChangeProcessing) { FakeServerChange dels(&trans); dels.Delete(u2); dels.Delete(u3); + dels.Delete(u6); ExpectBrowserNodeKnown(u2); ExpectBrowserNodeKnown(u3); @@ -649,6 +677,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, ServerChangeProcessing) { ExpectBrowserNodeUnknown(u2); ExpectBrowserNodeUnknown(u3); + ExpectBrowserNodeUnknown(u6); ExpectModelMatch(&trans); } @@ -935,23 +964,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 +1035,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 +1116,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 +1157,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 +1235,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 d7d203e..9d7f7a8 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..65c03b7 100644 --- a/chrome/browser/ui/gtk/bookmarks/bookmark_editor_gtk_unittest.cc +++ b/chrome/browser/ui/gtk/bookmarks/bookmark_editor_gtk_unittest.cc @@ -6,11 +6,13 @@ #include <string> +#include "base/command_line.h" #include "base/string_util.h" #include "base/utf_string_conversions.h" #include "chrome/browser/bookmarks/bookmark_model.h" #include "chrome/browser/ui/gtk/bookmarks/bookmark_editor_gtk.h" #include "chrome/browser/ui/gtk/bookmarks/bookmark_tree_model.h" +#include "chrome/common/chrome_switches.h" #include "chrome/test/testing_profile.h" #include "content/browser/browser_thread.h" #include "testing/gtest/include/gtest/gtest.h" @@ -33,6 +35,8 @@ class BookmarkEditorGtkTest : public testing::Test { } virtual void SetUp() { + CommandLine::ForCurrentProcess()->AppendSwitch( + switches::kEnableSyncedBookmarksFolder); profile_.reset(new TestingProfile()); profile_->CreateBookmarkModel(true); profile_->BlockUntilBookmarkModelLoaded(); @@ -71,6 +75,8 @@ class BookmarkEditorGtkTest : public testing::Test { // oa // OF1 // of1a + // synced node + // sa void AddTestData() { std::string test_base = base_path(); @@ -89,6 +95,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")); } }; @@ -106,6 +116,8 @@ TEST_F(BookmarkEditorGtkTest, ModelsMatch) { GtkTreeIter bookmark_bar_node = toplevel; ASSERT_TRUE(gtk_tree_model_iter_next(store, &toplevel)); GtkTreeIter other_node = toplevel; + ASSERT_TRUE(gtk_tree_model_iter_next(store, &toplevel)); + GtkTreeIter synced_node = toplevel; ASSERT_FALSE(gtk_tree_model_iter_next(store, &toplevel)); // The bookmark bar should have 2 nodes: folder F1 and F2. @@ -130,6 +142,9 @@ TEST_F(BookmarkEditorGtkTest, ModelsMatch) { ASSERT_TRUE(gtk_tree_model_iter_children(store, &child, &other_node)); ASSERT_EQ("OF1", UTF16ToUTF8(GetTitleFromTreeIter(store, &child))); ASSERT_FALSE(gtk_tree_model_iter_next(store, &child)); + + // Synced node should have one child (sa). + ASSERT_EQ(0, gtk_tree_model_iter_n_children(store, &synced_node)); } // Changes the title and makes sure parent/visual order doesn't change. diff --git a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc index b8e25ef..6b18180 100644 --- a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc +++ b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc @@ -485,9 +485,10 @@ 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() == 3); DCHECK(bb_root_node->GetChild(0)->type() == BookmarkNode::BOOKMARK_BAR); DCHECK(bb_root_node->GetChild(1)->type() == BookmarkNode::OTHER_NODE); + DCHECK(bb_root_node->GetChild(2)->type() == BookmarkNode::SYNCED); return root_node; } 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..8a63a2d 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,9 @@ 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 three children: bookmark bar, other bookmarks and + // synced bookmarks. + ASSERT_EQ(3, editor_root->child_count()); BookmarkEditorView::EditorNode* bb_node = editor_root->GetChild(0); // The root should have 2 nodes: folder F1 and F2. |