[webrtc-pc] Algorithm touches state after firing an event (#2893)

jan-ivar has just created a new issue for https://github.com/w3c/webrtc-pc:

== Algorithm touches state after firing an event ==
The [spec](https://w3c.github.io/webrtc-pc/#rtcicetransport) says:

> When the [ICE Agent](https://w3c.github.io/webrtc-pc/#dfn-ice-agent) is finished gathering a [generation](https://w3c.github.io/webrtc-pc/#dfn-generation) of candidates for an [RTCIceTransport](https://w3c.github.io/webrtc-pc/#dom-rtcicetransport), and those candidates have been surfaced to the application, the user agent MUST queue a task that runs the following steps: [...]
> 
> 5. [Fire an event](https://dom.spec.whatwg.org/#concept-event-fire) named [icecandidate](https://w3c.github.io/webrtc-pc/#event-icecandidate) using the [RTCPeerConnectionIceEvent](https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnectioniceevent) interface with the candidate attribute set to newCandidate at connection.
> 6. [...]
> 7. Set transport.[[[IceGathererState]]](https://w3c.github.io/webrtc-pc/#dfn-icegathererstate) to [complete](https://w3c.github.io/webrtc-pc/#dom-rtcicegathererstate-complete).

Since [fire an event](https://dom.spec.whatwg.org/#concept-event-fire) is just a JS-callback, touching any kind of state afterwards is prone to security exploits. We need to avoid that.

Also, once #2892 is addressed by flattening things, we'd have 4 events fired on the same task here (in order):
1. the `{candidate: ""}` end-of-candidates icecandidate,
2.  gatheringstatechange,
3. icegatheringstatechange, and
4. the legacy `null` icecandidate.

That's a bit weird, as moving the state transition ahead of firing would make `"complete"` observable in the  end-of-candidates icecandidate callback ahead of seeing `gatheringstatechange`, which would be different from today.

It seems cleaner to queue two tasks here:
 1. one to surface the end-of-candidates candidate, treating it like just another candidate, followed by
 2. a second task for the state transition.

This would preserve today's behavior where gatheringstate changes to `"complete"` afterwards, and would avoid surfacing two `icecandidate` events in the same task, which could trip up apps using promises.

Please view or discuss this issue at https://github.com/w3c/webrtc-pc/issues/2893 using your GitHub account


-- 
Sent via github-notify-ml as configured in https://github.com/w3c/github-notify-ml-config

Received on Monday, 21 August 2023 20:26:27 UTC