MaximizeModeWindowManagerTest.KeepPinnedModeOn_Case1, MaximizeModeWindowManagerTest.KeepPinnedModeOn_Case4 fail with jessie chroot |
||||||||||
Issue descriptionI'm trying to make the chromeos/desktop build use the jessie sysroot here: https://codereview.chromium.org/2772403002/ This mostly works, except for two tests that fail: https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/392236 https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/349011 https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.linux%2Flinux_chromium_chromeos_rel_ng%2F392236%2F%2B%2Frecipes%2Fsteps%2Fash_unittests__with_patch_%2F0%2Flogs%2FMaximizeModeWindowManagerTest.KeepPinnedModeOn_Case1%2F0 MaximizeModeWindowManagerTest.KeepPinnedModeOn_Case1 (run #1): [ RUN ] MaximizeModeWindowManagerTest.KeepPinnedModeOn_Case1 Xlib: extension "RANDR" missing on display ":99". [11125:11125:0327/150512.883997:21482069746:FATAL:window.cc(818)] Check failed: this == child->parent() (0xa6b627a79a0 vs. 0) #0 0x000000b20177 base::debug::StackTrace::StackTrace() #1 0x000000b2c48a logging::LogMessage::~LogMessage() #2 0x00000117a7c8 aura::Window::StackChildRelativeTo() #3 0x000000acfde8 ash::ScreenPinningController::KeepPinnedWindowOnTop() #4 0x00000117c5a4 aura::Window::OnStackingChanged() #5 0x00000117ac85 aura::Window::StackChildRelativeTo() #6 0x000000a7085c ash::WorkspaceBackdropDelegate::RestackBackdrop() #7 0x00000117b523 aura::Window::RemoveChildImpl() #8 0x000001178f94 aura::Window::~Window() #9 0x0000011795d9 aura::Window::~Window() #10 0x000000a84b1f ash::WindowDimmer::~WindowDimmer() #11 0x000000a816c2 std::_Rb_tree<>::_M_erase() #12 0x000000acf659 ash::ScreenPinningController::SetPinnedWindow() #13 0x000000a702db ash::MaximizeModeWindowState::UpdateWindow() #14 0x000000a87286 ash::wm::WindowState::Restore() #15 0x0000007f79fb ash::MaximizeModeWindowManagerTest_KeepPinnedModeOn_Case1_Test::TestBody() #16 0x00000114c326 testing::Test::Run() #17 0x00000114cd10 testing::TestInfo::Run() #18 0x00000114d237 testing::TestCase::Run() #19 0x000001154397 testing::internal::UnitTestImpl::RunAllTests() #20 0x000001154017 testing::UnitTest::Run() #21 0x000000b95d41 base::TestSuite::Run() #22 0x000000b970be base::(anonymous namespace)::LaunchUnitTestsInternal() #23 0x000000b96f44 base::LaunchUnitTests() #24 0x0000005bb3c9 main #25 0x7f64da117f45 __libc_start_main #26 0x000000490412 <unknown> https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.linux%2Flinux_chromium_chromeos_rel_ng%2F392236%2F%2B%2Frecipes%2Fsteps%2Fash_unittests__with_patch_%2F0%2Flogs%2FMaximizeModeWindowManagerTest.KeepPinnedModeOn_Case4%2F0 MaximizeModeWindowManagerTest.KeepPinnedModeOn_Case4 (run #1): [ RUN ] MaximizeModeWindowManagerTest.KeepPinnedModeOn_Case4 Xlib: extension "RANDR" missing on display ":99". [11975:11975:0327/150516.698358:21485884105:FATAL:window.cc(818)] Check failed: this == child->parent() (0x143d2272b420 vs. 0) #0 0x000000b20177 base::debug::StackTrace::StackTrace() #1 0x000000b2c48a logging::LogMessage::~LogMessage() #2 0x00000117a7c8 aura::Window::StackChildRelativeTo() #3 0x000000acfde8 ash::ScreenPinningController::KeepPinnedWindowOnTop() #4 0x00000117c5a4 aura::Window::OnStackingChanged() #5 0x00000117ac85 aura::Window::StackChildRelativeTo() #6 0x000000a7085c ash::WorkspaceBackdropDelegate::RestackBackdrop() #7 0x00000117b523 aura::Window::RemoveChildImpl() #8 0x000001178f94 aura::Window::~Window() #9 0x0000011795d9 aura::Window::~Window() #10 0x000000a84b1f ash::WindowDimmer::~WindowDimmer() #11 0x000000a816c2 std::_Rb_tree<>::_M_erase() #12 0x000000acf659 ash::ScreenPinningController::SetPinnedWindow() #13 0x000000a702db ash::MaximizeModeWindowState::UpdateWindow() #14 0x000000a87286 ash::wm::WindowState::Restore() #15 0x0000007f9c49 ash::MaximizeModeWindowManagerTest_KeepPinnedModeOn_Case4_Test::TestBody() #16 0x00000114c326 testing::Test::Run() #17 0x00000114cd10 testing::TestInfo::Run() #18 0x00000114d237 testing::TestCase::Run() #19 0x000001154397 testing::internal::UnitTestImpl::RunAllTests() #20 0x000001154017 testing::UnitTest::Run() #21 0x000000b95d41 base::TestSuite::Run() #22 0x000000b970be base::(anonymous namespace)::LaunchUnitTestsInternal() #23 0x000000b96f44 base::LaunchUnitTests() #24 0x0000005bb3c9 main #25 0x7f4680becf45 __libc_start_main #26 0x000000490412 <unknown> hidehiko, does this make sense to you?
,
Mar 28 2017
Yes, we already had one bug relating to std::map ordering changing (can't find the change right now).
,
Mar 28 2017
,
Mar 28 2017
,
Mar 28 2017
Better stack from a local run with debug info: [ RUN ] MaximizeModeWindowManagerTest.KeepPinnedModeOn_Case1 [129112:129112:0328/115306.145831:4312016525339:FATAL:window.cc(818)] Check failed: this == child->parent() (0x20cbc8521020 vs. 0) #0 0x7fca513029fb base::debug::StackTrace::StackTrace() #1 0x7fca5130173c base::debug::StackTrace::StackTrace() #2 0x7fca513638ac logging::LogMessage::~LogMessage() #3 0x7fca4e09e8d5 aura::Window::StackChildRelativeTo() #4 0x7fca4e09eeca aura::Window::StackChildBelow() #5 0x7fca51efcbe9 ash::WmWindow::StackChildBelow() #6 0x7fca51ff49f9 ash::ScreenPinningController::KeepPinnedWindowOnTop() #7 0x7fca51ff4a99 ash::ScreenPinningController::OnPinnedContainerWindowStackingChanged() #8 0x7fca51ff5bf4 ash::ScreenPinningController::PinnedContainerChildWindowObserver::OnWindowStackingChanged() #9 0x7fca4e0a19c7 aura::Window::OnStackingChanged() #10 0x7fca4e09edfa aura::Window::StackChildRelativeTo() #11 0x7fca4e09e647 aura::Window::StackChildAbove() #12 0x7fca51efcb99 ash::WmWindow::StackChildAbove() #13 0x7fca51e84c3c ash::WorkspaceBackdropDelegate::RestackBackdrop() #14 0x7fca51e84db9 ash::WorkspaceBackdropDelegate::OnWindowRemovedFromLayout() #15 0x7fca51ee8d73 ash::WorkspaceLayoutManager::OnWindowRemovedFromLayout() #16 0x7fca51c2db35 ash::AuraLayoutManagerAdapter::OnWindowRemovedFromLayout() #17 0x7fca4e09f8d6 aura::Window::RemoveChildImpl() #18 0x7fca4e09cc97 aura::Window::RemoveChild() #19 0x7fca4e09c670 aura::Window::~Window() #20 0x7fca4e09cd89 aura::Window::~Window() #21 0x7fca51eccabf ash::WindowDimmer::~WindowDimmer() #22 0x7fca51eccaf9 ash::WindowDimmer::~WindowDimmer() #23 0x7fca51c29432 std::default_delete<>::operator()() #24 0x7fca51ebf3b3 std::unique_ptr<>::~unique_ptr() #25 0x7fca51ebfcb9 std::pair<>::~pair() #26 0x7fca51ebfc99 std::_Rb_tree_node<>::~_Rb_tree_node() #27 0x7fca51ebfc79 __gnu_cxx::new_allocator<>::destroy<>() #28 0x7fca51ebfc4c std::_Rb_tree<>::_M_destroy_node() #29 0x7fca51ec0888 std::_Rb_tree<>::_M_erase() #30 0x7fca51ec07c5 std::_Rb_tree<>::clear() #31 0x7fca51ec0645 std::__cxx1998::map<>::clear() #32 0x7fca51ec0132 std::__debug::map<>::clear() #33 0x7fca51ebfd6d ash::WindowUserData<>::clear() #34 0x7fca51ff3e82 ash::ScreenPinningController::SetPinnedWindow() #35 0x7fca51c34709 ash::WmShellAura::SetPinnedWindow() #36 0x7fca51e83f9a ash::MaximizeModeWindowState::UpdateWindow() #37 0x7fca51e83b01 ash::MaximizeModeWindowState::OnWMEvent() #38 0x7fca51ed2266 ash::wm::WindowState::OnWMEvent() #39 0x7fca51ed2209 ash::wm::WindowState::Restore() #40 0x0000007e9e11 ash::MaximizeModeWindowManagerTest_KeepPinnedModeOn_Case1_Test::TestBody() #41 0x000000c4547e testing::internal::HandleSehExceptionsInMethodIfSupported<>() #42 0x000000c366c2 testing::internal::HandleExceptionsInMethodIfSupported<>() #43 0x000000c2b556 testing::Test::Run() #44 0x000000c2bc8d testing::TestInfo::Run() #45 0x000000c2c22f testing::TestCase::Run() #46 0x000000c3159c testing::internal::UnitTestImpl::RunAllTests() #47 0x000000c494ce testing::internal::HandleSehExceptionsInMethodIfSupported<>() #48 0x000000c37df2 testing::internal::HandleExceptionsInMethodIfSupported<>() #49 0x000000c3123f testing::UnitTest::Run() #50 0x000000bcea71 RUN_ALL_TESTS() #51 0x000000bcd9a2 base::TestSuite::Run() #52 0x000000557c65 _ZN4base8internal13FunctorTraitsIMN3ash36ResolutionNotificationControllerTestEFvvEvE6InvokeIPS3_JEEEvS5_OT_DpOT0_ #53 0x000000557b81 _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKMN3ash36ResolutionNotificationControllerTestEFvvEJPS5_EEEvOT_DpOT0_ #54 0x0000005c38f7 _ZN4base8internal7InvokerINS0_9BindStateIMNS_9TestSuiteEFivEJNS0_17UnretainedWrapperIN3ash4test12AshTestSuiteEEEEEEFivEE7RunImplIRKS5_RKSt5tupleIJSA_EEJLm0EEEEiOT_OT0_NS_13IndexSequenceIJXspT1_EEEE #55 0x0000005c383c _ZN4base8internal7InvokerINS0_9BindStateIMNS_9TestSuiteEFivEJNS0_17UnretainedWrapperIN3ash4test12AshTestSuiteEEEEEEFivEE3RunEPNS0_13BindStateBaseE #56 0x0000004f87ad _ZNKR4base8CallbackIFvvELNS_8internal8CopyModeE1ELNS2_10RepeatModeE1EE3RunEv #57 0x000000bd066e base::(anonymous namespace)::LaunchUnitTestsInternal() #58 0x000000bd0504 base::LaunchUnitTests() #59 0x0000005c36c8 main #60 0x7fca498fbf45 __libc_start_main #61 0x0000004e4e84 <unknown> So the `window_dimmers_->clear();` here seems to trigger this: https://chromium.googlesource.com/chromium/src/+blame/95cf7a59de278ede5e772ba752215c80a80ed122/ash/wm/screen_pinning_controller.cc#174
,
Mar 28 2017
The original code deletes dim windows after the related observer is removed, so the problematic call stack didn't happen regardless of std::map ordering change. Then, a CL seemed to change the order for dim window redesigning. https://codereview.chromium.org/2320273002/diff/160001/ash/wm/screen_pinning_controller.cc I think moving "window_dimmers_->clear();" to the original location (after https://cs.chromium.org/chromium/src/ash/wm/screen_pinning_controller.cc?&l=227) will fix the issue, but I'm not WmWindow expert, so I'd like to hear the advice from the expert. sky@, can we safely move the clear() invocation there? Could you help me understand the intention of moving clear() in your CL?
,
Mar 28 2017
I can confirm that the change described by hidehiko in comment 6 fixes the test with the new sysroot:
diff --git a/ash/wm/screen_pinning_controller.cc b/ash/wm/screen_pinning_controller.cc
index 4a708dbf21d3..d3f9aa1cf469 100644
--- a/ash/wm/screen_pinning_controller.cc
+++ b/ash/wm/screen_pinning_controller.cc
@@ -171,8 +171,6 @@ bool ScreenPinningController::IsPinned() const {
}
void ScreenPinningController::SetPinnedWindow(WmWindow* pinned_window) {
- window_dimmers_->clear();
-
if (pinned_window->GetWindowState()->IsPinned()) {
if (pinned_window_) {
LOG(DFATAL) << "Pinned mode is enabled, while it is already in "
@@ -228,6 +226,7 @@ void ScreenPinningController::SetPinnedWindow(WmWindow* pinned_window) {
pinned_container_child_window_observer_.get());
WmWindow::GetAuraWindow(container)->RemoveObserver(
pinned_container_window_observer_.get());
+ window_dimmers_->clear();
pinned_window_ = nullptr;
}
So it'd be cool if we could get that landed, unless it was intentional to put clear() to where it is for some reason.
(hidehiko, let me know if you want me to make a CL, else I'll assume you'll handle this.)
,
Mar 28 2017
And thanks for investigating!
,
Mar 28 2017
Thanks for the investigation hidehiko. I thought that code was simpler with the clear at the top, but you're right, it leads to badness which you found. I'll fix it.
,
Mar 28 2017
Thank you for confirmation, sky@. FYI: I've already made a CL, am running on try and waiting for its completion. https://codereview.chromium.org/2785513002/
,
Mar 28 2017
I landed a fix here https://codereview.chromium.org/2784603002 . It also makes WindowUserData::clear() slightly saner.
,
May 30 2017
,
Aug 1 2017
,
Jan 22 2018
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by thakis@chromium.org
, Mar 27 2017