oficina / meet Public

room.js: the polite side gives way before it offers

Two peers sharing a screen at once made the polite one roll back its own offer, and Chromium answers a rollback of an offer that added an m-line by rebuilding every send stream on the connection: the camera's comes back on the same SSRC having encoded nothing and never encodes again, on an m-line that goes on reading sendrecv with a live track while the far side's receiver goes muted. So the side that would roll back now waits 300 ms and offers only if it is still stable — the offer already on its way is answered from stable, nothing collides, nothing rolls back. Once per negotiation, or two peers renegotiating in turn keep one of them permanently a beat behind.

Measured in the gate's mutualshare scenario, which is now un-skipped: three failures in five before, none in fourteen after, 30/30 twice. The prescribed fix was implemented and measured first and does not work — releasing the unassociated share transceiver before the rollback never fires as written (setLocalDescription has already assigned it a mid), and asking currentDirection instead makes it fire and the cameras die anyway, because it is the rollback and not the transceiver that rebuilds the streams. Both it and round 1's regainShare, which could never add anything from have-remote-offer, are gone rather than left dead. The two other suspects are cleared and now pinned by tests: the camera transceiver keeps mid 1 sendrecv throughout, and removeShareFrom's fallback never fires because tx.stop() does not throw.

negotiationneeded's body is a named function so a node test can drive it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CzvnuKhLzudvq7jZ7SvVt2

Paul Campbell pushed by paul@keymail.dev 1 month ago 8797ea9f23613ce54d81bab516f5c35fb1a4207c parent a11ad9a

5 files changed, +391 −119

internal/meet/static/room.js +104 −39
internal/meettest/browser_test.go +10 −11
test/browser/negotiation.test.mjs +1 −38
test/browser/room-lane.test.mjs +276 −0
test/browser/room-share.test.mjs +0 −31

CI passed — run details

diff --git a/internal/meet/static/room.js b/internal/meet/static/room.js

index f258f35..38507b4 100644

--- a/internal/meet/static/room.js

+++ b/internal/meet/static/room.js

@@ -134,6 +134,23 @@

// for one probe and one retry, and short enough that a tab which has

// really lost its participation is not left looking live.

const MIN_RECOVERY = 5000;

+ // §5.1: how long the polite peer waits before offering. Perfect

+ // negotiation's answer to glare is a rollback on the polite side, and

+ // in Chromium rolling back an offer that added an m-line rebuilds the

+ // whole connection's send streams: the camera's comes back on the same

+ // SSRC having encoded nothing, on an m-line that goes on reading

+ // sendrecv with a live track, and it never encodes again. Two peers

+ // sharing a screen in the same moment is how a call finds that, and

+ // the gate's mutualshare scenario is where it was measured.

+ //

+ // So the peer that would roll back yields instead. A remote offer that

+ // was already on its way arrives while this side is still stable and

+ // is answered rather than collided with, and no rollback happens at

+ // all. The cost is this much delay before the polite side offers, and

+ // only when the other side has nothing to offer — at every moment when

+ // both sides do, which is the case this exists for, the yield ends

+ // early because the remote offer has arrived.

+ const POLITE_YIELD = 300;

// §5.4: the tile key a peer's screen gets, built by shareKey.

const SHARE_SUFFIX = '#share';

const HAS_DISPLAY_MEDIA = typeof navigator !== 'undefined' &&

@@ -968,6 +985,9 @@

// the outside — signalingState is still have-local-offer — and is

// not.

isSettingRemoteAnswerPending: false,

+ // §5.1: whether this negotiation has already had its polite

+ // yield. One per offer, so giving way can never become a habit.

+ yielded: false,

pending: [],

descSeq: 0,

gen: g,

@@ -1044,18 +1064,14 @@

// below leaves a connection that can still be reset and rebuilt,

// rather than a live entry nobody is watching.

- pc.addEventListener('negotiationneeded', async () => {

- if (pcs.get(id) !== entry) return;

- try {

- entry.makingOffer = true;

- await pc.setLocalDescription();

- send(id, 'offer', { description: describe(pc.localDescription) });

- } catch {

- // A failed local description is not fatal: the pair recovers

- // through a reset if it never connects.

- } finally {

- entry.makingOffer = false;

- }

+ // The listener is not async, for the reason the pageshow handler is

+ // not: a rejection escaping an event listener goes where neither a

+ // person nor a gate could see it. The work is a named function, so

+ // it is also the one §5.1 path a node test can drive.

+ pc.addEventListener('negotiationneeded', () => {

+ negotiate(id, entry).catch((err) => {

+ console.error('meet: negotiate', err);

+ });

});

pc.addEventListener('icecandidate', ({ candidate }) => {

@@ -1138,6 +1154,55 @@

return entry;

}

+ // politeYield waits POLITE_YIELD and then answers whether this

+ // connection is still ours and still stable — that is, whether no

+ // remote offer arrived while we waited. False means one did: it is

+ // being answered, and this negotiation belongs to the stable moment

+ // after that rather than to a collision now.

+ async function politeYield(id, entry) {

+ await sleep(POLITE_YIELD);

+ return pcs.get(id) === entry && !state.stopped &&

+ entry.pc.signalingState === 'stable';

+ }

+ // negotiate is §5.1's offering half: one offer per negotiation, with

+ // the polite side's yield in front of it.

+ //

+ // The yield is fix round 2's finding, and it is the whole reason this

+ // function is not two lines. Perfect negotiation's answer to glare is

+ // a rollback on the polite side, and in Chromium rolling back an offer

+ // that added an m-line rebuilds every send stream on the connection:

+ // the camera's comes back having encoded nothing and never encodes

+ // again, on an m-line that goes on reading sendrecv with a live track

+ // the whole time. So the side that would roll back gives way *before*

+ // it offers instead, and the offer that was already on its way is

+ // answered from a stable state with no rollback anywhere.

+ //

+ // Once per negotiation, which is what `yielded` is for: a side that

+ // gave way and came back to find another offer waiting would give way

+ // again, and two peers renegotiating in turn would keep one of them

+ // permanently a beat behind. The second time round it offers and takes

+ // the ordinary collision path, which is what the pattern is for. The

+ // flag is cleared by the offer that actually goes out.

+ async function negotiate(id, entry) {

+ if (pcs.get(id) !== entry) return;

+ if (entry.polite && !entry.yielded) {

+ entry.yielded = true;

+ if (!(await politeYield(id, entry))) return;

+ }

+ try {

+ entry.makingOffer = true;

+ await entry.pc.setLocalDescription();

+ entry.yielded = false;

+ send(id, 'offer', { description: describe(entry.pc.localDescription) });

+ } catch {

+ // A failed local description is not fatal: the pair recovers

+ // through a reset if it never connects.

+ } finally {

+ entry.makingOffer = false;

+ }

// hasLiveTrack answers whether the local stream has a track of this

// kind that could actually carry media. An ended track is not one:

// the browser keeps it in the stream, and `addTrack` will take it,

@@ -1253,12 +1318,6 @@

entry.isSettingRemoteAnswerPending = false; // this one is an offer

await entry.pc.setRemoteDescription(desc);

entry.ignoreOffer = false;

- // §5.4: the polite peer's implicit rollback happened inside that

- // call, and a share transceiver that was still unassociated can

- // come out of it stopped, or with its m-line taken by the offer.

- // Either way the screen is no longer being sent on this

- // connection, and it needs one again.

- regainShare(entry);

entry.descSeq = msg.seq || entry.descSeq;

await drainCandidates(entry);

await entry.pc.setLocalDescription();

@@ -2827,6 +2886,18 @@

// else at all — `sharing`, the `share {on:true}` message, breaks the

// tie. It is last on purpose: it says this peer is showing a screen

// somewhere, not that *this* track is it.

+ //

+ // One hole is left, and it is left knowingly. A peer that joined with

+ // no camera *and* no microphone — startLocal's bottom rung, where

+ // getUserMedia gave nothing at all — has sent this connection nothing

+ // before its screen, so there is no camera mid, no camera stream and

+ // no peer stream, and if this tab has no camera either the direction

+ // says nothing. Everything then rests on the `share` message, which is

+ // sent before the track and is what the tiebreaker reads. If that

+ // message were ever lost, that peer's screen would be captioned as its

+ // face. Nothing in §4.3 loses it — a fan-out message is delivered to

+ // an admitted peer or the sender is told it failed — so the hole is a

+ // consequence of the wire, not of this judgement.

function classifyTrack(known, info, self) {

if (info.kind !== 'video') return 'audio';

if (info.direction === 'recvonly' && self.hasVideo) return 'share';

@@ -2988,27 +3059,6 @@

}

- // shareTxLost: this connection's share transceiver is no longer the

- // one carrying our display track. A rollback can stop it, or give its

- // m-line to the description that rolled us back — and a transceiver

- // that is not sending our track is not a share transceiver, however

- // much it looks like one. Pure, and lifted by name in the node test.

- function shareTxLost(tx, track) {

- if (!tx) return false;

- if (tx.stopped) return true;

- return !(tx.sender && tx.sender.track === track);

- }

- // regainShare puts the screen back on a connection that lost it in a

- // rollback. Called where the rollback happens — inside onOffer, on the

- // far side of setRemoteDescription — and a no-op for every connection

- // that kept it.

- function regainShare(entry) {

- if (!share || !entry) return;

- if (shareTxLost(entry.shareTx, share.track)) entry.shareTx = null;

- addShareTo(entry);

- }

// stopSharing is the off half, and every way out uses it: the button,

// the browser's own stop bar, and the end of the call.

function stopSharing() {

@@ -3041,6 +3091,11 @@

// track *end*; the fallback is for a browser whose transceivers

// cannot be stopped, where nulling the sender and removing it at

// least stops the media and renegotiates.

+ //

+ // Only ever this connection's own share transceiver, which fix round 2

+ // pins with a test: a camera that stopped encoding was that round's

+ // whole hunt, and `removeTrack` on the wrong sender is one of the two

+ // ways this file could have done that to itself.

function removeShareFrom(entry) {

const tx = entry.shareTx;

if (!tx) return;

@@ -3598,6 +3653,16 @@

// connection that has two video tiles, is left out rather than

// credited to the wrong one. A connection with one tile is not

// ambiguous, so a missing mid there still counts.

+ //

+ // What that costs, on a browser that reports no `mid` on inbound-rtp

+ // and no requestVideoFrameCallback either: while a peer is sharing,

+ // *both* of its tiles stop counting — the camera's number stands

+ // still rather than being replaced by the screen's. It is the right

+ // way round (a stale count is better than a wrong one, and only the

+ // gate reads these), and no browser this app supports is in that

+ // position: rVFC has been in Chromium since 83 and in Safari since

+ // 15.4, and every browser without it that carries getStats reports

+ // `mid`.

function frameKey(id, entry, mid) {

if (mid && entry.shareMid && mid === entry.shareMid) return shareKey(id);

if (mid && entry.cameraMid && mid === entry.cameraMid) return id;

diff --git a/internal/meettest/browser_test.go b/internal/meettest/browser_test.go

index 31d67f0..64e7963 100644

--- a/internal/meettest/browser_test.go

+++ b/internal/meettest/browser_test.go

@@ -177,6 +177,16 @@ var scenarios = []string{

// roster for the first door to prefer. The three ICE scenarios and

// staletarget have run in the gate ever since.

var skipped = map[string]string{

+ // FIXED in Task 4's fix round 2 — see task-4-report.md. mutualshare

+ // has run in the gate ever since, and the account below is left as

+ // written because it is what the fix was made against. What it could

+ // not know is where the damage came from: not the m-line matching it

+ // suspected, but the rollback itself. Chromium rebuilds every send

+ // stream on a connection when it rolls back an offer that added an

+ // m-line, and the camera's comes back having encoded nothing. The

+ // fix is that the side which would roll back gives way before it

+ // offers, so nothing collides — see POLITE_YIELD in room.js.

+ //

// Task 4 fix round 2 owns this. The scenario keeps its place above

// and every assertion as written; what follows is exactly what the

// gate printed, and the evidence under it.

@@ -234,17 +244,6 @@ var skipped = map[string]string{

// The fix belongs in room.js and nowhere in this gate. Delete this

// entry to put the scenario back — that, and nothing else, is what

// "fixed" means.

- "mutualshare": "Task 4 fix round 2: a mutual share stops both cameras. " +

- "round 2: member A is still receiving the other's camera (5 -> 5); " +

- "round 2: member B is still receiving the other's camera (6 -> 6); " +

- "round 3: member A is still receiving the other's camera (5 -> 5); " +

- "round 3: member B is still receiving the other's camera (6 -> 6); " +

- "member B still has the camera through the one-sided stop (6 -> 6). " +

- "Every screen assertion passes; the camera m-line keeps sendrecv/sendrecv " +

- "with a live track while its outbound stream reports framesSent 0, " +

- "framesEncoded 0, active true, limited \"none\", and the other direction " +

- "arrives (513 received) on a receiver track reporting muted true. " +

- "See the account above this entry for the full getStats.",

}

func TestCallInARealBrowser(t *testing.T) {

diff --git a/test/browser/negotiation.test.mjs b/test/browser/negotiation.test.mjs

index 0e751a3..945b2dc 100644

--- a/test/browser/negotiation.test.mjs

+++ b/test/browser/negotiation.test.mjs

@@ -87,7 +87,7 @@ test('a remote offer already applied is glare', () => {

// when the next offer arrives, which is the race Codex S2-7 named and

// which the six cases above would pass without.

const handlers = new Function(

- 'accept', 'send', 'describe', 'drainCandidates', 'regainShare',

+ 'accept', 'send', 'describe', 'drainCandidates',

[lift(src, 'offerCollision'), lift(src, 'onOffer'), lift(src, 'onAnswer')].join('\n') +

'\nreturn { onOffer, onAnswer };',

);

@@ -142,7 +142,6 @@ test('an offer arriving while an answer is being applied is answered, not discar

(to, type) => sent.push(type),

(d) => d,

async () => {},

- () => {},

);

// The answer goes in and is left half-applied: its promise is still

@@ -163,42 +162,6 @@ test('an offer arriving while an answer is being applied is answered, not discar

assert.equal(e.isSettingRemoteAnswerPending, false, 'the flag must be cleared once the answer is applied');

});

-test('a screen lost in the polite side\u2019s rollback is asked for again, before the answer', () => {

- // §5.4: the implicit rollback lives inside setRemoteDescription, and a

- // share transceiver that was still unassociated can come out of it

- // stopped or with its m-line given to the offer that rolled us back.

- // Nothing else in this file runs at that instant, so the check belongs

- // here — and before the answer goes out, so the description this side

- // sends already carries the screen again.

- const pc = fakePC();

- pc.signalingState = 'stable';

- const e = {

- pc,

- polite: true,

- makingOffer: false,

- ignoreOffer: false,

- isSettingRemoteAnswerPending: false,

- pending: [],

- descSeq: 0,

- };

- const order = [];

- const { onOffer } = handlers(

- () => e,

- (to, type) => order.push('sent:' + type),

- (d) => d,

- async () => {},

- (entry) => {

- order.push('regain');

- assert.equal(entry, e, 'the share was regained for some other connection');

- },

- );

- return onOffer({ from: 'other', seq: 1, payload: { description: { type: 'offer', sdp: 'o' } } })

- .then(() => {

- assert.deepEqual(order, ['regain', 'sent:answer']);

- });

-});

// The recovery ladder, lifted the same way. This one is not about

// negotiation at all — it lives here because this file is where room.js

// can be run without a browser, and because a browser scenario cannot

diff --git a/test/browser/room-lane.test.mjs b/test/browser/room-lane.test.mjs

index b9c3e9b..f2776fa 100644

--- a/test/browser/room-lane.test.mjs

+++ b/test/browser/room-lane.test.mjs

@@ -458,3 +458,279 @@ test('chatEnabled is silent with no panel to enable', () => {

d.chatEnabled(false);

assert.deepEqual(d.enabled, []);

});

+// ---------------------------------------------------------------

+// §5.1's polite yield, and §5.4's share transceiver

+// ---------------------------------------------------------------

+//

+// Fix round 2's finding, gated where it can be: two peers sharing a

+// screen in the same moment made the polite one roll back an offer that

+// had added an m-line, and Chromium answers that by rebuilding every

+// send stream on the connection — the camera's comes back having

+// encoded nothing and never encodes again, on an m-line that goes on

+// reading sendrecv with a live track. The browser half is the gate's

+// `mutualshare`; what is here is the decision that keeps it from

+// happening: the side that would roll back gives way before it offers.

+const negotiator = new Function(

+ 'pcs', 'send', 'describe', 'sleep', 'state', 'POLITE_YIELD', 'console',

+ [lift(src, 'negotiate'), lift(src, 'politeYield')].join('\n') +

+ '\nreturn { negotiate };',

+);

+// A connection as `negotiate` sees one, over a fake pc whose

+// setLocalDescription can be watched and whose signalingState a test

+// moves under it — which is what a remote offer arriving during the

+// yield actually looks like.

+function negotiating(opts) {

+ const o = opts || {};

+ const sent = [];

+ const waits = [];

+ const pcs = new Map();

+ const entry = {

+ polite: !!o.polite,

+ yielded: !!o.yielded,

+ makingOffer: false,

+ pc: {

+ signalingState: o.signalingState || 'stable',

+ localDescription: { type: 'offer', sdp: 'local' },

+ async setLocalDescription() {

+ this.signalingState = 'have-local-offer';

+ },

+ };

+ pcs.set('peer1', entry);

+ const { negotiate } = negotiator(

+ pcs,

+ (to, type, payload) => sent.push([to, type, !!(payload && payload.description)]),

+ (d) => d,

+ async (ms) => {

+ waits.push(ms);

+ if (o.duringYield) o.duringYield(entry, pcs);

+ },

+ { stopped: !!o.stopped },

+ 250,

+ quiet,

+ );

+ return { negotiate, entry, pcs, sent, waits };

+test('the impolite side offers at once, and waits for nothing', async () => {

+ const n = negotiating({ polite: false });

+ await n.negotiate('peer1', n.entry);

+ assert.deepEqual(n.waits, [], 'the impolite side gave way');

+ assert.deepEqual(n.sent, [['peer1', 'offer', true]]);

+});

+test('the polite side gives way first, then offers', async () => {

+ const n = negotiating({ polite: true });

+ await n.negotiate('peer1', n.entry);

+ assert.deepEqual(n.waits, [250], 'the polite side did not wait, or waited the wrong amount');

+ assert.deepEqual(n.sent, [['peer1', 'offer', true]]);

+ assert.equal(n.entry.yielded, false, 'the offer that went out must clear the flag');

+});

+test('a remote offer arriving during the yield takes the negotiation', async () => {

+ // This is the whole fix. The offer that was already on its way is

+ // answered from a stable state; nothing collides, so nothing rolls

+ // back, so the camera keeps its encoder.

+ const n = negotiating({

+ polite: true,

+ duringYield: (entry) => { entry.pc.signalingState = 'have-remote-offer'; },

+ });

+ await n.negotiate('peer1', n.entry);

+ assert.deepEqual(n.sent, [], 'the polite side offered into a remote offer it could see');

+ assert.equal(n.entry.yielded, true, 'the yield it spent must be remembered');

+});

+test('giving way happens once per negotiation, never twice', async () => {

+ // A side that gave way and came back to find another offer waiting

+ // would give way again, and two peers renegotiating in turn would keep

+ // one of them permanently a beat behind.

+ const n = negotiating({ polite: true, yielded: true });

+ await n.negotiate('peer1', n.entry);

+ assert.deepEqual(n.waits, [], 'it gave way a second time');

+ assert.deepEqual(n.sent, [['peer1', 'offer', true]]);

+ assert.equal(n.entry.yielded, false);

+});

+test('a connection that went away during the yield is not offered to', async () => {

+ const n = negotiating({

+ polite: true,

+ duringYield: (entry, pcs) => pcs.delete('peer1'),

+ });

+ await n.negotiate('peer1', n.entry);

+ assert.deepEqual(n.sent, []);

+});

+test('a call that ended during the yield is not offered to', async () => {

+ const n = negotiating({ polite: true });

+ const stopped = negotiating({ polite: true, stopped: true });

+ await stopped.negotiate('peer1', stopped.entry);

+ assert.deepEqual(stopped.sent, [], 'an offer went out after the call ended');

+ // And the same connection, with the call still up, does offer — so the

+ // assertion above is about `stopped` and not about the harness.

+ await n.negotiate('peer1', n.entry);

+ assert.deepEqual(n.sent, [['peer1', 'offer', true]]);

+});

+test('a connection replaced under the negotiation is not offered to', async () => {

+ const n = negotiating({ polite: false });

+ n.pcs.set('peer1', { pc: {} });

+ await n.negotiate('peer1', n.entry);

+ assert.deepEqual(n.sent, []);

+});

+// ---------------------------------------------------------------

+// addShareTo: only on a stable connection

+// ---------------------------------------------------------------

+const sharerBox = new Function(

+ 'share', 'pcs', 'renderTiles', 'renderShare', 'broadcast', 'stopStream',

+ [lift(src, 'addShareTo'), lift(src, 'removeShareFrom'), lift(src, 'dropShare')].join('\n') +

+ '\nreturn { addShareTo, removeShareFrom, dropShare };',

+);

+function connection(signalingState) {

+ const removed = [];

+ const camera = { track: { kind: 'video', id: 'camera' }, replaceTrack() { throw new Error('the camera was touched'); } };

+ const entry = {

+ shareTx: null,

+ pc: {

+ signalingState,

+ removed,

+ added: [],

+ camera,

+ addTransceiver(track, init) {

+ const tx = { track, init, sender: { track }, stopped: false, mid: null, currentDirection: null, stop() { this.stopped = true; } };

+ this.added.push(tx);

+ return tx;

+ },

+ removeTrack(sender) { removed.push(sender); },

+ },

+ };

+ return entry;

+function shareOver(signalingState) {

+ const display = { kind: 'video', id: 'display' };

+ const stream = { id: 'display-stream' };

+ const entry = connection(signalingState);

+ const pcs = new Map([['peer1', entry]]);

+ const rendered = [];

+ const box = sharerBox({ stream, track: display }, pcs,

+ () => rendered.push('tiles'), () => rendered.push('share'), () => {}, () => {});

+ return Object.assign(box, { entry, display, stream, rendered });

+test('a screen is added to a stable connection', () => {

+ const s = shareOver('stable');

+ s.addShareTo(s.entry);

+ assert.equal(s.entry.pc.added.length, 1);

+ assert.equal(s.entry.pc.added[0].track, s.display);

+ assert.deepEqual(s.entry.pc.added[0].init, { direction: 'sendonly', streams: [s.stream] });

+ assert.equal(s.entry.shareTx, s.entry.pc.added[0]);

+});

+test('a screen is not added to a connection that is mid-negotiation', () => {

+ // An unassociated sendonly transceiver is exactly what a remote

+ // offer's m-line matching claims on a rollback, and a rollback is what

+ // costs this connection its camera. The connection's own

+ // signalingstatechange adds it at the next stable moment instead.

+ for (const busy of ['have-local-offer', 'have-remote-offer', 'closed']) {

+ const s = shareOver(busy);

+ s.addShareTo(s.entry);

+ assert.deepEqual(s.entry.pc.added, [], 'a screen was added during ' + busy);

+ assert.equal(s.entry.shareTx, null);

+ }

+});

+test('a connection that already carries the screen is left alone', () => {

+ const s = shareOver('stable');

+ s.addShareTo(s.entry);

+ assert.equal(s.entry.pc.added.length, 1);

+});

+// ---------------------------------------------------------------

+// Stopping a screen never reaches the camera

+// ---------------------------------------------------------------

+test('stopping a screen stops its own transceiver and nothing else', () => {

+ const s = shareOver('stable');

+ s.addShareTo(s.entry);

+ const tx = s.entry.shareTx;

+ s.removeShareFrom(s.entry);

+ assert.equal(tx.stopped, true, 'the screen m-line was not stopped');

+ assert.equal(s.entry.shareTx, null);

+ // The camera is the thing this must never touch: fix round 2 chased a

+ // camera that stopped encoding, and `removeTrack` on the wrong sender

+ // is one of the two ways this file could have done that itself. The

+ // fake camera sender throws from replaceTrack, so reaching it at all

+ // fails loudly rather than quietly.

+ assert.deepEqual(s.entry.pc.removed, [], 'a sender was removed from the connection');

+ assert.equal(s.entry.pc.camera.track.id, 'camera');

+});

+test('the fallback is only for a transceiver that cannot be stopped', () => {

+ // A browser whose transceivers have no stop(): the sender is nulled

+ // and removed instead. Still the share's own sender, never the

+ // camera's.

+ const s = shareOver('stable');

+ s.addShareTo(s.entry);

+ const tx = s.entry.shareTx;

+ delete tx.stop;

+ let replaced = 'not called';

+ tx.sender.replaceTrack = (t) => { replaced = t; return Promise.resolve(); };

+ s.removeShareFrom(s.entry);

+ assert.equal(replaced, null, 'the share sender was not nulled');

+ assert.deepEqual(s.entry.pc.removed, [tx.sender]);

+ assert.notEqual(s.entry.pc.removed[0], s.entry.pc.camera);

+});

+test('a connection with no screen on it is not renegotiated at all', () => {

+ const s = shareOver('stable');

+ s.removeShareFrom(s.entry);

+ assert.deepEqual(s.entry.pc.removed, []);

+ assert.deepEqual(s.entry.pc.added, []);

+});

+// ---------------------------------------------------------------

+// dropShare

+// ---------------------------------------------------------------

+test('dropping a screen clears the label, the media and the mid', () => {

+ const s = shareOver('stable');

+ const entry = { sharing: true, share: { stream: {}, track: {} }, shareMid: '2' };

+ s.dropShare(entry);

+ assert.deepEqual(entry, { sharing: false, share: null, shareMid: '' });

+});

+test('dropping a screen twice is the same as dropping it once', () => {

+ // §5.4: the track's `ended` and `share {on:false}` both arrive, in

+ // either order, and whichever is second must be a no-op — not a second

+ // render, and not an exception.

+ const s = shareOver('stable');

+ const entry = { sharing: true, share: { stream: {}, track: {} }, shareMid: '2' };

+ s.dropShare(entry);

+ s.dropShare({ sharing: false, share: null, shareMid: '' });

+ assert.deepEqual(entry, { sharing: false, share: null, shareMid: '' });

+ // Once, not three times: the grid is redrawn by the ending that

+ // actually ends something, and a screen removed twice must not make

+ // the tiles jump twice.

+ assert.deepEqual(s.rendered, ['tiles']);

+});

+test('a screen announced but never arrived is still dropped', () => {

+ // `share {on:true}` drew the tile and the track never came: the

+ // `share {on:false}` has to take the tile away all the same.

+ const s = shareOver('stable');

+ const entry = { sharing: true, share: null, shareMid: '' };

+ s.dropShare(entry);

+ assert.equal(entry.sharing, false);

+});

diff --git a/test/browser/room-share.test.mjs b/test/browser/room-share.test.mjs

index 6fd3b4c..8efd41f 100644

--- a/test/browser/room-share.test.mjs

+++ b/test/browser/room-share.test.mjs

@@ -18,7 +18,6 @@

// collide with a peer id

// shareCaption the words under a screen, which the browser gate

// rebuilds from the roster and compares

-// shareTxLost a share transceiver a rollback took away

// frameKey which of a peer's two video tiles a stats report

// belongs to

// broadcastBody the wire shape of an untargeted message — the

@@ -79,7 +78,6 @@ const suffix = 'const SHARE_SUFFIX = ' + JSON.stringify(SHARE_SUFFIX) + ';';

const shareKey = build('shareKey', suffix);

const shareCaption = build('shareCaption');

const classifyTrack = build('classifyTrack');

-const shareTxLost = build('shareTxLost');

const frameKey = build('frameKey', suffix + lift(src, 'shareKey'));

const broadcastBody = build('broadcastBody');

const broadcastVerdict = build('broadcastVerdict');

@@ -254,35 +252,6 @@ test('the share message breaks the tie, and only the tie', () => {

'camera');

});

-// ---------------------------------------------------------------

-// shareTxLost (§5.4, sending: the rollback)

-// ---------------------------------------------------------------

-const display = { id: 'display' };

-test('a transceiver still sending our display track is not lost', () => {

- assert.equal(shareTxLost({ stopped: false, sender: { track: display } }, display), false);

-});

-test('a stopped transceiver is lost', () => {

- assert.equal(shareTxLost({ stopped: true, sender: { track: display } }, display), true);

-});

-test('a transceiver whose m-line went to the offer that rolled us back is lost', () => {

- // What a rollback leaves behind: the transceiver is still there and

- // still ours to read, but it is not carrying the screen any more.

- assert.equal(shareTxLost({ stopped: false, sender: { track: null } }, display), true);

- assert.equal(shareTxLost({ stopped: false, sender: { track: { id: 'camera' } } }, display), true);

- assert.equal(shareTxLost({ stopped: false, sender: null }, display), true);

-});

-test('no transceiver at all is not a loss — there was nothing to lose', () => {

- // The difference matters: `false` here means addShareTo decides, and

- // `true` would mean clearing a field that is already empty.

- assert.equal(shareTxLost(null, display), false);

- assert.equal(shareTxLost(undefined, display), false);

-});

// ---------------------------------------------------------------

// frameKey (the getStats fallback, two video tiles)

// ---------------------------------------------------------------