GYP->GN of audio_device_tests. Remove dead code. Fix new warnings.

GYP->GN of audio_device_tests. Remove dead code. Fix new warnings.

Some old TODOs were fixed while fixing the new warnings from using GN.

BUG=webrtc:6170, webrtc:5949

[Max Morin WebRTC, 2016-08-04 15:17:15]

The CQ bit was checked by maxmorin@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-04 15:17:22]

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

[commit-bot: I haz the power, 2016-08-04 15:21:36]

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

[commit-bot: I haz the power, 2016-08-04 15:21:37]

Dry run: Exceeded global retry quota

[Max Morin WebRTC, 2016-08-04 15:26:30]

Description was changed from

==========
GYP->GN of audio_device_tests. Remove dead code. Fix new warnings.

BUG=webrtc:163,webrtc:6170,webrtc:5949
==========

to

==========
GYP->GN of audio_device_tests. Remove dead code. Fix new warnings.

Some old TODOs were fixed while fixing the new warnings from using GN.

BUG=webrtc:6170,webrtc:5949
==========

[Max Morin WebRTC, 2016-08-04 15:26:30]

maxmorin@webrtc.org changed reviewers:
+ henrika@webrtc.org, kjellander@webrtc.org, pthatcher@webrtc.org

[Max Morin WebRTC, 2016-08-05 09:09:23]

The CQ bit was checked by maxmorin@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-05 09:09:29]

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

[commit-bot: I haz the power, 2016-08-05 09:13:18]

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

[commit-bot: I haz the power, 2016-08-05 09:13:19]

Dry run: Try jobs failed on following builders:
  ios64_gn_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_gn_rel/builds/1628)

[Max Morin WebRTC, 2016-08-05 10:47:02]

maxmorin@webrtc.org changed reviewers:
- henrika@webrtc.org, kjellander@webrtc.org, pthatcher@webrtc.org

[Max Morin WebRTC, 2016-08-05 10:47:17]

The CQ bit was checked by maxmorin@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-05 10:47:23]

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

[commit-bot: I haz the power, 2016-08-05 10:52:53]

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

[commit-bot: I haz the power, 2016-08-05 10:52:54]

Dry run: Try jobs failed on following builders:
  ios64_gn_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_gn_dbg/builds/1608)

[Max Morin WebRTC, 2016-08-05 10:58:53]

The CQ bit was checked by maxmorin@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-05 10:59:03]

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

[Max Morin WebRTC, 2016-08-05 11:42:24]

maxmorin@webrtc.org changed reviewers:
+ henrika@webrtc.org, kjellander@webrtc.org, pthatcher@webrtc.org

[Max Morin WebRTC, 2016-08-05 11:52:29]

Kjellander: PTAL at the build files.
Thatcher: PTAL at the small change in webrtc/media/engine.
Henrika: PTAL at /webrtc/modules/audio_device and webrtc/voice_engine. By the
way, do you ever use audio_device_test_func? It doesn't seem to work, so I'll
just remove it unless you have some reason to keep it.

https://codereview.webrtc.org/2214003002/diff/100001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvoiceengine.cc (right):

https://codereview.webrtc.org/2214003002/diff/100001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvoiceengine.cc:1223: number_of_channels,
number_of_frames);
This was the only call to OnData, and all it did was forward to call to
PushCaptureData anyways.

[commit-bot: I haz the power, 2016-08-05 11:58:27]

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

[commit-bot: I haz the power, 2016-08-05 11:58:28]

Dry run: This issue passed the CQ dry run.

[the sun, 2016-08-05 12:15:10]

On 2016/08/05 11:52:29, Max Morin wrote:
> Kjellander: PTAL at the build files.
> Thatcher: PTAL at the small change in webrtc/media/engine.
> Henrika: PTAL at /webrtc/modules/audio_device and webrtc/voice_engine. By the
> way, do you ever use audio_device_test_func? It doesn't seem to work, so I'll
> just remove it unless you have some reason to keep it.
> 
>
https://codereview.webrtc.org/2214003002/diff/100001/webrtc/media/engine/webr...
> File webrtc/media/engine/webrtcvoiceengine.cc (right):
> 
>
https://codereview.webrtc.org/2214003002/diff/100001/webrtc/media/engine/webr...
> webrtc/media/engine/webrtcvoiceengine.cc:1223: number_of_channels,
> number_of_frames);
> This was the only call to OnData, and all it did was forward to call to
> PushCaptureData anyways.

Great cleanup!

You should split this up into several CLs (e.g. one for the AudioTransport
changes, one for the ADM interface changes, one for the .gn file changes). That
will make it easier to review (your above comment would've been unnecessary) and
if anything happens to break down stream, only part needs to be reverted.

[kjellander_webrtc, 2016-08-05 12:47:58]

https://codereview.webrtc.org/2214003002/diff/100001/webrtc/modules/audio_dev...
File webrtc/modules/audio_device/BUILD.gn (right):

https://codereview.webrtc.org/2214003002/diff/100001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/BUILD.gn:219: if (!is_ios) {
Make this:
if (rtc_include_tests && !is_ios) {
to prevent Chromium from including the test in its 'all' target.

https://codereview.webrtc.org/2214003002/diff/100001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/BUILD.gn:229: "../../../testing/gtest:gtest",
Make this 
//testing/gtest:gtest
or just 
//testing/gtest 
(which is more common).

The rest needs to be relative, as they're inside the WebRTC repo (needs to be
like that to avoid errors when parsed by a Chromium build)

https://codereview.webrtc.org/2214003002/diff/100001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/BUILD.gn:231: "../../test:test_support",
you don't need this if you depend on test_support_main.

[kjellander_webrtc, 2016-08-05 12:49:13]

On 2016/08/05 12:47:58, kjellander_webrtc wrote:
>
https://codereview.webrtc.org/2214003002/diff/100001/webrtc/modules/audio_dev...
> File webrtc/modules/audio_device/BUILD.gn (right):
> 
>
https://codereview.webrtc.org/2214003002/diff/100001/webrtc/modules/audio_dev...
> webrtc/modules/audio_device/BUILD.gn:219: if (!is_ios) {
> Make this:
> if (rtc_include_tests && !is_ios) {
> to prevent Chromium from including the test in its 'all' target.
> 
>
https://codereview.webrtc.org/2214003002/diff/100001/webrtc/modules/audio_dev...
> webrtc/modules/audio_device/BUILD.gn:229: "../../../testing/gtest:gtest",
> Make this 
> //testing/gtest:gtest
> or just 
> //testing/gtest 
> (which is more common).
> 
> The rest needs to be relative, as they're inside the WebRTC repo (needs to be
> like that to avoid errors when parsed by a Chromium build)
> 
>
https://codereview.webrtc.org/2214003002/diff/100001/webrtc/modules/audio_dev...
> webrtc/modules/audio_device/BUILD.gn:231: "../../test:test_support",
> you don't need this if you depend on test_support_main.

I'm not sure this code is even used by Chromium, but please run at least one
Chromium trybot per platform. See examples at
https://webrtc.org/contributing/#tryjobs-on-chromium-trybots

[Max Morin WebRTC, 2016-08-05 14:55:56]

On 2016/08/05 12:15:10, the sun (OOO until August 22nd wrote:
> On 2016/08/05 11:52:29, Max Morin wrote:
> > Kjellander: PTAL at the build files.
> > Thatcher: PTAL at the small change in webrtc/media/engine.
> > Henrika: PTAL at /webrtc/modules/audio_device and webrtc/voice_engine. By
the
> > way, do you ever use audio_device_test_func? It doesn't seem to work, so
I'll
> > just remove it unless you have some reason to keep it.
> > 
> >
>
https://codereview.webrtc.org/2214003002/diff/100001/webrtc/media/engine/webr...
> > File webrtc/media/engine/webrtcvoiceengine.cc (right):
> > 
> >
>
https://codereview.webrtc.org/2214003002/diff/100001/webrtc/media/engine/webr...
> > webrtc/media/engine/webrtcvoiceengine.cc:1223: number_of_channels,
> > number_of_frames);
> > This was the only call to OnData, and all it did was forward to call to
> > PushCaptureData anyways.
> 
> Great cleanup!
> 
> You should split this up into several CLs (e.g. one for the AudioTransport
> changes, one for the ADM interface changes, one for the .gn file changes).
That
> will make it easier to review (your above comment would've been unnecessary)
and
> if anything happens to break down stream, only part needs to be reverted.

Good point, I'll do that.

[Max Morin WebRTC, 2016-08-05 14:57:15]

On 2016/08/05 12:49:13, kjellander_webrtc wrote:
> On 2016/08/05 12:47:58, kjellander_webrtc wrote:
> >
>
https://codereview.webrtc.org/2214003002/diff/100001/webrtc/modules/audio_dev...
> > File webrtc/modules/audio_device/BUILD.gn (right):
> > 
> >
>
https://codereview.webrtc.org/2214003002/diff/100001/webrtc/modules/audio_dev...
> > webrtc/modules/audio_device/BUILD.gn:219: if (!is_ios) {
> > Make this:
> > if (rtc_include_tests && !is_ios) {
> > to prevent Chromium from including the test in its 'all' target.
> > 
> >
>
https://codereview.webrtc.org/2214003002/diff/100001/webrtc/modules/audio_dev...
> > webrtc/modules/audio_device/BUILD.gn:229: "../../../testing/gtest:gtest",
> > Make this 
> > //testing/gtest:gtest
> > or just 
> > //testing/gtest 
> > (which is more common).
> > 
> > The rest needs to be relative, as they're inside the WebRTC repo (needs to
be
> > like that to avoid errors when parsed by a Chromium build)
> > 
> >
>
https://codereview.webrtc.org/2214003002/diff/100001/webrtc/modules/audio_dev...
> > webrtc/modules/audio_device/BUILD.gn:231: "../../test:test_support",
> > you don't need this if you depend on test_support_main.
> 
> I'm not sure this code is even used by Chromium, but please run at least one
> Chromium trybot per platform. See examples at
> https://webrtc.org/contributing/#tryjobs-on-chromium-trybots

I didn't even know we had Chromium trybots, awesome! This CL is now
https://codereview.webrtc.org/2222563002/
https://codereview.webrtc.org/2219653004/
https://codereview.webrtc.org/2216423002/

[Max Morin WebRTC, 2016-08-05 20:58:48]

https://codereview.webrtc.org/2214003002/diff/100001/webrtc/modules/audio_dev...
File webrtc/modules/audio_device/BUILD.gn (right):

https://codereview.webrtc.org/2214003002/diff/100001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/BUILD.gn:219: if (!is_ios) {
On 2016/08/05 12:47:58, kjellander_webrtc wrote:
> Make this:
> if (rtc_include_tests && !is_ios) {
> to prevent Chromium from including the test in its 'all' target.

Done.

https://codereview.webrtc.org/2214003002/diff/100001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/BUILD.gn:229: "../../../testing/gtest:gtest",
On 2016/08/05 12:47:58, kjellander_webrtc wrote:
> Make this 
> //testing/gtest:gtest
> or just 
> //testing/gtest 
> (which is more common).
> 
> The rest needs to be relative, as they're inside the WebRTC repo (needs to be
> like that to avoid errors when parsed by a Chromium build)

Done.

https://codereview.webrtc.org/2214003002/diff/100001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/BUILD.gn:231: "../../test:test_support",
On 2016/08/05 12:47:58, kjellander_webrtc wrote:
> you don't need this if you depend on test_support_main.

"gn check" warns without it, since audio_device_test_api.cc includes
webrtc/test/testsupport/fileutils.h, which isn't directly in test_support_main

[kjellander_webrtc, 2016-08-08 04:24:25]

I think you forgot to upload a new patch set?

[Max Morin WebRTC, 2016-08-08 13:42:23]

Patchset #7 (id:120001) has been deleted