diff options
33 files changed, 529 insertions, 396 deletions
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index a5bf53e..d6552aa 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -6678,8 +6678,8 @@ The following plug-in is unresponsive: <ph name="PLUGIN_NAME">$1 <message name="IDS_BOOKMARK_BAR_FOLDER_NAME" desc="Name shown in the tree for the bookmarks bar folder"> Bookmarks bar </message> - <message name="IDS_BOOKMARK_BAR_SYNCED_FOLDER_NAME" desc="Name shown in the tree for the synced bookmarks folder"> - Synced bookmarks + <message name="IDS_BOOKMARK_BAR_MOBILE_FOLDER_NAME" desc="Name shown in the tree for the mobile bookmarks folder"> + Mobile bookmarks </message> <message name="IDS_BOOKMARK_BAR_OTHER_FOLDER_NAME" desc="Name shown in the tree for the other bookmarks folder"> Other bookmarks @@ -6689,8 +6689,8 @@ The following plug-in is unresponsive: <ph name="PLUGIN_NAME">$1 <message name="IDS_BOOKMARK_BAR_FOLDER_NAME" desc="In Title Case: Name shown in the tree for the bookmarks bar folder"> Bookmarks Bar </message> - <message name="IDS_BOOKMARK_BAR_SYNCED_FOLDER_NAME" desc="In Title Case: Name shown in the tree for the synced bookmarks folder"> - Synced Bookmarks + <message name="IDS_BOOKMARK_BAR_MOBILE_FOLDER_NAME" desc="In Title Case: Name shown in the tree for the mobile bookmarks folder"> + Mobile Bookmarks </message> <message name="IDS_BOOKMARK_BAR_OTHER_FOLDER_NAME" desc="In Title Case: Name shown in the tree for the other bookmarks folder"> Other Bookmarks diff --git a/chrome/browser/bookmarks/bookmark_codec.cc b/chrome/browser/bookmarks/bookmark_codec.cc index 55d1acb..bc5e1c1 100644 --- a/chrome/browser/bookmarks/bookmark_codec.cc +++ b/chrome/browser/bookmarks/bookmark_codec.cc @@ -19,7 +19,8 @@ using base::Time; const char* BookmarkCodec::kRootsKey = "roots"; const char* BookmarkCodec::kRootFolderNameKey = "bookmark_bar"; const char* BookmarkCodec::kOtherBookmarkFolderNameKey = "other"; -const char* BookmarkCodec::kSyncedBookmarkFolderNameKey = "synced"; +// The value is left as 'synced' for historical reasons. +const char* BookmarkCodec::kMobileBookmarkFolderNameKey = "synced"; const char* BookmarkCodec::kVersionKey = "version"; const char* BookmarkCodec::kChecksumKey = "checksum"; const char* BookmarkCodec::kIdKey = "id"; @@ -46,18 +47,18 @@ BookmarkCodec::~BookmarkCodec() {} Value* BookmarkCodec::Encode(BookmarkModel* model) { return Encode(model->bookmark_bar_node(), model->other_node(), - model->synced_node()); + model->mobile_node()); } Value* BookmarkCodec::Encode(const BookmarkNode* bookmark_bar_node, const BookmarkNode* other_folder_node, - const BookmarkNode* synced_folder_node) { + const BookmarkNode* mobile_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)); + roots->Set(kMobileBookmarkFolderNameKey, EncodeNode(mobile_folder_node)); DictionaryValue* main = new DictionaryValue(); main->SetInteger(kVersionKey, kCurrentVersion); @@ -72,7 +73,7 @@ Value* BookmarkCodec::Encode(const BookmarkNode* bookmark_bar_node, bool BookmarkCodec::Decode(BookmarkNode* bb_node, BookmarkNode* other_folder_node, - BookmarkNode* synced_folder_node, + BookmarkNode* mobile_folder_node, int64* max_id, const Value& value) { ids_.clear(); @@ -81,13 +82,13 @@ bool BookmarkCodec::Decode(BookmarkNode* bb_node, maximum_id_ = 0; stored_checksum_.clear(); InitializeChecksum(); - bool success = DecodeHelper(bb_node, other_folder_node, synced_folder_node, + bool success = DecodeHelper(bb_node, other_folder_node, mobile_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, synced_folder_node); + ReassignIDs(bb_node, other_folder_node, mobile_folder_node); *max_id = maximum_id_ + 1; return success; } @@ -122,7 +123,7 @@ Value* BookmarkCodec::EncodeNode(const BookmarkNode* node) { bool BookmarkCodec::DecodeHelper(BookmarkNode* bb_node, BookmarkNode* other_folder_node, - BookmarkNode* synced_folder_node, + BookmarkNode* mobile_folder_node, const Value& value) { if (value.GetType() != Value::TYPE_DICTIONARY) return false; // Unexpected type. @@ -164,23 +165,22 @@ bool BookmarkCodec::DecodeHelper(BookmarkNode* 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 + // Fail silently if we can't deserialize mobile 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); + Value* mobile_folder_value; + if (roots_d_value->Get(kMobileBookmarkFolderNameKey, &mobile_folder_value) && + mobile_folder_value->GetType() == Value::TYPE_DICTIONARY) { + DecodeNode(*static_cast<DictionaryValue*>(mobile_folder_value), NULL, + mobile_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 + // If we didn't find the mobile folder, we're almost guaranteed to have a + // duplicate id when we add the mobile 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 + // least reassign the mobile bookmarks to avoid it colliding with anything // else. - if (ids_valid_) { - ReassignIDsHelper(synced_folder_node); - } + if (ids_valid_) + ReassignIDsHelper(mobile_folder_node); } // Need to reset the type as decoding resets the type to FOLDER. Similarly @@ -188,12 +188,12 @@ bool BookmarkCodec::DecodeHelper(BookmarkNode* bb_node, // the file. bb_node->set_type(BookmarkNode::BOOKMARK_BAR); other_folder_node->set_type(BookmarkNode::OTHER_NODE); - synced_folder_node->set_type(BookmarkNode::SYNCED); + mobile_folder_node->set_type(BookmarkNode::MOBILE); bb_node->set_title(l10n_util::GetStringUTF16(IDS_BOOKMARK_BAR_FOLDER_NAME)); other_folder_node->set_title( l10n_util::GetStringUTF16(IDS_BOOKMARK_BAR_OTHER_FOLDER_NAME)); - synced_folder_node->set_title( - l10n_util::GetStringUTF16(IDS_BOOKMARK_BAR_SYNCED_FOLDER_NAME)); + mobile_folder_node->set_title( + l10n_util::GetStringUTF16(IDS_BOOKMARK_BAR_MOBILE_FOLDER_NAME)); return true; } @@ -322,11 +322,11 @@ bool BookmarkCodec::DecodeNode(const DictionaryValue& value, void BookmarkCodec::ReassignIDs(BookmarkNode* bb_node, BookmarkNode* other_node, - BookmarkNode* synced_node) { + BookmarkNode* mobile_node) { maximum_id_ = 0; ReassignIDsHelper(bb_node); ReassignIDsHelper(other_node); - ReassignIDsHelper(synced_node); + ReassignIDsHelper(mobile_node); ids_reassigned_ = true; } diff --git a/chrome/browser/bookmarks/bookmark_codec.h b/chrome/browser/bookmarks/bookmark_codec.h index daa2e91..cd3f1d5 100644 --- a/chrome/browser/bookmarks/bookmark_codec.h +++ b/chrome/browser/bookmarks/bookmark_codec.h @@ -50,7 +50,7 @@ class BookmarkCodec { // bookmarks out of the database. base::Value* Encode(const BookmarkNode* bookmark_bar_node, const BookmarkNode* other_folder_node, - const BookmarkNode* synced_folder_node); + const BookmarkNode* mobile_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, @@ -59,7 +59,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, + BookmarkNode* mobile_folder_node, int64* max_node_id, const base::Value& value); @@ -81,7 +81,7 @@ class BookmarkCodec { static const char* kRootsKey; static const char* kRootFolderNameKey; static const char* kOtherBookmarkFolderNameKey; - static const char* kSyncedBookmarkFolderNameKey; + static const char* kMobileBookmarkFolderNameKey; static const char* kVersionKey; static const char* kChecksumKey; static const char* kIdKey; @@ -104,7 +104,7 @@ class BookmarkCodec { // Helper to perform decoding. bool DecodeHelper(BookmarkNode* bb_node, BookmarkNode* other_folder_node, - BookmarkNode* synced_folder_node, + BookmarkNode* mobile_folder_node, const base::Value& value); // Decodes the children of the specified node. Returns true on success. @@ -114,7 +114,7 @@ class BookmarkCodec { // Reassigns bookmark IDs for all nodes. void ReassignIDs(BookmarkNode* bb_node, BookmarkNode* other_node, - BookmarkNode* synced_node); + BookmarkNode* mobile_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 7447124..9598250 100644 --- a/chrome/browser/bookmarks/bookmark_codec_unittest.cc +++ b/chrome/browser/bookmarks/bookmark_codec_unittest.cc @@ -116,7 +116,7 @@ class BookmarkCodecTest : public testing::Test { int64 max_id; bool result = codec->Decode(AsMutable(model->bookmark_bar_node()), AsMutable(model->other_node()), - AsMutable(model->synced_node()), + AsMutable(model->mobile_node()), &max_id, value); model->set_next_node_id(max_id); return result; @@ -167,7 +167,7 @@ class BookmarkCodecTest : public testing::Test { std::set<int64> assigned_ids; CheckIDs(model->bookmark_bar_node(), &assigned_ids); CheckIDs(model->other_node(), &assigned_ids); - CheckIDs(model->synced_node(), &assigned_ids); + CheckIDs(model->mobile_node(), &assigned_ids); } }; @@ -295,7 +295,7 @@ TEST_F(BookmarkCodecTest, PersistIDsTest) { true); } -TEST_F(BookmarkCodecTest, CanDecodeModelWithoutSyncedBookmarks) { +TEST_F(BookmarkCodecTest, CanDecodeModelWithoutMobileBookmarks) { FilePath test_data_directory; ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_data_directory)); FilePath test_file = test_data_directory.AppendASCII( @@ -334,5 +334,5 @@ TEST_F(BookmarkCodecTest, CanDecodeModelWithoutSyncedBookmarks) { EXPECT_EQ(BookmarkNode::URL, child->type()); EXPECT_EQ(ASCIIToUTF16("Get started with Google Chrome"), child->GetTitle()); - ASSERT_TRUE(decoded_model.synced_node() != NULL); + ASSERT_TRUE(decoded_model.mobile_node() != NULL); } diff --git a/chrome/browser/bookmarks/bookmark_extension_helpers.cc b/chrome/browser/bookmarks/bookmark_extension_helpers.cc index a9873be..cb44c0a 100644 --- a/chrome/browser/bookmarks/bookmark_extension_helpers.cc +++ b/chrome/browser/bookmarks/bookmark_extension_helpers.cc @@ -17,11 +17,9 @@ void AddNode(const BookmarkNode* node, base::ListValue* list, bool recurse, bool only_folders) { - if (node->IsVisible()) { - base::DictionaryValue* dict = bookmark_extension_helpers::GetNodeDictionary( - node, recurse, only_folders); - list->Append(dict); - } + base::DictionaryValue* dict = bookmark_extension_helpers::GetNodeDictionary( + node, recurse, only_folders); + list->Append(dict); } } // namespace @@ -61,7 +59,7 @@ base::DictionaryValue* GetNodeDictionary(const BookmarkNode* node, base::ListValue* children = new base::ListValue; for (int i = 0; i < node->child_count(); ++i) { const BookmarkNode* child = node->GetChild(i); - if (child->IsVisible() && (!only_folders || child->is_folder())) { + if (!only_folders || child->is_folder()) { DictionaryValue* dict = GetNodeDictionary(child, true, only_folders); children->Append(dict); } diff --git a/chrome/browser/bookmarks/bookmark_html_writer.cc b/chrome/browser/bookmarks/bookmark_html_writer.cc index 73103c0..c933680 100644 --- a/chrome/browser/bookmarks/bookmark_html_writer.cc +++ b/chrome/browser/bookmarks/bookmark_html_writer.cc @@ -114,16 +114,16 @@ class Writer : public base::RefCountedThreadSafe<Writer> { DictionaryValue* roots_d_value = static_cast<DictionaryValue*>(roots); Value* root_folder_value; Value* other_folder_value; - Value* synced_folder_value; + Value* mobile_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 || - !roots_d_value->Get(BookmarkCodec::kSyncedBookmarkFolderNameKey, - &synced_folder_value) || - synced_folder_value->GetType() != Value::TYPE_DICTIONARY) { + !roots_d_value->Get(BookmarkCodec::kMobileBookmarkFolderNameKey, + &mobile_folder_value) || + mobile_folder_value->GetType() != Value::TYPE_DICTIONARY) { NOTREACHED(); return; // Invalid type for root folder and/or other folder. } @@ -134,8 +134,8 @@ class Writer : public base::RefCountedThreadSafe<Writer> { BookmarkNode::BOOKMARK_BAR) || !WriteNode(*static_cast<DictionaryValue*>(other_folder_value), BookmarkNode::OTHER_NODE) || - !WriteNode(*static_cast<DictionaryValue*>(synced_folder_value), - BookmarkNode::SYNCED)) { + !WriteNode(*static_cast<DictionaryValue*>(mobile_folder_value), + BookmarkNode::MOBILE)) { return; } @@ -296,8 +296,8 @@ class Writer : public base::RefCountedThreadSafe<Writer> { return false; } if (folder_type != BookmarkNode::OTHER_NODE && - folder_type != BookmarkNode::SYNCED) { - // The other/synced folder name are not written out. This gives the effect + folder_type != BookmarkNode::MOBILE) { + // The other/mobile 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() || @@ -340,7 +340,7 @@ class Writer : public base::RefCountedThreadSafe<Writer> { } } if (folder_type != BookmarkNode::OTHER_NODE && - folder_type != BookmarkNode::SYNCED) { + folder_type != BookmarkNode::MOBILE) { // Close out the folder. DecrementIndent(); if (!WriteIndent() || @@ -396,7 +396,7 @@ BookmarkFaviconFetcher::~BookmarkFaviconFetcher() { void BookmarkFaviconFetcher::ExportBookmarks() { ExtractUrls(profile_->GetBookmarkModel()->bookmark_bar_node()); ExtractUrls(profile_->GetBookmarkModel()->other_node()); - ExtractUrls(profile_->GetBookmarkModel()->synced_node()); + ExtractUrls(profile_->GetBookmarkModel()->mobile_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 6da3b3b..c6e72df 100644 --- a/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc +++ b/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc @@ -169,7 +169,7 @@ TEST_F(BookmarkHTMLWriterTest, Test) { // F3 // F4 // url1 - // Synced + // Mobile // url1 string16 f1_title = ASCIIToUTF16("F\"&;<1\""); string16 f2_title = ASCIIToUTF16("F2"); @@ -208,7 +208,7 @@ TEST_F(BookmarkHTMLWriterTest, Test) { model->AddURLWithCreationTime(f4, 0, url1_title, url1, t1); model->AddURLWithCreationTime(model->bookmark_bar_node(), 2, url4_title, url4, t4); - model->AddURLWithCreationTime(model->synced_node(), 0, url1_title, url1, t1); + model->AddURLWithCreationTime(model->mobile_node(), 0, url1_title, url1, t1); // Write to a temp file. BookmarksObserver observer(&message_loop); diff --git a/chrome/browser/bookmarks/bookmark_model.cc b/chrome/browser/bookmarks/bookmark_model.cc index bcfa3cd..c45f87b 100644 --- a/chrome/browser/bookmarks/bookmark_model.cc +++ b/chrome/browser/bookmarks/bookmark_model.cc @@ -9,7 +9,6 @@ #include "base/bind.h" #include "base/bind_helpers.h" -#include "base/command_line.h" #include "base/memory/scoped_vector.h" #include "base/string_util.h" #include "build/build_config.h" @@ -23,7 +22,6 @@ #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/profiles/profile.h" #include "chrome/common/chrome_notification_types.h" -#include "chrome/common/chrome_switches.h" #include "chrome/common/pref_names.h" #include "content/public/browser/notification_service.h" #include "grit/generated_resources.h" @@ -57,10 +55,6 @@ BookmarkNode::BookmarkNode(int64 id, const GURL& url) BookmarkNode::~BookmarkNode() { } -bool BookmarkNode::IsVisible() const { - return true; -} - void BookmarkNode::Initialize(int64 id) { id_ = id; type_ = url_.is_empty() ? FOLDER : URL; @@ -74,27 +68,6 @@ void BookmarkNode::InvalidateFavicon() { is_favicon_loaded_ = false; } -// BookmarkPermanentNode ------------------------------------------------------ - -BookmarkPermanentNode::BookmarkPermanentNode(int64 id, - const GURL& url, - Profile* profile) - : BookmarkNode(id, url), - profile_(profile) { -} - -BookmarkPermanentNode::~BookmarkPermanentNode() { -} - -bool BookmarkPermanentNode::IsVisible() const { - // The synced bookmark folder is invisible if the flag isn't set and there are - // no bookmarks under it. - return type() != BookmarkNode::SYNCED || - CommandLine::ForCurrentProcess()->HasSwitch( - switches::kEnableSyncedBookmarksFolder) || - !empty(); -} - // BookmarkModel -------------------------------------------------------------- namespace { @@ -133,7 +106,7 @@ BookmarkModel::BookmarkModel(Profile* profile) root_(GURL()), bookmark_bar_node_(NULL), other_node_(NULL), - synced_node_(NULL), + mobile_node_(NULL), next_node_id_(1), observers_(ObserverList<BookmarkModelObserver>::NOTIFY_EXISTING_ONLY), loaded_signal_(true, false) { @@ -598,14 +571,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(); + mobile_node_ = details->release_mobile_folder_node(); index_.reset(details->release_index()); // WARNING: order is important here, various places assume the order is // constant. root_.Add(bookmark_bar_node_, 0); root_.Add(other_node_, 1); - root_.Add(synced_node_, 2); + root_.Add(mobile_node_, 2); { base::AutoLock url_lock(url_lock_); @@ -729,9 +702,8 @@ bool BookmarkModel::IsValidIndex(const BookmarkNode* parent, BookmarkNode* BookmarkModel::CreatePermanentNode(BookmarkNode::Type type) { DCHECK(type == BookmarkNode::BOOKMARK_BAR || type == BookmarkNode::OTHER_NODE || - type == BookmarkNode::SYNCED); - BookmarkPermanentNode* node = new BookmarkPermanentNode( - generate_next_node_id(), GURL(), profile_); + type == BookmarkNode::MOBILE); + BookmarkNode* node = new BookmarkNode(generate_next_node_id(), GURL()); node->set_type(type); if (type == BookmarkNode::BOOKMARK_BAR) { node->set_title(l10n_util::GetStringUTF16(IDS_BOOKMARK_BAR_FOLDER_NAME)); @@ -740,7 +712,7 @@ BookmarkNode* BookmarkModel::CreatePermanentNode(BookmarkNode::Type type) { l10n_util::GetStringUTF16(IDS_BOOKMARK_BAR_OTHER_FOLDER_NAME)); } else { node->set_title( - l10n_util::GetStringUTF16(IDS_BOOKMARK_BAR_SYNCED_FOLDER_NAME)); + l10n_util::GetStringUTF16(IDS_BOOKMARK_BAR_MOBILE_FOLDER_NAME)); } return node; } @@ -839,7 +811,7 @@ int64 BookmarkModel::generate_next_node_id() { BookmarkLoadDetails* BookmarkModel::CreateLoadDetails() { BookmarkNode* bb_node = CreatePermanentNode(BookmarkNode::BOOKMARK_BAR); BookmarkNode* other_node = CreatePermanentNode(BookmarkNode::OTHER_NODE); - BookmarkNode* synced_node = CreatePermanentNode(BookmarkNode::SYNCED); - return new BookmarkLoadDetails(bb_node, other_node, synced_node, + BookmarkNode* mobile_node = CreatePermanentNode(BookmarkNode::MOBILE); + return new BookmarkLoadDetails(bb_node, other_node, mobile_node, new BookmarkIndex(profile_), next_node_id_); } diff --git a/chrome/browser/bookmarks/bookmark_model.h b/chrome/browser/bookmarks/bookmark_model.h index 6108e0b..4b242c2 100644 --- a/chrome/browser/bookmarks/bookmark_model.h +++ b/chrome/browser/bookmarks/bookmark_model.h @@ -50,7 +50,7 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode> { FOLDER, BOOKMARK_BAR, OTHER_NODE, - SYNCED + MOBILE }; // Creates a new node with an id of 0 and |url|. @@ -110,15 +110,6 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode> { favicon_load_handle_ = handle; } - // 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. - virtual bool IsVisible() const; - // TODO(sky): Consider adding last visit time here, it'll greatly simplify // HistoryContentsProvider. @@ -160,30 +151,11 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode> { DISALLOW_COPY_AND_ASSIGN(BookmarkNode); }; -// BookmarkPermanentNode ------------------------------------------------------ - -// The permanent nodes are the three special top level nodes "bookmark_bar", -// "synced" and "other". Their visibility is dependent on information from the -// profile, hence this special subclass to accomodate them. -class BookmarkPermanentNode : public BookmarkNode { - public: - // Creates a new node with |id| and |url|. - BookmarkPermanentNode(int64 id, const GURL& url, Profile* profile); - - virtual ~BookmarkPermanentNode(); - virtual bool IsVisible() const OVERRIDE; - - private: - Profile* profile_; - - DISALLOW_COPY_AND_ASSIGN(BookmarkPermanentNode); -}; - // BookmarkModel -------------------------------------------------------------- // BookmarkModel provides a directed acyclic graph of URLs and folders. // Three graphs are provided for the three entry points: those on the 'bookmarks -// bar', those in the 'other bookmarks' folder and those in the 'synced' folder. +// bar', those in the 'other bookmarks' folder and those in the 'mobile' folder. // // An observer may be attached to observe relevant events. // @@ -218,18 +190,18 @@ class BookmarkModel : public content::NotificationObserver, // 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 'mobile' node. This is NULL until loaded. + const BookmarkNode* mobile_node() { return mobile_node_; } bool is_root_node(const BookmarkNode* node) const { return node == &root_; } // Returns whether the given |node| is one of the permanent nodes - root node, - // 'bookmark bar' node, 'other' node or 'synced' node. + // 'bookmark bar' node, 'other' node or 'mobile' node. bool is_permanent_node(const BookmarkNode* node) const { return node == &root_ || node == bookmark_bar_node_ || node == other_node_ || - node == synced_node_; + node == mobile_node_; } Profile* profile() const { return profile_; } @@ -399,7 +371,7 @@ class BookmarkModel : public content::NotificationObserver, bool IsValidIndex(const BookmarkNode* parent, int index, bool allow_end); // Creates one of the possible permanent nodes (bookmark bar node, other node - // and synced node) from |type|. + // and mobile node) from |type|. BookmarkNode* CreatePermanentNode(BookmarkNode::Type type); // Notification that a favicon has finished loading. If we can decode the @@ -451,7 +423,7 @@ class BookmarkModel : public content::NotificationObserver, BookmarkNode* bookmark_bar_node_; BookmarkNode* other_node_; - BookmarkNode* synced_node_; + BookmarkNode* mobile_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 478a092..15370d4 100644 --- a/chrome/browser/bookmarks/bookmark_model_test_utils.cc +++ b/chrome/browser/bookmarks/bookmark_model_test_utils.cc @@ -39,7 +39,7 @@ void BookmarkModelTestUtils::AssertModelsEqual(BookmarkModel* expected, AssertNodesEqual(expected->other_node(), actual->other_node(), check_ids); - AssertNodesEqual(expected->synced_node(), - actual->synced_node(), + AssertNodesEqual(expected->mobile_node(), + actual->mobile_node(), check_ids); } diff --git a/chrome/browser/bookmarks/bookmark_model_unittest.cc b/chrome/browser/bookmarks/bookmark_model_unittest.cc index 1f7449a..d4bf5bc 100644 --- a/chrome/browser/bookmarks/bookmark_model_unittest.cc +++ b/chrome/browser/bookmarks/bookmark_model_unittest.cc @@ -211,14 +211,14 @@ TEST_F(BookmarkModelTest, InitialState) { 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()); + const BookmarkNode* mobile_node = model_.mobile_node(); + ASSERT_TRUE(mobile_node != NULL); + EXPECT_EQ(0, mobile_node->child_count()); + EXPECT_EQ(BookmarkNode::MOBILE, mobile_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()); + EXPECT_TRUE(bb_node->id() != mobile_node->id()); + EXPECT_TRUE(other_node->id() != mobile_node->id()); } TEST_F(BookmarkModelTest, AddURL) { @@ -238,7 +238,7 @@ TEST_F(BookmarkModelTest, AddURL) { EXPECT_TRUE(new_node->id() != root->id() && new_node->id() != model_.other_node()->id() && - new_node->id() != model_.synced_node()->id()); + new_node->id() != model_.mobile_node()->id()); } TEST_F(BookmarkModelTest, AddURLWithWhitespaceTitle) { @@ -257,8 +257,8 @@ TEST_F(BookmarkModelTest, AddURLWithWhitespaceTitle) { } } -TEST_F(BookmarkModelTest, AddURLToSyncedBookmarks) { - const BookmarkNode* root = model_.synced_node(); +TEST_F(BookmarkModelTest, AddURLToMobileBookmarks) { + const BookmarkNode* root = model_.mobile_node(); const string16 title(ASCIIToUTF16("foo")); const GURL url("http://foo.com"); @@ -274,7 +274,7 @@ TEST_F(BookmarkModelTest, AddURLToSyncedBookmarks) { EXPECT_TRUE(new_node->id() != root->id() && new_node->id() != model_.other_node()->id() && - new_node->id() != model_.synced_node()->id()); + new_node->id() != model_.mobile_node()->id()); } TEST_F(BookmarkModelTest, AddFolder) { @@ -291,7 +291,7 @@ TEST_F(BookmarkModelTest, AddFolder) { EXPECT_TRUE(new_node->id() != root->id() && new_node->id() != model_.other_node()->id() && - new_node->id() != model_.synced_node()->id()); + new_node->id() != model_.mobile_node()->id()); // Add another folder, just to make sure folder_ids are incremented correctly. ClearCounts(); @@ -491,14 +491,14 @@ TEST_F(BookmarkModelTest, ParentForNewNodes) { } // Tests that adding a URL to a folder updates the last modified time. -TEST_F(BookmarkModelTest, ParentForNewSyncedNodes) { +TEST_F(BookmarkModelTest, ParentForNewMobileNodes) { ASSERT_EQ(model_.bookmark_bar_node(), model_.GetParentForNewNodes()); const string16 title(ASCIIToUTF16("foo")); const GURL url("http://foo.com"); - model_.AddURL(model_.synced_node(), 0, title, url); - ASSERT_EQ(model_.synced_node(), model_.GetParentForNewNodes()); + model_.AddURL(model_.mobile_node(), 0, title, url); + ASSERT_EQ(model_.mobile_node(), model_.GetParentForNewNodes()); } // Make sure recently modified stays in sync when adding a URL. @@ -836,7 +836,7 @@ TEST_F(BookmarkModelTestWithProfile, CreateAndRestore) { // 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; + const std::string mobile_contents; } data[] = { // See PopulateNodeFromString for a description of these strings. { "", "" }, @@ -864,16 +864,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()); + TestNode mobile; + PopulateNodeFromString(data[i].mobile_contents, &mobile); + PopulateBookmarkNode(&mobile, bb_model_, bb_model_->mobile_node()); profile_->CreateBookmarkModel(false); BlockTillBookmarkModelLoaded(); VerifyModelMatchesNode(&bbn, bb_model_->bookmark_bar_node()); VerifyModelMatchesNode(&other, bb_model_->other_node()); - VerifyModelMatchesNode(&synced, bb_model_->synced_node()); + VerifyModelMatchesNode(&mobile, bb_model_->mobile_node()); VerifyNoDuplicateIDs(bb_model_); } } @@ -943,7 +943,7 @@ class BookmarkModelTestWithProfile2 : public BookmarkModelTestWithProfile { ASSERT_TRUE(child->url() == GURL("http://www.google.com/intl/en/about.html")); - parent = bb_model_->synced_node(); + parent = bb_model_->mobile_node(); ASSERT_EQ(0, parent->child_count()); ASSERT_TRUE(bb_model_->IsBookmarked(GURL("http://www.google.com"))); @@ -1081,33 +1081,4 @@ TEST_F(BookmarkModelTest, Sort) { EXPECT_EQ(parent->GetChild(3)->GetTitle(), ASCIIToUTF16("d")); } -TEST_F(BookmarkModelTest, NodeVisibility) { - EXPECT_TRUE(model_.bookmark_bar_node()->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_.bookmark_bar_node(); - 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()); -} - } // namespace diff --git a/chrome/browser/bookmarks/bookmark_storage.cc b/chrome/browser/bookmarks/bookmark_storage.cc index 58881cf..e3daffd 100644 --- a/chrome/browser/bookmarks/bookmark_storage.cc +++ b/chrome/browser/bookmarks/bookmark_storage.cc @@ -62,7 +62,7 @@ void LoadCallback(const FilePath& path, BookmarkCodec codec; TimeTicks start_time = TimeTicks::Now(); codec.Decode(details->bb_node(), details->other_folder_node(), - details->synced_folder_node(), &max_node_id, *root.get()); + details->mobile_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()); @@ -73,7 +73,7 @@ void LoadCallback(const FilePath& path, start_time = TimeTicks::Now(); AddBookmarksToIndex(details, details->bb_node()); AddBookmarksToIndex(details, details->other_folder_node()); - AddBookmarksToIndex(details, details->synced_folder_node()); + AddBookmarksToIndex(details, details->mobile_folder_node()); UMA_HISTOGRAM_TIMES("Bookmarks.CreateBookmarkIndexTime", TimeTicks::Now() - start_time); } @@ -91,12 +91,12 @@ void LoadCallback(const FilePath& path, BookmarkLoadDetails::BookmarkLoadDetails(BookmarkNode* bb_node, BookmarkNode* other_folder_node, - BookmarkNode* synced_folder_node, + BookmarkNode* mobile_folder_node, BookmarkIndex* index, int64 max_id) : bb_node_(bb_node), other_folder_node_(other_folder_node), - synced_folder_node_(synced_folder_node), + mobile_folder_node_(mobile_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 5314726..e5f5738 100644 --- a/chrome/browser/bookmarks/bookmark_storage.h +++ b/chrome/browser/bookmarks/bookmark_storage.h @@ -30,16 +30,16 @@ class BookmarkLoadDetails { public: BookmarkLoadDetails(BookmarkNode* bb_node, BookmarkNode* other_folder_node, - BookmarkNode* synced_folder_node, + BookmarkNode* mobile_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* mobile_folder_node() { return mobile_folder_node_.get(); } + BookmarkNode* release_mobile_folder_node() { + return mobile_folder_node_.release(); } BookmarkNode* other_folder_node() { return other_folder_node_.get(); } BookmarkNode* release_other_folder_node() { @@ -74,7 +74,7 @@ class BookmarkLoadDetails { private: scoped_ptr<BookmarkNode> bb_node_; scoped_ptr<BookmarkNode> other_folder_node_; - scoped_ptr<BookmarkNode> synced_folder_node_; + scoped_ptr<BookmarkNode> mobile_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 3d4b806..7515017 100644 --- a/chrome/browser/bookmarks/bookmark_utils.cc +++ b/chrome/browser/bookmarks/bookmark_utils.cc @@ -468,17 +468,11 @@ string16 GetNameForURL(const GURL& url) { } } -// This is used with a tree iterator to skip subtrees which are not visible. -static bool PruneInvisibleFolders(const BookmarkNode* node) { - return !node->IsVisible(); -} - std::vector<const BookmarkNode*> GetMostRecentlyModifiedFolders( BookmarkModel* model, size_t max_count) { std::vector<const BookmarkNode*> nodes; - ui::TreeNodeIterator<const BookmarkNode> - iterator(model->root_node(), PruneInvisibleFolders); + ui::TreeNodeIterator<const BookmarkNode> iterator(model->root_node()); while (iterator.has_next()) { const BookmarkNode* parent = iterator.Next(); @@ -506,8 +500,7 @@ std::vector<const BookmarkNode*> GetMostRecentlyModifiedFolders( for (int i = 0; i < root_node->child_count(); ++i) { const BookmarkNode* node = root_node->GetChild(i); - if (node->IsVisible() && - find(nodes.begin(), nodes.end(), node) == nodes.end()) { + if (find(nodes.begin(), nodes.end(), node) == nodes.end()) { nodes.push_back(node); if (nodes.size() == max_count) diff --git a/chrome/browser/bookmarks/recently_used_folders_combo_model.cc b/chrome/browser/bookmarks/recently_used_folders_combo_model.cc index ba1b830..79c8463 100644 --- a/chrome/browser/bookmarks/recently_used_folders_combo_model.cc +++ b/chrome/browser/bookmarks/recently_used_folders_combo_model.cc @@ -27,7 +27,7 @@ RecentlyUsedFoldersComboModel::RecentlyUsedFoldersComboModel( // We special case the placement of these, so remove them from the list, then // fix up the order. RemoveNode(model->bookmark_bar_node()); - RemoveNode(model->synced_node()); + RemoveNode(model->mobile_node()); RemoveNode(model->other_node()); RemoveNode(node->parent()); @@ -43,8 +43,7 @@ RecentlyUsedFoldersComboModel::RecentlyUsedFoldersComboModel( // And put the bookmark bar and other nodes at the end of the list. nodes_.push_back(model->bookmark_bar_node()); nodes_.push_back(model->other_node()); - if (model->synced_node()->IsVisible()) - nodes_.push_back(model->synced_node()); + nodes_.push_back(model->mobile_node()); std::vector<const BookmarkNode*>::iterator it = std::find(nodes_.begin(), nodes_.end(), diff --git a/chrome/browser/history/history_types.h b/chrome/browser/history/history_types.h index 5567052..df626a7 100644 --- a/chrome/browser/history/history_types.h +++ b/chrome/browser/history/history_types.h @@ -305,8 +305,8 @@ struct StarredEntry { // The "other bookmarks" folder that holds uncategorized bookmarks. OTHER, - // The synced folder. - SYNCED, + // The mobile folder. + MOBILE, }; StarredEntry(); diff --git a/chrome/browser/history/starred_url_database.cc b/chrome/browser/history/starred_url_database.cc index 166e20c..8328d16 100644 --- a/chrome/browser/history/starred_url_database.cc +++ b/chrome/browser/history/starred_url_database.cc @@ -104,8 +104,8 @@ void ResetBookmarkNode(const history::StarredEntry& entry, case history::StarredEntry::OTHER: node->set_type(BookmarkNode::OTHER_NODE); break; - case history::StarredEntry::SYNCED: - node->set_type(BookmarkNode::SYNCED); + case history::StarredEntry::MOBILE: + node->set_type(BookmarkNode::MOBILE); break; default: NOTREACHED(); @@ -587,12 +587,12 @@ bool StarredURLDatabase::MigrateBookmarksToFileImpl(const FilePath& path) { entry.type = history::StarredEntry::OTHER; BookmarkNode other_node(0, GURL()); ResetBookmarkNode(entry, &other_node); - // 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()); - ResetBookmarkNode(entry, &synced_node); + // NOTE(yfriedman): We don't do anything with the mobile 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::MOBILE; + BookmarkNode mobile_node(0, GURL()); + ResetBookmarkNode(entry, &mobile_node); std::map<history::UIStarID, history::StarID> folder_id_to_id_map; typedef std::map<history::StarID, BookmarkNode*> IDToNodeMap; @@ -626,7 +626,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::MOBILE || i->type == history::StarredEntry::OTHER); // Only entries with no parent should be the bookmark bar and other // bookmarks folders. @@ -664,7 +664,7 @@ bool StarredURLDatabase::MigrateBookmarksToFileImpl(const FilePath& path) { // Save to file. BookmarkCodec encoder; scoped_ptr<Value> encoded_bookmarks( - encoder.Encode(&bookmark_bar_node, &other_node, &synced_node)); + encoder.Encode(&bookmark_bar_node, &other_node, &mobile_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 f97ae98..f0ceb30 100644 --- a/chrome/browser/sync/engine/download_updates_command.cc +++ b/chrome/browser/sync/engine/download_updates_command.cc @@ -6,13 +6,11 @@ #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; @@ -37,10 +35,8 @@ 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); - } + // TODO: make this default to true. + 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_model_associator.cc b/chrome/browser/sync/glue/bookmark_model_associator.cc index e50506c..a010fad 100644 --- a/chrome/browser/sync/glue/bookmark_model_associator.cc +++ b/chrome/browser/sync/glue/bookmark_model_associator.cc @@ -47,7 +47,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 kMobileBookmarksTag[] = "synced_bookmarks"; static const char kOtherBookmarksTag[] = "other_bookmarks"; static const char kServerError[] = "Server did not create top-level nodes. Possibly we are running against " @@ -243,7 +243,7 @@ void BookmarkModelAssociator::Disassociate(int64 sync_id) { bool BookmarkModelAssociator::SyncModelHasUserCreatedNodes(bool* has_nodes) { DCHECK(has_nodes); *has_nodes = false; - bool has_synced_folder = true; + bool has_mobile_folder = true; int64 bookmark_bar_sync_id; if (!GetSyncIdForTaggedNode(kBookmarkBarTag, &bookmark_bar_sync_id)) { return false; @@ -252,9 +252,9 @@ 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; + int64 mobile_bookmarks_sync_id; + if (!GetSyncIdForTaggedNode(kMobileBookmarksTag, &mobile_bookmarks_sync_id)) { + has_mobile_folder = false; } sync_api::ReadTransaction trans(FROM_HERE, user_share_); @@ -269,17 +269,17 @@ 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)) { + sync_api::ReadNode mobile_bookmarks_node(&trans); + if (has_mobile_folder && + !mobile_bookmarks_node.InitByIdLookup(mobile_bookmarks_sync_id)) { return false; } - // Sync model has user created nodes if either one of the permanent nodes - // has children. + // Sync model has user created nodes if any of the permanent nodes has + // children. *has_nodes = bookmark_bar_node.HasChildren() || other_bookmarks_node.HasChildren() || - (has_synced_folder && synced_bookmarks_node.HasChildren()); + (has_mobile_folder && mobile_bookmarks_node.HasChildren()); return true; } @@ -368,12 +368,8 @@ bool BookmarkModelAssociator::BuildAssociations(SyncError* error) { error->Reset(FROM_HERE, kServerError, model_type()); 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)) { + if (!AssociateTaggedPermanentNode(bookmark_model_->mobile_node(), + kMobileBookmarksTag)) { error->Reset(FROM_HERE, kServerError, model_type()); return false; } @@ -383,16 +379,13 @@ bool BookmarkModelAssociator::BuildAssociations(SyncError* error) { int64 other_bookmarks_sync_id = GetSyncIdFromChromeId( bookmark_model_->other_node()->id()); 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); - } + int64 mobile_bookmarks_sync_id = GetSyncIdFromChromeId( + bookmark_model_->mobile_node()->id()); + DCHECK_NE(mobile_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); + if (mobile_bookmarks_sync_id != sync_api::kInvalidId) + dfs_stack.push(mobile_bookmarks_sync_id); dfs_stack.push(other_bookmarks_sync_id); dfs_stack.push(bookmark_bar_sync_id); @@ -531,11 +524,8 @@ 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)) { - + int64 mobile_bookmarks_id = -1; + if (!GetSyncIdForTaggedNode(kMobileBookmarksTag, &mobile_bookmarks_id)) { // We should always be able to find the permanent nodes. return false; } @@ -545,11 +535,10 @@ bool BookmarkModelAssociator::LoadAssociations() { BookmarkNodeIdIndex id_index; id_index.AddAll(bookmark_model_->bookmark_bar_node()); id_index.AddAll(bookmark_model_->other_node()); - id_index.AddAll(bookmark_model_->synced_node()); + id_index.AddAll(bookmark_model_->mobile_node()); std::stack<int64> dfs_stack; - if (synced_bookmarks_id != -1) - dfs_stack.push(synced_bookmarks_id); + dfs_stack.push(mobile_bookmarks_id); dfs_stack.push(other_bookmarks_id); dfs_stack.push(bookmark_bar_id); @@ -578,7 +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_->bookmark_bar_node() && - node != bookmark_model_->synced_node() && + node != bookmark_model_->mobile_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 da29f98..57bdf2f 100644 --- a/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc @@ -296,8 +296,6 @@ class ProfileSyncServiceBookmarkTest : public testing::Test { virtual void SetUp() { test_user_share_.SetUp(); - CommandLine::ForCurrentProcess()->AppendSwitch( - switches::kEnableSyncedBookmarksFolder); } virtual void TearDown() { @@ -465,7 +463,7 @@ class ProfileSyncServiceBookmarkTest : public testing::Test { const BookmarkNode* root = model_->root_node(); EXPECT_EQ(root->GetIndexOf(model_->bookmark_bar_node()), 0); EXPECT_EQ(root->GetIndexOf(model_->other_node()), 1); - EXPECT_EQ(root->GetIndexOf(model_->synced_node()), 2); + EXPECT_EQ(root->GetIndexOf(model_->mobile_node()), 2); std::stack<int64> stack; stack.push(bookmark_bar_id()); @@ -488,9 +486,9 @@ class ProfileSyncServiceBookmarkTest : public testing::Test { ExpectModelMatch(&trans); } - int64 synced_bookmarks_id() { + int64 mobile_bookmarks_id() { return - model_associator_->GetSyncIdFromChromeId(model_->synced_node()->id()); + model_associator_->GetSyncIdFromChromeId(model_->mobile_node()->id()); } int64 other_bookmarks_id() { @@ -526,7 +524,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, InitialState) { EXPECT_TRUE(other_bookmarks_id()); EXPECT_TRUE(bookmark_bar_id()); - EXPECT_TRUE(synced_bookmarks_id()); + EXPECT_TRUE(mobile_bookmarks_id()); ExpectModelMatch(); } @@ -554,9 +552,9 @@ TEST_F(ProfileSyncServiceBookmarkTest, BookmarkModelOperations) { ExpectSyncerNodeMatching(url2); ExpectModelMatch(); // Test addition. - const BookmarkNode* synced_folder = - model_->AddFolder(model_->synced_node(), 0, ASCIIToUTF16("pie")); - ExpectSyncerNodeMatching(synced_folder); + const BookmarkNode* mobile_folder = + model_->AddFolder(model_->mobile_node(), 0, ASCIIToUTF16("pie")); + ExpectSyncerNodeMatching(mobile_folder); ExpectModelMatch(); // Test modification. @@ -574,7 +572,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, BookmarkModelOperations) { ExpectModelMatch(); model_->Copy(url2, model_->bookmark_bar_node(), 0); ExpectModelMatch(); - model_->SetTitle(synced_folder, ASCIIToUTF16("strawberry")); + model_->SetTitle(mobile_folder, ASCIIToUTF16("strawberry")); ExpectModelMatch(); // Test deletion. @@ -585,7 +583,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, BookmarkModelOperations) { model_->Remove(folder2->parent(), folder2->parent()->GetIndexOf(folder2)); ExpectModelMatch(); - model_->Remove(model_->synced_node(), 0); + model_->Remove(model_->mobile_node(), 0); ExpectModelMatch(); } @@ -614,7 +612,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, ServerChangeProcessing) { "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); + mobile_bookmarks_id(), 0); sync_api::ChangeRecordList::const_iterator it; // The bookmark model shouldn't yet have seen any of the nodes of |adds|. @@ -643,7 +641,7 @@ 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"); + std::wstring u6_old_title = mods.ModifyTitle(u6, L"Mobile Folder A"); // Test that the property changes have not yet taken effect. ExpectBrowserNodeTitle(u2, u2_old_title); @@ -982,7 +980,7 @@ namespace { // | +-- dup // | +-- dupu2, http://www.dupu1.com/ // | -// +-- Synced bookmarks +// +-- Mobile bookmarks // |-- f5 // | |-- f5u1, http://www.f5u1.com/ // |-- f6 @@ -1036,7 +1034,7 @@ static TestData kDup2Children[] = { { L"dupu2", "http://www.dupu2.com/" }, }; -static TestData kSyncedBookmarkChildren[] = { +static TestData kMobileBookmarkChildren[] = { { L"f5", NULL }, { L"f6", NULL }, { L"u5", "http://www.u5.com/" }, @@ -1117,15 +1115,15 @@ 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)); + const BookmarkNode* mobile_bookmarks_node = model_->mobile_node(); + PopulateFromTestData(mobile_bookmarks_node, + kMobileBookmarkChildren, + arraysize(kMobileBookmarkChildren)); - ASSERT_GE(synced_bookmarks_node->child_count(), 3); - const BookmarkNode* f5_node = synced_bookmarks_node->GetChild(0); + ASSERT_GE(mobile_bookmarks_node->child_count(), 3); + const BookmarkNode* f5_node = mobile_bookmarks_node->GetChild(0); PopulateFromTestData(f5_node, kF5Children, arraysize(kF5Children)); - const BookmarkNode* f6_node = synced_bookmarks_node->GetChild(1); + const BookmarkNode* f6_node = mobile_bookmarks_node->GetChild(1); PopulateFromTestData(f6_node, kF6Children, arraysize(kF6Children)); ExpectBookmarkModelMatchesTestData(); @@ -1159,15 +1157,15 @@ void ProfileSyncServiceBookmarkTestWithData:: 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)); + const BookmarkNode* mobile_bookmarks_node = model_->mobile_node(); + CompareWithTestData(mobile_bookmarks_node, + kMobileBookmarkChildren, + arraysize(kMobileBookmarkChildren)); - ASSERT_GE(synced_bookmarks_node->child_count(), 3); - const BookmarkNode* f5_node = synced_bookmarks_node->GetChild(0); + ASSERT_GE(mobile_bookmarks_node->child_count(), 3); + const BookmarkNode* f5_node = mobile_bookmarks_node->GetChild(0); CompareWithTestData(f5_node, kF5Children, arraysize(kF5Children)); - const BookmarkNode* f6_node = synced_bookmarks_node->GetChild(1); + const BookmarkNode* f6_node = mobile_bookmarks_node->GetChild(1); CompareWithTestData(f6_node, kF6Children, arraysize(kF6Children)); } @@ -1236,7 +1234,7 @@ TEST_F(ProfileSyncServiceBookmarkTestWithData, MergeWithEmptyBookmarkModel) { LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE); EXPECT_EQ(model_->bookmark_bar_node()->child_count(), 0); EXPECT_EQ(model_->other_node()->child_count(), 0); - EXPECT_EQ(model_->synced_node()->child_count(), 0); + EXPECT_EQ(model_->mobile_node()->child_count(), 0); // Now restart the sync service. Starting it should populate the bookmark // model -- test for consistency. diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm index 86498e8..de93d0d 100644 --- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm +++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm @@ -537,11 +537,8 @@ void RecordAppLaunch(Profile* profile, GURL url) { } - (BOOL)canEditBookmark:(const BookmarkNode*)node { - // Don't allow edit/delete of the bar node, or of "Other Bookmarks" - if (node == nil || - node == bookmarkModel_->bookmark_bar_node() || - node == bookmarkModel_->other_node() || - node == bookmarkModel_->synced_node()) + // Don't allow edit/delete of the permanent nodes. + if (node == nil || bookmarkModel_->is_permanent_node(node)) return NO; return YES; } @@ -799,7 +796,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::MOBILE || type == BookmarkNode::FOLDER) { parent = senderNode; newIndex = parent->child_count(); diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller.mm index 28dcab6..f2dd831 100644 --- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller.mm +++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller.mm @@ -391,7 +391,7 @@ void BookmarkBubbleNotificationBridge::Observe( } for (int i = 0; i < parent->child_count(); i++) { const BookmarkNode* child = parent->GetChild(i); - if (child->is_folder() && child->IsVisible()) + if (child->is_folder()) [self addFolderNodes:child toPopUpButton:button indentation:indentation]; diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller_unittest.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller_unittest.mm index e2b50d3..b4739d2 100644 --- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller_unittest.mm +++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller_unittest.mm @@ -175,11 +175,7 @@ TEST_F(BookmarkBubbleControllerTest, TestFillInFolder) { // Verify that the top level folders are displayed correctly. EXPECT_TRUE([titles containsObject:@"Other Bookmarks"]); EXPECT_TRUE([titles containsObject:@"Bookmarks Bar"]); - if (model->synced_node()->IsVisible()) { - EXPECT_TRUE([titles containsObject:@"Synced Bookmarks"]); - } else { - EXPECT_FALSE([titles containsObject:@"Synced Bookmarks"]); - } + EXPECT_TRUE([titles containsObject:@"Mobile Bookmarks"]); } // Confirm ability to handle folders with blank name. diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_base_controller.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_base_controller.mm index bbf6628..605c4ea 100644 --- a/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_base_controller.mm +++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_base_controller.mm @@ -456,7 +456,7 @@ class BookmarkEditorBaseControllerBridge : public BookmarkModelObserver { int childCount = node->child_count(); for (int i = 0; i < childCount; ++i) { const BookmarkNode* childNode = node->GetChild(i); - if (childNode->is_folder() && childNode->IsVisible()) { + if (childNode->is_folder()) { NSString* childName = base::SysUTF16ToNSString(childNode->GetTitle()); NSMutableArray* children = [self addChildFoldersFromNode:childNode]; BookmarkFolderInfo* folderInfo = 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 a1ec34a..56fbe51 100644 --- a/chrome/browser/ui/gtk/bookmarks/bookmark_editor_gtk_unittest.cc +++ b/chrome/browser/ui/gtk/bookmarks/bookmark_editor_gtk_unittest.cc @@ -70,7 +70,7 @@ class BookmarkEditorGtkTest : public testing::Test { // oa // OF1 // of1a - // synced node + // mobile node // sa void AddTestData() { std::string test_base = base_path(); @@ -91,8 +91,8 @@ class BookmarkEditorGtkTest : public testing::Test { 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"), + // Children of the mobile node. + model_->AddURL(model_->mobile_node(), 0, ASCIIToUTF16("sa"), GURL(test_base + "sa")); } @@ -118,14 +118,8 @@ TEST_F(BookmarkEditorGtkTest, ModelsMatch) { GtkTreeIter bookmark_bar_node = toplevel; ASSERT_TRUE(gtk_tree_model_iter_next(store, &toplevel)); GtkTreeIter other_node = 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)); - } + ASSERT_TRUE(gtk_tree_model_iter_next(store, &toplevel)); + 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 7d86a47..89bc9bd 100644 --- a/chrome/browser/ui/gtk/bookmarks/bookmark_tree_model.cc +++ b/chrome/browser/ui/gtk/bookmarks/bookmark_tree_model.cc @@ -118,9 +118,7 @@ void AddToTreeStore(BookmarkModel* model, int64 selected_id, const BookmarkNode* root_node = model->root_node(); for (int i = 0; i < root_node->child_count(); ++i) { const BookmarkNode* child = root_node->GetChild(i); - if (child->IsVisible()) { - AddToTreeStoreAt(child, selected_id, store, selected_iter, NULL); - } + 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 fe3d337..bf394ab 100644 --- a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc +++ b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc @@ -535,12 +535,10 @@ BookmarkEditorView::EditorNode* BookmarkEditorView::CreateRootNode() { EditorNode* root_node = new EditorNode(string16(), 0); const BookmarkNode* bb_root_node = bb_model_->root_node(); CreateNodes(bb_root_node, root_node); - DCHECK(root_node->child_count() >= 2 && root_node->child_count() <= 3); + DCHECK_EQ(3, root_node->child_count()); 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); - } + DCHECK(bb_root_node->GetChild(2)->type() == BookmarkNode::MOBILE); return root_node; } @@ -548,7 +546,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->IsVisible() && child_bb_node->is_folder()) { + if (child_bb_node->is_folder()) { EditorNode* new_b_node = new EditorNode(child_bb_node->GetTitle(), child_bb_node->id()); b_node->Add(new_b_node, b_node->child_count()); 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 39d1dc1..6269a38 100644 --- a/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc +++ b/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc @@ -135,13 +135,7 @@ TEST_F(BookmarkEditorViewTest, ModelsMatch) { BookmarkEditor::EditDetails::AddNodeInFolder(NULL, -1), BookmarkEditorView::SHOW_TREE); BookmarkEditorView::EditorNode* editor_root = editor_tree_model()->GetRoot(); - // 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()); - } + 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. diff --git a/chrome/browser/ui/webui/ntp/bookmarks_handler.cc b/chrome/browser/ui/webui/ntp/bookmarks_handler.cc new file mode 100644 index 0000000..e91e644 --- /dev/null +++ b/chrome/browser/ui/webui/ntp/bookmarks_handler.cc @@ -0,0 +1,335 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/ui/webui/ntp/bookmarks_handler.h" + +#include "base/auto_reset.h" +#include "base/bind.h" +#include "base/bind_helpers.h" +#include "base/string_number_conversions.h" +#include "base/values.h" +#include "chrome/browser/bookmarks/bookmark_extension_api_constants.h" +#include "chrome/browser/bookmarks/bookmark_model.h" +#include "chrome/browser/prefs/pref_service.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/sync/profile_sync_service.h" +#include "chrome/browser/ui/webui/ntp/new_tab_ui.h" +#include "chrome/common/chrome_notification_types.h" +#include "chrome/common/pref_names.h" + +// TODO(csilv): +// Much of this implementation is based on the classes defined in +// extension_bookmarks_module.cc. Longer term we should consider migrating +// NTP into an embedded extension which would allow us to leverage the same +// bookmark APIs as the bookmark manager. + +namespace keys = bookmark_extension_api_constants; + +BookmarksHandler::BookmarksHandler() : model_(NULL), + dom_ready_(false), + from_current_page_(false) { +} + +BookmarksHandler::~BookmarksHandler() { + if (model_) + model_->RemoveObserver(this); +} + +WebUIMessageHandler* BookmarksHandler::Attach(WebUI* web_ui) { + WebUIMessageHandler::Attach(web_ui); + model_ = Profile::FromWebUI(web_ui)->GetBookmarkModel(); + if (model_) + model_->AddObserver(this); + return this; +} + +void BookmarksHandler::RegisterMessages() { + web_ui_->RegisterMessageCallback("createBookmark", + base::Bind(&BookmarksHandler::HandleCreateBookmark, + base::Unretained(this))); + web_ui_->RegisterMessageCallback("getBookmarksData", + base::Bind(&BookmarksHandler::HandleGetBookmarksData, + base::Unretained(this))); + web_ui_->RegisterMessageCallback("moveBookmark", + base::Bind(&BookmarksHandler::HandleMoveBookmark, + base::Unretained(this))); + web_ui_->RegisterMessageCallback("removeBookmark", + base::Bind(&BookmarksHandler::HandleRemoveBookmark, + base::Unretained(this))); +} + +void BookmarksHandler::Loaded(BookmarkModel* model, bool ids_reassigned) { + if (dom_ready_) + HandleGetBookmarksData(NULL); +} + +void BookmarksHandler::BookmarkModelBeingDeleted(BookmarkModel* model) { + // If this occurs it probably means that this tab will close shortly. + // Discard our reference to the model so that we won't use it again. + model_ = NULL; +} + +void BookmarksHandler::BookmarkNodeMoved(BookmarkModel* model, + const BookmarkNode* old_parent, + int old_index, + const BookmarkNode* new_parent, + int new_index) { + if (!dom_ready_) return; + const BookmarkNode* node = new_parent->GetChild(new_index); + base::StringValue id(base::Int64ToString(node->id())); + base::DictionaryValue move_info; + move_info.SetString(keys::kParentIdKey, + base::Int64ToString(new_parent->id())); + move_info.SetInteger(keys::kIndexKey, new_index); + move_info.SetString(keys::kOldParentIdKey, + base::Int64ToString(old_parent->id())); + move_info.SetInteger(keys::kOldIndexKey, old_index); + base::FundamentalValue from_page(from_current_page_); + + web_ui_->CallJavascriptFunction("ntp4.bookmarkNodeMoved", id, move_info, + from_page); +} + +void BookmarksHandler::BookmarkNodeAdded(BookmarkModel* model, + const BookmarkNode* parent, + int index) { + if (!dom_ready_) return; + const BookmarkNode* node = parent->GetChild(index); + base::StringValue id(base::Int64ToString(node->id())); + scoped_ptr<base::DictionaryValue> node_info(GetNodeDictionary(node)); + base::FundamentalValue from_page(from_current_page_); + + web_ui_->CallJavascriptFunction("ntp4.bookmarkNodeAdded", id, *node_info, + from_page); +} + +void BookmarksHandler::BookmarkNodeRemoved(BookmarkModel* model, + const BookmarkNode* parent, + int index, + const BookmarkNode* node) { + if (!dom_ready_) return; + + base::StringValue id(base::Int64ToString(node->id())); + base::DictionaryValue remove_info; + remove_info.SetString(keys::kParentIdKey, base::Int64ToString(parent->id())); + remove_info.SetInteger(keys::kIndexKey, index); + base::FundamentalValue from_page(from_current_page_); + + web_ui_->CallJavascriptFunction("ntp4.bookmarkNodeRemoved", id, remove_info, + from_page); +} + +void BookmarksHandler::BookmarkNodeChanged(BookmarkModel* model, + const BookmarkNode* node) { + if (!dom_ready_) return; + base::StringValue id(base::Int64ToString(node->id())); + base::DictionaryValue change_info; + change_info.SetString(keys::kTitleKey, node->GetTitle()); + if (node->is_url()) + change_info.SetString(keys::kUrlKey, node->url().spec()); + base::FundamentalValue from_page(from_current_page_); + + web_ui_->CallJavascriptFunction("ntp4.bookmarkNodeChanged", id, change_info, + from_page); +} + +void BookmarksHandler::BookmarkNodeFaviconChanged(BookmarkModel* model, + const BookmarkNode* node) { + // Favicons are handled by through use of the chrome://favicon protocol, so + // there's nothing for us to do here (but we need to provide an + // implementation). +} + +void BookmarksHandler::BookmarkNodeChildrenReordered(BookmarkModel* model, + const BookmarkNode* node) { + if (!dom_ready_) return; + base::StringValue id(base::Int64ToString(node->id())); + base::ListValue* children = new base::ListValue; + for (int i = 0; i < node->child_count(); ++i) { + const BookmarkNode* child = node->GetChild(i); + Value* child_id = new StringValue(base::Int64ToString(child->id())); + children->Append(child_id); + } + base::DictionaryValue reorder_info; + reorder_info.Set(keys::kChildIdsKey, children); + base::FundamentalValue from_page(from_current_page_); + + web_ui_->CallJavascriptFunction("ntp4.bookmarkNodeChildrenReordered", id, + reorder_info, from_page); +} + +void BookmarksHandler::BookmarkImportBeginning(BookmarkModel* model) { + if (dom_ready_) + web_ui_->CallJavascriptFunction("ntp4.bookmarkImportBegan"); +} + +void BookmarksHandler::BookmarkImportEnding(BookmarkModel* model) { + if (dom_ready_) + web_ui_->CallJavascriptFunction("ntp4.bookmarkImportEnded"); +} + +void BookmarksHandler::HandleCreateBookmark(const ListValue* args) { + if (!model_) return; + + // This is the only required argument - a stringified int64 parent ID. + std::string parent_id_string; + CHECK(args->GetString(0, &parent_id_string)); + int64 parent_id; + CHECK(base::StringToInt64(parent_id_string, &parent_id)); + + const BookmarkNode* parent = model_->GetNodeByID(parent_id); + if (!parent) return; + + double index; + if (!args->GetDouble(1, &index) || + (index > parent->child_count() || index < 0)) { + index = parent->child_count(); + } + + // If they didn't pass the argument, just use a blank string. + string16 title; + if (!args->GetString(2, &title)) + title = string16(); + + // We'll be creating either a bookmark or a folder, so set this for both. + AutoReset<bool> from_page(&from_current_page_, true); + + // According to the bookmarks API, "If url is NULL or missing, it will be a + // folder.". Let's just follow the same behavior. + std::string url_string; + if (args->GetString(3, &url_string)) { + GURL url(url_string); + if (!url.is_empty() && url.is_valid()) { + // Only valid case for a bookmark as opposed to folder. + model_->AddURL(parent, static_cast<int>(index), title, url); + return; + } + } + + // We didn't have all the valid parts for a bookmark, just make a folder. + model_->AddFolder(parent, static_cast<int>(index), title); +} + +void BookmarksHandler::HandleGetBookmarksData(const base::ListValue* args) { + dom_ready_ = true; + + // At startup, Bookmarks may not be fully loaded. If this is the case, + // we'll wait for the notification to arrive. + Profile* profile = Profile::FromWebUI(web_ui_); + BookmarkModel* model = profile->GetBookmarkModel(); + if (!model->IsLoaded()) return; + + int64 id; + std::string id_string; + PrefService* prefs = profile->GetPrefs(); + if (args && args->GetString(0, &id_string) && + base::StringToInt64(id_string, &id)) { + // A folder ID was requested, so persist this value. + prefs->SetInt64(prefs::kNTPShownBookmarksFolder, id); + } else { + // No folder ID was requested, so get the default (persisted) value. + id = prefs->GetInt64(prefs::kNTPShownBookmarksFolder); + } + + const BookmarkNode* node = model->GetNodeByID(id); + if (!node) + node = model->root_node(); + + // We wish to merge the root node with the bookmarks bar node. + if (model->is_root_node(node)) + node = model->bookmark_bar_node(); + + base::ListValue* items = new base::ListValue; + for (int i = 0; i < node->child_count(); ++i) { + const BookmarkNode* child = node->GetChild(i); + AddNode(child, items); + } + if (node == model->bookmark_bar_node() && model->other_node()->child_count()) + AddNode(model->other_node(), items); + + base::ListValue* navigation_items = new base::ListValue; + while (node) { + if (node != model->bookmark_bar_node()) + AddNode(node, navigation_items); + node = node->parent(); + } + + base::DictionaryValue bookmarksData; + bookmarksData.Set("items", items); + bookmarksData.Set("navigationItems", navigation_items); + web_ui_->CallJavascriptFunction("ntp4.setBookmarksData", bookmarksData); +} + +void BookmarksHandler::HandleRemoveBookmark(const ListValue* args) { + if (!model_) return; + + std::string id_string; + CHECK(args->GetString(0, &id_string)); + int64 id; + CHECK(base::StringToInt64(id_string, &id)); + + const BookmarkNode* node = model_->GetNodeByID(id); + if (!node) return; + + AutoReset<bool> from_page(&from_current_page_, true); + model_->Remove(node->parent(), node->parent()->GetIndexOf(node)); +} + +void BookmarksHandler::HandleMoveBookmark(const ListValue* args) { + if (!model_) return; + + std::string id_string; + CHECK(args->GetString(0, &id_string)); + int64 id; + CHECK(base::StringToInt64(id_string, &id)); + + std::string parent_id_string; + CHECK(args->GetString(1, &parent_id_string)); + int64 parent_id; + CHECK(base::StringToInt64(parent_id_string, &parent_id)); + + double index; + args->GetDouble(2, &index); + + const BookmarkNode* parent = model_->GetNodeByID(parent_id); + if (!parent) return; + + const BookmarkNode* node = model_->GetNodeByID(id); + if (!node) return; + + AutoReset<bool> from_page(&from_current_page_, true); + model_->Move(node, parent, static_cast<int>(index)); +} + +base::DictionaryValue* BookmarksHandler::GetNodeDictionary( + const BookmarkNode* node) { + base::DictionaryValue* dict = new base::DictionaryValue(); + dict->SetString(keys::kIdKey, base::Int64ToString(node->id())); + + const BookmarkNode* parent = node->parent(); + if (parent) { + dict->SetString(keys::kParentIdKey, base::Int64ToString(parent->id())); + dict->SetInteger(keys::kIndexKey, parent->GetIndexOf(node)); + } + + NewTabUI::SetURLTitleAndDirection(dict, node->GetTitle(), node->url()); + + if (!node->is_folder()) + dict->SetString(keys::kUrlKey, node->url().spec()); + + return dict; +} + +void BookmarksHandler::AddNode(const BookmarkNode* node, + base::ListValue* list) { + list->Append(GetNodeDictionary(node)); +} + +// static +void BookmarksHandler::RegisterUserPrefs(PrefService* prefs) { + // Default folder is the root node. + // TODO(csilv): Should we sync this preference? + prefs->RegisterInt64Pref(prefs::kNTPShownBookmarksFolder, 0, + PrefService::UNSYNCABLE_PREF); +} diff --git a/chrome/test/data/extensions/api_test/bookmark_manager/edit_disabled/test.js b/chrome/test/data/extensions/api_test/bookmark_manager/edit_disabled/test.js index 8d86a6c..a048a7e 100644 --- a/chrome/test/data/extensions/api_test/bookmark_manager/edit_disabled/test.js +++ b/chrome/test/data/extensions/api_test/bookmark_manager/edit_disabled/test.js @@ -22,13 +22,14 @@ var ERROR = "Bookmark editing is disabled."; // "BBB" // "AAA" // Other Bookmarks/ +// Mobile Bookmarks/ var tests = [ function verifyModel() { bookmarks.getTree(pass(function(result) { assertEq(1, result.length); var root = result[0]; - assertEq(2, root.children.length); + assertEq(3, root.children.length); bar = root.children[0]; assertEq(2, bar.children.length); folder = bar.children[0]; diff --git a/net/tools/testserver/chromiumsync.py b/net/tools/testserver/chromiumsync.py index cabbb7d..07497ea 100755 --- a/net/tools/testserver/chromiumsync.py +++ b/net/tools/testserver/chromiumsync.py @@ -394,6 +394,8 @@ class SyncDataModel(object): parent_tag='google_chrome_bookmarks', sync_type=BOOKMARK), PermanentItem('other_bookmarks', name='Other Bookmarks', parent_tag='google_chrome_bookmarks', sync_type=BOOKMARK), + PermanentItem('synced_bookmarks', name='Mobile Bookmarks', + parent_tag='google_chrome_bookmarks', sync_type=BOOKMARK), PermanentItem('google_chrome_preferences', name='Preferences', parent_tag='google_chrome', sync_type=PREFERENCE), PermanentItem('google_chrome_autofill', name='Autofill', diff --git a/ui/base/models/tree_node_iterator.h b/ui/base/models/tree_node_iterator.h index 5a05df9..26d6dea 100644 --- a/ui/base/models/tree_node_iterator.h +++ b/ui/base/models/tree_node_iterator.h @@ -23,27 +23,7 @@ namespace ui { template <class NodeType> class TreeNodeIterator { public: - // This contructor accepts an optional filter function |prune| which could be - // used to prune complete branches of the tree. The filter function will be - // evaluated on each tree node and if it evaluates to true the node and all - // its descendants will be skipped by the iterator. - TreeNodeIterator(NodeType* node, bool (*prune)(NodeType*)) - : prune_(prune) { - int index = 0; - - // Move forward through the children list until the first non prunable node. - // This is to satisfy the iterator invariant that the current index in the - // Position at the top of the _positions list must point to a node the - // iterator will be returning. - for (; index < node->child_count(); ++index) - if (!prune || !prune(node->GetChild(index))) - break; - - if (index < node->child_count()) - positions_.push(Position<NodeType>(node, index)); - } - - explicit TreeNodeIterator(NodeType* node) : prune_(NULL) { + explicit TreeNodeIterator(NodeType* node) { if (!node->empty()) positions_.push(Position<NodeType>(node, 0)); } @@ -67,18 +47,9 @@ class TreeNodeIterator { // Iterate over result's children. positions_.push(Position<NodeType>(result, 0)); - // Advance to next valid node by skipping over the pruned nodes and the - // empty Positions. At the end of this loop two cases are possible: - // - the current index of the top() Position points to a valid node - // - the _position list is empty, the iterator has_next() will return false. - while (!positions_.empty()) { - if (positions_.top().index >= positions_.top().node->child_count()) - positions_.pop(); // This Position is all processed, move to the next. - else if (prune_ && - prune_(positions_.top().node->GetChild(positions_.top().index))) - positions_.top().index++; // Prune the branch. - else - break; // Now positioned at the next node to be returned. + while (!positions_.empty() && + positions_.top().index >= positions_.top().node->child_count()) { + positions_.pop(); // This Position is all processed, move to the next. } return result; @@ -95,7 +66,6 @@ class TreeNodeIterator { }; std::stack<Position<NodeType> > positions_; - bool (*prune_)(NodeType*); DISALLOW_COPY_AND_ASSIGN(TreeNodeIterator); }; diff --git a/ui/base/models/tree_node_iterator_unittest.cc b/ui/base/models/tree_node_iterator_unittest.cc index 0aefe4d..13140e9 100644 --- a/ui/base/models/tree_node_iterator_unittest.cc +++ b/ui/base/models/tree_node_iterator_unittest.cc @@ -38,44 +38,4 @@ TEST(TreeNodeIteratorTest, Test) { ASSERT_FALSE(iterator.has_next()); } -static bool PruneOdd(TreeNodeWithValue<int>* node) { - return node->value % 2; -} - -static bool PruneEven(TreeNodeWithValue<int>* node) { - return !(node->value % 2); -} - -// The tree used for testing: -// * + 1 -// + 2 -// + 3 + 4 + 5 -// + 7 - -TEST(TreeNodeIteratorPruneTest, Test) { - TreeNodeWithValue<int> root; - root.Add(new TreeNodeWithValue<int>(1), 0); - root.Add(new TreeNodeWithValue<int>(2), 1); - TreeNodeWithValue<int>* f3 = new TreeNodeWithValue<int>(3); - root.Add(f3, 2); - TreeNodeWithValue<int>* f4 = new TreeNodeWithValue<int>(4); - f3->Add(f4, 0); - f4->Add(new TreeNodeWithValue<int>(5), 0); - f3->Add(new TreeNodeWithValue<int>(7), 1); - - TreeNodeIterator<TreeNodeWithValue<int> > oddIterator(&root, PruneOdd); - ASSERT_TRUE(oddIterator.has_next()); - ASSERT_EQ(2, oddIterator.Next()->value); - ASSERT_FALSE(oddIterator.has_next()); - - TreeNodeIterator<TreeNodeWithValue<int> > evenIterator(&root, PruneEven); - ASSERT_TRUE(evenIterator.has_next()); - ASSERT_EQ(1, evenIterator.Next()->value); - ASSERT_TRUE(evenIterator.has_next()); - ASSERT_EQ(3, evenIterator.Next()->value); - ASSERT_TRUE(evenIterator.has_next()); - ASSERT_EQ(7, evenIterator.Next()->value); - ASSERT_FALSE(evenIterator.has_next()); -} - } // namespace ui |