Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1236)

Unified Diff: chrome/browser/ui/views/bookmarks/bookmark_menu_delegate_unittest.cc

Issue 26350003: OLD: reland "views: change WrenchMenu to use each model's command ID's when creating MenuItemView's" (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: fix integer overflow w/ kint32max, add test Created 7 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/ui/views/bookmarks/bookmark_menu_delegate_unittest.cc
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate_unittest.cc b/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate_unittest.cc
index fb51c407add94746d90ac5838791996041661427..67e242ac13e21ecc7a44ec3ec4cb71ab6367024e 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate_unittest.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate_unittest.cc
@@ -13,11 +13,11 @@
#include "chrome/test/base/browser_with_test_window_test.h"
#include "chrome/test/base/testing_profile.h"
#include "ui/views/controls/menu/menu_runner.h"
+#include "ui/views/controls/menu/submenu_view.h"
class BookmarkMenuDelegateTest : public BrowserWithTestWindowTest {
public:
- BookmarkMenuDelegateTest() : model_(NULL) {
- }
+ BookmarkMenuDelegateTest() : model_(NULL) {}
virtual void SetUp() OVERRIDE {
BrowserWithTestWindowTest::SetUp();
@@ -28,9 +28,6 @@ class BookmarkMenuDelegateTest : public BrowserWithTestWindowTest {
test::WaitForBookmarkModelToLoad(model_);
AddTestData();
-
- bookmark_menu_delegate_.reset(
- new BookmarkMenuDelegate(browser(), NULL, NULL, 0));
}
virtual void TearDown() OVERRIDE {
@@ -42,6 +39,21 @@ class BookmarkMenuDelegateTest : public BrowserWithTestWindowTest {
}
protected:
+ void NewDelegate(int min_menu_id, int max_menu_id) {
+ bookmark_menu_delegate_.reset(
+ new BookmarkMenuDelegate(browser(), NULL, NULL,
+ min_menu_id, max_menu_id));
+ }
+
+ void NewAndInitDelegateForPermanent(int min_menu_id,
+ int max_menu_id) {
+ const BookmarkNode* node = model_->bookmark_bar_node();
+ NewDelegate(min_menu_id, max_menu_id);
+ bookmark_menu_delegate_->Init(&test_delegate_, NULL, node, 0,
+ BookmarkMenuDelegate::SHOW_PERMANENT_FOLDERS,
+ BOOKMARK_LAUNCH_LOCATION_NONE);
+ }
+
BookmarkModel* model_;
scoped_ptr<BookmarkMenuDelegate> bookmark_menu_delegate_;
@@ -79,6 +91,8 @@ class BookmarkMenuDelegateTest : public BrowserWithTestWindowTest {
model_->AddURL(of1, 0, ASCIIToUTF16("of1a"), GURL(test_base + "of1a"));
}
+ views::MenuDelegate test_delegate_;
+
DISALLOW_COPY_AND_ASSIGN(BookmarkMenuDelegateTest);
};
@@ -87,6 +101,7 @@ class BookmarkMenuDelegateTest : public BrowserWithTestWindowTest {
TEST_F(BookmarkMenuDelegateTest, RemoveBookmarks) {
views::MenuDelegate test_delegate;
const BookmarkNode* node = model_->bookmark_bar_node()->GetChild(1);
+ NewDelegate(0, kint32max);
bookmark_menu_delegate_->Init(&test_delegate, NULL, node, 0,
BookmarkMenuDelegate::HIDE_PERMANENT_FOLDERS,
BOOKMARK_LAUNCH_LOCATION_NONE);
@@ -95,4 +110,111 @@ TEST_F(BookmarkMenuDelegateTest, RemoveBookmarks) {
bookmark_menu_delegate_->WillRemoveBookmarks(nodes_to_remove);
nodes_to_remove.clear();
bookmark_menu_delegate_->DidRemoveBookmarks();
+
+}
+
+// Verifies menu ID's of items in menu fall within the specified range.
+TEST_F(BookmarkMenuDelegateTest, MenuIdRange) {
+ // Start with maximum menu Id of 10 - the number of items that AddTestData()
+ // populated. Everything should be created.
+ NewAndInitDelegateForPermanent(0, 10);
+ views::MenuItemView* root_item = bookmark_menu_delegate_->menu_;
+ ASSERT_TRUE(root_item->HasSubmenu());
+ EXPECT_EQ(4, root_item->GetSubmenu()->GetMenuItemCount());
+ EXPECT_EQ(5, root_item->GetSubmenu()->child_count()); // Includes separator.
+ views::MenuItemView* F1_item = root_item->GetSubmenu()->GetMenuItemAt(1);
+ ASSERT_TRUE(F1_item->HasSubmenu());
+ EXPECT_EQ(2, F1_item->GetSubmenu()->GetMenuItemCount());
+ views::MenuItemView* F11_item = F1_item->GetSubmenu()->GetMenuItemAt(1);
+ ASSERT_TRUE(F11_item->HasSubmenu());
+ EXPECT_EQ(1, F11_item->GetSubmenu()->GetMenuItemCount());
+ views::MenuItemView* other_item = root_item->GetSubmenu()->GetMenuItemAt(3);
+ ASSERT_TRUE(other_item->HasSubmenu());
+ EXPECT_EQ(2, other_item->GetSubmenu()->GetMenuItemCount());
+ views::MenuItemView* OF1_item = other_item->GetSubmenu()->GetMenuItemAt(1);
+ ASSERT_TRUE(OF1_item->HasSubmenu());
+ EXPECT_EQ(1, OF1_item->GetSubmenu()->GetMenuItemCount());
+
+ // Reduce maximum 9. "of1a" item should not be created.
+ NewAndInitDelegateForPermanent(0, 9);
+ root_item = bookmark_menu_delegate_->menu_;
+ EXPECT_EQ(4, root_item->GetSubmenu()->GetMenuItemCount());
+ EXPECT_EQ(5, root_item->GetSubmenu()->child_count()); // Includes separator.
+ other_item = root_item->GetSubmenu()->GetMenuItemAt(3);
+ OF1_item = other_item->GetSubmenu()->GetMenuItemAt(1);
+ EXPECT_EQ(0, OF1_item->GetSubmenu()->GetMenuItemCount());
+
+ // Reduce maximum 8. "OF1" submenu should not be created.
+ NewAndInitDelegateForPermanent(0, 8);
+ root_item = bookmark_menu_delegate_->menu_;
+ EXPECT_EQ(4, root_item->GetSubmenu()->GetMenuItemCount());
+ EXPECT_EQ(5, root_item->GetSubmenu()->child_count()); // Includes separator.
+ other_item = root_item->GetSubmenu()->GetMenuItemAt(3);
+ EXPECT_EQ(1, other_item->GetSubmenu()->GetMenuItemCount());
+
+ // Reduce maximum 7. "Other" submenu should be empty.
+ NewAndInitDelegateForPermanent(0, 7);
+ root_item = bookmark_menu_delegate_->menu_;
+ EXPECT_EQ(4, root_item->GetSubmenu()->GetMenuItemCount());
+ EXPECT_EQ(5, root_item->GetSubmenu()->child_count()); // Includes separator.
+ other_item = root_item->GetSubmenu()->GetMenuItemAt(3);
+ EXPECT_EQ(0, other_item->GetSubmenu()->GetMenuItemCount());
+
+ // Reduce maximum to 6. "Other" submenu should not be created, and no
+ // separator.
+ NewAndInitDelegateForPermanent(0, 6);
+ root_item = bookmark_menu_delegate_->menu_;
+ EXPECT_EQ(3, root_item->GetSubmenu()->GetMenuItemCount());
+ EXPECT_EQ(3, root_item->GetSubmenu()->child_count()); // No separator.
+
+ // Reduce maximum 5. "F2" and "Other" submenus shouldn't be created.
+ NewAndInitDelegateForPermanent(0, 5);
+ root_item = bookmark_menu_delegate_->menu_;
+ EXPECT_EQ(2, root_item->GetSubmenu()->GetMenuItemCount());
+ EXPECT_EQ(2, root_item->GetSubmenu()->child_count()); // No separator.
+ F1_item = root_item->GetSubmenu()->GetMenuItemAt(1);
+ F11_item = F1_item->GetSubmenu()->GetMenuItemAt(1);
+ EXPECT_EQ(1, F11_item->GetSubmenu()->GetMenuItemCount());
+
+ // Reduce maximum to 4. "f11a" item and "F2" and "Other" submenus should
+ // not be created.
+ NewAndInitDelegateForPermanent(0, 4);
+ root_item = bookmark_menu_delegate_->menu_;
+ EXPECT_EQ(2, root_item->GetSubmenu()->GetMenuItemCount());
+ EXPECT_EQ(2, root_item->GetSubmenu()->child_count()); // No separator.
+ F1_item = root_item->GetSubmenu()->GetMenuItemAt(1);
+ F11_item = F1_item->GetSubmenu()->GetMenuItemAt(1);
+ EXPECT_EQ(0, F11_item->GetSubmenu()->GetMenuItemCount());
+
+ // Reduce maximum to 3. "F11", "F2" and "Other" submenus should not be
+ // created.
+ NewAndInitDelegateForPermanent(0, 3);
+ root_item = bookmark_menu_delegate_->menu_;
+ EXPECT_EQ(2, root_item->GetSubmenu()->GetMenuItemCount());
+ EXPECT_EQ(2, root_item->GetSubmenu()->child_count()); // No separator.
+ F1_item = root_item->GetSubmenu()->GetMenuItemAt(1);
+ EXPECT_EQ(views::MenuItemView::SUBMENU, F1_item->GetType());
+ EXPECT_EQ(1, F1_item->GetSubmenu()->GetMenuItemCount());
+
+ // Reduce maximum 2. Only "a" item and empty "F1" submenu should be created.
+ NewAndInitDelegateForPermanent(0, 2);
+ root_item = bookmark_menu_delegate_->menu_;
+ EXPECT_EQ(2, root_item->GetSubmenu()->GetMenuItemCount());
+ EXPECT_EQ(2, root_item->GetSubmenu()->child_count()); // No separator.
+ F1_item = root_item->GetSubmenu()->GetMenuItemAt(1);
+ EXPECT_EQ(views::MenuItemView::SUBMENU, F1_item->GetType());
+ EXPECT_EQ(0, F1_item->GetSubmenu()->GetMenuItemCount());
+
+ // Reduce maximum to 1. Only "a" item should be created.
+ NewAndInitDelegateForPermanent(0, 1);
+ root_item = bookmark_menu_delegate_->menu_;
+ EXPECT_EQ(1, root_item->GetSubmenu()->GetMenuItemCount());
+ EXPECT_EQ(1, root_item->GetSubmenu()->child_count()); // No separator.
+
+ // Verify correct handling of integer overflow with range, set kint32max as
+ // maximum and 1 less as minimum. Only "a" item should be created.
+ NewAndInitDelegateForPermanent(kint32max - 1, kint32max);
+ root_item = bookmark_menu_delegate_->menu_;
+ EXPECT_EQ(1, root_item->GetSubmenu()->GetMenuItemCount());
+ EXPECT_EQ(1, root_item->GetSubmenu()->child_count()); // No separator.
}

Powered by Google App Engine
This is Rietveld 408576698