This is behavior is detailed in the RTCPeerConnection.addTrack algorithm step 5 [1]. [1] https://w3c.github.io/webrtc-pc/archives/20160513/webrtc.html#dom-rtcpeerconnection-addtrack
Created attachment 281013 [details] Proposed patch
Comment on attachment 281013 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=281013&action=review > LayoutTests/fast/mediastream/RTCPeerConnection-addTrack-reuse-sender.html:1 > +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> I just noticed that you are using the HTML 2.0 doctype for these tests. Why not just "<!DOCTYPE html>"? > LayoutTests/fast/mediastream/RTCPeerConnection-addTrack-reuse-sender.html:52 > + debug(""); Minor nit: you can get rid of all debug("")s by adding a '<br>' to the beginning of the next debug string. > LayoutTests/fast/mediastream/RTCPeerConnection-addTrack-reuse-sender.html:58 > + debug("*** An extra sender should have been added"); Another minor nit: why no blank line before this? > LayoutTests/fast/mediastream/RTCPeerConnection-addTrack-reuse-sender.html:69 > + debug("*** The number of senders should not have changed"); Ditto.
(In reply to comment #2) > Comment on attachment 281013 [details] > Proposed patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=281013&action=review > > > LayoutTests/fast/mediastream/RTCPeerConnection-addTrack-reuse-sender.html:1 > > +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> > > I just noticed that you are using the HTML 2.0 doctype for these tests. Why > not just "<!DOCTYPE html>"? I had not noticed that, but you're right. I based this on some other test which had this old doctype. Fixed now. > > LayoutTests/fast/mediastream/RTCPeerConnection-addTrack-reuse-sender.html:52 > > + debug(""); > > Minor nit: you can get rid of all debug("")s by adding a '<br>' to the > beginning of the next debug string. Let's use that. Fixed. > > LayoutTests/fast/mediastream/RTCPeerConnection-addTrack-reuse-sender.html:58 > > + debug("*** An extra sender should have been added"); > > Another minor nit: why no blank line before this? These lines belong to the same chunk as the ones above. I can see that the initial "***" on debug messages makes them look like "headers". I borrowed that style from some other test to highlight debug messages. > > LayoutTests/fast/mediastream/RTCPeerConnection-addTrack-reuse-sender.html:69 > > + debug("*** The number of senders should not have changed"); > > Ditto.
(In reply to comment #3) > > > LayoutTests/fast/mediastream/RTCPeerConnection-addTrack-reuse-sender.html:58 > > > + debug("*** An extra sender should have been added"); > > > > Another minor nit: why no blank line before this? > > These lines belong to the same chunk as the ones above. I can see that the > initial "***" on debug messages makes them look like "headers". I borrowed > that style from some other test to highlight debug messages. > But then again, looking at the actual test these sections are separated by newlines. Let's space them out in the results as well. :)
Created attachment 281163 [details] Updated patch Fixed review comments
Comment on attachment 281163 [details] Updated patch Thanks for reviewing Eric!
Comment on attachment 281163 [details] Updated patch Clearing flags on attachment: 281163 Committed r201999: <http://trac.webkit.org/changeset/201999>
All reviewed patches have been landed. Closing bug.