media: Make media mojo services a component

media/mojo/services/BUILD.gn

 # Copyright 2014 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import("//media/media_options.gni")
 import("//testing/test.gni")
 import("//mojo/public/mojo_application.gni")
 import("//mojo/public/mojo_application_manifest.gni")
 
 # Target naming conventions:
@@ skipping 66 matching lines @@
     "//media",
     "//media/mojo/common",
     "//media/mojo/interfaces",
     "//mojo/common",
     "//mojo/public/c/system:for_component",
     "//services/shell/public/cpp:sources",
     "//services/shell/public/interfaces",
   ]
 }
 
-source_set("cdm_service") {
-  deps = [
-    "//base",
-    "//media",
-    "//media/mojo/common",
-    "//media/mojo/interfaces",
-    "//mojo/common",
-    "//mojo/public/c/system:for_component",
-    "//services/shell/public/interfaces",
-    "//url",
-  ]
+component("services") {
+  output_name = "media_mojo_services"
 
   sources = [
+    "demuxer_stream_provider_shim.cc",

[ddorwin, 2016/05/11 23:02:27]

We now build everything regardless of whether a given platform/build
configuration needs/uses it?

[xhwang, 2016/05/16 05:41:20]

Yes. Previously I was trying to keep things separate so that we only build
things that we need. But it becomes a pain to maintain. Now I need to use
component (shared/static library). Having multiple components would be even
harder: we need to have different MEDIA_MOJO_XXX_EXPORT for each of them.

By building everything, we rely on the linker to remove symbols that are not
reachable. So it won't affect production build. It does increase build time a
little bit when mojo media is enabled, but makes the build file much easier to
manage.

Another option is to include things based on the config above, e.g. only include
audio decoder service when ENABLE_MOJO_AUDIO_DECODER is true. That's doable. But
again I am not sure whether it's worth it.

WDYT?

+    "demuxer_stream_provider_shim.h",
+    "mojo_audio_decoder_service.cc",
+    "mojo_audio_decoder_service.h",
     "mojo_cdm_allocator.cc",
     "mojo_cdm_allocator.h",
     "mojo_cdm_promise.cc",
     "mojo_cdm_promise.h",
     "mojo_cdm_service.cc",
     "mojo_cdm_service.h",
     "mojo_cdm_service_context.cc",
     "mojo_cdm_service_context.h",
     "mojo_decryptor_service.cc",
     "mojo_decryptor_service.h",
+    "mojo_demuxer_stream_adapter.cc",
+    "mojo_demuxer_stream_adapter.h",
+    "mojo_media_application.cc",
+    "mojo_media_application.h",
+    "mojo_media_application_factory.cc",
+    "mojo_media_application_factory.h",
+    "mojo_media_client.cc",
+    "mojo_media_client.h",
+    "mojo_renderer_service.cc",
+    "mojo_renderer_service.h",
     "mojo_type_trait.h",
+    "service_factory_impl.cc",
+    "service_factory_impl.h",
+  ]
+
+  defines = [ "MEDIA_MOJO_IMPLEMENTATION" ]

[ddorwin, 2016/05/11 23:02:27]

This is new?

[xhwang, 2016/05/16 05:41:20]

Yes, this is needed since we are "component" now.

+  configs += [ "//build/config/compiler:no_size_t_to_int_warning" ]

[ddorwin, 2016/05/11 23:02:28]

Should we comment the reason for this?

[xhwang, 2016/05/16 05:41:20]

Done

+  public_configs = [ ":mojo_media_config" ]

[ddorwin, 2016/05/11 23:02:27]

This was associated with a TODO about cdm_service. Is this no longer
incorrect/needing correction?

[xhwang, 2016/05/16 05:41:20]

cdm_service doesn't need this but has to have it for the reason documented in
the TODO. Now we have the app in this target and we should always have this
public_configs.

+
+  deps = [
+    "//base",
+    "//media",
+    "//media:shared_memory_support",
+    "//media/mojo/common",
+    "//media/mojo/interfaces",
+    "//mojo/common",
+    "//mojo/public/c/system:for_component",
+    "//services/shell/public/cpp",
+    "//services/shell/public/cpp:sources",
+    "//services/shell/public/interfaces",
+    "//url",
   ]
 
   if (is_android) {
     sources += [
       "mojo_provision_fetcher.cc",
       "mojo_provision_fetcher.h",
     ]
   }
 
-  # TODO(xhwang): Needed because targets can depend on cdm_service directly,
-  # which is a bit hacky since we need to access CdmService directly
-  # from C++ code (AVDA). In the future we'll make those decoders part of
-  # MojoMediaApplication, then we won't need this.
-  public_configs = [ ":mojo_media_config" ]
-
-  configs += [ "//build/config/compiler:no_size_t_to_int_warning" ]
-}
-
-source_set("audio_decoder_service") {
-  sources = [
-    "mojo_audio_decoder_service.cc",
-    "mojo_audio_decoder_service.h",
-  ]
-
-  deps = [
-    ":cdm_service",
-    "//base",
-    "//media",
-    "//media:shared_memory_support",
-    "//media/mojo/common",
-    "//media/mojo/interfaces",
-    "//mojo/common",
-  ]
-}
-
-source_set("renderer_service") {
-  sources = [
-    "demuxer_stream_provider_shim.cc",
-    "demuxer_stream_provider_shim.h",
-    "mojo_demuxer_stream_adapter.cc",
-    "mojo_demuxer_stream_adapter.h",
-    "mojo_renderer_service.cc",
-    "mojo_renderer_service.h",
-  ]
-
-  deps = [
-    ":cdm_service",
-    "//base",
-    "//media",
-    "//media:shared_memory_support",
-    "//media/mojo/common",
-    "//media/mojo/interfaces",
-    "//mojo/common",
-  ]
-}
-
-source_set("application") {
-  sources = [
-    "mojo_media_application.cc",
-    "mojo_media_application.h",
-    "mojo_media_client.cc",
-    "mojo_media_client.h",
-    "service_factory_impl.cc",
-    "service_factory_impl.h",
-  ]
-
-  public_configs = [ ":mojo_media_config" ]
-
-  deps = [
-    ":audio_decoder_service",
-    ":cdm_service",
-    ":renderer_service",
-    "//base",
-    "//media",
-    "//media/mojo/interfaces",
-    "//services/shell/public/cpp",
-  ]
-}
-
-source_set("application_factory") {
-  sources = [
-    "mojo_media_application_factory.cc",
-    "mojo_media_application_factory.h",
-  ]
-
-  public_configs = [ ":mojo_media_config" ]
-
-  deps = [
-    ":application",
-    "//base",
-    "//media",
-    "//services/shell/public/cpp:sources",
-  ]
-
   if (enable_test_mojo_media_client) {

[ddorwin, 2016/05/11 23:02:28]

Does this mean we build all the files above multiple times - once for the
product and once for the test?

[xhwang, 2016/05/16 05:41:20]

This is used for test in chromium by developers, not in unittests. The files
above will only be built once.

-    defines = [ "ENABLE_TEST_MOJO_MEDIA_CLIENT" ]
+    defines += [ "ENABLE_TEST_MOJO_MEDIA_CLIENT" ]
     sources += [
       "test_mojo_media_client.cc",
       "test_mojo_media_client.h",
     ]
   } else if (is_android) {
     sources += [
       "android_mojo_media_client.cc",
       "android_mojo_media_client.h",
     ]
   } else {
     sources += [
       "default_mojo_media_client.cc",
       "default_mojo_media_client.h",
     ]
   }
 }
 
 mojo_native_application("media") {
   testonly = true
 
   sources = [
     "main.cc",
     "test_mojo_media_client.cc",
     "test_mojo_media_client.h",
   ]
 
   deps = [
-    ":application",
+    ":services",
     "//base",
     "//media",
     "//mojo/logging",
     "//mojo/public/c/system:for_shared_library",
     "//services/shell/public/cpp:sources",
   ]
 }
 
 mojo_application_manifest("media_manifest") {
   application_name = "media"
@@ skipping 43 matching lines @@
     ":media",
     ":pipeline_test_manifest",
   ]
 }
 
 mojo_application_manifest("pipeline_test_manifest") {
   application_name = "media_pipeline_integration_unittests"
   type = "exe"
   source = "pipeline_test_manifest.json"
 }