Skip to content

Commit

Permalink
Revert "Revert "WPT tests for setRemoteDescription for adding and rem…
Browse files Browse the repository at this point in the history
…oving tracks.""

This reverts commit 4e8a1ff7c4f07b2154a40f9ff50b418ecf6df0a9.

Reason for revert:
The original CL was reverted because the eventSequence keeping track of the order
of ontrack firing and setRemoteDescription resolving was not modified when ontrack
fired, rather upon a promise resolving that was resolved when ontrack fires. This
added a delay and the order of promise then() callbacks became indeterminate. In
this reland, ontrack has been modified to do eventSequence += 'ontrack;' instead.

The original CL was reverted because of expecting:
  FAIL ontrack fires before setRemoteDescription resolves. assert_equals: expected
  "ontrack;setRemoteDescription;" but got "setRemoteDescription;ontrack;"
But got:
  PASS ontrack fires before setRemoteDescription resolves.

Original change's description:
> Revert "WPT tests for setRemoteDescription for adding and removing tracks."
>
> This reverts commit 7224091bcdc52f1d9c7f6a8eecb3e7414f29aaeb.
>
> Reason for revert: causes tests failures on mac, e.g. https://uberchromegw.corp.google.com/i/chromium.mac/builders/Mac10.11%20Tests/builds/19421
>
> May or may not be related to an unrelated revert that followed this original patch, https://chromium-review.googlesource.com/729719
>
> Original change's description:
> > WPT tests for setRemoteDescription for adding and removing tracks.
> >
> > These are behavior-driven tests, each test testing a specific behavior:
> > - addTrack() with a track and no stream should fire ontrack with a
> >   remote track with the same ID and no streams.
> > - addTrack() with a track and a stream should fire ontrack with a
> >   remote track and stream with matching IDs.
> > - addTrack() with two tracks and a shared stream should fire ontrack
> >   twice with remote tracks and a shared remote stream, with matching
> >   IDs.
> > - addTrack() with a track and two streams should fire ontrack with a
> >   track and two streams with matching IDs.
> > - ontrack should fire before setRemoteDescription()'s promise resolves.
> > - ontrack's receiver should match getReceivers().
> > - removeTrack() should not result in a receiver being removed.
> >
> > Unlike RTCPeerConnection-ontrack.https.html, these tests do not rely on
> > transceiver support. When testing a behavior like "fires an event with
> > a track" the test does not test unrelated things like the event being a
> > complete implementation of RTCTrackEvent with transceivers and all.
> >
> > In a follow-up, I will add tests for other behavior associated with the
> > removal of a track, including onmute events firing and the track being
> > removed from the remote streams.
> >
> > Bug: 774303
> > Change-Id: I87d8a79d9e2e385610f749a9146b740cc649cf3f
> > Reviewed-on: https://chromium-review.googlesource.com/719615
> > Commit-Queue: Henrik Boström <[email protected]>
> > Reviewed-by: Taylor Brandstetter <[email protected]>
> > Reviewed-by: Philip Jägenstedt <[email protected]>
> > Cr-Commit-Position: refs/heads/master@{#510241}
>
> [email protected],[email protected],[email protected]
>
> Change-Id: I8684d63e478a2ce63d760d9ec2a973b27f985729
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 774303
> Reviewed-on: https://chromium-review.googlesource.com/730723
> Reviewed-by: Mikel Astiz <[email protected]>
> Commit-Queue: Mikel Astiz <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#510404}

[email protected],[email protected],[email protected],[email protected]

Change-Id: I5f18c4426b9e39afbdf59537ad084c4ae461b54a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 774303
Reviewed-on: https://chromium-review.googlesource.com/731245
Reviewed-by: Henrik Boström <[email protected]>
Commit-Queue: Henrik Boström <[email protected]>
Cr-Commit-Position: refs/heads/master@{#510858}
  • Loading branch information
henbos authored and chromium-wpt-export-bot committed Oct 23, 2017
1 parent 50d08dd commit 0c76451
Show file tree
Hide file tree
Showing 2 changed files with 273 additions and 0 deletions.
51 changes: 51 additions & 0 deletions webrtc/RTCPeerConnection-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -394,3 +394,54 @@ function getTrackFromUserMedia(kind) {
return [track, mediaStream];
});
}

// Obtain |count| MediaStreamTracks of type |kind| and MediaStreams. The tracks
// do not belong to any stream and the streams are empty. Returns a Promise
// resolved with a pair of arrays [tracks, streams].
// Assumes there is at least one available device to generate the tracks and
// streams and that the getUserMedia() calls resolve.
function getUserMediaTracksAndStreams(count, type = 'audio') {
let otherTracksPromise;
if (count > 1)
otherTracksPromise = getUserMediaTracksAndStreams(count - 1, type);
else
otherTracksPromise = Promise.resolve([[], []]);
return otherTracksPromise.then(([tracks, streams]) => {
return getTrackFromUserMedia(type)
.then(([track, stream]) => {
// Remove the default stream-track relationship.
stream.removeTrack(track);
tracks.push(track);
streams.push(stream);
return [tracks, streams];
});
});
}

// Creates an offer for the caller, set it as the caller's local description and
// then sets the callee's remote description to the offer. Returns the Promise
// of the setRemoteDescription call.
function performOffer(caller, callee) {
let sessionDescription;
return caller.createOffer()
.then(offer => {
sessionDescription = offer;
return caller.setLocalDescription(offer);
}).then(() => callee.setRemoteDescription(sessionDescription));
}


// The resolver has a |promise| that can be resolved or rejected using |resolve|
// or |reject|.
class Resolver {
constructor() {
let promiseResolve;
let promiseReject;
this.promise = new Promise(function(resolve, reject) {
promiseResolve = resolve;
promiseReject = reject;
});
this.resolve = promiseResolve;
this.reject = promiseReject;
}
}
222 changes: 222 additions & 0 deletions webrtc/RTCPeerConnection-setRemoteDescription-tracks.https.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
<!doctype html>
<meta charset=utf-8>
<title>RTCPeerConnection.prototype.setRemoteDescription - add/remove remote tracks</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="RTCPeerConnection-helper.js"></script>
<script>
'use strict';

// Test is based on the following editor draft:
// https://w3c.github.io/webrtc-pc/archives/20171002/webrtc.html

// The following helper functions are called from RTCPeerConnection-helper.js:
// getUserMediaTracksAndStreams
// performOffer
// Resolver

// These tests are concerned with the observable consequences of processing
// the addition or removal of remote tracks, including events firing and the
// states of RTCPeerConnection, MediaStream and MediaStreamTrack.

async_test(t => {
const caller = new RTCPeerConnection();
const callee = new RTCPeerConnection();
return getUserMediaTracksAndStreams(1)
.then(t.step_func(([tracks, streams]) => {
let localTrack = tracks[0];
caller.addTrack(localTrack);
let offerPromise = performOffer(caller, callee);
callee.ontrack = t.step_func(trackEvent => {
let remoteTrack = trackEvent.track;
assert_equals(remoteTrack.id, localTrack.id,
'Expected local and remote track IDs to match.');
assert_equals(trackEvent.streams.length, 0,
'Expected remote track not to belong to a stream.');
t.done();
});
return offerPromise;
}))
.catch(t.step_func(reason => {
assert_unreached(reason);
}));
}, 'addTrack with a track and no stream makes ontrack fire with a track and no stream.');

async_test(t => {
const caller = new RTCPeerConnection();
const callee = new RTCPeerConnection();
return getUserMediaTracksAndStreams(1)
.then(t.step_func(([tracks, streams]) => {
let localTrack = tracks[0];
let localStream = streams[0];
caller.addTrack(localTrack, localStream);
let offerPromise = performOffer(caller, callee);
callee.ontrack = t.step_func(trackEvent => {
assert_equals(trackEvent.streams.length, 1,
'Expected track event to fire with a single stream.');
let remoteTrack = trackEvent.track;
let remoteStream = trackEvent.streams[0];
assert_equals(remoteTrack.id, localTrack.id,
'Expected local and remote track IDs to match.');
assert_equals(remoteStream.id, localStream.id,
'Expected local and remote stream IDs to match.');
assert_array_equals(remoteStream.getTracks(), [remoteTrack],
'Expected the remote stream\'s tracks to be the remote track.');
t.done();
});
return offerPromise;
}))
.catch(t.step_func(reason => {
assert_unreached(reason);
}));
}, 'addTrack with a track and a stream makes ontrack fire with a track and a stream.');

async_test(t => {
const caller = new RTCPeerConnection();
const callee = new RTCPeerConnection();
return getUserMediaTracksAndStreams(2)
.then(t.step_func(([tracks, streams]) => {
let localTrack1 = tracks[0];
let localTrack2 = tracks[1];
let localStream = streams[0];
caller.addTrack(localTrack1, localStream);
caller.addTrack(localTrack2, localStream);
let offerPromise = performOffer(caller, callee);
callee.ontrack = t.step_func(trackEvent => {
assert_equals(trackEvent.streams.length, 1,
'Expected track event to fire with a single stream.');
let remoteTrack1 = trackEvent.track;
let remoteStream = trackEvent.streams[0];
assert_equals(remoteTrack1.id, localTrack1.id,
'Expected first remote track ID to match first local track ID.');
assert_equals(remoteStream.getTracks().length, 2,
'Expected the remote stream to contain two tracks.');
callee.ontrack = t.step_func(trackEvent => {
assert_equals(trackEvent.streams.length, 1,
'Expected track event to fire with a single stream.');
let remoteTrack2 = trackEvent.track;
assert_equals(trackEvent.streams[0], remoteStream,
'Expected both track events to fire with the same remote stream.');
assert_equals(remoteTrack2.id, localTrack2.id,
'Expected second remote track ID to match second local track ID.');
assert_equals(remoteStream.getTracks().length, 2,
'Expected the remote stream to contain two tracks.');
assert_array_equals(remoteStream.getTracks(), [remoteTrack1, remoteTrack2],
'Expected the remote stream\'s tracks to be the [first, second] remote tracks.');
t.done();
});
});
return offerPromise;
}))
.catch(t.step_func(reason => {
assert_unreached(reason);
}));
}, 'addTrack with two tracks and one stream makes ontrack fire twice with the tracks and shared stream.');

async_test(t => {
const caller = new RTCPeerConnection();
const callee = new RTCPeerConnection();
return getUserMediaTracksAndStreams(2)
.then(t.step_func(([tracks, streams]) => {
let localTrack = tracks[0];
let localStreams = streams;
caller.addTrack(localTrack, localStreams[0], localStreams[1]);
let performOffer = performOffer(caller, callee);
callee.ontrack = t.step_func(trackEvent => {
assert_equals(trackEvent.streams.length, 2,
'Expected the track event to fire with two streams.');
let remoteTrack = trackEvent.track;
let remoteStreams = trackEvent.streams;
assert_equals(remoteTrack.id, localTrack.id,
'Expected local and remote track IDs to match.');
assert_equals(remoteStreams[0].id, localStreams[0].id,
'Expected the first remote stream ID to match the first local stream ID.');
assert_equals(remoteStreams[1].id, localStreams[1].id,
'Expected the second remote stream ID to match the second local stream ID.');
assert_array_equals(remoteStreams[0].getTracks(), [remoteTrack],
'Expected the remote stream\'s tracks to be the remote track.');
assert_array_equals(remoteStreams[1].getTracks(), [remoteTrack],
'Expected the remote stream\'s tracks to be the remote track.');
t.done();
});
return performOffer;
}))
.catch(t.step_func(reason => {
assert_unreached(reason);
}));
}, 'addTrack with a track and two streams makes ontrack fire with a track and two streams.');

async_test(t => {
const caller = new RTCPeerConnection();
const callee = new RTCPeerConnection();
let eventSequence = '';
return getUserMediaTracksAndStreams(1)
.then(t.step_func(([tracks, streams]) => {
caller.addTrack(tracks[0]);
let ontrackResolver = new Resolver();
callee.ontrack = () => {
eventSequence += 'ontrack;';
ontrackResolver.resolve();
}
return Promise.all([
ontrackResolver.promise,
performOffer(caller, callee).then(() => {
eventSequence += 'setRemoteDescription;';
})
]);
}))
.then(t.step_func(() => {
assert_equals(eventSequence, 'ontrack;setRemoteDescription;');
t.done();
}))
.catch(t.step_func(reason => {
assert_unreached(reason);
}));
}, 'ontrack fires before setRemoteDescription resolves.');

async_test(t => {
const caller = new RTCPeerConnection();
const callee = new RTCPeerConnection();
return getUserMediaTracksAndStreams(1)
.then(t.step_func(([tracks, streams]) => {
caller.addTrack(tracks[0]);
let offerPromise = performOffer(caller, callee);
callee.ontrack = t.step_func(trackEvent => {
assert_array_equals(callee.getReceivers(), [trackEvent.receiver]);
t.done();
});
return offerPromise;
}))
.catch(t.step_func(reason => {
assert_unreached(reason);
}));
}, 'ontrack\'s receiver matches getReceivers().');

async_test(t => {
const caller = new RTCPeerConnection();
const callee = new RTCPeerConnection();
return getUserMediaTracksAndStreams(1)
.then(t.step_func(([tracks, streams]) => {
let sender = caller.addTrack(tracks[0]);
assert_true(sender != null);
let offerPromise = performOffer(caller, callee);
callee.ontrack = t.step_func(trackEvent => {
let receivers = callee.getReceivers();
assert_equals(receivers.length, 1,
'Expected getReceivers() to be the track event\'s receiver.');
caller.removeTrack(sender);
performOffer(caller, callee)
.then(t.step_func(() => {
assert_array_equals(callee.getReceivers(), receivers,
'Expected the set of receivers to remain the same.');
t.done();
}));
});
return offerPromise;
}))
.catch(t.step_func(reason => {
assert_unreached(reason);
}));
}, 'removeTrack does not remove the receiver.');

</script>

0 comments on commit 0c76451

Please sign in to comment.