In-mus ozone drm platform implementation.

In-mus ozone drm platform implementation.

Add an in-process bridge between the gpu, main and drm threads so that
the drm ozone implementation can operate in single-process mode.

BUG=558602
Committed: https://crrev.com/5f8dbf8e02accdd13c43556494a5812ff80136ec
Cr-Commit-Position: refs/heads/master@{#383856}

[rjkroege, 2016-03-24 00:46:56]

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

[rjkroege, 2016-03-24 00:46:56]

PTAL

[dnicoara, 2016-03-24 02:07:44]

https://codereview.chromium.org/1831693002/diff/1/ui/ozone/platform/drm/mus_t...
File ui/ozone/platform/drm/mus_thread_proxy.cc (right):

https://codereview.chromium.org/1831693002/diff/1/ui/ozone/platform/drm/mus_t...
ui/ozone/platform/drm/mus_thread_proxy.cc:299:
DCHECK(on_drm_thread_.CalledOnValidThread());
I think these DCHECKs are failing since CreateSafeCallback() will handle posting
the callback to the calling thread (which is the window server thread).

https://codereview.chromium.org/1831693002/diff/1/ui/ozone/platform/drm/ozone...
File ui/ozone/platform/drm/ozone_platform_gbm.cc (right):

https://codereview.chromium.org/1831693002/diff/1/ui/ozone/platform/drm/ozone...
ui/ozone/platform/drm/ozone_platform_gbm.cc:160: gl_api_loader_.reset(new
GlApiLoader());
Is this really needed here? Or can it be shared inside InitializeGPU()?

[rjkroege, 2016-03-28 22:53:30]

ptal

https://codereview.chromium.org/1831693002/diff/1/ui/ozone/platform/drm/mus_t...
File ui/ozone/platform/drm/mus_thread_proxy.cc (right):

https://codereview.chromium.org/1831693002/diff/1/ui/ozone/platform/drm/mus_t...
ui/ozone/platform/drm/mus_thread_proxy.cc:299:
DCHECK(on_drm_thread_.CalledOnValidThread());
On 2016/03/24 02:07:44, dnicoara wrote:
> I think these DCHECKs are failing since CreateSafeCallback() will handle
posting
> the callback to the calling thread (which is the window server thread).

Fixed here and elsewhere.

https://codereview.chromium.org/1831693002/diff/1/ui/ozone/platform/drm/ozone...
File ui/ozone/platform/drm/ozone_platform_gbm.cc (right):

https://codereview.chromium.org/1831693002/diff/1/ui/ozone/platform/drm/ozone...
ui/ozone/platform/drm/ozone_platform_gbm.cc:160: gl_api_loader_.reset(new
GlApiLoader());
On 2016/03/24 02:07:44, dnicoara wrote:
> Is this really needed here? Or can it be shared inside InitializeGPU()?

It's needed here to permit mus to load the GL library before (future work)
enabling the sandbox.

[dnicoara, 2016-03-29 13:48:22]

https://codereview.chromium.org/1831693002/diff/20001/ui/ozone/platform/drm/m...
File ui/ozone/platform/drm/mus_thread_proxy.cc (right):

https://codereview.chromium.org/1831693002/diff/20001/ui/ozone/platform/drm/m...
ui/ozone/platform/drm/mus_thread_proxy.cc:58: on_drm_thread_.DetachFromThread();
This shouldn't detach from DRM thread if this is called on the DRM thread. This
should actually check that it is called on the DRM thread. The detach should
happen on the thread that creates it (if different than the thread it should be
running on).

... That said I can only spot 1 call site in here, so probably just remove it
entirely?

https://codereview.chromium.org/1831693002/diff/20001/ui/ozone/platform/drm/m...
ui/ozone/platform/drm/mus_thread_proxy.cc:296: void
MusThreadProxy::GpuCheckOverlayCapabilitiesCallback(
Given that all these callback are supposed to run on the window server thread,
maybe DCHECK that.

https://codereview.chromium.org/1831693002/diff/20001/ui/ozone/platform/drm/m...
ui/ozone/platform/drm/mus_thread_proxy.cc:299: ws_task_runner_->PostTask(
I think the post task can be skipped since this should already be running on the
window server thread. (Same for the other callbacks.)

[rjkroege, 2016-03-29 21:48:34]

PTAL

https://codereview.chromium.org/1831693002/diff/20001/ui/ozone/platform/drm/m...
File ui/ozone/platform/drm/mus_thread_proxy.cc (right):

https://codereview.chromium.org/1831693002/diff/20001/ui/ozone/platform/drm/m...
ui/ozone/platform/drm/mus_thread_proxy.cc:58: on_drm_thread_.DetachFromThread();
On 2016/03/29 13:48:22, dnicoara wrote:
> This shouldn't detach from DRM thread if this is called on the DRM thread. 

The on_drm_thread is initialized on a different thread and hence has that thread
as its identity.

> This
> should actually check that it is called on the DRM thread. The detach should
> happen on the thread that creates it (if different than the thread it should
be
> running on).

Yes Probably.

> 
> ... That said I can only spot 1 call site in here, so probably just remove it
> entirely?

I like that choice. Done.

https://codereview.chromium.org/1831693002/diff/20001/ui/ozone/platform/drm/m...
ui/ozone/platform/drm/mus_thread_proxy.cc:299: ws_task_runner_->PostTask(
On 2016/03/29 13:48:22, dnicoara wrote:
> I think the post task can be skipped since this should already be running on
the
> window server thread. (Same for the other callbacks.)

Excellent point. Thanks. Done.

[dnicoara, 2016-03-29 21:56:35]

Thanks! LGTM

[dnicoara, 2016-03-29 21:58:38]

https://codereview.chromium.org/1831693002/diff/1/ui/ozone/platform/drm/ozone...
File ui/ozone/platform/drm/ozone_platform_gbm.cc (right):

https://codereview.chromium.org/1831693002/diff/1/ui/ozone/platform/drm/ozone...
ui/ozone/platform/drm/ozone_platform_gbm.cc:160: gl_api_loader_.reset(new
GlApiLoader());
On 2016/03/28 22:53:30, rjkroege wrote:
> On 2016/03/24 02:07:44, dnicoara wrote:
> > Is this really needed here? Or can it be shared inside InitializeGPU()?
> 
> It's needed here to permit mus to load the GL library before (future work)
> enabling the sandbox.

I forgot about this ... InitializeGPU() should always be called before the
sandbox is initialized. What's different in MUS that violates this assumption?

[rjkroege, 2016-03-29 22:14:03]

On 2016/03/29 21:58:38, dnicoara wrote:
>
https://codereview.chromium.org/1831693002/diff/1/ui/ozone/platform/drm/ozone...
> File ui/ozone/platform/drm/ozone_platform_gbm.cc (right):
> 
>
https://codereview.chromium.org/1831693002/diff/1/ui/ozone/platform/drm/ozone...
> ui/ozone/platform/drm/ozone_platform_gbm.cc:160: gl_api_loader_.reset(new
> GlApiLoader());
> On 2016/03/28 22:53:30, rjkroege wrote:
> > On 2016/03/24 02:07:44, dnicoara wrote:
> > > Is this really needed here? Or can it be shared inside InitializeGPU()?
> > 
> > It's needed here to permit mus to load the GL library before (future work)
> > enabling the sandbox.
> 
> I forgot about this ... InitializeGPU() should always be called before the
> sandbox is initialized. What's different in MUS that violates this assumption?

My understanding is that the sandbox must be entered before threads are made. UI
and GPU are in different processes in Chrome but are merely different threads in
Mus. So initialization has to be divided into pre-sandbox portion and
post-sandbox GPU-thread portion.

[rjkroege, 2016-03-29 22:14:43]

The CQ bit was checked by rjkroege@chromium.org

[commit-bot: I haz the power, 2016-03-29 22:15:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1831693002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1831693002/40001

[commit-bot: I haz the power, 2016-03-29 23:07:19]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-29 23:08:45]

Description was changed from

==========
In-mus ozone drm platform implementation.

Add an in-process bridge between the gpu, main and drm threads so that
the drm ozone implementation can operate in single-process mode.

BUG=558602
==========

to

==========
In-mus ozone drm platform implementation.

Add an in-process bridge between the gpu, main and drm threads so that
the drm ozone implementation can operate in single-process mode.

BUG=558602

Committed: https://crrev.com/5f8dbf8e02accdd13c43556494a5812ff80136ec
Cr-Commit-Position: refs/heads/master@{#383856}
==========

[commit-bot: I haz the power, 2016-03-29 23:08:48]

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

[dnicoara, 2016-03-30 13:33:30]

On 2016/03/29 22:14:03, rjkroege wrote:
> On 2016/03/29 21:58:38, dnicoara wrote:
> >
>
https://codereview.chromium.org/1831693002/diff/1/ui/ozone/platform/drm/ozone...
> > File ui/ozone/platform/drm/ozone_platform_gbm.cc (right):
> > 
> >
>
https://codereview.chromium.org/1831693002/diff/1/ui/ozone/platform/drm/ozone...
> > ui/ozone/platform/drm/ozone_platform_gbm.cc:160: gl_api_loader_.reset(new
> > GlApiLoader());
> > On 2016/03/28 22:53:30, rjkroege wrote:
> > > On 2016/03/24 02:07:44, dnicoara wrote:
> > > > Is this really needed here? Or can it be shared inside InitializeGPU()?
> > > 
> > > It's needed here to permit mus to load the GL library before (future work)
> > > enabling the sandbox.
> > 
> > I forgot about this ... InitializeGPU() should always be called before the
> > sandbox is initialized. What's different in MUS that violates this
assumption?
> 
> My understanding is that the sandbox must be entered before threads are made.
UI
> and GPU are in different processes in Chrome but are merely different threads
in
> Mus. So initialization has to be divided into pre-sandbox portion and
> post-sandbox GPU-thread portion.

I think GPU initialization will still be broken when the sandbox is enabled
since the GL bindings are loaded during GLSurface::InitializeOneOff() which
would be called in the GPU thread after the sandbox is enabled.