Enable keyboard accelerators while a menu is open

ash/accelerators/ash_menu_event_filter_delegate.cc

Index: ash/accelerators/ash_menu_event_filter_delegate.cc
diff --git a/ash/accelerators/ash_menu_event_filter_delegate.cc b/ash/accelerators/ash_menu_event_filter_delegate.cc
new file mode 100644
index 0000000000000000000000000000000000000000..015f390778f271b7fc0970f3477105707d654cb1
--- /dev/null
+++ b/ash/accelerators/ash_menu_event_filter_delegate.cc
@@ -0,0 +1,65 @@
+// Copyright 2015 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 "ash/accelerators/ash_menu_event_filter_delegate.h"
+
+#include "ash/accelerators/accelerator_controller.h"
+#include "ash/shell.h"
+#include "ash/wm/window_util.h"
+#include "ui/aura/window.h"
+#include "ui/views/controls/menu/menu_event_filter.h"
+#include "ui/views/focus/focus_manager.h"
+#include "ui/views/widget/widget.h"
+
+namespace ash {
+
+AshMenuEventFilterDelegate::AshMenuEventFilterDelegate(
+    ui::AcceleratorHistory* history)
+    : accelerator_history_(history) {
+  DCHECK(history);
+}
+
+AshMenuEventFilterDelegate::~AshMenuEventFilterDelegate() {}
+
+void AshMenuEventFilterDelegate::StoreInHistory(
+    const ui::Accelerator& accelerator) {
+  accelerator_history_->StoreCurrentAccelerator(accelerator);
+}
+
+views::MenuEventFilter::Delegate::Result
+AshMenuEventFilterDelegate::ProcessAccelerator(
+    const ui::Accelerator& accelerator) {
+  ash::AcceleratorController* accelerator_controller =
+      ash::Shell::GetInstance()->accelerator_controller();
+  if (accelerator_controller->ShouldMenuBeKeptOpenForAccelerator(accelerator)) {
+    // Menu will be kept open and accelerator will be handled now.
+    ProcessAcceleratorNow(accelerator);
+    return views::MenuEventFilter::Delegate::RESULT_PROCESSED;
+  }
+
+  // Menu should be closed and the accelerator should be reposted for processing
+  // later by the MenuEventFilter after the menu's nested runloop exits.
+  return views::MenuEventFilter::Delegate::RESULT_PROCESS_LATER;
+}
+
+void AshMenuEventFilterDelegate::ProcessAcceleratorNow(
+    const ui::Accelerator& accelerator) {
+  aura::Window* active_window = ash::wm::GetActiveWindow();
+  if (active_window) {
+    views::Widget* widget =
+        views::Widget::GetWidgetForNativeWindow(active_window);
+    if (widget) {
+      views::FocusManager* focus_manager = widget->GetFocusManager();
+      DCHECK(focus_manager);
+      if (focus_manager->ProcessAccelerator(accelerator))

[pkotwicz, 2015/08/26 01:26:54]

What's the thinking behind calling FocusManager::ProcessAccelerator() ?
With the CL applied pressing Ctrl+W closes the browser window when the menu was
open when it did not do so before. It is also possible to trigger any extension
browser actions which have keyboard shortcuts assigned. I do not know whether
this change is a good change. However, this change should not be part of this CL

[afakhry, 2015/08/26 18:39:21]

Exactly. I did this after a discussion with Oshima to make it possible to
performed accelerators handled by the BrowserView like Ctrl+W. Not sure if
everyone will agree to enable this. Unfortunately he's not here now to ask him
for some input. :(

I think this changes the current behavior so I removed it from this CL for now.
I think it can be easy to add it back later on if needed.

[oshima, 2015/09/02 17:08:21]

The whole point of this work is that an accelerator should work regardless of
the UI state as long as it makes sense. It's very confusing if ctrl-t can open a
new tab, while ctrl-w does not work, or ctrl-p does not work.

I'm ok to do this in a separate CL though.

+        return;
+    }
+  }
+
+  ash::AcceleratorController* accelerator_controller =
+      ash::Shell::GetInstance()->accelerator_controller();
+  accelerator_controller->Process(accelerator);
+}
+
+}  // namespace ash