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

Commit

Permalink
Fix call splitbrains when switching between rooms (#9692)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
robintown committed Dec 2, 2022
1 parent 81098b9 commit 62a740d
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 115 deletions.
27 changes: 2 additions & 25 deletions src/models/Call.ts
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,6 @@ export class ElementCall extends Call {
client,
);

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

this.groupCall.enteredViaAnotherSession = true;
this.messaging!.on(`action:${ElementWidgetActions.HangupCall}`, this.onHangup);
this.messaging!.on(`action:${ElementWidgetActions.TileLayout}`, this.onTileLayout);
this.messaging!.on(`action:${ElementWidgetActions.SpotlightLayout}`, this.onSpotlightLayout);
Expand All @@ -724,11 +724,11 @@ export class ElementCall extends Call {
this.messaging!.off(`action:${ElementWidgetActions.SpotlightLayout}`, this.onSpotlightLayout);
this.messaging!.off(`action:${ElementWidgetActions.ScreenshareRequest}`, this.onScreenshareRequest);
super.setDisconnected();
this.groupCall.enteredViaAnotherSession = false;
}

public destroy() {
WidgetStore.instance.removeVirtualWidget(this.widget.id, this.groupCall.room.roomId);
this.off(CallEvent.ConnectionState, this.onConnectionState);
this.off(CallEvent.Participants, this.onParticipants);
this.groupCall.off(GroupCallEvent.ParticipantsChanged, this.onGroupCallParticipants);
this.groupCall.off(GroupCallEvent.GroupCallStateChanged, this.onGroupCallState);
Expand Down Expand Up @@ -760,20 +760,6 @@ export class ElementCall extends Call {
participants.set(member, new Set(deviceMap.keys()));
}

// We never enter group calls natively, so the GroupCall will think it's
// disconnected regardless of what our call member state says. Thus we
// have to insert our own device manually when connected via the widget.
if (this.connected) {
const localMember = this.room.getMember(this.client.getUserId()!)!;
let devices = participants.get(localMember);
if (devices === undefined) {
devices = new Set();
participants.set(localMember, devices);
}

devices.add(this.client.getDeviceId()!);
}

this.participants = participants;
}

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

private onConnectionState = (state: ConnectionState, prevState: ConnectionState) => {
if (
(state === ConnectionState.Connected && !isConnected(prevState))
|| (state === ConnectionState.Disconnected && isConnected(prevState))
) {
this.updateParticipants(); // Local echo
}
};

private onParticipants = async (
participants: Map<RoomMember, Set<string>>,
prevParticipants: Map<RoomMember, Set<string>>,
Expand Down
90 changes: 0 additions & 90 deletions test/models/Call-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -939,96 +939,6 @@ describe("ElementCall", () => {

call.off(CallEvent.Destroy, onDestroy);
});

describe("clean", () => {
const aliceWeb: IMyDevice = {
device_id: "aliceweb",
last_seen_ts: 0,
};
const aliceDesktop: IMyDevice = {
device_id: "alicedesktop",
last_seen_ts: 0,
};
const aliceDesktopOffline: IMyDevice = {
device_id: "alicedesktopoffline",
last_seen_ts: 1000 * 60 * 60 * -2, // 2 hours ago
};
const aliceDesktopNeverOnline: IMyDevice = {
device_id: "alicedesktopneveronline",
};

const mkContent = (devices: IMyDevice[]) => ({
"m.calls": [{
"m.call_id": call.groupCall.groupCallId,
"m.devices": devices.map(d => ({
device_id: d.device_id, session_id: "1", feeds: [], expires_ts: 1000 * 60 * 10,
})),
}],
});
const expectDevices = (devices: IMyDevice[]) => expect(
room.currentState.getStateEvents(ElementCall.MEMBER_EVENT_TYPE.name, alice.userId)?.getContent(),
).toEqual({
"m.calls": [{
"m.call_id": call.groupCall.groupCallId,
"m.devices": devices.map(d => ({
device_id: d.device_id, session_id: "1", feeds: [], expires_ts: expect.any(Number),
})),
}],
});

beforeEach(() => {
client.getDeviceId.mockReturnValue(aliceWeb.device_id);
client.getDevices.mockResolvedValue({
devices: [
aliceWeb,
aliceDesktop,
aliceDesktopOffline,
aliceDesktopNeverOnline,
],
});
});

it("doesn't clean up valid devices", async () => {
await client.sendStateEvent(
room.roomId,
ElementCall.MEMBER_EVENT_TYPE.name,
mkContent([aliceDesktop]),
alice.userId,
);

await call.clean();
expectDevices([aliceDesktop]);
});

it("cleans up our own device if we're disconnected", async () => {
await client.sendStateEvent(
room.roomId,
ElementCall.MEMBER_EVENT_TYPE.name,
mkContent([aliceWeb, aliceDesktop]),
alice.userId,
);

await call.clean();
expectDevices([aliceDesktop]);
});

it("cleans up devices that have never been online", async () => {
await client.sendStateEvent(
room.roomId,
ElementCall.MEMBER_EVENT_TYPE.name,
mkContent([aliceDesktop, aliceDesktopNeverOnline]),
alice.userId,
);

await call.clean();
expectDevices([aliceDesktop]);
});

it("no-ops if there are no state events", async () => {
await call.clean();
expect(room.currentState.getStateEvents(JitsiCall.MEMBER_EVENT_TYPE, alice.userId)).toBe(null);
});
});
});

describe("instance in a video room", () => {
Expand Down
1 change: 1 addition & 0 deletions test/test-utils/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ export function createTestClient(): MatrixClient {
getDeviceId: jest.fn().mockReturnValue("ABCDEFGHI"),
deviceId: "ABCDEFGHI",
getDevices: jest.fn().mockResolvedValue({ devices: [{ device_id: "ABCDEFGHI" }] }),
getSessionId: jest.fn().mockReturnValue("iaszphgvfku"),
credentials: { userId: "@userId:matrix.org" },

store: {
Expand Down

0 comments on commit 62a740d

Please sign in to comment.