Watch for a 'join' action to know when the call is connected

robintown · robintown · commit 019b88b434c4 · 2025-05-05T18:26:26.000+02:00
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.
diff --git a/src/models/Call.ts b/src/models/Call.ts
@@ -31,7 +31,6 @@ import {
 import type EventEmitter from "events";
 import type { IApp } from "../stores/WidgetStore";
 import SettingsStore from "../settings/SettingsStore";
-import MediaDeviceHandler, { MediaDeviceKindEnum } from "../MediaDeviceHandler";
 import { timeout } from "../utils/promise";
 import WidgetUtils from "../utils/WidgetUtils";
 import { WidgetType } from "../widgets/WidgetType";
@@ -192,47 +191,17 @@ export abstract class Call extends TypedEventEmitter<CallEvent, CallEventHandler
      */
     public abstract clean(): Promise<void>;
 
-    /**
-     * Contacts the widget to connect to the call or prompt the user to connect to the call.
-     * @param {MediaDeviceInfo | null} audioInput The audio input to use, or
-     *   null to start muted.
-     * @param {MediaDeviceInfo | null} audioInput The video input to use, or
-     *   null to start muted.
-     */
-    protected abstract performConnection(
-        audioInput: MediaDeviceInfo | null,
-        videoInput: MediaDeviceInfo | null,
-    ): Promise<void>;
-
     /**
      * Contacts the widget to disconnect from the call.
      */
     protected abstract performDisconnection(): Promise<void>;
 
     /**
      * Starts the communication between the widget and the call.
-     * The call then waits for the necessary requirements to actually perform the connection
-     * or connects right away depending on the call type. (Jitsi, Legacy, ElementCall...)
-     * It uses the media devices set in MediaDeviceHandler.
-     * The widget associated with the call must be active
-     * for this to succeed.
+     * The widget associated with the call must be active for this to succeed.
      * Only call this if the call state is: ConnectionState.Disconnected.
      */
     public async start(): Promise<void> {
-        const { [MediaDeviceKindEnum.AudioInput]: audioInputs, [MediaDeviceKindEnum.VideoInput]: videoInputs } =
-            (await MediaDeviceHandler.getDevices())!;
-
-        let audioInput: MediaDeviceInfo | null = null;
-        if (!MediaDeviceHandler.startWithAudioMuted) {
-            const deviceId = MediaDeviceHandler.getAudioInput();
-            audioInput = audioInputs.find((d) => d.deviceId === deviceId) ?? audioInputs[0] ?? null;
-        }
-        let videoInput: MediaDeviceInfo | null = null;
-        if (!MediaDeviceHandler.startWithVideoMuted) {
-            const deviceId = MediaDeviceHandler.getVideoInput();
-            videoInput = videoInputs.find((d) => d.deviceId === deviceId) ?? videoInputs[0] ?? null;
-        }
-
         const messagingStore = WidgetMessagingStore.instance;
         this.messaging = messagingStore.getMessagingForUid(this.widgetUid) ?? null;
         if (!this.messaging) {
@@ -253,13 +222,23 @@ export abstract class Call extends TypedEventEmitter<CallEvent, CallEventHandler
                 throw new Error(`Failed to bind call widget in room ${this.roomId}: ${e}`);
             }
         }
-        await this.performConnection(audioInput, videoInput);
+    }
 
+    protected setConnected(): void {
         this.room.on(RoomEvent.MyMembership, this.onMyMembership);
         window.addEventListener("beforeunload", this.beforeUnload);
         this.connectionState = ConnectionState.Connected;
     }
 
+    /**
+     * Manually marks the call as disconnected.
+     */
+    protected setDisconnected(): void {
+        this.room.off(RoomEvent.MyMembership, this.onMyMembership);
+        window.removeEventListener("beforeunload", this.beforeUnload);
+        this.connectionState = ConnectionState.Disconnected;
+    }
+
     /**
      * Disconnects the user from the call.
      */
@@ -272,15 +251,6 @@ export abstract class Call extends TypedEventEmitter<CallEvent, CallEventHandler
         this.close();
     }
 
-    /**
-     * Manually marks the call as disconnected.
-     */
-    public setDisconnected(): void {
-        this.room.off(RoomEvent.MyMembership, this.onMyMembership);
-        window.removeEventListener("beforeunload", this.beforeUnload);
-        this.connectionState = ConnectionState.Disconnected;
-    }
-
     /**
      * Stops further communication with the widget and tells the UI to close.
      */
@@ -466,66 +436,10 @@ export class JitsiCall extends Call {
         });
     }
 
-    protected async performConnection(
-        audioInput: MediaDeviceInfo | null,
-        videoInput: MediaDeviceInfo | null,
-    ): Promise<void> {
-        // Ensure that the messaging doesn't get stopped while we're waiting for responses
-        const dontStopMessaging = new Promise<void>((resolve, reject) => {
-            const messagingStore = WidgetMessagingStore.instance;
-
-            const listener = (uid: string): void => {
-                if (uid === this.widgetUid) {
-                    cleanup();
-                    reject(new Error("Messaging stopped"));
-                }
-            };
-            const done = (): void => {
-                cleanup();
-                resolve();
-            };
-            const cleanup = (): void => {
-                messagingStore.off(WidgetMessagingStoreEvent.StopMessaging, listener);
-                this.off(CallEvent.ConnectionState, done);
-            };
-
-            messagingStore.on(WidgetMessagingStoreEvent.StopMessaging, listener);
-            this.on(CallEvent.ConnectionState, done);
-        });
-
-        // Empirically, it's possible for Jitsi Meet to crash instantly at startup,
-        // sending a hangup event that races with the rest of this method, so we need
-        // to add the hangup listener now rather than later
+    public async start(): Promise<void> {
+        await super.start();
+        this.messaging!.on(`action:${ElementWidgetActions.JoinCall}`, this.onJoin);
         this.messaging!.on(`action:${ElementWidgetActions.HangupCall}`, this.onHangup);
-
-        // Actually perform the join
-        const response = waitForEvent(
-            this.messaging!,
-            `action:${ElementWidgetActions.JoinCall}`,
-            (ev: CustomEvent<IWidgetApiRequest>) => {
-                ev.preventDefault();
-                this.messaging!.transport.reply(ev.detail, {}); // ack
-                return true;
-            },
-        );
-        const request = this.messaging!.transport.send(ElementWidgetActions.JoinCall, {
-            audioInput: audioInput?.label ?? null,
-            videoInput: videoInput?.label ?? null,
-        });
-        try {
-            await Promise.race([Promise.all([request, response]), dontStopMessaging]);
-        } catch (e) {
-            // If it timed out, clean up our advance preparations
-            this.messaging!.off(`action:${ElementWidgetActions.HangupCall}`, this.onHangup);
-
-            if (this.messaging!.transport.ready) {
-                // The messaging still exists, which means Jitsi might still be going in the background
-                this.messaging!.transport.send(ElementWidgetActions.HangupCall, { force: true });
-            }
-
-            throw new Error(`Failed to join call in room ${this.roomId}: ${e}`);
-        }
-
         ActiveWidgetStore.instance.on(ActiveWidgetStoreEvent.Dock, this.onDock);
         ActiveWidgetStore.instance.on(ActiveWidgetStoreEvent.Undock, this.onUndock);
     }
@@ -548,18 +462,17 @@ export class JitsiCall extends Call {
         }
     }
 
-    public setDisconnected(): void {
-        // During tests this.messaging can be undefined
-        this.messaging?.off(`action:${ElementWidgetActions.HangupCall}`, this.onHangup);
+    public close(): void {
+        this.messaging!.off(`action:${ElementWidgetActions.JoinCall}`, this.onJoin);
+        this.messaging!.off(`action:${ElementWidgetActions.HangupCall}`, this.onHangup);
         ActiveWidgetStore.instance.off(ActiveWidgetStoreEvent.Dock, this.onDock);
         ActiveWidgetStore.instance.off(ActiveWidgetStoreEvent.Undock, this.onUndock);
-
-        super.setDisconnected();
+        super.close();
     }
 
     public destroy(): void {
         this.room.off(RoomStateEvent.Update, this.onRoomState);
-        this.on(CallEvent.ConnectionState, this.onConnectionState);
+        this.off(CallEvent.ConnectionState, this.onConnectionState);
         if (this.participantsExpirationTimer !== null) {
             clearTimeout(this.participantsExpirationTimer);
             this.participantsExpirationTimer = null;
@@ -611,27 +524,21 @@ export class JitsiCall extends Call {
         await this.messaging!.transport.send(ElementWidgetActions.SpotlightLayout, {});
     };
 
+    private readonly onJoin = (ev: CustomEvent<IWidgetApiRequest>): void => {
+        ev.preventDefault();
+        this.messaging!.transport.reply(ev.detail, {}); // ack
+        this.setConnected();
+    };
+
     private readonly onHangup = async (ev: CustomEvent<IWidgetApiRequest>): Promise<void> => {
         // If we're already in the middle of a client-initiated disconnection,
         // ignore the event
         if (this.connectionState === ConnectionState.Disconnecting) return;
 
         ev.preventDefault();
-
-        // In case this hangup is caused by Jitsi Meet crashing at startup,
-        // wait for the connection event in order to avoid racing
-        if (this.connectionState === ConnectionState.Disconnected) {
-            await waitForEvent(this, CallEvent.ConnectionState);
-        }
-
         this.messaging!.transport.reply(ev.detail, {}); // ack
         this.setDisconnected();
-        this.close();
-        // In video rooms we immediately want to restart the call after hangup
-        // The lobby will be shown again and it connects to all signals from Jitsi.
-        if (isVideoRoom(this.room)) {
-            this.start();
-        }
+        if (!isVideoRoom(this.room)) this.close();
     };
 }
 
@@ -855,55 +762,38 @@ export class ElementCall extends Call {
         ElementCall.createOrGetCallWidget(room.roomId, room.client, skipLobby, isVideoRoom(room));
     }
 
-    protected async performConnection(
-        audioInput: MediaDeviceInfo | null,
-        videoInput: MediaDeviceInfo | null,
-    ): Promise<void> {
+    public async start(): Promise<void> {
+        await super.start();
+        this.messaging!.on(`action:${ElementWidgetActions.JoinCall}`, this.onJoin);
         this.messaging!.on(`action:${ElementWidgetActions.HangupCall}`, this.onHangup);
-        this.messaging!.once(`action:${ElementWidgetActions.Close}`, this.onClose);
+        this.messaging!.on(`action:${ElementWidgetActions.Close}`, this.onClose);
         this.messaging!.on(`action:${ElementWidgetActions.DeviceMute}`, this.onDeviceMute);
-
-        // TODO: if the widget informs us when the join button is clicked (widget action), so we can
-        // - set state to connecting
-        // - send call notify
-        const session = this.client.matrixRTC.getActiveRoomSession(this.room);
-        if (session) {
-            await waitForEvent(
-                session,
-                MatrixRTCSessionEvent.MembershipsChanged,
-                (_, newMemberships: CallMembership[]) =>
-                    newMemberships.some((m) => m.sender === this.client.getUserId()),
-                false, // allow user to wait as long as they want (no timeout)
-            );
-        } else {
-            await waitForEvent(
-                this.client.matrixRTC,
-                MatrixRTCSessionManagerEvents.SessionStarted,
-                (roomId: string, session: MatrixRTCSession) =>
-                    this.session.callId === session.callId && roomId === this.roomId,
-                false, // allow user to wait as long as they want (no timeout)
-            );
-        }
     }
 
     protected async performDisconnection(): Promise<void> {
+        const response = waitForEvent(
+            this.messaging!,
+            `action:${ElementWidgetActions.HangupCall}`,
+            (ev: CustomEvent<IWidgetApiRequest>) => {
+                ev.preventDefault();
+                this.messaging!.transport.reply(ev.detail, {}); // ack
+                return true;
+            },
+        );
+        const request = this.messaging!.transport.send(ElementWidgetActions.HangupCall, {});
         try {
-            await this.messaging!.transport.send(ElementWidgetActions.HangupCall, {});
-            await waitForEvent(
-                this.session,
-                MatrixRTCSessionEvent.MembershipsChanged,
-                (_, newMemberships: CallMembership[]) =>
-                    !newMemberships.some((m) => m.sender === this.client.getUserId()),
-            );
+            await Promise.all([request, response]);
         } catch (e) {
             throw new Error(`Failed to hangup call in room ${this.roomId}: ${e}`);
         }
     }
 
-    public setDisconnected(): void {
+    public close(): void {
+        this.messaging!.off(`action:${ElementWidgetActions.JoinCall}`, this.onJoin);
         this.messaging!.off(`action:${ElementWidgetActions.HangupCall}`, this.onHangup);
+        this.messaging!.off(`action:${ElementWidgetActions.Close}`, this.onClose);
         this.messaging!.off(`action:${ElementWidgetActions.DeviceMute}`, this.onDeviceMute);
-        super.setDisconnected();
+        super.close();
     }
 
     public destroy(): void {
@@ -950,15 +840,20 @@ export class ElementCall extends Call {
         this.messaging!.transport.reply(ev.detail, {}); // ack
     };
 
+    private readonly onJoin = (ev: CustomEvent<IWidgetApiRequest>): void => {
+        ev.preventDefault();
+        this.messaging!.transport.reply(ev.detail, {}); // ack
+        this.setConnected();
+    };
+
     private readonly onHangup = async (ev: CustomEvent<IWidgetApiRequest>): Promise<void> => {
+        // If we're already in the middle of a client-initiated disconnection,
+        // ignore the event
+        if (this.connectionState === ConnectionState.Disconnecting) return;
+
         ev.preventDefault();
         this.messaging!.transport.reply(ev.detail, {}); // ack
         this.setDisconnected();
-        // In video rooms we immediately want to reconnect after hangup
-        // This starts the lobby again and connects to all signals from EC.
-        if (isVideoRoom(this.room)) {
-            this.start();
-        }
     };
 
     private readonly onClose = async (ev: CustomEvent<IWidgetApiRequest>): Promise<void> => {
diff --git a/test/test-utils/call.ts b/test/test-utils/call.ts
@@ -79,7 +79,6 @@ export class MockedCall extends Call {
     // No action needed for any of the following methods since this is just a mock
     public async clean(): Promise<void> {}
     // Public to allow spying
-    public async performConnection(): Promise<void> {}
     public async performDisconnection(): Promise<void> {}
 
     public destroy() {
diff --git a/test/unit-tests/components/views/messages/CallEvent-test.tsx b/test/unit-tests/components/views/messages/CallEvent-test.tsx
@@ -151,7 +151,7 @@ describe("CallEvent", () => {
             }),
         );
         defaultDispatcher.unregister(dispatcherRef);
-        await act(() => call.start());
+        act(() => call.setConnectionState(ConnectionState.Connected));
 
         // Test that the leave button works
         fireEvent.click(screen.getByRole("button", { name: "Leave" }));
diff --git a/test/unit-tests/components/views/rooms/RoomTile-test.tsx b/test/unit-tests/components/views/rooms/RoomTile-test.tsx
@@ -46,6 +46,7 @@ import { UIComponent } from "../../../../../src/settings/UIFeature";
 import { MessagePreviewStore } from "../../../../../src/stores/room-list/MessagePreviewStore";
 import { MatrixClientPeg } from "../../../../../src/MatrixClientPeg";
 import SettingsStore from "../../../../../src/settings/SettingsStore";
+import { ConnectionState } from "../../../../../src/models/Call";
 
 jest.mock("../../../../../src/customisations/helpers/UIComponents", () => ({
     shouldShowComponent: jest.fn(),
@@ -215,7 +216,7 @@ describe("RoomTile", () => {
             it("tracks connection state", async () => {
                 renderRoomTile();
                 screen.getByText("Video");
-                await act(() => call.start());
+                act(() => call.setConnectionState(ConnectionState.Connected));
                 screen.getByText("Joined");
                 await act(() => call.disconnect());
                 screen.getByText("Video");
diff --git a/test/unit-tests/models/Call-test.ts b/test/unit-tests/models/Call-test.ts
diff --git a/test/unit-tests/stores/room-list/algorithms/Algorithm-test.ts b/test/unit-tests/stores/room-list/algorithms/Algorithm-test.ts
