oshima@ - The problem is that DisplayPrefs::LoadDisplayPreferences is now called asynchronously in Ash from OnLocalStatePrefServiceInitialized, and LoadDisplayPreferences calls display_configurator()->SetInitialDisplayPower().

Is the DCHECK needed? It's not entirely clear why it is there.

As a temporary workaround, the DCHECK can be commented out and the code will run, but I'm not certain whether there will be artifacts when run on a device.

Here is the problem:

1. Currently for mus/mash we call display_configurator_->ForceInitialConfigure() which sets DisplayConfigurator::current_display_state_. This triggers the DCHECK above.

2. Shell::Init calls WindowTreeHostManager::CreatePrimaryHost() about halfway through Init; deferring that is highly problematic. WindowTreeHostManager::CreatePrimaryHost() calls Shell::display_manager()->GetPrimaryDisplayCandidate(), which will return null in mus/mash if display_configurator_->ForceInitialConfigure() has not been called.

Long term I think someone more familiar with this code needs to carefully map out the timing of all of this for mus/mash.

Short term it seems like we should be able to just call SetDisplayPower() from SetInitialDisplayPower() if current_display_state_ has already been set?

I took a stab at deferring the code in DisplayConfigurator::OnConfigured that sets current_display_state_ if SetInitialDisplayPower has not been called yet, but that gets kind of complicated pretty quickly.

oshima@ - any thoughts here?

Is the change of making LoadDisplayPreferences async new? If that's the core of the problem then it seems that should be reverted until we sort out dependencies.

It is new, but since we unfortunately did not identify this when the breaking change first landed, several other dependent changes have also landed so reverting is not trivial.

Unfortunately I am encoutering strange errors when attempting to run with --mash via chromecast making it difficult for me to test any fixes or reverts:

[134889:134889:0111/085552.629439:ERROR:shell_delegate_mus.cc(61)] Not implemented reached in virtual bool ash::ShellDelegateMus::CanShowWindowForUser(aura::Window *) const
Received signal 11 SEGV_MAPERR 01521e97a800
#0 0x55652ea9caac base::debug::StackTrace::StackTrace()
#1 0x55652ea9c5a1 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#2 0x7f8295b400c0 <unknown>
#3 0x7f829282fdef <unknown>
#4 0x7f829437111e <unknown>
#5 0x7f8294371b21 <unknown>
#6 0x7f82943712ae <unknown>
#7 0x7f82943720d2 XPutImage
#8 0x7f82895f5f09 <unknown>
#9 0x7f82876f30c1 <unknown>
#10 0x7f8287831b01 <unknown>
#11 0x7f82876f31cb <unknown>
#12 0x55652f8da66b gl::NativeViewGLSurfaceGLX::PostSubBuffer()
#13 0x55652f8c860c gl::GLSurfaceAdapter::PostSubBuffer()
#14 0x55652ff47939 gpu::PassThroughImageTransportSurface::PostSubBuffer()
#15 0x5565327fb2b7 gpu::gles2::GLES2DecoderImpl::HandlePostSubBufferCHROMIUM()
#16 0x556532815b45 gpu::gles2::GLES2DecoderImpl::DoCommandsImpl<>()
#17 0x5565328d4ad9 LaunchProcess: failed to execvp:
/usr/local/sbin/chrome-devel-sandbox
gpu::CommandBufferService::Flush()
#18 0x55652ff4043a gpu::InProcessCommandBuffer::FlushOnGpuThread()
#19 0x55652e6bd32e _ZN4base8internal7InvokerINS0_9BindStateIMN10extensions17MessagingBindingsEFvibEJNS_7WeakPtrIS4_EEibEEEFvvEE7RunImplIRKS6_RKNSt3__15tupleIJS8_ibEEEJLm0ELm1ELm2EEEEvOT_OT0_NSF_16integer_sequenceImJXspT1_EEEE
#20 0x55652ff40008 gpu::InProcessCommandBuffer::ProcessTasksOnGpuThread()
#21 0x55652c339917 _ZN4base8internal7InvokerINS0_9BindStateIMN11google_apis19UrlFetchRequestBaseEFvvEJNS_7WeakPtrINS3_5drive30SingleBatchableDelegateRequestEEEEEEFvvEE3RunEPNS0_13BindStateBaseE
#22 0x55652ea9d265 base::debug::TaskAnnotator::RunTask()
#23 0x55652eb51c49 base::internal::IncomingTaskQueue::RunTask()
#24 0x55652eabcdeb base::MessageLoop::RunTask()
#25 0x55652eabd183 base::MessageLoop::DeferOrRunPendingTask()
#26 0x55652eabd416 LaunchProcess: failed to execvp:
/usr/local/sbin/chrome-devel-sandbox
base::MessageLoop::DoWork()
#27 0x55652eabe640 base::MessagePumpDefault::Run()

If we don't have a clear path forward, please revert all the dependent patches too. 

Also, do we have any idea as to why none of the bots caught this?

There is a CL up with a fix/workaround up that oshima has approved:
https://chromium-review.googlesource.com/c/chromium/src/+/861288 

The pref state (prefs::kDisplayPowerState) that triggers this crash probably does not exist for browser tests (if the pref deoes not exist we don't call the method that triggers the DCHECK).

Once we have Chrome OS VM tests running on the chromium waterfall (still in progress) we should consider adding a mus version sooner than later. That's really the only environment where we might have caught this.

So this will only crash if you have a user-data-directory that you used before that has a local-state pref related to the display? That might explain why existing tests didn't catch it -- I think they default to an empty data dir.

Note that VM tests would not have caught this one. We have an autotest called desktopui_MusLogin that starts chrome on device with --mus. That test runs on the FYI waterfall in a step called HWTest chrome-informational. That step is green on the waterfall:
https://uberchromegw.corp.google.com/i/chromeos.chrome/waterfall?reload=300

(We also have desktopui_MashLogin for --mash.)

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

commit e30d522fe90b365b0d33e1c28a66b89776771c9a
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Thu Jan 11 21:13:16 2018

DisplayPrefs: Fix mus/mash behavior

Bug:  http://crbug.com/800925 
Change-Id: I83b0282c069da427111fa546168474eacfc2be24
Reviewed-on: https://chromium-review.googlesource.com/861288
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528753}
[modify] https://crrev.com/e30d522fe90b365b0d33e1c28a66b89776771c9a/ash/display/display_prefs.cc
[modify] https://crrev.com/e30d522fe90b365b0d33e1c28a66b89776771c9a/ash/display/display_prefs_unittest.cc

OK, this should be fixed on ToT. Apologies for the breakage.

Note: Before I moved DisplayPrefs to src/ash, mus/mash did not have a DisplayPrefs instance because its shell delegate was not implemented. The new behavior is slightly different on mus/mash than on classic ash and may or may not work correctlym but it is commented (referencing this bug) and is at least provides a potential path forward.

It turns out we have a similar issue in classic Ash on a device. I am going to revert the original changes instead.

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

commit 5247528b5edd247838ee72887c5c3afac284208a
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Fri Jan 12 19:39:15 2018

Revert "DisplayPrefs: Fix mus/mash behavior"

This reverts commit e30d522fe90b365b0d33e1c28a66b89776771c9a.

Reason for revert: Dependent on http://crrev.com/c/849633 which
needs to be reverted because of:
*  http://crbug.com/801394 
*  http://crbug.com/800925 

Original change's description:
> DisplayPrefs: Fix mus/mash behavior
> 
> Bug:  http://crbug.com/800925 
> Change-Id: I83b0282c069da427111fa546168474eacfc2be24
> Reviewed-on: https://chromium-review.googlesource.com/861288
> Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
> Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#528753}

TBR=stevenjb@chromium.org,oshima@chromium.org

Change-Id: I527434f1d7d8ce49f0d4f867d4315027ea1710cf
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  http://crbug.com/800925 
Reviewed-on: https://chromium-review.googlesource.com/864923
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529025}
[modify] https://crrev.com/5247528b5edd247838ee72887c5c3afac284208a/ash/display/display_prefs.cc
[modify] https://crrev.com/5247528b5edd247838ee72887c5c3afac284208a/ash/display/display_prefs_unittest.cc