Fix content_unittests for Ozone GBM

Fix content_unittests for Ozone GBM

This doesn't actually fix all cases but a few at least now can pass when GBM
backend is used.

BUG=471261
TEST=content_unittests --ozone-platform=gbm --gtest_filter=GpuMemoryBufferImplTests/*

[vignatti (out of this project), 2015-03-28 00:40:41]

Patchset #2 (id:20001) has been deleted

[vignatti (out of this project), 2015-03-28 00:41:21]

tiago.vignatti@intel.com changed reviewers:
+ emircan@chromium.org, sky@chromium.org

[vignatti (out of this project), 2015-03-28 00:41:46]

emircan@ PTAL.

[sky, 2015-03-30 14:46:39]

https://codereview.chromium.org/1040873002/diff/1/content/test/content_test_s...
File content/test/content_test_suite.cc (right):

https://codereview.chromium.org/1040873002/diff/1/content/test/content_test_s...
content/test/content_test_suite.cc:118: base::MessageLoopForUI main_loop;
None of the other platforms initialize the message loop here. Why does ozone
need to do it here and not else where like other platforms?

[vignatti (out of this project), 2015-03-30 15:01:50]

https://codereview.chromium.org/1040873002/diff/1/content/test/content_test_s...
File content/test/content_test_suite.cc (right):

https://codereview.chromium.org/1040873002/diff/1/content/test/content_test_s...
content/test/content_test_suite.cc:118: base::MessageLoopForUI main_loop;
On 2015/03/30 14:46:38, sky wrote:
> None of the other platforms initialize the message loop here. Why does ozone
> need to do it here and not else where like other platforms?

A few Ozone implementations like the GBM requires early initialization of the
message loop because GL stuff, display management and etc (most of hardware
accesses) happen in the GPU process only. So we need the message loop in place
already for the GPU process initial handshake behave correctly... ideally X11
backend ought to be like that.

I can write a comment in the code like that if you prefer.

[sky, 2015-03-30 15:30:50]

This will be problematic for any tests that also try to create a
messageloop. I don't think this should be done in ContentTestSuite.

On Mon, Mar 30, 2015 at 8:01 AM,  <tiago.vignatti@intel.com> wrote:
>
>
https://codereview.chromium.org/1040873002/diff/1/content/test/content_test_s...
> File content/test/content_test_suite.cc (right):
>
>
https://codereview.chromium.org/1040873002/diff/1/content/test/content_test_s...
> content/test/content_test_suite.cc:118: base::MessageLoopForUI
> main_loop;
> On 2015/03/30 14:46:38, sky wrote:
>>
>> None of the other platforms initialize the message loop here. Why does
>
> ozone
>>
>> need to do it here and not else where like other platforms?
>
>
> A few Ozone implementations like the GBM requires early initialization
> of the message loop because GL stuff, display management and etc (most
> of hardware accesses) happen in the GPU process only. So we need the
> message loop in place already for the GPU process initial handshake
> behave correctly... ideally X11 backend ought to be like that.
>
> I can write a comment in the code like that if you prefer.
>
> https://codereview.chromium.org/1040873002/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[vignatti (out of this project), 2015-03-30 19:12:21]

On 2015/03/30 15:30:50, sky wrote:
> This will be problematic for any tests that also try to create a
> messageloop. I don't think this should be done in ContentTestSuite.

cc-ing dnicoara and spang, Ozone GBM owners. Let me try to summarize what's
going on: 

- Ozone GBM is special because it requires the UI message loop initialized early
for event polling & other background tests (for display, cursor and other
graphics stuff), way before InitializeOneOff is called and several unit tests
start to process.

- chrome, ozone_demo, video_decode_accelerator_unittest and other targets
already initializes the message loop early. That works just fine. Though a bunch
of unit tests, including content_unittests, now in Ozone GBM are broken due the
lack of this UI message loop (check also
https://codereview.chromium.org/1025523005/ for gl_tests, gpu_unittests). Now
I'd d like to follow this idea in here.

- A possible alternative is to be more resilient when starting Ozone GBM,
letting the main_thread_task_runner_ be NULL when the channel handshake happens
(gpu/drm_gpu_platform_support.cc). I've tried that for content_unittests and it
works fine. wdyt dnicoara/spang?

[vignatti (out of this project), 2015-03-31 14:08:45]

On 2015/03/30 19:12:21, vignatti wrote:
> 
> - A possible alternative is to be more resilient when starting Ozone GBM,
> letting the main_thread_task_runner_ be NULL when the channel handshake
happens
> (gpu/drm_gpu_platform_support.cc). I've tried that for content_unittests and
it
> works fine. wdyt dnicoara/spang?

FYI this is the inner GBM solution I'm proposing that fix most of tests,
including content_unittests: https://codereview.chromium.org/1042903004/

I'm just waiting dnicoara@ review now. Thanks.

[spang, 2015-03-31 17:00:24]

On 2015/03/31 14:08:45, vignatti wrote:
> On 2015/03/30 19:12:21, vignatti wrote:
> > 
> > - A possible alternative is to be more resilient when starting Ozone GBM,
> > letting the main_thread_task_runner_ be NULL when the channel handshake
> happens
> > (gpu/drm_gpu_platform_support.cc). I've tried that for content_unittests and
> it
> > works fine. wdyt dnicoara/spang?
> 
> FYI this is the inner GBM solution I'm proposing that fix most of tests,
> including content_unittests: https://codereview.chromium.org/1042903004/
> 
> I'm just waiting dnicoara@ review now. Thanks.

I've been thinking we needed a 2nd initializer that runs after message loop
start for a while now - see crbug.com/450919.

I think that's probably the way to handle this. I don't think we want to fall
through using ThreadTaskRunnerHandle::IsSet() - if we allow it to happen later,
we should make a new function for it.

[emircan, 2015-03-31 19:09:32]

On 2015/03/31 17:00:24, spang wrote:
> I've been thinking we needed a 2nd initializer that runs after message loop
> start for a while now - see crbug.com/450919.

Is there a delegate method from the base::LazyInstance that informs you on
completion? I am sorry if I am missing obvious things, as I'm relatively a
noogler and still finding out things. 

Another alternative can be what they are doing over here
https://code.google.com/p/chromium/codesearch#chromium/src/sync/internal_api/...
We can do a similar conditional initialization in ctor for
main_thread_task_runner_.

[vignatti (out of this project), 2015-04-08 21:56:03]

On 2015/03/31 17:00:24, spang wrote:
> 
> I've been thinking we needed a 2nd initializer that runs after message loop
> start for a while now - see crbug.com/450919.

this is now tracked here: https://codereview.chromium.org/1043233003/


> I think that's probably the way to handle this. I don't think we want to fall
> through using ThreadTaskRunnerHandle::IsSet() - if we allow it to happen
later,
> we should make a new function for it.

yup, fine. Per offline discussions I had with spang@ (summarized in
https://code.google.com/p/chromium/issues/detail?id=471261#c3), I'll will close
this CL. Thanks!