[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

5 files changed, 16 insertions, 123 deletions

diff --git a/views/focus/accelerator_handler.h b/views/focus/accelerator_handler.h
index 6140d364..3726d96 100644
--- a/views/focus/accelerator_handler.h
+++ b/views/focus/accelerator_handler.h
@@ -35,17 +35,6 @@ class AcceleratorHandler : public MessageLoopForUI::Dispatcher {
 #if defined(OS_WIN)
   // The keys currently pressed and consumed by the FocusManager.
   std::set<WPARAM> pressed_keys_;
-#else  // OS_LINUX
-  // The set of hardware keycodes that are currently pressed.  We use this
-  // to keep track of whether or not a key is being pressed in isolation
-  // or not. We must use hardware keycodes because higher-level keycodes
-  // may change between press and release depending on what modifier keys
-  // are pressed in the interim.
-  std::set<int> pressed_hardware_keys_;
-  // The set of vkey codes that were consumed by the FocusManager.
-  std::set<int> consumed_vkeys_;
-  // True if only the menu key (Alt key) was pressed, and no other keys.
-  bool only_menu_pressed_;
 #endif
 
   DISALLOW_COPY_AND_ASSIGN(AcceleratorHandler);
diff --git a/views/focus/accelerator_handler_gtk.cc b/views/focus/accelerator_handler_gtk.cc
index ee91906..e0af89b 100644
--- a/views/focus/accelerator_handler_gtk.cc
+++ b/views/focus/accelerator_handler_gtk.cc
@@ -13,112 +13,11 @@
 
 namespace views {
 
-AcceleratorHandler::AcceleratorHandler() : only_menu_pressed_(false) {}
+AcceleratorHandler::AcceleratorHandler() {}
 
 bool AcceleratorHandler::Dispatch(GdkEvent* event) {
-  // When focus changes, we may not see a full key-press / key-release
-  // pair, so clear the set of keys we think were pressed.
-  if (event->type == GDK_FOCUS_CHANGE) {
-    pressed_hardware_keys_.clear();
-    consumed_vkeys_.clear();
-    only_menu_pressed_ = false;
-  }
-
-  // Skip accelerator handling for non key events.
-  bool skip = event->type != GDK_KEY_PRESS && event->type != GDK_KEY_RELEASE;
-
-  // Skip if there is a grabbed widget and it is not a window. This is because
-  // of the following two reasons:
-  // 1. Widget such as pop up from GtkComboBox is a grabbed widget and use ESC
-  //    as its accelerator key to dismiss itself. We will break Gtk's code if
-  //    we eat this key. See http://crosbug.com/2355
-  // 2. Modal dialogs in Gtk are grabbed windows and we want to have our
-  //    accelerator key processing in this case. See http://crogbug.com/3701
-  if (!skip) {
-    GtkWidget* grabbed_widget = gtk_grab_get_current();
-    skip = grabbed_widget && !GTK_IS_WINDOW(grabbed_widget);
-  }
-
-  if (skip) {
-    // Let Gtk processes the event.
-    gtk_main_do_event(event);
-    return true;
-  }
-
-  GdkEventKey* key_event = reinterpret_cast<GdkEventKey*>(event);
-  // Let's retrieve the focus manager for the GdkWindow.
-  GdkWindow* window = gdk_window_get_toplevel(key_event->window);
-  gpointer ptr;
-  gdk_window_get_user_data(window, &ptr);
-  if (!ptr && !gdk_window_is_visible(window)) {
-    // The window is destroyed while we're handling key events.
-    gtk_main_do_event(event);
-    return true;
-  }
-
-  DCHECK(ptr);
-
-  // The top-level window or window widget is expected to always be associated
-  // with the top-level gtk widget.
-  WidgetGtk* widget =
-      WidgetGtk::GetViewForNative(reinterpret_cast<GtkWidget*>(ptr));
-  if (!widget) {
-    // During dnd we get events for windows we don't control (such as the
-    // window being dragged).
-    gtk_main_do_event(event);
-    return true;
-  }
-  FocusManager* focus_manager = widget->GetFocusManager();
-  if (!focus_manager) {
-    NOTREACHED();
-    return true;
-  }
-
-  KeyEvent view_key_event(key_event);
-  int key_code = view_key_event.GetKeyCode();
-  int hardware_keycode = key_event->hardware_keycode;
-  if (event->type == GDK_KEY_PRESS) {
-    pressed_hardware_keys_.insert(hardware_keycode);
-
-    // If it's the Alt key, don't send it to the focus manager until release
-    // (to handle focusing the menu bar).
-    if (key_code == base::VKEY_MENU && pressed_hardware_keys_.size() == 1) {
-      only_menu_pressed_ = true;
-      return true;
-    }
-
-    if (pressed_hardware_keys_.size() != 1)
-      only_menu_pressed_ = false;
-
-    // FocusManager::OnKeyEvent will return false if this message has been
-    // consumed and should not be propagated further.
-    if (!focus_manager->OnKeyEvent(view_key_event)) {
-      consumed_vkeys_.insert(key_code);
-      return true;
-    }
-  }
-
-  if (event->type == GDK_KEY_RELEASE) {
-    pressed_hardware_keys_.erase(hardware_keycode);
-
-    // Make sure to filter out the key release for a key press consumed
-    // as an accelerator, to avoid calling gtk_main_do_event with an
-    // unpaired key release.
-    if (consumed_vkeys_.find(key_code) != consumed_vkeys_.end()) {
-      consumed_vkeys_.erase(key_code);
-      return true;
-    }
-
-    // Special case: the Alt key can trigger an accelerator on release
-    // rather than on press, but only if no other keys were pressed.
-    if (key_code == base::VKEY_MENU && only_menu_pressed_) {
-      Accelerator accelerator(base::VKEY_MENU, false, false, false);
-      focus_manager->ProcessAccelerator(accelerator);
-      return true;
-    }
-  }
-
-  // Pass the event to gtk if we didn't consume it above.
+  // The logic for handling keyboard accelerators has been moved into
+  // WidgetGtk::OnKeyEvent handler (views/widget/widget_gtk.cc).
   gtk_main_do_event(event);
   return true;
 }
diff --git a/views/focus/accelerator_handler_gtk_unittest.cc b/views/focus/accelerator_handler_gtk_unittest.cc
index ef05312..78e9110 100644
--- a/views/focus/accelerator_handler_gtk_unittest.cc
+++ b/views/focus/accelerator_handler_gtk_unittest.cc
@@ -99,7 +99,7 @@ TEST_F(AcceleratorHandlerGtkTest, TestHomepageAccelerator) {
   ASSERT_FALSE(menu_pressed_);
   ASSERT_FALSE(home_pressed_);
 
-  evt = CreateKeyEvent(GDK_KEY_PRESS, GDK_Menu, 0);
+  evt = CreateKeyEvent(GDK_KEY_PRESS, GDK_Alt_L, 0);
   EXPECT_TRUE(handler.Dispatch(reinterpret_cast<GdkEvent*>(&evt)));
 
   ASSERT_FALSE(menu_pressed_);
@@ -113,7 +113,7 @@ TEST_F(AcceleratorHandlerGtkTest, TestHomepageAccelerator) {
 
   evt = CreateKeyEvent(GDK_KEY_RELEASE, GDK_Home, GDK_MOD1_MASK);
   EXPECT_TRUE(handler.Dispatch(reinterpret_cast<GdkEvent*>(&evt)));
-  evt = CreateKeyEvent(GDK_KEY_RELEASE, GDK_Menu, 0);
+  evt = CreateKeyEvent(GDK_KEY_RELEASE, GDK_Alt_L, 0);
   EXPECT_TRUE(handler.Dispatch(reinterpret_cast<GdkEvent*>(&evt)));
 
   ASSERT_FALSE(menu_pressed_);
@@ -127,12 +127,12 @@ TEST_F(AcceleratorHandlerGtkTest, TestMenuAccelerator) {
 
   ASSERT_FALSE(menu_pressed_);
 
-  evt = CreateKeyEvent(GDK_KEY_PRESS, GDK_Menu, 0);
+  evt = CreateKeyEvent(GDK_KEY_PRESS, GDK_Alt_L, 0);
   EXPECT_TRUE(handler.Dispatch(reinterpret_cast<GdkEvent*>(&evt)));
 
   ASSERT_FALSE(menu_pressed_);
 
-  evt = CreateKeyEvent(GDK_KEY_RELEASE, GDK_Menu, 0);
+  evt = CreateKeyEvent(GDK_KEY_RELEASE, GDK_Alt_L, 0);
   EXPECT_TRUE(handler.Dispatch(reinterpret_cast<GdkEvent*>(&evt)));
 
   ASSERT_TRUE(menu_pressed_);
diff --git a/views/focus/focus_manager.cc b/views/focus/focus_manager.cc
index 5a94c95..f0f5f99 100644
--- a/views/focus/focus_manager.cc
+++ b/views/focus/focus_manager.cc
@@ -81,9 +81,13 @@ FocusManager::~FocusManager() {
 }
 
 bool FocusManager::OnKeyEvent(const KeyEvent& event) {
+#if defined(OS_WIN)
   // If the focused view wants to process the key event as is, let it be.
+  // On Linux we always dispatch key events to the focused view first, so
+  // we should not do this check here. See also WidgetGtk::OnKeyEvent().
   if (focused_view_ && focused_view_->SkipDefaultKeyEventProcessing(event))
     return true;
+#endif
 
   // Intercept Tab related messages for focus traversal.
   // Note that we don't do focus traversal if the root window is not part of the
diff --git a/views/focus/focus_manager_unittest.cc b/views/focus/focus_manager_unittest.cc
index 955a83a..f786b1d 100644
--- a/views/focus/focus_manager_unittest.cc
+++ b/views/focus/focus_manager_unittest.cc
@@ -1485,6 +1485,10 @@ class MessageTrackingView : public View {
   DISALLOW_COPY_AND_ASSIGN(MessageTrackingView);
 };
 
+#if defined(OS_WIN)
+// This test is now Windows only. Linux Views port does not handle accelerator
+// keys in AcceleratorHandler anymore. The logic has been moved into
+// WidgetGtk::OnKeyEvent().
 // Tests that the keyup messages are eaten for accelerators.
 TEST_F(FocusManagerTest, IgnoreKeyupForAccelerators) {
   FocusManager* focus_manager = GetFocusManager();
@@ -1553,15 +1557,12 @@ TEST_F(FocusManagerTest, IgnoreKeyupForAccelerators) {
   PostKeyUp(base::VKEY_0);
   MessageLoopForUI::current()->PostTask(FROM_HERE, new MessageLoop::QuitTask());
   MessageLoopForUI::current()->Run(&accelerator_handler);
-#if defined(OS_WIN)
-  // Linux eats only last accelerator's release event.
-  // See http://crbug.com/23383 for details.
   EXPECT_TRUE(mtv->keys_pressed().empty());
   EXPECT_TRUE(mtv->keys_released().empty());
-#endif
   EXPECT_TRUE(mtv->accelerator_pressed());
   mtv->Reset();
 }
+#endif
 
 #if defined(OS_WIN)
 // Test that the focus manager is created successfully for the first view