Makes the following changes to the bookmark bar context menu:

. Nukes the open menu item.
. Adds open incognito and for folders open all incognito.

BUG=144
TEST=Fully test the context menu of bookmark folders/urls.

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

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

6 files changed, 91 insertions, 93 deletions

diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index 2f0254f..d3563d6 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -2108,8 +2108,8 @@ each locale. -->
       <message name="IDS_BOOMARK_BAR_DEFAULT_FOLDER" desc="Title of the default bookmarks folder">
         Bookmarks
       </message>
-      <message name="IDS_BOOMARK_BAR_OPEN" desc="Menu description for loading bookmark in current page">
-        Open
+      <message name="IDS_BOOMARK_BAR_OPEN_INCOGNITO" desc="Menu description for openning bookmark in incognito window">
+        Open in incognito window
       </message>
       <message name="IDS_BOOMARK_BAR_OPEN_IN_NEW_TAB" desc="Menu description for loading bookmark in a new tab">
         Open in new tab
@@ -2129,6 +2129,9 @@ each locale. -->
       <message name="IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW" desc="Menu title for opening all urls in a bookmark folder in a new window">
         Open all bookmarks in new window
       </message>
+      <message name="IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO" desc="Menu description for openning all urls in a bookmark folder in an incognito window">
+        Open all bookmarks in incognito window
+      </message>
       <message name="IDS_BOOKMARK_BAR_REMOVE" desc="Menu title for removing/unstarring a bookmark">
         Delete
       </message>
diff --git a/chrome/browser/bookmark_bar_context_menu_controller.cc b/chrome/browser/bookmark_bar_context_menu_controller.cc
index 75d33b8..12b2566 100644
--- a/chrome/browser/bookmark_bar_context_menu_controller.cc
+++ b/chrome/browser/bookmark_bar_context_menu_controller.cc
@@ -193,18 +193,6 @@ class EditFolderController : public InputWindowDelegate,
 
 // BookmarkBarContextMenuController -------------------------------------------
 
-const int BookmarkBarContextMenuController::always_show_command_id = 1;
-const int BookmarkBarContextMenuController::open_bookmark_id = 2;
-const int BookmarkBarContextMenuController::open_bookmark_in_new_window_id = 3;
-const int BookmarkBarContextMenuController::open_bookmark_in_new_tab_id = 4;
-const int BookmarkBarContextMenuController::open_all_bookmarks_id = 5;
-const int
-    BookmarkBarContextMenuController::open_all_bookmarks_in_new_window_id = 6;
-const int BookmarkBarContextMenuController::edit_bookmark_id = 7;
-const int BookmarkBarContextMenuController::delete_bookmark_id = 8;
-const int BookmarkBarContextMenuController::add_bookmark_id = 9;
-const int BookmarkBarContextMenuController::new_folder_id = 10;
-
 // static
 void BookmarkBarContextMenuController::OpenAll(
     HWND parent,
@@ -227,41 +215,44 @@ BookmarkBarContextMenuController::BookmarkBarContextMenuController(
       menu_(this) {
   if (node->GetType() == history::StarredEntry::URL) {
     menu_.AppendMenuItemWithLabel(
-        open_bookmark_id,
-        l10n_util::GetString(IDS_BOOMARK_BAR_OPEN));
-    menu_.AppendMenuItemWithLabel(
-        open_bookmark_in_new_tab_id,
+        IDS_BOOMARK_BAR_OPEN_IN_NEW_TAB,
         l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_IN_NEW_TAB));
     menu_.AppendMenuItemWithLabel(
-        open_bookmark_in_new_window_id,
+        IDS_BOOMARK_BAR_OPEN_IN_NEW_WINDOW,
         l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_IN_NEW_WINDOW));
+    menu_.AppendMenuItemWithLabel(
+        IDS_BOOMARK_BAR_OPEN_INCOGNITO,
+        l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_INCOGNITO));
   } else {
     menu_.AppendMenuItemWithLabel(
-        open_all_bookmarks_id,
+        IDS_BOOMARK_BAR_OPEN_ALL,
         l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_ALL));
     menu_.AppendMenuItemWithLabel(
-        open_all_bookmarks_in_new_window_id,
+        IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW,
         l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW));
+    menu_.AppendMenuItemWithLabel(
+        IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO,
+        l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO));
   }
   menu_.AppendSeparator();
 
   if (node->GetParent() !=
       view->GetProfile()->GetBookmarkModel()->root_node()) {
-    menu_.AppendMenuItemWithLabel(edit_bookmark_id,
+    menu_.AppendMenuItemWithLabel(IDS_BOOKMARK_BAR_EDIT,
                                   l10n_util::GetString(IDS_BOOKMARK_BAR_EDIT));
     menu_.AppendMenuItemWithLabel(
-        delete_bookmark_id,
+        IDS_BOOKMARK_BAR_REMOVE,
         l10n_util::GetString(IDS_BOOKMARK_BAR_REMOVE));
   }
 
   menu_.AppendMenuItemWithLabel(
-      add_bookmark_id,
+      IDS_BOOMARK_BAR_ADD_NEW_BOOKMARK,
       l10n_util::GetString(IDS_BOOMARK_BAR_ADD_NEW_BOOKMARK));
   menu_.AppendMenuItemWithLabel(
-      new_folder_id,
+      IDS_BOOMARK_BAR_NEW_FOLDER,
       l10n_util::GetString(IDS_BOOMARK_BAR_NEW_FOLDER));
   menu_.AppendSeparator();
-  menu_.AppendMenuItem(always_show_command_id,
+  menu_.AppendMenuItem(IDS_BOOMARK_BAR_ALWAYS_SHOW,
                        l10n_util::GetString(IDS_BOOMARK_BAR_ALWAYS_SHOW),
                        ChromeViews::MenuItemView::CHECKBOX);
 }
@@ -289,14 +280,15 @@ void BookmarkBarContextMenuController::ExecuteCommand(int id) {
   Profile* profile = view_->GetProfile();
 
   switch (id) {
-    case open_bookmark_id:
-      UserMetrics::RecordAction(L"BookmarkBar_ContextMenu_Open", profile);
+    case IDS_BOOMARK_BAR_OPEN_INCOGNITO:
+      UserMetrics::RecordAction(L"BookmarkBar_ContextMenu_OpenInIncognito",
+                                profile);
 
-      view_->GetPageNavigator()->OpenURL(node_->GetURL(), CURRENT_TAB,
+      view_->GetPageNavigator()->OpenURL(node_->GetURL(), OFF_THE_RECORD,
                                          PageTransition::AUTO_BOOKMARK);
       break;
 
-    case open_bookmark_in_new_window_id:
+    case IDS_BOOMARK_BAR_OPEN_IN_NEW_WINDOW:
       UserMetrics::RecordAction(L"BookmarkBar_ContextMenu_OpenInNewWindow",
                                 profile);
 
@@ -304,7 +296,7 @@ void BookmarkBarContextMenuController::ExecuteCommand(int id) {
                                          PageTransition::AUTO_BOOKMARK);
       break;
 
-    case open_bookmark_in_new_tab_id:
+    case IDS_BOOMARK_BAR_OPEN_IN_NEW_TAB:
       UserMetrics::RecordAction(L"BookmarkBar_ContextMenu_OpenInNewTab",
                                 profile);
 
@@ -312,22 +304,24 @@ void BookmarkBarContextMenuController::ExecuteCommand(int id) {
                                          PageTransition::AUTO_BOOKMARK);
       break;
 
-    case open_all_bookmarks_id:
-    case open_all_bookmarks_in_new_window_id: {
-      if (id == open_all_bookmarks_id) {
+    case IDS_BOOMARK_BAR_OPEN_ALL:
+    case IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO:
+    case IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW: {
+      WindowOpenDisposition initial_disposition;
+      if (id == IDS_BOOMARK_BAR_OPEN_ALL) {
+        initial_disposition = CURRENT_TAB;
         UserMetrics::RecordAction(L"BookmarkBar_ContextMenu_OpenAll",
                                   profile);
-      } else {
+      } else if (id == IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW) {
+        initial_disposition = NEW_WINDOW;
         UserMetrics::RecordAction(
             L"BookmarkBar_ContextMenu_OpenAllInNewWindow", profile);
+      } else {
+        initial_disposition = OFF_THE_RECORD;
+        UserMetrics::RecordAction(
+            L"BookmarkBar_ContextMenu_OpenAllIncognito", profile);
       }
 
-      WindowOpenDisposition initial_disposition;
-      if (id == open_all_bookmarks_in_new_window_id)
-        initial_disposition = NEW_WINDOW;
-      else
-        initial_disposition = CURRENT_TAB;
-
       // GetContainer is NULL during testing.
       HWND parent_hwnd = view_->GetContainer() ?
           view_->GetContainer()->GetHWND() : 0;
@@ -337,7 +331,7 @@ void BookmarkBarContextMenuController::ExecuteCommand(int id) {
       break;
     }
 
-    case edit_bookmark_id:
+    case IDS_BOOKMARK_BAR_EDIT:
       UserMetrics::RecordAction(L"BookmarkBar_ContextMenu_Edit", profile);
 
       if (node_->GetType() == history::StarredEntry::URL) {
@@ -351,7 +345,7 @@ void BookmarkBarContextMenuController::ExecuteCommand(int id) {
       }
       break;
 
-    case delete_bookmark_id: {
+    case IDS_BOOKMARK_BAR_REMOVE: {
       UserMetrics::RecordAction(L"BookmarkBar_ContextMenu_Remove", profile);
 
       view_->GetModel()->Remove(node_->GetParent(),
@@ -359,7 +353,7 @@ void BookmarkBarContextMenuController::ExecuteCommand(int id) {
       break;
     }
 
-    case add_bookmark_id: {
+    case IDS_BOOMARK_BAR_ADD_NEW_BOOKMARK: {
       UserMetrics::RecordAction(L"BookmarkBar_ContextMenu_Add", profile);
 
       BookmarkEditorView::Show(view_->GetContainer()->GetHWND(),
@@ -367,7 +361,7 @@ void BookmarkBarContextMenuController::ExecuteCommand(int id) {
       break;
     }
 
-    case new_folder_id: {
+    case IDS_BOOMARK_BAR_NEW_FOLDER: {
       UserMetrics::RecordAction(L"BookmarkBar_ContextMenu_NewFolder",
                                 profile);
 
@@ -382,7 +376,7 @@ void BookmarkBarContextMenuController::ExecuteCommand(int id) {
       break;
     }
 
-    case always_show_command_id:
+    case IDS_BOOMARK_BAR_ALWAYS_SHOW:
       view_->ToggleWhenVisible();
       break;
 
@@ -392,14 +386,22 @@ void BookmarkBarContextMenuController::ExecuteCommand(int id) {
 }
 
 bool BookmarkBarContextMenuController::IsItemChecked(int id) const {
-  DCHECK(id == always_show_command_id);
+  DCHECK(id == IDS_BOOMARK_BAR_ALWAYS_SHOW);
   return view_->GetProfile()->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar);
 }
 
 bool BookmarkBarContextMenuController::IsCommandEnabled(int id) const {
-  if (id == open_all_bookmarks_id || id == open_all_bookmarks_in_new_window_id)
-    return NodeHasURLs(node_);
+  switch (id) {
+    case IDS_BOOMARK_BAR_OPEN_INCOGNITO:
+      return !view_->GetProfile()->IsOffTheRecord();
 
+    case IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO:
+      return NodeHasURLs(node_) && !view_->GetProfile()->IsOffTheRecord();
+
+    case IDS_BOOMARK_BAR_OPEN_ALL:
+    case IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW:
+      return NodeHasURLs(node_);
+  }
   return true;
 }
 
diff --git a/chrome/browser/bookmark_bar_context_menu_controller.h b/chrome/browser/bookmark_bar_context_menu_controller.h
index a6fd38a..3052a3d 100644
--- a/chrome/browser/bookmark_bar_context_menu_controller.h
+++ b/chrome/browser/bookmark_bar_context_menu_controller.h
@@ -43,18 +43,6 @@ class BookmarkBarContextMenuController : public ChromeViews::MenuDelegate,
   virtual bool IsItemChecked(int id) const;
   virtual bool IsCommandEnabled(int id) const;
 
-  // IDs used for the menus. Public for testing.
-  static const int always_show_command_id;
-  static const int open_bookmark_id;
-  static const int open_bookmark_in_new_window_id;
-  static const int open_bookmark_in_new_tab_id;
-  static const int open_all_bookmarks_id;
-  static const int open_all_bookmarks_in_new_window_id;
-  static const int edit_bookmark_id;
-  static const int delete_bookmark_id;
-  static const int add_bookmark_id;
-  static const int new_folder_id;
-
  private:
   // Returns the parent node and visual_order to use when adding new
   // bookmarks/folders.
diff --git a/chrome/browser/bookmark_bar_context_menu_controller_test.cc b/chrome/browser/bookmark_bar_context_menu_controller_test.cc
index fd3f5cc..01d4613 100644
--- a/chrome/browser/bookmark_bar_context_menu_controller_test.cc
+++ b/chrome/browser/bookmark_bar_context_menu_controller_test.cc
@@ -12,6 +12,8 @@
 #include "chrome/test/testing_profile.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#include "generated_resources.h"
+
 namespace {
 
 // PageNavigator implementation that records the URL.
@@ -90,55 +92,50 @@ TEST_F(BookmarkBarContextMenuControllerTest, DeleteURL) {
   BookmarkBarContextMenuController controller(
       bb_view_.get(), model_->GetBookmarkBarNode()->GetChild(0));
   GURL url = model_->GetBookmarkBarNode()->GetChild(0)->GetURL();
-  ASSERT_TRUE(controller.IsCommandEnabled(
-                  BookmarkBarContextMenuController::delete_bookmark_id));
+  ASSERT_TRUE(controller.IsCommandEnabled(IDS_BOOKMARK_BAR_REMOVE));
   // Delete the URL.
-  controller.ExecuteCommand(
-      BookmarkBarContextMenuController::delete_bookmark_id);
+  controller.ExecuteCommand(IDS_BOOKMARK_BAR_REMOVE);
   // Model shouldn't have URL anymore.
   ASSERT_FALSE(model_->IsBookmarked(url));
 }
 
-// Tests openning from the menu.
-TEST_F(BookmarkBarContextMenuControllerTest, OpenURL) {
-  BookmarkBarContextMenuController controller(
-      bb_view_.get(), model_->GetBookmarkBarNode()->GetChild(0));
-  GURL url = model_->GetBookmarkBarNode()->GetChild(0)->GetURL();
-  ASSERT_TRUE(controller.IsCommandEnabled(
-                  BookmarkBarContextMenuController::open_bookmark_id));
-  // Open it.
-  controller.ExecuteCommand(
-      BookmarkBarContextMenuController::open_bookmark_id);
-  // Should have navigated to it.
-  ASSERT_EQ(1, navigator_.urls_.size());
-  ASSERT_TRUE(url == navigator_.urls_[0]);
-}
-
 // Tests open all on a folder with a couple of bookmarks.
 TEST_F(BookmarkBarContextMenuControllerTest, OpenAll) {
   BookmarkNode* folder = model_->GetBookmarkBarNode()->GetChild(1);
   BookmarkBarContextMenuController controller(bb_view_.get(), folder);
-  ASSERT_TRUE(controller.IsCommandEnabled(
-      BookmarkBarContextMenuController::open_all_bookmarks_id));
-  ASSERT_TRUE(controller.IsCommandEnabled(
-      BookmarkBarContextMenuController::open_all_bookmarks_in_new_window_id));
+  ASSERT_TRUE(controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL));
+  ASSERT_TRUE(controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO));
+  ASSERT_TRUE(controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW));
   // Open it.
-  controller.ExecuteCommand(
-      BookmarkBarContextMenuController::open_all_bookmarks_id);
+  controller.ExecuteCommand(IDS_BOOMARK_BAR_OPEN_ALL);
   // Should have navigated to F1's children.
   ASSERT_EQ(2, navigator_.urls_.size());
   ASSERT_TRUE(folder->GetChild(0)->GetURL() == navigator_.urls_[0]);
   ASSERT_TRUE(folder->GetChild(1)->GetChild(0)->GetURL() ==
               navigator_.urls_[1]);
+
+  // Make sure incognito is disabled when OTR.
+  profile_->set_off_the_record(true);
+  ASSERT_TRUE(controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL));
+  ASSERT_FALSE(controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO));
+  ASSERT_TRUE(
+      controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW));
 }
 
 // Tests that menus are appropriately disabled for empty folders.
 TEST_F(BookmarkBarContextMenuControllerTest, DisableForEmptyFolder) {
   BookmarkNode* folder = model_->GetBookmarkBarNode()->GetChild(2);
   BookmarkBarContextMenuController controller(bb_view_.get(), folder);
-  EXPECT_FALSE(controller.IsCommandEnabled(
-      BookmarkBarContextMenuController::open_all_bookmarks_id));
-  EXPECT_FALSE(controller.IsCommandEnabled(
-      BookmarkBarContextMenuController::open_all_bookmarks_in_new_window_id));
+  EXPECT_FALSE(controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL));
+  EXPECT_FALSE(
+      controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW));
 }
 
+// Tests the enabled state of open incognito.
+TEST_F(BookmarkBarContextMenuControllerTest, DisableIncognito) {
+  BookmarkBarContextMenuController controller(
+      bb_view_.get(), model_->GetBookmarkBarNode()->GetChild(0));
+  EXPECT_TRUE(controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_INCOGNITO));
+  profile_->set_off_the_record(true);
+  EXPECT_FALSE(controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_INCOGNITO));
+}
diff --git a/chrome/test/testing_profile.cc b/chrome/test/testing_profile.cc
index 272f94c..8abb4ec 100644
--- a/chrome/test/testing_profile.cc
+++ b/chrome/test/testing_profile.cc
@@ -43,7 +43,8 @@ class BookmarkLoadObserver : public BookmarkModelObserver {
 }  // namespace
 
 TestingProfile::TestingProfile()
-    : start_time_(Time::Now()), has_history_service_(false) {
+    : start_time_(Time::Now()), has_history_service_(false),
+      off_the_record_(false) {
   PathService::Get(base::DIR_TEMP, &path_);
   file_util::AppendToPath(&path_, L"TestingProfilePath");
   file_util::Delete(path_, true);
@@ -51,7 +52,8 @@ TestingProfile::TestingProfile()
 }
 
 TestingProfile::TestingProfile(int count)
-    : start_time_(Time::Now()), has_history_service_(false) {
+    : start_time_(Time::Now()), has_history_service_(false),
+      off_the_record_(false) {
   PathService::Get(base::DIR_TEMP, &path_);
   file_util::AppendToPath(&path_, L"TestingProfilePath" + IntToWString(count));
   file_util::Delete(path_, true);
diff --git a/chrome/test/testing_profile.h b/chrome/test/testing_profile.h
index 892d357..dbbda33 100644
--- a/chrome/test/testing_profile.h
+++ b/chrome/test/testing_profile.h
@@ -50,8 +50,12 @@ class TestingProfile : public Profile {
   virtual std::wstring GetPath() {
     return path_;
   }
+  // Sets whether we're off the record. Default is false.
+  void set_off_the_record(bool off_the_record) {
+    off_the_record_ = off_the_record;
+  }
   virtual bool IsOffTheRecord() {
-    return false;
+    return off_the_record_;
   }
   virtual Profile* GetOffTheRecordProfile() {
     return NULL;
@@ -179,6 +183,8 @@ class TestingProfile : public Profile {
   bool has_history_service_;
 
   std::wstring id_;
+
+  bool off_the_record_;
 };
 
 #endif  // CHROME_TEST_TESTING_PROFILE_H__