Fixes bug in bookmark menus where using arrow keys would select

invisible menu items.

BUG=54800
TEST=see bug

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

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

2 files changed, 29 insertions, 3 deletions

diff --git a/views/controls/menu/menu_controller.cc b/views/controls/menu/menu_controller.cc
index 9b4a0b3..06c86fc 100644
--- a/views/controls/menu/menu_controller.cc
+++ b/views/controls/menu/menu_controller.cc
@@ -1527,10 +1527,11 @@ void MenuController::IncrementSelection(int delta) {
     if (parent_count > 1) {
       for (int i = 0; i < parent_count; ++i) {
         if (parent->GetSubmenu()->GetMenuItemAt(i) == item) {
-          int next_index = (i + delta + parent_count) % parent_count;
-          ScrollToVisible(parent->GetSubmenu()->GetMenuItemAt(next_index));
           MenuItemView* to_select =
-              parent->GetSubmenu()->GetMenuItemAt(next_index);
+              FindNextSelectableMenuItem(parent, i, delta);
+          if (!to_select)
+            break;
+          ScrollToVisible(to_select);
           SetSelection(to_select, false, false);
           View* to_make_hot = GetInitialFocusableView(to_select, delta == 1);
           if (to_make_hot)
@@ -1542,6 +1543,24 @@ void MenuController::IncrementSelection(int delta) {
   }
 }
 
+MenuItemView* MenuController::FindNextSelectableMenuItem(MenuItemView* parent,
+                                                         int index,
+                                                         int delta) {
+  int start_index = index;
+  int parent_count = parent->GetSubmenu()->GetMenuItemCount();
+  // Loop through the menu items skipping any invisible menus. The loop stops
+  // when we wrap or find a visible child.
+  do {
+    index = (index + delta + parent_count) % parent_count;
+    if (index == start_index)
+      return NULL;
+    MenuItemView* child = parent->GetSubmenu()->GetMenuItemAt(index);
+    if (child->IsVisible())
+      return child;
+  } while (index != start_index);
+  return NULL;
+}
+
 void MenuController::OpenSubmenuChangeSelectionIfCan() {
   MenuItemView* item = pending_state_.item;
   if (item->HasSubmenu()) {
diff --git a/views/controls/menu/menu_controller.h b/views/controls/menu/menu_controller.h
index 3c641c1..fe379b8 100644
--- a/views/controls/menu/menu_controller.h
+++ b/views/controls/menu/menu_controller.h
@@ -311,6 +311,13 @@ class MenuController : public MessageLoopForUI::Dispatcher {
   // Selects the next/previous menu item.
   void IncrementSelection(int delta);
 
+  // Returns the next selectable child menu item of |parent| starting at |index|
+  // and incrementing index by |delta|. If there are no more selected menu items
+  // NULL is returned.
+  MenuItemView* FindNextSelectableMenuItem(MenuItemView* parent,
+                                           int index,
+                                           int delta);
+
   // If the selected item has a submenu and it isn't currently open, the
   // the selection is changed such that the menu opens immediately.
   void OpenSubmenuChangeSelectionIfCan();