mash: some tab-dragging uitests are still broken |
||||||||
Issue descriptionI've fixed some of them, but the followings are still failing with --enable-features=SingleProcessMash. DetachToBrowserInSeparateDisplayAndCancelTabDragControllerTest.CancelDragTabToWindowIn1stDisplay TabDragging/DetachToBrowserInSeparateDisplayTabDragControllerTest.DragBrowserWindowWhenMajorityOfBoundsInSecondDisplay/0 TabDragging/DetachToBrowserInSeparateDisplayTabDragControllerTest.DragSingleTabToSeparateWindowInSecondDisplay/0 TabDragging/DetachToBrowserInSeparateDisplayTabDragControllerTest.DragTabToWindowInSeparateDisplay/0 TabDragging/DetachToBrowserInSeparateDisplayTabDragControllerTest.DragTabToWindowOnSecondDisplay/0 TabDragging/DetachToBrowserTabDragControllerTest.DeferredTargetTabStripTest/1 TabDragging/DetachToBrowserTabDragControllerTest.DetachToOwnWindowFromMaximizedWindow/1 TabDragging/DetachToBrowserTabDragControllerTest.DetachToOwnWindowWhileInImmersiveFullscreenMode/1 TabDragging/DetachToBrowserTabDragControllerTest.DoNotAttachToOtherWindowTest/1 TabDragging/DetachToBrowserTabDragControllerTest.DoNotObserveDraggedWidgetAfterDragEnds/1 TabDragging/DetachToBrowserTabDragControllerTest.DragToOverviewNewWindowItem/1 TabDragging/DetachToBrowserTabDragControllerTest.DragToOverviewWindow/1 TabDragging/DetachToBrowserTabDragControllerTest.DragToSeparateWindow/1 TabDragging/DetachToBrowserTabDragControllerTest.DragWithMaskedWindows/0 TabDragging/DetachToBrowserTabDragControllerTest.DragWithMaskedWindows/1 TabDragging/DetachToBrowserTabDragControllerTestTouch.PressSecondFingerWhileDetached/0 TabDragging/DetachToBrowserTabDragControllerTestTouch.SecondFingerPressTest/0 The tests with '/1' are with touch events -- I'll revisit them after fixing issue 867074 . Some are related to multiple displays.
,
Oct 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b26f9161c145f083f6a65183462d8f4b0a10118d commit b26f9161c145f083f6a65183462d8f4b0a10118d Author: Jun Mukai <mukai@chromium.org> Date: Fri Oct 26 23:38:24 2018 Send DisplayChanged on display removal My recent change on client_root actually had a side effect; the window may change its display when the display is gone (e.g. disconnected). Window server should send DisplayChanged event even in such case. BUG= 890071 TEST=the new test case covers Change-Id: I8566e6439bc8e87a0e3252aded8c9aa08cb1c7a9 Reviewed-on: https://chromium-review.googlesource.com/c/1302734 Commit-Queue: Jun Mukai <mukai@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#603252} [modify] https://crrev.com/b26f9161c145f083f6a65183462d8f4b0a10118d/ash/ws/window_service_delegate_impl_unittest.cc [modify] https://crrev.com/b26f9161c145f083f6a65183462d8f4b0a10118d/services/ws/client_root.cc
,
Oct 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/94890fd9bc726083185dbcd3d0702856fe6cae9f commit 94890fd9bc726083185dbcd3d0702856fe6cae9f Author: Jun Mukai <mukai@chromium.org> Date: Fri Oct 26 23:46:39 2018 Fix some multi-process tab-dragging tests With recent fixes for multi-display ( crbug.com/892714 ), now some tests in tab-dragging works. Still the test code needs to wait for the bounds changes, since related display ID change arrives asynchronously. Two tests are still failing because of other reasons. BUG= 890071 TEST=single_process_mash_interactive_ui_tests Change-Id: I2b73f5ded68fcf0431b67d03737cd137ea57e3d1 Reviewed-on: https://chromium-review.googlesource.com/c/1300689 Commit-Queue: Jun Mukai <mukai@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#603257} [modify] https://crrev.com/94890fd9bc726083185dbcd3d0702856fe6cae9f/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc [modify] https://crrev.com/94890fd9bc726083185dbcd3d0702856fe6cae9f/testing/buildbot/filters/chromeos.single_process_mash.interactive_ui_tests.filter
,
Oct 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b027b38a0726d515cc725be85247fa353166ee46 commit b027b38a0726d515cc725be85247fa353166ee46 Author: Jun Mukai <mukai@chromium.org> Date: Sat Oct 27 01:28:49 2018 Refactor TopmostWindowObserver to be EventObserver It has been an window observer + pre-target event handler to conduct the watching of topmost windows. During debugging of a test of tab-dragging, I've noticed that this can't deal with the destruction of the root window itself. Actually, it doesn't have to be a pre-target event handler anymore since there's EventObserver now. BUG= 890071 TEST=existing ash_unittests / services_unittests pass Change-Id: Ic6750700e73fd3694be5aa00f080f3019e4fbd95 Reviewed-on: https://chromium-review.googlesource.com/c/1303034 Commit-Queue: Jun Mukai <mukai@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#603286} [modify] https://crrev.com/b027b38a0726d515cc725be85247fa353166ee46/services/ws/topmost_window_observer.cc [modify] https://crrev.com/b027b38a0726d515cc725be85247fa353166ee46/services/ws/topmost_window_observer.h
,
Oct 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9692b75a25dc4f027aabac4e620b78a66c219854 commit 9692b75a25dc4f027aabac4e620b78a66c219854 Author: Jun Mukai <mukai@chromium.org> Date: Mon Oct 29 17:49:09 2018 Re-enable a test of TabDragController With recent fixes of myself, this test now succeeds. BUG= 890071 TEST=none R=sky@chromium.org Change-Id: I73e8918771225d9855cbf6299d3c852476adf707 Reviewed-on: https://chromium-review.googlesource.com/c/1305194 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Jun Mukai <mukai@chromium.org> Cr-Commit-Position: refs/heads/master@{#603549} [modify] https://crrev.com/9692b75a25dc4f027aabac4e620b78a66c219854/testing/buildbot/filters/chromeos.single_process_mash.interactive_ui_tests.filter
,
Nov 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5788540b7585c1c678775d6409469fe13bda1252 commit 5788540b7585c1c678775d6409469fe13bda1252 Author: Jun Mukai <mukai@chromium.org> Date: Fri Nov 02 17:27:41 2018 Add SendTouchEvents to ui_controls for ChromeOS This is used to test asynchronous behaviors of touch in interactive ui tests for ChromeOS, which also fits with the design of Mash. See https://docs.google.com/document/d/1h6-dBGUUF2Us4C8ELzqlNyZx1VzcGlenLPDeOBJDJJw/edit?usp=sharing for the details of the failure reason. BUG= 890071 TEST=tab_drag_controller_intearctive_uitest Change-Id: I07a292e8f8cd69afa4212f90a52400d612cf6f63 Reviewed-on: https://chromium-review.googlesource.com/c/1309093 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Jun Mukai <mukai@chromium.org> Cr-Commit-Position: refs/heads/master@{#604972} [modify] https://crrev.com/5788540b7585c1c678775d6409469fe13bda1252/ash/test/ui_controls_factory_ash.cc [modify] https://crrev.com/5788540b7585c1c678775d6409469fe13bda1252/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc [modify] https://crrev.com/5788540b7585c1c678775d6409469fe13bda1252/testing/buildbot/filters/chromeos.single_process_mash.interactive_ui_tests.filter [modify] https://crrev.com/5788540b7585c1c678775d6409469fe13bda1252/ui/aura/test/ui_controls_factory_ozone.cc [modify] https://crrev.com/5788540b7585c1c678775d6409469fe13bda1252/ui/base/test/ui_controls.h [modify] https://crrev.com/5788540b7585c1c678775d6409469fe13bda1252/ui/base/test/ui_controls_aura.cc [modify] https://crrev.com/5788540b7585c1c678775d6409469fe13bda1252/ui/base/test/ui_controls_aura.h
,
Nov 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/790f54b0e7ec638ad5d1cd44bf6da291034dcd10 commit 790f54b0e7ec638ad5d1cd44bf6da291034dcd10 Author: Jun Mukai <mukai@chromium.org> Date: Fri Nov 02 23:05:35 2018 reenable TabDragging DragToOverviewWindow/1 Fixed by crrev.com/604972 and crrev.com/604715 BUG= 900363 , 890071 TEST=trybot Change-Id: I726db69c603595a2f4ec4f49a6976b9f57c737c8 Reviewed-on: https://chromium-review.googlesource.com/c/1314598 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Jun Mukai <mukai@chromium.org> Cr-Commit-Position: refs/heads/master@{#605086} [modify] https://crrev.com/790f54b0e7ec638ad5d1cd44bf6da291034dcd10/testing/buildbot/filters/chromeos.single_process_mash.interactive_ui_tests.filter
,
Nov 2
,
Nov 2
,
Nov 2
,
Nov 2
,
Nov 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/51258edc2b093efd843bd1a23f8341112293740b commit 51258edc2b093efd843bd1a23f8341112293740b Author: Jun Mukai <mukai@chromium.org> Date: Tue Nov 06 00:24:17 2018 Allow notifying injected event which turns out invalid Some tab-dragging test cases never finishes on SingleProcessMash, and it turns out that WindowEventDispatcher thinks some TOUCH_RELEASED events can be invalid; it won't generate any gestures and does not do anything. In that case, OnEvent() won't be called and any other methods are not called, therefore InjectedEventHandler couldn't notify it's done. This CL adds a new method to WindowEventDispatcherObserver to catch such cases and allow handling it's done. BUG= 890071 TEST=see the filter update Change-Id: I6dcda9af52e5cbd4013b8d228bda2e9b1f14bdd6 Reviewed-on: https://chromium-review.googlesource.com/c/1316767 Commit-Queue: Jun Mukai <mukai@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#605531} [modify] https://crrev.com/51258edc2b093efd843bd1a23f8341112293740b/services/ws/injected_event_handler.cc [modify] https://crrev.com/51258edc2b093efd843bd1a23f8341112293740b/services/ws/injected_event_handler.h [modify] https://crrev.com/51258edc2b093efd843bd1a23f8341112293740b/testing/buildbot/filters/chromeos.single_process_mash.interactive_ui_tests.filter [modify] https://crrev.com/51258edc2b093efd843bd1a23f8341112293740b/ui/aura/window_event_dispatcher.cc [modify] https://crrev.com/51258edc2b093efd843bd1a23f8341112293740b/ui/aura/window_event_dispatcher_observer.h
,
Dec 10
,
Jan 2
a lot of these still fail (flakily) for multi process mash, and the link in the filter file points to this bug, so I'm reopening.
,
Jan 2
also, I'm disabling them all for multi process mash because it seems that they can pretty much all flake. See for example the following builds where different tab dragging tests failed each time. https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mojo%20ChromiumOS/38659 https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mojo%20ChromiumOS/38658 https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mojo%20ChromiumOS/38596
,
Jan 2
I believe the flakes on multi-process-mash is for asynchronousness of event-injector; will look into this later. Thanks for raising this.
,
Jan 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8212c5bf187dab6408d2bee052ad11020ae084cb commit 8212c5bf187dab6408d2bee052ad11020ae084cb Author: Evan Stade <estade@chromium.org> Date: Wed Jan 02 22:52:45 2019 Disable tab dragging tests with multi process mash. Broaden the filters to disable all tab dragging tests, since a lot (all?) are flakily failing on mash_fyi_interactive_ui_tests TBR=mukai@chromium.org Bug: 890071 Change-Id: Ifd4415d4326c49f374e0f023875a003365905cbe Reviewed-on: https://chromium-review.googlesource.com/c/1393534 Reviewed-by: Evan Stade <estade@chromium.org> Commit-Queue: Evan Stade <estade@chromium.org> Cr-Commit-Position: refs/heads/master@{#619522} [modify] https://crrev.com/8212c5bf187dab6408d2bee052ad11020ae084cb/testing/buildbot/filters/chromeos.mash.fyi.interactive_ui_tests.filter
,
Jan 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aa8479c836085da2744836e9b010a1642c745d39 commit aa8479c836085da2744836e9b010a1642c745d39 Author: Jun Mukai <mukai@chromium.org> Date: Tue Jan 08 02:30:39 2019 Fix TabDragging ui_test's waiting for drag ending timing This is a fix for multi-process Mash; - remove touch code path of waiting; this is not correct anymore and makes a difference on multi-process Mash since RunUntilIdle can't wait properly. This resulted with refactoring/removal of QuitCurrentWhenIdleDeprecated - add AddRemoveDisplay to ShellTestApi instead of using display manager directly. - remove the usage of cursor_manager_test_api - do not refer to ash::wm::WindowState, this does not work on Mash - as the result, this can remove the includes of non-public ash files except for window_state (this is needed for classic environment). Bug: 890071 Test: interactive_ui_tests with Mash Change-Id: I7f4b99ee9bf07951d3367893ef59148c51f34501 Reviewed-on: https://chromium-review.googlesource.com/c/1395149 Commit-Queue: Jun Mukai <mukai@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#620587} [modify] https://crrev.com/aa8479c836085da2744836e9b010a1642c745d39/ash/public/interfaces/shell_test_api.test-mojom [modify] https://crrev.com/aa8479c836085da2744836e9b010a1642c745d39/ash/shell_test_api.cc [modify] https://crrev.com/aa8479c836085da2744836e9b010a1642c745d39/ash/shell_test_api.h [modify] https://crrev.com/aa8479c836085da2744836e9b010a1642c745d39/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc [modify] https://crrev.com/aa8479c836085da2744836e9b010a1642c745d39/testing/buildbot/filters/chromeos.mash.fyi.interactive_ui_tests.filter [modify] https://crrev.com/aa8479c836085da2744836e9b010a1642c745d39/ui/aura/test/ui_controls_factory_ozone.cc
,
Jan 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2af6db5919536b52e67eff0bc0b608b63dcedd3f commit 2af6db5919536b52e67eff0bc0b608b63dcedd3f Author: Jun Mukai <mukai@chromium.org> Date: Wed Jan 09 00:22:42 2019 Set minimum fling velocity in Ash for tab-dragging test TabDragging UI tests are still flaky on multi-process Mash. The reason turned out the fling gesture which minimizes the window unexpectedly. The test code has increased the minimum fling velocity to suppress this, but this code isn't working well with multi-process Mash (since they're separated). This CL adds a new test-api to support this case. Bug: 890071 Test: interactive_ui_tests Change-Id: I61e606d1a5df079bb47e208e8ad78415c7d1aadd Reviewed-on: https://chromium-review.googlesource.com/c/1401382 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Jun Mukai <mukai@chromium.org> Cr-Commit-Position: refs/heads/master@{#620958} [modify] https://crrev.com/2af6db5919536b52e67eff0bc0b608b63dcedd3f/ash/public/interfaces/shell_test_api.test-mojom [modify] https://crrev.com/2af6db5919536b52e67eff0bc0b608b63dcedd3f/ash/shell_test_api.cc [modify] https://crrev.com/2af6db5919536b52e67eff0bc0b608b63dcedd3f/ash/shell_test_api.h [modify] https://crrev.com/2af6db5919536b52e67eff0bc0b608b63dcedd3f/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
,
Jan 9
mash_fyi_interactive_ui_tests in the FYI bot seems stabilized, I think now this is good with multi-process Mash. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by mukai@chromium.org
, Oct 26