New issue
Advanced search Search tips

Issue 631545 link

Starred by 3 users

Issue metadata

Status: WontFix
Owner: ----
Closed: Aug 13
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 612331



Sign in to add a comment

mash needs to register accelerators for both pre and post

Project Member Reported by sky@chromium.org, Jul 26 2016

Issue description

Here's how ash processes accelerators:

  // The flow of accelerators in ash is:
  // . wm::AcceleratorFilter() sees events first as it's a pre-target handler.
  // . AcceleratorFilter forwards to its delegate, which indirectly is
  //   implemented by AcceleratorRouter.
  // . AcceleratorRouter may early out, if not it calls through to
  //   AcceleratorController. This stop may stop propagation entirely.
  // . If focus is on a Widget, then NativeWidgetAura gets the key event, calls
  //   to Widget::OnKeyEvent(), which calls to FocusManager::OnKeyEvent(), which
  //   calls to AshFocusManagerFactory::Delegate::ProcessAccelerator() finally
  //   ending up in AcceleratorController::Process().
  // . OTOH if focus is on a content, then
  //   RenderWidgetHostViewAura::OnKeyEvent() is called and may end up consuming
  //   the event.
 
In the mus world the closes we can get to this is to register accelerators for both pre and post processing. The pre should call to the Router, the post to the AcceleratorController.
 

Comment 1 by sky@chromium.org, Jul 26 2016

Blocking: 612331
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f0891a90a6cdb92d0a205d17df42acd81e796a30

commit f0891a90a6cdb92d0a205d17df42acd81e796a30
Author: sky <sky@chromium.org>
Date: Wed Jul 27 19:00:06 2016

Wires up registering accelerators from mash with the wm

AcceleratorControllerRegistrar is responsible for registering
accelerators known to ash (in AcceleratorController) with the wm. In addition
AcceleratorRegistrarImpl now registers keyboard accelerators with
AcceleratorController.

Accelerators are now registered for pre and post. This better matches
how ash/chrome interact to process accelerators. See comment in
AcceleratorControllerRegistrar for details on this.

BUG= 612331 , 631545 
TEST=none
R=sadrul@chromium.org

Review-Url: https://codereview.chromium.org/2187703003
Cr-Commit-Position: refs/heads/master@{#408205}

[modify] https://crrev.com/f0891a90a6cdb92d0a205d17df42acd81e796a30/ash/ash.gyp
[modify] https://crrev.com/f0891a90a6cdb92d0a205d17df42acd81e796a30/ash/common/accelerators/accelerator_controller.cc
[modify] https://crrev.com/f0891a90a6cdb92d0a205d17df42acd81e796a30/ash/common/accelerators/accelerator_controller.h
[rename] https://crrev.com/f0891a90a6cdb92d0a205d17df42acd81e796a30/ash/common/accelerators/ash_focus_manager_factory.cc
[rename] https://crrev.com/f0891a90a6cdb92d0a205d17df42acd81e796a30/ash/common/accelerators/ash_focus_manager_factory.h
[modify] https://crrev.com/f0891a90a6cdb92d0a205d17df42acd81e796a30/ash/common/wm_shell.cc
[modify] https://crrev.com/f0891a90a6cdb92d0a205d17df42acd81e796a30/ash/mus/BUILD.gn
[add] https://crrev.com/f0891a90a6cdb92d0a205d17df42acd81e796a30/ash/mus/accelerators/accelerator_controller_registrar.cc
[add] https://crrev.com/f0891a90a6cdb92d0a205d17df42acd81e796a30/ash/mus/accelerators/accelerator_controller_registrar.h
[modify] https://crrev.com/f0891a90a6cdb92d0a205d17df42acd81e796a30/ash/mus/accelerators/accelerator_registrar_impl.cc
[modify] https://crrev.com/f0891a90a6cdb92d0a205d17df42acd81e796a30/ash/mus/accelerators/accelerator_registrar_impl.h
[modify] https://crrev.com/f0891a90a6cdb92d0a205d17df42acd81e796a30/ash/mus/accelerators/accelerator_registrar_unittest.cc
[modify] https://crrev.com/f0891a90a6cdb92d0a205d17df42acd81e796a30/ash/mus/bridge/wm_shell_mus.cc
[modify] https://crrev.com/f0891a90a6cdb92d0a205d17df42acd81e796a30/ash/mus/bridge/wm_shell_mus.h
[modify] https://crrev.com/f0891a90a6cdb92d0a205d17df42acd81e796a30/ash/mus/window_manager.cc
[modify] https://crrev.com/f0891a90a6cdb92d0a205d17df42acd81e796a30/ash/shell.cc
[modify] https://crrev.com/f0891a90a6cdb92d0a205d17df42acd81e796a30/ui/base/accelerators/accelerator_history.cc

Comment 3 by sky@chromium.org, Aug 24 2016

Labels: Proj-Mustash-Mash
Components: MUS
Labels: Proj-Mustash
Components: Internals>MUS
Components: -Internals>MUS Internals>Services>WindowService
Components: -MUS
Components: -Internals>Services>WindowService Internals>Services>Ash
Labels: -Proj-Mustash-Mash
Status: WontFix (was: Untriaged)
Now that ash runs the window-service, this is no longer an issue.

Sign in to add a comment