[Linux Views] Refactor accelerator handler related code.

This CL removes the accelerator handling logic in
accelerator_handler_gtk.cc and implements a much simpler solution in
WidgetGtk. The new approach always sends a key event to the focused View
and native GtkWidget first and only sends it to the focus manager if it's not
handled by any View or native GtkWidget.

BUG=23383 AcceleratorHandler on Windows should not dispatch the KEYUP messages eaten by the FocusManager
BUG=40966 BrowserKeyEventsTest.AccessKeys is crashy
BUG=49701 [Linux Views]Some Emacs keybindings are broken in omnibox and find in page box.
TEST=Press Alt key in different place (web page, omnibox, find bar, etc.) to see if menu bar can be focused correctly. Press alt-F to popup wrench menu and Escape to close it, then try alt key again.

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

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

3 files changed, 68 insertions, 12 deletions

diff --git a/chrome/browser/browser_keyevents_browsertest.cc b/chrome/browser/browser_keyevents_browsertest.cc
index e943bef..0c4bd37db 100644
--- a/chrome/browser/browser_keyevents_browsertest.cc
+++ b/chrome/browser/browser_keyevents_browsertest.cc
@@ -532,14 +532,7 @@ IN_PROC_BROWSER_TEST_F(BrowserKeyEventsTest, CommandKeyEvents) {
 }
 #endif
 
-#if defined(TOOLKIT_VIEWS) && defined(OS_LINUX)
-// See http://crbug.com/40037 for details.
-#define MAYBE_AccessKeys DISABLED_AccessKeys
-#else
-#define MAYBE_AccessKeys AccessKeys
-#endif
-
-IN_PROC_BROWSER_TEST_F(BrowserKeyEventsTest, MAYBE_AccessKeys) {
+IN_PROC_BROWSER_TEST_F(BrowserKeyEventsTest, AccessKeys) {
 #if defined(OS_MACOSX)
   // On Mac, access keys use ctrl+alt modifiers.
   static const KeyEventTestData kTestAccessA = {
@@ -891,4 +884,57 @@ IN_PROC_BROWSER_TEST_F(BrowserKeyEventsTest, PageUpDownKeys) {
   EXPECT_NO_FATAL_FAILURE(CheckTextBoxValue(tab_index, L"A", L""));
 }
 
+#if defined(OS_WIN) || defined(TOOLKIT_VIEWS)
+IN_PROC_BROWSER_TEST_F(BrowserKeyEventsTest, FocusMenuBarByAltKey) {
+  static const KeyEventTestData kTestAltKey = {
+    base::VKEY_MENU, false, false, false, false,
+    false, false, false, false, 2,
+    { "D 18 0 false false true false",
+      "U 18 0 false false true false" }
+  };
+
+  static const KeyEventTestData kTestAltKeySuppress = {
+    base::VKEY_MENU, false, false, false, false,
+    true, false, false, false, 2,
+    { "D 18 0 false false true false",
+      "U 18 0 false false true false" }
+  };
+
+  static const KeyEventTestData kTestCtrlAltKey = {
+    base::VKEY_MENU, true, false, false, false,
+    false, false, false, false, 4,
+    { "D 17 0 true false false false",
+      "D 18 0 true false true false",
+      "U 18 0 true false true false",
+      "U 17 0 true false false false" }
+  };
+
+  net::HTTPTestServer* server = StartHTTPServer();
+  ASSERT_TRUE(server);
+
+  BringBrowserWindowToFront();
+  GURL url = server->TestServerPage(kTestingPage);
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  ASSERT_NO_FATAL_FAILURE(ClickOnView(VIEW_ID_TAB_CONTAINER));
+  ASSERT_TRUE(IsViewFocused(VIEW_ID_TAB_CONTAINER_FOCUS_VIEW));
+
+  int tab_index = browser()->selected_index();
+  // Press and release Alt key to focus wrench menu button.
+  EXPECT_NO_FATAL_FAILURE(TestKeyEvent(tab_index, kTestAltKey));
+  EXPECT_TRUE(IsViewFocused(VIEW_ID_APP_MENU));
+
+  ASSERT_NO_FATAL_FAILURE(ClickOnView(VIEW_ID_TAB_CONTAINER));
+  ASSERT_TRUE(IsViewFocused(VIEW_ID_TAB_CONTAINER_FOCUS_VIEW));
+
+  // Alt key can be suppressed.
+  EXPECT_NO_FATAL_FAILURE(TestKeyEvent(tab_index, kTestAltKeySuppress));
+  ASSERT_TRUE(IsViewFocused(VIEW_ID_TAB_CONTAINER_FOCUS_VIEW));
+
+  // Ctrl+Alt should have no effect.
+  EXPECT_NO_FATAL_FAILURE(TestKeyEvent(tab_index, kTestCtrlAltKey));
+  ASSERT_TRUE(IsViewFocused(VIEW_ID_TAB_CONTAINER_FOCUS_VIEW));
+}
+#endif
+
 }  // namespace
diff --git a/chrome/browser/renderer_host/gtk_im_context_wrapper.cc b/chrome/browser/renderer_host/gtk_im_context_wrapper.cc
index 8aa7f8c..4ac138b 100644
--- a/chrome/browser/renderer_host/gtk_im_context_wrapper.cc
+++ b/chrome/browser/renderer_host/gtk_im_context_wrapper.cc
@@ -132,6 +132,13 @@ void GtkIMContextWrapper::ProcessKeyEvent(GdkEventKey* event) {
 
   NativeWebKeyboardEvent wke(event);
 
+  // If the key event was handled by the input method, then we need to prevent
+  // RenderView::UnhandledKeyboardEvent() from processing it.
+  // Otherwise unexpected result may occur. For example if it's a
+  // Backspace key event, the browser may go back to previous page.
+  if (filtered)
+    wke.skip_in_browser = true;
+
   // Send filtered keydown event before sending IME result.
   if (event->type == GDK_KEY_PRESS && filtered)
     ProcessFilteredKeyPressEvent(&wke);
@@ -321,10 +328,6 @@ void GtkIMContextWrapper::ProcessFilteredKeyPressEvent(
     // keyidentifier must be updated accordingly, otherwise this key event may
     // still be processed by webkit.
     wke->setKeyIdentifierFromWindowsKeyCode();
-    // Prevent RenderView::UnhandledKeyboardEvent() from processing it.
-    // Otherwise unexpected result may occur. For example if it's a
-    // Backspace key event, the browser may go back to previous page.
-    wke->skip_in_browser = true;
   }
   host_view_->ForwardKeyboardEvent(*wke);
 }
diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc
index 0e11eaa..d8ab622 100644
--- a/chrome/browser/views/frame/browser_view.cc
+++ b/chrome/browser/views/frame/browser_view.cc
@@ -73,6 +73,7 @@
 #elif defined(OS_LINUX)
 #include "chrome/browser/views/accelerator_table_gtk.h"
 #include "views/window/hit_test.h"
+#include "views/window/window_gtk.h"
 #endif
 
 using base::TimeDelta;
@@ -1238,8 +1239,14 @@ bool BrowserView::PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event,
 }
 
 void BrowserView::HandleKeyboardEvent(const NativeWebKeyboardEvent& event) {
+#if defined(OS_LINUX)
+  views::Window* window = GetWidget()->GetWindow();
+  if (window && event.os_event && !event.skip_in_browser)
+    static_cast<views::WindowGtk*>(window)->HandleKeyboardEvent(event.os_event);
+#else
   unhandled_keyboard_event_handler_.HandleKeyboardEvent(event,
                                                         GetFocusManager());
+#endif
 }
 
 // TODO(devint): http://b/issue?id=1117225 Cut, Copy, and Paste are always