Skip to content

Commit 244c80c

Browse files
committed
Watch for a 'join' action to know when the call is connected
Previously we were watching for changes to the room state to know when you become connected to a call. However, the room state might not change if you had a stuck membership event prior to re-joining the call. It's going to be more reliable to watch for the 'join' action that Element Call sends, and use that to track the connection state.
1 parent 494bbb1 commit 244c80c

File tree

6 files changed

+145
-394
lines changed

6 files changed

+145
-394
lines changed

src/models/Call.ts

Lines changed: 58 additions & 163 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,12 @@ import { type IWidgetApiRequest, type ClientWidgetApi, type IWidgetData } from "
2323
import {
2424
type MatrixRTCSession,
2525
MatrixRTCSessionEvent,
26-
type CallMembership,
2726
MatrixRTCSessionManagerEvents,
2827
} from "matrix-js-sdk/src/matrixrtc";
2928

3029
import type EventEmitter from "events";
3130
import type { IApp } from "../stores/WidgetStore";
3231
import SettingsStore from "../settings/SettingsStore";
33-
import MediaDeviceHandler, { MediaDeviceKindEnum } from "../MediaDeviceHandler";
3432
import { timeout } from "../utils/promise";
3533
import WidgetUtils from "../utils/WidgetUtils";
3634
import { WidgetType } from "../widgets/WidgetType";
@@ -193,47 +191,17 @@ export abstract class Call extends TypedEventEmitter<CallEvent, CallEventHandler
193191
*/
194192
public abstract clean(): Promise<void>;
195193

196-
/**
197-
* Contacts the widget to connect to the call or prompt the user to connect to the call.
198-
* @param {MediaDeviceInfo | null} audioInput The audio input to use, or
199-
* null to start muted.
200-
* @param {MediaDeviceInfo | null} audioInput The video input to use, or
201-
* null to start muted.
202-
*/
203-
protected abstract performConnection(
204-
audioInput: MediaDeviceInfo | null,
205-
videoInput: MediaDeviceInfo | null,
206-
): Promise<void>;
207-
208194
/**
209195
* Contacts the widget to disconnect from the call.
210196
*/
211197
protected abstract performDisconnection(): Promise<void>;
212198

213199
/**
214200
* Starts the communication between the widget and the call.
215-
* The call then waits for the necessary requirements to actually perform the connection
216-
* or connects right away depending on the call type. (Jitsi, Legacy, ElementCall...)
217-
* It uses the media devices set in MediaDeviceHandler.
218-
* The widget associated with the call must be active
219-
* for this to succeed.
201+
* The widget associated with the call must be active for this to succeed.
220202
* Only call this if the call state is: ConnectionState.Disconnected.
221203
*/
222204
public async start(): Promise<void> {
223-
const { [MediaDeviceKindEnum.AudioInput]: audioInputs, [MediaDeviceKindEnum.VideoInput]: videoInputs } =
224-
(await MediaDeviceHandler.getDevices())!;
225-
226-
let audioInput: MediaDeviceInfo | null = null;
227-
if (!MediaDeviceHandler.startWithAudioMuted) {
228-
const deviceId = MediaDeviceHandler.getAudioInput();
229-
audioInput = audioInputs.find((d) => d.deviceId === deviceId) ?? audioInputs[0] ?? null;
230-
}
231-
let videoInput: MediaDeviceInfo | null = null;
232-
if (!MediaDeviceHandler.startWithVideoMuted) {
233-
const deviceId = MediaDeviceHandler.getVideoInput();
234-
videoInput = videoInputs.find((d) => d.deviceId === deviceId) ?? videoInputs[0] ?? null;
235-
}
236-
237205
const messagingStore = WidgetMessagingStore.instance;
238206
this.messaging = messagingStore.getMessagingForUid(this.widgetUid) ?? null;
239207
if (!this.messaging) {
@@ -254,13 +222,23 @@ export abstract class Call extends TypedEventEmitter<CallEvent, CallEventHandler
254222
throw new Error(`Failed to bind call widget in room ${this.roomId}: ${e}`);
255223
}
256224
}
257-
await this.performConnection(audioInput, videoInput);
225+
}
258226

227+
protected setConnected(): void {
259228
this.room.on(RoomEvent.MyMembership, this.onMyMembership);
260229
window.addEventListener("beforeunload", this.beforeUnload);
261230
this.connectionState = ConnectionState.Connected;
262231
}
263232

233+
/**
234+
* Manually marks the call as disconnected.
235+
*/
236+
protected setDisconnected(): void {
237+
this.room.off(RoomEvent.MyMembership, this.onMyMembership);
238+
window.removeEventListener("beforeunload", this.beforeUnload);
239+
this.connectionState = ConnectionState.Disconnected;
240+
}
241+
264242
/**
265243
* Disconnects the user from the call.
266244
*/
@@ -273,15 +251,6 @@ export abstract class Call extends TypedEventEmitter<CallEvent, CallEventHandler
273251
this.close();
274252
}
275253

276-
/**
277-
* Manually marks the call as disconnected.
278-
*/
279-
public setDisconnected(): void {
280-
this.room.off(RoomEvent.MyMembership, this.onMyMembership);
281-
window.removeEventListener("beforeunload", this.beforeUnload);
282-
this.connectionState = ConnectionState.Disconnected;
283-
}
284-
285254
/**
286255
* Stops further communication with the widget and tells the UI to close.
287256
*/
@@ -467,66 +436,10 @@ export class JitsiCall extends Call {
467436
});
468437
}
469438

470-
protected async performConnection(
471-
audioInput: MediaDeviceInfo | null,
472-
videoInput: MediaDeviceInfo | null,
473-
): Promise<void> {
474-
// Ensure that the messaging doesn't get stopped while we're waiting for responses
475-
const dontStopMessaging = new Promise<void>((resolve, reject) => {
476-
const messagingStore = WidgetMessagingStore.instance;
477-
478-
const listener = (uid: string): void => {
479-
if (uid === this.widgetUid) {
480-
cleanup();
481-
reject(new Error("Messaging stopped"));
482-
}
483-
};
484-
const done = (): void => {
485-
cleanup();
486-
resolve();
487-
};
488-
const cleanup = (): void => {
489-
messagingStore.off(WidgetMessagingStoreEvent.StopMessaging, listener);
490-
this.off(CallEvent.ConnectionState, done);
491-
};
492-
493-
messagingStore.on(WidgetMessagingStoreEvent.StopMessaging, listener);
494-
this.on(CallEvent.ConnectionState, done);
495-
});
496-
497-
// Empirically, it's possible for Jitsi Meet to crash instantly at startup,
498-
// sending a hangup event that races with the rest of this method, so we need
499-
// to add the hangup listener now rather than later
439+
public async start(): Promise<void> {
440+
await super.start();
441+
this.messaging!.on(`action:${ElementWidgetActions.JoinCall}`, this.onJoin);
500442
this.messaging!.on(`action:${ElementWidgetActions.HangupCall}`, this.onHangup);
501-
502-
// Actually perform the join
503-
const response = waitForEvent(
504-
this.messaging!,
505-
`action:${ElementWidgetActions.JoinCall}`,
506-
(ev: CustomEvent<IWidgetApiRequest>) => {
507-
ev.preventDefault();
508-
this.messaging!.transport.reply(ev.detail, {}); // ack
509-
return true;
510-
},
511-
);
512-
const request = this.messaging!.transport.send(ElementWidgetActions.JoinCall, {
513-
audioInput: audioInput?.label ?? null,
514-
videoInput: videoInput?.label ?? null,
515-
});
516-
try {
517-
await Promise.race([Promise.all([request, response]), dontStopMessaging]);
518-
} catch (e) {
519-
// If it timed out, clean up our advance preparations
520-
this.messaging!.off(`action:${ElementWidgetActions.HangupCall}`, this.onHangup);
521-
522-
if (this.messaging!.transport.ready) {
523-
// The messaging still exists, which means Jitsi might still be going in the background
524-
this.messaging!.transport.send(ElementWidgetActions.HangupCall, { force: true });
525-
}
526-
527-
throw new Error(`Failed to join call in room ${this.roomId}: ${e}`);
528-
}
529-
530443
ActiveWidgetStore.instance.on(ActiveWidgetStoreEvent.Dock, this.onDock);
531444
ActiveWidgetStore.instance.on(ActiveWidgetStoreEvent.Undock, this.onUndock);
532445
}
@@ -549,18 +462,17 @@ export class JitsiCall extends Call {
549462
}
550463
}
551464

552-
public setDisconnected(): void {
553-
// During tests this.messaging can be undefined
554-
this.messaging?.off(`action:${ElementWidgetActions.HangupCall}`, this.onHangup);
465+
public close(): void {
466+
this.messaging!.off(`action:${ElementWidgetActions.JoinCall}`, this.onJoin);
467+
this.messaging!.off(`action:${ElementWidgetActions.HangupCall}`, this.onHangup);
555468
ActiveWidgetStore.instance.off(ActiveWidgetStoreEvent.Dock, this.onDock);
556469
ActiveWidgetStore.instance.off(ActiveWidgetStoreEvent.Undock, this.onUndock);
557-
558-
super.setDisconnected();
470+
super.close();
559471
}
560472

561473
public destroy(): void {
562474
this.room.off(RoomStateEvent.Update, this.onRoomState);
563-
this.on(CallEvent.ConnectionState, this.onConnectionState);
475+
this.off(CallEvent.ConnectionState, this.onConnectionState);
564476
if (this.participantsExpirationTimer !== null) {
565477
clearTimeout(this.participantsExpirationTimer);
566478
this.participantsExpirationTimer = null;
@@ -612,27 +524,21 @@ export class JitsiCall extends Call {
612524
await this.messaging!.transport.send(ElementWidgetActions.SpotlightLayout, {});
613525
};
614526

527+
private readonly onJoin = (ev: CustomEvent<IWidgetApiRequest>): void => {
528+
ev.preventDefault();
529+
this.messaging!.transport.reply(ev.detail, {}); // ack
530+
this.setConnected();
531+
};
532+
615533
private readonly onHangup = async (ev: CustomEvent<IWidgetApiRequest>): Promise<void> => {
616534
// If we're already in the middle of a client-initiated disconnection,
617535
// ignore the event
618536
if (this.connectionState === ConnectionState.Disconnecting) return;
619537

620538
ev.preventDefault();
621-
622-
// In case this hangup is caused by Jitsi Meet crashing at startup,
623-
// wait for the connection event in order to avoid racing
624-
if (this.connectionState === ConnectionState.Disconnected) {
625-
await waitForEvent(this, CallEvent.ConnectionState);
626-
}
627-
628539
this.messaging!.transport.reply(ev.detail, {}); // ack
629540
this.setDisconnected();
630-
this.close();
631-
// In video rooms we immediately want to restart the call after hangup
632-
// The lobby will be shown again and it connects to all signals from Jitsi.
633-
if (isVideoRoom(this.room)) {
634-
this.start();
635-
}
541+
if (!isVideoRoom(this.room)) this.close();
636542
};
637543
}
638544

@@ -860,54 +766,38 @@ export class ElementCall extends Call {
860766
ElementCall.createOrGetCallWidget(room.roomId, room.client, skipLobby, isVideoRoom(room));
861767
}
862768

863-
protected async performConnection(
864-
audioInput: MediaDeviceInfo | null,
865-
videoInput: MediaDeviceInfo | null,
866-
): Promise<void> {
769+
public async start(): Promise<void> {
770+
await super.start();
771+
this.messaging!.on(`action:${ElementWidgetActions.JoinCall}`, this.onJoin);
867772
this.messaging!.on(`action:${ElementWidgetActions.HangupCall}`, this.onHangup);
868-
this.messaging!.once(`action:${ElementWidgetActions.Close}`, this.onClose);
773+
this.messaging!.on(`action:${ElementWidgetActions.Close}`, this.onClose);
869774
this.messaging!.on(`action:${ElementWidgetActions.DeviceMute}`, this.onDeviceMute);
870-
871-
// TODO: Watch for a widget action telling us that the join button was clicked, rather than
872-
// relying on the MatrixRTC session state, to set the state to connecting
873-
const session = this.client.matrixRTC.getActiveRoomSession(this.room);
874-
if (session) {
875-
await waitForEvent(
876-
session,
877-
MatrixRTCSessionEvent.MembershipsChanged,
878-
(_, newMemberships: CallMembership[]) =>
879-
newMemberships.some((m) => m.sender === this.client.getUserId()),
880-
false, // allow user to wait as long as they want (no timeout)
881-
);
882-
} else {
883-
await waitForEvent(
884-
this.client.matrixRTC,
885-
MatrixRTCSessionManagerEvents.SessionStarted,
886-
(roomId: string, session: MatrixRTCSession) =>
887-
this.session.callId === session.callId && roomId === this.roomId,
888-
false, // allow user to wait as long as they want (no timeout)
889-
);
890-
}
891775
}
892776

893777
protected async performDisconnection(): Promise<void> {
778+
const response = waitForEvent(
779+
this.messaging!,
780+
`action:${ElementWidgetActions.HangupCall}`,
781+
(ev: CustomEvent<IWidgetApiRequest>) => {
782+
ev.preventDefault();
783+
this.messaging!.transport.reply(ev.detail, {}); // ack
784+
return true;
785+
},
786+
);
787+
const request = this.messaging!.transport.send(ElementWidgetActions.HangupCall, {});
894788
try {
895-
await this.messaging!.transport.send(ElementWidgetActions.HangupCall, {});
896-
await waitForEvent(
897-
this.session,
898-
MatrixRTCSessionEvent.MembershipsChanged,
899-
(_, newMemberships: CallMembership[]) =>
900-
!newMemberships.some((m) => m.sender === this.client.getUserId()),
901-
);
789+
await Promise.all([request, response]);
902790
} catch (e) {
903791
throw new Error(`Failed to hangup call in room ${this.roomId}: ${e}`);
904792
}
905793
}
906794

907-
public setDisconnected(): void {
795+
public close(): void {
796+
this.messaging!.off(`action:${ElementWidgetActions.JoinCall}`, this.onJoin);
908797
this.messaging!.off(`action:${ElementWidgetActions.HangupCall}`, this.onHangup);
798+
this.messaging!.off(`action:${ElementWidgetActions.Close}`, this.onClose);
909799
this.messaging!.off(`action:${ElementWidgetActions.DeviceMute}`, this.onDeviceMute);
910-
super.setDisconnected();
800+
super.close();
911801
}
912802

913803
public destroy(): void {
@@ -954,22 +844,27 @@ export class ElementCall extends Call {
954844
this.messaging!.transport.reply(ev.detail, {}); // ack
955845
};
956846

847+
private readonly onJoin = (ev: CustomEvent<IWidgetApiRequest>): void => {
848+
ev.preventDefault();
849+
this.messaging!.transport.reply(ev.detail, {}); // ack
850+
this.setConnected();
851+
};
852+
957853
private readonly onHangup = async (ev: CustomEvent<IWidgetApiRequest>): Promise<void> => {
854+
// If we're already in the middle of a client-initiated disconnection,
855+
// ignore the event
856+
if (this.connectionState === ConnectionState.Disconnecting) return;
857+
958858
ev.preventDefault();
959859
this.messaging!.transport.reply(ev.detail, {}); // ack
960860
this.setDisconnected();
961-
// In video rooms we immediately want to reconnect after hangup
962-
// This starts the lobby again and connects to all signals from EC.
963-
if (isVideoRoom(this.room)) {
964-
this.start();
965-
}
966861
};
967862

968863
private readonly onClose = async (ev: CustomEvent<IWidgetApiRequest>): Promise<void> => {
969864
ev.preventDefault();
970865
this.messaging!.transport.reply(ev.detail, {}); // ack
971-
// User is done with the call; tell the UI to close it
972-
this.close();
866+
this.setDisconnected(); // Just in case the widget forgot to emit a hangup action (maybe it's in an error state)
867+
this.close(); // User is done with the call; tell the UI to close it
973868
};
974869

975870
public clean(): Promise<void> {

test/test-utils/call.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ export class MockedCall extends Call {
7979
// No action needed for any of the following methods since this is just a mock
8080
public async clean(): Promise<void> {}
8181
// Public to allow spying
82-
public async performConnection(): Promise<void> {}
8382
public async performDisconnection(): Promise<void> {}
8483

8584
public destroy() {

test/unit-tests/components/views/messages/CallEvent-test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ describe("CallEvent", () => {
151151
}),
152152
);
153153
defaultDispatcher.unregister(dispatcherRef);
154-
await act(() => call.start());
154+
act(() => call.setConnectionState(ConnectionState.Connected));
155155

156156
// Test that the leave button works
157157
fireEvent.click(screen.getByRole("button", { name: "Leave" }));

test/unit-tests/components/views/rooms/RoomTile-test.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import { UIComponent } from "../../../../../src/settings/UIFeature";
4646
import { MessagePreviewStore } from "../../../../../src/stores/room-list/MessagePreviewStore";
4747
import { MatrixClientPeg } from "../../../../../src/MatrixClientPeg";
4848
import SettingsStore from "../../../../../src/settings/SettingsStore";
49+
import { ConnectionState } from "../../../../../src/models/Call";
4950

5051
jest.mock("../../../../../src/customisations/helpers/UIComponents", () => ({
5152
shouldShowComponent: jest.fn(),
@@ -215,7 +216,7 @@ describe("RoomTile", () => {
215216
it("tracks connection state", async () => {
216217
renderRoomTile();
217218
screen.getByText("Video");
218-
await act(() => call.start());
219+
act(() => call.setConnectionState(ConnectionState.Connected));
219220
screen.getByText("Joined");
220221
await act(() => call.disconnect());
221222
screen.getByText("Video");

0 commit comments

Comments
 (0)