Add Widevine signature files into Mac build

Add Widevine signature files into Mac build

On official Mac builds Widevine is included in the app. This change adds
the matching signature files needed by Widevine.

BUG=684871
TEST=signature files created in the correct place when is_official_build

Review-Url: https://codereview.chromium.org/2647393004
Cr-Commit-Position: refs/heads/master@{#452603}
Committed: https://chromium.googlesource.com/chromium/src/+/5762e61a17e4e05c490be128fd96204f24c1a457

[jrummell, 2017-01-25 00:35:43]

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

[jrummell, 2017-01-25 00:35:43]

PTAL. It does require that the third_party/widevine/ directory have the .sig
file checked in.

[xhwang, 2017-01-26 01:22:56]

I didn't review the details. Just some high level comments about the steps.

https://codereview.chromium.org/2647393004/diff/1/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2647393004/diff/1/chrome/BUILD.gn#newcode856
chrome/BUILD.gn:856: "$root_out_dir/libwidevinecdm.dylib",
Is it possible to just bundle the cdm/adapter sig files here, then we don't need
widevine_copy_cdm_library_signature (I guess).

https://codereview.chromium.org/2647393004/diff/1/chrome/BUILD.gn#newcode866
chrome/BUILD.gn:866: ":widevine_cdm_library_copy",
I wonder whether we could make widevinecdm always a copy target so that we don't
need this hack, i.e. when !is_chrome_branded, the CDM will be generated by a
shared_library(widevine_cdm_lib) target, followed by a target copy(widevinecdm)
which copies the file somewhere.

https://codereview.chromium.org/2647393004/diff/1/chrome/BUILD.gn#newcode895
chrome/BUILD.gn:895: # Bundle the signature files for Widevine and the
CdmAdapter.
nit: s/Widevine and the CdmAdapter/Widevine CDM and CDM adapter/

https://codereview.chromium.org/2647393004/diff/1/chrome/BUILD.gn#newcode928
chrome/BUILD.gn:928: signature_file =
"$root_out_dir/signatures/widevinecdmadapter.plugin.sig"
Is it possible that we generate the sig file directly at
$root_out_dir/$widevine_cdm_path/widevinecdmadapter.plugin.sig?

https://codereview.chromium.org/2647393004/diff/1/chrome/BUILD.gn#newcode982
chrome/BUILD.gn:982: signature_file =
"$root_out_dir/signatures/$chrome_framework_name.sig"
Is it possible to generate the framework sig file to the
$root_out_dir/$chrome_framework_name.framework folder directly? Then we don't
need the widevine_copy_framework_signature step.

[jrummell, 2017-01-27 23:12:31]

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

[commit-bot: I haz the power, 2017-01-27 23:13:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jrummell, 2017-01-27 23:13:41]

Updated.

https://codereview.chromium.org/2647393004/diff/1/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2647393004/diff/1/chrome/BUILD.gn#newcode856
chrome/BUILD.gn:856: "$root_out_dir/libwidevinecdm.dylib",
On 2017/01/26 01:22:56, xhwang_slow wrote:
> Is it possible to just bundle the cdm/adapter sig files here, then we don't
need
> widevine_copy_cdm_library_signature (I guess).

Done. A little bit complicated since the .sigs are only there for chrome builds.

https://codereview.chromium.org/2647393004/diff/1/chrome/BUILD.gn#newcode866
chrome/BUILD.gn:866: ":widevine_cdm_library_copy",
On 2017/01/26 01:22:56, xhwang_slow wrote:
> I wonder whether we could make widevinecdm always a copy target so that we
don't
> need this hack, i.e. when !is_chrome_branded, the CDM will be generated by a
> shared_library(widevine_cdm_lib) target, followed by a target
copy(widevinecdm)
> which copies the file somewhere.

A future CL. cdmadapter would need to link to the correct object. I assume today
the real CDM is linked via ldflags, while the stub one gets linked since it's a
library. If that's the correct understanding, lots of cleanup needed there.

https://codereview.chromium.org/2647393004/diff/1/chrome/BUILD.gn#newcode895
chrome/BUILD.gn:895: # Bundle the signature files for Widevine and the
CdmAdapter.
On 2017/01/26 01:22:56, xhwang_slow wrote:
> nit: s/Widevine and the CdmAdapter/Widevine CDM and CDM adapter/

Removed.

https://codereview.chromium.org/2647393004/diff/1/chrome/BUILD.gn#newcode928
chrome/BUILD.gn:928: signature_file =
"$root_out_dir/signatures/widevinecdmadapter.plugin.sig"
On 2017/01/26 01:22:56, xhwang_slow wrote:
> Is it possible that we generate the sig file directly at
> $root_out_dir/$widevine_cdm_path/widevinecdmadapter.plugin.sig?

Done.

https://codereview.chromium.org/2647393004/diff/1/chrome/BUILD.gn#newcode982
chrome/BUILD.gn:982: signature_file =
"$root_out_dir/signatures/$chrome_framework_name.sig"
On 2017/01/26 01:22:56, xhwang_slow wrote:
> Is it possible to generate the framework sig file to the
> $root_out_dir/$chrome_framework_name.framework folder directly? Then we don't
> need the widevine_copy_framework_signature step.

Done.

[xhwang, 2017-01-28 00:29:38]

looking much better, I only have a few comments.

https://codereview.chromium.org/2647393004/diff/20001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2647393004/diff/20001/chrome/BUILD.gn#newcode908
chrome/BUILD.gn:908: action("widevine_generate_cdm_adapter_signature") {
It seems we'll use similar action in a lot of places. Does it make sense to have
a template to avoid duplication?

https://codereview.chromium.org/2647393004/diff/20001/chrome/BUILD.gn#newcode928
chrome/BUILD.gn:928: # Bundle the framework signature into the app. As the
framework can only
here and everywhere else, s/framework/chrome framework/. On MacOS "framework" is
a generic term similar to app or bundle. Here we are very specific, we only need
the sig for the "chrome framework".

https://codereview.chromium.org/2647393004/diff/20001/chrome/BUILD.gn#newcode944
chrome/BUILD.gn:944: action("widevine_generate_framework_signature") {
ditto about naming, how about

sign_chrome_framework_by_widevine

[jrummell, 2017-01-28 02:04:09]

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

[commit-bot: I haz the power, 2017-01-28 02:04:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jrummell, 2017-01-28 02:05:10]

Updated.

https://codereview.chromium.org/2647393004/diff/20001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2647393004/diff/20001/chrome/BUILD.gn#newcode908
chrome/BUILD.gn:908: action("widevine_generate_cdm_adapter_signature") {
On 2017/01/28 00:29:38, xhwang_slow wrote:
> It seems we'll use similar action in a lot of places. Does it make sense to
have
> a template to avoid duplication?

Done.

https://codereview.chromium.org/2647393004/diff/20001/chrome/BUILD.gn#newcode928
chrome/BUILD.gn:928: # Bundle the framework signature into the app. As the
framework can only
On 2017/01/28 00:29:37, xhwang_slow wrote:
> here and everywhere else, s/framework/chrome framework/. On MacOS "framework"
is
> a generic term similar to app or bundle. Here we are very specific, we only
need
> the sig for the "chrome framework".

Done.

https://codereview.chromium.org/2647393004/diff/20001/chrome/BUILD.gn#newcode944
chrome/BUILD.gn:944: action("widevine_generate_framework_signature") {
On 2017/01/28 00:29:37, xhwang_slow wrote:
> ditto about naming, how about
> 
> sign_chrome_framework_by_widevine

Done (as sign_chrome_framework_for_widevine).

[commit-bot: I haz the power, 2017-01-28 03:06:20]

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

[commit-bot: I haz the power, 2017-01-28 03:06:21]

Dry run: This issue passed the CQ dry run.

[xhwang, 2017-01-28 05:21:53]

lgtm % nits

https://codereview.chromium.org/2647393004/diff/40001/third_party/widevine/cd...
File third_party/widevine/cdm/widevine.gni (right):

https://codereview.chromium.org/2647393004/diff/40001/third_party/widevine/cd...
third_party/widevine/cdm/widevine.gni:10: template("widevine_sign_file") {
Please add a comment about what this template is doing.

https://codereview.chromium.org/2647393004/diff/40001/third_party/widevine/cd...
third_party/widevine/cdm/widevine.gni:15: "deps",
Is "deps" needed? Seems not used in this template.

[Greg K, 2017-01-31 02:42:14]

kerrnel@chromium.org changed reviewers:
+ kerrnel@chromium.org

[Greg K, 2017-01-31 02:42:15]

I decided to give this CL a try and hit the following error:

kerrnel$ ninja -C out/Official -j 1000 -l 13 chrome
ninja: Entering directory `out/Official'
[1/1] Regenerating ninja files
ninja: error: '../../third_party/widevine/cdm/mac/x64/libwidevinecdm.dylib.sig',
needed by 'WidevineCdm/_platform_specific/mac_x64/libwidevinecdm.dylib.sig',
missing and no known rule to make it

[xhwang, 2017-01-31 06:08:29]

On 2017/01/31 02:42:15, Greg K wrote:
> I decided to give this CL a try and hit the following error:
> 
> kerrnel$ ninja -C out/Official -j 1000 -l 13 chrome
> ninja: Entering directory `out/Official'
> [1/1] Regenerating ninja files
> ninja: error:
'../../third_party/widevine/cdm/mac/x64/libwidevinecdm.dylib.sig',
> needed by 'WidevineCdm/_platform_specific/mac_x64/libwidevinecdm.dylib.sig',
> missing and no known rule to make it

Thanks for trying. The CDM sig files are not checked in yet, which is blocking
the landing of this CL.

[Greg K, 2017-01-31 10:06:04]

On 2017/01/31 06:08:29, xhwang_slow wrote:
> On 2017/01/31 02:42:15, Greg K wrote:
> > I decided to give this CL a try and hit the following error:
> > 
> > kerrnel$ ninja -C out/Official -j 1000 -l 13 chrome
> > ninja: Entering directory `out/Official'
> > [1/1] Regenerating ninja files
> > ninja: error:
> '../../third_party/widevine/cdm/mac/x64/libwidevinecdm.dylib.sig',
> > needed by 'WidevineCdm/_platform_specific/mac_x64/libwidevinecdm.dylib.sig',
> > missing and no known rule to make it
> 
> Thanks for trying. The CDM sig files are not checked in yet, which is blocking
> the landing of this CL.

Thanks for the information.

Have you tried an official, Chrome branded build locally with this CL? I have
tried to do a clean build twice and both times it produces a Chromium branded
build instead of a Google Chrome branded builded. My gn args are:

use_goma = true
enable_nacl = false
is_component_build = false
is_chrome_branded = true
is_official_build = true

If I build without the CL it produces Google Chrome.app just fine. Does anyone
else see this behavior?

- Greg

[Robert Sesek, 2017-01-31 18:40:17]

rsesek@chromium.org changed reviewers:
+ rsesek@chromium.org

[Robert Sesek, 2017-01-31 18:40:18]

https://codereview.chromium.org/2647393004/diff/60001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2647393004/diff/60001/chrome/BUILD.gn#newcode924
chrome/BUILD.gn:924:
"{{bundle_root_dir}}/Versions/$chrome_version_full/$chrome_framework_name.framework/{{source_file_part}}",
I do not think this is going to work. The way bundle_data works means that this
rule will not be executed unless something causes chrome_app to need a rebuild.
If only something in chrome_framework changes, the Versions/X folder will be
removed and re-copied, and this rule will not re-run. This will cause the build
to fail to stabilize, which is not permitted.

You can test this like so:

% ninja -C out/release chrome
% touch out/release/Chromium\ Framework.framework/Chromium\ Framework
% ninja -C out/release chrome -v
% ls -l out/release/Chromium.app/Contents/Versions/Chromium\ Framework
# note that the file is not there
% ninja -C out/release chrome -v
# build does more stuff, not allowed because you chrome should have been built
completely at the last ninja invocation

[jrummell, 2017-01-31 22:29:18]

Greg K: I've been building this locally for a while. To do that I've created a
dummy .sig file for widevine (touch
third_party/widevine/cdm/mac/x64/libwidevinecdm.dylib.sig). My GN args are:

is_chrome_branded = true
is_debug = false
is_official_build  = true
use_goma = true

I tried adding enable_nacl = false and is_component_build = false, and after
running gn clean out/Release, a full build produces a Google Chrome.app for me.
Not sure why it doesn't work for you. I'll try to find another Mac and build
there.

I am following up with rsesek@ to find the correct way to generate the framework
signature.

https://codereview.chromium.org/2647393004/diff/40001/third_party/widevine/cd...
File third_party/widevine/cdm/widevine.gni (right):

https://codereview.chromium.org/2647393004/diff/40001/third_party/widevine/cd...
third_party/widevine/cdm/widevine.gni:10: template("widevine_sign_file") {
On 2017/01/28 05:21:53, xhwang_slow wrote:
> Please add a comment about what this template is doing.

Done.

https://codereview.chromium.org/2647393004/diff/40001/third_party/widevine/cd...
third_party/widevine/cdm/widevine.gni:15: "deps",
On 2017/01/28 05:21:53, xhwang_slow wrote:
> Is "deps" needed? Seems not used in this template.

We need the generated action to have a deps so it gets built in the correct
order.

[jrummell, 2017-02-02 23:04:55]

Updated.

https://codereview.chromium.org/2647393004/diff/60001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2647393004/diff/60001/chrome/BUILD.gn#newcode924
chrome/BUILD.gn:924:
"{{bundle_root_dir}}/Versions/$chrome_version_full/$chrome_framework_name.framework/{{source_file_part}}",
On 2017/01/31 18:40:18, Robert Sesek wrote:
> I do not think this is going to work. The way bundle_data works means that
this
> rule will not be executed unless something causes chrome_app to need a
rebuild.
> If only something in chrome_framework changes, the Versions/X folder will be
> removed and re-copied, and this rule will not re-run. This will cause the
build
> to fail to stabilize, which is not permitted.
> 
> You can test this like so:
> 
> % ninja -C out/release chrome
> % touch out/release/Chromium\ Framework.framework/Chromium\ Framework
> % ninja -C out/release chrome -v
> % ls -l out/release/Chromium.app/Contents/Versions/Chromium\ Framework
> # note that the file is not there
> % ninja -C out/release chrome -v
> # build does more stuff, not allowed because you chrome should have been built
> completely at the last ninja invocation

Updated. First -v build does sign the framework again (step 4/23), second -v
build says nothing to do.

[Robert Sesek, 2017-02-07 21:45:04]

LGTM

[jrummell, 2017-02-08 22:38:07]

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

[jrummell, 2017-02-08 22:38:08]

+dpranke@ for OWNERS review of chrome/BUILD.gn

[Dirk Pranke, 2017-02-09 01:22:43]

lgtm

[jrummell, 2017-02-11 01:11:50]

Thanks for the reviews. I'm waiting to submit this until the Widevine CDM
signature file actually gets checked in.

[Robert Sesek, 2017-02-17 18:55:08]

FYI you may want to re-check this now that we are versioning the Mac framework
as part of https://crbug.com/662466

[jrummell, 2017-02-18 01:23:38]

Updated to take in the latest version changes.

rsesek@, can you take another look at the chrome/BUILD.gn changes. The
$root_dir/framework.sig file is generated after the framework is built, and then
a new step copies it directly into the app's Versions/58.0.3016.0/Google Chrome
Framework.framework/Versions/A folder.

[jrummell, 2017-02-18 01:23:46]

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

[commit-bot: I haz the power, 2017-02-18 01:25:11]

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-02-18 03:27:38]

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

[commit-bot: I haz the power, 2017-02-18 03:27:39]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[xhwang, 2017-02-18 03:28:06]

The CQ bit was checked by xhwang@chromium.org

[xhwang, 2017-02-18 03:28:06]

The patchset sent to the CQ was uploaded after l-g-t-m from xhwang@chromium.org,
rsesek@chromium.org, dpranke@chromium.org
Link to the patchset: https://codereview.chromium.org/2647393004/#ps100001
(title: "rebase with version changes")

[commit-bot: I haz the power, 2017-02-18 03:28:49]

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

[Robert Sesek, 2017-02-18 03:39:03]

The CQ bit was unchecked by rsesek@chromium.org

[Robert Sesek, 2017-02-21 16:15:00]

https://codereview.chromium.org/2647393004/diff/100001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2647393004/diff/100001/chrome/BUILD.gn#newcod...
chrome/BUILD.gn:663: final_directory =
"$root_out_dir/$chrome_product_full_name.app/Contents/Versions/$chrome_version_full/$chrome_framework_name.framework/"
I think this is just re-introducing the issue I raised initially: you cannot
copy the sig file into the framework after it has been packaged. You shouldn't
need to change where the .sig file lives from PS5. The only real difference is
that the input to widevine_sign_file is now a symlink.

[jrummell, 2017-02-21 19:54:56]

Comments only. I'll update the CL shortly.

https://codereview.chromium.org/2647393004/diff/100001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2647393004/diff/100001/chrome/BUILD.gn#newcod...
chrome/BUILD.gn:663: final_directory =
"$root_out_dir/$chrome_product_full_name.app/Contents/Versions/$chrome_version_full/$chrome_framework_name.framework/"
On 2017/02/21 16:15:00, Robert Sesek wrote:
> I think this is just re-introducing the issue I raised initially: you cannot
> copy the sig file into the framework after it has been packaged. You shouldn't
> need to change where the .sig file lives from PS5. The only real difference is
> that the input to widevine_sign_file is now a symlink.

I tried that earlier. With "sources += [
"$root_out_dir/$chrome_framework_name.sig" ]", the signature file is copied into
Contents/Versions/58.0.3016.0/Google Chrome Framework.framework" (actual
framework is 2 levels down in Versions/A). My concern was that this didn't
handle different versions, but based on your comment in email, it looks like
this won't be an issue. I'll make that change.

[Robert Sesek, 2017-02-21 22:26:48]

https://codereview.chromium.org/2647393004/diff/100001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2647393004/diff/100001/chrome/BUILD.gn#newcod...
chrome/BUILD.gn:663: final_directory =
"$root_out_dir/$chrome_product_full_name.app/Contents/Versions/$chrome_version_full/$chrome_framework_name.framework/"
On 2017/02/21 19:54:56, jrummell wrote:
> On 2017/02/21 16:15:00, Robert Sesek wrote:
> > I think this is just re-introducing the issue I raised initially: you cannot
> > copy the sig file into the framework after it has been packaged. You
shouldn't
> > need to change where the .sig file lives from PS5. The only real difference
is
> > that the input to widevine_sign_file is now a symlink.
> 
> I tried that earlier. With "sources += [
> "$root_out_dir/$chrome_framework_name.sig" ]", the signature file is copied
into
> Contents/Versions/58.0.3016.0/Google Chrome Framework.framework" (actual

Shouldn't it be copied into Contents/Versions/58.0.3016.0/ to live next to the
.framework?

> framework is 2 levels down in Versions/A). My concern was that this didn't
> handle different versions, but based on your comment in email, it looks like
> this won't be an issue. I'll make that change.

Because Chrome externally versions its frameworks
(Chrome.app/Contents/Versions/X), there will definitely only ever be one version
in the Google Chrome Framework.framework per outer version directory.

[jrummell, 2017-02-22 01:02:07]

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

[jrummell, 2017-02-22 01:02:08]

Updated to move the framework.sig file back.

+sky@ for OWNERS review of chrome/common/media/cdm_host_file_path.cc (trivial
path change).

I'll also wait for rsesek@'s re-approval.

https://codereview.chromium.org/2647393004/diff/100001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2647393004/diff/100001/chrome/BUILD.gn#newcod...
chrome/BUILD.gn:663: final_directory =
"$root_out_dir/$chrome_product_full_name.app/Contents/Versions/$chrome_version_full/$chrome_framework_name.framework/"
On 2017/02/21 22:26:48, Robert Sesek wrote:
> On 2017/02/21 19:54:56, jrummell wrote:
> > On 2017/02/21 16:15:00, Robert Sesek wrote:
> > > I think this is just re-introducing the issue I raised initially: you
cannot
> > > copy the sig file into the framework after it has been packaged. You
> shouldn't
> > > need to change where the .sig file lives from PS5. The only real
difference
> is
> > > that the input to widevine_sign_file is now a symlink.
> > 
> > I tried that earlier. With "sources += [
> > "$root_out_dir/$chrome_framework_name.sig" ]", the signature file is copied
> into
> > Contents/Versions/58.0.3016.0/Google Chrome Framework.framework" (actual
> 
> Shouldn't it be copied into Contents/Versions/58.0.3016.0/ to live next to the
> .framework?

Yes. I had the directories all expanded in the Finder view and didn't match the
indentation properly.

> > framework is 2 levels down in Versions/A). My concern was that this didn't
> > handle different versions, but based on your comment in email, it looks like
> > this won't be an issue. I'll make that change.
> 
> Because Chrome externally versions its frameworks
> (Chrome.app/Contents/Versions/X), there will definitely only ever be one
version
> in the Google Chrome Framework.framework per outer version directory.

Good to know. Added comment in the code.

[sky, 2017-02-22 17:00:07]

https://codereview.chromium.org/2647393004/diff/120001/chrome/common/media/cd...
File chrome/common/media/cdm_host_file_path.cc (right):

https://codereview.chromium.org/2647393004/diff/120001/chrome/common/media/cd...
chrome/common/media/cdm_host_file_path.cc:83: // the actual framework
executable. As the framework is now internally
Isn't there a better way to get the location rather than assuming you have to go
up three directories? Maybe DIR_EXE?

[jrummell, 2017-02-22 23:14:51]

With the symlinks run-time thinks the framework is in the directory it was prior
to the version changes. So I've removed the change in cdm_host_file_path, as
it's not needed.

This PS includes a rebase, so all the changes shown are due to the rebase (as
cdm_host_file_path.cc no longer included).

sky@, looks like your review is no longer necessary.

https://codereview.chromium.org/2647393004/diff/120001/chrome/common/media/cd...
File chrome/common/media/cdm_host_file_path.cc (right):

https://codereview.chromium.org/2647393004/diff/120001/chrome/common/media/cd...
chrome/common/media/cdm_host_file_path.cc:83: // the actual framework
executable. As the framework is now internally
On 2017/02/22 17:00:07, sky wrote:
> Isn't there a better way to get the location rather than assuming you have to
go
> up three directories? Maybe DIR_EXE?

It turns out that with the symlinks added it still thinks it's in the original
directory, so I've reverted this change.

[sky, 2017-02-23 00:56:56]

Description was changed from

==========
Add Widevine signature files into Mac build

On official Mac builds Widevine is included in the app. This change adds
the matching signature files needed by Widevine.

BUG=684871
TEST=signature files created in the correct place when is_official_build
==========

to

==========
Add Widevine signature files into Mac build

On official Mac builds Widevine is included in the app. This change adds
the matching signature files needed by Widevine.

BUG=684871
TEST=signature files created in the correct place when is_official_build
==========

[sky, 2017-02-23 00:56:56]

sky@chromium.org changed reviewers:
- sky@chromium.org

[sky, 2017-02-23 00:57:05]

-sky then

[Robert Sesek, 2017-02-23 18:09:32]

This LG, just one question:

https://codereview.chromium.org/2647393004/diff/140001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2647393004/diff/140001/chrome/BUILD.gn#newcod...
chrome/BUILD.gn:920: if (defined(chrome_framework_version)) {
Can the script that produces the signature file go through symlink at
"$root_out_dir/$chrome_framework_name.framework/$chrome_framework_name" ?

[jrummell, 2017-02-23 18:39:03]

https://codereview.chromium.org/2647393004/diff/140001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2647393004/diff/140001/chrome/BUILD.gn#newcod...
chrome/BUILD.gn:920: if (defined(chrome_framework_version)) {
On 2017/02/23 18:09:32, Robert Sesek wrote:
> Can the script that produces the signature file go through symlink at
> "$root_out_dir/$chrome_framework_name.framework/$chrome_framework_name" ? 

I tried it, and get the error:
ERROR Input to target not generated by a dependency.
The file:
  //out/Release/Google Chrome Framework.framework/Google Chrome Framework
is listed as an input or source for the target:
  //chrome:sign_chrome_framework_for_widevine
but no targets in the build generate that file.

The outputs for ":chrome_framework" include:
//out/Release/Google Chrome Framework.framework/Versions/A/Google Chrome
Framework

I don't see anything in this file that creates the symlink. Where is that done?

[Robert Sesek, 2017-02-23 18:54:49]

LGTM

https://codereview.chromium.org/2647393004/diff/140001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2647393004/diff/140001/chrome/BUILD.gn#newcod...
chrome/BUILD.gn:920: if (defined(chrome_framework_version)) {
On 2017/02/23 18:39:03, jrummell wrote:
> On 2017/02/23 18:09:32, Robert Sesek wrote:
> > Can the script that produces the signature file go through symlink at
> > "$root_out_dir/$chrome_framework_name.framework/$chrome_framework_name" ? 
> 
> I tried it, and get the error:
> ERROR Input to target not generated by a dependency.
> The file:
>   //out/Release/Google Chrome Framework.framework/Google Chrome Framework
> is listed as an input or source for the target:
>   //chrome:sign_chrome_framework_for_widevine
> but no targets in the build generate that file.
> 
> The outputs for ":chrome_framework" include:
> //out/Release/Google Chrome Framework.framework/Versions/A/Google Chrome
> Framework
> 
> I don't see anything in this file that creates the symlink. Where is that
done?

Ah, that makes sense. There's an issue with ninja where it does not properly
stat links (it uses stat instead of lstat), so you cannot actually have a build
edge be a symbolic link. To get around that, those symlinks are actually
generated at gn gen time, since that's the only reliable way to do it.

[jrummell, 2017-02-23 19:10:56]

The CQ bit was checked by jrummell@chromium.org

[jrummell, 2017-02-23 19:10:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, xhwang@chromium.org
Link to the patchset: https://codereview.chromium.org/2647393004/#ps140001
(title: "changes (plus rebase)")

[commit-bot: I haz the power, 2017-02-23 19:11:26]

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-02-23 20:19:11]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1487877056044600,
"parent_rev": "3d7b06a12ea2897550ef82afef840a06501ab544", "commit_rev":
"5762e61a17e4e05c490be128fd96204f24c1a457"}

[commit-bot: I haz the power, 2017-02-23 20:19:42]

Description was changed from

==========
Add Widevine signature files into Mac build

On official Mac builds Widevine is included in the app. This change adds
the matching signature files needed by Widevine.

BUG=684871
TEST=signature files created in the correct place when is_official_build
==========

to

==========
Add Widevine signature files into Mac build

On official Mac builds Widevine is included in the app. This change adds
the matching signature files needed by Widevine.

BUG=684871
TEST=signature files created in the correct place when is_official_build

Review-Url: https://codereview.chromium.org/2647393004
Cr-Commit-Position: refs/heads/master@{#452603}
Committed:
https://chromium.googlesource.com/chromium/src/+/5762e61a17e4e05c490be128fd96...
==========

[commit-bot: I haz the power, 2017-02-23 20:19:44]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/5762e61a17e4e05c490be128fd96...

[Robert Sesek, 2017-02-24 15:38:05]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/2714643009/ by rsesek@chromium.org.

The reason for reverting is: Broke official build https://crbug.com/695761.