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

Fix widgets not being cleaned up correctly. #12616

Merged
merged 3 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/structures/MatrixChat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
case "logout":
LegacyCallHandler.instance.hangupAllCalls();
Promise.all([
...[...CallStore.instance.activeCalls].map((call) => call.disconnect()),
...[...CallStore.instance.connectedCalls].map((call) => call.disconnect()),
cleanUpBroadcasts(this.stores),
]).finally(() => Lifecycle.logout(this.stores.oidcClientStore));
break;
Expand Down
6 changes: 3 additions & 3 deletions src/components/structures/RoomView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
WidgetEchoStore.on(UPDATE_EVENT, this.onWidgetEchoStoreUpdate);
context.widgetStore.on(UPDATE_EVENT, this.onWidgetStoreUpdate);

CallStore.instance.on(CallStoreEvent.ActiveCalls, this.onActiveCalls);
CallStore.instance.on(CallStoreEvent.ConnectedCalls, this.onConnectedCalls);

this.props.resizeNotifier.on("isResizing", this.onIsResizing);

Expand Down Expand Up @@ -811,7 +811,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
}
};

private onActiveCalls = (): void => {
private onConnectedCalls = (): void => {
if (this.state.roomId === undefined) return;
const activeCall = CallStore.instance.getActiveCall(this.state.roomId);
if (activeCall === null) {
Expand Down Expand Up @@ -1052,7 +1052,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
);
}

CallStore.instance.off(CallStoreEvent.ActiveCalls, this.onActiveCalls);
CallStore.instance.off(CallStoreEvent.ConnectedCalls, this.onConnectedCalls);
this.context.legacyCallHandler.off(LegacyCallHandlerEvent.CallState, this.onCallState);

// cancel any pending calls to the throttled updated
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/voip/CallView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const JoinCallView: FC<JoinCallViewProps> = ({ room, resizing, call, skipLobby,
const disconnectAllOtherCalls: () => Promise<void> = useCallback(async () => {
// The stickyPromise has to resolve before the widget actually becomes sticky.
// We only let the widget become sticky after disconnecting all other active calls.
const calls = [...CallStore.instance.activeCalls].filter(
const calls = [...CallStore.instance.connectedCalls].filter(
(call) => SdkContextClass.instance.roomViewStore.getRoomId() !== call.roomId,
);
await Promise.all(calls.map(async (call) => await call.disconnect()));
Expand Down
8 changes: 4 additions & 4 deletions src/hooks/room/useRoomCall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,13 +176,13 @@ export const useRoomCall = (
// We only want to prompt to pin the widget if it's not element call based.
const isECWidget = WidgetType.CALL.matches(widget?.type ?? "");
const promptPinWidget = !isECWidget && canPinWidget && !widgetPinned;
const activeCalls = useEventEmitterState(CallStore.instance, CallStoreEvent.ActiveCalls, () =>
Array.from(CallStore.instance.activeCalls),
const connectedCalls = useEventEmitterState(CallStore.instance, CallStoreEvent.ConnectedCalls, () =>
Array.from(CallStore.instance.connectedCalls),
);
const { canInviteGuests } = useGuestAccessInformation(room);

const state = useMemo((): State => {
if (activeCalls.find((call) => call.roomId != room.roomId)) {
if (connectedCalls.find((call) => call.roomId != room.roomId)) {
return State.Ongoing;
}
if (hasGroupCall && (hasJitsiWidget || hasManagedHybridWidget)) {
Expand All @@ -200,7 +200,7 @@ export const useRoomCall = (
}
return State.NoCall;
}, [
activeCalls,
connectedCalls,
canInviteGuests,
hasGroupCall,
hasJitsiWidget,
Expand Down
2 changes: 1 addition & 1 deletion src/models/Call.ts
Original file line number Diff line number Diff line change
Expand Up @@ -963,7 +963,7 @@ export class ElementCall extends Call {

private onRTCSessionEnded = (roomId: string, session: MatrixRTCSession): void => {
// Don't destroy the call on hangup for video call rooms.
if (roomId == this.roomId && !this.room.isCallRoom()) {
if (roomId === this.roomId && !this.room.isCallRoom()) {
this.destroy();
}
};
Expand Down
6 changes: 5 additions & 1 deletion src/stores/ActiveWidgetStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,12 @@ export default class ActiveWidgetStore extends EventEmitter {

public destroyPersistentWidget(widgetId: string, roomId: string | null): void {
if (!this.getWidgetPersistence(widgetId, roomId)) return;
WidgetMessagingStore.instance.stopMessagingByUid(WidgetUtils.calcWidgetUid(widgetId, roomId ?? undefined));
// We first need to set the widget persistence to false
this.setWidgetPersistence(widgetId, roomId, false);
// Then we can stop the messaging. Stopping the messaging emits - we might move the widget out of sight.
// If we would do this before setting the persistence to false, it would stay in the DOM (hidden) because
// its still persistent. We need to avoid this.
WidgetMessagingStore.instance.stopMessagingByUid(WidgetUtils.calcWidgetUid(widgetId, roomId ?? undefined));
}

public setWidgetPersistence(widgetId: string, roomId: string | null, val: boolean): void {
Expand Down
30 changes: 14 additions & 16 deletions src/stores/CallStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export enum CallStoreEvent {
// Signals a change in the call associated with a given room
Call = "call",
// Signals a change in the active calls
ActiveCalls = "active_calls",
ConnectedCalls = "connected_calls",
}

export class CallStore extends AsyncStoreWithClient<{}> {
Expand Down Expand Up @@ -66,8 +66,7 @@ export class CallStore extends AsyncStoreWithClient<{}> {
}
this.matrixClient.on(GroupCallEventHandlerEvent.Incoming, this.onGroupCall);
this.matrixClient.on(GroupCallEventHandlerEvent.Outgoing, this.onGroupCall);
this.matrixClient.matrixRTC.on(MatrixRTCSessionManagerEvents.SessionStarted, this.onRTCSession);
this.matrixClient.matrixRTC.on(MatrixRTCSessionManagerEvents.SessionEnded, this.onRTCSession);
this.matrixClient.matrixRTC.on(MatrixRTCSessionManagerEvents.SessionStarted, this.onRTCSessionStart);
WidgetStore.instance.on(UPDATE_EVENT, this.onWidgets);

// If the room ID of a previously connected call is still in settings at
Expand Down Expand Up @@ -95,28 +94,27 @@ export class CallStore extends AsyncStoreWithClient<{}> {
}
this.callListeners.clear();
this.calls.clear();
this._activeCalls.clear();
this._connectedCalls.clear();

if (this.matrixClient) {
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.onRTCSession);
this.matrixClient.matrixRTC.off(MatrixRTCSessionManagerEvents.SessionEnded, this.onRTCSession);
this.matrixClient.matrixRTC.off(MatrixRTCSessionManagerEvents.SessionStarted, this.onRTCSessionStart);
}
WidgetStore.instance.off(UPDATE_EVENT, this.onWidgets);
}

private _activeCalls: Set<Call> = new Set();
private _connectedCalls: Set<Call> = new Set();
/**
* The calls to which the user is currently connected.
*/
public get activeCalls(): Set<Call> {
return this._activeCalls;
public get connectedCalls(): Set<Call> {
return this._connectedCalls;
}
private set activeCalls(value: Set<Call>) {
this._activeCalls = value;
this.emit(CallStoreEvent.ActiveCalls, value);
private set connectedCalls(value: Set<Call>) {
this._connectedCalls = value;
this.emit(CallStoreEvent.ConnectedCalls, value);

// The room IDs are persisted to settings so we can detect unclean disconnects
SettingsStore.setValue(
Expand All @@ -137,9 +135,9 @@ export class CallStore extends AsyncStoreWithClient<{}> {
if (call) {
const onConnectionState = (state: ConnectionState): void => {
if (state === ConnectionState.Connected) {
this.activeCalls = new Set([...this.activeCalls, call]);
this.connectedCalls = new Set([...this.connectedCalls, call]);
} else if (state === ConnectionState.Disconnected) {
this.activeCalls = new Set([...this.activeCalls].filter((c) => c !== call));
this.connectedCalls = new Set([...this.connectedCalls].filter((c) => c !== call));
}
};
const onDestroy = (): void => {
Expand Down Expand Up @@ -181,7 +179,7 @@ export class CallStore extends AsyncStoreWithClient<{}> {
*/
public getActiveCall(roomId: string): Call | null {
const call = this.getCall(roomId);
return call !== null && this.activeCalls.has(call) ? call : null;
return call !== null && this.connectedCalls.has(call) ? call : null;
}

private onWidgets = (roomId: string | null): void => {
Expand All @@ -200,7 +198,7 @@ export class CallStore extends AsyncStoreWithClient<{}> {
};

private onGroupCall = (groupCall: GroupCall): void => this.updateRoom(groupCall.room);
private onRTCSession = (roomId: string, session: MatrixRTCSession): void => {
private onRTCSessionStart = (roomId: string, session: MatrixRTCSession): void => {
this.updateRoom(session.room);
};
}
10 changes: 5 additions & 5 deletions src/stores/room-list/algorithms/Algorithm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ export class Algorithm extends EventEmitter {
public updatesInhibited = false;

public start(): void {
CallStore.instance.on(CallStoreEvent.ActiveCalls, this.onActiveCalls);
CallStore.instance.on(CallStoreEvent.ConnectedCalls, this.onConnectedCalls);
}

public stop(): void {
CallStore.instance.off(CallStoreEvent.ActiveCalls, this.onActiveCalls);
CallStore.instance.off(CallStoreEvent.ConnectedCalls, this.onConnectedCalls);
}

public get stickyRoom(): Room | null {
Expand Down Expand Up @@ -302,7 +302,7 @@ export class Algorithm extends EventEmitter {
return this._stickyRoom;
}

private onActiveCalls = (): void => {
private onConnectedCalls = (): void => {
// In case we're unsticking a room, sort it back into natural order
this.recalculateStickyRoom();

Expand Down Expand Up @@ -396,12 +396,12 @@ export class Algorithm extends EventEmitter {
return;
}

if (CallStore.instance.activeCalls.size) {
if (CallStore.instance.connectedCalls.size) {
// We operate on the sticky rooms map
if (!this._cachedStickyRooms) this.initCachedStickyRooms();
const rooms = this._cachedStickyRooms![updatedTag];

const activeRoomIds = new Set([...CallStore.instance.activeCalls].map((call) => call.roomId));
const activeRoomIds = new Set([...CallStore.instance.connectedCalls].map((call) => call.roomId));
const activeRooms: Room[] = [];
const inactiveRooms: Room[] = [];

Expand Down
3 changes: 2 additions & 1 deletion src/stores/widgets/StopGapWidget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,6 @@ export class StopGapWidget extends EventEmitter {
async (ev: CustomEvent<IStickyActionRequest>) => {
if (this.messaging?.hasCapability(MatrixCapabilities.AlwaysOnScreen)) {
ev.preventDefault();
this.messaging.transport.reply(ev.detail, <IWidgetApiRequestEmptyData>{}); // ack
if (ev.detail.data.value) {
// If the widget wants to become sticky we wait for the stickyPromise to resolve
if (this.stickyPromise) await this.stickyPromise();
Expand All @@ -356,6 +355,8 @@ export class StopGapWidget extends EventEmitter {
this.roomId ?? null,
ev.detail.data.value,
);
// Send the ack after the widget actually has become sticky.
this.messaging.transport.reply(ev.detail, <IWidgetApiRequestEmptyData>{});
}
},
);
Expand Down
30 changes: 12 additions & 18 deletions src/toasts/IncomingCallToast.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ limitations under the License.

import React, { useCallback, useEffect, useMemo, useState } from "react";
import { MatrixEvent } from "matrix-js-sdk/src/matrix";
// eslint-disable-next-line no-restricted-imports
import { MatrixRTCSessionManagerEvents } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSessionManager";
// eslint-disable-next-line no-restricted-imports
import { MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSession";
import { Button, Tooltip } from "@vector-im/compound-web";
import { Icon as VideoCallIcon } from "@vector-im/compound-design-tokens/icons/video-call-solid.svg";

Expand All @@ -41,7 +37,7 @@ import { useDispatcher } from "../hooks/useDispatcher";
import { ActionPayload } from "../dispatcher/payloads";
import { Call } from "../models/Call";
import { AudioID } from "../LegacyCallHandler";
import { useEventEmitter, useTypedEventEmitter } from "../hooks/useEventEmitter";
import { useEventEmitter } from "../hooks/useEventEmitter";
import { CallStore, CallStoreEvent } from "../stores/CallStore";

export const getIncomingCallToastKey = (callId: string, roomId: string): string => `call_${callId}_${roomId}`;
Expand Down Expand Up @@ -83,11 +79,11 @@ export function IncomingCallToast({ notifyEvent }: Props): JSX.Element {
const room = MatrixClientPeg.safeGet().getRoom(roomId) ?? undefined;
const call = useCall(roomId);
const audio = useMemo(() => document.getElementById(AudioID.Ring) as HTMLMediaElement, []);
const [activeCalls, setActiveCalls] = useState<Call[]>(Array.from(CallStore.instance.activeCalls));
useEventEmitter(CallStore.instance, CallStoreEvent.ActiveCalls, () => {
setActiveCalls(Array.from(CallStore.instance.activeCalls));
const [connectedCalls, setConnectedCalls] = useState<Call[]>(Array.from(CallStore.instance.connectedCalls));
useEventEmitter(CallStore.instance, CallStoreEvent.ConnectedCalls, () => {
setConnectedCalls(Array.from(CallStore.instance.connectedCalls));
});
const otherCallIsOngoing = activeCalls.find((call) => call.roomId !== roomId);
const otherCallIsOngoing = connectedCalls.find((call) => call.roomId !== roomId);
// Start ringing if not already.
useEffect(() => {
const isRingToast = (notifyEvent.getContent() as unknown as { notify_type: string })["notify_type"] == "ring";
Expand All @@ -105,13 +101,15 @@ export function IncomingCallToast({ notifyEvent }: Props): JSX.Element {
}, [audio, notifyEvent, roomId]);

// Dismiss if session got ended remotely.
const onSessionEnded = useCallback(
(endedSessionRoomId: string, session: MatrixRTCSession): void => {
if (roomId == endedSessionRoomId && session.callId == notifyEvent.getContent().call_id) {
const onCall = useCallback(
(call: Call, callRoomId: string): void => {
const roomId = notifyEvent.getRoomId();
if (!roomId && roomId !== callRoomId) return;
if (call === null || call.participants.size === 0) {
dismissToast();
}
},
[dismissToast, notifyEvent, roomId],
[dismissToast, notifyEvent],
);

// Dismiss on timeout.
Expand Down Expand Up @@ -160,11 +158,7 @@ export function IncomingCallToast({ notifyEvent }: Props): JSX.Element {
[dismissToast],
);

useTypedEventEmitter(
MatrixClientPeg.safeGet().matrixRTC,
MatrixRTCSessionManagerEvents.SessionEnded,
onSessionEnded,
);
useEventEmitter(CallStore.instance, CallStoreEvent.Call, onCall);

return (
<>
Expand Down
2 changes: 1 addition & 1 deletion test/components/structures/MatrixChat-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ describe("<MatrixChat />", () => {
jest.spyOn(PosthogAnalytics.instance, "logout").mockImplementation(() => {});
jest.spyOn(EventIndexPeg, "deleteEventIndex").mockImplementation(async () => {});

jest.spyOn(CallStore.instance, "activeCalls", "get").mockReturnValue(new Set([call1, call2]));
jest.spyOn(CallStore.instance, "connectedCalls", "get").mockReturnValue(new Set([call1, call2]));

mockPlatformPeg({
destroyPickleKey: jest.fn(),
Expand Down
4 changes: 2 additions & 2 deletions test/components/views/rooms/RoomHeader-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ describe("RoomHeader", () => {
it("buttons are disabled if there is an ongoing call", async () => {
mockRoomMembers(room, 3);

jest.spyOn(CallStore.prototype, "activeCalls", "get").mockReturnValue(
jest.spyOn(CallStore.prototype, "connectedCalls", "get").mockReturnValue(
new Set([{ roomId: "some_other_room" } as Call]),
);
const { container } = render(<RoomHeader room={room} />, getWrapper());
Expand All @@ -514,7 +514,7 @@ describe("RoomHeader", () => {
it("join button is disabled if there is an other ongoing call", async () => {
mockRoomMembers(room, 3);
jest.spyOn(UseCall, "useParticipantCount").mockReturnValue(3);
jest.spyOn(CallStore.prototype, "activeCalls", "get").mockReturnValue(
jest.spyOn(CallStore.prototype, "connectedCalls", "get").mockReturnValue(
new Set([{ roomId: "some_other_room" } as Call]),
);
const { container } = render(<RoomHeader room={room} />, getWrapper());
Expand Down
14 changes: 3 additions & 11 deletions test/toasts/IncomingCallToast-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,6 @@ import { mocked, Mocked } from "jest-mock";
import { Room, RoomStateEvent, MatrixEvent, MatrixEventEvent, MatrixClient } from "matrix-js-sdk/src/matrix";
import { ClientWidgetApi, Widget } from "matrix-widget-api";
// eslint-disable-next-line no-restricted-imports
import { MatrixRTCSessionManagerEvents } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSessionManager";
// eslint-disable-next-line no-restricted-imports
import { MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSession";
// eslint-disable-next-line no-restricted-imports
import { ICallNotifyContent } from "matrix-js-sdk/src/matrixrtc/types";

import type { RoomMember } from "matrix-js-sdk/src/matrix";
Expand Down Expand Up @@ -82,14 +78,14 @@ describe("IncomingCallEvent", () => {
client.getRoom.mockImplementation((roomId) => (roomId === room.roomId ? room : null));
client.getRooms.mockReturnValue([room]);
client.reEmitter.reEmit(room, [RoomStateEvent.Events]);
MockedCall.create(room, "1");

await Promise.all(
[CallStore.instance, WidgetMessagingStore.instance].map((store) =>
setupAsyncStoreWithClient(store, client),
),
);

MockedCall.create(room, "1");
const maybeCall = CallStore.instance.getCall(room.roomId);
if (!(maybeCall instanceof MockedCall)) throw new Error("Failed to create call");
call = maybeCall;
Expand Down Expand Up @@ -179,7 +175,7 @@ describe("IncomingCallEvent", () => {

defaultDispatcher.unregister(dispatcherRef);
});
it("skips lobby when using shift key click", async () => {
it("Dismiss toast if user starts call and skips lobby when using shift key click", async () => {
renderToast();

const dispatcherSpy = jest.fn();
Expand Down Expand Up @@ -250,11 +246,7 @@ describe("IncomingCallEvent", () => {

it("closes toast when the matrixRTC session has ended", async () => {
renderToast();

client.matrixRTC.emit(MatrixRTCSessionManagerEvents.SessionEnded, room.roomId, {
callId: notifyContent.call_id,
room: room,
} as unknown as MatrixRTCSession);
call.destroy();

await waitFor(() =>
expect(toastStore.dismissToast).toHaveBeenCalledWith(
Expand Down
Loading