[WIP] Pass the single_process parameter to Ozone's initialization functions

[WIP] Pass the single_process parameter to Ozone's initialization functions

Calls to the InitializeForUI and InitializeForGPU functions of Ozone
are updated to pass the single_process parameter. It is always false
for Mus and otherwise parsed from the command line.

The call to InitializeForGPU is done in the initialization of the GPU
thread, as suggested in https://codereview.chromium.org/2270463002.
This is also more convenient to determine the init parameters.

Similarly, the determination of whether Mus is used and of the value
of single_process is moved from ui/aura to content/browser, where
this task becomes easier. This information is transmitted to
aura::Env::CreateInstance via new boolean value parameters.

ui:OzonePlatform::InitializeFor* and aura::Env::CreateInstance calls
from unit tests are not updated in this CL and still use default
values. The connector parameters of ui:OzonePlatform::InitializeFor*
functions is still not set either.

BUG=620934, 620927

[fwang, 2016-09-09 15:40:24]

Description was changed from

==========
[WIP] Pass the single_process parameter to Ozone's initialization functions

Calls to the InitializeForUI and InitializeForGPU functions of Ozone
are updated to pass the single_process parameter. It is always false
for Mus and otherwise parsed from the command line.

The call to InitializeForGPU is done in the initialization of the GPU
thread, as suggested in https://codereview.chromium.org/2270463002.
This is also more convenient to determine the init parameters.

Similarly, the determination of whether Mus is used and of the value
of single_process is moved from ui/aura to content/browser, where
this task becomes easier. This information is transmitted to
aura::Env::CreateInstance via new boolean value parameters.

ui:OzonePlatform::InitializeFor* and aura::Env::CreateInstance calls
from unit tests are not updated in this CL and still use default
values. The connector parameters of ui:OzonePlatform::InitializeFor*
functions is still not set either.

BUG=620934, 620927
==========

to

==========
[WIP] Pass the single_process parameter to Ozone's initialization functions

Calls to the InitializeForUI and InitializeForGPU functions of Ozone
are updated to pass the single_process parameter. It is always false
for Mus and otherwise parsed from the command line.

The call to InitializeForGPU is done in the initialization of the GPU
thread, as suggested in https://codereview.chromium.org/2270463002.
This is also more convenient to determine the init parameters.

Similarly, the determination of whether Mus is used and of the value
of single_process is moved from ui/aura to content/browser, where
this task becomes easier. This information is transmitted to
aura::Env::CreateInstance via new boolean value parameters.

ui:OzonePlatform::InitializeFor* and aura::Env::CreateInstance calls
from unit tests are not updated in this CL and still use default
values. The connector parameters of ui:OzonePlatform::InitializeFor*
functions is still not set either.

BUG=620934, 620927
==========

[fwang, 2016-09-09 15:40:25]

fwang@igalia.com changed reviewers:
+ rjkroege@chromium.org

[fwang, 2016-09-09 15:41:08]

Hi Robert. I'm not sure this patch is really ready yet, but I'm adding you as a
reviewer in order to get some preliminary feedback.

Also, with the goal of having hardware acceleration in a GPU process for the
Ozone/Wayland platform, the comment from OzonePlatformGbm::InitializeUI seems to
suggest that the proper parameters should be passed to InitializeFor* and that
unfortunately refactoring is still in progress (I guess this is
https://bugs.chromium.org/p/chromium/issues/detail?id=620927). Can you please
provide more information on the status and plans for the mentioned refactoring?
Thanks!

[rjkroege, 2016-09-12 20:39:12]

https://codereview.chromium.org/2327073002/diff/1/content/gpu/gpu_child_threa...
File content/gpu/gpu_child_thread.cc (right):

https://codereview.chromium.org/2327073002/diff/1/content/gpu/gpu_child_threa...
content/gpu/gpu_child_thread.cc:185: #if defined(USE_OZONE)
you shouldn't need this here.

https://codereview.chromium.org/2327073002/diff/1/services/ui/gpu/gpu_service...
File services/ui/gpu/gpu_service_internal.cc (right):

https://codereview.chromium.org/2327073002/diff/1/services/ui/gpu/gpu_service...
services/ui/gpu/gpu_service_internal.cc:166: params.single_process = false;
This will break downstream code.  Please don't do this yet.

https://codereview.chromium.org/2327073002/diff/1/ui/aura/env.h
File ui/aura/env.h (right):

https://codereview.chromium.org/2327073002/diff/1/ui/aura/env.h#newcode38
ui/aura/env.h:38: static std::unique_ptr<Env> CreateInstance(bool
running_inside_mus = false,
Why are you plumbing this?

https://codereview.chromium.org/2327073002/diff/1/ui/gl/init/gl_factory_ozone.cc
File ui/gl/init/gl_factory_ozone.cc (left):

https://codereview.chromium.org/2327073002/diff/1/ui/gl/init/gl_factory_ozone...
ui/gl/init/gl_factory_ozone.cc:63: ui::OzonePlatform::InitializeForGPU();
So I don't want this here for a variety of reasons but it's probably not time to
move it yet.

[rjkroege, 2016-09-13 02:24:11]

On 2016/09/09 15:41:08, fwang wrote:

> Hi Robert. I'm not sure this patch is really ready yet, but I'm adding you as
a
> reviewer in order to get some preliminary feedback.

I'm not sure what the goal is here. Can you explain.

> 
> Also, with the goal of having hardware acceleration in a GPU process for the
> Ozone/Wayland platform, the comment from OzonePlatformGbm::InitializeUI seems
to
> suggest that the proper parameters should be passed to InitializeFor* and that
> unfortunately refactoring is still in progress (I guess this is
> https://bugs.chromium.org/p/chromium/issues/detail?id=620927). Can you please
> provide more information on the status and plans for the mentioned
refactoring?
> Thanks!

Status: not done yet. Plan: get it done. General idea: ozone should use mojo
internally. As part of this, I'l modify the plumbing that you've changed here to
deliver the right mojo connector to the gpu side of mojo. You will want the
wayland code to do the right thing internally for mus-gpu and mus-ws
communication and that will require a mojo pipe of some kind.

[fwang, 2016-09-13 13:48:11]

On 2016/09/13 02:24:11, rjkroege wrote:
> On 2016/09/09 15:41:08, fwang wrote:
> I'm not sure what the goal is here. Can you explain.
> Status: not done yet. Plan: get it done. General idea: ozone should use mojo
> internally. As part of this, I'l modify the plumbing that you've changed here
to
> deliver the right mojo connector to the gpu side of mojo. You will want the
> wayland code to do the right thing internally for mus-gpu and mus-ws
> communication and that will require a mojo pipe of some kind.

So in theory this commit is preparing for the long-term goal to move to
with-argument versions of init functions
(https://bugs.chromium.org/p/chromium/issues/detail?id=620934).

However, my short-term goal is really to have the Ozone/Wayland platform with
hardware acceleration in the GPU process and also having non-chromeos build. As
I said I'm not sure this commit is really ready nor necessary for now (but that
was a good exercise for me to study the init calls). As I understand:

1) From the comment in OzonePlatformGbm::InitializeUI, the only thing the
Ozone/Wayland code needs to communicate between UI/GPU is the mojo connector.
The is_single_process added here is not really important, actually.

2) From your comment https://codereview.chromium.org/2024953007/#msg15 it seems
that it'd be best to focus on Mus in order to support ChromeOS=0 mode. In that
case passing init params from ui/aura/ os probably not really useful either.

OzonePlatformGbm::InitializeUI mentions three modes. "single process" mode is
the only one currently supported by Ozone/Wayland (using the Wayland connection
to communication). The "mojo" mode is the one we want to have for Ozone/Wayland
but IIUC for now we can only have UI/GPU components in the same process. The
"legacy IPC mode" is probably not interesting for the long term, but we might
have to use if we want to test separate processes without waiting the mojo
refactoring.

[fwang, 2016-09-13 13:49:19]

https://codereview.chromium.org/2327073002/diff/1/content/gpu/gpu_child_threa...
File content/gpu/gpu_child_thread.cc (right):

https://codereview.chromium.org/2327073002/diff/1/content/gpu/gpu_child_threa...
content/gpu/gpu_child_thread.cc:185: #if defined(USE_OZONE)
On 2016/09/12 20:39:12, rjkroege wrote:
> you shouldn't need this here.

There is a comment in gpu_main.cc:

"With in-process GPU, gl::init::InitializeGLOneOff() is called from
GpuChildThread before getting here."

Indeed, in that case the InitializeGLOneOff call here happens before the one in
gpu_main.cc and hence will crash without the InitializeForGPU call added here.

https://codereview.chromium.org/2327073002/diff/1/services/ui/gpu/gpu_service...
File services/ui/gpu/gpu_service_internal.cc (right):

https://codereview.chromium.org/2327073002/diff/1/services/ui/gpu/gpu_service...
services/ui/gpu/gpu_service_internal.cc:166: params.single_process = false;
On 2016/09/12 20:39:12, rjkroege wrote:
> This will break downstream code.  Please don't do this yet.

I'm not sure I understand this... Can you elaborate on what you mean here? Maybe
that we should wait to plumb the connector before doing this? Or that
implementers of ozone_platform.h may not have the with-parameter version? As I
see the parameter-less InitializeForGPU already sets single_process = false and
connector = nullptr by default, so I'm not sure why it would be a problem.

https://codereview.chromium.org/2327073002/diff/1/ui/aura/env.h
File ui/aura/env.h (right):

https://codereview.chromium.org/2327073002/diff/1/ui/aura/env.h#newcode38
ui/aura/env.h:38: static std::unique_ptr<Env> CreateInstance(bool
running_inside_mus = false,
On 2016/09/12 20:39:12, rjkroege wrote:
> Why are you plumbing this?

This is not essential here, but currently ui/aura/env.cc seems to have a hacky
function RunningInsideMus() and it seems cleaner to determine this info on the
content side instead. Similarly, in_process_gpu and the mojo connector seem more
easily determined on the content side (although if I follow how the sequence is
initialized, the latter does not seem available yet when CreateInstance is
called).

[fwang, 2016-09-14 06:34:46]

On 2016/09/13 13:49:19, fwang wrote:
>  (although if I follow how the sequence is
> initialized, the latter does not seem available yet when CreateInstance is
> called).

Just to clarify what I meant here, I see the following calls:

BrowserMainRunnerImpl::Initialize
  BrowserMainLoop::InitializeToolkit
    aura::Env::CreateInstance
      aura::Env::Init
        ui::OzonePlatform::InitializeForUI
  BrowserMainLoop::CreateStartupTasks
    BrowserMainLoop::BrowserThreadsStarted
      BrowserMainLoop::InitializeMojo

which would mean that the mojo connector is not available yet when
InitializeForUI is called.

[rjkroege, 2016-09-21 13:40:52]

On 2016/09/14 06:34:46, fwang wrote:
> On 2016/09/13 13:49:19, fwang wrote:
> >  (although if I follow how the sequence is
> > initialized, the latter does not seem available yet when CreateInstance is
> > called).
> 
> Just to clarify what I meant here, I see the following calls:
> 
> BrowserMainRunnerImpl::Initialize
>   BrowserMainLoop::InitializeToolkit
>     aura::Env::CreateInstance
>       aura::Env::Init
>         ui::OzonePlatform::InitializeForUI
>   BrowserMainLoop::CreateStartupTasks
>     BrowserMainLoop::BrowserThreadsStarted
>       BrowserMainLoop::InitializeMojo
> 
> which would mean that the mojo connector is not available yet when
> InitializeForUI is called.

Mojo is not supported in standard Chrome for ozone yet.  See
https://cs.chromium.org/chromium/src/services/ui/service.cc?rcl=0&l=160

[fwang, 2016-09-21 14:25:52]

On 2016/09/21 13:40:52, rjkroege wrote:
> Mojo is not supported in standard Chrome for ozone yet.  See
> https://cs.chromium.org/chromium/src/services/ui/service.cc?rcl=0&l=160

Yes. With a small patch to gpu_process_transport_factory.cc I can force the
software rendering on ChromeOS and then chrome follows the non-mus path. With
more patches from Antonio, I'm also able to run the non-mus path on non-ChromeOS
builds.

Anyway, I agree that this is not really useful for now so I'll remove the review
request. Antonio moved the discussion on ozone-dev so I'll reply there.

[fwang, 2016-09-21 14:26:26]

fwang@igalia.com changed reviewers:
- rjkroege@chromium.org

[fwang, 2017-02-23 12:03:36]

I'm closing this CL.