Fix media_mojo builds

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}
Committed: https://chromium.googlesource.com/chromium/src/+/6e462aab03239f5ddfcec2adc598e5d775fb70e2

[jrummell, 2017-03-07 00:05:31]

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

[commit-bot: I haz the power, 2017-03-07 00:06:29]

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-03-07 01:12:03]

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

[commit-bot: I haz the power, 2017-03-07 01:12:04]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[jrummell, 2017-03-07 21:26:58]

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

[jrummell, 2017-03-07 21:26:58]

PTAL. The problem is that //media/mojo/services:lib is a source_set, and so
everybody needs to use it with MEDIA_MOJO_IMPLEMENTATION set (as otherwise the
same executable has both import and export of the same symbol, which confuses
the linker).

[xhwang, 2017-03-08 00:35:18]

Thanks! Just a few comments.

https://codereview.chromium.org/2737513004/diff/1/media/base/cdm_promise.cc
File media/base/cdm_promise.cc (right):

https://codereview.chromium.org/2737513004/diff/1/media/base/cdm_promise.cc#n...
media/base/cdm_promise.cc:20: CdmPromise::STRING_TYPE;
Does this also fix
https://bugs.chromium.org/p/chromium/issues/detail?id=656706#c5 ?

If so, you may want to remove some unnecessary includes mentioned in the
comment.

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

https://codereview.chromium.org/2737513004/diff/1/media/mojo/BUILD.gn#newcode33
media/mojo/BUILD.gn:33: "//media/mojo/services:lib",
Can you depend on //media/mojo/services instead then we don't need the hack?

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

https://codereview.chromium.org/2737513004/diff/1/media/mojo/services/BUILD.g...
media/mojo/services/BUILD.gn:142: # set as the source will be bundled with this
service code.
Could you please provide a few more details, e.g. the import/export is causing
conflict...

[xhwang, 2017-04-19 06:26:19]

Kindly ping on this CL, which we still need to fix the bug. Thanks!

[jrummell, 2017-07-11 21:06:58]

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

[commit-bot: I haz the power, 2017-07-11 21:07:37]

Dry run: CQ is trying da patch.

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

[xhwang, 2017-07-11 21:16:38]

Please make sure you test with component build on windows with is_clang being
true AND false.

https://codereview.chromium.org/2737513004/diff/20001/media/base/cdm_promise.h
File media/base/cdm_promise.h (right):

https://codereview.chromium.org/2737513004/diff/20001/media/base/cdm_promise....
media/base/cdm_promise.h:15: #include "media/base/content_decryption_module.h"
Does this CL also fix this?

https://codereview.chromium.org/2737513004/diff/20001/media/base/cdm_promise....
media/base/cdm_promise.h:140: template <>
Add a comment why we need to this. Link to bugs for example.

[commit-bot: I haz the power, 2017-07-11 21:22:06]

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

[commit-bot: I haz the power, 2017-07-11 21:22:07]

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

[jrummell, 2017-07-11 23:24:02]

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

[commit-bot: I haz the power, 2017-07-11 23:24:40]

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-07-11 23:38:01]

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

[commit-bot: I haz the power, 2017-07-11 23:38:04]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jrummell, 2017-07-12 17:59:01]

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

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

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-07-12 18:02:15]

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

[commit-bot: I haz the power, 2017-07-12 18:02:19]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[jrummell, 2017-07-12 18:47:01]

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

[commit-bot: I haz the power, 2017-07-12 18:47:22]

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-07-12 19:53:16]

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

[commit-bot: I haz the power, 2017-07-12 19:53:17]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jrummell, 2017-07-12 20:55:09]

Description was changed from

==========
Fix media_mojo builds

Due to issue 670094, //media/mojo/services:lib is a source_set. Thus all
components that use it need to be aware that "MEDIA_MOJO_IMPLEMENTATION"
is set, so add settings to reflect this.

BUG=676418
TEST=media_mojo tests compile and run on Windows
==========

to

==========
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
==========

[jrummell, 2017-07-12 20:57:10]

Looks like this finally fixes all the link errors. PTAL.

https://codereview.chromium.org/2737513004/diff/20001/media/base/cdm_promise.h
File media/base/cdm_promise.h (right):

https://codereview.chromium.org/2737513004/diff/20001/media/base/cdm_promise....
media/base/cdm_promise.h:15: #include "media/base/content_decryption_module.h"
On 2017/07/11 21:16:38, xhwang wrote:
> Does this CL also fix this?

Trying.

https://codereview.chromium.org/2737513004/diff/20001/media/base/cdm_promise....
media/base/cdm_promise.h:140: template <>
On 2017/07/11 21:16:38, xhwang wrote:
> Add a comment why we need to this. Link to bugs for example.

Done (although we don't have an explicit bug for this issue -- this is just
exposed as a result of fixing some of the other linking issues).

[xhwang, 2017-07-12 22:21:07]

lgtm

https://codereview.chromium.org/2737513004/diff/80001/media/base/cdm_promise.h
File media/base/cdm_promise.h (right):

https://codereview.chromium.org/2737513004/diff/80001/media/base/cdm_promise....
media/base/cdm_promise.h:93: struct
CdmPromiseTraits<CdmKeyInformation::KeyStatus> {
export?

[jrummell, 2017-07-13 00:06:39]

Thanks for the reviews.

https://codereview.chromium.org/2737513004/diff/80001/media/base/cdm_promise.h
File media/base/cdm_promise.h (right):

https://codereview.chromium.org/2737513004/diff/80001/media/base/cdm_promise....
media/base/cdm_promise.h:93: struct
CdmPromiseTraits<CdmKeyInformation::KeyStatus> {
On 2017/07/12 22:21:07, xhwang wrote:
> export?

Added. Thanks.

[jrummell, 2017-07-13 00:06:54]

The CQ bit was checked by jrummell@chromium.org

[jrummell, 2017-07-13 00:06:55]

The patchset sent to the CQ was uploaded after l-g-t-m from xhwang@chromium.org
Link to the patchset: https://codereview.chromium.org/2737513004/#ps100001
(title: "add export")

[commit-bot: I haz the power, 2017-07-13 00:07:08]

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-07-13 04:31:50]

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

[commit-bot: I haz the power, 2017-07-13 04:31:51]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jrummell, 2017-07-13 20:41:05]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2017-07-13 20:41:29]

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-07-13 20:49:50]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1499978465190480,
"parent_rev": "628c09217d960537d690dabe59ecd0ab9a3f28d3", "commit_rev":
"6e462aab03239f5ddfcec2adc598e5d775fb70e2"}

[commit-bot: I haz the power, 2017-07-13 20:50:07]

Description was changed from

==========
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
==========

to

==========
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}
Committed:
https://chromium.googlesource.com/chromium/src/+/6e462aab03239f5ddfcec2adc598...
==========

[commit-bot: I haz the power, 2017-07-13 20:50:08]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/6e462aab03239f5ddfcec2adc598...