When is negotiation complete?

[jan-ivar]

A problem arose while writing perfect negotiation wpt tests: When is negotiation complete?
You know, something like:

const transceiver = pc.addTransceiver("video");
await /* some event or promise */
assert_equals(transceiver.currentDirection, "sendonly", "negotiates to sendonly");

Naively, one might think this works:

const state = (pc, s) => new Promise(r => pc.onsignalingstatechange =
                                          () => pc.signalingState == s && r());

const transceiver = pc.addTransceiver("video");
await state(pc, "stable");
assert_equals(transceiver.currentDirection, "sendonly", "negotiates to sendonly");

Unfortunately, outside a lab, →"stable" might come from rollback, or answering a remote offer.
Or even from juuust missing our own negotiation train triggered by a previous local action.

This leaves us stuck writing action-specific spin-tests, not a great API:

const transceiver = pc.addTransceiver("video");
while (!transceiver.currentDirection) {
  await state(pc, "stable");
}
assert_true(true, "we didn't time out!");
assert_equals(transceiver.currentDirection, "sendonly", "negotiates to sendonly");

So I tried solving this in JS by dispatching my own negotiated event in SRD(answer):

// - The perfect negotiation logic, separated from the rest of the application ---

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

      ignoreOffer = !polite && offerCollision;
      if (ignoreOffer) {
        return;
      }
      await pc.setRemoteDescription(description); // SRD rolls back as needed
      if (description.type == "offer") {
        await pc.setLocalDescription();
        signaling.send({description: pc.localDescription});
      } else {
        pc.dispatchEvent(new Event("negotiated")); // <--- here!
      }
    } else if (candidate) {

This avoids rollbacks and remote offers, but we still have to account for just missing a local train:

const negotiated = pc => new Promise(r => pc.addEventListener("negotiated", r,
                                                              {once: true});

const transceiver = pc.addTransceiver("video");
await negotiated(pc);
if (!transceiver.currentDirection) {
  await negotiated(pc); // catch the next train
}
assert_equals(transceiver.currentDirection, "sendonly", "We're negotiated!");

This avoids the dreaded while loop. But who's going to remember this over some intermittent?

[jan-ivar]

Proposal A: negotiationcomplete event from SRD(answer) only if renegotiation is not needed.

const transceiver = pc.addTransceiver("video");
await new Promise(r => pc.onnegotiationcomplete = r);
assert_equals(transceiver.currentDirection, "sendonly", "negotiates to sendonly");

A downside is subsequent actions might delay this event.

Proposal B: Expose a negotiationneeded boolean attribute.

const transceiver = pc.addTransceiver("video");
while (pc.negotiationneeded) await state(pc, "stable");
assert_equals(transceiver.currentDirection, "sendonly", "negotiates to sendonly");

Complication: once set, we MUST fire negotiationneeded event, to meet JS expectations.

Proposal C: Expose a negotiationcomplete promise attribute.

const transceiver = pc.addTransceiver("video");
await pc.negotiationcomplete;
assert_equals(transceiver.currentDirection, "sendonly", "negotiates to sendonly");

Its fulfillment would not be delayed by subsequent actions, besting proposals A & B
(browsers would replace this attribute with a new promise each time a train leaves the station).

Think of it as the promise addTransceiver etc. should have returned.

Proposal D: The Road.

[alvestrand]

What is the question that needs answering here?
Why would the app need to know that negotiation is complete?
I can see two options:

• Needs to know when it can be certain the track is signalled to the other side
• Needs to know when it can turn off the negotiation machinery because it's not needed

The last one is silly (outside of testing), we shouldn't bother with designing for that.
The first one seems less frivolous. The logical thing here seems to be to answer exactly that question - which would seem to indicate that an event on the sender is the Right Thing.

[henbos]

If this is just for testing...

Wouldn't the following be able to tell if negotiation was needed in most cases? There might be special cases like when pc closes, if data channels are used or if setCodecPreference or setStreams has happened, but those would probably be separate tests anyway:

function isNegotiationNeeded() {
  for (transceiver of pc.getTransceivers()) {
    if (transceiver.currentDirection != transceiver.direction)
      return true;
  }
  return false;
}

And at strategic points in WPTs, probably when returning to stable, you could...

const wasNegotiationNeeded = isNegotiationNeeded();
let didFireNegotiationNeeded = false;
pc.onnegotiationneeded = () => { didFireNegotiationNeeded = true; }
await new Promise(r => setTimeout(r, 0));
assert_equals(wasNegotiationNeeded, didFireNegotiationNeeded);

With regards to Proposal B (Expose a negotiationneeded boolean attribute)

Complication: once set, we MUST fire negotiationneeded event, to meet JS expectations.

Why is this complicated? Why is having to fire the event a problem? Because of "maybe fire" logic...?

[henbos]

Also: The test should know whether or not negotiation should fire, and by the time you have reached stable you should already know that the PC has performed the check to determine if negotiation is needed, so why couldn't you assert that within the next execution cycle onnegotiatonneeded must have or must not have happened?

[jan-ivar]

Why would the app need to know that negotiation is complete?
... Needs to know when it can be certain the track is signalled to the other side

That's specific to addTransceiver and addTrack. Here are all our modification methods:

1. pc.addTransceiver(kind) - When does other side have transceiver mid/track & can send/recv?
2. pc.addTrack(track, stream) - same as above
3. pc.createDataChannel(name) - dc.onopen
4. pc.restartIce() - Was ICE restart successful or did it fail?
5. pc.removeTrack(sender) - When has track been removed remotely?
6. transceiver.direction = newDirection - When has track been added/removed remotely?
7. transceiver.sender.setStreams(streams) - When have streams appeared remotely?
8. transceiver.stop() - pc.onsigalingstate in "stable" && transceiver.currentDirection == "stopped"

There are still 4 question marks in that list. Solving them all with one solution seems desirable.

Moreover, I wouldn't dismiss the value of this information to an application. E.g.

async function addParticipant(someArgs) {
  pc.addTransceiver("audio");
  return pc.negotiationcomplete;
}

button.onclick = async () => {
  await addParticipant(someArgs);
  writeOnScreen(`User ${name} has joined. Say hello!`);
};

Being able to authoritatively make statements about a connection seems desirable over, e.g.

writeOnScreen(`User ${name} should have joined. Try saying hello!`);

[jan-ivar]

Wouldn't the following be able to tell if negotiation was needed in most cases?

Why specify check if negotiation is needed if we're forcing JS to write their own buggy version?

We'll end up with lots of JS that work in "most cases" but not all. A recipe for sadness.

We should at minimum expose pc.negotiationneeded then.

why couldn't you assert that within the next execution cycle onnegotiatonneeded must have or must not have happened?

Great question, and the argument against exposing pc.negotiationneeded years ago, as I recall!

const negotiationNotNeeded = !await Promise.race([
  new Promise(r => pc.onnegotiationneeded = r),
  new Promise(r => setTimeout(r))
]); 

Unfortunately, it is no longer reliable, since we decided to chain ONN in w3c/webrtc-pc#2405 and w3c/webrtc-pc#2478

So this is a functional spec regression I think we need to fix.