Provide callback to create mojo AndroidOverlays to AVDA.

Provide callback to create mojo AndroidOverlays to AVDA.

In order to create an overlay, AVDA must be able to talk to the
AndroidOverlayProviderImpl mojo service in the browser.  However,
media/ doesn't have access to much that would let it look up the
service by itself.

This CL plumbs a callback for creating these overlays to AVDA from
GpuChildThread.  GpuChildThread has access to the mojo Connector
object, which allows it to look up services in the browser process.

The callback takes as input the routing token and a configuration
struct, and returns the overlay.

BUG=721201
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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

Review-Url: https://codereview.chromium.org/2864603002
Cr-Commit-Position: refs/heads/master@{#471589}
Committed: https://chromium.googlesource.com/chromium/src/+/441ca70d0885c3cefafce97b8f72922ee05111e7

[liberato (no reviews please), 2017-05-05 05:23:22]

Description was changed from

==========
started creating overlays


made gpu_child_thread set the overlay factory.

rather than sending a connector to gpu_service, gpu_child_thread
sets the factory on the media gpu channel manager.  this means
that services/.../gpu_main creates a gpu_service that doesn't
have a factory on the channel manager.

however, for chrome, i think this is okay.

almost compiles.

the service version of gpu_main.cc doesn't provide a connector.
just sends in nullptr.

nothing sends in the browser service name.

media_gpu_channel doesn't actually do anything with the factory.

media_gpu_channel_manager


media_gpu_channel

BUG=
==========

to

==========
started creating overlays


made gpu_child_thread set the overlay factory.

rather than sending a connector to gpu_service, gpu_child_thread
sets the factory on the media gpu channel manager.  this means
that services/.../gpu_main creates a gpu_service that doesn't
have a factory on the channel manager.

however, for chrome, i think this is okay.

almost compiles.

the service version of gpu_main.cc doesn't provide a connector.
just sends in nullptr.

nothing sends in the browser service name.

media_gpu_channel doesn't actually do anything with the factory.

media_gpu_channel_manager


media_gpu_channel

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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
==========

[liberato (no reviews please), 2017-05-10 20:08:16]

Description was changed from

==========
started creating overlays


made gpu_child_thread set the overlay factory.

rather than sending a connector to gpu_service, gpu_child_thread
sets the factory on the media gpu channel manager.  this means
that services/.../gpu_main creates a gpu_service that doesn't
have a factory on the channel manager.

however, for chrome, i think this is okay.

almost compiles.

the service version of gpu_main.cc doesn't provide a connector.
just sends in nullptr.

nothing sends in the browser service name.

media_gpu_channel doesn't actually do anything with the factory.

media_gpu_channel_manager


media_gpu_channel

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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

==========
started creating overlays


made gpu_child_thread set the overlay factory.

rather than sending a connector to gpu_service, gpu_child_thread
sets the factory on the media gpu channel manager.  this means
that services/.../gpu_main creates a gpu_service that doesn't
have a factory on the channel manager.

however, for chrome, i think this is okay.

almost compiles.

the service version of gpu_main.cc doesn't provide a connector.
just sends in nullptr.

nothing sends in the browser service name.

media_gpu_channel doesn't actually do anything with the factory.

media_gpu_channel_manager


media_gpu_channel

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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
==========

[liberato (no reviews please), 2017-05-10 20:08:16]

liberato@chromium.org changed reviewers:
+ tguilbert@chromium.org, watk@chromium.org

[liberato (no reviews please), 2017-05-10 20:11:51]

This needs some cleanup, but I'm interested in what you think of this approach.
The goal is to get a mojo factory into AVDA. It's separate from the routing
token from WMPI in the other CL. This CL provides a call back that creates a
mojo overlay given the routing token from the other CL.

Doesn't need full review, just sanity check. I'll clean it up if you folks like
the general approach.

Thanks
-Do

[tguilbert, 2017-05-10 23:52:52]

I have no objection, but I also have the least amount of experience with AVDA. I
will deffer judgement to watk@

[watk, 2017-05-11 00:20:45]

Not quite sure what the goal of this CL is, sorry. Can't follow the description.

[watk, 2017-05-11 00:21:06]

On 2017/05/11 00:20:45, watk wrote:
> Not quite sure what the goal of this CL is, sorry. Can't follow the
description.

Seems fine to me, but it seemed fine before too.

[liberato (no reviews please), 2017-05-11 04:45:07]

Description was changed from

==========
started creating overlays


made gpu_child_thread set the overlay factory.

rather than sending a connector to gpu_service, gpu_child_thread
sets the factory on the media gpu channel manager.  this means
that services/.../gpu_main creates a gpu_service that doesn't
have a factory on the channel manager.

however, for chrome, i think this is okay.

almost compiles.

the service version of gpu_main.cc doesn't provide a connector.
just sends in nullptr.

nothing sends in the browser service name.

media_gpu_channel doesn't actually do anything with the factory.

media_gpu_channel_manager


media_gpu_channel

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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

==========
Provide factory for mojo AndroidOverlays to AVDA.

In order to create an overlay, AVDA must be able to talk to the
AndroidOverlayProvideImpl mojo service.  However, media/ doesn't
have access to much that would let it look up the service.

This CL plumbs a callback for creating these factories to AVDA
from GpuChildThread.  GpuChildThread has access to the mojo
Connector object, which allows it to look up services in the browser
process.  The callback takes as input the routing token and a
configuration struct, and returns the overlay.

BUG=721201
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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
==========

[liberato (no reviews please), 2017-05-11 04:45:39]

Description was changed from

==========
Provide factory for mojo AndroidOverlays to AVDA.

In order to create an overlay, AVDA must be able to talk to the
AndroidOverlayProvideImpl mojo service.  However, media/ doesn't
have access to much that would let it look up the service.

This CL plumbs a callback for creating these factories to AVDA
from GpuChildThread.  GpuChildThread has access to the mojo
Connector object, which allows it to look up services in the browser
process.  The callback takes as input the routing token and a
configuration struct, and returns the overlay.

BUG=721201
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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

==========
Provide factory for mojo AndroidOverlays to AVDA.

In order to create an overlay, AVDA must be able to talk to the
AndroidOverlayProvideImpl mojo service.  However, media/ doesn't
have access to much that would let it look up the service.

This CL plumbs a callback for creating these factories to AVDA
from GpuChildThread.  GpuChildThread has access to the mojo
Connector object, which allows it to look up services in the browser
process.  The callback takes as input the routing token and a
configuration struct, and returns the overlay.

BUG=721201
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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
==========

[liberato (no reviews please), 2017-05-11 05:15:43]

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

[commit-bot: I haz the power, 2017-05-11 05:17:06]

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, 2017-05-11 05:22:51]

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

[commit-bot: I haz the power, 2017-05-11 05:22:52]

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

[liberato (no reviews please), 2017-05-11 16:58:21]

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

[commit-bot: I haz the power, 2017-05-11 16:59:05]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[liberato (no reviews please), 2017-05-11 17:03:11]

Description was changed from

==========
Provide factory for mojo AndroidOverlays to AVDA.

In order to create an overlay, AVDA must be able to talk to the
AndroidOverlayProvideImpl mojo service.  However, media/ doesn't
have access to much that would let it look up the service.

This CL plumbs a callback for creating these factories to AVDA
from GpuChildThread.  GpuChildThread has access to the mojo
Connector object, which allows it to look up services in the browser
process.  The callback takes as input the routing token and a
configuration struct, and returns the overlay.

BUG=721201
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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

==========
Provide callback to crate mojo AndroidOverlays to AVDA.

In order to create an overlay, AVDA must be able to talk to the
AndroidOverlayProvideImpl mojo service in the browser.  However,
media/ doesn't have access to much that would let it look up the
service by itself.

This CL plumbs a callback for creating these overlays to AVDA from
GpuChildThread.  GpuChildThread has access to the mojo Connector
object, which allows it to look up services in the browser process.

The callback takes as input the routing token and a configuration
struct, and returns the overlay.

BUG=721201
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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
==========

[liberato (no reviews please), 2017-05-11 17:05:04]

Description was changed from

==========
Provide callback to crate mojo AndroidOverlays to AVDA.

In order to create an overlay, AVDA must be able to talk to the
AndroidOverlayProvideImpl mojo service in the browser.  However,
media/ doesn't have access to much that would let it look up the
service by itself.

This CL plumbs a callback for creating these overlays to AVDA from
GpuChildThread.  GpuChildThread has access to the mojo Connector
object, which allows it to look up services in the browser process.

The callback takes as input the routing token and a configuration
struct, and returns the overlay.

BUG=721201
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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

==========
Provide callback to crate mojo AndroidOverlays to AVDA.

In order to create an overlay, AVDA must be able to talk to the
AndroidOverlayProvideImpl mojo service in the browser.  However,
media/ doesn't have access to much that would let it look up the
service by itself.

This CL plumbs a callback for creating these overlays to AVDA from
GpuChildThread.  GpuChildThread has access to the mojo Connector
object, which allows it to look up services in the browser process.

The callback takes as input the routing token and a configuration
struct, and returns the overlay.

BUG=721201
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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
==========

[liberato (no reviews please), 2017-05-11 17:05:04]

liberato@chromium.org changed reviewers:
+ jbauman@chromium.org

[liberato (no reviews please), 2017-05-11 17:08:51]

hi all

jbauman: PTAL @ content/gpu
watk, tguilbert: PTAL @ media/

incidentally, this sends a callback rather than the Connector itself, since
it'll make unit testing quite a bit easier in AVDA.  one might argue that we
should plumb the connector further down, and convert to a callback in (e.g.)
gpu_video_deocde_accelerator.cc .  opinions welcome, as always.

thanks
-fl

[commit-bot: I haz the power, 2017-05-11 17:28:44]

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

[commit-bot: I haz the power, 2017-05-11 17:28:45]

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

[liberato (no reviews please), 2017-05-11 17:51:22]

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

[commit-bot: I haz the power, 2017-05-11 17:52:03]

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, 2017-05-11 19:07:52]

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

[commit-bot: I haz the power, 2017-05-11 19:07:53]

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

[liberato (no reviews please), 2017-05-11 19:51:09]

Description was changed from

==========
Provide callback to crate mojo AndroidOverlays to AVDA.

In order to create an overlay, AVDA must be able to talk to the
AndroidOverlayProvideImpl mojo service in the browser.  However,
media/ doesn't have access to much that would let it look up the
service by itself.

This CL plumbs a callback for creating these overlays to AVDA from
GpuChildThread.  GpuChildThread has access to the mojo Connector
object, which allows it to look up services in the browser process.

The callback takes as input the routing token and a configuration
struct, and returns the overlay.

BUG=721201
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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

==========
Provide callback to create mojo AndroidOverlays to AVDA.

In order to create an overlay, AVDA must be able to talk to the
AndroidOverlayProvideImpl mojo service in the browser.  However,
media/ doesn't have access to much that would let it look up the
service by itself.

This CL plumbs a callback for creating these overlays to AVDA from
GpuChildThread.  GpuChildThread has access to the mojo Connector
object, which allows it to look up services in the browser process.

The callback takes as input the routing token and a configuration
struct, and returns the overlay.

BUG=721201
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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
==========

[liberato (no reviews please), 2017-05-11 20:38:56]

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

[commit-bot: I haz the power, 2017-05-11 20:41:06]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tguilbert, 2017-05-11 21:30:51]

media/ LGTM.

NIT: In the description: s/AndroidOverlayProvideImpl/AndroidOverlayProviderImpl

[liberato (no reviews please), 2017-05-11 21:35:30]

Description was changed from

==========
Provide callback to create mojo AndroidOverlays to AVDA.

In order to create an overlay, AVDA must be able to talk to the
AndroidOverlayProvideImpl mojo service in the browser.  However,
media/ doesn't have access to much that would let it look up the
service by itself.

This CL plumbs a callback for creating these overlays to AVDA from
GpuChildThread.  GpuChildThread has access to the mojo Connector
object, which allows it to look up services in the browser process.

The callback takes as input the routing token and a configuration
struct, and returns the overlay.

BUG=721201
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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

==========
Provide callback to create mojo AndroidOverlays to AVDA.

In order to create an overlay, AVDA must be able to talk to the
AndroidOverlayProviderImpl mojo service in the browser.  However,
media/ doesn't have access to much that would let it look up the
service by itself.

This CL plumbs a callback for creating these overlays to AVDA from
GpuChildThread.  GpuChildThread has access to the mojo Connector
object, which allows it to look up services in the browser process.

The callback takes as input the routing token and a configuration
struct, and returns the overlay.

BUG=721201
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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
==========

[watk, 2017-05-11 22:27:01]

Seems like binding the interface in GpuVideoDecodeAccelerator would certainly
reduce the plumbing.

MCVD will want to call overlay_factory_cb on the MCVD thread. So I guess we'll
need to bind a new kind of factory cb for MCVD?

(service_manager::Connector has a Clone() method to create a clone that can be
used on another thread.)

https://codereview.chromium.org/2864603002/diff/120001/content/gpu/gpu_child_...
File content/gpu/gpu_child_thread.h (right):

https://codereview.chromium.org/2864603002/diff/120001/content/gpu/gpu_child_...
content/gpu/gpu_child_thread.h:120: std::unique_ptr<media::AndroidOverlay>
OverlayFactory(
CreateAndroidOverlay()?

Its name makes me think it will return a factory.

[commit-bot: I haz the power, 2017-05-11 23:56:31]

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

[commit-bot: I haz the power, 2017-05-11 23:56:32]

Dry run: This issue passed the CQ dry run.

[liberato (no reviews please), 2017-05-12 16:47:17]

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

[commit-bot: I haz the power, 2017-05-12 16:47:45]

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, 2017-05-12 16:52:20]

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

[commit-bot: I haz the power, 2017-05-12 16:52:22]

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_...)

[liberato (no reviews please), 2017-05-12 18:11:59]

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

[commit-bot: I haz the power, 2017-05-12 18:12:54]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[liberato (no reviews please), 2017-05-12 18:14:28]

On 2017/05/11 22:27:01, watk wrote:
> Seems like binding the interface in GpuVideoDecodeAccelerator would certainly
> reduce the plumbing.
> 
> MCVD will want to call overlay_factory_cb on the MCVD thread. So I guess we'll
> need to bind a new kind of factory cb for MCVD?
> 
> (service_manager::Connector has a Clone() method to create a clone that can be
> used on another thread.)
> 
>
https://codereview.chromium.org/2864603002/diff/120001/content/gpu/gpu_child_...
> File content/gpu/gpu_child_thread.h (right):
> 
>
https://codereview.chromium.org/2864603002/diff/120001/content/gpu/gpu_child_...
> content/gpu/gpu_child_thread.h:120: std::unique_ptr<media::AndroidOverlay>
> OverlayFactory(
> CreateAndroidOverlay()?
> 
> Its name makes me think it will return a factory.

GpuVideoDecodeAcceleratorFactory now takes the connector, and converts it into a
factory CB for
AVDA.  that way, we can still inject mock factory callbacks.

it also takes a const char* for the browser service name, since that's also
defined in
content (kBrowserServiceName).

[liberato (no reviews please), 2017-05-12 18:14:44]

https://codereview.chromium.org/2864603002/diff/120001/content/gpu/gpu_child_...
File content/gpu/gpu_child_thread.h (right):

https://codereview.chromium.org/2864603002/diff/120001/content/gpu/gpu_child_...
content/gpu/gpu_child_thread.h:120: std::unique_ptr<media::AndroidOverlay>
OverlayFactory(
On 2017/05/11 22:27:00, watk wrote:
> CreateAndroidOverlay()?
> 
> Its name makes me think it will return a factory.

Done, now in GpuVideoDecodeAcceleratorFactory.

[commit-bot: I haz the power, 2017-05-12 18:17:58]

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

[commit-bot: I haz the power, 2017-05-12 18:17:59]

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_...)

[liberato (no reviews please), 2017-05-12 18:25:51]

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

[commit-bot: I haz the power, 2017-05-12 18:27:39]

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, 2017-05-12 18:53:10]

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

[commit-bot: I haz the power, 2017-05-12 18:53:14]

Dry run: Try jobs failed on following builders:
  linux_optional_gpu_tests_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)

[watk, 2017-05-12 19:13:54]

Oh I was thinking you could call GetConnector()  directly from GVDA to reduce
the plumbing. But I guess that doesn't make sense, sorry.

lgtm either way.

https://codereview.chromium.org/2864603002/diff/200001/content/gpu/gpu_child_...
File content/gpu/gpu_child_thread.h (right):

https://codereview.chromium.org/2864603002/diff/200001/content/gpu/gpu_child_...
content/gpu/gpu_child_thread.h:32: #include
"media/base/android_overlay_mojo_factory.h"
can be deleted?

[liberato (no reviews please), 2017-05-12 19:33:51]

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

[commit-bot: I haz the power, 2017-05-12 19:34:48]

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, 2017-05-12 20:12:28]

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

[commit-bot: I haz the power, 2017-05-12 20:12:29]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jbauman, 2017-05-12 20:57:39]

lgtm with one nit.

https://codereview.chromium.org/2864603002/diff/220001/content/gpu/gpu_child_...
File content/gpu/gpu_child_thread.cc (right):

https://codereview.chromium.org/2864603002/diff/220001/content/gpu/gpu_child_...
content/gpu/gpu_child_thread.cc:315: const base::UnguessableToken&
routing_token,
Could you make this static (so we don't need base::Unretained above)?

[liberato (no reviews please), 2017-05-12 20:57:45]

https://codereview.chromium.org/2864603002/diff/200001/content/gpu/gpu_child_...
File content/gpu/gpu_child_thread.h (right):

https://codereview.chromium.org/2864603002/diff/200001/content/gpu/gpu_child_...
content/gpu/gpu_child_thread.h:32: #include
"media/base/android_overlay_mojo_factory.h"
On 2017/05/12 19:13:54, watk wrote:
> can be deleted?

Reverted.

[liberato (no reviews please), 2017-05-12 21:08:01]

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

[commit-bot: I haz the power, 2017-05-12 21:09:22]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[liberato (no reviews please), 2017-05-12 21:09:25]

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

[liberato (no reviews please), 2017-05-12 21:10:08]

The CQ bit was unchecked by liberato@chromium.org

[commit-bot: I haz the power, 2017-05-12 21:10:12]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[liberato (no reviews please), 2017-05-12 21:10:27]

thanks for the feedback everybody.

-fl

https://codereview.chromium.org/2864603002/diff/220001/content/gpu/gpu_child_...
File content/gpu/gpu_child_thread.cc (right):

https://codereview.chromium.org/2864603002/diff/220001/content/gpu/gpu_child_...
content/gpu/gpu_child_thread.cc:315: const base::UnguessableToken&
routing_token,
On 2017/05/12 20:57:39, jbauman wrote:
> Could you make this static (so we don't need base::Unretained above)?

Done.

[liberato (no reviews please), 2017-05-12 21:10:35]

The CQ bit was checked by liberato@chromium.org

[liberato (no reviews please), 2017-05-12 21:10:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
tguilbert@chromium.org, watk@chromium.org, jbauman@chromium.org
Link to the patchset: https://codereview.chromium.org/2864603002/#ps260001
(title: "added comment about static-ness")

[commit-bot: I haz the power, 2017-05-12 21:11:46]

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, 2017-05-12 23:12:58]

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

[commit-bot: I haz the power, 2017-05-12 23:13:01]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[liberato (no reviews please), 2017-05-13 06:25:26]

The CQ bit was checked by liberato@chromium.org

[commit-bot: I haz the power, 2017-05-13 06:25:41]

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, 2017-05-13 06:33:14]

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

[commit-bot: I haz the power, 2017-05-13 06:33:15]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[liberato (no reviews please), 2017-05-13 07:02:30]

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

[commit-bot: I haz the power, 2017-05-13 07:02:41]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[liberato (no reviews please), 2017-05-13 07:02:42]

The CQ bit was unchecked by liberato@chromium.org

[liberato (no reviews please), 2017-05-13 07:02:47]

The CQ bit was checked by liberato@chromium.org

[liberato (no reviews please), 2017-05-13 07:02:48]

The patchset sent to the CQ was uploaded after l-g-t-m from watk@chromium.org,
tguilbert@chromium.org, jbauman@chromium.org
Link to the patchset: https://codereview.chromium.org/2864603002/#ps180002
(title: "rebased")

[commit-bot: I haz the power, 2017-05-13 07:03:03]

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, 2017-05-13 16:50:41]

CQ is committing da patch.

Bot data: {"patchset_id": 180002, "attempt_start_ts": 1494658967981130,
"parent_rev": "c3445e93b940e12b2e2275e9a985880a58aaa4b0", "commit_rev":
"441ca70d0885c3cefafce97b8f72922ee05111e7"}

[commit-bot: I haz the power, 2017-05-13 16:50:52]

Description was changed from

==========
Provide callback to create mojo AndroidOverlays to AVDA.

In order to create an overlay, AVDA must be able to talk to the
AndroidOverlayProviderImpl mojo service in the browser.  However,
media/ doesn't have access to much that would let it look up the
service by itself.

This CL plumbs a callback for creating these overlays to AVDA from
GpuChildThread.  GpuChildThread has access to the mojo Connector
object, which allows it to look up services in the browser process.

The callback takes as input the routing token and a configuration
struct, and returns the overlay.

BUG=721201
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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

==========
Provide callback to create mojo AndroidOverlays to AVDA.

In order to create an overlay, AVDA must be able to talk to the
AndroidOverlayProviderImpl mojo service in the browser.  However,
media/ doesn't have access to much that would let it look up the
service by itself.

This CL plumbs a callback for creating these overlays to AVDA from
GpuChildThread.  GpuChildThread has access to the mojo Connector
object, which allows it to look up services in the browser process.

The callback takes as input the routing token and a configuration
struct, and returns the overlay.

BUG=721201
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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

Review-Url: https://codereview.chromium.org/2864603002
Cr-Commit-Position: refs/heads/master@{#471589}
Committed:
https://chromium.googlesource.com/chromium/src/+/441ca70d0885c3cefafce97b8f72...
==========

[commit-bot: I haz the power, 2017-05-13 16:50:54]

Committed patchset #15 (id:180002) as
https://chromium.googlesource.com/chromium/src/+/441ca70d0885c3cefafce97b8f72...