Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
sam-vi opened this issue Oct 31, 2023 · 1 comment · Fixed by #183
Closed

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

sam-vi opened this issue Oct 31, 2023 · 1 comment · Fixed by #183
Assignees

Comments

@sam-vi
Copy link
Contributor

sam-vi commented Oct 31, 2023

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
Copy link
Member

jan-ivar commented Oct 31, 2023

  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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants