Added layouttests for checking that RTCPeerConnection.onaddstream

Added layouttests for checking that RTCPeerConnection.onaddstream
and RTCPeerConnection.onremovestream is called with a correct MediaStream.
This is based on MockWebRtcPeerConnectionHandler.

BUG=417245
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=184251

[perkj_chrome, 2014-09-25 14:53:41]

perkj@chromium.org changed reviewers:
+ hta@chromium.org

[perkj_chrome, 2014-09-25 14:53:41]

Can you please take a look?

[hta - Chromium, 2014-09-29 12:59:18]

Reasonably compelling - but I think you should fork the test, not extend it.

The original test checks that you can do the negotiation *without* any streams
attached; the new test only tests that you can do the negotiation *with* streams
attached.

(actually, in both tests, the negotiation fails. I'm not completely clear why it
should fail.)

I think both tests are worth running, but separately.

https://codereview.chromium.org/607523002/diff/1/LayoutTests/fast/mediastream...
File LayoutTests/fast/mediastream/RTCPeerConnection-remoteDescription.html
(right):

https://codereview.chromium.org/607523002/diff/1/LayoutTests/fast/mediastream...
LayoutTests/fast/mediastream/RTCPeerConnection-remoteDescription.html:39:
function requestFailed2()
Nit: Can you rename this to requestFailedAsExpected? It is a bit surprising to
discover that the setRemoteDescription was expected to fail.

https://codereview.chromium.org/607523002/diff/1/LayoutTests/fast/mediastream...
LayoutTests/fast/mediastream/RTCPeerConnection-remoteDescription.html:50:
function requestSucceeded2()
Same naming thing here....

https://codereview.chromium.org/607523002/diff/1/LayoutTests/fast/mediastream...
LayoutTests/fast/mediastream/RTCPeerConnection-remoteDescription.html:95:
getUserMedia({audio:true, video:true}, gotStream);
Nit: remember the error handler.

[perkj_chrome, 2014-09-30 19:15:15]

PTAL

https://codereview.chromium.org/607523002/diff/1/LayoutTests/fast/mediastream...
File LayoutTests/fast/mediastream/RTCPeerConnection-remoteDescription.html
(right):

https://codereview.chromium.org/607523002/diff/1/LayoutTests/fast/mediastream...
LayoutTests/fast/mediastream/RTCPeerConnection-remoteDescription.html:39:
function requestFailed2()
On 2014/09/29 12:59:17, hta - Chromium wrote:
> Nit: Can you rename this to requestFailedAsExpected? It is a bit surprising to
> discover that the setRemoteDescription was expected to fail.

Removed from test. Forked test instead.

https://codereview.chromium.org/607523002/diff/1/LayoutTests/fast/mediastream...
LayoutTests/fast/mediastream/RTCPeerConnection-remoteDescription.html:95:
getUserMedia({audio:true, video:true}, gotStream);
On 2014/09/29 12:59:17, hta - Chromium wrote:
> Nit: remember the error handler.

getUserMedia is wrapper that handle that.

[perkj_chrome, 2014-10-09 07:56:59]

And now actually uploaded.

[hta - Chromium, 2014-10-09 08:07:33]

lgtm

Just grammar.

https://codereview.chromium.org/607523002/diff/20001/LayoutTests/fast/mediast...
File LayoutTests/fast/mediastream/RTCPeerConnection-remotestreams.html (right):

https://codereview.chromium.org/607523002/diff/20001/LayoutTests/fast/mediast...
LayoutTests/fast/mediastream/RTCPeerConnection-remotestreams.html:8:
description("Tests RTCPeerConnection remote MediaStreams.");
How long should these descriptions be?

I would prefer "Tests that an RTCPeerConnection can signal that remote streams
are added and removed".

https://codereview.chromium.org/607523002/diff/20001/LayoutTests/fast/mediast...
LayoutTests/fast/mediastream/RTCPeerConnection-remotestreams.html:43: function
requestFailedUnexpected()
English grammar: Unexpectedly

[perkj_chrome, 2014-10-21 09:19:57]

https://codereview.chromium.org/607523002/diff/20001/LayoutTests/fast/mediast...
File LayoutTests/fast/mediastream/RTCPeerConnection-remotestreams.html (right):

https://codereview.chromium.org/607523002/diff/20001/LayoutTests/fast/mediast...
LayoutTests/fast/mediastream/RTCPeerConnection-remotestreams.html:8:
description("Tests RTCPeerConnection remote MediaStreams.");
On 2014/10/09 08:07:33, hta - Chromium wrote:
> How long should these descriptions be?
> 
> I would prefer "Tests that an RTCPeerConnection can signal that remote streams
> are added and removed".

Done.

https://codereview.chromium.org/607523002/diff/20001/LayoutTests/fast/mediast...
LayoutTests/fast/mediastream/RTCPeerConnection-remotestreams.html:43: function
requestFailedUnexpected()
On 2014/10/09 08:07:33, hta - Chromium wrote:
> English grammar: Unexpectedly

Done.

[hta - Chromium, 2014-10-22 13:15:29]

lgtm

[perkj_chrome, 2014-10-23 07:58:53]

Patchset #4 (id:60001) has been deleted

[perkj_chrome, 2014-10-23 08:00:22]

perkj@chromium.org changed reviewers:
+ jochen@chromium.org

[perkj_chrome, 2014-10-23 08:00:22]

Jochen, can you help with this one as well? Do I/we need you for added tests?

[jochen (gone - plz use gerrit), 2014-10-23 09:39:06]

any committer can approve in LayoutTests/ I think

lgtm nevertheless

[perkj_chrome, 2014-10-23 09:57:05]

The CQ bit was checked by perkj@chromium.org

[commit-bot: I haz the power, 2014-10-23 09:57:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/607523002/80001

[commit-bot: I haz the power, 2014-10-23 10:01:08]

Committed patchset #4 (id:80001) as 184251