Bug 1136871 - cleanup RtpSenders accounting to not rely on streams r=mt

This commit is contained in:
Jan-Ivar Bruaroey 2015-03-11 12:24:38 -04:00
parent 663ac350cc
commit 28242b71ba
7 changed files with 158 additions and 51 deletions

View File

@ -832,17 +832,27 @@ RTCPeerConnection.prototype = {
"InvalidParameterError");
}
this._checkClosed();
this._senders.forEach(sender => {
if (sender.track == track) {
throw new this._win.DOMException("already added.",
"InvalidParameterError");
}
});
this._impl.addTrack(track, stream);
let sender = this._win.RTCRtpSender._create(this._win,
new RTCRtpSender(this, track,
stream));
this._senders.push({ sender: sender, stream: stream });
this._senders.push(sender);
return sender;
},
removeTrack: function(sender) {
this._checkClosed();
this._impl.removeTrack(sender.track);
var i = this._senders.indexOf(sender);
if (i >= 0) {
this._senders.splice(i, 1);
this._impl.removeTrack(sender.track); // fires negotiation needed
}
},
_replaceTrack: function(sender, withTrack) {
@ -888,33 +898,11 @@ RTCPeerConnection.prototype = {
},
getSenders: function() {
this._checkClosed();
let streams = this._impl.getLocalStreams();
let senders = [];
// prune senders in case any streams have disappeared down below
for (let i = this._senders.length - 1; i >= 0; i--) {
if (streams.indexOf(this._senders[i].stream) != -1) {
senders.push(this._senders[i].sender);
} else {
this._senders.splice(i,1);
}
}
return senders;
return this._senders;
},
getReceivers: function() {
this._checkClosed();
let streams = this._impl.getRemoteStreams();
let receivers = [];
// prune receivers in case any streams have disappeared down below
for (let i = this._receivers.length - 1; i >= 0; i--) {
if (streams.indexOf(this._receivers[i].stream) != -1) {
receivers.push(this._receivers[i].receiver);
} else {
this._receivers.splice(i,1);
}
}
return receivers;
return this._receivers;
},
get localDescription() {
@ -1061,7 +1049,7 @@ PeerConnectionObserver.prototype = {
"",
"InternalError",
"InvalidCandidateError",
"InvalidParameter",
"InvalidParameterError",
"InvalidStateError",
"InvalidSessionDescriptionError",
"IncompatibleSessionDescriptionError",

View File

@ -166,6 +166,8 @@ skip-if = toolkit == 'gonk' # b2g (Bug 1059867)
skip-if = toolkit == 'gonk' # b2g (Bug 1059867)
[test_peerConnection_removeThenAddVideoTrack.html]
skip-if = toolkit == 'gonk' # b2g (Bug 1059867)
[test_peerConnection_replaceVideoThenRenegotiate.html]
skip-if = toolkit == 'gonk' # b2g (Bug 1059867)
[test_peerConnection_addSecondAudioStreamNoBundle.html]
skip-if = toolkit == 'gonk' # b2g (Bug 1059867)
[test_peerConnection_removeThenAddAudioTrackNoBundle.html]

View File

@ -897,6 +897,13 @@ PeerConnectionWrapper.prototype = {
this._pc.removeTrack(sender);
},
senderReplaceTrack : function(index, withTrack) {
var sender = this._pc.getSenders()[index];
delete this.expectedLocalTrackTypesById[sender.track.id];
this.expectedLocalTrackTypesById[withTrack.id] = withTrack.kind;
return sender.replaceTrack(withTrack);
},
/**
* Requests all the media streams as specified in the constrains property.
*

View File

@ -18,7 +18,6 @@
[
function PC_LOCAL_REMOVE_AUDIO_TRACK(test) {
test.setOfferOptions({ offerToReceiveAudio: true });
test.setMediaConstraints([], [{audio: true}]);
return test.pcLocal.removeSender(0);
},
]

View File

@ -1,4 +1,4 @@
<!DOCTYPE HTML>
<!DOCTYPE HTML>
<html>
<head>
<script type="application/javascript" src="pc.js"></script>
@ -16,8 +16,6 @@
return sender.track == this;
}
// Test basically just verifies that success callback is called at this point
runNetworkTest(function () {
test = new PeerConnectionTest();
test.setMediaConstraints([{video: true}], [{video: true}]);
@ -26,16 +24,18 @@
test.chain.append(flowtest);
var replacetest = [ function PC_LOCAL_REPLACE_VIDEOTRACK(test) {
var stream = test.pcLocal._pc.getLocalStreams()[0];
var track = stream.getVideoTracks()[0];
var sender = test.pcLocal._pc.getSenders().find(isSenderOfTrack, track);
var oldstream = test.pcLocal._pc.getLocalStreams()[0];
var oldtrack = oldstream.getVideoTracks()[0];
var sender = test.pcLocal._pc.getSenders().find(isSenderOfTrack, oldtrack);
ok(sender, "track has a sender");
var newtrack;
return navigator.mediaDevices.getUserMedia({video:true, fake: true})
.then(newStream => {
newtrack = newStream.getVideoTracks()[0];
isnot(newtrack, track, "replacing with a different track");
isnot(newStream, stream, "from a different stream");
var audiotrack;
return navigator.mediaDevices.getUserMedia({video:true, audio:true, fake:true})
.then(newstream => {
newtrack = newstream.getVideoTracks()[0];
audiotrack = newstream.getAudioTracks()[0];
isnot(newtrack, oldtrack, "replacing with a different track");
isnot(newstream, oldstream, "from a different stream");
return sender.replaceTrack(newtrack);
})
.then(() => {
@ -43,22 +43,56 @@
var stream = test.pcLocal._pc.getLocalStreams()[0];
var track = stream.getVideoTracks()[0];
is(track, newtrack, "track has been replaced in stream");
return sender.replaceTrack(audiotrack)
.then(() => ok(false, "replacing with different kind should fail"),
e => is(e.name, "IncompatibleMediaStreamTrackError",
"replacing with different kind should fail"));
});
} ];
// Do it twice to make sure it still works.
// Do it twice to make sure it still works (does audio twice too, but hey)
test.chain.append(replacetest);
test.chain.append(flowtest);
test.chain.append(replacetest);
test.chain.append(flowtest);
test.chain.append([
function PC_LOCAL_INVALID_REPLACE_VIDEOTRACK(test) {
function PC_LOCAL_REPLACE_VIDEOTRACK_WITHSAME(test) {
var oldstream = test.pcLocal._pc.getLocalStreams()[0];
var oldtrack = oldstream.getVideoTracks()[0];
var sender = test.pcLocal._pc.getSenders().find(isSenderOfTrack, oldtrack);
return sender.replaceTrack(oldtrack) // same track
.then(() => ok(true, "replacing with itself should succeed"));
}
]);
test.chain.append(flowtest);
test.chain.append([
function PC_LOCAL_INVALID_ADD_VIDEOTRACKS(test) {
var stream = test.pcLocal._pc.getLocalStreams()[0];
var track = stream.getVideoTracks()[0];
var sender = test.pcLocal._pc.getSenders().find(isSenderOfTrack, track);
return sender.replaceTrack(track) // same track
.then(() => ok(false, "replacing with itself should fail"),
e => is(e.name, "InvalidParameter",
"replacing with itself should fail"));
try {
test.pcLocal._pc.addTrack(track, stream);
ok(false, "addTrack existing track should fail");
} catch (e) {
is(e.name, "InvalidParameterError",
"addTrack existing track should fail");
}
try {
test.pcLocal._pc.addTrack(track, stream);
ok(false, "addTrack existing track should fail");
} catch (e) {
is(e.name, "InvalidParameterError",
"addTrack existing track should fail");
}
return navigator.mediaDevices.getUserMedia({video:true, fake: true})
.then(differentStream => {
var track = differentStream.getVideoTracks()[0];
try {
test.pcLocal._pc.addTrack(track, stream);
ok(false, "addTrack w/wrong stream should fail");
} catch (e) {
is(e.name, "InvalidParameterError",
"addTrack w/wrong stream should fail");
}
});
}
]);
test.run();

View File

@ -0,0 +1,46 @@
<!DOCTYPE HTML>
<html>
<head>
<script type="application/javascript" src="pc.js"></script>
</head>
<body>
<pre id="test">
<script type="application/javascript">
createHTML({
bug: "1017888",
title: "Renegotiation: replaceTrack followed by adding a second video stream"
});
var test;
runNetworkTest(function (options) {
test = new PeerConnectionTest(options);
test.setMediaConstraints([{video:true}], [{video:true}]);
addRenegotiation(test.chain,
[
function PC_LOCAL_REPLACE_VIDEO_TRACK_THEN_ADD_SECOND_STREAM(test) {
var oldstream = test.pcLocal._pc.getLocalStreams()[0];
var oldtrack = oldstream.getVideoTracks()[0];
var sender = test.pcLocal._pc.getSenders()[0];
return navigator.mediaDevices.getUserMedia({video:true, fake:true})
.then(newstream => {
var newtrack = newstream.getVideoTracks()[0];
return test.pcLocal.senderReplaceTrack(0, newtrack);
})
.then(() => {
test.setMediaConstraints([{video: true}, {video: true}],
[{video: true}]);
return test.pcLocal.getAllUserMedia([{video: true}]);
});
},
]
);
// TODO(bug 1093835):
// figure out how to verify if media flows through the new stream
// figure out how to verify that media stopped flowing from old stream
test.run();
});
</script>
</pre>
</body>
</html>

View File

@ -2089,12 +2089,38 @@ PeerConnectionImpl::ReplaceTrack(MediaStreamTrack& aThisTrack,
MediaStreamTrack& aWithTrack) {
PC_AUTO_ENTER_API_CALL(true);
JSErrorResult jrv;
nsRefPtr<PeerConnectionObserver> pco = do_QueryObjectReferent(mPCObserver);
if (!pco) {
return NS_ERROR_UNEXPECTED;
}
JSErrorResult jrv;
#ifdef MOZILLA_INTERNAL_API
if (&aThisTrack == &aWithTrack) {
pco->OnReplaceTrackSuccess(jrv);
if (jrv.Failed()) {
CSFLogError(logTag, "Error firing replaceTrack success callback");
return NS_ERROR_UNEXPECTED;
}
return NS_OK;
}
nsString thisKind;
aThisTrack.GetKind(thisKind);
nsString withKind;
aWithTrack.GetKind(withKind);
if (thisKind != withKind) {
pco->OnReplaceTrackError(kIncompatibleMediaStreamTrack,
ObString(mJsepSession->GetLastError().c_str()),
jrv);
if (jrv.Failed()) {
CSFLogError(logTag, "Error firing replaceTrack success callback");
return NS_ERROR_UNEXPECTED;
}
return NS_OK;
}
#endif
std::string origTrackId = PeerConnectionImpl::GetTrackId(aThisTrack);
std::string newTrackId = PeerConnectionImpl::GetTrackId(aWithTrack);
@ -2107,11 +2133,14 @@ PeerConnectionImpl::ReplaceTrack(MediaStreamTrack& aThisTrack,
origTrackId,
newStreamId,
newTrackId);
if (NS_FAILED(rv)) {
pco->OnReplaceTrackError(kInvalidMediastreamTrack,
ObString(mJsepSession->GetLastError().c_str()),
jrv);
if (jrv.Failed()) {
CSFLogError(logTag, "Error firing replaceTrack error callback");
return NS_ERROR_UNEXPECTED;
}
return NS_OK;
}
@ -2127,13 +2156,15 @@ PeerConnectionImpl::ReplaceTrack(MediaStreamTrack& aThisTrack,
pco->OnReplaceTrackError(kInvalidMediastreamTrack,
ObString("Failed to replace track"),
jrv);
if (jrv.Failed()) {
CSFLogError(logTag, "Error firing replaceTrack error callback");
return NS_ERROR_UNEXPECTED;
}
return NS_OK;
}
pco->OnReplaceTrackSuccess(jrv);
if (jrv.Failed()) {
CSFLogError(logTag, "Error firing replaceTrack callback");
CSFLogError(logTag, "Error firing replaceTrack success callback");
return NS_ERROR_UNEXPECTED;
}