Fix media_mojo builds

Previously media/mojo/sevices:lib was done as a source_set due to issue
670094. Now that that issue is fixed, the intermediate "lib" target is
no longer needed.

Removing the suppression of duplicate symbols revealed that some of the
CdmPromiseTemplate methods end up in multiple modules when doing a
component build. So fix CdmPromiseTemplate<>::GetResolveParameterType()
implementation to avoid this.

This also enables //media/mojo/services/media_service_unittests on
Windows (which was disabled due to linking issues that are now fixed).

BUG=676418,676055,656706
TEST=media_mojo tests compile and run

Review-Url: https://codereview.chromium.org/2737513004
Cr-Commit-Position: refs/heads/master@{#486472}

media/BUILD.gn

@@ -397,14 +397,6 @@ source_set("unit_tests") {
 # Contains tests for all targets in the "media" folder.
 # TODO(xhwang): Move mojo/capture/remoting tests here where applicable.
 test("media_unittests") {
-  # crbug.com/676418: Suppress symbol import warnings.
-  if (is_win && is_component_build) {
-    ldflags = [
-      "/ignore:4217",
-      "/ignore:4049",
-    ]
-  }
-
   deps = [
     ":unit_tests",
     "//media/audio:unit_tests",

media/base/cdm_callback_promise.h

@@ -12,8 +12,6 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "media/base/cdm_promise.h"
-// TODO(xhwang): Remove this include after http://crbug.com/656706 is fixed.
-#include "media/base/content_decryption_module.h"
 #include "media/base/media_export.h"
 
 namespace media {

media/base/cdm_promise.cc

@@ -6,10 +6,41 @@
 
 namespace media {
 
-CdmPromise::CdmPromise() {
+const CdmPromise::ResolveParameterType CdmPromiseTraits<>::kType =
+    CdmPromise::VOID_TYPE;
+
+const CdmPromise::ResolveParameterType CdmPromiseTraits<int>::kType =
+    CdmPromise::INT_TYPE;
+
+const CdmPromise::ResolveParameterType CdmPromiseTraits<std::string>::kType =
+    CdmPromise::STRING_TYPE;
+
+const CdmPromise::ResolveParameterType
+    CdmPromiseTraits<CdmKeyInformation::KeyStatus>::kType =
+        CdmPromise::KEY_STATUS_TYPE;
+
+template <>
+CdmPromise::ResolveParameterType CdmPromiseTemplate<>::GetResolveParameterType()
+    const {
+  return CdmPromiseTraits<>::kType;
+}
+
+template <>
+CdmPromise::ResolveParameterType
+CdmPromiseTemplate<int>::GetResolveParameterType() const {
+  return CdmPromiseTraits<int>::kType;
+}
+
+template <>
+CdmPromise::ResolveParameterType
+CdmPromiseTemplate<std::string>::GetResolveParameterType() const {
+  return CdmPromiseTraits<std::string>::kType;
 }
 
-CdmPromise::~CdmPromise() {
+template <>
+CdmPromise::ResolveParameterType CdmPromiseTemplate<
+    CdmKeyInformation::KeyStatus>::GetResolveParameterType() const {
+  return CdmPromiseTraits<CdmKeyInformation::KeyStatus>::kType;
 }
 
 }  // namespace media

media/base/cdm_promise.h

@@ -11,8 +11,7 @@
 
 #include "base/logging.h"
 #include "base/macros.h"
-// TODO(xhwang): Remove this include after http://crbug.com/656706 is fixed.
-#include "media/base/content_decryption_module.h"
+#include "media/base/cdm_key_information.h"
 #include "media/base/media_export.h"
 
 namespace media {
@@ -53,8 +52,8 @@ class MEDIA_EXPORT CdmPromise {
     KEY_STATUS_TYPE
   };
 
-  CdmPromise();
-  virtual ~CdmPromise();
+  CdmPromise() = default;
+  virtual ~CdmPromise() = default;
 
   // Used to indicate that the operation failed. |exception_code| must be
   // specified. |system_code| is a Key System-specific value for the error
@@ -72,30 +71,27 @@ class MEDIA_EXPORT CdmPromise {
   DISALLOW_COPY_AND_ASSIGN(CdmPromise);
 };
 
-// For some reason the Windows compiler is not happy with the implementation
-// of CdmPromiseTemplate being in the .cc file, so moving it here.
 template <typename... T>
 struct CdmPromiseTraits {};
 
 template <>
-struct CdmPromiseTraits<> {
-  static const CdmPromise::ResolveParameterType kType = CdmPromise::VOID_TYPE;
+struct MEDIA_EXPORT CdmPromiseTraits<> {
+  static const CdmPromise::ResolveParameterType kType;
 };
 
 template <>
-struct CdmPromiseTraits<int> {
-  static const CdmPromise::ResolveParameterType kType = CdmPromise::INT_TYPE;
+struct MEDIA_EXPORT CdmPromiseTraits<int> {
+  static const CdmPromise::ResolveParameterType kType;
 };
 
 template <>
-struct CdmPromiseTraits<std::string> {
-  static const CdmPromise::ResolveParameterType kType = CdmPromise::STRING_TYPE;
+struct MEDIA_EXPORT CdmPromiseTraits<std::string> {
+  static const CdmPromise::ResolveParameterType kType;
 };
 
 template <>
-struct CdmPromiseTraits<CdmKeyInformation::KeyStatus> {
-  static const CdmPromise::ResolveParameterType kType =
-      CdmPromise::KEY_STATUS_TYPE;
+struct MEDIA_EXPORT CdmPromiseTraits<CdmKeyInformation::KeyStatus> {
+  static const CdmPromise::ResolveParameterType kType;
 };
 
 // This class adds the resolve(T) method. This class is still an interface, and
@@ -114,9 +110,7 @@ class CdmPromiseTemplate : public CdmPromise {
                       uint32_t system_code,
                       const std::string& error_message) = 0;
 
-  ResolveParameterType GetResolveParameterType() const override {
-    return CdmPromiseTraits<T...>::kType;
-  }
+  ResolveParameterType GetResolveParameterType() const final;
 
  protected:
   bool IsPromiseSettled() const { return is_settled_; }
@@ -147,6 +141,25 @@ class CdmPromiseTemplate : public CdmPromise {
   DISALLOW_COPY_AND_ASSIGN(CdmPromiseTemplate);
 };
 
+// Explicitly defining all variants of GetResolveParameterType().
+// Without this component builds on Windows fail due to versions of the same
+// method being generated in multiple DLLs.
+template <>
+MEDIA_EXPORT CdmPromise::ResolveParameterType
+CdmPromiseTemplate<>::GetResolveParameterType() const;
+
+template <>
+MEDIA_EXPORT CdmPromise::ResolveParameterType
+CdmPromiseTemplate<int>::GetResolveParameterType() const;
+
+template <>
+MEDIA_EXPORT CdmPromise::ResolveParameterType
+CdmPromiseTemplate<std::string>::GetResolveParameterType() const;
+
+template <>
+MEDIA_EXPORT CdmPromise::ResolveParameterType CdmPromiseTemplate<
+    CdmKeyInformation::KeyStatus>::GetResolveParameterType() const;
+
 }  // namespace media
 
 #endif  // MEDIA_BASE_CDM_PROMISE_H_

media/cdm/BUILD.gn

@@ -91,7 +91,7 @@ config("cdm_manager_implementation") {
 component("cdm_manager") {
   visibility = [
     "//media/gpu",
-    "//media/mojo/services:lib",
+    "//media/mojo/services",
   ]
   sources = [
     "cdm_manager.cc",

media/mojo/BUILD.gn

@@ -97,7 +97,7 @@ source_set("unit_tests") {
     "//media/mojo/common:mojo_shared_buffer_video_frame",
     "//media/mojo/interfaces",
     "//media/mojo/interfaces:test_interfaces",
-    "//media/mojo/services:lib",
+    "//media/mojo/services",
     "//services/service_manager/tests:interfaces",
     "//testing/gmock",
     "//testing/gtest",
@@ -113,14 +113,6 @@ source_set("unit_tests") {
 }
 
 test("media_mojo_unittests") {
-  # crbug.com/676418: Suppress symbol import warnings.
-  if (is_win && is_component_build) {
-    ldflags = [
-      "/ignore:4217",
-      "/ignore:4049",
-    ]
-  }
-
   deps = [
     ":unit_tests",
     "//mojo/edk/test:run_all_unittests",

media/mojo/services/BUILD.gn

@@ -11,16 +11,6 @@ import("//testing/test.gni")
 
 component("services") {
   output_name = "media_mojo_services"
-
-  public_deps = [
-    ":lib",
-  ]
-}
-
-# TODO(xhwang): Add this intermediate target because ServiceTest doesn't support
-# services that depend on shared libraries in component build.
-# See http://crbug.com/670094
-source_set("lib") {
   sources = [
     "gpu_mojo_media_client.cc",
     "gpu_mojo_media_client.h",
@@ -106,11 +96,6 @@ source_set("lib") {
 service("media") {
   testonly = true
 
-  # crbug.com/676418: Suppress symbol import warnings.
-  if (is_win && is_component_build) {
-    ldflags = [ "/ignore:4217" ]
-  }
-
   sources = [
     "main.cc",
   ]
@@ -121,44 +106,39 @@ service("media") {
   ]
 
   deps = [
-    ":lib",
+    ":services",
     "//mojo/public/c/system",
     "//services/service_manager/public/cpp",
   ]
 }
 
-# crbug.com/676055: media_service_unittests currently fails to link in Windows
-# component builds, so don't declare it, otherwise the "all" target will still
-# try to build it.
-if (!(is_win && is_component_build)) {
-  service_test("media_service_unittests") {
-    testonly = true
+service_test("media_service_unittests") {
+  testonly = true
 
-    sources = [
-      "media_service_unittest.cc",
-    ]
+  sources = [
+    "media_service_unittest.cc",
+  ]
 
-    catalog = ":media_service_unittests_catalog"
-
-    deps = [
-      ":lib",
-      "//base",
-      "//media",
-      "//media/base:test_support",
-      "//media/mojo/clients",
-      "//media/mojo/common",
-      "//media/mojo/interfaces",
-      "//media/mojo/interfaces:constants",
-      "//services/service_manager/public/cpp:service_test_support",
-      "//testing/gmock",
-      "//testing/gtest",
-    ]
+  catalog = ":media_service_unittests_catalog"
 
-    data_deps = [
-      ":media",
-    ]
-  }
-}  # !(is_win && is_component_build)
+  deps = [
+    ":services",
+    "//base",
+    "//media",
+    "//media/base:test_support",
+    "//media/mojo/clients",
+    "//media/mojo/common",
+    "//media/mojo/interfaces",
+    "//media/mojo/interfaces:constants",
+    "//services/service_manager/public/cpp:service_test_support",
+    "//testing/gmock",
+    "//testing/gtest",
+  ]
+
+  data_deps = [
+    ":media",
+  ]
+}
 
 service_manifest("media_manifest") {
   name = "media"

media/remoting/proto_utils.h

@@ -15,6 +15,7 @@
 #include "media/base/cdm_config.h"
 #include "media/base/cdm_key_information.h"
 #include "media/base/cdm_promise.h"
+#include "media/base/content_decryption_module.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/demuxer_stream.h"
 #include "media/base/eme_constants.h"