Add test for sending square resolution video on a PeerConnection.

Add test for sending square resolution video on a PeerConnection.
Adds expectations for video resolution output on all tests that use peerconnection-call.html call.

BUG=350538
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=260179

[perkj_chrome, 2014-03-27 14:44:18]

Hej
A small test...

[phoglund_chromium, 2014-03-28 08:33:30]

https://codereview.chromium.org/214663004/diff/1/content/browser/media/webrtc...
File content/browser/media/webrtc_browsertest.cc (right):

https://codereview.chromium.org/214663004/diff/1/content/browser/media/webrtc...
content/browser/media/webrtc_browsertest.cc:75:
MakeTypicalPeerConnectionCall("call({video: true}, 640, 480);");
This is confusing - it looks like we're setting up a call to be 640x480, but we
are setting up a call according to the constraints and just verifying the
resolution is 640x480.

Also, this function is now effectively verifying that the default constraint
gives you 640x480, which should be clearer.

I can imagine ways to get this to read better. Unfortunately we can't use named
parameters like in python (call(c, expected_width=640, ...) would have been
nice), so what about this:

Keep the original call() and make a new function callAndVerifyResolution which
does what you want above.

I would also like to not duplicate the assumption that the default gives you
640x480. So keep CanSetupVideoCall and the other tests as they are and add a new
test method DefaultVideoConstraintsGivesVga which explicitly tests the above.

https://codereview.chromium.org/214663004/diff/1/content/test/data/media/peer...
File content/test/data/media/peerconnection-call.html (right):

https://codereview.chromium.org/214663004/diff/1/content/test/data/media/peer...
content/test/data/media/peerconnection-call.html:62:
waitForVideoAndExpectResolution('remote-view-1',
You can shorten this to waitForVideoWithResolution (I think it will read a bit
nicer too).

https://codereview.chromium.org/214663004/diff/1/content/test/data/media/webr...
File content/test/data/media/webrtc_test_utilities.js (right):

https://codereview.chromium.org/214663004/diff/1/content/test/data/media/webr...
content/test/data/media/webrtc_test_utilities.js:82: videoElement,
Nit: indent 4 (or maybe if you can pull up the parameters and align all on the (
is better?)

https://codereview.chromium.org/214663004/diff/1/content/test/data/media/webr...
content/test/data/media/webrtc_test_utilities.js:87: function (width, height) {
Nit: if you line break within the parameter list, this indent should be 4 steps
I think.

[perkj_chrome, 2014-03-28 09:14:23]

PTAL

https://codereview.chromium.org/214663004/diff/1/content/browser/media/webrtc...
File content/browser/media/webrtc_browsertest.cc (right):

https://codereview.chromium.org/214663004/diff/1/content/browser/media/webrtc...
content/browser/media/webrtc_browsertest.cc:75:
MakeTypicalPeerConnectionCall("call({video: true}, 640, 480);");
On 2014/03/28 08:33:30, phoglund wrote:
> This is confusing - it looks like we're setting up a call to be 640x480, but
we
> are setting up a call according to the constraints and just verifying the
> resolution is 640x480.
> 
> Also, this function is now effectively verifying that the default constraint
> gives you 640x480, which should be clearer.
> 
> I can imagine ways to get this to read better. Unfortunately we can't use
named
> parameters like in python (call(c, expected_width=640, ...) would have been
> nice), so what about this:
> 
> Keep the original call() and make a new function callAndVerifyResolution which
> does what you want above.
> 
> I would also like to not duplicate the assumption that the default gives you
> 640x480. So keep CanSetupVideoCall and the other tests as they are and add a
new
> test method DefaultVideoConstraintsGivesVga which explicitly tests the above.

Done.

https://codereview.chromium.org/214663004/diff/1/content/test/data/media/peer...
File content/test/data/media/peerconnection-call.html (right):

https://codereview.chromium.org/214663004/diff/1/content/test/data/media/peer...
content/test/data/media/peerconnection-call.html:62:
waitForVideoAndExpectResolution('remote-view-1',
On 2014/03/28 08:33:30, phoglund wrote:
> You can shorten this to waitForVideoWithResolution (I think it will read a bit
> nicer too).

Done.

https://codereview.chromium.org/214663004/diff/1/content/test/data/media/webr...
File content/test/data/media/webrtc_test_utilities.js (right):

https://codereview.chromium.org/214663004/diff/1/content/test/data/media/webr...
content/test/data/media/webrtc_test_utilities.js:82: videoElement,
On 2014/03/28 08:33:30, phoglund wrote:
> Nit: indent 4 (or maybe if you can pull up the parameters and align all on the
(
> is better?)

Done.

https://codereview.chromium.org/214663004/diff/1/content/test/data/media/webr...
content/test/data/media/webrtc_test_utilities.js:87: function (width, height) {
On 2014/03/28 08:33:30, phoglund wrote:
> Nit: if you line break within the parameter list, this indent should be 4
steps
> I think.

Done.

[phoglund_chromium, 2014-03-28 12:43:34]

lgtm

https://codereview.chromium.org/214663004/diff/20001/content/test/data/media/...
File content/test/data/media/peerconnection-call.html (right):

https://codereview.chromium.org/214663004/diff/20001/content/test/data/media/...
content/test/data/media/peerconnection-call.html:56: 
Nit: remove

[perkj_chrome, 2014-03-28 14:20:38]

The CQ bit was checked by perkj@chromium.org

[commit-bot: I haz the power, 2014-03-28 14:21:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/perkj@chromium.org/214663004/40001

[commit-bot: I haz the power, 2014-03-28 17:12:12]

Change committed as 260179

[perkj_chrome, 2014-03-31 08:13:31]

https://codereview.chromium.org/214663004/diff/20001/content/test/data/media/...
File content/test/data/media/peerconnection-call.html (right):

https://codereview.chromium.org/214663004/diff/20001/content/test/data/media/...
content/test/data/media/peerconnection-call.html:56: 
On 2014/03/28 12:43:34, phoglund wrote:
> Nit: remove

Done.