Adds more unit tests for WebRTC.

Adds more unit tests for WebRTC.
These tests verifies that rendering and capturing starts as they should.
It is also possible to enable a full-duplex audio test.

BUG=none
TEST=content_unittests with --gtest_filter=*WebRTC*
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110289

[tommi (sloooow) - chröme, 2011-11-14 17:25:35]

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
File content/renderer/media/webrtc_audio_device_impl.cc (right):

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_impl.cc:364: // Mac OS X
align comment to code and move into the mac section

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_impl.cc:403: // Linux
same for this comment.  actually, I don't think these comments really add value
to the actual #if

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
File content/renderer/media/webrtc_audio_device_unittest.cc (right):

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_unittest.cc:53: channels_(0) {}
} on the next line since this is not a one liner

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_unittest.cc:63: channel_id_ =
channel;
too much indentation?

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_unittest.cc:63: channel_id_ =
channel;
Is it worth it doing something like:
if (channel_id_ == -1) {
  channel_id = channel;
} else if (channel_id != channel) {
  ADD_FAILURE();
}
EXPECT_NE(-1, channel_id_);

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_unittest.cc:68: if (event_) {
EXPECT_TRUE(event_)

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_unittest.cc:124:
OnSetAudioStreamStatus(_, 1, StrEq("created"))).Times(1);
fix indent on these lines

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_unittest.cc:197: new
WebRtcAudioDeviceImpl());
indent

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_unittest.cc:215: new
WebRTCMediaProcessImpl(&event));
ditto.  and below

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_unittest.cc:249:
TEST_F(WebRTCAudioDeviceTest, DISABLED_PlayLocalFile) {
any reason why?

[henrika (OOO until Aug 14), 2011-11-15 09:27:39]

Modified.

Please advise regarding usage of DISABLED_.

Is it OK to force all users who run locally to wait for 10 seconds?

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
File content/renderer/media/webrtc_audio_device_impl.cc (right):

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_impl.cc:364: // Mac OS X
This file has been removed from the CL. Sorry.

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_impl.cc:403: // Linux
This file has been removed from the CL. Sorry.

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
File content/renderer/media/webrtc_audio_device_unittest.cc (right):

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_unittest.cc:53: channels_(0) {}
On 2011/11/14 17:25:35, tommi wrote:
> } on the next line since this is not a one liner

Done.

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_unittest.cc:63: channel_id_ =
channel;
On 2011/11/14 17:25:35, tommi wrote:
> too much indentation?

Done.

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_unittest.cc:63: channel_id_ =
channel;
No. Because -1 is valid and means "all channels". We provide -1 if the callback
is for a mixed signal and not for a unique channel. It is better to check the
channel in the actual test when you know if -1 is OK or not.

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_unittest.cc:124:
OnSetAudioStreamStatus(_, 1, StrEq("created"))).Times(1);
On 2011/11/14 17:25:35, tommi wrote:
> fix indent on these lines

Done.

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_unittest.cc:197: new
WebRtcAudioDeviceImpl());
On 2011/11/14 17:25:35, tommi wrote:
> indent

Done.

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_unittest.cc:215: new
WebRTCMediaProcessImpl(&event));
On 2011/11/14 17:25:35, tommi wrote:
> ditto.  and below

Done.

http://codereview.chromium.org/8528026/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_unittest.cc:249:
TEST_F(WebRTCAudioDeviceTest, DISABLED_PlayLocalFile) {
If you think it is OK to play a file for 10 seconds.

Please advise!

[tommi (sloooow) - chröme, 2011-11-15 10:57:48]

LGTM.

Yes, I think we should have the test enabled for those that run them manually. 
If it is taking too long, we can either shorten the timeout or simply suggest
using gtest_filter to exclude the test from being run.
If we disable the test, nobody will run it.

http://codereview.chromium.org/8528026/diff/2008/content/renderer/media/webrt...
File content/renderer/media/webrtc_audio_device_unittest.cc (right):

http://codereview.chromium.org/8528026/diff/2008/content/renderer/media/webrt...
content/renderer/media/webrtc_audio_device_unittest.cc:54: {}
{ should be on the preceding line.

[henrika (OOO until Aug 14), 2011-11-15 12:04:33]

http://codereview.chromium.org/8528026/diff/2008/content/renderer/media/webrt...
File content/renderer/media/webrtc_audio_device_unittest.cc (right):

http://codereview.chromium.org/8528026/diff/2008/content/renderer/media/webrt...
content/renderer/media/webrtc_audio_device_unittest.cc:54: {}
On 2011/11/15 10:57:49, tommi wrote:
> { should be on the preceding line.

Done.

[henrika (OOO until Aug 14), 2011-11-15 12:15:23]

John,

it would be great if you had time to take a look at this CL as owner in
content/renderer.

The CL contains some new unit tests for WebRTC and I use a frame work written by
tommi which has already been submitted.

I do not expect you to comment on the exact details of the test since it
requires detailed knowledge about the WebRTC stack.

When this CL has landed, we will finally have some sort of test which will
prevent others from breaking WebRTC and it will also enable the users to run
audio in loopback.

NOTE - this CL will not be landed until a pending CL
(http://codereview.chromium.org/8440002/) has been committed. I am waiting for
an updated WebKit there. Hence

[jam, 2011-11-15 21:43:51]

lgtm

[scherkus (not reviewing), 2011-11-16 01:23:13]

drive by nit but LGTM++

nice tests!

http://codereview.chromium.org/8528026/diff/9001/content/renderer/media/webrt...
File content/renderer/media/webrtc_audio_device_unittest.cc (right):

http://codereview.chromium.org/8528026/diff/9001/content/renderer/media/webrt...
content/renderer/media/webrtc_audio_device_unittest.cc:48: : event_(event),
indent by two more spaces

[henrika (OOO until Aug 14), 2011-11-16 11:25:34]

Thanks all!

http://codereview.chromium.org/8528026/diff/9001/content/renderer/media/webrt...
File content/renderer/media/webrtc_audio_device_unittest.cc (right):

http://codereview.chromium.org/8528026/diff/9001/content/renderer/media/webrt...
content/renderer/media/webrtc_audio_device_unittest.cc:48: : event_(event),
On 2011/11/16 01:23:13, scherkus wrote:
> indent by two more spaces

Done.