Update src/room/InCallView.tsx

Co-authored-by: Robin <robin@robin.town>

src/e2ee/sharedKeyManagement.ts

@@ -6,58 +6,70 @@ Please see LICENSE in the repository root for full details.
 */
 
 import { useEffect, useMemo } from "react";
+import { logger } from "matrix-js-sdk/lib/logger";
 
-import { setLocalStorageItem, useLocalStorage } from "../useLocalStorage";
-import { type UrlParams, getUrlParams, useUrlParams } from "../UrlParams";
+import {
+  setLocalStorageItemReactive,
+  useLocalStorage,
+} from "../useLocalStorage";
+import { getUrlParams } from "../UrlParams";
 import { E2eeType } from "./e2eeType";
 import { useClient } from "../ClientContext";
 
+/**
+ * This setter will update the state for all `useRoomSharedKey` hooks
+ * if the password is different from the one in local storage or if its not yet in the local storage.
+ */
 export function saveKeyForRoom(roomId: string, password: string): void {
-  setLocalStorageItem(getRoomSharedKeyLocalStorageKey(roomId), password);
+  if (
+    localStorage.getItem(getRoomSharedKeyLocalStorageKey(roomId)) !== password
+  )
+    setLocalStorageItemReactive(
+      getRoomSharedKeyLocalStorageKey(roomId),
+      password,
+    );
 }
 
 const getRoomSharedKeyLocalStorageKey = (roomId: string): string =>
   `room-shared-key-${roomId}`;
 
-const useInternalRoomSharedKey = (roomId: string): string | null => {
-  const key = getRoomSharedKeyLocalStorageKey(roomId);
-  const [roomSharedKey] = useLocalStorage(key);
+/**
+ * An upto-date shared key for the room. Either from local storage or the value from `setInitialValue`.
+ * @param roomId
+ * @param setInitialValue The value we get from the URL. The hook will overwrite the local storage value with this.
+ * @returns [roomSharedKey, setRoomSharedKey] like a react useState hook.
+ */
+const useRoomSharedKey = (
+  roomId: string,
+  setInitialValue?: string,
+): [string | null, setKey: (key: string) => void] => {
+  const [roomSharedKey, setRoomSharedKey] = useLocalStorage(
+    getRoomSharedKeyLocalStorageKey(roomId),
+  );
+  useEffect(() => {
+    // If setInitialValue is available, update the local storage (usually the password from the url).
+    // This will update roomSharedKey but wont update the returned value since
+    // that already defaults to setInitialValue.
+    if (setInitialValue) setRoomSharedKey(setInitialValue);
+  }, [setInitialValue, setRoomSharedKey]);
 
-  return roomSharedKey;
+  // make sure we never return the initial null value from `useLocalStorage`
+  return [setInitialValue ?? roomSharedKey, setRoomSharedKey];
 };
 
 export function getKeyForRoom(roomId: string): string | null {
-  saveKeyFromUrlParams(getUrlParams());
-  const key = getRoomSharedKeyLocalStorageKey(roomId);
-  return localStorage.getItem(key);
+  const { roomId: urlRoomId, password } = getUrlParams();
+  if (roomId !== urlRoomId)
+    logger.warn(
+      "requested key for a roomId which is not the current call room id (from the URL)",
+      roomId,
+      urlRoomId,
+    );
+  return (
+    password ?? localStorage.getItem(getRoomSharedKeyLocalStorageKey(roomId))
+  );
 }
 
-function saveKeyFromUrlParams(urlParams: UrlParams): void {
-  if (!urlParams.password || !urlParams.roomId) return;
-
-  // Take the key from the URL and save it.
-  // It's important to always use the room ID specified in the URL
-  // when saving keys rather than whatever the current room ID might be,
-  // in case we've moved to a different room but the URL hasn't changed.
-  saveKeyForRoom(urlParams.roomId, urlParams.password);
-}
-
-/**
- * Extracts the room password from the URL if one is present, saving it in localstorage
- * and returning it in a tuple with the corresponding room ID from the URL.
- * @returns A tuple of the roomId and password from the URL if the URL has both,
- *          otherwise [undefined, undefined]
- */
-const useKeyFromUrl = (): [string, string] | [undefined, undefined] => {
-  const urlParams = useUrlParams();
-
-  useEffect(() => saveKeyFromUrlParams(urlParams), [urlParams]);
-
-  return urlParams.roomId && urlParams.password
-    ? [urlParams.roomId, urlParams.password]
-    : [undefined, undefined];
-};
-
 export type Unencrypted = { kind: E2eeType.NONE };
 export type SharedSecret = { kind: E2eeType.SHARED_KEY; secret: string };
 export type PerParticipantE2EE = { kind: E2eeType.PER_PARTICIPANT };
@@ -66,12 +78,11 @@ export type EncryptionSystem = Unencrypted | SharedSecret | PerParticipantE2EE;
 export function useRoomEncryptionSystem(roomId: string): EncryptionSystem {
   const { client } = useClient();
 
-  // make sure we've extracted the key from the URL first
-  // (and we still need to take the value it returns because
-  // the effect won't run in time for it to save to localstorage in
-  // time for us to read it out again).
-  const [urlRoomId, passwordFromUrl] = useKeyFromUrl();
-  const storedPassword = useInternalRoomSharedKey(roomId);
+  const [storedPassword] = useRoomSharedKey(
+    getRoomSharedKeyLocalStorageKey(roomId),
+    getKeyForRoom(roomId) ?? undefined,
+  );
+
   const room = client?.getRoom(roomId);
   const e2eeSystem = <EncryptionSystem>useMemo(() => {
     if (!room) return { kind: E2eeType.NONE };
@@ -80,15 +91,10 @@ export function useRoomEncryptionSystem(roomId: string): EncryptionSystem {
         kind: E2eeType.SHARED_KEY,
         secret: storedPassword,
       };
-    if (urlRoomId === roomId)
-      return {
-        kind: E2eeType.SHARED_KEY,
-        secret: passwordFromUrl,
-      };
     if (room.hasEncryptionStateEvent()) {
       return { kind: E2eeType.PER_PARTICIPANT };
     }
     return { kind: E2eeType.NONE };
-  }, [passwordFromUrl, room, roomId, storedPassword, urlRoomId]);
+  }, [room, storedPassword]);
   return e2eeSystem;
 }

src/home/useGroupCallRooms.ts

@@ -81,8 +81,9 @@ function sortRooms(client: MatrixClient, rooms: Room[]): Room[] {
 }
 
 const roomIsJoinable = (room: Room): boolean => {
-  if (!room.hasEncryptionStateEvent() && !getKeyForRoom(room.roomId)) {
-    // if we have an non encrypted room (no encryption state event) we need a locally stored shared key.
+  const password = getKeyForRoom(room.roomId);
+  if (!room.hasEncryptionStateEvent() && !password) {
+    // if we have a non encrypted room (no encryption state event) we need a locally stored shared key.
     // in case this key also does not exists we cannot join the room.
     return false;
   }

src/livekit/useLivekit.ts

@@ -14,7 +14,7 @@ import {
   type RoomOptions,
   Track,
 } from "livekit-client";
-import { useEffect, useMemo, useRef } from "react";
+import { useEffect, useRef } from "react";
 import E2EEWorker from "livekit-client/e2ee-worker?worker";
 import { logger } from "matrix-js-sdk/lib/logger";
 import { type MatrixRTCSession } from "matrix-js-sdk/lib/matrixrtc";
@@ -24,11 +24,7 @@ import { map } from "rxjs";
 import { defaultLiveKitOptions } from "./options";
 import { type SFUConfig } from "./openIDSFU";
 import { type MuteStates } from "../room/MuteStates";
-import {
-  type MediaDeviceHandle,
-  type MediaDevices,
-  useMediaDevices,
-} from "./MediaDevicesContext";
+import { type MediaDeviceHandle, useMediaDevices } from "./MediaDevicesContext";
 import {
   type ECConnectionState,
   useECConnectionState,
@@ -40,9 +36,9 @@ import {
   useTrackProcessor,
   useTrackProcessorSync,
 } from "./TrackProcessorContext";
-import { useInitial } from "../useInitial";
 import { observeTrackReference$ } from "../state/MediaViewModel";
 import { useUrlParams } from "../UrlParams";
+import { useInitial } from "../useInitial";
 
 interface UseLivekitResult {
   livekitRoom?: Room;
@@ -57,27 +53,78 @@ export function useLivekit(
 ): UseLivekitResult {
   const { controlledAudioDevices } = useUrlParams();
 
-  const e2eeOptions = useMemo((): E2EEManagerOptions | undefined => {
-    if (e2eeSystem.kind === E2eeType.NONE) return undefined;
+  const initialMuteStates = useInitial(() => muteStates);
 
+  const devices = useMediaDevices();
+  const initialDevices = useInitial(() => devices);
+
+  // Store if audio/video are currently updating. If to prohibit unnecessary calls
+  // to setMicrophoneEnabled/setCameraEnabled
+  const audioMuteUpdating = useRef(false);
+  const videoMuteUpdating = useRef(false);
+  // Store the current button mute state that gets passed to this hook via props.
+  // We need to store it for awaited code that relies on the current value.
+  const buttonEnabled = useRef({
+    audio: initialMuteStates.audio.enabled,
+    video: initialMuteStates.video.enabled,
+  });
+
+  const { processor } = useTrackProcessor();
+
+  // Only ever create the room once via useInitial.
+  const room = useInitial(() => {
+    logger.info("[LivekitRoom] Create LiveKit room");
+
+    let e2ee: E2EEManagerOptions | undefined;
     if (e2eeSystem.kind === E2eeType.PER_PARTICIPANT) {
       logger.info("Created MatrixKeyProvider (per participant)");
-      return {
+      e2ee = {
         keyProvider: new MatrixKeyProvider(),
         worker: new E2EEWorker(),
       };
     } else if (e2eeSystem.kind === E2eeType.SHARED_KEY && e2eeSystem.secret) {
       logger.info("Created ExternalE2EEKeyProvider (shared key)");
-
-      return {
+      e2ee = {
         keyProvider: new ExternalE2EEKeyProvider(),
         worker: new E2EEWorker(),
       };
     }
-  }, [e2eeSystem]);
 
+    const roomOptions: RoomOptions = {
+      ...defaultLiveKitOptions,
+      videoCaptureDefaults: {
+        ...defaultLiveKitOptions.videoCaptureDefaults,
+        deviceId: initialDevices.videoInput.selectedId,
+        processor,
+      },
+      audioCaptureDefaults: {
+        ...defaultLiveKitOptions.audioCaptureDefaults,
+        deviceId: initialDevices.audioInput.selectedId,
+      },
+      audioOutput: {
+        deviceId: initialDevices.audioOutput.selectedId,
+      },
+      e2ee,
+    };
+    // We have to create the room manually here due to a bug inside
+    // @livekit/components-react. JSON.stringify() is used in deps of a
+    // useEffect() with an argument that references itself, if E2EE is enabled
+    const room = new Room(roomOptions);
+    room.setE2EEEnabled(e2eeSystem.kind !== E2eeType.NONE).catch((e) => {
+      logger.error("Failed to set E2EE enabled on room", e);
+    });
+
+    return room;
+  });
+
+  // Setup and update the keyProvider which was create by `createRoom`
   useEffect(() => {
-    if (e2eeSystem.kind === E2eeType.NONE || !e2eeOptions) return;
+    const e2eeOptions = room.options.e2ee;
+    if (
+      e2eeSystem.kind === E2eeType.NONE ||
+      !(e2eeOptions && "keyProvider" in e2eeOptions)
+    )
+      return;
 
     if (e2eeSystem.kind === E2eeType.PER_PARTICIPANT) {
       (e2eeOptions.keyProvider as MatrixKeyProvider).setRTCSession(rtcSession);
@@ -88,56 +135,7 @@ export function useLivekit(
           logger.error("Failed to set shared key for E2EE", e);
         });
     }
-  }, [e2eeOptions, e2eeSystem, rtcSession]);
-
-  const initialMuteStates = useRef<MuteStates>(muteStates);
-  const devices = useMediaDevices();
-  const initialDevices = useRef<MediaDevices>(devices);
-
-  const { processor } = useTrackProcessor();
-  const initialProcessor = useInitial(() => processor);
-  const roomOptions = useMemo(
-    (): RoomOptions => ({
-      ...defaultLiveKitOptions,
-      videoCaptureDefaults: {
-        ...defaultLiveKitOptions.videoCaptureDefaults,
-        deviceId: initialDevices.current.videoInput.selectedId,
-        processor: initialProcessor,
-      },
-      audioCaptureDefaults: {
-        ...defaultLiveKitOptions.audioCaptureDefaults,
-        deviceId: initialDevices.current.audioInput.selectedId,
-      },
-      audioOutput: {
-        deviceId: initialDevices.current.audioOutput.selectedId,
-      },
-      e2ee: e2eeOptions,
-    }),
-    [e2eeOptions, initialProcessor],
-  );
-
-  // Store if audio/video are currently updating. If to prohibit unnecessary calls
-  // to setMicrophoneEnabled/setCameraEnabled
-  const audioMuteUpdating = useRef(false);
-  const videoMuteUpdating = useRef(false);
-  // Store the current button mute state that gets passed to this hook via props.
-  // We need to store it for awaited code that relies on the current value.
-  const buttonEnabled = useRef({
-    audio: initialMuteStates.current.audio.enabled,
-    video: initialMuteStates.current.video.enabled,
-  });
-
-  // We have to create the room manually here due to a bug inside
-  // @livekit/components-react. JSON.stringify() is used in deps of a
-  // useEffect() with an argument that references itself, if E2EE is enabled
-  const room = useMemo(() => {
-    logger.info("[LivekitRooms] Create LiveKit room with options", roomOptions);
-    const r = new Room(roomOptions);
-    r.setE2EEEnabled(e2eeSystem.kind !== E2eeType.NONE).catch((e) => {
-      logger.error("Failed to set E2EE enabled on room", e);
-    });
-    return r;
-  }, [roomOptions, e2eeSystem]);
+  }, [room.options.e2ee, e2eeSystem, rtcSession]);
 
   // Sync the requested track processors with LiveKit
   useTrackProcessorSync(
@@ -159,8 +157,8 @@ export function useLivekit(
   );
 
   const connectionState = useECConnectionState(
-    initialDevices.current.audioInput.selectedId,
-    initialMuteStates.current.audio.enabled,
+    initialDevices.audioInput.selectedId,
+    initialMuteStates.audio.enabled,
     room,
     sfuConfig,
   );

src/room/GroupCallView.tsx

@@ -43,7 +43,10 @@ import { MUTE_PARTICIPANT_COUNT, type MuteStates } from "./MuteStates";
 import { useMediaDevices } from "../livekit/MediaDevicesContext";
 import { useMatrixRTCSessionMemberships } from "../useMatrixRTCSessionMemberships";
 import { enterRTCSession, leaveRTCSession } from "../rtcSessionHelpers";
-import { useRoomEncryptionSystem } from "../e2ee/sharedKeyManagement";
+import {
+  saveKeyForRoom,
+  useRoomEncryptionSystem,
+} from "../e2ee/sharedKeyManagement";
 import { useRoomAvatar } from "./useRoomAvatar";
 import { useRoomName } from "./useRoomName";
 import { useJoinRule } from "./useJoinRule";
@@ -167,6 +170,12 @@ export const GroupCallView: FC<Props> = ({
     useExperimentalToDeviceTransportSetting,
   );
 
+  // Save the password once we start the groupCallView
+  const { password: passwordFromUrl } = useUrlParams();
+  useEffect(() => {
+    if (passwordFromUrl) saveKeyForRoom(room.roomId, passwordFromUrl);
+  }, [passwordFromUrl, room.roomId]);
+
   usePageTitle(roomName);
 
   const matrixInfo = useMemo((): MatrixInfo => {

src/room/InCallView.tsx

@@ -147,8 +147,7 @@ export const ActiveCall: FC<ActiveCallProps> = (props) => {
           logger.error("[Lifecycle] Failed to disconnect from livekit room", e);
         });
     };
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, []);
+  }, [livekitRoom]);
 
   useEffect(() => {
     if (livekitRoom !== undefined) {

src/useLocalStorage.ts

@@ -13,7 +13,11 @@ type LocalStorageItem = ReturnType<typeof localStorage.getItem>;
 // Bus to notify other useLocalStorage consumers when an item is changed
 export const localStorageBus = new EventEmitter();
 
-// Like useState, but reads from and persists the value to localStorage
+/**
+ * Like useState, but reads from and persists the value to localStorage
+ * This hook will not update when we write to localStorage.setItem(key, value) directly.
+ * For the hook to react either use the returned setter or `setLocalStorageItemReactive`.
+ */
 export const useLocalStorage = (
   key: string,
 ): [LocalStorageItem, (value: string) => void] => {
@@ -41,15 +45,10 @@ export const useLocalStorage = (
   ];
 };
 
-export const setLocalStorageItem = (key: string, value: string): void => {
-  // Avoid unnecessary updates. Not avoiding them so can cause unexpected state updates across hooks.
-  // For instance:
-  // - In call view uses useRoomEncryptionSystem
-  // - This will set the key again.
-  // - All other instances of useRoomEncryptionSystem will now do a useMemo update of the e2eeSystem
-  //   - because the dependency `storedPassword = useInternalRoomSharedKey(roomId);` would change.
-  if (localStorage.getItem(key) === value) return;
-
+export const setLocalStorageItemReactive = (
+  key: string,
+  value: string,
+): void => {
   localStorage.setItem(key, value);
   localStorageBus.emit(key, value);
 };