AURA/X11: Handle VKEY_MENU accelerator on content area

-Moved the code to handle vkey_menu to focus manager.
-Unify the code between win/aura/gtk to handle unhandled web keyevent. We were using different code path for unhandled web keyboard for regular page and login. This should fix this issue also.
-Improved focus test not to use fixed wait. This should also speedup the test a bit.
-Removed OmniboxViewViews tests that runs only on views/gtk. This is no longer supported and we can re-enable for win when ready.

BUG=99861,106998, 108480, 108459
TEST=manual: set focus to content area and hit alt key. the focus should be set to wrench menu on release.


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

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

6 files changed, 95 insertions, 91 deletions

diff --git a/ui/views/focus/focus_manager.cc b/ui/views/focus/focus_manager.cc
index d6d1c1e..4ec263c 100644
--- a/ui/views/focus/focus_manager.cc
+++ b/ui/views/focus/focus_manager.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -26,6 +26,9 @@ FocusManager::FocusManager(Widget* widget)
       focused_view_(NULL),
       accelerator_manager_(new ui::AcceleratorManager),
       focus_change_reason_(kReasonDirectFocusChange),
+#if defined(USE_X11)
+      should_handle_menu_key_release_(false),
+#endif
       is_changing_focus_(false) {
   DCHECK(widget_);
   stored_focused_view_storage_id_ =
@@ -36,6 +39,41 @@ FocusManager::~FocusManager() {
 }
 
 bool FocusManager::OnKeyEvent(const KeyEvent& event) {
+  const int key_code = event.key_code();
+
+#if defined(USE_X11)
+  // TODO(ben): beng believes that this should be done in
+  // RootWindowHosLinux for aura/linux.
+
+  // Always reset |should_handle_menu_key_release_| unless we are handling a
+  // VKEY_MENU key release event. It ensures that VKEY_MENU accelerator can only
+  // be activated when handling a VKEY_MENU key release event which is preceded
+  // by an un-handled VKEY_MENU key press event.
+  if (key_code != ui::VKEY_MENU || event.type() != ui::ET_KEY_RELEASED)
+    should_handle_menu_key_release_ = false;
+
+  if (event.type() == ui::ET_KEY_PRESSED) {
+    // VKEY_MENU is triggered by key release event.
+    // FocusManager::OnKeyEvent() returns false when the key has been consumed.
+    if (key_code == ui::VKEY_MENU) {
+      should_handle_menu_key_release_ = true;
+      return false;
+    }
+    // Pass through to the reset of OnKeyEvent.
+  } else if (key_code == ui::VKEY_MENU && should_handle_menu_key_release_ &&
+             (event.flags() & ~ui::EF_ALT_DOWN) == 0) {
+    // Trigger VKEY_MENU when only this key is pressed and released, and both
+    // press and release events are not handled by others.
+    ui::Accelerator accelerator(ui::VKEY_MENU, false, false, false);
+    return ProcessAccelerator(accelerator);
+  } else {
+    return false;
+  }
+#else
+  if (event.type() != ui::ET_KEY_PRESSED)
+    return false;
+#endif
+
 #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
@@ -64,7 +102,6 @@ bool FocusManager::OnKeyEvent(const KeyEvent& event) {
 #endif
 
   // Intercept arrow key messages to switch between grouped views.
-  ui::KeyboardCode key_code = event.key_code();
   if (focused_view_ && focused_view_->GetGroup() != -1 &&
       (key_code == ui::VKEY_UP || key_code == ui::VKEY_DOWN ||
        key_code == ui::VKEY_LEFT || key_code == ui::VKEY_RIGHT)) {
@@ -254,6 +291,10 @@ void FocusManager::ClearFocus() {
 }
 
 void FocusManager::StoreFocusedView() {
+#if defined(USE_X11)
+  // Forget menu key state when the window lost focus.
+  should_handle_menu_key_release_ = false;
+#endif
   ViewStorage* view_storage = ViewStorage::GetInstance();
   if (!view_storage) {
     // This should never happen but bug 981648 seems to indicate it could.
@@ -287,6 +328,9 @@ void FocusManager::StoreFocusedView() {
 }
 
 void FocusManager::RestoreFocusedView() {
+#if defined(USE_X11)
+  DCHECK(!should_handle_menu_key_release_);
+#endif
   ViewStorage* view_storage = ViewStorage::GetInstance();
   if (!view_storage) {
     // This should never happen but bug 981648 seems to indicate it could.
@@ -378,6 +422,23 @@ bool FocusManager::ProcessAccelerator(const ui::Accelerator& accelerator) {
   return accelerator_manager_->Process(accelerator);
 }
 
+void FocusManager::MaybeResetMenuKeyState(const KeyEvent& key) {
+#if defined(USE_X11)
+  // Always reset |should_handle_menu_key_release_| unless we are handling a
+  // VKEY_MENU key release event. It ensures that VKEY_MENU accelerator can only
+  // be activated when handling a VKEY_MENU key release event which is preceded
+  // by an unhandled VKEY_MENU key press event. See also HandleKeyboardEvent().
+  if (key.key_code() != ui::VKEY_MENU || key.type() != ui::ET_KEY_RELEASED)
+    should_handle_menu_key_release_ = false;
+#endif
+}
+
+#if defined(TOOLKIT_USES_GTK)
+void FocusManager::ResetMenuKeyState() {
+  should_handle_menu_key_release_ = false;
+}
+#endif
+
 ui::AcceleratorTarget* FocusManager::GetCurrentTargetForAccelerator(
     const ui::Accelerator& accelerator) const {
   return accelerator_manager_->GetCurrentTarget(accelerator);
diff --git a/ui/views/focus/focus_manager.h b/ui/views/focus/focus_manager.h
index 0cb4671..1f45bc8 100644
--- a/ui/views/focus/focus_manager.h
+++ b/ui/views/focus/focus_manager.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -213,6 +213,15 @@ class VIEWS_EXPORT FocusManager {
   // Returns true if an accelerator was activated.
   bool ProcessAccelerator(const ui::Accelerator& accelerator);
 
+  // Resets menu key state if |event| is not menu key release.
+  // This is effective only on x11.
+  void MaybeResetMenuKeyState(const KeyEvent& key);
+
+#if defined(TOOLKIT_USES_GTK)
+  // Resets menu key state. TODO(oshima): Remove this when views/gtk is removed.
+  void ResetMenuKeyState();
+#endif
+
   // Called by a RootView when a view within its hierarchy is removed
   // from its parent. This will only be called by a RootView in a
   // hierarchy of Widgets that this FocusManager is attached to the
@@ -269,6 +278,11 @@ class VIEWS_EXPORT FocusManager {
   // The list of registered FocusChange listeners.
   ObserverList<FocusChangeListener, true> focus_change_listeners_;
 
+#if defined(USE_X11)
+  // Indicates if we should handle the upcoming Alt key release event.
+  bool should_handle_menu_key_release_;
+#endif
+
   // See description above getter.
   bool is_changing_focus_;
 
diff --git a/ui/views/widget/native_widget_aura.cc b/ui/views/widget/native_widget_aura.cc
index b314228..2c5c2c1 100644
--- a/ui/views/widget/native_widget_aura.cc
+++ b/ui/views/widget/native_widget_aura.cc
@@ -129,9 +129,6 @@ NativeWidgetAura::NativeWidgetAura(internal::NativeWidgetDelegate* delegate)
       ownership_(Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET),
       ALLOW_THIS_IN_INITIALIZER_LIST(close_widget_factory_(this)),
       can_activate_(true),
-#if defined(USE_X11)
-      should_handle_menu_key_release_(false),
-#endif
       cursor_(gfx::kNullCursor),
       saved_window_state_(ui::SHOW_STATE_DEFAULT) {
 }
@@ -589,42 +586,12 @@ void NativeWidgetAura::SetVisibilityChangedAnimationsEnabled(bool value) {
 // NativeWidgetAura, views::InputMethodDelegate implementation:
 
 void NativeWidgetAura::DispatchKeyEventPostIME(const KeyEvent& key) {
-  if (delegate_->OnKeyEvent(key) || !GetWidget()->GetFocusManager())
+  FocusManager* focus_manager = GetWidget()->GetFocusManager();
+  if (focus_manager)
+    focus_manager->MaybeResetMenuKeyState(key);
+  if (delegate_->OnKeyEvent(key) || !focus_manager)
     return;
-
-#if defined(USE_X11)
-  // TODO(oshima): This is copied from native_widget_gtk for now.
-  // RenderWidgetHostViewAura doesn't work and needs more work.
-  // oshima thinks this should be moved to focus manager (see
-  // crbug.com/106998), but beng believes that this should be done
-  // in RootWindowHosLinux for aura/linux.
-  const int key_code = key.key_code();
-
-  // Always reset |should_handle_menu_key_release_| unless we are handling a
-  // VKEY_MENU key release event. It ensures that VKEY_MENU accelerator can only
-  // be activated when handling a VKEY_MENU key release event which is preceded
-  // by an un-handled VKEY_MENU key press event.
-  if (key_code != ui::VKEY_MENU || key.type() != ui::ET_KEY_RELEASED)
-    should_handle_menu_key_release_ = false;
-
-  if (key.type() == ui::ET_KEY_PRESSED) {
-    // VKEY_MENU is triggered by key release event.
-    // FocusManager::OnKeyEvent() returns false when the key has been consumed.
-    if (key_code != ui::VKEY_MENU)
-      GetWidget()->GetFocusManager()->OnKeyEvent(key);
-    else
-      should_handle_menu_key_release_ = true;
-  } else if (key_code == ui::VKEY_MENU && should_handle_menu_key_release_ &&
-             (key.flags() & ~ui::EF_ALT_DOWN) == 0) {
-    // Trigger VKEY_MENU when only this key is pressed and released, and both
-    // press and release events are not handled by others.
-    ui::Accelerator accelerator(ui::VKEY_MENU, false, false, false);
-    GetWidget()->GetFocusManager()->ProcessAccelerator(accelerator);
-  }
-#else
-  if (key.type() == ui::ET_KEY_PRESSED)
-    GetWidget()->GetFocusManager()->OnKeyEvent(key);
-#endif
+  focus_manager->OnKeyEvent(key);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -643,9 +610,6 @@ void NativeWidgetAura::OnBoundsChanged(const gfx::Rect& old_bounds,
 }
 
 void NativeWidgetAura::OnFocus() {
-#if defined(USE_X11)
-  should_handle_menu_key_release_ = false;
-#endif
   Widget* widget = GetWidget();
   if (widget->is_top_level()) {
     InputMethod* input_method = widget->GetInputMethod();
diff --git a/ui/views/widget/native_widget_aura.h b/ui/views/widget/native_widget_aura.h
index c026715..a4eb003 100644
--- a/ui/views/widget/native_widget_aura.h
+++ b/ui/views/widget/native_widget_aura.h
@@ -174,11 +174,6 @@ class VIEWS_EXPORT NativeWidgetAura : public internal::NativeWidgetPrivate,
   // Can we be made active?
   bool can_activate_;
 
-#if defined(USE_X11)
-  // Indicates if we should handle the upcoming Alt key release event.
-  bool should_handle_menu_key_release_;
-#endif
-
   gfx::NativeCursor cursor_;
 
   // The saved window state for exiting full screen state.
diff --git a/ui/views/widget/native_widget_gtk.cc b/ui/views/widget/native_widget_gtk.cc
index 772d780..7471ae7 100644
--- a/ui/views/widget/native_widget_gtk.cc
+++ b/ui/views/widget/native_widget_gtk.cc
@@ -355,7 +355,6 @@ NativeWidgetGtk::NativeWidgetGtk(internal::NativeWidgetDelegate* delegate)
       has_focus_(false),
       always_on_top_(false),
       is_double_buffered_(false),
-      should_handle_menu_key_release_(false),
       dragged_view_(NULL),
       painted_(false),
       has_pointer_grab_(false),
@@ -562,33 +561,7 @@ void NativeWidgetGtk::ActiveWindowChanged(GdkWindow* active_window) {
 bool NativeWidgetGtk::HandleKeyboardEvent(const KeyEvent& key) {
   if (!GetWidget()->GetFocusManager())
     return false;
-
-  const int key_code = key.key_code();
-  bool handled = false;
-
-  // Always reset |should_handle_menu_key_release_| unless we are handling a
-  // VKEY_MENU key release event. It ensures that VKEY_MENU accelerator can only
-  // be activated when handling a VKEY_MENU key release event which is preceded
-  // by an un-handled VKEY_MENU key press event.
-  if (key_code != ui::VKEY_MENU || key.type() != ui::ET_KEY_RELEASED)
-    should_handle_menu_key_release_ = false;
-
-  if (key.type() == ui::ET_KEY_PRESSED) {
-    // VKEY_MENU is triggered by key release event.
-    // FocusManager::OnKeyEvent() returns false when the key has been consumed.
-    if (key_code != ui::VKEY_MENU)
-      handled = !GetWidget()->GetFocusManager()->OnKeyEvent(key);
-    else
-      should_handle_menu_key_release_ = true;
-  } else if (key_code == ui::VKEY_MENU && should_handle_menu_key_release_ &&
-             (key.flags() & ~ui::EF_ALT_DOWN) == 0) {
-    // Trigger VKEY_MENU when only this key is pressed and released, and both
-    // press and release events are not handled by others.
-    ui::Accelerator accelerator(ui::VKEY_MENU, false, false, false);
-    handled = GetWidget()->GetFocusManager()->ProcessAccelerator(accelerator);
-  }
-
-  return handled;
+  return GetWidget()->GetFocusManager()->OnKeyEvent(key);
 }
 
 bool NativeWidgetGtk::SuppressFreezeUpdates() {
@@ -1606,18 +1579,22 @@ gboolean NativeWidgetGtk::OnScroll(GtkWidget* widget, GdkEventScroll* event) {
   return delegate_->OnMouseEvent(mouse_event);
 }
 
-gboolean NativeWidgetGtk::OnFocusIn(GtkWidget* widget, GdkEventFocus* event) {
+gboolean NativeWidgetGtk::OnFocusIn(GtkWidget* gtk_widget,
+                                    GdkEventFocus* event) {
   if (has_focus_)
     return false;  // This is the second focus-in event in a row, ignore it.
   has_focus_ = true;
 
-  should_handle_menu_key_release_ = false;
+  Widget* widget = GetWidget();
 
-  if (!GetWidget()->is_top_level())
+  if (widget->GetFocusManager())
+    widget->GetFocusManager()->ResetMenuKeyState();
+
+  if (!widget->is_top_level())
     return false;
 
   // Only top-level Widget should have an InputMethod instance.
-  InputMethod* input_method = GetWidget()->GetInputMethod();
+  InputMethod* input_method = widget->GetInputMethod();
   if (input_method)
     input_method->OnFocus();
 
@@ -1627,7 +1604,7 @@ gboolean NativeWidgetGtk::OnFocusIn(GtkWidget* widget, GdkEventFocus* event) {
     // Sets initial focus here. On X11/Gtk, window creation
     // is asynchronous and a focus request has to be made after a window
     // gets created.
-    GetWidget()->SetInitialFocus();
+    widget->SetInitialFocus();
   }
   return false;
 }
@@ -1772,12 +1749,8 @@ void NativeWidgetGtk::ScheduleDraw() {
 }
 
 void NativeWidgetGtk::DispatchKeyEventPostIME(const KeyEvent& key) {
-  // Always reset |should_handle_menu_key_release_| unless we are handling a
-  // VKEY_MENU key release event. It ensures that VKEY_MENU accelerator can only
-  // be activated when handling a VKEY_MENU key release event which is preceded
-  // by an unhandled VKEY_MENU key press event. See also HandleKeyboardEvent().
-  if (key.key_code() != ui::VKEY_MENU || key.type() != ui::ET_KEY_RELEASED)
-    should_handle_menu_key_release_ = false;
+  if (GetWidget()->GetFocusManager())
+    GetWidget()->GetFocusManager()->MaybeResetMenuKeyState(key);
 
   // Send the key event to View hierarchy first.
   bool handled = delegate_->OnKeyEvent(key);
diff --git a/ui/views/widget/native_widget_gtk.h b/ui/views/widget/native_widget_gtk.h
index d3437d4..dd0f8d7 100644
--- a/ui/views/widget/native_widget_gtk.h
+++ b/ui/views/widget/native_widget_gtk.h
@@ -441,9 +441,6 @@ class VIEWS_EXPORT NativeWidgetGtk : public internal::NativeWidgetPrivate,
   // This is false by default.
   bool is_double_buffered_;
 
-  // Indicates if we should handle the upcoming Alt key release event.
-  bool should_handle_menu_key_release_;
-
   // Valid for the lifetime of StartDragForViewFromMouseEvent, indicates the
   // view the drag started from.
   View* dragged_view_;