Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Commit 62a740d

Browse files
authored
Fix call splitbrains when switching between rooms (#9692)
* Fix call splitbrains when switching between rooms Mounting CallView causes the user's call membership room state to be cleaned up. However, because the GroupCall object always thought the local device was disconnected from the call, it would remove the local device from room state when the clean method is called, causing a splitbrain. This uses GroupCall's new enteredViaAnotherSession field to fix that, and also simplify participant tracking. * Remove clean tests that have been moved to matrix-js-sdk
1 parent 81098b9 commit 62a740d

File tree

3 files changed

+3
-115
lines changed

3 files changed

+3
-115
lines changed

src/models/Call.ts

+2-25
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,6 @@ export class ElementCall extends Call {
647647
client,
648648
);
649649

650-
this.on(CallEvent.ConnectionState, this.onConnectionState);
651650
this.on(CallEvent.Participants, this.onParticipants);
652651
groupCall.on(GroupCallEvent.ParticipantsChanged, this.onGroupCallParticipants);
653652
groupCall.on(GroupCallEvent.GroupCallStateChanged, this.onGroupCallState);
@@ -704,6 +703,7 @@ export class ElementCall extends Call {
704703
throw new Error(`Failed to join call in room ${this.roomId}: ${e}`);
705704
}
706705

706+
this.groupCall.enteredViaAnotherSession = true;
707707
this.messaging!.on(`action:${ElementWidgetActions.HangupCall}`, this.onHangup);
708708
this.messaging!.on(`action:${ElementWidgetActions.TileLayout}`, this.onTileLayout);
709709
this.messaging!.on(`action:${ElementWidgetActions.SpotlightLayout}`, this.onSpotlightLayout);
@@ -724,11 +724,11 @@ export class ElementCall extends Call {
724724
this.messaging!.off(`action:${ElementWidgetActions.SpotlightLayout}`, this.onSpotlightLayout);
725725
this.messaging!.off(`action:${ElementWidgetActions.ScreenshareRequest}`, this.onScreenshareRequest);
726726
super.setDisconnected();
727+
this.groupCall.enteredViaAnotherSession = false;
727728
}
728729

729730
public destroy() {
730731
WidgetStore.instance.removeVirtualWidget(this.widget.id, this.groupCall.room.roomId);
731-
this.off(CallEvent.ConnectionState, this.onConnectionState);
732732
this.off(CallEvent.Participants, this.onParticipants);
733733
this.groupCall.off(GroupCallEvent.ParticipantsChanged, this.onGroupCallParticipants);
734734
this.groupCall.off(GroupCallEvent.GroupCallStateChanged, this.onGroupCallState);
@@ -760,20 +760,6 @@ export class ElementCall extends Call {
760760
participants.set(member, new Set(deviceMap.keys()));
761761
}
762762

763-
// We never enter group calls natively, so the GroupCall will think it's
764-
// disconnected regardless of what our call member state says. Thus we
765-
// have to insert our own device manually when connected via the widget.
766-
if (this.connected) {
767-
const localMember = this.room.getMember(this.client.getUserId()!)!;
768-
let devices = participants.get(localMember);
769-
if (devices === undefined) {
770-
devices = new Set();
771-
participants.set(localMember, devices);
772-
}
773-
774-
devices.add(this.client.getDeviceId()!);
775-
}
776-
777763
this.participants = participants;
778764
}
779765

@@ -782,15 +768,6 @@ export class ElementCall extends Call {
782768
&& this.room.currentState.mayClientSendStateEvent(ElementCall.CALL_EVENT_TYPE.name, this.client);
783769
}
784770

785-
private onConnectionState = (state: ConnectionState, prevState: ConnectionState) => {
786-
if (
787-
(state === ConnectionState.Connected && !isConnected(prevState))
788-
|| (state === ConnectionState.Disconnected && isConnected(prevState))
789-
) {
790-
this.updateParticipants(); // Local echo
791-
}
792-
};
793-
794771
private onParticipants = async (
795772
participants: Map<RoomMember, Set<string>>,
796773
prevParticipants: Map<RoomMember, Set<string>>,

test/models/Call-test.ts

-90
Original file line numberDiff line numberDiff line change
@@ -939,96 +939,6 @@ describe("ElementCall", () => {
939939

940940
call.off(CallEvent.Destroy, onDestroy);
941941
});
942-
943-
describe("clean", () => {
944-
const aliceWeb: IMyDevice = {
945-
device_id: "aliceweb",
946-
last_seen_ts: 0,
947-
};
948-
const aliceDesktop: IMyDevice = {
949-
device_id: "alicedesktop",
950-
last_seen_ts: 0,
951-
};
952-
const aliceDesktopOffline: IMyDevice = {
953-
device_id: "alicedesktopoffline",
954-
last_seen_ts: 1000 * 60 * 60 * -2, // 2 hours ago
955-
};
956-
const aliceDesktopNeverOnline: IMyDevice = {
957-
device_id: "alicedesktopneveronline",
958-
};
959-
960-
const mkContent = (devices: IMyDevice[]) => ({
961-
"m.calls": [{
962-
"m.call_id": call.groupCall.groupCallId,
963-
"m.devices": devices.map(d => ({
964-
device_id: d.device_id, session_id: "1", feeds: [], expires_ts: 1000 * 60 * 10,
965-
})),
966-
}],
967-
});
968-
const expectDevices = (devices: IMyDevice[]) => expect(
969-
room.currentState.getStateEvents(ElementCall.MEMBER_EVENT_TYPE.name, alice.userId)?.getContent(),
970-
).toEqual({
971-
"m.calls": [{
972-
"m.call_id": call.groupCall.groupCallId,
973-
"m.devices": devices.map(d => ({
974-
device_id: d.device_id, session_id: "1", feeds: [], expires_ts: expect.any(Number),
975-
})),
976-
}],
977-
});
978-
979-
beforeEach(() => {
980-
client.getDeviceId.mockReturnValue(aliceWeb.device_id);
981-
client.getDevices.mockResolvedValue({
982-
devices: [
983-
aliceWeb,
984-
aliceDesktop,
985-
aliceDesktopOffline,
986-
aliceDesktopNeverOnline,
987-
],
988-
});
989-
});
990-
991-
it("doesn't clean up valid devices", async () => {
992-
await client.sendStateEvent(
993-
room.roomId,
994-
ElementCall.MEMBER_EVENT_TYPE.name,
995-
mkContent([aliceDesktop]),
996-
alice.userId,
997-
);
998-
999-
await call.clean();
1000-
expectDevices([aliceDesktop]);
1001-
});
1002-
1003-
it("cleans up our own device if we're disconnected", async () => {
1004-
await client.sendStateEvent(
1005-
room.roomId,
1006-
ElementCall.MEMBER_EVENT_TYPE.name,
1007-
mkContent([aliceWeb, aliceDesktop]),
1008-
alice.userId,
1009-
);
1010-
1011-
await call.clean();
1012-
expectDevices([aliceDesktop]);
1013-
});
1014-
1015-
it("cleans up devices that have never been online", async () => {
1016-
await client.sendStateEvent(
1017-
room.roomId,
1018-
ElementCall.MEMBER_EVENT_TYPE.name,
1019-
mkContent([aliceDesktop, aliceDesktopNeverOnline]),
1020-
alice.userId,
1021-
);
1022-
1023-
await call.clean();
1024-
expectDevices([aliceDesktop]);
1025-
});
1026-
1027-
it("no-ops if there are no state events", async () => {
1028-
await call.clean();
1029-
expect(room.currentState.getStateEvents(JitsiCall.MEMBER_EVENT_TYPE, alice.userId)).toBe(null);
1030-
});
1031-
});
1032942
});
1033943

1034944
describe("instance in a video room", () => {

test/test-utils/test-utils.ts

+1
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ export function createTestClient(): MatrixClient {
9191
getDeviceId: jest.fn().mockReturnValue("ABCDEFGHI"),
9292
deviceId: "ABCDEFGHI",
9393
getDevices: jest.fn().mockResolvedValue({ devices: [{ device_id: "ABCDEFGHI" }] }),
94+
getSessionId: jest.fn().mockReturnValue("iaszphgvfku"),
9495
credentials: { userId: "@userId:matrix.org" },
9596

9697
store: {

0 commit comments

Comments
 (0)