w3c / webrtc-extensions

A repository for experimental additions to the WebRTC API
https://w3c.github.io/webrtc-extensions/
Other
58 stars 19 forks source link

Unclear when all side-effects of setSelectedCandidatePair have taken effect #182

Closed sam-vi closed 10 months ago

sam-vi commented 10 months ago

The setSelectedCandidatePair method in § 9. RTCIceTransport extensions causes the selected candidate pair to change asynchronously.

This has several side-effects:

  1. The candidate pair returned by getSelectedCandidatePair changes, backed by the [[SelectedCandidatePair]] internal slot.
  2. A selectedcandidatepairchange event is fired.
  3. The transport's RTCIceTransportState may change.

This would be a good scenario to return a promise that resolves once all the changes have been applied. Then it is possible to do this:

await transceiver.transport.iceTransport.setSelectedCandidatePair(foo);
const bar = transceiver.transport.iceTransport.getSelectedCandidatePair();
// foo and bar now match
jan-ivar commented 10 months ago
  1. A selectedcandidatepairchange event is fired.

I'm supportive of a promise. Even though the following might seem to suffice:

transceiver.transport.iceTransport.setSelectedCandidatePair(foo);
await new Promise(r => transceiver.transport.iceTransport.selectedcandidatepairchange = r);
const bar = transceiver.transport.iceTransport.getSelectedCandidatePair();
// foo and bar now match

...it risks a race with the ICE agent once in a while if the method is called when a task is already queued to fire the event.

This would be a good scenario to return a promise that resolves once all the changes have been applied

UNLESS we elide firing selectedcandidatepairchange from the method — WebRTC has some precedent of not firing events on JS from JS's own actions, whether that was a good decision or not has been questioned — then we need to nail down the order: whether the event fires before or after the promise resolves.