Use creation content to signal virtual-ness

This makes things a lot simpler.

src/CallHandler.tsx

@@ -93,7 +93,8 @@ export const PROTOCOL_SIP_NATIVE = 'im.vector.protocol.sip_native';
 export const PROTOCOL_SIP_VIRTUAL = 'im.vector.protocol.sip_virtual';
 
 const CHECK_PROTOCOLS_ATTEMPTS = 3;
-// Event type for room account data used to mark rooms as virtual rooms (and store the ID of their native room)
+// Event type for room account data and room creation content used to mark rooms as virtual rooms
+// (and store the ID of their native room)
 export const VIRTUAL_ROOM_EVENT_TYPE = 'im.vector.is_virtual_room';
 
 enum AudioID {

src/VoipUserMapper.ts

@@ -14,19 +14,16 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import { ensureDMExists, findDMForUser } from './createRoom';
+import { ensureVirtualRoomExists, findDMForUser } from './createRoom';
 import { MatrixClientPeg } from "./MatrixClientPeg";
 import DMRoomMap from "./utils/DMRoomMap";
 import CallHandler, { VIRTUAL_ROOM_EVENT_TYPE } from './CallHandler';
-import RoomListStore from './stores/room-list/RoomListStore';
 import { Room } from 'matrix-js-sdk/src/models/room';
 
 // Functions for mapping virtual users & rooms. Currently the only lookup
 // is sip virtual: there could be others in the future.
 
 export default class VoipUserMapper {
-    private virtualRoomIdCache = new Set<string>();
-
     public static sharedInstance(): VoipUserMapper {
         if (window.mxVoipUserMapper === undefined) window.mxVoipUserMapper = new VoipUserMapper();
         return window.mxVoipUserMapper;
@@ -45,25 +42,12 @@ export default class VoipUserMapper {
         const virtualUser = await this.userToVirtualUser(userId);
         if (!virtualUser) return null;
 
-        // There's quite a bit of acrobatics here to prevent the virtual room being shown
+        const virtualRoomId = await ensureVirtualRoomExists(MatrixClientPeg.get(), virtualUser, roomId);
-        // while it's being created: firstly, we have to stop the RoomListStore from showing
+        MatrixClientPeg.get().setRoomAccountData(virtualRoomId, VIRTUAL_ROOM_EVENT_TYPE, {
-        // new rooms for a bit, because we can't set the room account data to say it's a virtual
+            native_room: roomId,
-        // room until we have the room ID. Secondly, once we have the new room ID, we have to
+        });
-        // temporarily cache the fact it's a virtual room because there's no local echo on
-        // room account data so it won't show up in the room model until it comes down the
-        // sync stream again. Ick.
-        RoomListStore.instance.startHoldingNewRooms();
-        try {
-            const virtualRoomId = await ensureDMExists(MatrixClientPeg.get(), virtualUser);
-            MatrixClientPeg.get().setRoomAccountData(virtualRoomId, VIRTUAL_ROOM_EVENT_TYPE, {
-                native_room: roomId,
-            });
-            this.virtualRoomIdCache.add(virtualRoomId);
 
-            return virtualRoomId;
+        return virtualRoomId;
-        } finally {
-            RoomListStore.instance.stopHoldingNewRooms();
-        }
     }
 
     public nativeRoomForVirtualRoom(roomId: string):string {
@@ -74,10 +58,20 @@ export default class VoipUserMapper {
         return virtualRoomEvent.getContent()['native_room'] || null;
     }
 
-    public isVirtualRoom(roomId: string):boolean {
+    public isVirtualRoom(room: Room):boolean {
-        if (this.nativeRoomForVirtualRoom(roomId)) return true;
+        if (this.nativeRoomForVirtualRoom(room.roomId)) return true;
 
-        return this.virtualRoomIdCache.has(roomId);
+        // also look in the create event for the claimed native room ID, which is the only
+        // way we can recognise a virtual room we've created when it first arrives down
+        // our stream. We don't trust this in general though, as it could be faked by an
+        // inviter: our main source of truth is the DM state.
+        const roomCreateEvent = room.currentState.getStateEvents("m.room.create", "");
+        if (!roomCreateEvent || !roomCreateEvent.getContent()) return false;
+        // we only look at this for rooms we created (so inviters can't just cause rooms
+        // to be invisible)
+        if (roomCreateEvent.getSender() !== MatrixClientPeg.get().getUserId()) return false;
+        const claimedNativeRoomId = roomCreateEvent.getContent()[VIRTUAL_ROOM_EVENT_TYPE];
+        return Boolean(claimedNativeRoomId);
     }
 
     public async onNewInvitedRoom(invitedRoom: Room) {

src/createRoom.ts

@@ -30,6 +30,7 @@ import { getE2EEWellKnown } from "./utils/WellKnownUtils";
 import GroupStore from "./stores/GroupStore";
 import CountlyAnalytics from "./CountlyAnalytics";
 import { isJoinedOrNearlyJoined } from "./utils/membership";
+import { VIRTUAL_ROOM_EVENT_TYPE } from "./CallHandler";
 
 // we define a number of interfaces which take their names from the js-sdk
 /* eslint-disable camelcase */
@@ -300,6 +301,34 @@ export async function canEncryptToAllUsers(client: MatrixClient, userIds: string
     }
 }
 
+// Similar to ensureDMExists but also adds creation content
+// without polluting ensureDMExists with unrelated stuff (also
+// they're never encrypted).
+export async function ensureVirtualRoomExists(
+    client: MatrixClient, userId: string, nativeRoomId: string,
+): Promise<string> {
+    const existingDMRoom = findDMForUser(client, userId);
+    let roomId;
+    if (existingDMRoom) {
+        roomId = existingDMRoom.roomId;
+    } else {
+        roomId = await createRoom({
+            dmUserId: userId,
+            spinner: false,
+            andView: false,
+            createOpts: {
+                creation_content: {
+                    // This allows us to recognise that the room is a virtual room
+                    // when it comes down our sync stream (we also put the ID of the
+                    // respective native room in there because why not?)
+                    [VIRTUAL_ROOM_EVENT_TYPE]: nativeRoomId,
+                },
+            },
+        });
+    }
+    return roomId;
+}
+
 export async function ensureDMExists(client: MatrixClient, userId: string): Promise<string> {
     const existingDMRoom = findDMForUser(client, userId);
     let roomId;
@@ -310,6 +339,7 @@ export async function ensureDMExists(client: MatrixClient, userId: string): Prom
         if (privateShouldBeEncrypted()) {
             encryption = await canEncryptToAllUsers(client, [userId]);
         }
+
         roomId = await createRoom({encryption, dmUserId: userId, spinner: false, andView: false});
         await _waitForMember(client, roomId, userId);
     }

src/stores/room-list/RoomListStore.ts

@@ -63,9 +63,6 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
         }
         this.emit(LISTS_UPDATE_EVENT);
     });
-    // When new rooms arrive, we may hold them here until we have enough info to know whether we should before display them.
-    private roomHoldingPen: Room[] = [];
-    private holdNewRooms = false;
 
     private readonly watchedSettings = [
         'feature_custom_tags',
@@ -129,24 +126,6 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
         this.updateFn.trigger();
     }
 
-    // After calling this, any new rooms that appear are not displayed until stopHoldingNewRooms()
-    // is called. Be sure to always call this in a try/finally block to ensure stopHoldingNewRooms
-    // is called afterwards.
-    public startHoldingNewRooms() {
-        console.log("hold-new-rooms mode enabled.");
-        this.holdNewRooms = true;
-    }
-
-    public stopHoldingNewRooms() {
-        console.log("hold-new-rooms mode disabled: processing " + this.roomHoldingPen.length + " held rooms");
-        this.holdNewRooms = false;
-        for (const heldRoom of this.roomHoldingPen) {
-            console.log("Processing held room: " + heldRoom.roomId);
-            this.handleRoomUpdate(heldRoom, RoomUpdateCause.NewRoom);
-        }
-        this.roomHoldingPen = [];
-    }
-
     private checkLoggingEnabled() {
         if (SettingsStore.getValue("advancedRoomListLogging")) {
             console.warn("Advanced room list logging is enabled");
@@ -420,18 +399,12 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
 
     private async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise<any> {
         if (cause === RoomUpdateCause.NewRoom) {
-            if (this.holdNewRooms) {
+            // Let the visibility provider know that there is a new invited room. It would be nice
-                console.log("Room updates are held: putting room " + room.roomId + " into the holding pen");
+            // if this could just be an event that things listen for but the point of this is that
-                this.roomHoldingPen.push(room);
+            // we delay doing anything about this room until the VoipUserMapper had had a chance
-                return;
+            // to do the things it needs to do to decide if we should show this room or not, so
-            } else {
+            // an even wouldn't et us do that.
-                // Let the visibility provider know that there is a new invited room. It would be nice
+            await VisibilityProvider.instance.onNewInvitedRoom(room);
-                // if this could just be an event that things listen for but the point of this is that
-                // we delay doing anything about this room until the VoipUserMapper had had a chance
-                // to do the things it needs to do to decide if we should show this room or not, so
-                // an even wouldn't et us do that.
-                await VisibilityProvider.instance.onNewInvitedRoom(room);
-            }
         }
 
         if (!VisibilityProvider.instance.isRoomVisible(room)) {

src/stores/room-list/filters/VisibilityProvider.ts

@@ -42,7 +42,7 @@ export class VisibilityProvider {
 
         if (
             CallHandler.sharedInstance().getSupportsVirtualRooms() &&
-            VoipUserMapper.sharedInstance().isVirtualRoom(room.roomId)
+            VoipUserMapper.sharedInstance().isVirtualRoom(room)
         ) {
             isVisible = false;
             forced = true;