mash: Support ui::EventHandlers that handle events outside the client |
|||||||||
Issue descriptionmash: Support ui::EventHandlers that handle events outside the client This is similar to Issue 647781 regarding EventRewriters. The Window Service only notifies clients about events targeting each clients' windows. However, some Chrome code assumes it can observe/handle events targeting Ash and mojo app windows. We'll need to update some ui::EventHandlers for SingleProcessMash and more for multi-process Mash. This issue will serve as a tracking bug for the work needed, and I'll file blocking bugs. Here's a concise first pass over the code that may need attention (some window pre-target handlers are questionable): *** env.*AddPreTargetHandler - https://cs.chromium.org/search/?q=env.*AddPreTargetHandler&type=cs 6 interesting cases, 3 need SingleProcessMash fixes (convert 2 to EventObserver, unknown action for ui_devtools) ImmersiveGestureHandler - marks events as handled (SingleProcessMash ok, needs Mash impl). Reveals top chrome, allows dragging window? ImmersiveHandlerFactoryMus::CreateGestureHandler is NOTIMPLEMENTED. See Issue 640365 and Issue 791148 MenuPreTargetHandlerAura - stops event propagation (SingleProcessMash ok w/warning?, needs Mash impl to handle null widget owner/root?). Consumes key events while a menu is showing. https://cs.chromium.org/chromium/src/ui/views/controls/menu/menu_pre_target_handler_aura.cc?rcl=c0f0cb93c2e34208e09114032ea0ae9ecf92f51d&l=35 RenderWidgetHostViewAura::EventFilterForPopupExit - observes events (SingleProcessMash broken, convert to EventObserver) Closes web popups on mouse/touch press outside the window/parent. https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_view_aura.cc?rcl=b0e3983f7a7a814c285c2302b16ffd345ba37815&l=203 OverlayAgentAura - marks events as handled (SingleProcessMash broken? needs system priority?). A ui_devtools feature for inspecting native views ui https://cs.chromium.org/chromium/src/components/ui_devtools/views/overlay_agent_aura.cc?rcl=57d7dded44f76ed86c845232d22b678fdb441e46&l=390 TouchSelectionControllerClientAura::EnvPreTargetHandler - observes events (SingleProcessMash broken, convert to EventObserver) Closes selection/handles/menu on events outside the client window, fix like TouchSelectionControllerImpl https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/touch_selection_controller_client_aura.cc?rcl=a563e29ee53b9b2f5e8a35da7c52f96310636a02&l=75 *** shell.*AddPreTargetHandler - https://cs.chromium.org/search/?q=shell.*AddPreTargetHandler&type=cs 3 interesting cases, no SingleProcessMash action needed exo::WmHelper::AddPreTargetHandler/PrependPreTargetHandler/AddPostTargetHandler - exo::Keyboard sets events as handled (SingleProcessMash ok, needs Mash impl) Used for exo keyboard/pointer/touch input; see exo::Seat, https://cs.chromium.org/chromium/src/components/exo/wm_helper.cc?rcl=745d385ecf5ac39ca2fd604dd5451e61e47d980b&l=203 TabScrubber - stops event propagation (SingleProcessMash ok, needs Mash impl?) Used for 3-finger swiping to switch/preview other tabs https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/tab_scrubber.cc?rcl=30fe2105ff384d8485ab2da6bc0be124dcb4c44f&l=95 See Issue 796366 InputEventsBlocker - stops event propagation (SingleProcessMash ok, needs Mash impl) Used to temporarily block all user input See Issue 854323 - https://cs.chromium.org/chromium/src/chrome/browser/chromeos/login/ui/input_events_blocker.cc?rcl=51c372b43f79848c0fe7ae2a2b5983d78b843a19&l=17 *** Other "AddPreTargetHandler -f:test f:cc$ -f:src/ash" https://cs.chromium.org/search/?q=AddPreTargetHandler+-f:test+f:cc$+-f:src/ash&type=cs webrunner::FrameImpl (Unclear if this needs events outside the client's windows) https://cs.chromium.org/chromium/src/webrunner/browser/frame_impl.cc?rcl=bef94ee00fcb2b8a94fbd9b0323094fbc37e9601&l=193 WindowModalityController (Unclear if this needs events outside the client's windows) https://cs.chromium.org/chromium/src/ui/wm/core/window_modality_controller.cc?rcl=fd9098da9479af98825bdc1bdcaf596aceacb081&l=97 PreMenuEventDispatchHandler (Unclear if this needs events outside the client's windows) https://cs.chromium.org/chromium/src/ui/views/controls/menu/menu_host.cc?rcl=fd9098da9479af98825bdc1bdcaf596aceacb081&l=47 CastSystemGestureEventHandler (Unclear if this needs events outside the client's windows) https://cs.chromium.org/chromium/src/chromecast/graphics/gestures/cast_system_gesture_event_handler.cc?rcl=5a8004ef413b75defefa1202584392a6b06db038&l=30 chromecast::shell::TouchBlocker (Unclear if this needs events outside the client's windows) https://cs.chromium.org/chromium/src/chromecast/browser/cast_content_window_aura.cc?rcl=5a8004ef413b75defefa1202584392a6b06db038&l=24 SwitchAccessEventHandler - marks events as handled and stops event propagation (SingleProcessMash ok, needs Mash impl) Proceses key events as switch access events... https://cs.chromium.org/chromium/src/chrome/browser/chromeos/accessibility/switch_access_event_handler.cc?rcl=5a8004ef413b75defefa1202584392a6b06db038&l=19 ShellDesktopControllerAura (For app_shell, test only? Unclear if this is applicable or needs events outside the client's windows) https://cs.chromium.org/chromium/src/extensions/shell/browser/shell_desktop_controller_aura.cc?rcl=2d39a8f3a2619183d32205f748a7a35d63539a6b&l=376 MouseCursorOverlayController::Observer (Unclear if this needs events outside the client's windows) https://cs.chromium.org/chromium/src/content/browser/media/capture/mouse_cursor_overlay_controller_aura.cc?rcl=c51a63f653d3104ce53f7fa378ce662e7271c50d&l=39 chromecast::PartialMagnificationController (Unclear if this needs events outside the client's windows) https://cs.chromium.org/chromium/src/chromecast/graphics/accessibility/partial_magnification_controller.cc?rcl=c51a63f653d3104ce53f7fa378ce662e7271c50d&l=199 LoginDisplayHostWebUI::KeyboardDrivenOobeKeyHandler (Unclear if this needs events outside the client's windows) https://cs.chromium.org/chromium/src/chrome/browser/chromeos/login/ui/login_display_host_webui.cc?rcl=c51a63f653d3104ce53f7fa378ce662e7271c50d&l=384 ⛆ |
|
|
,
Oct 19
,
Oct 19
,
Oct 22
,
Oct 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/51d03801aaa0c56cd2917df7c07bd2adef2ecb72 commit 51d03801aaa0c56cd2917df7c07bd2adef2ecb72 Author: Mike Wasserman <msw@chromium.org> Date: Thu Oct 25 16:48:50 2018 mash: Convert MenuPreTargetHandlerAura owner warning to DCHECK. Menus should always have an owner when using the Window Service. Excuse a test that hits this, we want to find any non-test cases. Test context: https://codereview.chromium.org/2208113002 Bug: 896947 Test: DCHECK not hit. Change-Id: I9331e531543b11c7e344a235dca7765610b40d3c Reviewed-on: https://chromium-review.googlesource.com/c/1298321 Reviewed-by: Jonathan Ross <jonross@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#602755} [modify] https://crrev.com/51d03801aaa0c56cd2917df7c07bd2adef2ecb72/ui/views/controls/menu/menu_controller_unittest.cc [modify] https://crrev.com/51d03801aaa0c56cd2917df7c07bd2adef2ecb72/ui/views/controls/menu/menu_pre_target_handler_aura.cc
,
Nov 8
The only outstanding work here for SingleProcessMash is Issue 896977 and I have a CL out for review: https://chromium-review.googlesource.com/c/chromium/src/+/1324373 I'm going to mark this as only blocking multi-process Mash for now.
,
Dec 4
Testing the virtual keyboard in SingleProcessMash (with https://chromium-review.googlesource.com/c/chromium/src/+/1300553 applied), I am running into NOTREACHED() in TouchSelectionControllerImpl::GetSelectedText().
,
Dec 5
Can you provide repro steps? I'm not sure why the virtual keyboard would trigger a DCHECK in ui for the touch selection handles rendered over textfields. Further, I'm not sure if the problem you're hitting is related to this issue about EventHandlers.
,
Jan 8
|
||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by msw@chromium.org
, Oct 19