1542370 - OperationError in addIceCandidate from stop() and rollback

Status: NEW

(Core :: WebRTC: Signaling, defect, P3)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

If an answerer stops a transceiver before all its ICE candidates have come in, what should happen?

STRs:

1. Open https://jsfiddle.net/jib1/k3butq5p/7/
2. Check ☐ Both noise
3. Quickly uncheck ☑ Both noise

Expected result:
No error

Actual result:
4125 OperationError: Cannot set ICE candidate for level=1 mid=1: No such transceiver.
4126 OperationError: Cannot set ICE candidate for level=1 mid=1: No such transceiver.
4135 OperationError: Cannot set ICE candidate for level=1 mid=1: No such transceiver.
4136 OperationError: Cannot set ICE candidate for level=1 mid=1: No such transceiver.
4136 OperationError: Cannot set ICE candidate for level=1 mid=1: No such transceiver.

I'm not sure whether this is to spec or not, but as a JS user, it seems unreasonably difficult to detect and suppress this failure situation.

It seems to me addIceCandidate() could and should merely silently discard these candidates without error. WDYT?

Comment 1

[Byron Campen [:bwc]]

That seems reasonable.

Comment 2

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

I've filed https://github.com/w3c/webrtc-pc/issues/2164

Comment 3

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

The spec issue in comment 2 has been merged, so we should be good to go with fixing this.

Comment 4

[Byron Campen [:bwc]]

Attachment: Bug 1542370: Be a little less zealous about disassociating transceivers. r?mjf

Comment 5

[Byron Campen [:bwc]]

Attachment: Bug 1542370: Updates to wpt to not expect transceiver to null out mid when negotiated away. r?jib

Depends on D39409

Comment 6

[Byron Campen [:bwc]]

Attachment: Bug 1542370: Update some unit-test expectations around transceiver mid becoming null. r?mjf

Depends on D39410

Comment 7

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e33089db32ec225957bd189783456aba9f452447

Comment 8

[Byron Campen [:bwc]]

Try looks good.

Comment 9

[Byron Campen [:bwc]]

So, what's the upshot with the latest spec discussion? It sounds like we aren't going to be able to change the way we handle transceiver mids like we want. Will we need to have some internal slots that remember the old mid?

Comment 10

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

For stop(), I think w3c/webrtc-pc#2196' "If ... stopped, resolve p with undefined" solves it, since JSEP says not to clear the mid until an m-line is recycled, which is much later. I don't think recycling can interfere with ICE, because

1. On the answerer side, calling stop() no longer sets [[Stopped]] in the spec, so I see no way to recycle the m-line back in "stable" already.
2. On the offerer side, calling stop() similarly cannot cause recycling to happen until after a full cycle, I believe.

For rollback, unless I missed something, I don't think our plans to keep mids past recycling helped any. Silencing "rolled-back candidates", would need some [[RolledBackCandidates]] internal slot for sure I think.

But since mids aren't necessarily unique—they can come over the signaling wire after all—I think a candidate's mid would need to be in that internal slot and not exist in any non-stopped transceiver before we could skip it. That, or try to add it first and only silence failures. Thoughts?

Lastly, this feels a bit similar to the "impolite peer ignoring an offer outright" problem, except in that case we have zero shot at fixing it without some new ignoreRemoteDescription(offer) method.

Comment 11

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Any progress here?

Comment 12

[Byron Campen [:bwc]]

No, bug 1591199 has been eating all my time.