Avoid creating multiple call objects for the same widget (#30839)

* Extract some setup code out of the call tests

* Don't force all rooms to be rechecked for calls when starting a call

* Remove misleading unused group call callbacks

The GroupCallEventHandler hasn't been relevant to our Element Call group calls for some time; instead we look at the state of the MatrixRTCSessionManager and WidgetStore to determine whether a call has been started.

* Avoid creating multiple call objects for the same widget

* fix test

---------

Co-authored-by: Will Hunt <will@half-shot.uk>

src/models/Call.ts

@@ -38,7 +38,6 @@ import { WidgetMessagingStore, WidgetMessagingStoreEvent } from "../stores/widge
 import ActiveWidgetStore, { ActiveWidgetStoreEvent } from "../stores/ActiveWidgetStore";
 import { getCurrentLanguage } from "../languageHandler";
 import { Anonymity, PosthogAnalytics } from "../PosthogAnalytics";
-import { UPDATE_EVENT } from "../stores/AsyncStore";
 import { isVideoRoom } from "../utils/video-rooms";
 import { FontWatcher } from "../settings/watchers/FontWatcher";
 import { type JitsiCallMemberContent, JitsiCallMemberEventType } from "../call-types";
@@ -702,7 +701,7 @@ export class ElementCall extends Call {
         // To use Element Call without touching room state, we create a virtual
         // widget (one that doesn't have a corresponding state event)
         const url = ElementCall.generateWidgetUrl(client, roomId);
-        const createdWidget = WidgetStore.instance.addVirtualWidget(
+        return WidgetStore.instance.addVirtualWidget(
             {
                 id: secureRandomString(24), // So that it's globally unique
                 creatorUserId: client.getUserId()!,
@@ -722,8 +721,6 @@ export class ElementCall extends Call {
             },
             roomId,
         );
-        WidgetStore.instance.emit(UPDATE_EVENT, null);
-        return createdWidget;
     }
 
     private static getWidgetData(

src/stores/CallStore.ts

@@ -7,10 +7,9 @@ Please see LICENSE files in the repository root for full details.
 */
 
 import { logger } from "matrix-js-sdk/src/logger";
-import { GroupCallEventHandlerEvent } from "matrix-js-sdk/src/webrtc/groupCallEventHandler";
 import { type MatrixRTCSession, MatrixRTCSessionManagerEvents } from "matrix-js-sdk/src/matrixrtc";
 
-import type { EmptyObject, GroupCall, Room } from "matrix-js-sdk/src/matrix";
+import type { EmptyObject, Room } from "matrix-js-sdk/src/matrix";
 import defaultDispatcher from "../dispatcher/dispatcher";
 import { UPDATE_EVENT } from "./AsyncStore";
 import { AsyncStoreWithClient } from "./AsyncStoreWithClient";
@@ -53,8 +52,6 @@ export class CallStore extends AsyncStoreWithClient<EmptyObject> {
         for (const room of this.matrixClient.getRooms()) {
             this.updateRoom(room);
         }
-        this.matrixClient.on(GroupCallEventHandlerEvent.Incoming, this.onGroupCall);
-        this.matrixClient.on(GroupCallEventHandlerEvent.Outgoing, this.onGroupCall);
         this.matrixClient.matrixRTC.on(MatrixRTCSessionManagerEvents.SessionStarted, this.onRTCSessionStart);
         WidgetStore.instance.on(UPDATE_EVENT, this.onWidgets);
 
@@ -85,12 +82,7 @@ export class CallStore extends AsyncStoreWithClient<EmptyObject> {
         this.calls.clear();
         this._connectedCalls.clear();
 
-        if (this.matrixClient) {
+        this.matrixClient?.matrixRTC.off(MatrixRTCSessionManagerEvents.SessionStarted, this.onRTCSessionStart);
-            this.matrixClient.off(GroupCallEventHandlerEvent.Incoming, this.onGroupCall);
-            this.matrixClient.off(GroupCallEventHandlerEvent.Outgoing, this.onGroupCall);
-            this.matrixClient.off(GroupCallEventHandlerEvent.Ended, this.onGroupCall);
-            this.matrixClient.matrixRTC.off(MatrixRTCSessionManagerEvents.SessionStarted, this.onRTCSessionStart);
-        }
         WidgetStore.instance.off(UPDATE_EVENT, this.onWidgets);
     }
 
@@ -117,8 +109,16 @@ export class CallStore extends AsyncStoreWithClient<EmptyObject> {
     private calls = new Map<string, Call>(); // Key is room ID
     private callListeners = new Map<Call, Map<CallEvent, (...args: unknown[]) => unknown>>();
 
+    private inUpdateRoom = false;
     private updateRoom(room: Room): void {
-        if (!this.calls.has(room.roomId)) {
+        // XXX: This method is guarded with the flag this.inUpdateRoom because
+        // we need to block this method from calling itself recursively. That
+        // could happen, for instance, if Call.get adds a new virtual widget to
+        // the WidgetStore, firing a WidgetStore update that we don't actually
+        // care about. Without the guard we could get duplicate Call objects
+        // fighting for control over the same widget.
+        if (!this.inUpdateRoom && !this.calls.has(room.roomId)) {
+            this.inUpdateRoom = true;
             const call = Call.get(room);
 
             if (call) {
@@ -149,6 +149,7 @@ export class CallStore extends AsyncStoreWithClient<EmptyObject> {
             }
 
             this.emit(CallStoreEvent.Call, call, room.roomId);
+            this.inUpdateRoom = false;
         }
     }
 
@@ -186,7 +187,6 @@ export class CallStore extends AsyncStoreWithClient<EmptyObject> {
         }
     };
 
-    private onGroupCall = (groupCall: GroupCall): void => this.updateRoom(groupCall.room);
     private onRTCSessionStart = (roomId: string, session: MatrixRTCSession): void => {
         this.updateRoom(session.room);
     };

src/stores/WidgetStore.ts

@@ -189,6 +189,7 @@ export default class WidgetStore extends AsyncStoreWithClient<EmptyObject> {
         const app = WidgetUtils.makeAppConfig(widget.id, widget, widget.creatorUserId, roomId, undefined);
         this.widgetMap.set(WidgetUtils.getWidgetUid(app), app);
         this.roomMap.get(roomId)!.widgets.push(app);
+        this.emit(UPDATE_EVENT, roomId);
         return app;
     }
 
@@ -198,6 +199,7 @@ export default class WidgetStore extends AsyncStoreWithClient<EmptyObject> {
         if (roomApps) {
             roomApps.widgets = roomApps.widgets.filter((app) => !(app.id === widgetId && app.roomId === roomId));
         }
+        this.emit(UPDATE_EVENT, roomId);
     }
 }
 

test/test-utils/call.ts

@@ -7,11 +7,29 @@ Please see LICENSE files in the repository root for full details.
 */
 
 import { MatrixWidgetType } from "matrix-widget-api";
+import {
+    type GroupCall,
+    Room,
+    type RoomMember,
+    type MatrixEvent,
+    type MatrixClient,
+    PendingEventOrdering,
+    KnownMembership,
+    RoomStateEvent,
+    type IContent,
+} from "matrix-js-sdk/src/matrix";
+import { mocked, type Mocked } from "jest-mock";
+import { type MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc";
 
-import type { GroupCall, Room, RoomMember, MatrixEvent } from "matrix-js-sdk/src/matrix";
+import { mkEvent, mkRoomMember, setupAsyncStoreWithClient, stubClient } from "./test-utils";
-import { mkEvent } from "./test-utils";
 import { Call, type ConnectionState, ElementCall, JitsiCall } from "../../src/models/Call";
 import { CallStore } from "../../src/stores/CallStore";
+import { MatrixClientPeg } from "../../src/MatrixClientPeg";
+import DMRoomMap from "../../src/utils/DMRoomMap";
+import { MockEventEmitter } from "./client";
+import WidgetStore from "../../src/stores/WidgetStore";
+import { WidgetMessagingStore } from "../../src/stores/widgets/WidgetMessagingStore";
+import SettingsStore from "../../src/settings/SettingsStore";
 
 export class MockedCall extends Call {
     public static readonly EVENT_TYPE = "org.example.mocked_call";
@@ -105,8 +123,92 @@ export class MockedCall extends Call {
 /**
  * Sets up the call store to use mocked calls.
  */
-export const useMockedCalls = () => {
+export function useMockedCalls() {
     Call.get = (room) => MockedCall.get(room);
     JitsiCall.create = async (room) => MockedCall.create(room, "1");
     ElementCall.create = (room) => MockedCall.create(room, "1");
-};
+}
+
+/**
+ * Enables the feature flags required for call tests.
+ */
+export function enableCalls(): { enabledSettings: Set<string> } {
+    const enabledSettings = new Set(["feature_group_calls", "feature_video_rooms", "feature_element_call_video_rooms"]);
+    jest.spyOn(SettingsStore, "getValue").mockImplementation((settingName): any => {
+        if (settingName.startsWith("feature_")) return enabledSettings.has(settingName);
+        if (settingName === "activeCallRoomIds") return [];
+        return undefined;
+    });
+    return { enabledSettings };
+}
+
+export function setUpClientRoomAndStores(): {
+    client: Mocked<MatrixClient>;
+    room: Room;
+    alice: RoomMember;
+    bob: RoomMember;
+    carol: RoomMember;
+    roomSession: Mocked<MatrixRTCSession>;
+} {
+    stubClient();
+    const client = mocked<MatrixClient>(MatrixClientPeg.safeGet());
+    DMRoomMap.makeShared(client);
+
+    const room = new Room("!1:example.org", client, "@alice:example.org", {
+        pendingEventOrdering: PendingEventOrdering.Detached,
+    });
+
+    const alice = mkRoomMember(room.roomId, "@alice:example.org");
+    const bob = mkRoomMember(room.roomId, "@bob:example.org");
+    const carol = mkRoomMember(room.roomId, "@carol:example.org");
+    jest.spyOn(room, "getMember").mockImplementation((userId) => {
+        switch (userId) {
+            case alice.userId:
+                return alice;
+            case bob.userId:
+                return bob;
+            case carol.userId:
+                return carol;
+            default:
+                return null;
+        }
+    });
+
+    jest.spyOn(room, "getMyMembership").mockReturnValue(KnownMembership.Join);
+
+    client.getRoom.mockImplementation((roomId) => (roomId === room.roomId ? room : null));
+
+    const roomSession = new MockEventEmitter({
+        memberships: [],
+        getOldestMembership: jest.fn().mockReturnValue(undefined),
+        room,
+    }) as Mocked<MatrixRTCSession>;
+
+    client.matrixRTC.getRoomSession.mockReturnValue(roomSession);
+    client.getRooms.mockReturnValue([room]);
+    client.getUserId.mockReturnValue(alice.userId);
+    client.getDeviceId.mockReturnValue("alices_device");
+    client.reEmitter.reEmit(room, [RoomStateEvent.Events]);
+    client.sendStateEvent.mockImplementation(async (roomId, eventType, content, stateKey = "") => {
+        if (roomId !== room.roomId) throw new Error("Unknown room");
+        const event = mkEvent({
+            event: true,
+            type: eventType,
+            room: roomId,
+            user: alice.userId,
+            skey: stateKey,
+            content: content as IContent,
+        });
+        room.addLiveEvents([event], { addToState: true });
+        return { event_id: event.getId()! };
+    });
+
+    setupAsyncStoreWithClient(WidgetStore.instance, client);
+    setupAsyncStoreWithClient(WidgetMessagingStore.instance, client);
+
+    return { client, room, alice, bob, carol, roomSession };
+}
+
+export function cleanUpClientRoomAndStores(client: MatrixClient, room: Room) {
+    client.reEmitter.stopReEmitting(room, [RoomStateEvent.Events]);
+}

test/unit-tests/models/Call-test.ts

@@ -11,12 +11,9 @@ import { mocked } from "jest-mock";
 import { waitFor } from "jest-matrix-react";
 import {
     RoomType,
-    Room,
+    type Room,
     RoomEvent,
     MatrixEvent,
-    RoomStateEvent,
-    PendingEventOrdering,
-    type IContent,
     type MatrixClient,
     type IMyDevice,
     type RoomMember,
@@ -41,15 +38,7 @@ import {
     ElementCall,
     ElementCallIntent,
 } from "../../../src/models/Call";
-import {
+import { cleanUpClientRoomAndStores, enableCalls, mockPlatformPeg, setUpClientRoomAndStores } from "../../test-utils";
-    stubClient,
-    mkEvent,
-    mkRoomMember,
-    setupAsyncStoreWithClient,
-    mockPlatformPeg,
-    MockEventEmitter,
-} from "../../test-utils";
-import { MatrixClientPeg } from "../../../src/MatrixClientPeg";
 import WidgetStore from "../../../src/stores/WidgetStore";
 import { WidgetMessagingStore } from "../../../src/stores/widgets/WidgetMessagingStore";
 import ActiveWidgetStore, { ActiveWidgetStoreEvent } from "../../../src/stores/ActiveWidgetStore";
@@ -60,80 +49,7 @@ import { type SettingKey } from "../../../src/settings/Settings.tsx";
 import SdkConfig from "../../../src/SdkConfig.ts";
 import DMRoomMap from "../../../src/utils/DMRoomMap.ts";
 
-const enabledSettings = new Set(["feature_group_calls", "feature_video_rooms", "feature_element_call_video_rooms"]);
+const { enabledSettings } = enableCalls();
-jest.spyOn(SettingsStore, "getValue").mockImplementation(
-    (settingName): any => enabledSettings.has(settingName) || undefined,
-);
-
-const setUpClientRoomAndStores = (): {
-    client: Mocked<MatrixClient>;
-    room: Room;
-    alice: RoomMember;
-    bob: RoomMember;
-    carol: RoomMember;
-    roomSession: Mocked<MatrixRTCSession>;
-} => {
-    stubClient();
-    const client = mocked<MatrixClient>(MatrixClientPeg.safeGet());
-    DMRoomMap.makeShared(client);
-
-    const room = new Room("!1:example.org", client, "@alice:example.org", {
-        pendingEventOrdering: PendingEventOrdering.Detached,
-    });
-
-    const alice = mkRoomMember(room.roomId, "@alice:example.org");
-    const bob = mkRoomMember(room.roomId, "@bob:example.org");
-    const carol = mkRoomMember(room.roomId, "@carol:example.org");
-    jest.spyOn(room, "getMember").mockImplementation((userId) => {
-        switch (userId) {
-            case alice.userId:
-                return alice;
-            case bob.userId:
-                return bob;
-            case carol.userId:
-                return carol;
-            default:
-                return null;
-        }
-    });
-
-    jest.spyOn(room, "getMyMembership").mockReturnValue(KnownMembership.Join);
-
-    client.getRoom.mockImplementation((roomId) => (roomId === room.roomId ? room : null));
-
-    const roomSession = new MockEventEmitter({
-        memberships: [],
-        getOldestMembership: jest.fn().mockReturnValue(undefined),
-    }) as Mocked<MatrixRTCSession>;
-
-    client.matrixRTC.getRoomSession.mockReturnValue(roomSession);
-    client.getRooms.mockReturnValue([room]);
-    client.getUserId.mockReturnValue(alice.userId);
-    client.getDeviceId.mockReturnValue("alices_device");
-    client.reEmitter.reEmit(room, [RoomStateEvent.Events]);
-    client.sendStateEvent.mockImplementation(async (roomId, eventType, content, stateKey = "") => {
-        if (roomId !== room.roomId) throw new Error("Unknown room");
-        const event = mkEvent({
-            event: true,
-            type: eventType,
-            room: roomId,
-            user: alice.userId,
-            skey: stateKey,
-            content: content as IContent,
-        });
-        room.addLiveEvents([event], { addToState: true });
-        return { event_id: event.getId()! };
-    });
-
-    setupAsyncStoreWithClient(WidgetStore.instance, client);
-    setupAsyncStoreWithClient(WidgetMessagingStore.instance, client);
-
-    return { client, room, alice, bob, carol, roomSession };
-};
-
-const cleanUpClientRoomAndStores = (client: MatrixClient, room: Room) => {
-    client.reEmitter.stopReEmitting(room, [RoomStateEvent.Events]);
-};
 
 const setUpWidget = (call: Call): { widget: Widget; messaging: Mocked<ClientWidgetApi> } => {
     call.widget.data = { ...call.widget, skipLobby: true };

test/unit-tests/stores/CallStore-test.ts

@@ -0,0 +1,38 @@
+/*
+ * Copyright 2025 New Vector Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+import { type CallMembership, MatrixRTCSessionManagerEvents } from "matrix-js-sdk/src/matrixrtc";
+
+import { ElementCall } from "../../../src/models/Call";
+import { CallStore } from "../../../src/stores/CallStore";
+import {
+    setUpClientRoomAndStores,
+    cleanUpClientRoomAndStores,
+    setupAsyncStoreWithClient,
+    enableCalls,
+} from "../../test-utils";
+
+enableCalls();
+
+test("CallStore constructs one call for one MatrixRTC session", () => {
+    const { client, room } = setUpClientRoomAndStores();
+    try {
+        setupAsyncStoreWithClient(CallStore.instance, client);
+        const getSpy = jest.spyOn(ElementCall, "get");
+
+        // Simulate another user starting a new MatrixRTC session
+        const session = client.matrixRTC.getRoomSession(room);
+        session.memberships.push({} as CallMembership);
+        client.matrixRTC.emit(MatrixRTCSessionManagerEvents.SessionStarted, room.roomId, session);
+
+        expect(getSpy).toHaveBeenCalledTimes(1);
+        expect(getSpy).toHaveReturnedWith(expect.any(ElementCall));
+        expect(CallStore.instance.getCall(room.roomId)).not.toBe(null);
+    } finally {
+        cleanUpClientRoomAndStores(client, room);
+    }
+});