Add local echo of connected devices in video rooms (#8368)

robintown · web-flow · commit c83ad1faa78a · 2022-04-21T07:41:38.000-04:00
diff --git a/src/components/views/rooms/RoomTile.tsx b/src/components/views/rooms/RoomTile.tsx
@@ -81,7 +81,7 @@ interface IState {
     messagePreview?: string;
     videoStatus: VideoStatus;
     // Active video channel members, according to room state
-    videoMembers: RoomMember[];
+    videoMembers: Set<RoomMember>;
     // Active video channel members, according to Jitsi
     jitsiParticipants: IJitsiParticipant[];
 }
@@ -124,7 +124,7 @@ export default class RoomTile extends React.PureComponent<IProps, IState> {
             // generatePreview() will return nothing if the user has previews disabled
             messagePreview: "",
             videoStatus,
-            videoMembers: getConnectedMembers(this.props.room.currentState),
+            videoMembers: getConnectedMembers(this.props.room, videoStatus === VideoStatus.Connected),
             jitsiParticipants: VideoChannelStore.instance.participants,
         };
         this.generatePreview();
@@ -593,7 +593,9 @@ export default class RoomTile extends React.PureComponent<IProps, IState> {
     }
 
     private updateVideoMembers = () => {
-        this.setState({ videoMembers: getConnectedMembers(this.props.room.currentState) });
+        this.setState(state => ({
+            videoMembers: getConnectedMembers(this.props.room, state.videoStatus === VideoStatus.Connected),
+        }));
     };
 
     private updateVideoStatus = () => {
@@ -610,7 +612,10 @@ export default class RoomTile extends React.PureComponent<IProps, IState> {
 
     private onConnectVideo = (roomId: string) => {
         if (roomId === this.props.room?.roomId) {
-            this.setState({ videoStatus: VideoStatus.Connected });
+            this.setState({
+                videoStatus: VideoStatus.Connected,
+                videoMembers: getConnectedMembers(this.props.room, true),
+            });
             VideoChannelStore.instance.on(VideoChannelEvent.Participants, this.updateJitsiParticipants);
         }
     };
@@ -623,7 +628,10 @@ export default class RoomTile extends React.PureComponent<IProps, IState> {
 
     private onDisconnectVideo = (roomId: string) => {
         if (roomId === this.props.room?.roomId) {
-            this.setState({ videoStatus: VideoStatus.Disconnected });
+            this.setState({
+                videoStatus: VideoStatus.Disconnected,
+                videoMembers: getConnectedMembers(this.props.room, false),
+            });
             VideoChannelStore.instance.off(VideoChannelEvent.Participants, this.updateJitsiParticipants);
         }
     };
@@ -668,12 +676,12 @@ export default class RoomTile extends React.PureComponent<IProps, IState> {
                 case VideoStatus.Disconnected:
                     videoText = _t("Video");
                     videoActive = false;
-                    participantCount = this.state.videoMembers.length;
+                    participantCount = this.state.videoMembers.size;
                     break;
                 case VideoStatus.Connecting:
                     videoText = _t("Connecting...");
                     videoActive = true;
-                    participantCount = this.state.videoMembers.length;
+                    participantCount = this.state.videoMembers.size;
                     break;
                 case VideoStatus.Connected:
                     videoText = _t("Connected");
diff --git a/src/components/views/voip/VideoLobby.tsx b/src/components/views/voip/VideoLobby.tsx
@@ -110,7 +110,7 @@ const MAX_FACES = 8;
 const VideoLobby: FC<{ room: Room }> = ({ room }) => {
     const [connecting, setConnecting] = useState(false);
     const me = useMemo(() => room.getMember(room.myUserId), [room]);
-    const connectedMembers = useConnectedMembers(room.currentState);
+    const connectedMembers = useConnectedMembers(room, false);
     const videoRef = useRef<HTMLVideoElement>();
 
     const devices = useAsyncMemo(async () => {
@@ -172,12 +172,12 @@ const VideoLobby: FC<{ room: Room }> = ({ room }) => {
     };
 
     let facePile;
-    if (connectedMembers.length) {
-        const shownMembers = connectedMembers.slice(0, MAX_FACES);
-        const overflow = connectedMembers.length > shownMembers.length;
+    if (connectedMembers.size) {
+        const shownMembers = [...connectedMembers].slice(0, MAX_FACES);
+        const overflow = connectedMembers.size > shownMembers.length;
 
         facePile = <div className="mx_VideoLobby_connectedMembers">
-            { _t("%(count)s people connected", { count: connectedMembers.length }) }
+            { _t("%(count)s people connected", { count: connectedMembers.size }) }
             <FacePile members={shownMembers} faceSize={24} overflow={overflow} />
         </div>;
     }
diff --git a/src/utils/VideoChannelUtils.ts b/src/utils/VideoChannelUtils.ts
@@ -17,7 +17,8 @@ limitations under the License.
 import { useState } from "react";
 import { throttle } from "lodash";
 import { CallType } from "matrix-js-sdk/src/webrtc/call";
-import { RoomState, RoomStateEvent } from "matrix-js-sdk/src/models/room-state";
+import { Room } from "matrix-js-sdk/src/models/room";
+import { RoomStateEvent } from "matrix-js-sdk/src/models/room-state";
 import { RoomMember } from "matrix-js-sdk/src/models/room-member";
 
 import { useTypedEventEmitter } from "../hooks/useEventEmitter";
@@ -42,17 +43,32 @@ export const addVideoChannel = async (roomId: string, roomName: string) => {
     await WidgetUtils.addJitsiWidget(roomId, CallType.Video, "Video channel", VIDEO_CHANNEL, roomName);
 };
 
-export const getConnectedMembers = (state: RoomState): RoomMember[] =>
-    state.getStateEvents(VIDEO_CHANNEL_MEMBER)
+export const getConnectedMembers = (room: Room, connectedLocalEcho: boolean): Set<RoomMember> => {
+    const members = new Set<RoomMember>();
+
+    for (const e of room.currentState.getStateEvents(VIDEO_CHANNEL_MEMBER)) {
+        const member = room.getMember(e.getStateKey());
+        let devices = e.getContent<IVideoChannelMemberContent>()?.devices ?? [];
+
+        // Apply local echo for the disconnected case
+        if (!connectedLocalEcho && member?.userId === room.client.getUserId()) {
+            devices = devices.filter(d => d !== room.client.getDeviceId());
+        }
         // Must have a device connected and still be joined to the room
-        .filter(e => e.getContent<IVideoChannelMemberContent>()?.devices?.length)
-        .map(e => state.getMember(e.getStateKey()))
-        .filter(member => member?.membership === "join");
-
-export const useConnectedMembers = (state: RoomState, throttleMs = 100) => {
-    const [members, setMembers] = useState<RoomMember[]>(getConnectedMembers(state));
-    useTypedEventEmitter(state, RoomStateEvent.Update, throttle(() => {
-        setMembers(getConnectedMembers(state));
+        if (devices.length && member?.membership === "join") members.add(member);
+    }
+
+    // Apply local echo for the connected case
+    if (connectedLocalEcho) members.add(room.getMember(room.client.getUserId()));
+    return members;
+};
+
+export const useConnectedMembers = (
+    room: Room, connectedLocalEcho: boolean, throttleMs = 100,
+): Set<RoomMember> => {
+    const [members, setMembers] = useState<Set<RoomMember>>(getConnectedMembers(room, connectedLocalEcho));
+    useTypedEventEmitter(room.currentState, RoomStateEvent.Update, throttle(() => {
+        setMembers(getConnectedMembers(room, connectedLocalEcho));
     }, throttleMs, { leading: true, trailing: true }));
     return members;
 };
diff --git a/test/components/views/rooms/RoomTile-test.tsx b/test/components/views/rooms/RoomTile-test.tsx
@@ -18,6 +18,8 @@ import React from "react";
 import { mount } from "enzyme";
 import { act } from "react-dom/test-utils";
 import { mocked } from "jest-mock";
+import { MatrixClient } from "matrix-js-sdk/src/client";
+import { Room } from "matrix-js-sdk/src/models/room";
 import { RoomMember } from "matrix-js-sdk/src/models/room-member";
 
 import {
@@ -26,6 +28,7 @@ import {
     mkRoom,
     mkVideoChannelMember,
     stubVideoChannelStore,
+    StubVideoChannelStore,
 } from "../../../test-utils";
 import RoomTile from "../../../../src/components/views/rooms/RoomTile";
 import SettingsStore from "../../../../src/settings/SettingsStore";
@@ -39,9 +42,8 @@ describe("RoomTile", () => {
     jest.spyOn(PlatformPeg, 'get')
         .mockReturnValue({ overrideBrowserShortcuts: () => false } as unknown as BasePlatform);
 
-    let cli;
-    let store;
-
+    let cli: MatrixClient;
+    let store: StubVideoChannelStore;
     beforeEach(() => {
         const realGetValue = SettingsStore.getValue;
         SettingsStore.getValue = <T, >(name: string, roomId?: string): T => {
@@ -52,16 +54,19 @@ describe("RoomTile", () => {
         };
 
         stubClient();
-        cli = mocked(MatrixClientPeg.get());
+        cli = MatrixClientPeg.get();
         store = stubVideoChannelStore();
         DMRoomMap.makeShared();
     });
 
     afterEach(() => jest.clearAllMocks());
 
     describe("video rooms", () => {
-        const room = mkRoom(cli, "!1:example.org");
-        room.isElementVideoRoom.mockReturnValue(true);
+        let room: Room;
+        beforeEach(() => {
+            room = mkRoom(cli, "!1:example.org");
+            mocked(room.isElementVideoRoom).mockReturnValue(true);
+        });
 
         it("tracks connection state", () => {
             const tile = mount(
@@ -97,7 +102,7 @@ describe("RoomTile", () => {
                 mkVideoChannelMember("@chris:example.org", ["device 1"]),
             ]));
 
-            mocked(room.currentState).getMember.mockImplementation(userId => ({
+            mocked(room).getMember.mockImplementation(userId => ({
                 userId,
                 membership: userId === "@chris:example.org" ? "leave" : "join",
                 name: userId,
@@ -117,8 +122,36 @@ describe("RoomTile", () => {
             );
 
             // Only Alice should display as connected
-            const participants = tile.find(".mx_RoomTile_videoParticipants");
-            expect(participants.text()).toEqual("1");
+            expect(tile.find(".mx_RoomTile_videoParticipants").text()).toEqual("1");
+        });
+
+        it("reflects local echo in connected members", () => {
+            mocked(room.currentState).getStateEvents.mockImplementation(mockStateEventImplementation([
+                // Make the remote echo claim that we're connected, while leaving the store disconnected
+                mkVideoChannelMember(cli.getUserId(), [cli.getDeviceId()]),
+            ]));
+
+            mocked(room).getMember.mockImplementation(userId => ({
+                userId,
+                membership: "join",
+                name: userId,
+                rawDisplayName: userId,
+                roomId: "!1:example.org",
+                getAvatarUrl: () => {},
+                getMxcAvatarUrl: () => {},
+            }) as unknown as RoomMember);
+
+            const tile = mount(
+                <RoomTile
+                    room={room}
+                    showMessagePreview={false}
+                    isMinimized={false}
+                    tag={DefaultTagID.Untagged}
+                />,
+            );
+
+            // Because of our local echo, we should still appear as disconnected
+            expect(tile.find(".mx_RoomTile_videoParticipants").exists()).toEqual(false);
         });
     });
 });
diff --git a/test/components/views/voip/VideoLobby-test.tsx b/test/components/views/voip/VideoLobby-test.tsx
@@ -18,11 +18,14 @@ import React from "react";
 import { mount } from "enzyme";
 import { act } from "react-dom/test-utils";
 import { mocked } from "jest-mock";
+import { MatrixClient } from "matrix-js-sdk/src/client";
+import { Room } from "matrix-js-sdk/src/models/room";
 import { RoomMember } from "matrix-js-sdk/src/models/room-member";
 
 import {
     stubClient,
     stubVideoChannelStore,
+    StubVideoChannelStore,
     mkRoom,
     mkVideoChannelMember,
     mockStateEventImplementation,
@@ -33,7 +36,6 @@ import MemberAvatar from "../../../../src/components/views/avatars/MemberAvatar"
 import VideoLobby from "../../../../src/components/views/voip/VideoLobby";
 
 describe("VideoLobby", () => {
-    stubClient();
     Object.defineProperty(navigator, "mediaDevices", {
         value: {
             enumerateDevices: jest.fn(),
@@ -42,19 +44,17 @@ describe("VideoLobby", () => {
     });
     jest.spyOn(HTMLMediaElement.prototype, "play").mockImplementation(async () => {});
 
-    const cli = MatrixClientPeg.get();
-    const room = mkRoom(cli, "!1:example.org");
-
-    let store;
+    let cli: MatrixClient;
+    let store: StubVideoChannelStore;
+    let room: Room;
     beforeEach(() => {
+        stubClient();
+        cli = MatrixClientPeg.get();
         store = stubVideoChannelStore();
+        room = mkRoom(cli, "!1:example.org");
         mocked(navigator.mediaDevices.enumerateDevices).mockResolvedValue([]);
     });
 
-    afterEach(() => {
-        jest.clearAllMocks();
-    });
-
     describe("connected members", () => {
         it("hides when no one is connected", async () => {
             const lobby = mount(<VideoLobby room={room} />);
@@ -75,7 +75,7 @@ describe("VideoLobby", () => {
                 mkVideoChannelMember("@chris:example.org", ["device 1"]),
             ]));
 
-            mocked(room.currentState).getMember.mockImplementation(userId => ({
+            mocked(room).getMember.mockImplementation(userId => ({
                 userId,
                 membership: userId === "@chris:example.org" ? "leave" : "join",
                 name: userId,
@@ -95,6 +95,31 @@ describe("VideoLobby", () => {
             expect(memberText).toEqual("1 person connected");
             expect(lobby.find(FacePile).find(MemberAvatar).props().member.userId).toEqual("@alice:example.org");
         });
+
+        it("doesn't include remote echo of this device being connected", async () => {
+            mocked(room.currentState).getStateEvents.mockImplementation(mockStateEventImplementation([
+                // Make the remote echo claim that we're connected, while leaving the store disconnected
+                mkVideoChannelMember(cli.getUserId(), [cli.getDeviceId()]),
+            ]));
+
+            mocked(room).getMember.mockImplementation(userId => ({
+                userId,
+                membership: "join",
+                name: userId,
+                rawDisplayName: userId,
+                roomId: "!1:example.org",
+                getAvatarUrl: () => {},
+                getMxcAvatarUrl: () => {},
+            }) as unknown as RoomMember);
+
+            const lobby = mount(<VideoLobby room={room} />);
+            // Wait for state to settle
+            await act(() => Promise.resolve());
+            lobby.update();
+
+            // Because of our local echo, we should still appear as disconnected
+            expect(lobby.find(".mx_VideoLobby_connectedMembers").exists()).toEqual(false);
+        });
     });
 
     describe("device buttons", () => {
diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts
@@ -379,6 +379,7 @@ export function mkStubRoom(roomId: string = null, name: string, client: MatrixCl
         getJoinRule: jest.fn().mockReturnValue("invite"),
         loadMembersIfNeeded: jest.fn(),
         client,
+        myUserId: client?.getUserId(),
         canInvite: jest.fn(),
     } as unknown as Room;
 }
diff --git a/test/test-utils/video.ts b/test/test-utils/video.ts
@@ -21,7 +21,7 @@ import { mkEvent } from "./test-utils";
 import { VIDEO_CHANNEL_MEMBER } from "../../src/utils/VideoChannelUtils";
 import VideoChannelStore, { VideoChannelEvent, IJitsiParticipant } from "../../src/stores/VideoChannelStore";
 
-class StubVideoChannelStore extends EventEmitter {
+export class StubVideoChannelStore extends EventEmitter {
     private _roomId: string;
     public get roomId(): string { return this._roomId; }
     private _connected: boolean;

Original file line number	Diff line number	Diff line change
`@@ -379,6 +379,7 @@ export function mkStubRoom(roomId: string = null, name: string, client: MatrixCl`
`379`	`379`	`getJoinRule: jest.fn().mockReturnValue("invite"),`
`380`	`380`	`loadMembersIfNeeded: jest.fn(),`
`381`	`381`	`client,`
	`382`	`+ myUserId: client?.getUserId(),`
`382`	`383`	`canInvite: jest.fn(),`
`383`	`384`	`} as unknown as Room;`
`384`	`385`	`}`