[Ozone/X11] events can be dispatched to the wrong window |
|||
Issue descriptionmus/external window mode support is being we are working out ( bug 666958 ), and multiple chrome windows is already possible - see https://www.youtube.com/watch?v=w0IQ6N781iQ . When in when chrome is running LinuxOs/Ozone/X11, platform windows are constructed using ui/platform_window/x11/x11_window_ozone.cc , which works mostly fine. Problem is that when there are two chrome windows, and the windows are overlapping each other, mouse events can get dispatched to the wrong window. In code, this is because X11WindowOzone::CanDispatchEvent only checks if the given event location is contained by a window bounds. It does not check whether the event was original targeting that window. (ui/platform_window/x11/x11_window_ozone.cc): 64 bool X11WindowOzone::CanDispatchEvent(const PlatformEvent& platform_event) { (..) 72 73 // TODO(kylechar): We may need to do something special for TouchEvents similar 74 // to how DrmWindowHost handles them. 75 if (static_cast<Event*>(platform_event)->IsLocatedEvent()) { 76 const LocatedEvent* event = 77 static_cast<const LocatedEvent*>(platform_event); 78 return GetBounds().Contains(event->root_location()); 79 } 80 81 return true; 82 } X11WindowOzone::CanDispatchEvent takes a ui::Event, and there is no access to the associated Xevent, to query the event's original window. In the case of ui/platform_window/x11/x11_window.cc (non-ozone), X11WindowOzone::CanDispatchEvent is also called, but it takes as parameter a XEvent, and we can call ::IsEventForXWindow. This checks if the Xevent targets a given xwindow 'A'. The same check is done on DesktopWindowTreeHostX11::CanDispatchEvent (ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc): 1964 bool DesktopWindowTreeHostX11::CanDispatchEvent( 1965 const ui::PlatformEvent& event) { 1966 return event->xany.window == xwindow_ || 1967 (event->type == GenericEvent && 1968 static_cast<XIDeviceEvent*>(event->xcookie.data)->event == xwindow_); 1969 }
,
Nov 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1b45b891d8cdf480b1862915027f160751ca544f commit 1b45b891d8cdf480b1862915027f160751ca544f Author: Maksim Sisov <msisov@igalia.com> Date: Mon Nov 27 10:02:51 2017 [ozone/x11] events can be dispatched to the wrong window When multiple X11Window/X11WindowOzone instances are created, each registers itself in PlatformEventSource as a "platform event dispatcher". Before dispatching an event to a specific window, PlatformEventSource iterates over the list of registered dispatchers, "asking" each dispatcher whether the event can be dispatch to it. In case of non-Ozone X11 windows, X11Window::CanDispatchEvent is called receiving a XEvent* parameter, and checks whether the xwindow targeted by the event is the same as the xwindow it wraps (see::IsEventForXWindow). If so it dispatches the event to this X11Window instance. In case of Ozone X11 windows, X11WindowOzone::CanDispatchEvent is called receiving a ui::Event* parameter, and it is not possible to access its associated NativeEvent*, because event was manually constructed in EventSourceLibevent::ProcessXEvent. Hence, ::CanDispatchEvent does not check if it is the event's target window, but instead whether the event location is contained in its bounds. This is fragile: if there are two overlapping X11WindowOzone instances, the first one that registered itself in PlatformEventSource will be the one receiving mouse events in the overlapping area, even if it is not the foreground window. CL fixes it by extending XEventDispatcher interface with three new methods: ::CheckCanDispatchNextPlatformEvent, ::PlatformEventDispatchFinished, and ::GetPlatformEventDispatcher. First, when an XEventDispatcher adds itself to the XEvenDispatchers list, it is explicitly called back to get a PlatformEventDispatcher to be added to a PlatformEventDispatcher list. This is done to ensure that an XEventDispatcher, which has also a PlatformEventDispatcher, is added to the list of PlatformEventDispatchers to ensure a proper handling of XEvents and PlatformEvent Once the XEvent is translated to an ui::Event, XEventDispatchers are iterated and asked to ::CheckCanDispatchNextPlatformEvent if they can process the translated event based on XEvent::XID. If so, they must make an internal promise to process next translated event sent by PlatformEventSource. Once done, ::PlatformEventDispatchFinished is called and the promise must be reset. Originally based on https://codereview.chromium.org/2791693003/ BUG= 707406 Change-Id: I2bfd1f3f0f5ed574e7ad9634ade7020d5a99dba5 Reviewed-on: https://chromium-review.googlesource.com/774778 Commit-Queue: Maksim Sisov <msisov@igalia.com> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Reviewed-by: kylechar <kylechar@chromium.org> Reviewed-by: Antonio Gomes <tonikitoo@igalia.com> Cr-Commit-Position: refs/heads/master@{#519265} [modify] https://crrev.com/1b45b891d8cdf480b1862915027f160751ca544f/ui/events/platform/x11/x11_event_source_libevent.cc [modify] https://crrev.com/1b45b891d8cdf480b1862915027f160751ca544f/ui/events/platform/x11/x11_event_source_libevent.h [modify] https://crrev.com/1b45b891d8cdf480b1862915027f160751ca544f/ui/ozone/platform/x11/gl_surface_glx_ozone.cc [modify] https://crrev.com/1b45b891d8cdf480b1862915027f160751ca544f/ui/ozone/platform/x11/gl_surface_glx_ozone.h [modify] https://crrev.com/1b45b891d8cdf480b1862915027f160751ca544f/ui/platform_window/x11/x11_window_ozone.cc [modify] https://crrev.com/1b45b891d8cdf480b1862915027f160751ca544f/ui/platform_window/x11/x11_window_ozone.h
,
Nov 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/08133d365ecb42b0a04d244fb4d40e4d0d0be9f6 commit 08133d365ecb42b0a04d244fb4d40e4d0d0be9f6 Author: Maksim Sisov <msisov@igalia.com> Date: Wed Nov 29 15:28:49 2017 Make X11WindowManagerOzone take X11WindowOzone instead of XID now. This is change doesn't bring any functionality change, but takes X11WindowOzone as an event_grabber instead. X11WindowOzone will be passed back to X11WindowOzone on request once events come and the events will be rerouted to that window once explicit grab is set. Bug: 707406 Change-Id: If35e5de10701e5eb71453ec19d007bcd2b699531 Reviewed-on: https://chromium-review.googlesource.com/790772 Commit-Queue: Maksim Sisov <msisov@igalia.com> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Cr-Commit-Position: refs/heads/master@{#520120} [modify] https://crrev.com/08133d365ecb42b0a04d244fb4d40e4d0d0be9f6/ui/platform_window/x11/x11_window_manager_ozone.cc [modify] https://crrev.com/08133d365ecb42b0a04d244fb4d40e4d0d0be9f6/ui/platform_window/x11/x11_window_manager_ozone.h [modify] https://crrev.com/08133d365ecb42b0a04d244fb4d40e4d0d0be9f6/ui/platform_window/x11/x11_window_ozone.cc
,
Nov 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/53549f0a1c27b29c88511bf330ecc428006c77ed commit 53549f0a1c27b29c88511bf330ecc428006c77ed Author: Maksim Sisov <msisov@igalia.com> Date: Thu Nov 30 13:36:18 2017 Move and rename ConvertEventToDifferentHost to event_utils.h This patch doesn't bring any functionality changes,but moves ConvertEventToDifferentHost to event_utils and makes it also available for ozone x11 builds. Bug: 707406 Change-Id: Iab9a034904d22464cb6d3e90f8511979f1932fdd Reviewed-on: https://chromium-review.googlesource.com/790771 Commit-Queue: Maksim Sisov <msisov@igalia.com> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Reviewed-by: Robert Kroeger <rjkroege@chromium.org> Cr-Commit-Position: refs/heads/master@{#520527} [modify] https://crrev.com/53549f0a1c27b29c88511bf330ecc428006c77ed/ui/events/event_utils.cc [modify] https://crrev.com/53549f0a1c27b29c88511bf330ecc428006c77ed/ui/events/event_utils.h [modify] https://crrev.com/53549f0a1c27b29c88511bf330ecc428006c77ed/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc [modify] https://crrev.com/53549f0a1c27b29c88511bf330ecc428006c77ed/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h
,
Jan 4 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bfdecad144812f52f0094c6f0ccdb8bb0bf1cad3 commit bfdecad144812f52f0094c6f0ccdb8bb0bf1cad3 Author: Maksim Sisov <msisov@igalia.com> Date: Thu Jan 04 17:35:25 2018 Revert "Move and rename ConvertEventToDifferentHost to event_utils.h" This reverts commit 53549f0a1c27b29c88511bf330ecc428006c77ed. Reason for revert: Blocks stable release. Issue 794140. Will revert the revert once issue is solved. Original change's description: > Move and rename ConvertEventToDifferentHost to event_utils.h > > This patch doesn't bring any functionality changes,but > moves ConvertEventToDifferentHost to event_utils and makes > it also available for ozone x11 builds. > > Bug: 707406 > Change-Id: Iab9a034904d22464cb6d3e90f8511979f1932fdd > Reviewed-on: https://chromium-review.googlesource.com/790771 > Commit-Queue: Maksim Sisov <msisov@igalia.com> > Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> > Reviewed-by: Robert Kroeger <rjkroege@chromium.org> > Cr-Commit-Position: refs/heads/master@{#520527} TBR=sadrul@chromium.org,rjkroege@chromium.org,msisov@igalia.com Change-Id: I3b146dfe9dc860bb0c0d2c45503347d6dd452d5f No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 707406 Reviewed-on: https://chromium-review.googlesource.com/851012 Reviewed-by: Maksim Sisov <msisov@igalia.com> Commit-Queue: Maksim Sisov <msisov@igalia.com> Cr-Commit-Position: refs/heads/master@{#527025} [modify] https://crrev.com/bfdecad144812f52f0094c6f0ccdb8bb0bf1cad3/ui/events/event_utils.cc [modify] https://crrev.com/bfdecad144812f52f0094c6f0ccdb8bb0bf1cad3/ui/events/event_utils.h [modify] https://crrev.com/bfdecad144812f52f0094c6f0ccdb8bb0bf1cad3/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc [modify] https://crrev.com/bfdecad144812f52f0094c6f0ccdb8bb0bf1cad3/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h
,
Jan 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e46169bf03375205b6cf62cde0d68ff88c80ab14 commit e46169bf03375205b6cf62cde0d68ff88c80ab14 Author: Maksim Sisov <msisov@igalia.com> Date: Tue Jan 23 09:07:38 2018 Reland "Move and rename ConvertEventToDifferentHost to event_utils.h" This relands commit 53549f0a1c2, reverted by bfdecad144, due to https://crbug.com/794140 Added a check in desktop_window_tree_host_x11, which doesn't send a MouseEvents, when a type of events is UNKNOWN. Events' type can be resolved to UNKNOWN because of a bug in DeviceDataManagerX11, when an event takes a path meant for events from cmt devices, which doesn't seem to exist on Linux, but ChromeOs. Original message stated that this patch doesn't bring any functionality changes,but moves ConvertEventToDifferentHost to event_utils and makes it also available for ozone x11 builds, but actually it has revealed a bug with coalescing XEvents. Bug: 707406 , 794140, 804418 Change-Id: Ic57be5baad2e4d9fb0543ee1b0a83294aee470f6 Reviewed-on: https://chromium-review.googlesource.com/853953 Commit-Queue: Maksim Sisov <msisov@igalia.com> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Cr-Commit-Position: refs/heads/master@{#531189} [modify] https://crrev.com/e46169bf03375205b6cf62cde0d68ff88c80ab14/ui/events/event_utils.cc [modify] https://crrev.com/e46169bf03375205b6cf62cde0d68ff88c80ab14/ui/events/event_utils.h [modify] https://crrev.com/e46169bf03375205b6cf62cde0d68ff88c80ab14/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc [modify] https://crrev.com/e46169bf03375205b6cf62cde0d68ff88c80ab14/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h
,
Feb 10 2018
A crash caused by 53549f0a1c27b29c88511bf330ecc428006c77ed on Chromium 64.0.3282.140 is being discussed in issue 808741 .
,
Feb 26 2018
,
Feb 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/da0fbc1c5e0a5506514014078893235dd361e2b9 commit da0fbc1c5e0a5506514014078893235dd361e2b9 Author: kylechar <kylechar@chromium.org> Date: Wed Feb 28 19:29:03 2018 Move Ozone X11 files into ui/ozone/platform/x11 This CL moves implementation files for Ozone X11 into a more appropriate location in ui/ozone/platform/x11, instead of ui/platform_window/x11. Bug: 707406 Change-Id: If5673cc5283bb523eb5b8b1a8aa4d0b97133ea94 Reviewed-on: https://chromium-review.googlesource.com/941481 Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Commit-Queue: kylechar <kylechar@chromium.org> Cr-Commit-Position: refs/heads/master@{#539920} [modify] https://crrev.com/da0fbc1c5e0a5506514014078893235dd361e2b9/ui/ozone/platform/x11/BUILD.gn [modify] https://crrev.com/da0fbc1c5e0a5506514014078893235dd361e2b9/ui/ozone/platform/x11/ozone_platform_x11.cc [modify] https://crrev.com/da0fbc1c5e0a5506514014078893235dd361e2b9/ui/ozone/platform/x11/x11_cursor_factory_ozone.h [rename] https://crrev.com/da0fbc1c5e0a5506514014078893235dd361e2b9/ui/ozone/platform/x11/x11_cursor_ozone.cc [rename] https://crrev.com/da0fbc1c5e0a5506514014078893235dd361e2b9/ui/ozone/platform/x11/x11_cursor_ozone.h [rename] https://crrev.com/da0fbc1c5e0a5506514014078893235dd361e2b9/ui/ozone/platform/x11/x11_window_manager_ozone.cc [rename] https://crrev.com/da0fbc1c5e0a5506514014078893235dd361e2b9/ui/ozone/platform/x11/x11_window_manager_ozone.h [rename] https://crrev.com/da0fbc1c5e0a5506514014078893235dd361e2b9/ui/ozone/platform/x11/x11_window_ozone.cc [rename] https://crrev.com/da0fbc1c5e0a5506514014078893235dd361e2b9/ui/ozone/platform/x11/x11_window_ozone.h [modify] https://crrev.com/da0fbc1c5e0a5506514014078893235dd361e2b9/ui/platform_window/x11/BUILD.gn
,
Mar 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ade517cc9004f334919525bd4cb6d0eecc2505f4 commit ade517cc9004f334919525bd4cb6d0eecc2505f4 Author: Maksim Sisov <msisov@igalia.com> Date: Tue Mar 13 21:57:22 2018 [ozone/x11] Resolve event clicking locations in X11 This patch concerns so called external window mode, which is being developed downstream now [1]. The problem this CL fixes is the following: Even though events are passed to the right X11WindowOzone based on an XID target now, there can be cases when the events must be rerouted to another X11WindowOzone, for example, a context menu or a nested 3-dot menu. In case of a nested 3-dot menu, events must always be passed to the parent menu, so that the menu controller would handle events properly. Another case is handling events when a user clicks outside the menu's area: even though the events can be targeted for another window, there can be an explicit grab that must reroute them to the menu window, which will be closed as soon as a user clicks outside the menu's area. The patch fixes the above mentioned problem the following way: 1) Once an explicit grab is set, the events are passed to an X11WindowOzone have their locations converted and are rerouted to a grabbed window. 2) Whenever a grabbed window looses an explicit capture, X11WindowManager tells X11WindowOzone the capture is lost and the |delegate_| is notified about that. [1] https://github.com/Igalia/chromium Bug: 707406 Change-Id: I1b00e79c8ddc17b000c2f55b7f39c9ea16c0c71a Reviewed-on: https://chromium-review.googlesource.com/790773 Commit-Queue: Maksim Sisov <msisov@igalia.com> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Reviewed-by: Robert Kroeger <rjkroege@chromium.org> Reviewed-by: kylechar <kylechar@chromium.org> Reviewed-by: Michael Spang <spang@chromium.org> Cr-Commit-Position: refs/heads/master@{#542927} [modify] https://crrev.com/ade517cc9004f334919525bd4cb6d0eecc2505f4/ui/events/BUILD.gn [modify] https://crrev.com/ade517cc9004f334919525bd4cb6d0eecc2505f4/ui/ozone/platform/x11/BUILD.gn [modify] https://crrev.com/ade517cc9004f334919525bd4cb6d0eecc2505f4/ui/ozone/platform/x11/x11_window_manager_ozone.cc [modify] https://crrev.com/ade517cc9004f334919525bd4cb6d0eecc2505f4/ui/ozone/platform/x11/x11_window_manager_ozone.h [modify] https://crrev.com/ade517cc9004f334919525bd4cb6d0eecc2505f4/ui/ozone/platform/x11/x11_window_ozone.cc [modify] https://crrev.com/ade517cc9004f334919525bd4cb6d0eecc2505f4/ui/ozone/platform/x11/x11_window_ozone.h [add] https://crrev.com/ade517cc9004f334919525bd4cb6d0eecc2505f4/ui/ozone/platform/x11/x11_window_ozone_unittest.cc
,
Jun 13 2018
|
|||
►
Sign in to add a comment |
|||
Comment 1 by toniki...@chromium.org
, Mar 31 2017Status: started (was: Untriaged)