[webrtc-pc] Redundant queuing in gathering state computation risks eliding events (#2892)

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

== Redundant queuing in gathering state computation risks eliding events ==
Similar to #2020 but for gathering states (we fixed this for connection states, but not gathering states).

Quoting the [spec](https://w3c.github.io/webrtc-pc/#rtcicetransport):

> When the [ICE Agent](https://w3c.github.io/webrtc-pc/#dfn-ice-agent) indicates that it began gathering a [generation](https://w3c.github.io/webrtc-pc/#dfn-generation) of candidates for an [RTCIceTransport](https://w3c.github.io/webrtc-pc/#dom-rtcicetransport), the user agent MUST queue a task that runs the following steps: [...]
>
> 4. Set transport.[[[IceGathererState]]](https://w3c.github.io/webrtc-pc/#dfn-icegathererstate) to [gathering](https://w3c.github.io/webrtc-pc/#dom-rtcicegathererstate-gathering).
> 5. [Fire an event](https://dom.spec.whatwg.org/#concept-event-fire) named [gatheringstatechange](https://w3c.github.io/webrtc-pc/#event-icetransport-gatheringstatechange) at transport.
>
> 6. [Update the ICE gathering state](https://w3c.github.io/webrtc-pc/#update-ice-gathering-state) of connection.

Since [Update the ICE gathering state](https://w3c.github.io/webrtc-pc/#update-ice-gathering-state) ALSO queues a task, we're queuing a task from inside another, to, among other things:

> 2. Let newState be the value of deriving a new state value as described by the [RTCIceGatheringState](https://w3c.github.io/webrtc-pc/#dom-rtcicegatheringstate) enum.

@docfaraday noticed this puts us at risk of eliding `icegatheringstatechange` events, since the state derived by the time the second queued task runs may be different.

The solution seems to be to fire both `gatheringstatechange` (on transport) and `icegatheringstatechange` (on connection) from the same task, like we did with the connection states in https://github.com/w3c/webrtc-pc/pull/2400 and https://github.com/w3c/webrtc-pc/pull/2444 to solve similar concerns.

### Web compat
WPT coverage here is poor, but this [fiddle](https://jsfiddle.net/jib1/rt36ns50/) shows only Safari is firing `"gathering"` on the ice transport at all (see `sender.transport.iceTransport.ongatheringstatechange`), so there should be time to align timing and ordering here with the rest of the spec.

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


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

Received on Friday, 18 August 2023 21:08:12 UTC