Refactor MenuController::SendAcceleratorToHotTrackedView() and OnKeyDown().

The space/return key handling has tri-states:
1. no hot_view: most menu items
2. menu should be closed: wrench menu cut/copy/paste
3. menu shouldn't be closed: wrench menu zoom buttons

The original SendAcceleratorToHotTrackedView() only distinguishes 1 and 2-3.  This CL defines an enum type to distinguish all of these three cases.

BUG=107919
TEST=manually

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

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

2 files changed, 30 insertions, 23 deletions

diff --git a/ui/views/controls/menu/menu_controller.cc b/ui/views/controls/menu/menu_controller.cc
index 0f16a60..7850c58 100644
--- a/ui/views/controls/menu/menu_controller.cc
+++ b/ui/views/controls/menu/menu_controller.cc
@@ -913,13 +913,6 @@ base::MessagePumpDispatcher::DispatchStatus
       if (!OnKeyDown(ui::KeyboardCodeFromNative(xev)))
         return base::MessagePumpDispatcher::EVENT_QUIT;
 
-      // OnKeyDown may have set exit_type_.
-      // TODO(sky): This shouldn't be necessary if OnKeyDown returns correct
-      // value for space key on edit menu. Fix OnKeyDown and remove this.
-      // See crbug.com/107919.
-      if (exit_type_ != EXIT_NONE)
-        return base::MessagePumpDispatcher::EVENT_QUIT;
-
       return SelectByChar(ui::KeyboardCodeFromNative(xev)) ?
           base::MessagePumpDispatcher::EVENT_QUIT :
           base::MessagePumpDispatcher::EVENT_PROCESSED;
@@ -950,11 +943,6 @@ bool MenuController::Dispatch(GdkEvent* event) {
       if (!OnKeyDown(ui::WindowsKeyCodeForGdkKeyCode(event->key.keyval)))
         return false;
 
-      // OnKeyDown may have set exit_type_.
-      // TODO(sky): Eliminate this. See crbug.com/107919.
-      if (exit_type_ != EXIT_NONE)
-        return false;
-
       guint32 keycode = gdk_keyval_to_unicode(event->key.keyval);
       if (keycode)
         return !SelectByChar(keycode);
@@ -1005,17 +993,23 @@ bool MenuController::OnKeyDown(ui::KeyboardCode key_code) {
       break;
 
     case ui::VKEY_SPACE:
-      SendAcceleratorToHotTrackedView();
+      if (SendAcceleratorToHotTrackedView() == ACCELERATOR_PROCESSED_EXIT)
+        return false;
       break;
 
     case ui::VKEY_RETURN:
       if (pending_state_.item) {
         if (pending_state_.item->HasSubmenu()) {
           OpenSubmenuChangeSelectionIfCan();
-        } else if (!SendAcceleratorToHotTrackedView() &&
-                   pending_state_.item->enabled()) {
-          Accept(pending_state_.item, 0);
-          return false;
+        } else {
+          SendAcceleratorResultType result = SendAcceleratorToHotTrackedView();
+          if (result == ACCELERATOR_NOT_PROCESSED &&
+              pending_state_.item->enabled()) {
+            Accept(pending_state_.item, 0);
+            return false;
+          } else if (result == ACCELERATOR_PROCESSED_EXIT) {
+            return false;
+          }
         }
       }
       break;
@@ -1074,15 +1068,17 @@ MenuController::~MenuController() {
   StopCancelAllTimer();
 }
 
-bool MenuController::SendAcceleratorToHotTrackedView() {
+MenuController::SendAcceleratorResultType
+    MenuController::SendAcceleratorToHotTrackedView() {
   View* hot_view = GetFirstHotTrackedView(pending_state_.item);
   if (!hot_view)
-    return false;
+    return ACCELERATOR_NOT_PROCESSED;
 
   ui::Accelerator accelerator(ui::VKEY_RETURN, false, false, false);
   hot_view->AcceleratorPressed(accelerator);
   hot_view->SetHotTracked(true);
-  return true;
+  return (exit_type_ == EXIT_NONE) ?
+      ACCELERATOR_PROCESSED : ACCELERATOR_PROCESSED_EXIT;
 }
 
 void MenuController::UpdateInitialLocation(
diff --git a/ui/views/controls/menu/menu_controller.h b/ui/views/controls/menu/menu_controller.h
index d1439b5..b48df02 100644
--- a/ui/views/controls/menu/menu_controller.h
+++ b/ui/views/controls/menu/menu_controller.h
@@ -155,6 +155,18 @@ class VIEWS_EXPORT MenuController : public MessageLoop::Dispatcher {
     SELECTION_EXIT                  = 1 << 2,
   };
 
+  // Result type for SendAcceleratorToHotTrackedView
+  enum SendAcceleratorResultType {
+    // Accelerator is not sent because of no hot tracked views.
+    ACCELERATOR_NOT_PROCESSED,
+
+    // Accelerator is sent to the hot tracked views.
+    ACCELERATOR_PROCESSED,
+
+    // Same as above and the accelerator causes the exit of the menu.
+    ACCELERATOR_PROCESSED_EXIT
+  };
+
   // Tracks selection information.
   struct State {
     State();
@@ -247,9 +259,8 @@ class VIEWS_EXPORT MenuController : public MessageLoop::Dispatcher {
 
   virtual ~MenuController();
 
-  // If there is a hot tracked view AcceleratorPressed is invoked on it and
-  // true is returned.
-  bool SendAcceleratorToHotTrackedView();
+  // AcceleratorPressed is invoked on the hot tracked view if it exists.
+  SendAcceleratorResultType SendAcceleratorToHotTrackedView();
 
   void UpdateInitialLocation(const gfx::Rect& bounds,
                              MenuItemView::AnchorPosition position);