media: Mark Widevine CDM and manifest as data_deps of the adapter

media: Mark Widevine CDM and manifest as data_deps of the adapter

The CDM is needed by the adapter at link time and at run time. The
manifest.json file is only required at run time. To make it always
available at run time (e.g. for isolated tests), we should mark them
as data_deps.

Note that shared libraries are automatically treated as runtime_deps.
Therefore, we don't need to do this for clearkeycdm.

There are two cases that's worth noting but I am not treating them
separately for the sake of simplicity:
1. In theory, we don't need this for the stub CDM because it's also
   a shared library.
2. On Windows, this will list widevinecdm.dll.lib as runtime_deps
   which isn't necessary.

BUG=614745
TEST=This should fix isolated browser_tests on official GN bots.

Committed: https://crrev.com/0f82503faa7ebe8efa43903908fda396ea8f40b9
Cr-Commit-Position: refs/heads/master@{#397324}

[xhwang, 2016-06-01 19:39:18]

With this CL

gn desc out/GN //chrome/test:browser_tests runtime_deps

shows

On Windows:

ClearKeyCdm/_platform_specific/win_x64/clearkeycdmadapter.dll
ClearKeyCdm/_platform_specific/win_x64/clearkeycdm.dll
WidevineCdm/_platform_specific/win_x64/widevinecdmadapter.dll
WidevineCdm/manifest.json
WidevineCdm/_platform_specific/win_x64/widevinecdm.dll
WidevineCdm/_platform_specific/win_x64/widevinecdm.dll.lib

On Linux:

./libclearkeycdmadapter.so
./libclearkeycdm.so
./libwidevinecdmadapter.so
libwidevinecdm.so

(note that we don't need manifest on Linux)

[xhwang, 2016-06-01 19:40:30]

xhwang@chromium.org changed reviewers:
+ ddorwin@chromium.org, thakis@chromium.org

[xhwang, 2016-06-01 19:40:30]

PTAL

[Nico, 2016-06-01 19:43:34]

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

[Nico, 2016-06-01 19:43:34]

Looks good to me, but I'm always confused by data_deps, so +dpranke :-)

I assume you did test that the test fails without this and passes with it if you
locally run it via mb run.

[xhwang, 2016-06-01 19:48:35]

On 2016/06/01 19:43:34, Nico wrote:
> Looks good to me, but I'm always confused by data_deps, so +dpranke :-)
> 
> I assume you did test that the test fails without this and passes with it if
you
> locally run it via mb run.

For the record, this is one of the suggested solutions in "gn help
runtime_deps":

Under "Actions and copies":
  - Have B list the action in both deps and data deps (if the outputs
    might be used in both contexts and you don't care about unnecessary
    entries in the list of files required at runtime).

For the testing part, I am still trying to get mb run working. Here's what I get
following the instructions at
https://www.chromium.org/developers/testing/isolated-testing/for-swes:

~/chrome2/src$ echo gn > out/GN/mb_type
~/chrome2/src$ tools/mb/mb.py run //out/GN browser_tests
ninja: Entering directory `out/GN'
[21899/21899] LINK ./browser_tests
python tools/swarming_client/isolate.py check -i out/GN/browser_tests.isolate -s
out/GN/browser_tests.isolated
Failed to start Xvfb or Openbox or xcompmgr: [Errno 2] No such file or directory

dpranke: Anything obvious to you before I dive deeper?

[xhwang, 2016-06-01 19:53:37]

On 2016/06/01 19:48:35, xhwang wrote:
> On 2016/06/01 19:43:34, Nico wrote:
> > Looks good to me, but I'm always confused by data_deps, so +dpranke :-)
> > 
> > I assume you did test that the test fails without this and passes with it if
> you
> > locally run it via mb run.
> 
> For the record, this is one of the suggested solutions in "gn help
> runtime_deps":
> 
> Under "Actions and copies":
>   - Have B list the action in both deps and data deps (if the outputs
>     might be used in both contexts and you don't care about unnecessary
>     entries in the list of files required at runtime).
> 
> For the testing part, I am still trying to get mb run working. Here's what I
get
> following the instructions at
> https://www.chromium.org/developers/testing/isolated-testing/for-swes:
> 
> ~/chrome2/src$ echo gn > out/GN/mb_type
> ~/chrome2/src$ tools/mb/mb.py run //out/GN browser_tests
> ninja: Entering directory `out/GN'
> [21899/21899] LINK ./browser_tests
> python tools/swarming_client/isolate.py check -i out/GN/browser_tests.isolate
-s
> out/GN/browser_tests.isolated
> Failed to start Xvfb or Openbox or xcompmgr: [Errno 2] No such file or
directory
> 
> dpranke: Anything obvious to you before I dive deeper?

BTW, I did verify that libwidevinecdm.so isn't in browser_tests.isolate without
this CL, but it is there with this CL.

[ddorwin, 2016-06-01 20:16:43]

rs LGMT

https://chromiumcodereview.appspot.com/2027373002/diff/1/third_party/widevine...
File third_party/widevine/cdm/BUILD.gn (right):

https://chromiumcodereview.appspot.com/2027373002/diff/1/third_party/widevine...
third_party/widevine/cdm/BUILD.gn:151: ":widevinecdm",
deps doesn't imply data_deps?

[ddorwin, 2016-06-01 20:17:44]

rs LGTM

[xhwang, 2016-06-01 20:24:12]

https://chromiumcodereview.appspot.com/2027373002/diff/1/third_party/widevine...
File third_party/widevine/cdm/BUILD.gn (right):

https://chromiumcodereview.appspot.com/2027373002/diff/1/third_party/widevine...
third_party/widevine/cdm/BUILD.gn:151: ":widevinecdm",
On 2016/06/01 20:16:43, ddorwin wrote:
> deps doesn't imply data_deps?

Apparently not based on "gn help runtime_deps" unless it's a shared library:

  To a first approximation, the runtime dependencies of a target are
  the set of "data" files, data directories, and the shared libraries
  from all transitive dependencies.

[Dirk Pranke, 2016-06-01 21:33:16]

lgtm.

https://chromiumcodereview.appspot.com/2027373002/diff/1/third_party/widevine...
File third_party/widevine/cdm/BUILD.gn (right):

https://chromiumcodereview.appspot.com/2027373002/diff/1/third_party/widevine...
third_party/widevine/cdm/BUILD.gn:151: ":widevinecdm",
On 2016/06/01 20:24:12, xhwang wrote:
> On 2016/06/01 20:16:43, ddorwin wrote:
> > deps doesn't imply data_deps?
> 
> Apparently not based on "gn help runtime_deps" unless it's a shared library:
> 
>   To a first approximation, the runtime dependencies of a target are
>   the set of "data" files, data directories, and the shared libraries
>   from all transitive dependencies.

Correct, `deps` does not imply `data_deps`. You might depend on `protoc` in
order to generate the proto buffer stubs, but that doesn't mean you need the
protocol compiler itself to be present at runtime.

[xhwang, 2016-06-02 05:44:54]

okay, I confirmed that the isotated test fails on Windows GN build without this
CL, and it passes with this CL.

Here's how I run it:

D:\src\chrome2\src>tools\mb\mb.bat run out\GN browser_tests --
--gtest_filter=PepperContentSettingsSpecialCasesPluginsBl
ockedTest.WidevineCdm

Note that my debug build on Windows is super slow. So the test kept timing out.
I tried  --test-launcher-timeout=30000000 but it doesn't work for some reason. I
have to hack base/test/test_timeouts.cc to manually hardcode a larger number of
time out to get the test pass.
                                                                

[xhwang, 2016-06-02 05:45:21]

The CQ bit was checked by xhwang@chromium.org

[commit-bot: I haz the power, 2016-06-02 05:45:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2027373002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2027373002/1

[commit-bot: I haz the power, 2016-06-02 07:02:32]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-06-02 07:04:09]

Description was changed from

==========
media: Mark Widevine CDM and manifest as data_deps of the adapter

The CDM is needed by the adapter at link time and at run time. The
manifest.json file is only required at run time. To make it always
available at run time (e.g. for isolated tests), we should mark them
as data_deps.

Note that shared libraries are automatically treated as runtime_deps.
Therefore, we don't need to do this for clearkeycdm.

There are two cases that's worth noting but I am not treating them
separately for the sake of simplicity:
1. In theory, we don't need this for the stub CDM because it's also
   a shared library.
2. On Windows, this will list widevinecdm.dll.lib as runtime_deps
   which isn't necessary.

BUG=614745
TEST=This should fix isolated browser_tests on official GN bots.
==========

to

==========
media: Mark Widevine CDM and manifest as data_deps of the adapter

The CDM is needed by the adapter at link time and at run time. The
manifest.json file is only required at run time. To make it always
available at run time (e.g. for isolated tests), we should mark them
as data_deps.

Note that shared libraries are automatically treated as runtime_deps.
Therefore, we don't need to do this for clearkeycdm.

There are two cases that's worth noting but I am not treating them
separately for the sake of simplicity:
1. In theory, we don't need this for the stub CDM because it's also
   a shared library.
2. On Windows, this will list widevinecdm.dll.lib as runtime_deps
   which isn't necessary.

BUG=614745
TEST=This should fix isolated browser_tests on official GN bots.

Committed: https://crrev.com/0f82503faa7ebe8efa43903908fda396ea8f40b9
Cr-Commit-Position: refs/heads/master@{#397324}
==========

[commit-bot: I haz the power, 2016-06-02 07:04:10]

Patchset 1 (id:??) landed as
https://crrev.com/0f82503faa7ebe8efa43903908fda396ea8f40b9
Cr-Commit-Position: refs/heads/master@{#397324}