Fixes issue with accelerators when a menu is open

ui/views/controls/menu/menu_controller.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/controls/menu/menu_controller.h"
 
 #include "base/i18n/case_conversion.h"
 #include "base/i18n/rtl.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
+#include "ui/base/accelerators/accelerator.h"
 #include "ui/base/dragdrop/os_exchange_data.h"
 #include "ui/base/events.h"
 #include "ui/base/keycodes/keyboard_codes.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/canvas_skia.h"
 #include "ui/gfx/screen.h"
 #include "ui/views/controls/button/menu_button.h"
 #include "ui/views/controls/menu/menu_controller_delegate.h"
 #include "ui/views/controls/menu/menu_scroll_view_container.h"
 #include "ui/views/controls/menu/submenu_view.h"
 #include "ui/views/drag_utils.h"
+#include "ui/views/focus/focus_manager.h"
 #include "ui/views/view_constants.h"
 #include "ui/views/views_delegate.h"
 #include "ui/views/widget/root_view.h"
 #include "ui/views/widget/widget.h"
 
 #if defined(USE_AURA)
+#include "ash/accelerators/accelerator_controller.h"
+#include "ash/shell.h"
+#include "ui/aura/event.h"
 #include "ui/aura/root_window.h"
 #elif defined(TOOLKIT_USES_GTK)
 #include "ui/base/keycodes/keyboard_code_conversion_gtk.h"
 #endif
 
 using base::Time;
 using base::TimeDelta;
 using ui::OSExchangeData;
 
 // Period of the scroll timer (in milliseconds).
@@ skipping 841 matching lines @@
       base::MessagePumpDispatcher::EVENT_PROCESSED;
 }
 
 #elif defined(USE_AURA)
 base::MessagePumpDispatcher::DispatchStatus
     MenuController::Dispatch(XEvent* xev) {
   if (exit_type_ == EXIT_ALL || exit_type_ == EXIT_DESTROYED) {
     aura::RootWindow::GetInstance()->GetDispatcher()->Dispatch(xev);
     return base::MessagePumpDispatcher::EVENT_QUIT;
   }
+
+  ui::KeyboardCode key_code = ui::KeyboardCodeFromNative(xev);
+  int flags = ui::EventFlagsFromNative(xev);
+
   switch (ui::EventTypeFromNative(xev)) {
     case ui::ET_KEY_PRESSED:
-      if (!OnKeyDown(ui::KeyboardCodeFromNative(xev)))
+      if (!OnKeyDown(key_code))
         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;
+      if (SelectByChar(key_code))
+        return base::MessagePumpDispatcher::EVENT_QUIT;
+
+      {
+        const int kModifierFlagMask = (ui::EF_SHIFT_DOWN |
+                                       ui::EF_CONTROL_DOWN |
+                                       ui::EF_ALT_DOWN);
+
+        ui::Accelerator accelerator = ui::Accelerator(key_code,
+            flags & kModifierFlagMask);

[oshima, 2012/01/19 18:29:48]

just accelerator(key_code, flags & kModifierFlagMask);

btw, why do you need this mask?

+        FocusManager* focus_manager = menu_button_->GetWidget()->
+            GetFocusManager();

[oshima, 2012/01/19 18:29:48]

menu_button_->GetFocusManager()

+        ash::AcceleratorController* accelerator_controller =
+            ash::Shell::GetInstance()->accelerator_controller();

[oshima, 2012/01/19 18:29:48]

have you looked into handling this on aura/ash side? If so, what's the
difficulty?

+        if (focus_manager && focus_manager->ProcessAccelerator(accelerator))
+          return base::MessagePumpDispatcher::EVENT_QUIT;

[oshima, 2012/01/19 18:29:48]

You're executing accelerator inside nested loop while menu is open and I'm
worrying that there may be an accelerator that shouldn't be executed in this
situation. Scott, WDYT?

+        else if (accelerator_controller && accelerator_controller->
+            Process(accelerator))
+          return base::MessagePumpDispatcher::EVENT_QUIT;
+      }
+      return base::MessagePumpDispatcher::EVENT_PROCESSED;
     case ui::ET_KEY_RELEASED:
       return base::MessagePumpDispatcher::EVENT_PROCESSED;
     default:
       break;
   }
 
   // TODO(oshima): Update Windows' Dispatcher to return DispatchStatus
   // instead of bool.
   if (aura::RootWindow::GetInstance()->GetDispatcher()->Dispatch(xev) ==
       base::MessagePumpDispatcher::EVENT_IGNORED)
     return EVENT_IGNORED;
   return exit_type_ != EXIT_NONE ?
       base::MessagePumpDispatcher::EVENT_QUIT :
       base::MessagePumpDispatcher::EVENT_PROCESSED;
 }
 #else
 bool MenuController::Dispatch(GdkEvent* event) {
   if (exit_type_ == EXIT_ALL || exit_type_ == EXIT_DESTROYED) {
-    gtk_main_do_event(event);
+    gtk_main_do_eSvent(event);

[oshima, 2012/01/19 18:29:48]

revert this change?

     return false;
   }
 
   switch (event->type) {
     case GDK_KEY_PRESS: {
       if (!OnKeyDown(ui::WindowsKeyCodeForGdkKeyCode(event->key.keyval)))
         return false;
 
       // OnKeyDown may have set exit_type_.
       // TODO(sky): Eliminate this. See crbug.com/107919.
@@ skipping 1036 matching lines @@
   // is necessary to exit from nested loop (See Dispatch methods).
   // Send non-op event so that Dispatch method will always be called.
   // crbug.com/104684.
   if (exit_type_ == EXIT_ALL || exit_type_ == EXIT_DESTROYED)
     aura::RootWindow::GetInstance()->PostNativeEvent(ui::CreateNoopEvent());
 #endif
 }
 
 
 }  // namespace views