openh264_unittests empty dummy test added

openh264_unittests empty dummy test added.

This will enable trybots to run this new test binary (openh264_unittests) for a future CL that adds actual test code and more to it.
In the follow-up CL OpenH264 will be pulled in (adding third_party/openh264/src) and build files and test code will be added. That CL needs the trybots to run its test code.

BUG=500605
Committed: https://crrev.com/e409ed4bb768e07371b5e4ebcf107c023796ac57
Cr-Commit-Position: refs/heads/master@{#353769}

[hbos_chromium, 2015-10-09 13:41:38]

hbos@chromium.org changed reviewers:
+ dpranke@chromium.org, phoglund@chromium.org

[hbos_chromium, 2015-10-09 13:41:38]

PTAL phoglund, dpranke.

[phoglund_chromium, 2015-10-09 13:54:32]

lgtm

[Dirk Pranke, 2015-10-09 19:05:12]

You should have changes to BUILD.gn files as well ..

[hbos_chromium, 2015-10-12 15:30:25]

PTAL dpranke.

Forgot the GN, added. Can you lgtm assuming I fix the "webview_licenses" failure
and there are no other issues?

I'll add the missing README.chromium and ask phoglund to review again before
landing (thought this file was not needed until I pulled in the source but a
trybot is red).
Will fix this tomorrow but due to time zone differences it would be nice if you
could review since this is blocking other work that I want to have in 48.

[Dirk Pranke, 2015-10-12 18:28:50]

lgtm w/ changes and green bots :).

https://codereview.chromium.org/1394983003/diff/20001/build/all.gyp
File build/all.gyp (right):

https://codereview.chromium.org/1394983003/diff/20001/build/all.gyp#newcode33
build/all.gyp:33: '../third_party/openh264/tests/openh264_unittests.gyp:*',
Please add openh264_unittests to the 'both_gn_and_gyp' target in
//build/gn_migration.gypi as well.

https://codereview.chromium.org/1394983003/diff/20001/third_party/openh264/te...
File third_party/openh264/tests/BUILD.gn (right):

https://codereview.chromium.org/1394983003/diff/20001/third_party/openh264/te...
third_party/openh264/tests/BUILD.gn:9: "//",
"//" should be unneeded (GN adds it automatically).

[hbos_chromium, 2015-10-13 10:21:04]

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

[commit-bot: I haz the power, 2015-10-13 10:21:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1394983003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1394983003/40001

[hbos_chromium, 2015-10-13 10:21:42]

PTAL phoglund

https://codereview.chromium.org/1394983003/diff/20001/build/all.gyp
File build/all.gyp (right):

https://codereview.chromium.org/1394983003/diff/20001/build/all.gyp#newcode33
build/all.gyp:33: '../third_party/openh264/tests/openh264_unittests.gyp:*',
On 2015/10/12 18:28:50, Dirk Pranke wrote:
> Please add openh264_unittests to the 'both_gn_and_gyp' target in
> //build/gn_migration.gypi as well.

Done.

https://codereview.chromium.org/1394983003/diff/20001/third_party/openh264/te...
File third_party/openh264/tests/BUILD.gn (right):

https://codereview.chromium.org/1394983003/diff/20001/third_party/openh264/te...
third_party/openh264/tests/BUILD.gn:9: "//",
On 2015/10/12 18:28:50, Dirk Pranke wrote:
> "//" should be unneeded (GN adds it automatically).

I believe they are necessary. Getting can't load input files error if I remove
them (or error for using an empty string in that case). No "//" meaning relative
to current folder, and "//" meaning relative to root directory.

https://codereview.chromium.org/1394983003/diff/40001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1394983003/diff/40001/BUILD.gn#newcode13
BUILD.gn:13: import("//build/config/sanitizers/sanitizers.gni")
(Changes in this file are due to rebase, sry!)

[commit-bot: I haz the power, 2015-10-13 12:55:19]

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

[commit-bot: I haz the power, 2015-10-13 12:55:20]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_r...)

[kjellander_chromium, 2015-10-13 14:18:25]

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

[kjellander_chromium, 2015-10-13 14:18:25]

lgtm with a question (actually two)

https://codereview.chromium.org/1394983003/diff/60001/third_party/openh264/te...
File third_party/openh264/tests/BUILD.gn (right):

https://codereview.chromium.org/1394983003/diff/60001/third_party/openh264/te...
third_party/openh264/tests/BUILD.gn:9: "//",
Is the root really needed? I thought it was implicit

https://codereview.chromium.org/1394983003/diff/60001/third_party/openh264/te...
File third_party/openh264/tests/openh264_unittests.gyp (right):

https://codereview.chromium.org/1394983003/diff/60001/third_party/openh264/te...
third_party/openh264/tests/openh264_unittests.gyp:11: '<(DEPTH)',
Same here, is the root really needed?

[hbos_chromium, 2015-10-13 14:43:11]

The CQ bit was checked by hbos@chromium.org

[hbos_chromium, 2015-10-13 14:43:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
phoglund@chromium.org, dpranke@chromium.org
Link to the patchset: https://codereview.chromium.org/1394983003/#ps60001
(title: "LICENSE file to satisfy webview_licenses.py (will be replaced by
third_party/openh264/src/LICENSE when future CL pulls in src)")

[commit-bot: I haz the power, 2015-10-13 14:44:38]

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

[hbos_chromium, 2015-10-13 14:45:47]

On 2015/10/13 14:18:25, kjellander (chromium) wrote:
> lgtm with a question (actually two)
> 
>
https://codereview.chromium.org/1394983003/diff/60001/third_party/openh264/te...
> File third_party/openh264/tests/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1394983003/diff/60001/third_party/openh264/te...
> third_party/openh264/tests/BUILD.gn:9: "//",
> Is the root really needed? I thought it was implicit
> 
>
https://codereview.chromium.org/1394983003/diff/60001/third_party/openh264/te...
> File third_party/openh264/tests/openh264_unittests.gyp (right):
> 
>
https://codereview.chromium.org/1394983003/diff/60001/third_party/openh264/te...
> third_party/openh264/tests/openh264_unittests.gyp:11: '<(DEPTH)',
> Same here, is the root really needed?

I believe so, I get compile errors if I don't specify the root, at least in the
GN file.
But I'll try again and see if I can get rid of them in the follow-up CL, in case
I missed something.

[commit-bot: I haz the power, 2015-10-13 16:27:55]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-13 16:45:32]

Patchset 4 (id:??) landed as
https://crrev.com/e409ed4bb768e07371b5e4ebcf107c023796ac57
Cr-Commit-Position: refs/heads/master@{#353769}

[Dirk Pranke, 2015-10-13 20:21:56]

On 2015/10/13 14:45:47, hbos_chromium wrote:
> On 2015/10/13 14:18:25, kjellander (chromium) wrote:
> > lgtm with a question (actually two)
> > 
> >
>
https://codereview.chromium.org/1394983003/diff/60001/third_party/openh264/te...
> > File third_party/openh264/tests/BUILD.gn (right):
> > 
> >
>
https://codereview.chromium.org/1394983003/diff/60001/third_party/openh264/te...
> > third_party/openh264/tests/BUILD.gn:9: "//",
> > Is the root really needed? I thought it was implicit
> > 
> >
>
https://codereview.chromium.org/1394983003/diff/60001/third_party/openh264/te...
> > File third_party/openh264/tests/openh264_unittests.gyp (right):
> > 
> >
>
https://codereview.chromium.org/1394983003/diff/60001/third_party/openh264/te...
> > third_party/openh264/tests/openh264_unittests.gyp:11: '<(DEPTH)',
> > Same here, is the root really needed?
> 
> I believe so, I get compile errors if I don't specify the root, at least in
the
> GN file.
> But I'll try again and see if I can get rid of them in the follow-up CL, in
case
> I missed something.

Yes, please do this (and CC me on the CL). 

The "//" directory is added by default in  the
//build/config/compiler:default_include_dirs config, which is part
of the native_compiler_configs variable in //build/config/BUILDCONFIG.gn, which
should be applied to all
targets by default.

If you're not seeing this get added automatically, there's something wrong.

[brettw, 2015-11-23 23:51:08]

Did you follow http://www.chromium.org/developers/adding-3rd-party-libraries ? I
don't see any third_party OWNERS on the review.

[Dirk Pranke, 2015-11-24 00:20:03]

On 2015/11/23 23:51:08, brettw wrote:
> Did you follow http://www.chromium.org/developers/adding-3rd-party-libraries ?
I
> don't see any third_party OWNERS on the review.

argh. yeah, good question. Normally chromium_presubmit would complain, but in
this case it saw that the
CL was being added by one of the OWNERS in the directory, and thought that was
okay.

[hbos_chromium, 2015-11-24 09:12:33]

On 2015/11/24 00:20:03, Dirk Pranke wrote:
> On 2015/11/23 23:51:08, brettw wrote:
> > Did you follow http://www.chromium.org/developers/adding-3rd-party-libraries
?
> I
> > don't see any third_party OWNERS on the review.
> 
> argh. yeah, good question. Normally chromium_presubmit would complain, but in
> this case it saw that the
> CL was being added by one of the OWNERS in the directory, and thought that was
> okay.

Oh, good catch. I did look at
https://www.chromium.org/developers/adding-3rd-party-libraries but for some
reason I was thinking the security review would occur at a later stage in the
process (after several follow-up CLs) and I guess I didn't look too carefully at
the "Get a Review" section. It was momentarily ignored but not forgotten. My
bad, I'll get right to it.