Some of the tab dragging tests fail on chromeos because of IsWindowPositionManaged |
|||||||||||
Issue descriptionI'm in the process of reenabling some of the tab dragging tests on chromeos. As part of reenabling them some are failing because of the wrong value from IsWindowPositionManaged(). I'm going to comment out the assertions for now until they can be reevaluated.
,
Nov 10 2016
,
Nov 10 2016
Failing tests due to this: 1 - TabDragging/DetachToBrowserTabDragControllerTest.DetachToOwnWindow/0, where GetParam() = "mouse" 2 - TabDragging/DetachToBrowserTabDragControllerTest.DetachFromFullsizeWindow/0, where GetParam() = "mouse" 3 - TabDragging/DetachToBrowserTabDragControllerTest.DetachToOwnWindowFromMaximizedWindow/0, where GetParam() = "mouse" 4 - TabDragging/DetachToBrowserTabDragControllerTest.DetachToDockedWindowFromMaximizedWindow/0, where GetParam() = "mouse" 5 - TabDragging/DetachToBrowserTabDragControllerTest.DetachToDockedWindowFromMaximizedWindow/1, where GetParam() = "touch"
,
Nov 12 2016
I managed to fix all the above tests except DetachFromFullsizeWindow. varkha@, it seems you wrote this test. This test fails on the bot which is using the openbox WM. Setting the browser window's bounds to the work area's bounds is NOT equivalent to the window being full screen in that Window Manager. Is that a bug in the test or the code itself?
,
Nov 16 2016
I will keep the test TabDragging/DetachToBrowserTabDragControllerTest.DetachFromFullsizeWindow/0 disabled on Linux for now. It passes locally but fails on the bot. It fails locally only if I run the test with openbox. The reason is the following: Setting the source browser window's bound to the work area bounds will go to DesktopWindowTreeHostX11::SetBounds() which will adjust the bounds by subtracting 1px from width and height. To the TabDragController, the source browser window is no longer full size, and it will not maximize the detached window on completing the drag. Hence the test fails because it expects that window to be maximized.
,
Nov 16 2016
#5, could also change the expectation of the newly created window to be maximized after the drag to be different on linux since the behavior is different (and you have a system-dependent disable anyway.
,
Dec 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c813d7634e5c6ba6c808e326778b0cdb4f6e773c commit c813d7634e5c6ba6c808e326778b0cdb4f6e773c Author: afakhry <afakhry@chromium.org> Date: Thu Dec 01 17:33:40 2016 Reenable Tabdragging tests failing because of IsWindowPositionManaged() It used to be exiting the move loop and clearing the move_loop_widget_ before EndDrag() had a chance to set the window position to manageable again. This CL adds a maximized browser window waiter, and moves setting the window position managed logic to ash. BUG= 626761 , 331924 TEST=interactive_ui_tests --gtest_filter=TabDragging* Review-Url: https://codereview.chromium.org/2494713003 Cr-Commit-Position: refs/heads/master@{#435638} [modify] https://crrev.com/c813d7634e5c6ba6c808e326778b0cdb4f6e773c/ash/wm/toplevel_window_event_handler.cc [modify] https://crrev.com/c813d7634e5c6ba6c808e326778b0cdb4f6e773c/ash/wm/toplevel_window_event_handler_unittest.cc [modify] https://crrev.com/c813d7634e5c6ba6c808e326778b0cdb4f6e773c/chrome/browser/ui/views/tabs/tab_drag_controller.cc [modify] https://crrev.com/c813d7634e5c6ba6c808e326778b0cdb4f6e773c/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
,
Dec 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3a9a59b140a23a82da474ede5c5abb311bb088c1 commit 3a9a59b140a23a82da474ede5c5abb311bb088c1 Author: rockot <rockot@chromium.org> Date: Thu Dec 01 22:39:55 2016 Revert of Reenable Tabdragging tests failing because of IsWindowPositionManaged() (patchset #10 id:200001 of https://codereview.chromium.org/2494713003/ ) Reason for revert: Causing failures in ash_unittests. See CL comments for details. If you have a fix, please reland with the fix applied. Original issue's description: > Reenable Tabdragging tests failing because of IsWindowPositionManaged() > > It used to be exiting the move loop and clearing the move_loop_widget_ before > EndDrag() had a chance to set the window position to manageable again. > > This CL adds a maximized browser window waiter, and moves setting the window > position managed logic to ash. > > BUG= 626761 , 331924 > TEST=interactive_ui_tests --gtest_filter=TabDragging* > > Committed: https://crrev.com/c813d7634e5c6ba6c808e326778b0cdb4f6e773c > Cr-Commit-Position: refs/heads/master@{#435638} TBR=pkasting@chromium.org,sky@chromium.org,skuhne@chromium.org,erg@chromium.org,afakhry@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 626761 , 331924 Review-Url: https://codereview.chromium.org/2550533002 Cr-Commit-Position: refs/heads/master@{#435742} [modify] https://crrev.com/3a9a59b140a23a82da474ede5c5abb311bb088c1/ash/wm/toplevel_window_event_handler.cc [modify] https://crrev.com/3a9a59b140a23a82da474ede5c5abb311bb088c1/ash/wm/toplevel_window_event_handler_unittest.cc [modify] https://crrev.com/3a9a59b140a23a82da474ede5c5abb311bb088c1/chrome/browser/ui/views/tabs/tab_drag_controller.cc [modify] https://crrev.com/3a9a59b140a23a82da474ede5c5abb311bb088c1/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
,
Dec 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/83e8abcce5801546b4bb2ff03fdaf5cb4590f0e5 commit 83e8abcce5801546b4bb2ff03fdaf5cb4590f0e5 Author: afakhry <afakhry@chromium.org> Date: Thu Dec 01 23:54:52 2016 Reland of Reenable Tabdragging tests failing because of IsWindowPositionManaged() It was reverted previously as it caused a crash in WorkspaceEventHandlerTest.DeleteWhileInRunLoop. It used to be exiting the move loop and clearing the move_loop_widget_ before EndDrag() had a chance to set the window position to manageable again. This CL adds a maximized browser window waiter, and moves setting the window position managed logic to ash. TBR=pkasting@chromium.org,sky@chromium.org,skuhne@chromium.org,oshima@chromium.org,erg@chromium.org BUG= 626761 , 331924 TEST=interactive_ui_tests --gtest_filter=TabDragging* Review-Url: https://codereview.chromium.org/2540013005 Cr-Commit-Position: refs/heads/master@{#435775} [modify] https://crrev.com/83e8abcce5801546b4bb2ff03fdaf5cb4590f0e5/ash/wm/toplevel_window_event_handler.cc [modify] https://crrev.com/83e8abcce5801546b4bb2ff03fdaf5cb4590f0e5/ash/wm/toplevel_window_event_handler_unittest.cc [modify] https://crrev.com/83e8abcce5801546b4bb2ff03fdaf5cb4590f0e5/chrome/browser/ui/views/tabs/tab_drag_controller.cc [modify] https://crrev.com/83e8abcce5801546b4bb2ff03fdaf5cb4590f0e5/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
,
Dec 12 2016
,
Mar 4 2017
,
Apr 17 2017
,
May 30 2017
,
Aug 1 2017
,
Oct 14 2017
,
Jan 24 2018
,
Jan 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9de4762785e0e8a03ae83f1c834b002cfbc0a868 commit 9de4762785e0e8a03ae83f1c834b002cfbc0a868 Author: Peter Kasting <pkasting@chromium.org> Date: Thu Jan 03 20:22:59 2019 Enable disabled tests in c/b/ui/views/tabs/. Bug: 331924 , 603562, 626761 , 837219 Change-Id: If2695993d98dc00168c367c3c171b99a96eb7bc5 Reviewed-on: https://chromium-review.googlesource.com/c/1351809 Commit-Queue: Peter Kasting <pkasting@chromium.org> Reviewed-by: Allen Bauer <kylixrd@chromium.org> Cr-Commit-Position: refs/heads/master@{#619736} [modify] https://crrev.com/9de4762785e0e8a03ae83f1c834b002cfbc0a868/chrome/browser/ui/views/frame/browser_frame.cc [modify] https://crrev.com/9de4762785e0e8a03ae83f1c834b002cfbc0a868/chrome/browser/ui/views/frame/browser_frame.h [modify] https://crrev.com/9de4762785e0e8a03ae83f1c834b002cfbc0a868/chrome/browser/ui/views/frame/browser_non_client_frame_view.h [modify] https://crrev.com/9de4762785e0e8a03ae83f1c834b002cfbc0a868/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc [modify] https://crrev.com/9de4762785e0e8a03ae83f1c834b002cfbc0a868/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h [modify] https://crrev.com/9de4762785e0e8a03ae83f1c834b002cfbc0a868/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.h [modify] https://crrev.com/9de4762785e0e8a03ae83f1c834b002cfbc0a868/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.mm [modify] https://crrev.com/9de4762785e0e8a03ae83f1c834b002cfbc0a868/chrome/browser/ui/views/frame/browser_view.h [modify] https://crrev.com/9de4762785e0e8a03ae83f1c834b002cfbc0a868/chrome/browser/ui/views/frame/glass_browser_frame_view.cc [modify] https://crrev.com/9de4762785e0e8a03ae83f1c834b002cfbc0a868/chrome/browser/ui/views/frame/glass_browser_frame_view.h [modify] https://crrev.com/9de4762785e0e8a03ae83f1c834b002cfbc0a868/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc [modify] https://crrev.com/9de4762785e0e8a03ae83f1c834b002cfbc0a868/chrome/browser/ui/views/frame/opaque_browser_frame_view.h [modify] https://crrev.com/9de4762785e0e8a03ae83f1c834b002cfbc0a868/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc [modify] https://crrev.com/9de4762785e0e8a03ae83f1c834b002cfbc0a868/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.h |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by tbuck...@chromium.org
, Oct 11 2016Status: Assigned (was: Untriaged)