Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 24 additions & 26 deletions src/polyfill/RTCDataChannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,19 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT
});

this.#dataChannel.onClosed(() => {
// Simulate closing event
if (this.#readyState === 'closed') return;

if (!this.#closeRequested) {
// if close was not requested, we emit 'closing' before 'close' to match the spec behavior
this.#readyState = 'closing';
this.dispatchEvent(new Event('closing'));
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic was incorrect. Why would we simulate a "closing" event when the data channel is already closed? Instead, we start the "closing" state at the beginning of the <RTCDataChannel>.close() method, not here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a gap in the libdatachannel API. Step 4 of 6.2.4 says:

Unless the procedure was initiated by channel.close, set channel.[[ReadyState]] to "closing" and fire an event named closing at channel.

We don't get a "closing" notification from libdatachannel before the channel is actually closed so this is the best we can do in order to emit the events required by the spec.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So how are we gonna handle both cases?

Step 5 of https://www.w3.org/TR/webrtc/#dom-rtcpeerconnection-close

NOTE

The RTCDataChannels will be closed abruptly and the closing procedure will not be invoked.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linked case is when the RTCPeerConnection is explicitly closed by the user so set a flag or a status and use that to decide whether or not to emit the event(s)?


setImmediate(() => {
this.#readyState = 'closed';
this.dispatchEvent(new Event('close'));
if (this.#readyState !== 'closed') {
this.#readyState = 'closed';
this.dispatchEvent(new Event('close'));
}
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to use setImmediate here. This can be done synchronously since the data channel already closed.

});

Expand All @@ -79,32 +83,22 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT

this.#dataChannel.onMessage((data) => {
if (ArrayBuffer.isView(data)) {
if (this.binaryType == 'arraybuffer') data = data.buffer;
else data = Buffer.from(data.buffer);
data =
this.binaryType === 'arraybuffer'
? (data.buffer as ArrayBuffer)
: Buffer.from(data.buffer);
}

this.dispatchEvent(new MessageEvent('message', { data }));
});

// forward events to properties
this.addEventListener('message', (e) => {
if (this.onmessage) this.onmessage(e as MessageEvent);
});
this.addEventListener('bufferedamountlow', (e) => {
if (this.onbufferedamountlow) this.onbufferedamountlow(e);
});
this.addEventListener('error', (e) => {
if (this.onerror) this.onerror(e as RTCErrorEvent);
});
this.addEventListener('close', (e) => {
if (this.onclose) this.onclose(e);
});
this.addEventListener('closing', (e) => {
if (this.onclosing) this.onclosing(e);
});
this.addEventListener('open', (e) => {
if (this.onopen) this.onopen(e);
});
this.addEventListener('open', (e) => this.onopen?.(e));
this.addEventListener('message', (e) => this.onmessage?.(e as MessageEvent));
this.addEventListener('error', (e) => this.onerror?.(e as RTCErrorEvent));
this.addEventListener('close', (e) => this.onclose?.(e));
this.addEventListener('closing', (e) => this.onclosing?.(e));
this.addEventListener('bufferedamountlow', (e) => this.onbufferedamountlow?.(e));
}

set binaryType(type) {
Expand Down Expand Up @@ -168,7 +162,7 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT
return this.#readyState;
}

send(data): void {
send(data: string | Blob | ArrayBuffer | ArrayBufferView): void {
if (this.#readyState !== 'open') {
throw new exceptions.InvalidStateError(
"Failed to execute 'send' on 'RTCDataChannel': RTCDataChannel.readyState is not 'open'",
Expand All @@ -190,15 +184,19 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT
this.#dataChannel.sendMessageBinary(data);
} else {
if (process?.versions?.bun) {
this.#dataChannel.sendMessageBinary(Buffer.from(data));
this.#dataChannel.sendMessageBinary(Buffer.from(data as ArrayBuffer));
} else {
this.#dataChannel.sendMessageBinary(new Uint8Array(data));
this.#dataChannel.sendMessageBinary(new Uint8Array(data as ArrayBuffer));
}
}
}

close(): void {
if (this.#readyState === 'closing' || this.#readyState === 'closed') return;

this.#closeRequested = true;
this.#readyState = 'closing';

setImmediate(() => {
this.#dataChannel.close();
});
Expand Down
11 changes: 11 additions & 0 deletions src/polyfill/RTCPeerConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,17 @@ export default class RTCPeerConnection extends EventTarget implements globalThis
}

close(): void {
for (const dc of this.#dataChannels) {
if (dc.readyState !== 'closed' && dc.readyState !== 'closing') {
Object.defineProperty(dc, 'readyState', {
value: 'closed',
writable: false,
enumerable: true,
configurable: true,
});
}
}

this.#peerConnection.close();
}

Expand Down