Bookmarks item of menubar didn't have following features on each folder,

 - Open All Bookmarks
 - Open All Bookmarks in New Window
 - Open All Bookmarks in Incognito Window

BUG=56106
TEST=modify and add new tests into unit_tests. changes pass unit_tests
Review URL: http://codereview.chromium.org/6508017

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

6 files changed, 230 insertions, 17 deletions

diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.h b/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.h
index db64b2c..37a4a2c 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.h
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.h
@@ -80,11 +80,20 @@ class BookmarkMenuBridge : public BookmarkModelObserver {
                         const BookmarkNode* node,
                         NSString* title);
 
-  // Helper for recursively adding items to our bookmark menu
+  // Helper for recursively adding items to our bookmark menu.
   // All children of |node| will be added to |menu|.
   // TODO(jrg): add a counter to enforce maximum nodes added
   void AddNodeToMenu(const BookmarkNode* node, NSMenu* menu);
 
+  // Helper for adding an item to our bookmark menu. An item which has a
+  // localized title specified by |message_id| will be added to |menu|.
+  // The item is also bound to |node| by tag. |command_id| selects the action.
+  void AddItemToMenu(int command_id,
+                     int message_id,
+                     const BookmarkNode* node,
+                     NSMenu* menu,
+                     bool enabled);
+
   // This configures an NSMenuItem with all the data from a BookmarkNode. This
   // is used to update existing menu items, as well as to configure newly
   // created ones, like in AddNodeToMenu().
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.mm
index f5ac98a..142bad0 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.mm
@@ -6,6 +6,7 @@
 
 #include "app/mac/nsimage_cache.h"
 #include "base/sys_string_conversions.h"
+#include "chrome/app/chrome_command_ids.h"
 #import "chrome/browser/app_controller_mac.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/profiles/profile.h"
@@ -145,16 +146,18 @@ Profile* BookmarkMenuBridge::GetProfile() {
 
 void BookmarkMenuBridge::ClearBookmarkMenu(NSMenu* menu) {
   bookmark_nodes_.clear();
-  // Recursively delete all menus that look like a bookmark.  Assume
-  // all items with submenus contain only bookmarks.  Also delete all
-  // separator items since we explicirly add them back in. This should
-  // deletes everything except the first item ("Add Bookmark...").
+  // Recursively delete all menus that look like a bookmark. Also delete all
+  // separator items since we explicitly add them back in. This deletes
+  // everything except the first item ("Add Bookmark...").
   NSArray* items = [menu itemArray];
   for (NSMenuItem* item in items) {
     // Convention: items in the bookmark list which are bookmarks have
     // an action of openBookmarkMenuItem:.  Also, assume all items
     // with submenus are submenus of bookmarks.
     if (([item action] == @selector(openBookmarkMenuItem:)) ||
+        ([item action] == @selector(openAllBookmarks:)) ||
+        ([item action] == @selector(openAllBookmarksNewWindow:)) ||
+        ([item action] == @selector(openAllBookmarksIncognitoWindow:)) ||
         [item hasSubmenu] ||
         [item isSeparatorItem]) {
       // This will eventually [obj release] all its kids, if it has
@@ -207,6 +210,46 @@ void BookmarkMenuBridge::AddNodeToMenu(const BookmarkNode* node, NSMenu* menu) {
       ConfigureMenuItem(child, item, false);
     }
   }
+
+  // Add menus for 'Open All Bookmarks'.
+  [menu addItem:[NSMenuItem separatorItem]];
+  bool enabled = child_count != 0;
+  AddItemToMenu(IDC_BOOKMARK_BAR_OPEN_ALL,
+                IDS_BOOMARK_BAR_OPEN_ALL,
+                node, menu, enabled);
+  AddItemToMenu(IDC_BOOKMARK_BAR_OPEN_ALL_NEW_WINDOW,
+                IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW,
+                node, menu, enabled);
+  AddItemToMenu(IDC_BOOKMARK_BAR_OPEN_ALL_INCOGNITO,
+                IDS_BOOMARK_BAR_OPEN_INCOGNITO,
+                node, menu, enabled);
+}
+
+void BookmarkMenuBridge::AddItemToMenu(int command_id,
+                                       int message_id,
+                                       const BookmarkNode* node,
+                                       NSMenu* menu,
+                                       bool enabled) {
+  NSString* title = l10n_util::GetNSString(message_id);
+  SEL action;
+  if (!enabled) {
+    // A nil action makes a menu item appear disabled. NSMenuItem setEnabled
+    // will not reflect the disabled state until the item title is set again.
+    action = nil;
+  } else if (command_id == IDC_BOOKMARK_BAR_OPEN_ALL) {
+    action = @selector(openAllBookmarks:);
+  } else if (command_id == IDC_BOOKMARK_BAR_OPEN_ALL_NEW_WINDOW) {
+    action = @selector(openAllBookmarksNewWindow:);
+  } else {
+    action = @selector(openAllBookmarksIncognitoWindow:);
+  }
+  NSMenuItem* item = [[[NSMenuItem alloc] initWithTitle:title
+                                                 action:action
+                                          keyEquivalent:@""] autorelease];
+  [item setTarget:controller_];
+  [item setTag:node->id()];
+  [item setEnabled:enabled];
+  [menu addItem:item];
 }
 
 void BookmarkMenuBridge::ConfigureMenuItem(const BookmarkNode* node,
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge_unittest.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge_unittest.mm
index 87fcb34..c4ea6a6 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge_unittest.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge_unittest.mm
@@ -12,9 +12,11 @@
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.h"
 #include "chrome/browser/ui/cocoa/browser_test_helper.h"
+#include "grit/generated_resources.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #import "testing/gtest_mac.h"
 #include "testing/platform_test.h"
+#include "ui/base/l10n/l10n_util.h"
 
 class TestBookmarkMenuBridge : public BookmarkMenuBridge {
  public:
@@ -53,17 +55,27 @@ class BookmarkMenuBridgeTest : public PlatformTest {
   void InvalidateMenu()  { bridge_->InvalidateMenu(); }
   bool menu_is_valid()  { return bridge_->menuIsValid_; }
 
-  void AddNodeToMenu(BookmarkMenuBridge* bridge, const BookmarkNode* root,
+  void AddNodeToMenu(BookmarkMenuBridge* bridge,
+                     const BookmarkNode* root,
                      NSMenu* menu) {
     bridge->AddNodeToMenu(root, menu);
   }
 
+  void AddItemToMenu(BookmarkMenuBridge* bridge,
+                     int command_id,
+                     int message_id,
+                     const BookmarkNode* node,
+                     NSMenu* menu,
+                     bool enable) {
+    bridge->AddItemToMenu(command_id, message_id, node, menu, enable);
+  }
+
   NSMenuItem* MenuItemForNode(BookmarkMenuBridge* bridge,
                               const BookmarkNode* node) {
     return bridge->MenuItemForNode(node);
   }
 
-  NSMenuItem* AddItemToMenu(NSMenu *menu, NSString *title, SEL selector) {
+  NSMenuItem* AddTestMenuItem(NSMenu *menu, NSString *title, SEL selector) {
     NSMenuItem *item = [[[NSMenuItem alloc] initWithTitle:title action:NULL
                                             keyEquivalent:@""] autorelease];
     if (selector)
@@ -71,7 +83,6 @@ class BookmarkMenuBridgeTest : public PlatformTest {
     [menu addItem:item];
     return item;
   }
-
   BrowserTestHelper browser_test_helper_;
   scoped_ptr<TestBookmarkMenuBridge> bridge_;
 };
@@ -84,13 +95,14 @@ TEST_F(BookmarkMenuBridgeTest, TestBookmarkMenuAutoSeparator) {
   // 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.
+  // Add a bookmark and reload and there should be 8 items: the previous
+  // menu contents plus two new separator, the new bookmark and three
+  // versions of 'Open All Bookmarks' menu items.
   const BookmarkNode* parent = model->GetBookmarkBarNode();
   const char* url = "http://www.zim-bop-a-dee.com/";
   model->AddURL(parent, 0, ASCIIToUTF16("Bookmark"), GURL(url));
   bridge_->UpdateMenu(menu);
-  EXPECT_EQ(4, [menu numberOfItems]);
+  EXPECT_EQ(8, [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);
@@ -102,12 +114,12 @@ TEST_F(BookmarkMenuBridgeTest, TestBookmarkMenuAutoSeparator) {
 TEST_F(BookmarkMenuBridgeTest, TestClearBookmarkMenu) {
   NSMenu* menu = bridge_->menu_.get();
 
-  AddItemToMenu(menu, @"hi mom", nil);
-  AddItemToMenu(menu, @"not", @selector(openBookmarkMenuItem:));
-  NSMenuItem* item = AddItemToMenu(menu, @"hi mom", nil);
+  AddTestMenuItem(menu, @"hi mom", nil);
+  AddTestMenuItem(menu, @"not", @selector(openBookmarkMenuItem:));
+  NSMenuItem* item = AddTestMenuItem(menu, @"hi mom", nil);
   [item setSubmenu:[[[NSMenu alloc] initWithTitle:@"bar"] autorelease]];
-  AddItemToMenu(menu, @"not", @selector(openBookmarkMenuItem:));
-  AddItemToMenu(menu, @"zippy", @selector(length));
+  AddTestMenuItem(menu, @"not", @selector(openBookmarkMenuItem:));
+  AddTestMenuItem(menu, @"zippy", @selector(length));
   [menu addItem:[NSMenuItem separatorItem]];
 
   ClearBookmarkMenu(bridge_.get(), menu);
@@ -223,6 +235,74 @@ TEST_F(BookmarkMenuBridgeTest, TestAddNodeToMenu) {
   EXPECT_TRUE([long_item image]);
 }
 
+// Test that AddItemToMenu() properly added versions of
+// 'Open All Bookmarks' as menu items.
+TEST_F(BookmarkMenuBridgeTest, TestAddItemToMenu) {
+  NSString* title;
+  NSMenuItem* item;
+  NSMenu* menu = bridge_->menu_.get();
+
+  BookmarkModel* model = bridge_->GetBookmarkModel();
+  const BookmarkNode* root = model->GetBookmarkBarNode();
+  EXPECT_TRUE(model && root);
+  EXPECT_EQ(0, [menu numberOfItems]);
+
+  AddItemToMenu(bridge_.get(), IDC_BOOKMARK_BAR_OPEN_ALL,
+                IDS_BOOMARK_BAR_OPEN_ALL, root, menu, true);
+  AddItemToMenu(bridge_.get(), IDC_BOOKMARK_BAR_OPEN_ALL_NEW_WINDOW,
+                IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW, root, menu, true);
+  AddItemToMenu(bridge_.get(), IDC_BOOKMARK_BAR_OPEN_ALL_INCOGNITO,
+                IDS_BOOMARK_BAR_OPEN_INCOGNITO, root, menu, true);
+  EXPECT_EQ(3, [menu numberOfItems]);
+
+  title = l10n_util::GetNSString(IDS_BOOMARK_BAR_OPEN_ALL);
+  item = [menu itemWithTitle:title];
+  EXPECT_TRUE(item);
+  EXPECT_EQ(@selector(openAllBookmarks:), [item action]);
+  EXPECT_TRUE([item isEnabled]);
+
+  title = l10n_util::GetNSString(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW);
+  item = [menu itemWithTitle:title];
+  EXPECT_TRUE(item);
+  EXPECT_EQ(@selector(openAllBookmarksNewWindow:), [item action]);
+  EXPECT_TRUE([item isEnabled]);
+
+  title = l10n_util::GetNSString(IDS_BOOMARK_BAR_OPEN_INCOGNITO);
+  item = [menu itemWithTitle:title];
+  EXPECT_TRUE(item);
+  EXPECT_EQ(@selector(openAllBookmarksIncognitoWindow:), [item action]);
+  EXPECT_TRUE([item isEnabled]);
+
+  ClearBookmarkMenu(bridge_.get(), menu);
+  EXPECT_EQ(0, [menu numberOfItems]);
+
+  AddItemToMenu(bridge_.get(), IDC_BOOKMARK_BAR_OPEN_ALL,
+                IDS_BOOMARK_BAR_OPEN_ALL, root, menu, false);
+  AddItemToMenu(bridge_.get(), IDC_BOOKMARK_BAR_OPEN_ALL_NEW_WINDOW,
+                IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW, root, menu, false);
+  AddItemToMenu(bridge_.get(), IDC_BOOKMARK_BAR_OPEN_ALL_INCOGNITO,
+                IDS_BOOMARK_BAR_OPEN_INCOGNITO, root, menu, false);
+  EXPECT_EQ(3, [menu numberOfItems]);
+
+  title = l10n_util::GetNSString(IDS_BOOMARK_BAR_OPEN_ALL);
+  item = [menu itemWithTitle:title];
+  EXPECT_TRUE(item);
+  EXPECT_EQ(nil, [item action]);
+  EXPECT_FALSE([item isEnabled]);
+
+  title = l10n_util::GetNSString(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW);
+  item = [menu itemWithTitle:title];
+  EXPECT_TRUE(item);
+  EXPECT_EQ(nil, [item action]);
+  EXPECT_FALSE([item isEnabled]);
+
+  title = l10n_util::GetNSString(IDS_BOOMARK_BAR_OPEN_INCOGNITO);
+  item = [menu itemWithTitle:title];
+  EXPECT_TRUE(item);
+  EXPECT_EQ(nil, [item action]);
+  EXPECT_FALSE([item isEnabled]);
+}
+
 // Makes sure our internal map of BookmarkNode to NSMenuItem works.
 TEST_F(BookmarkMenuBridgeTest, TestGetMenuItemForNode) {
   string16 empty;
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller.h b/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller.h
index c746311..24d5dc90f 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller.h
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller.h
@@ -13,6 +13,7 @@
 #import <Cocoa/Cocoa.h>
 
 #import "base/mac/cocoa_protocols.h"
+#include "webkit/glue/window_open_disposition.h"
 
 class BookmarkNode;
 class BookmarkMenuBridge;
@@ -34,6 +35,9 @@ class BookmarkMenuBridge;
 // Called by any Bookmark menu item.
 // The menu item's tag is the bookmark ID.
 - (IBAction)openBookmarkMenuItem:(id)sender;
+- (IBAction)openAllBookmarks:(id)sender;
+- (IBAction)openAllBookmarksNewWindow:(id)sender;
+- (IBAction)openAllBookmarksIncognitoWindow:(id)sender;
 
 @end  // BookmarkMenuCocoaController
 
@@ -41,6 +45,8 @@ class BookmarkMenuBridge;
 @interface BookmarkMenuCocoaController (ExposedForUnitTests)
 - (const BookmarkNode*)nodeForIdentifier:(int)identifier;
 - (void)openURLForNode:(const BookmarkNode*)node;
+- (void)openAll:(NSInteger)tag
+    withDisposition:(WindowOpenDisposition)disposition;
 @end  // BookmarkMenuCocoaController (ExposedForUnitTests)
 
 #endif  // CHROME_BROWSER_UI_COCOA_BOOKMARKS_BOOKMARK_MENU_COCOA_CONTROLLER_H_
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller.mm
index 7f9e616..8ea883e 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller.mm
@@ -9,11 +9,12 @@
 #include "chrome/app/chrome_command_ids.h"  // IDC_BOOKMARK_MENU
 #import "chrome/browser/app_controller_mac.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
+#include "chrome/browser/bookmarks/bookmark_utils.h"
+#include "chrome/browser/metrics/user_metrics.h"
 #include "chrome/browser/ui/browser.h"
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.h"
 #include "chrome/browser/ui/cocoa/event_utils.h"
 #include "ui/base/text/text_elider.h"
-#include "webkit/glue/window_open_disposition.h"
 
 namespace {
 
@@ -81,6 +82,37 @@ const NSUInteger kMaximumMenuPixelsWide = 300;
                    PageTransition::AUTO_BOOKMARK);
 }
 
+// Open sites under BookmarkNode with the specified disposition.
+- (void)openAll:(NSInteger)tag
+    withDisposition:(WindowOpenDisposition)disposition {
+  int identifier = tag;
+
+  const BookmarkNode* node = [self nodeForIdentifier:identifier];
+  DCHECK(node);
+
+  Browser* browser = Browser::GetTabbedBrowser(bridge_->GetProfile(), true);
+  if (!browser)
+    browser = Browser::Create(bridge_->GetProfile());
+  DCHECK(browser);
+
+  if (!node || !browser)
+    return; // shouldn't be reached
+
+  bookmark_utils::OpenAll(NULL, browser->profile(), browser, node,
+                          disposition);
+
+  const char* metrics_action = NULL;
+  if (disposition == NEW_FOREGROUND_TAB) {
+    metrics_action = "OpenAllBookmarks";
+  } else if (disposition == NEW_WINDOW) {
+    metrics_action = "OpenAllBookmarksNewWindow";
+  } else {
+    metrics_action = "OpenAllBookmarksIncognitoWindow";
+  }
+  UserMetrics::RecordAction(UserMetricsAction(metrics_action),
+                            browser->profile());
+}
+
 - (IBAction)openBookmarkMenuItem:(id)sender {
   NSInteger tag = [sender tag];
   int identifier = tag;
@@ -92,5 +124,17 @@ const NSUInteger kMaximumMenuPixelsWide = 300;
   [self openURLForNode:node];
 }
 
+- (IBAction)openAllBookmarks:(id)sender {
+  [self openAll:[sender tag] withDisposition:NEW_FOREGROUND_TAB];
+}
+
+- (IBAction)openAllBookmarksNewWindow:(id)sender {
+  [self openAll:[sender tag] withDisposition:NEW_WINDOW];
+}
+
+- (IBAction)openAllBookmarksIncognitoWindow:(id)sender {
+  [self openAll:[sender tag] withDisposition:OFF_THE_RECORD];
+}
+
 @end  // BookmarkMenuCocoaController
 
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller_unittest.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller_unittest.mm
index 22930bc..3b175e0 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller_unittest.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller_unittest.mm
@@ -4,6 +4,7 @@
 
 #include "base/string16.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
+#include "chrome/browser/bookmarks/bookmark_utils.h"
 #include "chrome/browser/ui/cocoa/browser_test_helper.h"
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller.h"
 #include "chrome/browser/ui/browser.h"
@@ -14,6 +15,9 @@
   BrowserTestHelper* helper_;
   const BookmarkNode* nodes_[2];
   BOOL opened_[2];
+  BOOL opened_new_foreground_tab;
+  BOOL opened_new_window;
+  BOOL opened_off_the_record;
 }
 @end
 
@@ -50,6 +54,17 @@
     opened_[1] = YES;
 }
 
+- (void)openAll:(NSInteger)tag
+    withDisposition:(WindowOpenDisposition)disposition {
+  if (disposition == NEW_FOREGROUND_TAB) {
+    opened_new_foreground_tab = YES;
+  } else if (disposition == NEW_WINDOW) {
+    opened_new_window = YES;
+  } else if (disposition == OFF_THE_RECORD) {
+    opened_off_the_record = YES;
+  }
+}
+
 @end  // FakeBookmarkMenuController
 
 
@@ -62,5 +77,21 @@ TEST(BookmarkMenuCocoaControllerTest, TestOpenItem) {
     [c openBookmarkMenuItem:item];
     ASSERT_NE(c->opened_[i], NO);
   }
+
+  // Test three versions of 'Open All Bookmarks' item. tag id means nothing.
+  // I just reset the tag id to zero.
+  [item setTag:0];
+  EXPECT_EQ(c->opened_new_foreground_tab, NO);
+  [c openAllBookmarks:item];
+  EXPECT_NE(c->opened_new_foreground_tab, NO);
+
+  EXPECT_EQ(c->opened_new_window, NO);
+  [c openAllBookmarksNewWindow:item];
+  EXPECT_NE(c->opened_new_window, NO);
+
+  EXPECT_EQ(c->opened_off_the_record, NO);
+  [c openAllBookmarksIncognitoWindow:item];
+  EXPECT_NE(c->opened_off_the_record, NO);
+
   [c release];
 }