Allow widevinecdmadapter to be built in Chromium with GN

Allow widevinecdmadapter to be built in Chromium with GN

BUG=468567
Committed: https://crrev.com/8a4aea4026a60d35935c2158c0fb311dfdb1d047
Cr-Commit-Position: refs/heads/master@{#325363}

[jrummell, 2015-04-09 20:35:15]

jrummell@chromium.org changed reviewers:
+ ddorwin@chromium.org, sandersd@chromium.org, xhwang@chromium.org

[jrummell, 2015-04-09 20:35:15]

PTAL.

[jrummell, 2015-04-09 21:42:50]

Updated to build both widevinecdm and widevinecdmadapter.

[jrummell, 2015-04-09 21:44:17]

Added comment in case anybody has an opinion.

https://codereview.chromium.org/1070253002/diff/20001/third_party/widevine/cd...
File third_party/widevine/cdm/BUILD.gn (right):

https://codereview.chromium.org/1070253002/diff/20001/third_party/widevine/cd...
third_party/widevine/cdm/BUILD.gn:136: [
rebase_path("$root_out_dir/libwidevinecdm.so", root_build_dir) ]
Not sure if the rebase_path() is needed now that the tag is set properly.

[jrummell, 2015-04-13 18:22:08]

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

[jrummell, 2015-04-13 18:22:09]

Adding dalecurtis@.

[DaleCurtis, 2015-04-13 18:35:58]

https://codereview.chromium.org/1070253002/diff/20001/third_party/widevine/cd...
File third_party/widevine/cdm/BUILD.gn (right):

https://codereview.chromium.org/1070253002/diff/20001/third_party/widevine/cd...
third_party/widevine/cdm/BUILD.gn:65: if (widevine_cdm_binary_files != []) {
is the != [] necessary, does !widevine_cdm_binary_files work?

https://codereview.chromium.org/1070253002/diff/20001/third_party/widevine/cd...
third_party/widevine/cdm/BUILD.gn:136: [
rebase_path("$root_out_dir/libwidevinecdm.so", root_build_dir) ]
On 2015/04/09 21:44:17, jrummell wrote:
> Not sure if the rebase_path() is needed now that the tag is set properly.

Does it build without it ? :)

[jrummell, 2015-04-13 22:22:35]

Comments only.

https://codereview.chromium.org/1070253002/diff/20001/third_party/widevine/cd...
File third_party/widevine/cdm/BUILD.gn (right):

https://codereview.chromium.org/1070253002/diff/20001/third_party/widevine/cd...
third_party/widevine/cdm/BUILD.gn:65: if (widevine_cdm_binary_files != []) {
On 2015/04/13 18:35:58, DaleCurtis wrote:
> is the != [] necessary, does !widevine_cdm_binary_files work?

Doesn't work. Error is "Operand of ! operator is not a boolean. Type is "list"
instead."

https://codereview.chromium.org/1070253002/diff/20001/third_party/widevine/cd...
third_party/widevine/cdm/BUILD.gn:136: [
rebase_path("$root_out_dir/libwidevinecdm.so", root_build_dir) ]
On 2015/04/13 18:35:58, DaleCurtis wrote:
> On 2015/04/09 21:44:17, jrummell wrote:
> > Not sure if the rebase_path() is needed now that the tag is set properly.
> 
> Does it build without it ? :)

It does. However, this appears to be the standard pattern in clearkeycdmadapter
and ffmpeg, so leaving it alone.

[DaleCurtis, 2015-04-14 18:53:16]

lgtm, but defer to EME folk if this is the structure you're looking for.  You'll
also need a build\ OWNER to sign off.

[sandersd (OOO until July 31), 2015-04-14 20:40:15]

lgtm

[jrummell, 2015-04-14 20:44:11]

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

[jrummell, 2015-04-14 20:44:12]

+jhawkins@ for OWNERS review of renames in chrome/ GN files.

[James Hawkins, 2015-04-14 20:53:55]

lgtm

[xhwang, 2015-04-14 21:27:30]

Looking good. I only have some nits/comments.

https://codereview.chromium.org/1070253002/diff/20001/third_party/widevine/cd...
File third_party/widevine/cdm/BUILD.gn (right):

https://codereview.chromium.org/1070253002/diff/20001/third_party/widevine/cd...
third_party/widevine/cdm/BUILD.gn:72: # TODO(GYP)
Here and below, put a real name after TODO.

https://codereview.chromium.org/1070253002/diff/20001/third_party/widevine/cd...
third_party/widevine/cdm/BUILD.gn:75: } else if (!is_android &&
!is_chrome_branded && enable_widevine) {
Can we do something like?

else if (!is_android && enable_widevine) {
  assert(!is_chrome_branded, "Branded Chrome should have binary files to copy.")

https://codereview.chromium.org/1070253002/diff/20001/third_party/widevine/cd...
third_party/widevine/cdm/BUILD.gn:75: } else if (!is_android &&
!is_chrome_branded && enable_widevine) {
Here !is_android is pretty much the same as enable_pepper_cdms (used on l.110).
Maybe we should be consistent on this. I think enable_pepper_cdms makes more
sense because the CDM needs pepper to work.

[jrummell, 2015-04-14 22:47:43]

Updated.

https://codereview.chromium.org/1070253002/diff/20001/third_party/widevine/cd...
File third_party/widevine/cdm/BUILD.gn (right):

https://codereview.chromium.org/1070253002/diff/20001/third_party/widevine/cd...
third_party/widevine/cdm/BUILD.gn:72: # TODO(GYP)
On 2015/04/14 21:27:30, xhwang wrote:
> Here and below, put a real name after TODO.

Done.

https://codereview.chromium.org/1070253002/diff/20001/third_party/widevine/cd...
third_party/widevine/cdm/BUILD.gn:75: } else if (!is_android &&
!is_chrome_branded && enable_widevine) {
On 2015/04/14 21:27:30, xhwang wrote:
> Can we do something like?
> 
> else if (!is_android && enable_widevine) {
>   assert(!is_chrome_branded, "Branded Chrome should have binary files to
copy.")

Done.

https://codereview.chromium.org/1070253002/diff/20001/third_party/widevine/cd...
third_party/widevine/cdm/BUILD.gn:75: } else if (!is_android &&
!is_chrome_branded && enable_widevine) {
On 2015/04/14 21:27:30, xhwang wrote:
> Here !is_android is pretty much the same as enable_pepper_cdms (used on
l.110).
> Maybe we should be consistent on this. I think enable_pepper_cdms makes more
> sense because the CDM needs pepper to work.

Done.

https://codereview.chromium.org/1070253002/diff/20001/third_party/widevine/cd...
third_party/widevine/cdm/BUILD.gn:76: # TODO(GYP) On Mac/Linux this should be a
loadable_module.
Removed this TODO since it came from cdm_adapter template, and clearkeycdm
doesn't have it. Plus there isn't loadable_module in GN anyway.

[xhwang, 2015-04-15 22:33:47]

lgtm

[jrummell, 2015-04-15 22:35:37]

The CQ bit was checked by jrummell@chromium.org

[jrummell, 2015-04-15 22:35:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
sandersd@chromium.org, jhawkins@chromium.org, dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/1070253002/#ps40001
(title: "change todos")

[commit-bot: I haz the power, 2015-04-15 22:36:28]

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

[commit-bot: I haz the power, 2015-04-16 00:58:23]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-04-16 01:00:03]

Patchset 3 (id:??) landed as
https://crrev.com/8a4aea4026a60d35935c2158c0fb311dfdb1d047
Cr-Commit-Position: refs/heads/master@{#325363}