Adding note about when ICE restarts are recommended.

[taylor-b]

Fixes #1445.

[fippo]

I think @nils-ohlmeier will strongly disagree

[nils-ohlmeier]

Indeed I disagree with the statement in this pull request. I'll comment in more details in #1445

[fippo]

if you used 'failed' instead of 'disconnected' 👍 on giving guidance (even if minimal)

[lgrahl]

I can't count how many times I've seen disconnected on a connection between a browser and a mobile app. But an ICE restart wouldn't have helped in 99% of the cases. I would agree with failed.
Edit: @fippo was faster. :)

[taylor-b]

Failed occurs after 30 seconds though (or at least, it's supposed to); that's too late to be useful.

[nils-ohlmeier]

Indeed 30 seconds is too late. But since disconnected is a volatile state which depends on browser implementation (see my comment over in #1445 for details) I would highly discourage people from using disconnected as the only input for your decision when to attempt an ICE restart.

[lgrahl]

What is too late highly depends on the use case. If its a video conference - certainly. If it's a long term data only connection, >10 seconds can be fine. So, maybe we can say something like: "If in the disconnected state for a longer period of time, you may want to do an ICE restart."

[taylor-b]

Even with Firefox's implementation, I'd argue that doing an ICE restart on "disconnected" is far better than doing it on "failed" or doing nothing. And even if it ends up being pointless, it shouldn't have a negative effect besides wasting some bandwidth.

I tried to allude to your points:

Performing an ICE restart is recommended when iceConnectionState transitions to "disconnected", which may occur due to a flaky network but may also indicate an issue requiring an ICE restart to repair. Using other sources of information (such as from getStats) could help make a more informed decision.

Is there alternative wording you'd suggest? Replace "is recommended" with "may be desired" or something less strong? I'm completely open to suggestions, I just want to say something about this.

[lgrahl]

I only want to prevent overly dramatic reactions on the disconnected state. As an example: I know of an app that did this immediately every time the state went into disconnected, draining the battery pretty fast.

[fippo]

Giving concrete guidance here is hard since "it depends". Also as Lennart points out a videochat has different requirements from a filetransfer.

I can sign off this:

Performing an ICE restart is required when iceConnectionState transitions to "failed".

For a recommendation:

An application may additionally choose to listen for the iceConnectionState transition to 
"disconnected" and then use other sources of information (such as using getStats to 
measure if the number of bytes sent or received over the next couple of seconds 
increases) to determine whether an ice restart is advisable.

So disconnected, getStats, wait 2 seconds, do getStats again (and also check if iceConnectionState is still disconnected!), figure out nothing is sent/received, then do an ice restart.

Thankfully @henbos fixed this issue for me which also allows this on the sender side

[nils-ohlmeier]

I like @fippo proposal a lot better. Because with the change in the PR I'm afraid to many will read the first sentence and only implement that, ignoring the second sentence. And this fear is based on lots of people don't understanding that Firefox disconnected state behaves differently then Chromes.

[taylor-b]

In that case, going with fippo's suggestion; thanks!