Number of immersive tests fail in mash |
||||||||
Issue descriptionThey need to be investigated.
,
Mar 7 2017
,
Nov 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f8675cbb337440a11bf9afb10ea11bae42bb92cb commit f8675cbb337440a11bf9afb10ea11bae42bb92cb Author: James Cook <jamescook@chromium.org> Date: Fri Nov 03 17:59:35 2017 cros: Enable some tests in //ash/wm in ash_unittests --mash For the ones that fail, disable them via filter file instead of in the code, per our disablement policy. Bug: 698085, 695556, 698878 , 698888, 698093, 698894 Test: ash_unittests --mash Change-Id: Ic145ab6a95508968d6884d14fac2a3ca08888d26 Reviewed-on: https://chromium-review.googlesource.com/752423 Commit-Queue: James Cook <jamescook@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Cr-Commit-Position: refs/heads/master@{#513836} [modify] https://crrev.com/f8675cbb337440a11bf9afb10ea11bae42bb92cb/ash/wm/immersive_fullscreen_controller_unittest.cc [modify] https://crrev.com/f8675cbb337440a11bf9afb10ea11bae42bb92cb/ash/wm/overview/window_selector_unittest.cc [modify] https://crrev.com/f8675cbb337440a11bf9afb10ea11bae42bb92cb/ash/wm/panels/panel_layout_manager_unittest.cc [modify] https://crrev.com/f8675cbb337440a11bf9afb10ea11bae42bb92cb/ash/wm/panels/panel_window_resizer_unittest.cc [modify] https://crrev.com/f8675cbb337440a11bf9afb10ea11bae42bb92cb/ash/wm/system_modal_container_layout_manager_unittest.cc [modify] https://crrev.com/f8675cbb337440a11bf9afb10ea11bae42bb92cb/ash/wm/tablet_mode/tablet_mode_window_manager_unittest.cc [modify] https://crrev.com/f8675cbb337440a11bf9afb10ea11bae42bb92cb/ash/wm/toplevel_window_event_handler_unittest.cc [modify] https://crrev.com/f8675cbb337440a11bf9afb10ea11bae42bb92cb/ash/wm/window_cycle_controller_unittest.cc [modify] https://crrev.com/f8675cbb337440a11bf9afb10ea11bae42bb92cb/testing/buildbot/filters/ash_unittests_mash.filter
,
Nov 29 2017
Did some investigation on this. While the mouse events do appear to be received by the ImmersiveFullscreenController, the pointer event handling is dependent on GetTopLevelWidgetForNativeView() not returning null, which is what happens in --mash mode. I don't know what the registration needs to happen here to make this work.
,
Nov 29 2017
Current theory: there's a big mismatch between how PointerWatcherEventRouter works and how PointerWatcherAdapterClassic works. The previous observation was about how the target aura::Window passed to ImmersiveFullscreenController::OnPointerEventObserved. This makes sense though because what was happening was that the toplevel ash aura::Window was being passed in. In classic ash/mus, the very specific target window from the ImmersiveFullscreenControllerTests is passed in. My explanation for *that* is that all pointer watching in classic ash/mus is done through PointerWatcherAdapterClassic which finds the target window by grabbing the target window off the ui::Event that it's about to dispatch. The pointer watching in mash works through WindowTreeClient::OnWindowInputEvent dispatching through the PointerWatcherEventRouter. This is using the mus Window as the target which differs from just stealing the original targeting out of the event. I'm not yet sure how much targeting should be performed here. This is worrisome since there's 2 PointerWatchers which are like ImmersiveFullscreenController and make decisions based on the |target|'s Widget.
,
Nov 30 2017
jamescook@ pointed out that the ImmersiveFullscreenController used to handle targeting properly. I went to the commit before cfee1fb0 (which changed the interface from Widget*s to aura::Windows) and at least some of the IFC tests worked and were performing targeting correctly.
,
Nov 30 2017
I wonder if the thread from #6 is a red herring. Actually tracing through the IFC tests on cfee1fb0^, we're creating the PointerWatcherEventRouter and are adding observers in --mash...but IFCTests.Delegate never actually goes through the PointerWatcherEventRouter, or the classic ash PointerWatcherAdapterClassic. Running cfee1fb0 itself also passes. I suspect that this might have worked on accident.
,
Dec 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3738ee850a6bdca56b1e8967760f7cf718130f7d commit 3738ee850a6bdca56b1e8967760f7cf718130f7d Author: Elliot Glaysher <erg@chromium.org> Date: Fri Dec 01 22:24:01 2017 Start fixing ImmersiveFullscreenController tests. The OnPointerEventObserved() interface acts differently between mash and mus/classic. In mus/classic, the target window is taken from the LocatedEvent's target(), relying on previous event dispatch metadata. On mash, this event came from a remote service which hasn't been targeted, target is therefore the top of the window tree. In the --mash case only, target the event on the window and use that as the window we pass to the rest of the system. Bug: 698085 Change-Id: I8a1859055d9427b269cd5256c7ff114a1e97574d Reviewed-on: https://chromium-review.googlesource.com/801976 Reviewed-by: James Cook <jamescook@chromium.org> Commit-Queue: Elliot Glaysher <erg@chromium.org> Cr-Commit-Position: refs/heads/master@{#521090} [modify] https://crrev.com/3738ee850a6bdca56b1e8967760f7cf718130f7d/ash/window_manager.cc [modify] https://crrev.com/3738ee850a6bdca56b1e8967760f7cf718130f7d/testing/buildbot/filters/ash_unittests_mash.filter
,
Dec 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d26fedcf5784cd7e087a3d367e6578da7ab02413 commit d26fedcf5784cd7e087a3d367e6578da7ab02413 Author: Elliot Glaysher <erg@chromium.org> Date: Mon Dec 11 19:16:37 2017 Revert "Start fixing ImmersiveFullscreenController tests." This reverts commit 3738ee850a6bdca56b1e8967760f7cf718130f7d. Reason for revert: Patch is wrong, and is probably causing a crash on device. When running ash, target is actually set correctly. It's not set correctly in the unit tests though. Original change's description: > Start fixing ImmersiveFullscreenController tests. > > The OnPointerEventObserved() interface acts differently between mash and > mus/classic. In mus/classic, the target window is taken from the > LocatedEvent's target(), relying on previous event dispatch metadata. On > mash, this event came from a remote service which hasn't been targeted, > target is therefore the top of the window tree. In the --mash case only, > target the event on the window and use that as the window we pass to the > rest of the system. > > Bug: 698085 > Change-Id: I8a1859055d9427b269cd5256c7ff114a1e97574d > Reviewed-on: https://chromium-review.googlesource.com/801976 > Reviewed-by: James Cook <jamescook@chromium.org> > Commit-Queue: Elliot Glaysher <erg@chromium.org> > Cr-Commit-Position: refs/heads/master@{#521090} TBR=jamescook@chromium.org,erg@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 698085, 793839 Change-Id: I5d9325b72853c8793f5ce1f3e66fe74461eea9be Reviewed-on: https://chromium-review.googlesource.com/820290 Reviewed-by: Elliot Glaysher <erg@chromium.org> Commit-Queue: Elliot Glaysher <erg@chromium.org> Cr-Commit-Position: refs/heads/master@{#523163} [modify] https://crrev.com/d26fedcf5784cd7e087a3d367e6578da7ab02413/ash/window_manager.cc [modify] https://crrev.com/d26fedcf5784cd7e087a3d367e6578da7ab02413/testing/buildbot/filters/ash_unittests_mash.filter
,
Jan 4 2018
,
Jan 24 2018
,
Jan 24 2018
,
Mar 6 2018
Mass unassigning bugs
,
Apr 19 2018
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by bugdroid1@chromium.org
, Mar 3 2017