[Webkit-unassigned] [Bug 158985] WebRTC: Add support for the negotiationneeded event in MediaEndpointPeerConnection

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jun 21 22:19:42 PDT 2016


https://bugs.webkit.org/show_bug.cgi?id=158985

--- Comment #3 from Adam Bergkvist <adam.bergkvist at ericsson.com> ---
(In reply to comment #2)
> Comment on attachment 281737 [details]
> Proposed patch

Thanks for reviewing

> View in context:
> https://bugs.webkit.org/attachment.cgi?id=281737&action=review
> 
> > LayoutTests/ChangeLog:15
> > +        * fast/mediastream/RTCPeerConnection-more-media-to-negotiate-expected.txt: Added.
> > +        * fast/mediastream/RTCPeerConnection-more-media-to-negotiate.html: Added.
> > +        Verify that a negotiationneeded event is fired when not all local media can be included in
> > +        an answer. (The answerer cannot reply with more m-lines than the offer had to begin with.)
> > +        * fast/mediastream/RTCPeerConnection-onnegotiationneeded-expected.txt:
> > +        * fast/mediastream/RTCPeerConnection-onnegotiationneeded.html:
> 
> Don't we also need a test for a stream track entering an ended state? A bug
> for adding that test is fine if you don't want to do it in this patch.

We should also test that case. I remember that we've talked about an internal test API to emulate events originating from the browser; something like "stopSourceOfTrack(track)". That would be useful here. About a bug, see next comment.

> Are there any other operations that should generate a 'negotiationneeded'
> event?

There's an issue [1] filed on the WebRTC 1.0 github repo about putting together a table will all cases that must set the 'negotiation needed flag'. I've also referenced this issue in a new bug about adding more tests [2].

[1] https://github.com/w3c/webrtc-pc/issues/661
[2] http://webkit.org/b/159016


> > LayoutTests/fast/mediastream/RTCPeerConnection-more-media-to-negotiate.html:10
> > +            description("Test that a negotiationneeded event is fired when all local media can't be included in an answer");
> 
> Nit: "when all local media can't be included" -> "when not all local media
> can be included"

Will fix.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160622/ab3eee2e/attachment.html>


More information about the webkit-unassigned mailing list