Implement the MediaEndponitPeerConnection implementation of RTCPeerConnection.setRemoteDescription [1]. [1] https://w3c.github.io/webrtc-pc/archives/20160513/webrtc.html#dom-rtcpeerconnection-setremotedescription
Created attachment 280807 [details] Proposed patch
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. > 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.
(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.
Created attachment 280824 [details] Updated patch Thanks for reviewing; updated patch.
Comment on attachment 280824 [details] Updated patch Clearing flags on attachment: 280824 Committed r201851: <http://trac.webkit.org/changeset/201851>
All reviewed patches have been landed. Closing bug.