Adds back BookmarkNode::IsVisible. Turns out we want to conditionally

show a node, so we need it. *SIGH*

For the most part this is a revert of
http://codereview.chromium.org/8759017/, so don't feel you need to
review it that deeply.

BUG=102714 
TEST=none
TBR=ben@chromium.org

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

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

18 files changed, 201 insertions, 369 deletions

diff --git a/chrome/browser/bookmarks/bookmark_extension_helpers.cc b/chrome/browser/bookmarks/bookmark_extension_helpers.cc
index cb44c0a..a9873be 100644
--- a/chrome/browser/bookmarks/bookmark_extension_helpers.cc
+++ b/chrome/browser/bookmarks/bookmark_extension_helpers.cc
@@ -17,9 +17,11 @@ void AddNode(const BookmarkNode* node,
              base::ListValue* list,
              bool recurse,
              bool only_folders) {
-  base::DictionaryValue* dict = bookmark_extension_helpers::GetNodeDictionary(
-      node, recurse, only_folders);
-  list->Append(dict);
+  if (node->IsVisible()) {
+    base::DictionaryValue* dict = bookmark_extension_helpers::GetNodeDictionary(
+        node, recurse, only_folders);
+    list->Append(dict);
+  }
 }
 
 }  // namespace
@@ -59,7 +61,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 (!only_folders || child->is_folder()) {
+      if (child->IsVisible() && (!only_folders || child->is_folder())) {
         DictionaryValue* dict = GetNodeDictionary(child, true, only_folders);
         children->Append(dict);
       }
diff --git a/chrome/browser/bookmarks/bookmark_model.cc b/chrome/browser/bookmarks/bookmark_model.cc
index c45f87b..b0dfada 100644
--- a/chrome/browser/bookmarks/bookmark_model.cc
+++ b/chrome/browser/bookmarks/bookmark_model.cc
@@ -55,6 +55,10 @@ 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;
@@ -68,8 +72,6 @@ void BookmarkNode::InvalidateFavicon() {
   is_favicon_loaded_ = false;
 }
 
-// BookmarkModel --------------------------------------------------------------
-
 namespace {
 
 // Comparator used when sorting bookmarks. Folders are sorted first, then
@@ -97,8 +99,40 @@ class SortComparator : public std::binary_function<const BookmarkNode*,
   icu::Collator* collator_;
 };
 
+// MobileNode ------------------------------------------------------------------
+
+// The visibility of the mobile node changes based on sync state, requiring a
+// special subclass.
+class MobileNode : public BookmarkNode {
+ public:
+  explicit MobileNode(int64 id);
+  virtual ~MobileNode();
+
+  // BookmarkNode overrides:
+  virtual bool IsVisible() const OVERRIDE;
+
+ private:
+  bool visible_;
+
+  DISALLOW_COPY_AND_ASSIGN(MobileNode);
+};
+
+MobileNode::MobileNode(int64 id)
+    : BookmarkNode(id, GURL()),
+      visible_(false) {
+}
+
+MobileNode::~MobileNode() {
+}
+
+bool MobileNode::IsVisible() const {
+  return visible_ || !empty();
+}
+
 }  // namespace
 
+// BookmarkModel --------------------------------------------------------------
+
 BookmarkModel::BookmarkModel(Profile* profile)
     : profile_(profile),
       loaded_(false),
@@ -703,17 +737,23 @@ BookmarkNode* BookmarkModel::CreatePermanentNode(BookmarkNode::Type type) {
   DCHECK(type == BookmarkNode::BOOKMARK_BAR ||
          type == BookmarkNode::OTHER_NODE ||
          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));
-  } else if (type == BookmarkNode::OTHER_NODE) {
-    node->set_title(
-        l10n_util::GetStringUTF16(IDS_BOOKMARK_BAR_OTHER_FOLDER_NAME));
-  } else {
+  BookmarkNode* node;
+  if (type == BookmarkNode::MOBILE) {
+    node = new MobileNode(generate_next_node_id());
     node->set_title(
         l10n_util::GetStringUTF16(IDS_BOOKMARK_BAR_MOBILE_FOLDER_NAME));
+  } else {
+    node = new BookmarkNode(generate_next_node_id(), GURL());
+    if (type == BookmarkNode::BOOKMARK_BAR) {
+      node->set_title(l10n_util::GetStringUTF16(IDS_BOOKMARK_BAR_FOLDER_NAME));
+    } else if (type == BookmarkNode::OTHER_NODE) {
+      node->set_title(
+          l10n_util::GetStringUTF16(IDS_BOOKMARK_BAR_OTHER_FOLDER_NAME));
+    } else {
+      NOTREACHED();
+    }
   }
+  node->set_type(type);
   return node;
 }
 
diff --git a/chrome/browser/bookmarks/bookmark_model.h b/chrome/browser/bookmarks/bookmark_model.h
index 4b242c2..e6d7c8f 100644
--- a/chrome/browser/bookmarks/bookmark_model.h
+++ b/chrome/browser/bookmarks/bookmark_model.h
@@ -110,6 +110,13 @@ 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.
+  virtual bool IsVisible() const;
+
   // TODO(sky): Consider adding last visit time here, it'll greatly simplify
   // HistoryContentsProvider.
 
diff --git a/chrome/browser/bookmarks/bookmark_model_unittest.cc b/chrome/browser/bookmarks/bookmark_model_unittest.cc
index d4bf5bc..b3bfb52 100644
--- a/chrome/browser/bookmarks/bookmark_model_unittest.cc
+++ b/chrome/browser/bookmarks/bookmark_model_unittest.cc
@@ -1081,4 +1081,27 @@ 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());
+  // Mobile node invisible by default
+  EXPECT_FALSE(model_.mobile_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, MobileNodeVisibileWithChildren) {
+  const BookmarkNode* root = model_.mobile_node();
+  const string16 title(ASCIIToUTF16("foo"));
+  const GURL url("http://foo.com");
+
+  model_.AddURL(root, 0, title, url);
+  EXPECT_TRUE(model_.mobile_node()->IsVisible());
+}
+
 }  // namespace
diff --git a/chrome/browser/bookmarks/bookmark_utils.cc b/chrome/browser/bookmarks/bookmark_utils.cc
index 7515017..3d4b806 100644
--- a/chrome/browser/bookmarks/bookmark_utils.cc
+++ b/chrome/browser/bookmarks/bookmark_utils.cc
@@ -468,11 +468,17 @@ 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());
+  ui::TreeNodeIterator<const BookmarkNode>
+      iterator(model->root_node(), PruneInvisibleFolders);
 
   while (iterator.has_next()) {
     const BookmarkNode* parent = iterator.Next();
@@ -500,7 +506,8 @@ std::vector<const BookmarkNode*> GetMostRecentlyModifiedFolders(
 
     for (int i = 0; i < root_node->child_count(); ++i) {
       const BookmarkNode* node = root_node->GetChild(i);
-      if (find(nodes.begin(), nodes.end(), node) == nodes.end()) {
+      if (node->IsVisible() &&
+          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 79c8463..ecf2dfea 100644
--- a/chrome/browser/bookmarks/recently_used_folders_combo_model.cc
+++ b/chrome/browser/bookmarks/recently_used_folders_combo_model.cc
@@ -43,7 +43,8 @@ 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());
-  nodes_.push_back(model->mobile_node());
+  if (model->mobile_node()->IsVisible())
+    nodes_.push_back(model->mobile_node());
 
   std::vector<const BookmarkNode*>::iterator it = std::find(nodes_.begin(),
                                                             nodes_.end(),
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller.mm
index 681d91e..10397ca 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller.mm
@@ -403,7 +403,7 @@ void BookmarkBubbleNotificationBridge::Observe(
   }
   for (int i = 0; i < parent->child_count(); i++) {
     const BookmarkNode* child = parent->GetChild(i);
-    if (child->is_folder())
+    if (child->is_folder() && child->IsVisible())
       [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 b4739d2..16c7fc0 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller_unittest.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller_unittest.mm
@@ -175,7 +175,11 @@ 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"]);
-  EXPECT_TRUE([titles containsObject:@"Mobile Bookmarks"]);
+  if (model->mobile_node()->IsVisible()) {
+    EXPECT_TRUE([titles containsObject:@"Mobile Bookmarks"]);
+  } else {
+    EXPECT_FALSE([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 5a56592..a79c7c6 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()) {
+    if (childNode->is_folder() && childNode->IsVisible()) {
       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 56fbe51..5c31943 100644
--- a/chrome/browser/ui/gtk/bookmarks/bookmark_editor_gtk_unittest.cc
+++ b/chrome/browser/ui/gtk/bookmarks/bookmark_editor_gtk_unittest.cc
@@ -118,8 +118,14 @@ 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));
-  ASSERT_FALSE(gtk_tree_model_iter_next(store, &toplevel));
+  if (model_->mobile_node()->IsVisible()) {
+    // If we have a mobile node, then the iterator should find one element after
+    // "other bookmarks"
+    ASSERT_TRUE(gtk_tree_model_iter_next(store, &toplevel));
+    ASSERT_FALSE(gtk_tree_model_iter_next(store, &toplevel));
+  } else {
+    ASSERT_FALSE(gtk_tree_model_iter_next(store, &toplevel));
+  }
 
   // The bookmark bar should have 2 nodes: folder F1 and F2.
   GtkTreeIter f1_iter;
diff --git a/chrome/browser/ui/gtk/bookmarks/bookmark_tree_model.cc b/chrome/browser/ui/gtk/bookmarks/bookmark_tree_model.cc
index 89bc9bd..04a9d79 100644
--- a/chrome/browser/ui/gtk/bookmarks/bookmark_tree_model.cc
+++ b/chrome/browser/ui/gtk/bookmarks/bookmark_tree_model.cc
@@ -118,7 +118,8 @@ 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);
-    AddToTreeStoreAt(child, selected_id, store, selected_iter, NULL);
+    if (child->IsVisible())
+      AddToTreeStoreAt(child, selected_id, store, selected_iter, NULL);
   }
 }
 
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
index bf394ab..fd7924d 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
@@ -535,10 +535,11 @@ 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_EQ(3, root_node->child_count());
+  DCHECK(root_node->child_count() >= 2 && root_node->child_count() <= 3);
   DCHECK(bb_root_node->GetChild(0)->type() == BookmarkNode::BOOKMARK_BAR);
   DCHECK(bb_root_node->GetChild(1)->type() == BookmarkNode::OTHER_NODE);
-  DCHECK(bb_root_node->GetChild(2)->type() == BookmarkNode::MOBILE);
+  if (root_node->child_count() == 3)
+    DCHECK(bb_root_node->GetChild(2)->type() == BookmarkNode::MOBILE);
   return root_node;
 }
 
@@ -546,7 +547,7 @@ void BookmarkEditorView::CreateNodes(const BookmarkNode* bb_node,
                                      BookmarkEditorView::EditorNode* b_node) {
   for (int i = 0; i < bb_node->child_count(); ++i) {
     const BookmarkNode* child_bb_node = bb_node->GetChild(i);
-    if (child_bb_node->is_folder()) {
+    if (child_bb_node->IsVisible() && child_bb_node->is_folder()) {
       EditorNode* new_b_node = new EditorNode(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 6269a38..f3086b4 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc
@@ -135,7 +135,13 @@ TEST_F(BookmarkEditorViewTest, ModelsMatch) {
                BookmarkEditor::EditDetails::AddNodeInFolder(NULL, -1),
                BookmarkEditorView::SHOW_TREE);
   BookmarkEditorView::EditorNode* editor_root = editor_tree_model()->GetRoot();
-  ASSERT_EQ(3, editor_root->child_count());
+  // The root should have two or three children: bookmark bar, other bookmarks
+  // and conditionally mobile bookmarks.
+  if (model_->mobile_node()->IsVisible()) {
+    ASSERT_EQ(3, editor_root->child_count());
+  } else {
+    ASSERT_EQ(2, editor_root->child_count());
+  }
 
   BookmarkEditorView::EditorNode* bb_node = editor_root->GetChild(0);
   // The root should have 2 nodes: folder F1 and F2.
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc b/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc
index 34eddb0..cfac24a 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc
@@ -423,7 +423,7 @@ void BookmarkMenuDelegate::BuildMenuForPermanentNode(
     MenuItemView* menu,
     int* next_menu_id,
     bool* added_separator) {
-  if (node->GetTotalNodeCount() == 1)
+  if (!node->IsVisible() || node->GetTotalNodeCount() == 1)
     return;  // No children, don't create a menu.
 
   if (!*added_separator) {
diff --git a/chrome/browser/ui/webui/ntp/bookmarks_handler.cc b/chrome/browser/ui/webui/ntp/bookmarks_handler.cc
deleted file mode 100644
index e91e644..0000000
--- a/chrome/browser/ui/webui/ntp/bookmarks_handler.cc
+++ /dev/null
@@ -1,335 +0,0 @@
-// 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 a048a7e..8d86a6c 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,14 +22,13 @@ 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(3, root.children.length);
+      assertEq(2, root.children.length);
       bar = root.children[0];
       assertEq(2, bar.children.length);
       folder = bar.children[0];
diff --git a/ui/base/models/tree_node_iterator.h b/ui/base/models/tree_node_iterator.h
index 26d6dea..5a05df9 100644
--- a/ui/base/models/tree_node_iterator.h
+++ b/ui/base/models/tree_node_iterator.h
@@ -23,7 +23,27 @@ namespace ui {
 template <class NodeType>
 class TreeNodeIterator {
  public:
-  explicit TreeNodeIterator(NodeType* node) {
+  // 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) {
     if (!node->empty())
       positions_.push(Position<NodeType>(node, 0));
   }
@@ -47,9 +67,18 @@ class TreeNodeIterator {
     // Iterate over result's children.
     positions_.push(Position<NodeType>(result, 0));
 
-    while (!positions_.empty() &&
-           positions_.top().index >= positions_.top().node->child_count()) {
-      positions_.pop(); // This Position is all processed, move to the next.
+    // 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.
     }
 
     return result;
@@ -66,6 +95,7 @@ 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 13140e9..0aefe4d 100644
--- a/ui/base/models/tree_node_iterator_unittest.cc
+++ b/ui/base/models/tree_node_iterator_unittest.cc
@@ -38,4 +38,44 @@ 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