From d7b75e4b9e25463b355f922b21fa3e7cd89ad4fc Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 24 Mar 2023 14:09:22 +0000 Subject: [PATCH] Add the call object to Call events As explained in the comment. I've added it to the end so this should be completely backwards compatible (although it would be much nicer if it were the first arg, probably). --- spec/unit/webrtc/call.spec.ts | 4 +- spec/unit/webrtc/callFeed.spec.ts | 2 +- spec/unit/webrtc/groupCall.spec.ts | 6 +- src/webrtc/call.ts | 110 ++++++++++++++++++----------- 4 files changed, 75 insertions(+), 47 deletions(-) diff --git a/spec/unit/webrtc/call.spec.ts b/spec/unit/webrtc/call.spec.ts index 51798d441..646a2c7ee 100644 --- a/spec/unit/webrtc/call.spec.ts +++ b/spec/unit/webrtc/call.spec.ts @@ -737,7 +737,7 @@ describe("Call", function () { const dataChannel = call.createDataChannel("data_channel_label", { id: 123 }); - expect(dataChannelCallback).toHaveBeenCalledWith(dataChannel); + expect(dataChannelCallback).toHaveBeenCalledWith(dataChannel, call); expect(dataChannel.label).toBe("data_channel_label"); expect(dataChannel.id).toBe(123); }); @@ -1604,7 +1604,7 @@ describe("Call", function () { hasAdvancedBy += advanceBy; expect(lengthChangedListener).toHaveBeenCalledTimes(hasAdvancedBy); - expect(lengthChangedListener).toHaveBeenCalledWith(hasAdvancedBy); + expect(lengthChangedListener).toHaveBeenCalledWith(hasAdvancedBy, call); } }); diff --git a/spec/unit/webrtc/callFeed.spec.ts b/spec/unit/webrtc/callFeed.spec.ts index 803e4648e..ea80d5267 100644 --- a/spec/unit/webrtc/callFeed.spec.ts +++ b/spec/unit/webrtc/callFeed.spec.ts @@ -102,7 +102,7 @@ describe("CallFeed", () => { [CallState.Connected, true], [CallState.Connecting, false], ])("should react to call state, when !isLocal()", (state: CallState, expected: Boolean) => { - call.emit(CallEvent.State, state); + call.emit(CallEvent.State, state, CallState.InviteSent, call.typed()); expect(feed.connected).toBe(expected); }); diff --git a/spec/unit/webrtc/groupCall.spec.ts b/spec/unit/webrtc/groupCall.spec.ts index 9743b332e..c1cd0d3cb 100644 --- a/spec/unit/webrtc/groupCall.spec.ts +++ b/spec/unit/webrtc/groupCall.spec.ts @@ -792,7 +792,7 @@ describe("Group Call", function () { call.isLocalVideoMuted = jest.fn().mockReturnValue(true); call.setLocalVideoMuted = jest.fn(); - call.emit(CallEvent.State, CallState.Connected); + call.emit(CallEvent.State, CallState.Connected, CallState.InviteSent, call); expect(call.setMicrophoneMuted).toHaveBeenCalledWith(false); expect(call.setLocalVideoMuted).toHaveBeenCalledWith(false); @@ -1080,7 +1080,7 @@ describe("Group Call", function () { }); it("handles regular case", () => { - oldMockCall.emit(CallEvent.Replaced, newMockCall.typed()); + oldMockCall.emit(CallEvent.Replaced, newMockCall.typed(), oldMockCall.typed()); expect(oldMockCall.hangup).toHaveBeenCalled(); expect(callChangedListener).toHaveBeenCalledWith(newCallsMap); @@ -1091,7 +1091,7 @@ describe("Group Call", function () { it("handles case where call is missing from the calls map", () => { // @ts-ignore groupCall.calls = new Map(); - oldMockCall.emit(CallEvent.Replaced, newMockCall.typed()); + oldMockCall.emit(CallEvent.Replaced, newMockCall.typed(), oldMockCall.typed()); expect(oldMockCall.hangup).toHaveBeenCalled(); expect(callChangedListener).toHaveBeenCalledWith(newCallsMap); diff --git a/src/webrtc/call.ts b/src/webrtc/call.ts index d74b755eb..85f4fb885 100644 --- a/src/webrtc/call.ts +++ b/src/webrtc/call.ts @@ -296,20 +296,34 @@ export interface VoipEvent { content: Record; } +/** + * These now all have the call object as an argument. Why? Well, to know which call a given event is + * about you have three options: + * 1. Use a closure as the callback that remembers what call it's listening to. This can be + * a pain because you need to pass the listener function again when you remove the listener, + * which might be somewhere else. + * 2. Use not-very-well-known fact that EventEmitter sets 'this' to the emitter object in the + * callback. This doesn't really play well with modern Typescript and eslint and doesn't work + * with our pattern of re-emitting events. + * 3. Pass the object in question as an argument to the callback. + * + * Now that we have group calls which have to deal with multiple call objects, this will + * become more important, and I think methods 1 and 2 are just going to cause issues. + */ export type CallEventHandlerMap = { - [CallEvent.DataChannel]: (channel: RTCDataChannel) => void; - [CallEvent.FeedsChanged]: (feeds: CallFeed[]) => void; - [CallEvent.Replaced]: (newCall: MatrixCall) => void; - [CallEvent.Error]: (error: CallError) => void; - [CallEvent.RemoteHoldUnhold]: (onHold: boolean) => void; - [CallEvent.LocalHoldUnhold]: (onHold: boolean) => void; - [CallEvent.LengthChanged]: (length: number) => void; - [CallEvent.State]: (state: CallState, oldState?: CallState) => void; + [CallEvent.DataChannel]: (channel: RTCDataChannel, call: MatrixCall) => void; + [CallEvent.FeedsChanged]: (feeds: CallFeed[], call: MatrixCall) => void; + [CallEvent.Replaced]: (newCall: MatrixCall, oldCall: MatrixCall) => void; + [CallEvent.Error]: (error: CallError, call: MatrixCall) => void; + [CallEvent.RemoteHoldUnhold]: (onHold: boolean, call: MatrixCall) => void; + [CallEvent.LocalHoldUnhold]: (onHold: boolean, call: MatrixCall) => void; + [CallEvent.LengthChanged]: (length: number, call: MatrixCall) => void; + [CallEvent.State]: (state: CallState, oldState: CallState, call: MatrixCall) => void; [CallEvent.Hangup]: (call: MatrixCall) => void; - [CallEvent.AssertedIdentityChanged]: () => void; + [CallEvent.AssertedIdentityChanged]: (call: MatrixCall) => void; /* @deprecated */ [CallEvent.HoldUnhold]: (onHold: boolean) => void; - [CallEvent.SendVoipEvent]: (event: VoipEvent) => void; + [CallEvent.SendVoipEvent]: (event: VoipEvent, call: MatrixCall) => void; }; // The key of the transceiver map (purpose + media type, separated by ':') @@ -459,7 +473,7 @@ export class MatrixCall extends TypedEventEmittererror), + this, ); } } @@ -1513,7 +1528,7 @@ export class MatrixCall extends TypedEventEmittererror)); + this.emit(CallEvent.Error, new CallError(code, message, error), this); throw error; } @@ -1987,7 +2002,7 @@ export class MatrixCall extends TypedEventEmittererror)); + this.emit(CallEvent.Error, new CallError(code, message, error), this); this.terminate(CallParty.Local, code, false); // no need to carry on & send the candidate queue, but we also @@ -2158,7 +2173,11 @@ export class MatrixCall extends TypedEventEmitter { logger.error(`Call ${this.callId} getLocalOfferFailed() running`, err); - this.emit(CallEvent.Error, new CallError(CallErrorCode.LocalOfferFailed, "Failed to get local offer!", err)); + this.emit( + CallEvent.Error, + new CallError(CallErrorCode.LocalOfferFailed, "Failed to get local offer!", err), + this, + ); this.terminate(CallParty.Local, CallErrorCode.LocalOfferFailed, false); }; @@ -2177,6 +2196,7 @@ export class MatrixCall extends TypedEventEmitter { - this.emit(CallEvent.LengthChanged, Math.round((Date.now() - this.callStartTime!) / 1000)); + this.emit(CallEvent.LengthChanged, Math.round((Date.now() - this.callStartTime!) / 1000), this); }, CALL_LENGTH_INTERVAL); } } else if (this.peerConn?.iceConnectionState == "failed") { @@ -2267,7 +2287,7 @@ export class MatrixCall extends TypedEventEmitter { - this.emit(CallEvent.DataChannel, ev.channel); + this.emit(CallEvent.DataChannel, ev.channel, this); }; /** @@ -2380,13 +2400,17 @@ export class MatrixCall extends TypedEventEmittererror)); + this.emit(CallEvent.Error, new CallError(code, message, error), this); this.hangup(code, false); return;