Issue with "Establishing a connection: The WebRTC perfect negotiation pattern": unclear what's being recommended

[cdgru]

MDN URL: https://developer.mozilla.org/en-US/docs/Web/API/WebRTC_API/Perfect_negotiation

What information was incorrect, unhelpful, or incomplete?

Specific section or headline?

What did you expect to see?

Did you test this? If so, how?

MDN Content page report details

• Folder: en-us/web/api/webrtc_api/perfect_negotiation
• MDN URL: https://developer.mozilla.org/en-US/docs/Web/API/WebRTC_API/Perfect_negotiation
• GitHub URL: https://github.com/mdn/content/blob/master/files/en-us/web/api/webrtc_api/perfect_negotiation/index.html
• Document last modified: 2021-01-02T19:18:52.000Z

[cdgru]

I am concerned about makingOffer. It is used in A), no more used/needed in B), but still needed in C) for offerCollision.

A) pc.onnegotiationneeded

let makingOffer = false;
pc.onnegotiationneeded = async () => {
  try {
    makingOffer = true;
    await pc.setLocalDescription();
    signaler.send({ description: pc.localDescription });
  } catch(err) {
    console.error(err);
  } finally {
    makingOffer = false;
  }
};

B) in the last section "restartice" where the focus in oniceconnectionstatechange, the (new?) pc.onnegotiationneeded handler doen not use makingOffer anymore:

pc.onnegotiationneeded = async options => {
  await pc.setLocalDescription(await pc.createOffer(options));
  signaler.send({ description: pc.localDescription });
};
pc.oniceconnectionstatechange ...

C) What I am concerned about, is the signaler.onmessage handler, which still using makingOffer for offerCollision:

let ignoreOffer = false;
signaler.onmessage = async ({ data: { description, candidate } }) => {
  try {
    if (description) {
      const offerCollision = (description.type == "offer") &&
                             (makingOffer || pc.signalingState != "stable");
      ignoreOffer = !polite && offerCollision;
      if (ignoreOffer) {
        return;
      }  ...

What You are recommending for makingOffer?
Keep „let makingOffer = false;“ from A) but looks like that it'll never be changed or am I missing something else?

[fippo]

@jan-ivar @henbos can you take a look?

[henbos]

The code quoted in B) is under the section "The old way" but just after this section in the article there is the section "Using restartIce()" which does not explicitly call createOffer(), it just calls restartIce() and relies on the Perfect Negotiation logic doing everything correctly for you.

[henbos]

Calling restartIce() triggers onnegotiationneeded and that event is still using makingOffer so you're safe.

We could update the article not to talk about the old versus the new way of doing things, it sounds a bit like a sales pitch for Perfect Negotiation or an interesting fact about how things were supposed to work before, but it's not really helpful in a guide to show how not to do it if you're only interested in how to do it

[sideshowbarker]

We could update the article not to talk about the old versus the new way of doing things

That sounds great to me

but it's not really helpful in a guide to show how not to do it if you're only interested in how to do it

100% agreed

[jan-ivar]

It should also be checked against the canonical example in the spec https://www.w3.org/TR/webrtc/#perfect-negotiation-example (since that did undergo a couple of iterations).