Eliminate the extra separator when there are no bookmarks in the bookmarks bar.

BUG=27795
TEST=Start with a fresh Chrome or else remove all bookmarks.  Bring up the Bookmarks menu.  Observe that there is a single separator above the Other Bookmarks menu item.  Add a bookmark to the bar.  Bring up the Bookmarks menu.  Observe that there is now a new bookmark item followed by a new separator.  Remove the new bookmark and repeat the observation noting that both the bookmark and the separator have been removed.

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

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

2 files changed, 27 insertions, 3 deletions

diff --git a/chrome/browser/cocoa/bookmark_menu_bridge.mm b/chrome/browser/cocoa/bookmark_menu_bridge.mm
index dd06da6..3de295d 100644
--- a/chrome/browser/cocoa/bookmark_menu_bridge.mm
+++ b/chrome/browser/cocoa/bookmark_menu_bridge.mm
@@ -46,9 +46,12 @@ void BookmarkMenuBridge::Loaded(BookmarkModel* model) {
 
   ClearBookmarkMenu(bookmark_menu);
 
-  // Add bookmark bar items
-  [bookmark_menu addItem:[NSMenuItem separatorItem]];
-  AddNodeToMenu(model->GetBookmarkBarNode(), bookmark_menu);
+  // Add bookmark bar items, if any.
+  const BookmarkNode* barNode = model->GetBookmarkBarNode();
+  if (barNode->GetChildCount()) {
+    [bookmark_menu addItem:[NSMenuItem separatorItem]];
+    AddNodeToMenu(model->GetBookmarkBarNode(), bookmark_menu);
+  }
 
   // Create a submenu for "other bookmarks", and fill it in.
   NSString* other_items_title = base::SysWideToNSString(
diff --git a/chrome/browser/cocoa/bookmark_menu_bridge_unittest.mm b/chrome/browser/cocoa/bookmark_menu_bridge_unittest.mm
index 7179c4e..0e84d36 100644
--- a/chrome/browser/cocoa/bookmark_menu_bridge_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_menu_bridge_unittest.mm
@@ -68,6 +68,27 @@ class BookmarkMenuBridgeTest : public PlatformTest {
   scoped_ptr<TestBookmarkMenuBridge> bridge_;
 };
 
+TEST_F(BookmarkMenuBridgeTest, TestBookmarkMenuAutoSeparator) {
+  BookmarkModel* model = bridge_->GetBookmarkModel();
+  bridge_->Loaded(model);
+  NSMenu* menu = bridge_->menu_.get();
+  // The bare menu after loading has a separator and an "Other Bookmarks"
+  // submenu.
+  EXPECT_EQ(2, [menu numberOfItems]);
+  // Add a bookmark and reload and there should be 4 items: the previous
+  // menu contents plus a new separator and the new bookmark.
+  const BookmarkNode* parent = model->GetBookmarkBarNode();
+  const char* url = "http://www.zim-bop-a-dee.com/";
+  std::wstring title(L"Bookmark");
+  model->AddURL(parent, 0,  title, GURL(url));
+  bridge_->Loaded(model);
+  EXPECT_EQ(4, [menu numberOfItems]);
+  // Remove the new bookmark and reload and we should have 2 items again
+  // because the separator should have been removed as well.
+  model->Remove(parent, 0);
+  bridge_->Loaded(model);
+  EXPECT_EQ(2, [menu numberOfItems]);
+}
 
 // Test that ClearBookmarkMenu() removes all bookmark menus.
 TEST_F(BookmarkMenuBridgeTest, TestClearBookmarkMenu) {