replaceTrack might be handy in various cases.
Created attachment 304840 [details] Patch
Created attachment 305119 [details] Patch
Comment on attachment 305119 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=305119&action=review > Source/WebCore/Modules/mediastream/RTCRtpSender.idl:42 > - [MayThrowException] Promise<void> replaceTrack(MediaStreamTrack withTrack); > + [MayThrowException] Promise<void> replaceTrack(MediaStreamTrack? withTrack); Why are we deviating from the spec here?
(In reply to Alex Christensen from comment #3) > Comment on attachment 305119 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=305119&action=review > > > Source/WebCore/Modules/mediastream/RTCRtpSender.idl:42 > > - [MayThrowException] Promise<void> replaceTrack(MediaStreamTrack withTrack); > > + [MayThrowException] Promise<void> replaceTrack(MediaStreamTrack? withTrack); > > Why are we deviating from the spec here? Probably a typo, I need to file an issue. Description at https://www.w3.org/TR/webrtc/#dom-rtcrtpsender-replacetrack expects withTrack to be nullable.
Comment on attachment 305119 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=305119&action=review > Source/WebCore/Modules/mediastream/RTCRtpSender.cpp:69 > { Are we missing step 4 of replaceTrack? > Source/WebCore/Modules/mediastream/RTCRtpSender.cpp:75 > + if (!withTrack) { shouldn't this be "if (withTrack)"? > Source/WebCore/Modules/mediastream/RTCRtpSender.cpp:85 > return Exception { TypeError }; Should this be promise.reject(TypeError) instead?
(In reply to Jon Lee from comment #5) > Comment on attachment 305119 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=305119&action=review > > > Source/WebCore/Modules/mediastream/RTCRtpSender.cpp:69 > > { > > Are we missing step 4 of replaceTrack? The implementation order is not matching the spec order. It is true that it might cause subtle differences between implementation. > > > Source/WebCore/Modules/mediastream/RTCRtpSender.cpp:75 > > + if (!withTrack) { > > shouldn't this be "if (withTrack)"? Again, we are not matching the spec step by step. It seems to me the spec should branch early on when withTrack is null. That would be more readable and easier to implement anyway. > > > Source/WebCore/Modules/mediastream/RTCRtpSender.cpp:85 > > return Exception { TypeError }; > > Should this be promise.reject(TypeError) instead? Oh, that is a left over when we were catching exceptions and rejecting promises. replaceTrack should not have MayThrowException anymore.
Created attachment 305221 [details] Patch
Comment on attachment 305221 [details] Patch Clearing flags on attachment: 305221 Committed r214357: <http://trac.webkit.org/changeset/214357>
All reviewed patches have been landed. Closing bug.