[Webkit-unassigned] [Bug 158467] WebRTC: Imlement MediaEndpointPeerConnection::setRemoteDescription()

[bugzilla-daemon at webkit.org]

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

--- Comment #3 from Adam Bergkvist <adam.bergkvist at ericsson.com> ---
(In reply to comment #2)
> Comment on attachment 280807 [details]
> Proposed patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=280807&action=review
> 
> > LayoutTests/fast/mediastream/RTCTrackEvent-constructor.html:64
> > +                debug("Dictionary members receiver, track and transceiver are not nullable");
> > +                shouldThrow("new RTCTrackEvent('eventType', { receiver: null, track: track, transceiver: transceiver})");
> > +                shouldThrow("new RTCTrackEvent('eventType', { receiver: receiver, track: null, transceiver: transceiver})");
> > +                shouldThrow("new RTCTrackEvent('eventType', { receiver: receiver, track: track, transceiver: null})");
> > +                debug("");
> > +
> > +                debug("Dictionary members receiver, track and transceiver are required");
> > +                shouldThrow("new RTCTrackEvent('eventType', { track: track, transceiver: transceiver})");
> > +                shouldThrow("new RTCTrackEvent('eventType', { receiver: receiver, transceiver: transceiver})");
> > +                shouldThrow("new RTCTrackEvent('eventType', { receiver: receiver, track: track})");
> > +                debug("");
> 
> Please add a FIXME and a bug # about these failures.

Fixed.

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

> > Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.cpp:389
> > +
> 
> Minor nit: I find the blank line here slightly distracting

Gone. :)

> > Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.cpp:401
> > +
> > +
> 
> Ditto.

Fixed.

> > Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.cpp:412
> > +    for (unsigned i = 0; i < mediaDescriptions.size(); ++i) {
> > +        PeerMediaDescription* mediaDescription = mediaDescriptions[i].get();
> 
> Nit: can you use a modern for loop here?

True. Fixed.

> > Source/WebCore/Modules/mediastream/RTCTrackEvent.idl:37
> > +   [InitializedByEventConstructor] readonly attribute MediaStream[] streams;
> 
> Can you use "attribute sequence<MediaStream> streams" instead?

A sequence can't be an interface attribute [1].

[1] http://heycam.github.io/webidl/#idl-sequence

> > Source/WebCore/platform/mock/MockMediaEndpoint.cpp:140
> > +        for (auto& p : defaultPayloads) {
> 
> Nit: a single letter variable name in a modern for loop - such an
> anachronism :-)
> 
> Might as well spell out the variable.

Ops. :) Let's use payload since defaultPayload is taken.

-- 
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/20160608/e1cb7d82/attachment.html>