[Webkit-unassigned] [Bug 150166] WebRTC: Update RTCPeerConnection API and introduce PeerConnectionBackend

[bugzilla-daemon at webkit.org]

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

--- Comment #32 from Adam Bergkvist <adam.bergkvist at ericsson.com> ---
(In reply to comment #31)
> Comment on attachment 265283 [details]
> Updated patch

Thanks for reviewing. I'll respond to your general comments now and fix the rest later today.

> This looks like a great move forward :)
> 
> Two general comments come to mind:
> - There is some refactoring as part of this patch that could have been
> splitted so as to ease the review. This may be good for future developments.

I agree that this patch could have been split up and some pure refactoring changes could have been in stand-alone patches. I was a bit lazy when I extracted it from the downstream fork. This patch contains the foundation and it will be much easier to make smaller patches that incrementally adds functionality from now on.

> - I am surprised that no test is added/few tests are rebased. How is WebRTC
> tested? It would be great to add/activate tests, even if they are mostly
> failing at the beginning.
> Without tests, it might be so easy to inadvertently break some behavior
> (like the promise/legacy switch for instance).
> There are a few tests available at
> https://github.com/w3c/web-platform-tests/tree/master/webrtc/ that can be
> easily imported (see http://trac.webkit.org/wiki/WebKitW3CTesting), maybe
> some other tests can be imported from Blink/Mozilla.

The reason why this patch doesn't add or enable any tests is that it's not possible to construct an RTCPeerConnection yet. The missing piece is an PeerConnectionBackend implementation and it's coming in a follow up patch. I didn't plan to make a mock object for it since it's a WebCore component.

-- 
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/20151112/6d263557/attachment-0001.html>