[Ozone-GBM] Handle GPU crashes

[Ozone-GBM] Handle GPU crashes

If the GPU process crashes, all the display configuration state is gone.
This allows the platform to gracefully handle GPU crashes and restore
the state once the GPU process spins up again.

BUG=none
NOTRY=true

Committed: https://crrev.com/a83cb2af0561e5f4f8f5d6be8b55613d18da95e0
Cr-Commit-Position: refs/heads/master@{#293399}

[dnicoara, 2014-08-29 22:36:48]

dnicoara@chromium.org changed reviewers:
+ alexst@chromium.org

[dnicoara, 2014-08-29 22:36:49]

PTAL

https://codereview.chromium.org/522463005/diff/1/ui/ozone/platform/dri/dri_wi...
File ui/ozone/platform/dri/dri_window_delegate_proxy.cc (right):

https://codereview.chromium.org/522463005/diff/1/ui/ozone/platform/dri/dri_wi...
ui/ozone/platform/dri/dri_window_delegate_proxy.cc:22: TRACE_EVENT1("dri",
"DriWindowDelegateProxy::Initialize", "widget", widget_);
Can I now remove the Initialize/Shutdown functions?

[alexst (slow to review), 2014-09-02 13:37:07]

https://codereview.chromium.org/522463005/diff/1/ui/ozone/platform/dri/dri_wi...
File ui/ozone/platform/dri/dri_window_delegate_proxy.cc (right):

https://codereview.chromium.org/522463005/diff/1/ui/ozone/platform/dri/dri_wi...
ui/ozone/platform/dri/dri_window_delegate_proxy.cc:22: TRACE_EVENT1("dri",
"DriWindowDelegateProxy::Initialize", "widget", widget_);
On 2014/08/29 22:36:48, dnicoara wrote:
> Can I now remove the Initialize/Shutdown functions?

Init can go, but shutdown still sends an IPC.

https://codereview.chromium.org/522463005/diff/1/ui/ozone/platform/dri/dri_wi...
ui/ozone/platform/dri/dri_window_delegate_proxy.cc:29: bool status =
sender_->Send(new OzoneGpuMsg_DestroyWindowDelegate(widget_));
Is it possible to not have a connection here? Something wacky like we were
restarting the GPU but then decided to shut down? Or we never received
OnChannelEstablished?

[dnicoara, 2014-09-03 17:17:25]

I've also updated the IPC message requesting the display configuration to allow
restoring the external monitor state correctly.

https://codereview.chromium.org/522463005/diff/1/ui/ozone/platform/dri/dri_wi...
File ui/ozone/platform/dri/dri_window_delegate_proxy.cc (right):

https://codereview.chromium.org/522463005/diff/1/ui/ozone/platform/dri/dri_wi...
ui/ozone/platform/dri/dri_window_delegate_proxy.cc:22: TRACE_EVENT1("dri",
"DriWindowDelegateProxy::Initialize", "widget", widget_);
On 2014/09/02 13:37:07, alexst wrote:
> On 2014/08/29 22:36:48, dnicoara wrote:
> > Can I now remove the Initialize/Shutdown functions?
> 
> Init can go, but shutdown still sends an IPC.

I like the symmetry so I'm going to leave it then.

https://codereview.chromium.org/522463005/diff/1/ui/ozone/platform/dri/dri_wi...
ui/ozone/platform/dri/dri_window_delegate_proxy.cc:29: bool status =
sender_->Send(new OzoneGpuMsg_DestroyWindowDelegate(widget_));
On 2014/09/02 13:37:07, alexst wrote:
> Is it possible to not have a connection here? Something wacky like we were
> restarting the GPU but then decided to shut down? Or we never received
> OnChannelEstablished?

Done.

[alexst (slow to review), 2014-09-04 14:44:45]

lgtm, with a nit

https://codereview.chromium.org/522463005/diff/20001/ui/ozone/platform/dri/sc...
File ui/ozone/platform/dri/screen_manager.cc (right):

https://codereview.chromium.org/522463005/diff/20001/ui/ozone/platform/dri/sc...
ui/ozone/platform/dri/screen_manager.cc:38: LOG(WARNING) << "Display controller
(crtc=" << crtc << ") already present.";
Do you expect this to be a regular frequent occurrence? If so does it make sense
to check this at call site? If it's meant as an unforeseen event, we should
DCHECK maybe?

[dnicoara, 2014-09-04 15:12:07]

https://codereview.chromium.org/522463005/diff/20001/ui/ozone/platform/dri/sc...
File ui/ozone/platform/dri/screen_manager.cc (right):

https://codereview.chromium.org/522463005/diff/20001/ui/ozone/platform/dri/sc...
ui/ozone/platform/dri/screen_manager.cc:38: LOG(WARNING) << "Display controller
(crtc=" << crtc << ") already present.";
On 2014/09/04 14:44:45, alexst wrote:
> Do you expect this to be a regular frequent occurrence? If so does it make
sense
> to check this at call site? If it's meant as an unforeseen event, we should
> DCHECK maybe?

This is a by-product of having to support a forced initialization for
content_shell and GPU process startup timing on CrOS. So it should really only
happen at start-up or when the GPU process crashes (and is restarted).

Though I did forget to add a comment, since this should be turned into a DCHECK
once display configuration is saner on the async path.

[dnicoara, 2014-09-04 15:13:30]

dnicoara@chromium.org changed reviewers:
+ kenrb@chromium.org

[dnicoara, 2014-09-04 15:13:31]

+Ken for IPC change

[kenrb, 2014-09-04 19:46:42]

ipc lgtm

[dnicoara, 2014-09-05 01:39:07]

The CQ bit was checked by dnicoara@chromium.org

[commit-bot: I haz the power, 2014-09-05 01:46:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dnicoara@chromium.org/522463005/60001

[commit-bot: I haz the power, 2014-09-05 01:58:09]

Committed patchset #4 (id:60001) as fe647532b9df0bfc9d0896fa3368b14ffca98936

[spang, 2014-09-05 18:27:47]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/548613002/ by spang@chromium.org.

The reason for reverting is: boots to blank screen.

[commit-bot: I haz the power, 2014-09-10 03:35:32]

Patchset 5 (id:??) landed as
https://crrev.com/a83cb2af0561e5f4f8f5d6be8b55613d18da95e0
Cr-Commit-Position: refs/heads/master@{#293399}