Enable content embedders to specify mojo application task runner.

Enable content embedders to specify mojo application task runner.

BUG=594234
Committed: https://crrev.com/ffd6d8448a8ee5f5b66243c179fc76954bc06991
Cr-Commit-Position: refs/heads/master@{#391904}

[alokp, 2016-05-05 13:16:49]

alokp@chromium.org changed reviewers:
+ rockot@chromium.org

[alokp, 2016-05-05 13:16:50]

Ken: I still need to update all callsites, but could you check if the API looks
reasonable. Thanks!

[Ken Rockot(use gerrit already), 2016-05-05 14:20:41]

rockot@chromium.org changed reviewers:
+ jam@chromium.org

[Ken Rockot(use gerrit already), 2016-05-05 14:20:41]

This seems fine to me, but +jam to look at the public content API change before
you update more call sites.

[jam, 2016-05-05 15:45:46]

lgtm

[alokp, 2016-05-05 17:27:04]

alokp@chromium.org changed reviewers:
+ xhwang@chromium.org

[alokp, 2016-05-05 17:27:05]

Xiaohan: Is it OK to delete code to register mojo:media in the browser process?
It is dead code. Do you use it for local testing?

Ken: It seems like MojoShellContext::SetApplicationsForTest is dead code as
well. At least grepping did not find any usage. Is it OK to delete?

[Ken Rockot(use gerrit already), 2016-05-05 17:28:26]

On 2016/05/05 at 17:27:05, alokp wrote:
> Xiaohan: Is it OK to delete code to register mojo:media in the browser
process? It is dead code. Do you use it for local testing?
> 
> Ken: It seems like MojoShellContext::SetApplicationsForTest is dead code as
well. At least grepping did not find any usage. Is it OK to delete?

It would be OK to delete, but I was going to do that anyway (there's even more
related dead code that can be deleted). Feel free to delete it or leave it for
later.

[xhwang, 2016-05-05 17:30:03]

On 2016/05/05 17:28:26, Ken Rockot wrote:
> On 2016/05/05 at 17:27:05, alokp wrote:
> > Xiaohan: Is it OK to delete code to register mojo:media in the browser
> process? It is dead code. Do you use it for local testing?

Yes, I still use it for testing. It's not compiled by default so it should be
harmless? What do you mean by "it is dead code"? :)

> > Ken: It seems like MojoShellContext::SetApplicationsForTest is dead code as
> well. At least grepping did not find any usage. Is it OK to delete?
> 
> It would be OK to delete, but I was going to do that anyway (there's even more
> related dead code that can be deleted). Feel free to delete it or leave it for
> later.

[alokp, 2016-05-05 17:31:06]

On 2016/05/05 17:28:26, Ken Rockot wrote:
> On 2016/05/05 at 17:27:05, alokp wrote:
> > Xiaohan: Is it OK to delete code to register mojo:media in the browser
> process? It is dead code. Do you use it for local testing?
> > 
> > Ken: It seems like MojoShellContext::SetApplicationsForTest is dead code as
> well. At least grepping did not find any usage. Is it OK to delete?
> 
> It would be OK to delete, but I was going to do that anyway (there's even more
> related dead code that can be deleted). Feel free to delete it or leave it for
> later.

The latest patch deletes it. Otherwise I would had to update it to use
MojoApplicationInfo.

[alokp, 2016-05-05 18:17:01]

On 2016/05/05 17:30:03, xhwang wrote:
> On 2016/05/05 17:28:26, Ken Rockot wrote:
> > On 2016/05/05 at 17:27:05, alokp wrote:
> > > Xiaohan: Is it OK to delete code to register mojo:media in the browser
> > process? It is dead code. Do you use it for local testing?
> 
> Yes, I still use it for testing. It's not compiled by default so it should be
> harmless? What do you mean by "it is dead code"? :)
> 

By dead code, I mean I did not see a way to compile it except by overriding the
settings in media_options.gni. Even with "enable_mojo_media=true",
mojo_media_host is either "gpu" or "utility". Do you manually set it to
"browser" for testing?

It is not really harmless. It is a maintenance burden :)

I can update it to use the new API if you register media application in the
browser process. Please let me know.

[xhwang, 2016-05-05 18:21:13]

On 2016/05/05 18:17:01, alokp wrote:
> On 2016/05/05 17:30:03, xhwang wrote:
> > On 2016/05/05 17:28:26, Ken Rockot wrote:
> > > On 2016/05/05 at 17:27:05, alokp wrote:
> > > > Xiaohan: Is it OK to delete code to register mojo:media in the browser
> > > process? It is dead code. Do you use it for local testing?
> > 
> > Yes, I still use it for testing. It's not compiled by default so it should
be
> > harmless? What do you mean by "it is dead code"? :)
> > 
> 
> By dead code, I mean I did not see a way to compile it except by overriding
the
> settings in media_options.gni. Even with "enable_mojo_media=true",
> mojo_media_host is either "gpu" or "utility". Do you manually set it to
> "browser" for testing?

Yes. That's how I test it.
 
> It is not really harmless. It is a maintenance burden :)
> 
> I can update it to use the new API if you register media application in the
> browser process. Please let me know.

I see. For now let's keep it if it's not a huge burden to maintain.

[alokp, 2016-05-05 18:36:27]

On 2016/05/05 18:21:13, xhwang wrote:
> On 2016/05/05 18:17:01, alokp wrote:
> > On 2016/05/05 17:30:03, xhwang wrote:
> > > On 2016/05/05 17:28:26, Ken Rockot wrote:
> > > > On 2016/05/05 at 17:27:05, alokp wrote:
> > > > > Xiaohan: Is it OK to delete code to register mojo:media in the browser
> > > > process? It is dead code. Do you use it for local testing?
> > > 
> > > Yes, I still use it for testing. It's not compiled by default so it should
> be
> > > harmless? What do you mean by "it is dead code"? :)
> > > 
> > 
> > By dead code, I mean I did not see a way to compile it except by overriding
> the
> > settings in media_options.gni. Even with "enable_mojo_media=true",
> > mojo_media_host is either "gpu" or "utility". Do you manually set it to
> > "browser" for testing?
> 
> Yes. That's how I test it.
>  
> > It is not really harmless. It is a maintenance burden :)
> > 
> > I can update it to use the new API if you register media application in the
> > browser process. Please let me know.
> 
> I see. For now let's keep it if it's not a huge burden to maintain.

DONE.

[alokp, 2016-05-05 18:36:55]

Ken/Xiaohan: All call sites updated. This is ready for review.

[Ken Rockot(use gerrit already), 2016-05-05 19:16:21]

lgtm

[alokp, 2016-05-05 19:38:04]

The CQ bit was checked by alokp@chromium.org

[alokp, 2016-05-05 19:38:05]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/1947313002/#ps80001
(title: "restores mojo:media in browser")

[commit-bot: I haz the power, 2016-05-05 19:38:39]

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

[commit-bot: I haz the power, 2016-05-05 21:33:41]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-05 21:35:05]

Description was changed from

==========
Enable content embedders to specify mojo application task runner.

BUG=594234
==========

to

==========
Enable content embedders to specify mojo application task runner.

BUG=594234

Committed: https://crrev.com/ffd6d8448a8ee5f5b66243c179fc76954bc06991
Cr-Commit-Position: refs/heads/master@{#391904}
==========

[commit-bot: I haz the power, 2016-05-05 21:35:06]

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