Mus experiment in content shell on Android.

Making the Mus demo work on the Android content shell:
- adding a button behind a flag in the content shell on Android to
  start the Mus demo.
- bringing up the UI and other required services in the Android content
  shell.
- fixing the PlatformWindowAndroid so when created it takes over the
  activity it was configured with.
    
BUG=657027

[Jay Civelli, 2016-10-18 16:52:23]

Description was changed from

==========
Mus experiment in content shell on Android.

Adding a button behind a flag in the content shell on Android that
starts the Mus demo.

BUG=657027
==========

to

==========
Mus experiment in content shell on Android.

Adding a button behind a flag in the content shell on Android that
starts the Mus demo.

BUG=657027
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Jay Civelli, 2016-10-18 17:33:41]

Description was changed from

==========
Mus experiment in content shell on Android.

Adding a button behind a flag in the content shell on Android that
starts the Mus demo.

BUG=657027
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Mus experiment in content shell on Android.
    
Making the Mus demo work on the Android content shell:
- adding a button behind a flag in the content shell on Android to
  start the Mus demo.
- bringing up the UI and other required services in the Android content
  shell.
- splitting the Mus demo so it can be launched as a Mojo app or
  from an exe.
- replacing the use of gfx::AcceleratedWidget with gpu::SurfaceHandle
  in several services/ui methods so they can be used on Android.
- fixing the PlatformWindowAndroid so when created it takes over the
  activity it was configured with.
- fixing SurfaceTracker in component builds.
    
BUG=657027
==========

[Jay Civelli, 2016-10-18 17:33:57]

The CQ bit was checked by jcivelli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-18 17:34:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-18 17:42:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-18 17:42:33]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Jay Civelli, 2016-10-18 18:33:35]

Description was changed from

==========
Mus experiment in content shell on Android.
    
Making the Mus demo work on the Android content shell:
- adding a button behind a flag in the content shell on Android to
  start the Mus demo.
- bringing up the UI and other required services in the Android content
  shell.
- splitting the Mus demo so it can be launched as a Mojo app or
  from an exe.
- replacing the use of gfx::AcceleratedWidget with gpu::SurfaceHandle
  in several services/ui methods so they can be used on Android.
- fixing the PlatformWindowAndroid so when created it takes over the
  activity it was configured with.
- fixing SurfaceTracker in component builds.
    
BUG=657027
==========

to

==========
Mus experiment in content shell on Android.
    
Making the Mus demo work on the Android content shell:
- adding a button behind a flag in the content shell on Android to
  start the Mus demo.
- bringing up the UI and other required services in the Android content
  shell.
- splitting the Mus demo so it can be launched as a Mojo app or
  from an exe.
- replacing the use of gfx::AcceleratedWidget with gpu::SurfaceHandle
  in several services/ui methods so they can be used on Android.
- fixing the PlatformWindowAndroid so when created it takes over the
  activity it was configured with.
- fixing SurfaceTracker in component builds.
    
BUG=657027
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Jay Civelli, 2016-10-18 18:33:53]

The CQ bit was checked by jcivelli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-18 18:47:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-18 18:51:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-18 18:51:15]

Dry run: Try jobs failed on following builders:
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[Jay Civelli, 2016-10-18 21:15:23]

The CQ bit was checked by jcivelli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-18 21:16:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jay Civelli, 2016-10-18 21:18:40]

jcivelli@chromium.org changed reviewers:
+ erg@chromium.org, fsamuel@chromium.org, kbr@chromium.org,
rjkroege@chromium.org, rockot@chromium.org, sadrul@chromium.org,
sky@chromium.org, tedchoc@chromium.org

[Jay Civelli, 2016-10-18 21:18:41]

@tedchoc please review content/shell/android
@rockot please review services/service_manager and
content/common/service_manager/embedded_service_runner.cc and factory and
manifest changes
@kbr please review gpu/ipc/common/
@rjkroege please review services/ui/demo
@fsamuel please review services/ui/ws and services/ui/surfaces
@sky please review services/ui other than covered above
@sadrul please review ui/platform_window
@erg please review services/tracing

[commit-bot: I haz the power, 2016-10-18 21:20:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-18 21:20:48]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[sky, 2016-10-18 21:52:30]

https://codereview.chromium.org/2431753002/diff/60001/services/ui/ime/test_im...
File services/ui/ime/test_ime_driver/mojo_test_ime_application_factory.cc
(right):

https://codereview.chromium.org/2431753002/diff/60001/services/ui/ime/test_im...
services/ui/ime/test_ime_driver/mojo_test_ime_application_factory.cc:16: return
std::unique_ptr<service_manager::Service>(new TestIMEApplication());
Similar comment here about creating TestIMEApplication directly.

https://codereview.chromium.org/2431753002/diff/60001/services/ui/mojo_ui_app...
File services/ui/mojo_ui_application_factory.cc (right):

https://codereview.chromium.org/2431753002/diff/60001/services/ui/mojo_ui_app...
services/ui/mojo_ui_application_factory.cc:15: return
std::unique_ptr<service_manager::Service>(new Service());
Why do you need this function rather than having
std::unique_ptr<service_manager::Service>(new Service()); directly?

https://codereview.chromium.org/2431753002/diff/60001/services/ui/public/cpp/...
File services/ui/public/cpp/BUILD.gn (right):

https://codereview.chromium.org/2431753002/diff/60001/services/ui/public/cpp/...
services/ui/public/cpp/BUILD.gn:80: "//services/ui/ws/*",
Is this working around a bug in gn? Can't we list all the targets?

[Jay Civelli, 2016-10-18 22:28:03]

https://codereview.chromium.org/2431753002/diff/60001/services/ui/ime/test_im...
File services/ui/ime/test_ime_driver/mojo_test_ime_application_factory.cc
(right):

https://codereview.chromium.org/2431753002/diff/60001/services/ui/ime/test_im...
services/ui/ime/test_ime_driver/mojo_test_ime_application_factory.cc:16: return
std::unique_ptr<service_manager::Service>(new TestIMEApplication());
On 2016/10/18 21:52:30, sky wrote:
> Similar comment here about creating TestIMEApplication directly.

See my other comment.

https://codereview.chromium.org/2431753002/diff/60001/services/ui/mojo_ui_app...
File services/ui/mojo_ui_application_factory.cc (right):

https://codereview.chromium.org/2431753002/diff/60001/services/ui/mojo_ui_app...
services/ui/mojo_ui_application_factory.cc:15: return
std::unique_ptr<service_manager::Service>(new Service());
On 2016/10/18 21:52:30, sky wrote:
> Why do you need this function rather than having
> std::unique_ptr<service_manager::Service>(new Service()); directly?

This function is a factory function bound in shell_content_browser_client.cc so
that the service manager can create the service directly.

https://codereview.chromium.org/2431753002/diff/60001/services/ui/public/cpp/...
File services/ui/public/cpp/BUILD.gn (right):

https://codereview.chromium.org/2431753002/diff/60001/services/ui/public/cpp/...
services/ui/public/cpp/BUILD.gn:80: "//services/ui/ws/*",
On 2016/10/18 21:52:30, sky wrote:
> Is this working around a bug in gn? Can't we list all the targets?

We can... but there are a bunch and there are synthetic. Here is the list:

//services/ui/ws:mus_ws_unittests__isolate__write_deps
//services/ui/ws:_mus_ws_unittests__library
//services/ui/ws:mus_ws_unittests_apk__process_resources
//services/ui/ws:mus_ws_unittests_apk__java__compile_java__javac
//services/ui/ws:mus_ws_unittests_apk__java__compile_java__process_prebuilt__copy
//services/ui/ws:mus_ws_unittests_apk__create__package_resources
//services/ui/ws:mus_ws_unittests_apk__create__package
//services/ui/ws:mus_ws_unittests_apk__create_incremental_generate_manifest
//services/ui/ws:mus_ws_unittests_apk__create_incremental__package_resources
//services/ui/ws:mus_ws_unittests_apk__create_incremental__package

Sadly we cannot use mus_ws_unittests* in GN so it's either the full list or
//services/ui/ws/*

[sky, 2016-10-18 22:54:52]

https://codereview.chromium.org/2431753002/diff/60001/services/ui/public/cpp/...
File services/ui/public/cpp/BUILD.gn (right):

https://codereview.chromium.org/2431753002/diff/60001/services/ui/public/cpp/...
services/ui/public/cpp/BUILD.gn:80: "//services/ui/ws/*",
On 2016/10/18 22:28:03, Jay Civelli wrote:
> On 2016/10/18 21:52:30, sky wrote:
> > Is this working around a bug in gn? Can't we list all the targets?
> 
> We can... but there are a bunch and there are synthetic. Here is the list:
> 
> //services/ui/ws:mus_ws_unittests__isolate__write_deps
> //services/ui/ws:_mus_ws_unittests__library
> //services/ui/ws:mus_ws_unittests_apk__process_resources
> //services/ui/ws:mus_ws_unittests_apk__java__compile_java__javac
>
//services/ui/ws:mus_ws_unittests_apk__java__compile_java__process_prebuilt__copy
> //services/ui/ws:mus_ws_unittests_apk__create__package_resources
> //services/ui/ws:mus_ws_unittests_apk__create__package
> //services/ui/ws:mus_ws_unittests_apk__create_incremental_generate_manifest
> //services/ui/ws:mus_ws_unittests_apk__create_incremental__package_resources
> //services/ui/ws:mus_ws_unittests_apk__create_incremental__package
> 
> Sadly we cannot use mus_ws_unittests* in GN so it's either the full list or
> //services/ui/ws/*

Ugh! Ok, keep what you have.

[Ted C, 2016-10-18 23:39:40]

content/shell/android - lgtm

https://codereview.chromium.org/2431753002/diff/60001/content/shell/android/j...
File content/shell/android/java/res/layout/shell_view.xml (right):

https://codereview.chromium.org/2431753002/diff/60001/content/shell/android/j...
content/shell/android/java/res/layout/shell_view.xml:52:
android:src="@android:drawable/btn_star"
tiniest of nits, but I'd be worried about using a star as it might conflict w/
bookmarks.  Is there some completely odd ball icon we can use instead?

[Jay Civelli, 2016-10-19 00:41:49]

The CQ bit was checked by jcivelli@chromium.org to run a CQ dry run

[Jay Civelli, 2016-10-19 00:42:04]

https://codereview.chromium.org/2431753002/diff/60001/content/shell/android/j...
File content/shell/android/java/res/layout/shell_view.xml (right):

https://codereview.chromium.org/2431753002/diff/60001/content/shell/android/j...
content/shell/android/java/res/layout/shell_view.xml:52:
android:src="@android:drawable/btn_star"
On 2016/10/18 23:39:40, Ted C wrote:
> tiniest of nits, but I'd be worried about using a star as it might conflict w/
> bookmarks.  Is there some completely odd ball icon we can use instead?

Good point. Switched to using the gallery one. Let me know if you have a better
suggestion.

https://codereview.chromium.org/2431753002/diff/60001/services/ui/mojo_ui_app...
File services/ui/mojo_ui_application_factory.cc (right):

https://codereview.chromium.org/2431753002/diff/60001/services/ui/mojo_ui_app...
services/ui/mojo_ui_application_factory.cc:15: return
std::unique_ptr<service_manager::Service>(new Service());
On 2016/10/18 22:28:02, Jay Civelli wrote:
> On 2016/10/18 21:52:30, sky wrote:
> > Why do you need this function rather than having
> > std::unique_ptr<service_manager::Service>(new Service()); directly?
> 
> This function is a factory function bound in shell_content_browser_client.cc
so
> that the service manager can create the service directly.

Per our discussion, moved them to shell_content_browser_client.cc

[commit-bot: I haz the power, 2016-10-19 01:33:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-19 01:56:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-19 01:56:32]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[sky, 2016-10-19 03:10:38]

Thanks files you wanted me to look at LGTM

[Fady Samuel, 2016-10-19 10:49:52]

lgtm + bikeshedding nits (feel free to ignore or handle in a separate CL). In a
nutshell we are mixing "AcceleratedWidget" terminology with "SurfaceHandle"
terminology and I'm wondering if we should try to unify that?

https://codereview.chromium.org/2431753002/diff/80001/services/ui/ws/frame_ge...
File services/ui/ws/frame_generator.cc (right):

https://codereview.chromium.org/2431753002/diff/80001/services/ui/ws/frame_ge...
services/ui/ws/frame_generator.cc:61: void
FrameGenerator::OnAcceleratedWidgetAvailable(gpu::SurfaceHandle widget) {
I'm wondering if we should replace references to "AcceleratedWidget" to
"SurfaceHandle" and |widget_| to |surface_handle_| just to keep terminology
consistent in Mus?

https://codereview.chromium.org/2431753002/diff/80001/ui/platform_window/plat...
File ui/platform_window/platform_window_delegate.h (left):

https://codereview.chromium.org/2431753002/diff/80001/ui/platform_window/plat...
ui/platform_window/platform_window_delegate.h:46: virtual void
OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget,
nit: OnSurfaceHandleAvailable?

[sadrul, 2016-10-19 13:53:43]

This CL does way too many things. Please split up into smaller CLs. I would
suggest:
 . Change bitmap_uploader to use async API + mus-demo to be executable + mojo
app.
 . Change to service_manager.
 . Change to tracing manifest (what does it do?)
 . Change from gfx::AcceleratedWidget to gpu::SurfaceHandle. We had
PlatformWindowAndroid working with AcceleratedWidget before (for the old
view_manager, and for clank-aura). Can you explain why that doesn't work
anymore, and why we need to switch to SurfaceHandle?

https://codereview.chromium.org/2431753002/diff/80001/content/public/app/BUIL...
File content/public/app/BUILD.gn (right):

https://codereview.chromium.org/2431753002/diff/80001/content/public/app/BUIL...
content/public/app/BUILD.gn:184: "//services/ui/ime/test_ime_driver:manifest",
I don't know if this is the right place for it. I would've guessed you would add
this to content_shell's manifest?

https://codereview.chromium.org/2431753002/diff/80001/services/tracing/manife...
File services/tracing/manifest.json (right):

https://codereview.chromium.org/2431753002/diff/80001/services/tracing/manife...
services/tracing/manifest.json:4: "process-group": "browser",
Explain what this does.

[Jay Civelli, 2016-10-19 16:18:25]

On 2016/10/19 13:53:43, sadrul wrote:
> This CL does way too many things. Please split up into smaller CLs. I would
> suggest:
>  . Change bitmap_uploader to use async API + mus-demo to be executable + mojo
> app.
>  . Change to service_manager.
>  . Change to tracing manifest (what does it do?)
>  . Change from gfx::AcceleratedWidget to gpu::SurfaceHandle. We had
> PlatformWindowAndroid working with AcceleratedWidget before (for the old
> view_manager, and for clank-aura). Can you explain why that doesn't work
> anymore, and why we need to switch to SurfaceHandle?

Fair enough. I'll split it up and keep this CL as the final one.

> 
>
https://codereview.chromium.org/2431753002/diff/80001/content/public/app/BUIL...
> File content/public/app/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2431753002/diff/80001/content/public/app/BUIL...
> content/public/app/BUILD.gn:184: "//services/ui/ime/test_ime_driver:manifest",
> I don't know if this is the right place for it. I would've guessed you would
add
> this to content_shell's manifest?
> 
>
https://codereview.chromium.org/2431753002/diff/80001/services/tracing/manife...
> File services/tracing/manifest.json (right):
> 
>
https://codereview.chromium.org/2431753002/diff/80001/services/tracing/manife...
> services/tracing/manifest.json:4: "process-group": "browser",
> Explain what this does.

[Jay Civelli, 2016-10-19 16:19:40]

For reviewers that have not yet review this CL, hold on, I'll split it up in
multiple CLs.
Thanks.

[Jay Civelli, 2016-10-19 19:44:07]

>  . Change from gfx::AcceleratedWidget to gpu::SurfaceHandle. We had
> PlatformWindowAndroid working with AcceleratedWidget before (for the old
> view_manager, and for clank-aura). Can you explain why that doesn't work
> anymore, and why we need to switch to SurfaceHandle?
> 
gfx::AcceleratedWidget and gpu::SurfaceHandle seem to be used interchangeably in
some places (ex: SurfacesContextProvider::SurfacesContextProvider() where we get
an AcceleratedWidget that we then provide as a SurfaceHandle to
gpu::CommandBufferProxyImpl::Create()). I don't think that Mus code was compiled
before on Android, so this probably wasn't a problem.
When I chatted with @fsamuel about this, he recommended switching to
SurfaceHandle (which seems like a better abstraction).

[sadrul, 2016-10-19 20:34:17]

On 2016/10/19 19:44:07, Jay Civelli wrote:
> >  . Change from gfx::AcceleratedWidget to gpu::SurfaceHandle. We had
> > PlatformWindowAndroid working with AcceleratedWidget before (for the old
> > view_manager, and for clank-aura). Can you explain why that doesn't work
> > anymore, and why we need to switch to SurfaceHandle?
> > 
> gfx::AcceleratedWidget and gpu::SurfaceHandle seem to be used interchangeably
in
> some places (ex: SurfacesContextProvider::SurfacesContextProvider() where we
get
> an AcceleratedWidget that we then provide as a SurfaceHandle to
> gpu::CommandBufferProxyImpl::Create()).

gfx::AcceleratedWidget and gpu::SurfaceHandle happen to be the same on some
platforms. We use gpu::SurfaceHandle when sending a handle over IPC to gpu
(https://cs.chromium.org/chromium/src/gpu/ipc/common/surface_handle.h?sq=packa...).
For process-local use, we should still use AcceleratedWidget. In this case,
(ui/platform_window/ + services/ui/ws/) usage is local, and using
gfx::AcceleratedWidget should work without issues. (an earlier version of mus,
then called 'view_manager', used this, and it did work on android).

> I don't think that Mus code was compiled
> before on Android, so this probably wasn't a problem.
> When I chatted with @fsamuel about this, he recommended switching to
> SurfaceHandle (which seems like a better abstraction).

[rjkroege, 2016-10-20 23:29:05]

https://codereview.chromium.org/2431753002/diff/80001/services/ui/demo/BUILD.gn
File services/ui/demo/BUILD.gn (right):

https://codereview.chromium.org/2431753002/diff/80001/services/ui/demo/BUILD....
services/ui/demo/BUILD.gn:11: "main.cc",
don't you need to rebase this based on the other CL?

https://codereview.chromium.org/2431753002/diff/80001/services/ui/demo/DEPS
File services/ui/demo/DEPS (right):

https://codereview.chromium.org/2431753002/diff/80001/services/ui/demo/DEPS#n...
services/ui/demo/DEPS:2: "+content/shell/browser",
I'd rather you not do this? mus demo is intended to not need Chrome. Is there
another way?

https://codereview.chromium.org/2431753002/diff/80001/services/ui/manifest.json
File services/ui/manifest.json (right):

https://codereview.chromium.org/2431753002/diff/80001/services/ui/manifest.js...
services/ui/manifest.json:4: "process-group": "browser",
what does this mean?

[Jay Civelli, 2016-11-02 21:58:27]

Description was changed from

==========
Mus experiment in content shell on Android.
    
Making the Mus demo work on the Android content shell:
- adding a button behind a flag in the content shell on Android to
  start the Mus demo.
- bringing up the UI and other required services in the Android content
  shell.
- splitting the Mus demo so it can be launched as a Mojo app or
  from an exe.
- replacing the use of gfx::AcceleratedWidget with gpu::SurfaceHandle
  in several services/ui methods so they can be used on Android.
- fixing the PlatformWindowAndroid so when created it takes over the
  activity it was configured with.
- fixing SurfaceTracker in component builds.
    
BUG=657027
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Mus experiment in content shell on Android.
    
Making the Mus demo work on the Android content shell:
- adding a button behind a flag in the content shell on Android to
  start the Mus demo.
- bringing up the UI and other required services in the Android content
  shell.
- fixing the PlatformWindowAndroid so when created it takes over the
  activity it was configured with.
- fixing SurfaceTracker in component builds.
    
BUG=657027
==========

[Jay Civelli, 2016-11-02 21:58:51]

Description was changed from

==========
Mus experiment in content shell on Android.
    
Making the Mus demo work on the Android content shell:
- adding a button behind a flag in the content shell on Android to
  start the Mus demo.
- bringing up the UI and other required services in the Android content
  shell.
- fixing the PlatformWindowAndroid so when created it takes over the
  activity it was configured with.
- fixing SurfaceTracker in component builds.
    
BUG=657027
==========

to

==========
Mus experiment in content shell on Android.
    
Making the Mus demo work on the Android content shell:
- adding a button behind a flag in the content shell on Android to
  start the Mus demo.
- bringing up the UI and other required services in the Android content
  shell.
- fixing the PlatformWindowAndroid so when created it takes over the
  activity it was configured with.
    
BUG=657027
==========

[Jay Civelli, 2016-11-02 21:59:02]

Description was changed from

==========
Mus experiment in content shell on Android.
    
Making the Mus demo work on the Android content shell:
- adding a button behind a flag in the content shell on Android to
  start the Mus demo.
- bringing up the UI and other required services in the Android content
  shell.
- fixing the PlatformWindowAndroid so when created it takes over the
  activity it was configured with.
    
BUG=657027
==========

to

==========
Making the Mus demo work on the Android content shell:
- adding a button behind a flag in the content shell on Android to
  start the Mus demo.
- bringing up the UI and other required services in the Android content
  shell.
- fixing the PlatformWindowAndroid so when created it takes over the
  activity it was configured with.
    
BUG=657027
==========

[Jay Civelli, 2016-11-02 22:02:18]

Few other CLs have landed so this change is now more specific to only bringing
up the MUS demo in the Android content shell.

Ready for review again.

Thanks.

https://codereview.chromium.org/2431753002/diff/80001/content/public/app/BUIL...
File content/public/app/BUILD.gn (right):

https://codereview.chromium.org/2431753002/diff/80001/content/public/app/BUIL...
content/public/app/BUILD.gn:184: "//services/ui/ime/test_ime_driver:manifest",
On 2016/10/19 13:53:43, sadrul wrote:
> I don't know if this is the right place for it. I would've guessed you would
add
> this to content_shell's manifest?

I believe packaged services have to be listed here (so the manifests can be
merged).
I simplified things a bit so now it only depends on the actual demo.

https://codereview.chromium.org/2431753002/diff/80001/services/tracing/manife...
File services/tracing/manifest.json (right):

https://codereview.chromium.org/2431753002/diff/80001/services/tracing/manife...
services/tracing/manifest.json:4: "process-group": "browser",
On 2016/10/19 13:53:43, sadrul wrote:
> Explain what this does.

This was needed for packaging to work (packaged service had to have the same
process group as their containing service or they would not be started
properly).
I removed the process group altogether in a different CL after chatting with Ken
and Ben, so this is not a problem anymore.

https://codereview.chromium.org/2431753002/diff/80001/services/ui/demo/BUILD.gn
File services/ui/demo/BUILD.gn (right):

https://codereview.chromium.org/2431753002/diff/80001/services/ui/demo/BUILD....
services/ui/demo/BUILD.gn:11: "main.cc",
On 2016/10/20 23:29:05, rjkroege wrote:
> don't you need to rebase this based on the other CL?

Turns out the split was not needed after all.

https://codereview.chromium.org/2431753002/diff/80001/services/ui/demo/DEPS
File services/ui/demo/DEPS (right):

https://codereview.chromium.org/2431753002/diff/80001/services/ui/demo/DEPS#n...
services/ui/demo/DEPS:2: "+content/shell/browser",
On 2016/10/20 23:29:05, rjkroege wrote:
> I'd rather you not do this? mus demo is intended to not need Chrome. Is there
> another way?

Not needed, removed.

https://codereview.chromium.org/2431753002/diff/80001/services/ui/manifest.json
File services/ui/manifest.json (right):

https://codereview.chromium.org/2431753002/diff/80001/services/ui/manifest.js...
services/ui/manifest.json:4: "process-group": "browser",
On 2016/10/20 23:29:05, rjkroege wrote:
> what does this mean?

Process-group is no more (see my comment in 	services/tracing/manifest.json)

https://codereview.chromium.org/2431753002/diff/80001/services/ui/ws/frame_ge...
File services/ui/ws/frame_generator.cc (right):

https://codereview.chromium.org/2431753002/diff/80001/services/ui/ws/frame_ge...
services/ui/ws/frame_generator.cc:61: void
FrameGenerator::OnAcceleratedWidgetAvailable(gpu::SurfaceHandle widget) {
On 2016/10/19 10:49:52, Fady Samuel wrote:
> I'm wondering if we should replace references to "AcceleratedWidget" to
> "SurfaceHandle" and |widget_| to |surface_handle_| just to keep terminology
> consistent in Mus?

Based on @sadrul comment I kept the AcceleratedWidget.

https://codereview.chromium.org/2431753002/diff/80001/ui/platform_window/plat...
File ui/platform_window/platform_window_delegate.h (left):

https://codereview.chromium.org/2431753002/diff/80001/ui/platform_window/plat...
ui/platform_window/platform_window_delegate.h:46: virtual void
OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget,
On 2016/10/19 10:49:52, Fady Samuel wrote:
> nit: OnSurfaceHandleAvailable? 

Ended up keeping the AcceleratedWidget.

[Jay Civelli, 2016-11-02 22:02:40]

The CQ bit was checked by jcivelli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-02 22:03:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-02 22:08:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-02 22:08:13]

Dry run: Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Jay Civelli, 2016-11-02 23:15:09]

The CQ bit was checked by jcivelli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-02 23:15:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-02 23:21:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-02 23:21:02]

Dry run: Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[sadrul, 2016-11-04 15:11:29]

Thanks! lgtm

https://codereview.chromium.org/2431753002/diff/180001/ui/platform_window/and...
File ui/platform_window/android/platform_window_android.h (right):

https://codereview.chromium.org/2431753002/diff/180001/ui/platform_window/and...
ui/platform_window/android/platform_window_android.h:10: #include
"base/memory/weak_ptr.h"
This can probably be removed too?

[Jay Civelli, 2016-11-04 17:46:16]

The CQ bit was checked by jcivelli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-04 17:46:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-04 18:21:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-04 18:21:01]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[Jay Civelli, 2016-11-04 20:15:31]

The CQ bit was checked by jcivelli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-04 20:16:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-04 20:50:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-04 20:50:04]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Jay Civelli, 2016-11-04 22:32:58]

The CQ bit was checked by jcivelli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-04 22:33:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-04 22:55:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-04 22:55:41]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[rjkroege, 2016-11-05 00:33:13]

lgtm