Clean up media/mojo target names.

Clean up media/mojo target names.

Changes include:
- Separate the media app and the renderer_service. In the future, we'll have
  more services in the same app (e.g. cdm_service). Also, we may need to host
  the services in Chromium directly (e.g. in the browser process) instead of in
  a mojo app.
- Rename several media/mojo target names for clarity and consistency.

BUG=432998
TEST=No functionality change.

Committed: https://crrev.com/6e81913ef56b25fea87f420874a631972565c94a
Cr-Commit-Position: refs/heads/master@{#307108}

[xhwang, 2014-12-03 22:55:39]

xhwang@chromium.org changed reviewers:
+ aa@chromium.org, dalecurtis@chromium.org

[xhwang, 2014-12-03 22:55:40]

PTAL, I'll update the gyp file after we agreed on these new names.

[DaleCurtis, 2014-12-03 23:18:03]

Hmm, mojo:media seems really generic. How do you see mojo clients specifying the
url for cdm service?

[xhwang, 2014-12-03 23:22:03]

https://codereview.chromium.org/780433005/diff/1/media/mojo/services/BUILD.gn
File media/mojo/services/BUILD.gn (right):

https://codereview.chromium.org/780433005/diff/1/media/mojo/services/BUILD.gn...
media/mojo/services/BUILD.gn:54: "mojo_renderer_impl.h",
Should we actually move these to a "cpp" folder, similar to
mojo/services/public/cpp?

[xhwang, 2014-12-04 00:54:27]

On 2014/12/03 23:18:03, DaleCurtis wrote:
> Hmm, mojo:media seems really generic. How do you see mojo clients specifying
the
> url for cdm service?

I talked to aa about this the other day. This is following existing examples,
e.g.: mojo:clipboard
https://code.google.com/p/chromium/codesearch#chromium/src/mojo/services/html...

mojo::media will be the app providing multiple media related services, including
MediaRenderer service and CDM service.

[DaleCurtis, 2014-12-04 02:39:28]

Hmm, okay, it still seems a bit weird, but we can always change it later. lgtm

[xhwang, 2014-12-04 20:56:24]

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

[xhwang, 2014-12-04 20:56:25]

+jamesr for his insight. I am especially not sure about the "proxy" name.

[jamesr, 2014-12-04 23:45:38]

https://codereview.chromium.org/780433005/diff/1/media/mojo/services/BUILD.gn
File media/mojo/services/BUILD.gn (right):

https://codereview.chromium.org/780433005/diff/1/media/mojo/services/BUILD.gn...
media/mojo/services/BUILD.gn:17: source_set("common") {
we call converters 'converters' in other parts of the tree

https://codereview.chromium.org/780433005/diff/1/media/mojo/services/BUILD.gn...
media/mojo/services/BUILD.gn:30: sources = [
sources should go before deps

https://codereview.chromium.org/780433005/diff/1/media/mojo/services/BUILD.gn...
media/mojo/services/BUILD.gn:47: ":common",
run 'gn format' - local deps should go before absolute ones

https://codereview.chromium.org/780433005/diff/1/media/mojo/services/BUILD.gn...
media/mojo/services/BUILD.gn:54: "mojo_renderer_impl.h",
On 2014/12/03 23:22:03, xhwang wrote:
> Should we actually move these to a "cpp" folder, similar to
> mojo/services/public/cpp?

I don't think so (that pattern is useful if you want to support client libraries
in multiple languages, but if you only support talking to your interface from
C++ the extra indirection is not very helpful)

https://codereview.chromium.org/780433005/diff/1/media/mojo/services/BUILD.gn...
media/mojo/services/BUILD.gn:84: output_name = "media"
it's kind of weird to have mojo_native_application() but then override the
output_name.  why not just name the target what you want the output to be?

https://codereview.chromium.org/780433005/diff/1/media/mojo/services/BUILD.gn...
media/mojo/services/BUILD.gn:123: # version of this test.
you can delete this comment now

[jamesr, 2014-12-04 23:47:22]

What is the roll that renderer_proxy serves?

[jamesr, 2014-12-05 00:26:43]

On 2014/12/04 23:47:22, jamesr wrote:
> What is the *role* that renderer_proxy serves?

fixed

[xhwang, 2014-12-05 04:55:21]

(comment only)

Thanks for the comments, these are super useful!

The *_proxy targets are those c++ classes backed/implemented by mojo. For
example, we have a media::Foo interface, now we have MojoFoo implementing it
using mojo interface:

class MojoFoo : public Foo {
public:
  void Bar() {
    mojo_foo_ptr_->Bar();
  }
private:
  MojoFooPtr* mojo_foo_ptr_;
};

[xhwang, 2014-12-05 06:23:29]

https://codereview.chromium.org/780433005/diff/1/media/mojo/services/BUILD.gn
File media/mojo/services/BUILD.gn (right):

https://codereview.chromium.org/780433005/diff/1/media/mojo/services/BUILD.gn...
media/mojo/services/BUILD.gn:17: source_set("common") {
On 2014/12/04 23:45:38, jamesr wrote:
> we call converters 'converters' in other parts of the tree

Done.

https://codereview.chromium.org/780433005/diff/1/media/mojo/services/BUILD.gn...
media/mojo/services/BUILD.gn:30: sources = [
On 2014/12/04 23:45:38, jamesr wrote:
> sources should go before deps

Done.

https://codereview.chromium.org/780433005/diff/1/media/mojo/services/BUILD.gn...
media/mojo/services/BUILD.gn:47: ":common",
On 2014/12/04 23:45:38, jamesr wrote:
> run 'gn format' - local deps should go before absolute ones

Thanks for the info!

However, "gn format" doesn't work for me now :(

ERROR Unable to load
"usr/local/google/workspace/chrome2/src/media/mojo/BUILD.gn".

https://codereview.chromium.org/780433005/diff/1/media/mojo/services/BUILD.gn...
media/mojo/services/BUILD.gn:54: "mojo_renderer_impl.h",
On 2014/12/04 23:45:38, jamesr wrote:
> On 2014/12/03 23:22:03, xhwang wrote:
> > Should we actually move these to a "cpp" folder, similar to
> > mojo/services/public/cpp?
> 
> I don't think so (that pattern is useful if you want to support client
libraries
> in multiple languages, but if you only support talking to your interface from
> C++ the extra indirection is not very helpful)

Acknowledged.

https://codereview.chromium.org/780433005/diff/1/media/mojo/services/BUILD.gn...
media/mojo/services/BUILD.gn:84: output_name = "media"
On 2014/12/04 23:45:38, jamesr wrote:
> it's kind of weird to have mojo_native_application() but then override the
> output_name.  why not just name the target what you want the output to be?

Done.

https://codereview.chromium.org/780433005/diff/1/media/mojo/services/BUILD.gn...
media/mojo/services/BUILD.gn:123: # version of this test.
On 2014/12/04 23:45:38, jamesr wrote:
> you can delete this comment now

Done.

[jamesr, 2014-12-05 20:56:22]

That sounds consistent with how we use the term 'proxy' elsewhere in the
codebase.

[xhwang, 2014-12-05 21:55:43]

On 2014/12/05 20:56:22, jamesr wrote:
> That sounds consistent with how we use the term 'proxy' elsewhere in the
> codebase.

Thanks! Could you also please OWNERS review
mojo/services/html_viewer/webmediaplayer_factory.cc?

[jamesr, 2014-12-05 21:56:29]

that change lgtm

[xhwang, 2014-12-05 22:05:13]

The CQ bit was checked by xhwang@chromium.org

[commit-bot: I haz the power, 2014-12-05 22:06:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/780433005/40001

[commit-bot: I haz the power, 2014-12-05 22:09:37]

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

[commit-bot: I haz the power, 2014-12-05 22:09:38]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/90659)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_rel_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_x64_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[xhwang, 2014-12-05 22:46:55]

rebase only

[xhwang, 2014-12-05 23:04:04]

The CQ bit was checked by xhwang@chromium.org

[commit-bot: I haz the power, 2014-12-05 23:04:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/780433005/60001

[commit-bot: I haz the power, 2014-12-05 23:27:45]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-12-05 23:28:43]

Patchset 4 (id:??) landed as
https://crrev.com/6e81913ef56b25fea87f420874a631972565c94a
Cr-Commit-Position: refs/heads/master@{#307108}