1171634 - DeferredCreateOffer causes tracks added AFTER createOffer to sometimes be included in offer, because GMP

Status: RESOLVED DUPLICATE of bug 1544107

(Core :: WebRTC, defect, P4)

Description

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

Basically, createOffer implementation does a one-time dispatch iff GMP is not loaded (see DeferredCreateOffer). With the fix in Bug 1169386 applied, this becomes observable from JS, which is not spec compliant:

    pc.addTrack(X); pc.createOffer(); pc.addTrack(Y);

should produce an offer that includes X only. Instead sometimes it includes X and Y, sometimes only X.

STR:
- Apply patches in Bug 1169386.
- ./mach mochitest dom/media/tests/mochitest/test_peerConnection_operations.html

Expected result:
- Test passes.

Actual result:
> 5 INFO TEST-UNEXPECTED-FAIL | dom/media/tests/mochitest/test_peerConnection_operations.html | Only one track should be in offer - got 2, expected 1

Alternate STR:
- Apply patches in Bug 1169386.
- Open a fresh Nightly with no other content open.
- Open URL and hit [Start]. Twice.

The first time I get 2. The second time I get 1 (the number of tracks in the offer).

Comment 1

[Martin Thomson [:mt:]]

It looks like I'm about to compound the problem with the certificate management API, which has a similar setup regime as GMP.

Capturing the set of tracks at the time that the method is called might be best, but I have JS-side delays (this is C++-side).

Comment 2

[Maire Reavy [:mreavy]]

Jib - This seems like a "normal" bug (P3) -- one that we should fix in the next 2 or 3 release cycles.  Do you agree or is this more or less important?

Comment 3

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

It's been like this for a while and nobody's cared, but I'm not psyched about landing new code with the wrong behavior out the gate, as these seem like basic API behaviors that we'd be happier nailing down sooner rather than later. I also can't land the test in Bug 1169386 until this is fixed.

I would prioritize this over new features, but probably not uplift over it.

Comment 4

[Bulk Bug Changes for mreavy's org]

Mass change P3->P4 to align with new Mozilla triage process.

Comment 6

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

To clarify, the spec was changed since comment 0. Instead:

 pc.addTrack(X); pc.createOffer(); pc.addTrack(Y);

should produce an offer that includes BOTH X and Y. See bug 1544107. Instead sometimes it includes X and Y, sometimes only X.

This is a relief perhaps for GMP and certificates, since it's perfectly ok to block createOffer. The remaining problem then is that whenever createOffer does take some time, it must pick up on JS state changes before succeeding.

The simplest way to accomplish this in our architecture where the core of createOffer today is synchronous, seems to be to do that part in the queued task that resolves or rejects.

We should be able to do that in bug 1544107, so I'm closing this one as a dup to avoid confusion.