Queue two tasks upon finishing ICE gathering, and fire gatheringstatechange & icegatheringstatechange in same task

[jan-ivar]

Fixes #2892.
Fixes #2893.
Fixes #2896.

cc @docfaraday

---

Preview | Diff

[alvestrand]

This looks good to me.
The general principle of state-setting seems to be that we change the state before we fire the event that informs the user of the state change; the old code violated this, so was clearly wrong.

[docfaraday]

I think we might be missing the steps that add an a=end-of-candidates to the local description.

[docfaraday]

Also, it looks like we'll fire duplicate null candidates here. We should only fire the null candidate if the RTCPeerConnection.iceGatheringState just transitioned to "complete".

[jan-ivar]

I think we might be missing the steps that add an a=end-of-candidates to the local description.

Isn't that covered by JSEP? This spec generally doesn't get into that. In any case that seems like a separate issue.

[docfaraday]

I think we might be missing the steps that add an a=end-of-candidates to the local description.

Isn't that covered by JSEP? This spec generally doesn't get into that. In any case that seems like a separate issue.

Hmm. So JSEP handles this when creating a new local description, but does not update the existing local description. Maybe we shouldn't be updating? Anyhow, yeah, separate issue.

[jan-ivar]

duplicate null candidates

Ah, I forgot to remove the now unused "update the ICE gathering state" algorithm. Thanks for catching that!

[alvestrand]

Seems good to merge. Needs test.

[jan-ivar]

WPT tests were merged in web-platform-tests/wpt#44687 (two tests).

https://wpt.fyi/results/webrtc/RTCPeerConnection-iceGatheringState.html

[jan-ivar]

Hi @dontcallmedom I'm getting this error. It says to see developer console, where can I see that? Trying to figure out how to bother you less ;)

  Error: ROR] Function listAmendments threw an error during preProcess.
        Plugin: core/pre-process
          Hint: See developer console.

Also the next step "Check amendment annotations on substantive changes to Rec / check-amendment (pull_reques" seems to have passed, which seems odd if it failed on amendments.

[dontcallmedom]

I'm getting this error. It says to see developer console, where can I see that? Trying to figure out how to bother you less ;)

The way I check it is by running the ReSpec document in my local browser (served via a local http server since otherwise you'd hit CORS issues). The error in the developer console says:

The container with id rtcicetransport marked as amended cannot embed the other container of amendment rtcicetransportstate-failed-description, see https://github.com/w3c/webrtc-pc/blob/main/amendments.md for amendments management

The problem here is that you're applying the change to the whole "rtcicetransport" section, which contains already a place with another change; I think the solution would probably to add a <div> wrapping the more specific changes; I think you would also need to markup the deletion of the "Update the ICE gathering state" section.

I'm happy to leave it to you, or do it on my end as you prefer.

Also the next step "Check amendment annotations on substantive changes to Rec / check-amendment (pull_reques" seems to have passed, which seems odd if it failed on amendments.

That job's role is only to check that a non-editorial PR is listed in amendments; let me know if you see a better name to use for the check.