[Ozone-Dri] Decouple the IO helper thread from DriWrapper

[Ozone-Dri] Decouple the IO helper thread from DriWrapper

Once we support multiple DRM devices we'll want to share the helper
thread between the different wrappers.

BUG=427959
NOTRY=true

Committed: https://crrev.com/3085f02cdf77021cc4307d45762573bb5c4fea5e
Cr-Commit-Position: refs/heads/master@{#312902}

[dnicoara, 2015-01-21 21:05:51]

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

[dnicoara, 2015-01-21 21:05:51]

PTAL

This depends on https://codereview.chromium.org/812933003/

[alexst (slow to review), 2015-01-22 15:27:48]

https://codereview.chromium.org/856423002/diff/20001/ui/ozone/platform/dri/dr...
File ui/ozone/platform/dri/dri.gypi (right):

https://codereview.chromium.org/856423002/diff/20001/ui/ozone/platform/dri/dr...
ui/ozone/platform/dri/dri.gypi:85: 'io_helper_thread.cc',
Let's call it drm_helper_thread, I think that would descibe its purpose better.

https://codereview.chromium.org/856423002/diff/20001/ui/ozone/platform/dri/dr...
File ui/ozone/platform/dri/dri_wrapper.cc (right):

https://codereview.chromium.org/856423002/diff/20001/ui/ozone/platform/dri/dr...
ui/ozone/platform/dri/dri_wrapper.cc:170: if (!io_thread_->IsRunning())
We should do this in
https://code.google.com/p/chromium/codesearch#chromium/src/ui/ozone/platform/...

It'll happen just before any ipc's start flowing.

[dnicoara, 2015-01-22 22:50:14]

https://codereview.chromium.org/856423002/diff/20001/ui/ozone/platform/dri/dr...
File ui/ozone/platform/dri/dri.gypi (right):

https://codereview.chromium.org/856423002/diff/20001/ui/ozone/platform/dri/dr...
ui/ozone/platform/dri/dri.gypi:85: 'io_helper_thread.cc',
On 2015/01/22 15:27:48, alexst wrote:
> Let's call it drm_helper_thread, I think that would descibe its purpose
better.

Done ... I named it dri_helper_thread in order to be more consistent with the
naming use within this folder. Let me know if you'd like it changed.

https://codereview.chromium.org/856423002/diff/20001/ui/ozone/platform/dri/dr...
File ui/ozone/platform/dri/dri_wrapper.cc (right):

https://codereview.chromium.org/856423002/diff/20001/ui/ozone/platform/dri/dr...
ui/ozone/platform/dri/dri_wrapper.cc:170: if (!io_thread_->IsRunning())
On 2015/01/22 15:27:48, alexst wrote:
> We should do this in
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/ozone/platform/...
> 
> It'll happen just before any ipc's start flowing.

Done.

[alexst (slow to review), 2015-01-22 23:09:24]

https://codereview.chromium.org/856423002/diff/40001/ui/ozone/platform/dri/dr...
File ui/ozone/platform/dri/dri_gpu_platform_support.cc (right):

https://codereview.chromium.org/856423002/diff/40001/ui/ozone/platform/dri/dr...
ui/ozone/platform/dri/dri_gpu_platform_support.cc:45: if (helper_thread_)
So it occurred to me, we shouldn't be passing the helper_thread object between
the threads, that's not safe, the only thing that should cross thread boundaries
is the task runner.

If we call init it here, the dri_wrapper could be getting the task runner while
it's being set on another thread. It's racy.

I am thinking, should we create the thread object and initialize it here and
pass its task_runner to dri by posting to main instead?

Eventually, we will need the task runner here in the filter as well because
that's where the messages will go.

[dnicoara, 2015-01-23 17:13:06]

https://codereview.chromium.org/856423002/diff/40001/ui/ozone/platform/dri/dr...
File ui/ozone/platform/dri/dri_gpu_platform_support.cc (right):

https://codereview.chromium.org/856423002/diff/40001/ui/ozone/platform/dri/dr...
ui/ozone/platform/dri/dri_gpu_platform_support.cc:45: if (helper_thread_)
On 2015/01/22 23:09:24, alexst wrote:
> So it occurred to me, we shouldn't be passing the helper_thread object between
> the threads, that's not safe, the only thing that should cross thread
boundaries
> is the task runner.
> 
> If we call init it here, the dri_wrapper could be getting the task runner
while
> it's being set on another thread. It's racy.
> 
> I am thinking, should we create the thread object and initialize it here and
> pass its task_runner to dri by posting to main instead?
> 
> Eventually, we will need the task runner here in the filter as well because
> that's where the messages will go.

Done.

I've moved the helper thread in here and Initialized on main as per your
suggestion.

I think we'll need to move the helper thread but currently it is not clear to me
how the GPU initialization needs to be ordered so I'm taking it one step at a
time.

[alexst (slow to review), 2015-01-23 18:28:52]

On 2015/01/23 17:13:06, dnicoara wrote:
>
https://codereview.chromium.org/856423002/diff/40001/ui/ozone/platform/dri/dr...
> File ui/ozone/platform/dri/dri_gpu_platform_support.cc (right):
> 
>
https://codereview.chromium.org/856423002/diff/40001/ui/ozone/platform/dri/dr...
> ui/ozone/platform/dri/dri_gpu_platform_support.cc:45: if (helper_thread_)
> On 2015/01/22 23:09:24, alexst wrote:
> > So it occurred to me, we shouldn't be passing the helper_thread object
between
> > the threads, that's not safe, the only thing that should cross thread
> boundaries
> > is the task runner.
> > 
> > If we call init it here, the dri_wrapper could be getting the task runner
> while
> > it's being set on another thread. It's racy.
> > 
> > I am thinking, should we create the thread object and initialize it here and
> > pass its task_runner to dri by posting to main instead?
> > 
> > Eventually, we will need the task runner here in the filter as well because
> > that's where the messages will go.
> 
> Done.
> 
> I've moved the helper thread in here and Initialized on main as per your
> suggestion.
> 
> I think we'll need to move the helper thread but currently it is not clear to
me
> how the GPU initialization needs to be ordered so I'm taking it one step at a
> time.

Okay, I agree, this is moving us in the right direction. lgtm.

[dnicoara, 2015-01-23 19:42:50]

The CQ bit was checked by dnicoara@chromium.org

[commit-bot: I haz the power, 2015-01-23 19:44:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/856423002/80001

[commit-bot: I haz the power, 2015-01-23 19:46:14]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-01-23 19:48:02]

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