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

Issue 779845 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 764472
issue 770243



Sign in to add a comment

Mus: DCHECK crashes adding and removing displays with dev shortcut

Project Member Reported by msw@chromium.org, Oct 30 2017

Issue description

Mus: DCHECK crashes adding and removing displays with dev shortcut

Running Chrome OS on linux desktop on ToT @ #512649:

CRASH #1:
(1) Run chrome --mus --ash-dev-shortcuts
(2) Press Ctrl+Shift+D (the add/remove display dev shortcut)
Expected: A second display window opens
Actual: Crash! @ https://cs.chromium.org/chromium/src/ui/display/display.cc?l=70
[100057:100057:1030/161358.989424:FATAL:display.cc(70)] Check failed: index_1 != index_2 (0 vs. 0)2200000000 and 36029742295586560
#0 0x7f2027d5fd9c base::debug::StackTrace::StackTrace()
#1 0x7f2027d8636c logging::LogMessage::~LogMessage()
#2 0x7f202401ae40 display::CompareDisplayIds()
#3 0x7f2022a05cc5 std::__1::__sort<>()
#4 0x7f20229fd749 display::DisplayManager::UpdateDisplaysWith()
#5 0x7f2022a014ab display::DisplayManager::AddRemoveDisplay()
#6 0x7f2022660549 ash::AcceleratorController::PerformAction()
...

CRASH #2:
(1) Run chrome --mus --ash-dev-shortcuts --screen-config=500x500,500x500
(2) Press Ctrl+Shift+D (the add/remove display dev shortcut)
Expected: The second display window closes
Actual: Crash! @ https://cs.chromium.org/chromium/src/ui/gl/gl_context_glx.cc?l=221
[100616:100658:1030/161756.230397:ERROR:gl_context_glx.cc(232)] Couldn't make context current with X drawable.
[100616:100658:1030/161756.231844:ERROR:gles2_cmd_decoder.cc(4445)]   GLES2DecoderImpl: Context lost during MakeCurrent.
[100616:100658:1030/161756.231874:ERROR:in_process_command_buffer.cc(242)] Context lost because MakeCurrent failed.
[100616:100658:1030/161756.231896:ERROR:in_process_command_buffer.cc(238)] MakeCurrent failed because context lost.
[100616:100658:1030/161756.234248:FATAL:gl_context_glx.cc(221)] Check failed: context_. 
#0 0x7f3b19a51d9c base::debug::StackTrace::StackTrace()
#1 0x7f3b19a7836c logging::LogMessage::~LogMessage()
#2 0x7f3b15df23a4 gl::GLContextGLX::MakeCurrent()
#3 0x7f3b15dc35ef gl::GLContext::MakeVirtuallyCurrent()
#4 0x7f3b15f45e4e gpu::GLContextVirtual::MakeCurrent()
#5 0x7f3b16041e66 gpu::InProcessCommandBuffer::DestroyOnGpuThread()
#6 0x7f3b16042129 gpu::(anonymous namespace)::RunTaskWithResult<>()

These are blocking my work on Mus unified/mirroring display support.
Kyle, I'll start taking a look, but I'm hoping you can help/triage, thanks!
 

Comment 1 by sadrul@chromium.org, Oct 31 2017

https://chromium-review.googlesource.com/c/chromium/src/+/734945 may help with this. The CL just landed. Can you try with that?
Status: Started (was: Untriaged)
For crash #1 it's because there are two ways to fake displays and --mus isn't wired correctly. It mixes things up. The display management stack looks like this:

<ash>
display::DisplayManager
display::DisplayChangeObserver
display::DisplayConfiguration
display::NativeDisplayDelegate
<ozone>
<hardware>

In classic ash everything is faked in display::DisplayManager. All the code underneath that doesn't get run off device.

In --mash I added FakeDisplayDelegate so we could exercise all the code. It implements NativeDisplayDelegate and pretends there is hardware to talk to. All the display configuration code runs, even off device, which avoids errors. I did this work before --mus and it never got completed. I'll fix --mus but I just tested and --mash does work correctly (for adding a display at least, crash #2 happens for removing still).

For crash #2 it's related to the issue sadrul fixed. In classic ash there is a synchronous IPC when shutting down the display compositor that makes sure certain GPU things are destroyed before the PlatformWindow is destroyed. This synchronous IPC doesn't exist in mus/mash. The problem is GLX specific and it disappears if you use EGL. 

msw: You can use this command line assuming --mash works for you.

$ ./chrome --mash --ash-dev-shortcuts --use-gl=egl

sadrul: There is a still a shutdown GLX crash in mus/mash with your CL landed.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 31 2017

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

commit 7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f
Author: kylechar <kylechar@chromium.org>
Date: Tue Oct 31 23:26:57 2017

mus: Fix ash add/remove display accelerator.

This CL fixes the ash add/remove display accelerators when running with
--mus. When the accelerator is pressed it needs to call
TestDisplayController::AddRemoveDisplay(). The TestDisplayControllerPtr
is moved into DisplayManager to simplify things. This allows removing
the special case for the DEV_ADD_REMOVE_DISPLAY accelerator.

Both --mus and --mash will set TestDisplayControllerPtr on startup if
they are running on Linux desktop. It would be possible to do something
similar with classic ash in the future.

Bug:  779845 
Change-Id: I70b058777944217e55ef105b9d1ac65f05e28b3a
Reviewed-on: https://chromium-review.googlesource.com/746375
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512996}
[modify] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/ash/accelerators/accelerator_controller.cc
[modify] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/ash/accelerators/accelerator_controller_delegate_classic.cc
[modify] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/ash/mus/accelerators/accelerator_controller_delegate_mus.cc
[modify] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/ash/mus/accelerators/accelerator_controller_delegate_mus.h
[modify] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/ash/mus/manifest.json
[modify] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/ash/mus/standalone/manifest.json
[modify] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/ash/shell.cc
[modify] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/chrome/browser/chrome_content_browser_manifest_overlay.json
[modify] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/services/ui/display/screen_manager_forwarding.cc
[modify] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/services/ui/display/screen_manager_forwarding.h
[modify] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/services/ui/display/screen_manager_ozone_internal.cc
[modify] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/services/ui/display/screen_manager_ozone_internal.h
[modify] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/services/ui/manifest.json
[modify] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/services/ui/public/interfaces/display/BUILD.gn
[modify] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/ui/display/manager/display_manager.cc
[modify] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/ui/display/manager/display_manager.h
[modify] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/ui/display/mojo/BUILD.gn
[rename] https://crrev.com/7e4f46fe768fd1a5f3ef36dea88550ab77de6d9f/ui/display/mojo/dev_display_controller.mojom

Status: Fixed (was: Started)
The add/remove shortcuts work at ToT in --mus now. Sadrul got crash #2 fixed in / crbug.com/777594  too.

Comment 5 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 6 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)
Components: -Internals>MUS Internals>Services>WindowService
Components: -MUS

Sign in to add a comment