New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 721452 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Closing lid with external display connected crashes chrome

Project Member Reported by kylec...@chromium.org, May 11 2017

Issue description

Chrome: built from ToT today
CrOS: 9538
board: link

Steps to reproduce:
1. Log in.
2. Connect external display.
3. Open a Chrome window.
4. Close lid.

Chrome crashes and restarts here. I get the following stack trace.

Received signal 11 SEGV_MAPERR 000000000018
#0 0x7f58b986ed4c base::debug::StackTrace::StackTrace()
#1 0x7f58b986e8b1 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#2 0x7f58b6a27580 <unknown>
#3 0x7f58bb62fcb4 ash::WallpaperController::GetInternalDisplayCompositorLock()
#4 0x7f58bb62fba7 ash::WallpaperController::OnDisplayConfigurationChanged()
#5 0x7f58bb65ce11 ash::ShellPortClassic::OnDisplayConfigurationChanged()
#6 0x7f58bb5caa01 ash::WindowTreeHostManager::PostDisplayConfigurationChange()
#7 0x7f58bb548d3b display::DisplayManager::UpdateDisplaysWith()
#8 0x7f58bb546486 display::DisplayManager::OnNativeDisplaysChanged()
#9 0x7f58bb539ec8 display::DisplayChangeObserver::OnDisplayModeChanged()
#10 0x7f58bb53f024 display::DisplayConfigurator::NotifyDisplayStateObservers()
#11 0x7f58bb53d562 display::DisplayConfigurator::OnConfigured()
#12 0x7f58bb543f53 display::UpdateDisplayConfigurationTask::OnStateEntered()
#13 0x7f58bb55a966 display::ConfigureDisplaysTask::Run()
#14 0x7f58bb55ac81 display::ConfigureDisplaysTask::OnConfigured()
#15 0x7f58b7e3f6e6 _ZN4base8internal7InvokerINS0_9BindStateINS_8CallbackIFvbELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEEJbEEEFvvEE3RunEPNS0_13BindStateBaseE
#16 0x7f58b98fff96 base::debug::TaskAnnotator::RunTask()
#17 0x7f58b988962e base::MessageLoop::RunTask()
#18 0x7f58b98899df base::MessageLoop::DeferOrRunPendingTask()
#19 0x7f58b9889d2d base::MessageLoop::DoWork()
#20 0x7f58b988b479 base::MessagePumpLibevent::Run()
#21 0x7f58b98ab050 base::RunLoop::Run()
#22 0x7f58b95007c2 ChromeBrowserMainParts::MainMessageLoopRun()
#23 0x7f58b815ab04 content::BrowserMainLoop::RunMainMessageLoopParts()
#24 0x7f58b815d412 content::BrowserMainRunnerImpl::Run()
#25 0x7f58b815631c content::BrowserMain()
#26 0x7f58b94d7fb4 content::ContentMainRunnerImpl::Run()
#27 0x7f58b94f6d49 service_manager::Main()
#28 0x7f58b94d6e91 content::ContentMain()
#29 0x7f58b7bfe31e ChromeMain
#30 0x7f58b572f816 __libc_start_main
#31 0x7f58b7bfe139 _start
  r8: 00000000000004b0  r9: 0000000000000780 r10: 0000000000000000 r11: 00000000000004b0
 r12: 00007ffec5cb6100 r13: 0000229a7570a530 r14: 0000229a74463700 r15: 00007ffec5cb6060
  di: 0000229a74524508  si: 0030e40000000000  bp: 00007ffec5cb5fe0  bx: 0000229a745244e0
  dx: 0000229a77f16ed0  ax: 0000000000000000  cx: 6d65d807805f6100  sp: 00007ffec5cb5fc0
  ip: 00007f58bb62fcb4 efl: 0000000000010206 cgf: 3f80000000000033 erf: 0000000000000004
 trp: 000000000000000e msk: 0000000000000000 cr2: 0000000000000018
[end of stack trace]
Calling _exit(1). Core file will not be generated.

It looks like this code is crashing trying to do something with the internal display that is no longer there.

https://cs.chromium.org/chromium/src/ash/wallpaper/wallpaper_controller.cc?type=cs&q=ash::WallpaperController::GetInternalDisplayCompositorLock&sq=package:chromium&l=417
 

Comment 1 by wutao@chromium.org, May 11 2017

Thanks.
That is this change: https://codereview.chromium.org/2872123002/

I will make change to check non-null for GetRootWindowForDisplayId(display::Display::InternalDisplayId())
When we find the id for the internal display it gets set and Display::HasInternalDisplay() returns true. The internal display id isn't reset, even when an internal display is removed.

https://cs.chromium.org/chromium/src/ui/display/manager/chromeos/display_change_observer.cc?l=57

So even though we might have an internal display id, there is no guarantee there is a RootWindow for it.

Comment 4 by wutao@chromium.org, May 11 2017

kylechar@, please take a look of the fix, thanks.
https://codereview.chromium.org/2875123002/
Project Member

Comment 5 by bugdroid1@chromium.org, May 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/afd793fce0cf63ed0ac18a37a5a5f2f92e92d9a4

commit afd793fce0cf63ed0ac18a37a5a5f2f92e92d9a4
Author: wutao <wutao@chromium.org>
Date: Fri May 12 01:38:50 2017

Check internal display RootWindow before getting the compositor.

When we find the id for the internal display it gets set and
Display::HasInternalDisplay() returns true. The internal display id
isn't reset, even when an internal display is removed. So even though we
might have an internal display id, there is no guarantee there is a
RootWindow for it.

BUG= 721452 
TEST=local device

Review-Url: https://codereview.chromium.org/2875123002
Cr-Commit-Position: refs/heads/master@{#471167}

[modify] https://crrev.com/afd793fce0cf63ed0ac18a37a5a5f2f92e92d9a4/ash/wallpaper/wallpaper_controller.cc

Comment 6 by wutao@chromium.org, May 12 2017

Status: Fixed (was: Assigned)
Cc: rjahagir@chromium.org helenzhang@chromium.org ka...@chromium.org sontis@chromium.org
Components: OS>Kernel>Display
Status: Verified (was: Fixed)
Verified with version 60.0.3100.0/9554.0.0 dev-channel

Sign in to add a comment