PrivateCoffee/element-web
4
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 3

Fix widgets not being cleaned up correctly. (#12616)

Browse source 
* Fix widgets not being cleaned up correctly.

Widgets could persist forever because they were still sticky when we end the messaging.
Ending the messaging emits an event to which we connect ui changes that move the widget out of the screen. It does not end up in a pip view however.

So we need to make sure the widget is not persistend anymore when we call `stopMessagingByUid` so that any dom changes that remove the AppTile happen when the widget is not persistend anymore and let it destroy.

This PR also makes the role for `MatrixRTCSessionManager` more strict. We do ONLY use it in `Call.ts` and `CallStore`  so that we dont end up in reaces where we updated the ui based on the session manager but not in sync with the call and callstore changes.

Rename activeCalls to connectedCalls. Active call can also be understood as a call where there are active participants but the user itself is not connected. Especially with the `hasActiveCallSession` field of the useRoomCall hook which is tracking active (not necassarly connected sessions)

* rest of the renaming

* fix test to adapt to reduced session manager event usage.
This commit is contained in:
Timo 2024-06-17 13:00:41 +02:00 committed by GitHub
parent 28bcbd4056
commit 5c26d580d8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 54 additions and 65 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/components/structures/MatrixChat.tsx
View file

@ -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;

6
src/components/structures/RoomView.tsx
View file

@ -494,7 +494,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);
@ -815,7 +815,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) {
@ -1056,7 +1056,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

2
src/components/views/voip/CallView.tsx
View file

@ -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()));

8
src/hooks/room/useRoomCall.ts
View file

@ -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)) {
@ -200,7 +200,7 @@ export const useRoomCall = (
}
return State.NoCall;
}, [
activeCalls,
connectedCalls,
canInviteGuests,
hasGroupCall,
hasJitsiWidget,

2
src/models/Call.ts
View file

@ -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();
}
};

6
src/stores/ActiveWidgetStore.ts
View file

@ -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 {

30
src/stores/CallStore.ts
View file

@ -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<{}> {
@ -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
@ -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(
@ -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 => {
@ -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 => {
@ -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
src/stores/room-list/algorithms/Algorithm.ts
View file

@ -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 {
@ -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();
@ -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[] = [];

3
src/stores/widgets/StopGapWidget.ts
View file

@ -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();
@ -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>{});
}
},
);

30
src/toasts/IncomingCallToast.tsx
View file

@ -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";
@ -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}`;
@ -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";
@ -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.
@ -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 (
<>

2
test/components/structures/MatrixChat-test.tsx
View file

@ -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(),

4
test/components/views/rooms/RoomHeader-test.tsx
View file

@ -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());
@ -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());

14
test/toasts/IncomingCallToast-test.tsx
View file

@ -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";
@ -82,6 +78,7 @@ 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) =>
@ -89,7 +86,6 @@ describe("IncomingCallEvent", () => {
),
);
MockedCall.create(room, "1");
const maybeCall = CallStore.instance.getCall(room.roomId);
if (!(maybeCall instanceof MockedCall)) throw new Error("Failed to create call");
call = maybeCall;
@ -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();
@ -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(