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

[bugzilla-daemon at webkit.org]

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

--- Comment #2 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 280807
  --> https://bugs.webkit.org/attachment.cgi?id=280807
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.

> Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.cpp:389
> +

Minor nit: I find the blank line here slightly distracting

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

Ditto.

> 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?

> Source/WebCore/Modules/mediastream/RTCTrackEvent.idl:37
> +   [InitializedByEventConstructor] readonly attribute MediaStream[] streams;

Can you use "attribute sequence<MediaStream> streams" instead?

> 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.

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