Don't assume that widget IDs are unique (#8052)

* Don't assume that widget IDs are unique

Signed-off-by: Robin Townsend <robin@robin.town>

* Don't remove live tiles that don't exist

Signed-off-by: Robin Townsend <robin@robin.town>

* Add unit test for AppTile's live tile tracking

Signed-off-by: Robin Townsend <robin@robin.town>

src/CallHandler.tsx

@@ -1123,7 +1123,7 @@ export default class CallHandler extends EventEmitter {
 
         const jitsiWidgets = roomInfo.widgets.filter(w => WidgetType.JITSI.matches(w.type));
         jitsiWidgets.forEach(w => {
-            const messaging = WidgetMessagingStore.instance.getMessagingForId(w.id);
+            const messaging = WidgetMessagingStore.instance.getMessagingForUid(WidgetUtils.getWidgetUid(w));
             if (!messaging) return; // more "should never happen" words
 
             messaging.transport.send(ElementWidgetActions.HangupCall, {});

src/components/views/context_menus/WidgetContextMenu.tsx

@@ -57,7 +57,7 @@ const WidgetContextMenu: React.FC<IProps> = ({
     const cli = useContext(MatrixClientContext);
     const { room, roomId } = useContext(RoomContext);
 
-    const widgetMessaging = WidgetMessagingStore.instance.getMessagingForId(app.id);
+    const widgetMessaging = WidgetMessagingStore.instance.getMessagingForUid(WidgetUtils.getWidgetUid(app));
     const canModify = userWidget || WidgetUtils.canUserModifyWidgets(roomId);
 
     let streamAudioStreamButton;

src/components/views/elements/AppTile.tsx

@@ -118,24 +118,27 @@ export default class AppTile extends React.Component<IProps, IState> {
         showLayoutButtons: true,
     };
 
-    // We track a count of all "live" `AppTile`s for a given widget ID.
+    // We track a count of all "live" `AppTile`s for a given widget UID.
     // For this purpose, an `AppTile` is considered live from the time it is
     // constructed until it is unmounted. This is used to aid logic around when
     // to tear down the widget iframe. See `componentWillUnmount` for details.
-    private static liveTilesById: { [key: string]: number } = {};
+    private static liveTilesByUid = new Map<string, number>();
 
-    public static addLiveTile(widgetId: string): void {
+    public static addLiveTile(widgetId: string, roomId: string): void {
-        const refs = this.liveTilesById[widgetId] || 0;
+        const uid = WidgetUtils.calcWidgetUid(widgetId, roomId);
-        this.liveTilesById[widgetId] = refs + 1;
+        const refs = this.liveTilesByUid.get(uid) ?? 0;
+        this.liveTilesByUid.set(uid, refs + 1);
     }
 
-    public static removeLiveTile(widgetId: string): void {
+    public static removeLiveTile(widgetId: string, roomId: string): void {
-        const refs = this.liveTilesById[widgetId] || 0;
+        const uid = WidgetUtils.calcWidgetUid(widgetId, roomId);
-        this.liveTilesById[widgetId] = refs - 1;
+        const refs = this.liveTilesByUid.get(uid);
+        if (refs) this.liveTilesByUid.set(uid, refs - 1);
     }
 
-    public static isLive(widgetId: string): boolean {
+    public static isLive(widgetId: string, roomId: string): boolean {
-        const refs = this.liveTilesById[widgetId] || 0;
+        const uid = WidgetUtils.calcWidgetUid(widgetId, roomId);
+        const refs = this.liveTilesByUid.get(uid) ?? 0;
         return refs > 0;
     }
 
@@ -150,10 +153,10 @@ export default class AppTile extends React.Component<IProps, IState> {
     constructor(props: IProps) {
         super(props);
 
-        AppTile.addLiveTile(this.props.app.id);
+        AppTile.addLiveTile(this.props.app.id, this.props.app.roomId);
 
         // The key used for PersistedElement
-        this.persistKey = getPersistKey(this.props.app.id);
+        this.persistKey = getPersistKey(WidgetUtils.getWidgetUid(this.props.app));
         try {
             this.sgWidget = new StopGapWidget(this.props);
             this.setupSgListeners();
@@ -189,7 +192,9 @@ export default class AppTile extends React.Component<IProps, IState> {
     };
 
     private onUserLeftRoom() {
-        const isActiveWidget = ActiveWidgetStore.instance.getWidgetPersistence(this.props.app.id);
+        const isActiveWidget = ActiveWidgetStore.instance.getWidgetPersistence(
+            this.props.app.id, this.props.app.roomId,
+        );
         if (isActiveWidget) {
             // We just left the room that the active widget was from.
             if (this.props.room && RoomViewStore.getRoomId() !== this.props.room.roomId) {
@@ -200,7 +205,7 @@ export default class AppTile extends React.Component<IProps, IState> {
                 this.reload();
             } else {
                 // Otherwise just cancel its persistence.
-                ActiveWidgetStore.instance.destroyPersistentWidget(this.props.app.id);
+                ActiveWidgetStore.instance.destroyPersistentWidget(this.props.app.id, this.props.app.roomId);
             }
         }
     }
@@ -241,7 +246,7 @@ export default class AppTile extends React.Component<IProps, IState> {
 
         if (this.state.hasPermissionToLoad && !hasPermissionToLoad) {
             // Force the widget to be non-persistent (able to be deleted/forgotten)
-            ActiveWidgetStore.instance.destroyPersistentWidget(this.props.app.id);
+            ActiveWidgetStore.instance.destroyPersistentWidget(this.props.app.id, this.props.app.roomId);
             PersistedElement.destroyElement(this.persistKey);
             this.sgWidget?.stopMessaging();
         }
@@ -291,14 +296,16 @@ export default class AppTile extends React.Component<IProps, IState> {
         // container is constructed before the old one unmounts. By counting the
         // mounted `AppTile`s for each widget, we know to only tear down the
         // widget iframe when the last the `AppTile` unmounts.
-        AppTile.removeLiveTile(this.props.app.id);
+        AppTile.removeLiveTile(this.props.app.id, this.props.app.roomId);
 
         // We also support a separate "persistence" mode where a single widget
         // can request to be "sticky" and follow you across rooms in a PIP
         // container.
-        const isActiveWidget = ActiveWidgetStore.instance.getWidgetPersistence(this.props.app.id);
+        const isActiveWidget = ActiveWidgetStore.instance.getWidgetPersistence(
+            this.props.app.id, this.props.app.roomId,
+        );
 
-        if (!AppTile.isLive(this.props.app.id) && !isActiveWidget) {
+        if (!AppTile.isLive(this.props.app.id, this.props.app.roomId) && !isActiveWidget) {
             this.endWidgetActions();
         }
 
@@ -408,7 +415,7 @@ export default class AppTile extends React.Component<IProps, IState> {
 
         // Delete the widget from the persisted store for good measure.
         PersistedElement.destroyElement(this.persistKey);
-        ActiveWidgetStore.instance.destroyPersistentWidget(this.props.app.id);
+        ActiveWidgetStore.instance.destroyPersistentWidget(this.props.app.id, this.props.app.roomId);
 
         this.sgWidget?.stopMessaging({ forceDestroy: true });
     }

src/components/views/elements/PersistentApp.tsx

@@ -16,8 +16,8 @@ limitations under the License.
 */
 
 import React, { ContextType } from 'react';
+import { Room } from "matrix-js-sdk/src/models/room";
 
-import ActiveWidgetStore from '../../../stores/ActiveWidgetStore';
 import WidgetUtils from '../../../utils/WidgetUtils';
 import { replaceableComponent } from "../../../utils/replaceableComponent";
 import AppTile from "./AppTile";
@@ -26,6 +26,7 @@ import MatrixClientContext from "../../../contexts/MatrixClientContext";
 
 interface IProps {
     persistentWidgetId: string;
+    persistentRoomId: string;
     pointerEvents?: string;
 }
 
@@ -33,32 +34,32 @@ interface IProps {
 export default class PersistentApp extends React.Component<IProps> {
     public static contextType = MatrixClientContext;
     context: ContextType<typeof MatrixClientContext>;
+    private room: Room;
+
+    constructor(props: IProps, context: ContextType<typeof MatrixClientContext>) {
+        super(props, context);
+        this.room = context.getRoom(this.props.persistentRoomId);
+    }
 
     private get app(): IApp {
-        const persistentWidgetInRoomId = ActiveWidgetStore.instance.getRoomId(this.props.persistentWidgetId);
-        const persistentWidgetInRoom = this.context.getRoom(persistentWidgetInRoomId);
-
         // get the widget data
-        const appEvent = WidgetUtils.getRoomWidgets(persistentWidgetInRoom).find((ev) => {
+        const appEvent = WidgetUtils.getRoomWidgets(this.room).find(ev =>
-            return ev.getStateKey() === ActiveWidgetStore.instance.getPersistentWidgetId();
+            ev.getStateKey() === this.props.persistentWidgetId,
-        });
+        );
         return WidgetUtils.makeAppConfig(
             appEvent.getStateKey(), appEvent.getContent(), appEvent.getSender(),
-            persistentWidgetInRoomId, appEvent.getId(),
+            this.room.roomId, appEvent.getId(),
         );
     }
 
     public render(): JSX.Element {
         const app = this.app;
         if (app) {
-            const persistentWidgetInRoomId = ActiveWidgetStore.instance.getRoomId(this.props.persistentWidgetId);
-            const persistentWidgetInRoom = this.context.getRoom(persistentWidgetInRoomId);
-
             return <AppTile
                 key={app.id}
                 app={app}
                 fullWidth={true}
-                room={persistentWidgetInRoom}
+                room={this.room}
                 userId={this.context.credentials.userId}
                 creatorUserId={app.creatorUserId}
                 widgetPageTitle={WidgetUtils.getWidgetDataTitle(app)}

src/components/views/rooms/Stickerpicker.tsx

@@ -233,7 +233,9 @@ export default class Stickerpicker extends React.PureComponent<IProps, IState> {
 
     private sendVisibilityToWidget(visible: boolean): void {
         if (!this.state.stickerpickerWidget) return;
-        const messaging = WidgetMessagingStore.instance.getMessagingForId(this.state.stickerpickerWidget.id);
+        const messaging = WidgetMessagingStore.instance.getMessagingForUid(
+            WidgetUtils.calcWidgetUid(this.state.stickerpickerWidget.id, null),
+        );
         if (messaging && visible !== this.prevSentVisibility) {
             messaging.updateVisibility(visible).catch(err => {
                 logger.error("Error updating widget visibility: ", err);

src/components/views/voip/PipView.tsx

@@ -60,6 +60,7 @@ interface IState {
 
     // widget candidate to be displayed in the pip view.
     persistentWidgetId: string;
+    persistentRoomId: string;
     showWidgetInPip: boolean;
 
     moving: boolean;
@@ -122,6 +123,7 @@ export default class PipView extends React.Component<IProps, IState> {
             primaryCall: primaryCall,
             secondaryCall: secondaryCalls[0],
             persistentWidgetId: ActiveWidgetStore.instance.getPersistentWidgetId(),
+            persistentRoomId: ActiveWidgetStore.instance.getPersistentRoomId(),
             showWidgetInPip: false,
         };
     }
@@ -187,7 +189,10 @@ export default class PipView extends React.Component<IProps, IState> {
     };
 
     private onActiveWidgetStoreUpdate = (): void => {
-        this.updateShowWidgetInPip(ActiveWidgetStore.instance.getPersistentWidgetId());
+        this.updateShowWidgetInPip(
+            ActiveWidgetStore.instance.getPersistentWidgetId(),
+            ActiveWidgetStore.instance.getPersistentRoomId(),
+        );
     };
 
     private updateCalls = (): void => {
@@ -213,30 +218,27 @@ export default class PipView extends React.Component<IProps, IState> {
 
     private onDoubleClick = (): void => {
         const callRoomId = this.state.primaryCall?.roomId;
-        const widgetRoomId = ActiveWidgetStore.instance.getRoomId(this.state.persistentWidgetId);
+        if (callRoomId ?? this.state.persistentRoomId) {
-        if (!!(callRoomId ?? widgetRoomId)) {
             dis.dispatch<ViewRoomPayload>({
                 action: Action.ViewRoom,
-                room_id: callRoomId ?? widgetRoomId,
+                room_id: callRoomId ?? this.state.persistentRoomId,
                 metricsTrigger: "WebFloatingCallWindow",
             });
         }
     };
 
     // Accepts a persistentWidgetId to be able to skip awaiting the setState for persistentWidgetId
-    public updateShowWidgetInPip(persistentWidgetId = this.state.persistentWidgetId) {
+    public updateShowWidgetInPip(
+        persistentWidgetId = this.state.persistentWidgetId,
+        persistentRoomId = this.state.persistentRoomId,
+    ) {
         let fromAnotherRoom = false;
         let notVisible = false;
-        if (persistentWidgetId) {
+        // Sanity check the room - the widget may have been destroyed between render cycles, and
-            const persistentWidgetInRoomId = ActiveWidgetStore.instance.getRoomId(persistentWidgetId);
+        // thus no room is associated anymore.
-            const persistentWidgetInRoom = MatrixClientPeg.get().getRoom(persistentWidgetInRoomId);
+        if (persistentWidgetId && MatrixClientPeg.get().getRoom(persistentRoomId)) {
-
+            notVisible = !AppTile.isLive(persistentWidgetId, persistentRoomId);
-            // Sanity check the room - the widget may have been destroyed between render cycles, and
+            fromAnotherRoom = this.state.viewedRoomId !== persistentRoomId;
-            // thus no room is associated anymore.
-            if (persistentWidgetInRoom) {
-                notVisible = !AppTile.isLive(persistentWidgetId);
-                fromAnotherRoom = this.state.viewedRoomId !== persistentWidgetInRoomId;
-            }
         }
 
         // The widget should only be shown as a persistent app (in a floating
@@ -245,7 +247,7 @@ export default class PipView extends React.Component<IProps, IState> {
         // containers of the room view.
         const showWidgetInPip = fromAnotherRoom || notVisible;
 
-        this.setState({ showWidgetInPip, persistentWidgetId });
+        this.setState({ showWidgetInPip, persistentWidgetId, persistentRoomId });
     }
 
     public render() {
@@ -269,8 +271,7 @@ export default class PipView extends React.Component<IProps, IState> {
                 mx_CallView_pip: pipMode,
                 mx_CallView_large: !pipMode,
             });
-            const roomId = ActiveWidgetStore.instance.getRoomId(this.state.persistentWidgetId);
+            const roomForWidget = MatrixClientPeg.get().getRoom(this.state.persistentRoomId);
-            const roomForWidget = MatrixClientPeg.get().getRoom(roomId);
 
             pipContent = ({ onStartMoving, _onResize }) =>
                 <div className={pipViewClasses}>
@@ -281,6 +282,7 @@ export default class PipView extends React.Component<IProps, IState> {
                     />
                     <PersistentApp
                         persistentWidgetId={this.state.persistentWidgetId}
+                        persistentRoomId={this.state.persistentRoomId}
                         pointerEvents={this.state.moving ? 'none' : undefined}
                     />
                 </div>;

src/stores/ActiveWidgetStore.ts

@@ -16,8 +16,10 @@ limitations under the License.
 
 import EventEmitter from 'events';
 import { MatrixEvent, RoomStateEvent } from "matrix-js-sdk/src/matrix";
+import { RoomState } from "matrix-js-sdk/src/models/room-state";
 
 import { MatrixClientPeg } from '../MatrixClientPeg';
+import WidgetUtils from "../utils/WidgetUtils";
 import { WidgetMessagingStore } from "./widgets/WidgetMessagingStore";
 
 export enum ActiveWidgetStoreEvent {
@@ -33,8 +35,7 @@ export enum ActiveWidgetStoreEvent {
 export default class ActiveWidgetStore extends EventEmitter {
     private static internalInstance: ActiveWidgetStore;
     private persistentWidgetId: string;
-    // What room ID each widget is associated with (if it's a room widget)
+    private persistentRoomId: string;
-    private roomIdByWidgetId = new Map<string, string>();
 
     public static get instance(): ActiveWidgetStore {
         if (!ActiveWidgetStore.internalInstance) {
@@ -48,64 +49,49 @@ export default class ActiveWidgetStore extends EventEmitter {
     }
 
     public stop(): void {
-        if (MatrixClientPeg.get()) {
+        MatrixClientPeg.get()?.removeListener(RoomStateEvent.Events, this.onRoomStateEvents);
-            MatrixClientPeg.get().removeListener(RoomStateEvent.Events, this.onRoomStateEvents);
-        }
-        this.roomIdByWidgetId.clear();
     }
 
-    private onRoomStateEvents = (ev: MatrixEvent): void => {
+    private onRoomStateEvents = (ev: MatrixEvent, { roomId }: RoomState): void => {
         // XXX: This listens for state events in order to remove the active widget.
         // Everything else relies on views listening for events and calling setters
         // on this class which is terrible. This store should just listen for events
         // and keep itself up to date.
         // TODO: Enable support for m.widget event type (https://github.com/vector-im/element-web/issues/13111)
-        if (ev.getType() !== 'im.vector.modular.widgets') return;
+        if (ev.getType() === "im.vector.modular.widgets") {
-
+            this.destroyPersistentWidget(ev.getStateKey(), roomId);
-        if (ev.getStateKey() === this.persistentWidgetId) {
-            this.destroyPersistentWidget(this.persistentWidgetId);
         }
     };
 
-    public destroyPersistentWidget(id: string): void {
+    public destroyPersistentWidget(widgetId: string, roomId: string): void {
-        if (id !== this.persistentWidgetId) return;
+        if (!this.getWidgetPersistence(widgetId, roomId)) return;
-        const toDeleteId = this.persistentWidgetId;
+        WidgetMessagingStore.instance.stopMessagingByUid(WidgetUtils.calcWidgetUid(widgetId, roomId));
-
+        this.setWidgetPersistence(widgetId, roomId, false);
-        WidgetMessagingStore.instance.stopMessagingById(id);
-
-        this.setWidgetPersistence(toDeleteId, false);
-        this.delRoomId(toDeleteId);
     }
 
-    public setWidgetPersistence(widgetId: string, val: boolean): void {
+    public setWidgetPersistence(widgetId: string, roomId: string, val: boolean): void {
-        if (this.persistentWidgetId === widgetId && !val) {
+        const isPersisted = this.getWidgetPersistence(widgetId, roomId);
+
+        if (isPersisted && !val) {
             this.persistentWidgetId = null;
-        } else if (this.persistentWidgetId !== widgetId && val) {
+            this.persistentRoomId = null;
+        } else if (!isPersisted && val) {
             this.persistentWidgetId = widgetId;
+            this.persistentRoomId = roomId;
         }
         this.emit(ActiveWidgetStoreEvent.Update);
     }
 
-    public getWidgetPersistence(widgetId: string): boolean {
+    public getWidgetPersistence(widgetId: string, roomId: string): boolean {
-        return this.persistentWidgetId === widgetId;
+        return this.persistentWidgetId === widgetId && this.persistentRoomId === roomId;
     }
 
     public getPersistentWidgetId(): string {
         return this.persistentWidgetId;
     }
 
-    public getRoomId(widgetId: string): string {
+    public getPersistentRoomId(): string {
-        return this.roomIdByWidgetId.get(widgetId);
+        return this.persistentRoomId;
-    }
-
-    public setRoomId(widgetId: string, roomId: string): void {
-        this.roomIdByWidgetId.set(widgetId, roomId);
-        this.emit(ActiveWidgetStoreEvent.Update);
-    }
-
-    public delRoomId(widgetId: string): void {
-        this.roomIdByWidgetId.delete(widgetId);
-        this.emit(ActiveWidgetStoreEvent.Update);
     }
 }
 

src/stores/ModalWidgetStore.ts

@@ -33,6 +33,7 @@ export class ModalWidgetStore extends AsyncStoreWithClient<IState> {
     private static internalInstance = new ModalWidgetStore();
     private modalInstance: IHandle<void[]> = null;
     private openSourceWidgetId: string = null;
+    private openSourceWidgetRoomId: string = null;
 
     private constructor() {
         super(defaultDispatcher, {});
@@ -57,31 +58,38 @@ export class ModalWidgetStore extends AsyncStoreWithClient<IState> {
     ) => {
         if (this.modalInstance) return;
         this.openSourceWidgetId = sourceWidget.id;
+        this.openSourceWidgetRoomId = widgetRoomId;
         this.modalInstance = Modal.createTrackedDialog('Modal Widget', '', ModalWidgetDialog, {
             widgetDefinition: { ...requestData },
             widgetRoomId,
             sourceWidgetId: sourceWidget.id,
             onFinished: (success: boolean, data?: IModalWidgetReturnData) => {
                 if (!success) {
-                    this.closeModalWidget(sourceWidget, { "m.exited": true });
+                    this.closeModalWidget(sourceWidget, widgetRoomId, { "m.exited": true });
                 } else {
-                    this.closeModalWidget(sourceWidget, data);
+                    this.closeModalWidget(sourceWidget, widgetRoomId, data);
                 }
 
                 this.openSourceWidgetId = null;
+                this.openSourceWidgetRoomId = null;
                 this.modalInstance = null;
             },
         }, null, /* priority = */ false, /* static = */ true);
     };
 
-    public closeModalWidget = (sourceWidget: Widget, data?: IModalWidgetReturnData) => {
+    public closeModalWidget = (
+        sourceWidget: Widget,
+        widgetRoomId?: string,
+        data?: IModalWidgetReturnData,
+    ) => {
         if (!this.modalInstance) return;
-        if (this.openSourceWidgetId === sourceWidget.id) {
+        if (this.openSourceWidgetId === sourceWidget.id && this.openSourceWidgetRoomId === widgetRoomId) {
             this.openSourceWidgetId = null;
+            this.openSourceWidgetRoomId = null;
             this.modalInstance.close();
             this.modalInstance = null;
 
-            const sourceMessaging = WidgetMessagingStore.instance.getMessaging(sourceWidget);
+            const sourceMessaging = WidgetMessagingStore.instance.getMessaging(sourceWidget, widgetRoomId);
             if (!sourceMessaging) {
                 logger.error("No source widget messaging for modal widget");
                 return;

src/stores/WidgetStore.ts

@@ -29,7 +29,6 @@ import ActiveWidgetStore from "../stores/ActiveWidgetStore";
 import WidgetUtils from "../utils/WidgetUtils";
 import { WidgetType } from "../widgets/WidgetType";
 import { UPDATE_EVENT } from "./AsyncStore";
-import { MatrixClientPeg } from "../MatrixClientPeg";
 
 interface IState {}
 
@@ -44,10 +43,6 @@ interface IRoomWidgets {
     widgets: IApp[];
 }
 
-function widgetUid(app: IApp): string {
-    return `${app.roomId ?? MatrixClientPeg.get().getUserId()}::${app.id}`;
-}
-
 // TODO consolidate WidgetEchoStore into this
 // TODO consolidate ActiveWidgetStore into this
 export default class WidgetStore extends AsyncStoreWithClient<IState> {
@@ -119,13 +114,13 @@ export default class WidgetStore extends AsyncStoreWithClient<IState> {
         // otherwise we are out of sync with the rest of the app with stale widget events during removal
         Array.from(this.widgetMap.values()).forEach(app => {
             if (app.roomId !== room.roomId) return; // skip - wrong room
-            this.widgetMap.delete(widgetUid(app));
+            this.widgetMap.delete(WidgetUtils.getWidgetUid(app));
         });
 
         let edited = false;
         this.generateApps(room).forEach(app => {
             // Sanity check for https://github.com/vector-im/element-web/issues/15705
-            const existingApp = this.widgetMap.get(widgetUid(app));
+            const existingApp = this.widgetMap.get(WidgetUtils.getWidgetUid(app));
             if (existingApp) {
                 logger.warn(
                     `Possible widget ID conflict for ${app.id} - wants to store in room ${app.roomId} ` +
@@ -133,7 +128,7 @@ export default class WidgetStore extends AsyncStoreWithClient<IState> {
                 );
             }
 
-            this.widgetMap.set(widgetUid(app), app);
+            this.widgetMap.set(WidgetUtils.getWidgetUid(app), app);
             roomInfo.widgets.push(app);
             edited = true;
         });
@@ -144,14 +139,13 @@ export default class WidgetStore extends AsyncStoreWithClient<IState> {
         // If a persistent widget is active, check to see if it's just been removed.
         // If it has, it needs to destroyed otherwise unmounting the node won't kill it
         const persistentWidgetId = ActiveWidgetStore.instance.getPersistentWidgetId();
-        if (persistentWidgetId) {
+        if (
-            if (
+            persistentWidgetId &&
-                ActiveWidgetStore.instance.getRoomId(persistentWidgetId) === room.roomId &&
+            ActiveWidgetStore.instance.getPersistentRoomId() === room.roomId &&
-                !roomInfo.widgets.some(w => w.id === persistentWidgetId)
+            !roomInfo.widgets.some(w => w.id === persistentWidgetId)
-            ) {
+        ) {
-                logger.log(`Persistent widget ${persistentWidgetId} removed from room ${room.roomId}: destroying.`);
+            logger.log(`Persistent widget ${persistentWidgetId} removed from room ${room.roomId}: destroying.`);
-                ActiveWidgetStore.instance.destroyPersistentWidget(persistentWidgetId);
+            ActiveWidgetStore.instance.destroyPersistentWidget(persistentWidgetId, room.roomId);
-            }
         }
 
         this.emit(room.roomId);
@@ -196,7 +190,7 @@ export default class WidgetStore extends AsyncStoreWithClient<IState> {
 
         // A persistent conference widget indicates that we're participating
         const widgets = roomInfo.widgets.filter(w => WidgetType.JITSI.matches(w.type));
-        return widgets.some(w => ActiveWidgetStore.instance.getWidgetPersistence(w.id));
+        return widgets.some(w => ActiveWidgetStore.instance.getWidgetPersistence(w.id, room.roomId));
     }
 }
 

src/stores/widgets/StopGapWidget.ts

@@ -264,11 +264,7 @@ export class StopGapWidget extends EventEmitter {
         this.messaging.on("ready", () => this.emit("ready"));
         this.messaging.on("capabilitiesNotified", () => this.emit("capabilitiesNotified"));
         this.messaging.on(`action:${WidgetApiFromWidgetAction.OpenModalWidget}`, this.onOpenModal);
-        WidgetMessagingStore.instance.storeMessaging(this.mockWidget, this.messaging);
+        WidgetMessagingStore.instance.storeMessaging(this.mockWidget, this.roomId, this.messaging);
-
-        if (!this.appTileProps.userWidget && this.appTileProps.room) {
-            ActiveWidgetStore.instance.setRoomId(this.mockWidget.id, this.appTileProps.room.roomId);
-        }
 
         // Always attach a handler for ViewRoom, but permission check it internally
         this.messaging.on(`action:${ElementWidgetActions.ViewRoom}`, (ev: CustomEvent<IViewRoomApiRequest>) => {
@@ -318,7 +314,9 @@ export class StopGapWidget extends EventEmitter {
         this.messaging.on(`action:${WidgetApiFromWidgetAction.UpdateAlwaysOnScreen}`,
             (ev: CustomEvent<IStickyActionRequest>) => {
                 if (this.messaging.hasCapability(MatrixCapabilities.AlwaysOnScreen)) {
-                    ActiveWidgetStore.instance.setWidgetPersistence(this.mockWidget.id, ev.detail.data.value);
+                    ActiveWidgetStore.instance.setWidgetPersistence(
+                        this.mockWidget.id, this.roomId, ev.detail.data.value,
+                    );
                     ev.preventDefault();
                     this.messaging.transport.reply(ev.detail, <IWidgetApiRequestEmptyData>{}); // ack
                 }
@@ -384,7 +382,7 @@ export class StopGapWidget extends EventEmitter {
         await (WidgetVariableCustomisations?.isReady?.() ?? Promise.resolve());
 
         if (this.scalarToken) return;
-        const existingMessaging = WidgetMessagingStore.instance.getMessaging(this.mockWidget);
+        const existingMessaging = WidgetMessagingStore.instance.getMessaging(this.mockWidget, this.roomId);
         if (existingMessaging) this.messaging = existingMessaging;
         try {
             if (WidgetUtils.isScalarUrl(this.mockWidget.templateUrl)) {
@@ -410,13 +408,12 @@ export class StopGapWidget extends EventEmitter {
      * @param opts
      */
     public stopMessaging(opts = { forceDestroy: false }) {
-        if (!opts?.forceDestroy && ActiveWidgetStore.instance.getPersistentWidgetId() === this.mockWidget.id) {
+        if (!opts?.forceDestroy && ActiveWidgetStore.instance.getWidgetPersistence(this.mockWidget.id, this.roomId)) {
             logger.log("Skipping destroy - persistent widget");
             return;
         }
         if (!this.started) return;
-        WidgetMessagingStore.instance.stopMessaging(this.mockWidget);
+        WidgetMessagingStore.instance.stopMessaging(this.mockWidget, this.roomId);
-        ActiveWidgetStore.instance.delRoomId(this.mockWidget.id);
         this.messaging = null;
 
         if (MatrixClientPeg.get()) {

src/stores/widgets/WidgetMessagingStore.ts

@@ -20,6 +20,7 @@ import { AsyncStoreWithClient } from "../AsyncStoreWithClient";
 import defaultDispatcher from "../../dispatcher/dispatcher";
 import { ActionPayload } from "../../dispatcher/payloads";
 import { EnhancedMap } from "../../utils/maps";
+import WidgetUtils from "../../utils/WidgetUtils";
 
 /**
  * Temporary holding store for widget messaging instances. This is eventually
@@ -29,8 +30,7 @@ import { EnhancedMap } from "../../utils/maps";
 export class WidgetMessagingStore extends AsyncStoreWithClient<unknown> {
     private static internalInstance = new WidgetMessagingStore();
 
-    // TODO: Fix uniqueness problem (widget IDs are not unique across the whole app)
+    private widgetMap = new EnhancedMap<string, ClientWidgetApi>(); // <widget UID, ClientWidgetAPi>
-    private widgetMap = new EnhancedMap<string, ClientWidgetApi>(); // <widget ID, ClientWidgetAPi>
 
     public constructor() {
         super(defaultDispatcher);
@@ -49,35 +49,34 @@ export class WidgetMessagingStore extends AsyncStoreWithClient<unknown> {
         this.widgetMap.clear();
     }
 
-    public storeMessaging(widget: Widget, widgetApi: ClientWidgetApi) {
+    public storeMessaging(widget: Widget, roomId: string, widgetApi: ClientWidgetApi) {
-        this.stopMessaging(widget);
+        this.stopMessaging(widget, roomId);
-        this.widgetMap.set(widget.id, widgetApi);
+        this.widgetMap.set(WidgetUtils.calcWidgetUid(widget.id, roomId), widgetApi);
     }
 
-    public stopMessaging(widget: Widget) {
+    public stopMessaging(widget: Widget, roomId: string) {
-        this.widgetMap.remove(widget.id)?.stop();
+        this.widgetMap.remove(WidgetUtils.calcWidgetUid(widget.id, roomId))?.stop();
     }
 
-    public getMessaging(widget: Widget): ClientWidgetApi {
+    public getMessaging(widget: Widget, roomId: string): ClientWidgetApi {
-        return this.widgetMap.get(widget.id);
+        return this.widgetMap.get(WidgetUtils.calcWidgetUid(widget.id, roomId));
     }
 
     /**
-     * Stops the widget messaging instance for a given widget ID.
+     * Stops the widget messaging instance for a given widget UID.
-     * @param {string} widgetId The widget ID.
+     * @param {string} widgetId The widget UID.
-     * @deprecated Widget IDs are not globally unique.
      */
-    public stopMessagingById(widgetId: string) {
+    public stopMessagingByUid(widgetUid: string) {
-        this.widgetMap.remove(widgetId)?.stop();
+        this.widgetMap.remove(widgetUid)?.stop();
     }
 
     /**
-     * Gets the widget messaging class for a given widget ID.
+     * Gets the widget messaging class for a given widget UID.
-     * @param {string} widgetId The widget ID.
+     * @param {string} widgetId The widget UID.
      * @returns {ClientWidgetApi} The widget API, or a falsey value if not found.
      * @deprecated Widget IDs are not globally unique.
      */
-    public getMessagingForId(widgetId: string): ClientWidgetApi {
+    public getMessagingForUid(widgetUid: string): ClientWidgetApi {
-        return this.widgetMap.get(widgetId);
+        return this.widgetMap.get(widgetUid);
     }
 }

src/utils/WidgetUtils.ts

@@ -508,6 +508,14 @@ export default class WidgetUtils {
         return app?.data?.title?.trim() || "";
     }
 
+    static getWidgetUid(app?: IApp): string {
+        return app ? WidgetUtils.calcWidgetUid(app.id, app.roomId) : "";
+    }
+
+    static calcWidgetUid(widgetId: string, roomId?: string): string {
+        return roomId ? `room_${roomId}_${widgetId}` : `user_${widgetId}`;
+    }
+
     static editWidget(room: Room, app: IApp): void {
         // TODO: Open the right manager for the widget
         if (SettingsStore.getValue("feature_many_integration_managers")) {

test/components/views/elements/AppTile-test.tsx

@@ -44,7 +44,20 @@ describe("AppTile", () => {
     let r1;
     let r2;
     const resizeNotifier = new ResizeNotifier();
-    let app: IApp;
+    let app1: IApp;
+    let app2: IApp;
+
+    const waitForRps = (roomId: string) => new Promise<void>(resolve => {
+        const update = () => {
+            if (
+                RightPanelStore.instance.currentCardForRoom(roomId).phase !==
+                RightPanelPhases.Widget
+            ) return;
+            RightPanelStore.instance.off(UPDATE_EVENT, update);
+            resolve();
+        };
+        RightPanelStore.instance.on(UPDATE_EVENT, update);
+    });
 
     beforeAll(async () => {
         stubClient();
@@ -66,18 +79,31 @@ describe("AppTile", () => {
             return [r1, r2];
         });
 
-        // Adjust various widget stores to add a mock app
+        // Adjust various widget stores to add mock apps
-        app = {
+        app1 = {
             id: "1",
             eventId: "1",
             roomId: "r1",
             type: MatrixWidgetType.Custom,
             url: "https://example.com",
-            name: "Example",
+            name: "Example 1",
             creatorUserId: cli.getUserId(),
             avatar_url: null,
         };
-        jest.spyOn(WidgetStore.instance, "getApps").mockReturnValue([app]);
+        app2 = {
+            id: "1",
+            eventId: "2",
+            roomId: "r2",
+            type: MatrixWidgetType.Custom,
+            url: "https://example.com",
+            name: "Example 2",
+            creatorUserId: cli.getUserId(),
+            avatar_url: null,
+        };
+        jest.spyOn(WidgetStore.instance, "getApps").mockImplementation(roomId => {
+            if (roomId === "r1") return [app1];
+            if (roomId === "r2") return [app2];
+        });
 
         // Wake up various stores we rely on
         WidgetLayoutStore.instance.useUnitTestClient(cli);
@@ -88,6 +114,26 @@ describe("AppTile", () => {
         await RightPanelStore.instance.onReady();
     });
 
+    it("tracks live tiles correctly", () => {
+        expect(AppTile.isLive("1", "r1")).toEqual(false);
+
+        // Try removing the tile before it gets added
+        AppTile.removeLiveTile("1", "r1");
+        expect(AppTile.isLive("1", "r1")).toEqual(false);
+
+        AppTile.addLiveTile("1", "r1");
+        expect(AppTile.isLive("1", "r1")).toEqual(true);
+
+        AppTile.addLiveTile("1", "r1");
+        expect(AppTile.isLive("1", "r1")).toEqual(true);
+
+        AppTile.removeLiveTile("1", "r1");
+        expect(AppTile.isLive("1", "r1")).toEqual(true);
+
+        AppTile.removeLiveTile("1", "r1");
+        expect(AppTile.isLive("1", "r1")).toEqual(false);
+    });
+
     it("destroys non-persisted right panel widget on room change", async () => {
         // Set up right panel state
         const realGetValue = SettingsStore.getValue;
@@ -115,24 +161,14 @@ describe("AppTile", () => {
             />
         </MatrixClientContext.Provider>);
         // Wait for RPS room 1 updates to fire
-        const rpsUpdated = new Promise<void>(resolve => {
+        const rpsUpdated = waitForRps("r1");
-            const update = () => {
-                if (
-                    RightPanelStore.instance.currentCardForRoom("r1").phase !==
-                    RightPanelPhases.Widget
-                ) return;
-                RightPanelStore.instance.off(UPDATE_EVENT, update);
-                resolve();
-            };
-            RightPanelStore.instance.on(UPDATE_EVENT, update);
-        });
         dis.dispatch({
             action: Action.ViewRoom,
             room_id: "r1",
         });
         await rpsUpdated;
 
-        expect(AppTile.isLive("1")).toBe(true);
+        expect(AppTile.isLive("1", "r1")).toBe(true);
 
         // We want to verify that as we change to room 2, we should close the
         // right panel and destroy the widget.
@@ -152,11 +188,88 @@ describe("AppTile", () => {
         </MatrixClientContext.Provider>);
 
         expect(endWidgetActions.mock.calls.length).toBe(1);
-        expect(AppTile.isLive("1")).toBe(false);
+        expect(AppTile.isLive("1", "r1")).toBe(false);
 
         mockSettings.mockRestore();
     });
 
+    it("distinguishes widgets with the same ID in different rooms", async () => {
+        // Set up right panel state
+        const realGetValue = SettingsStore.getValue;
+        SettingsStore.getValue = (name, roomId) => {
+            if (name === "RightPanel.phases") {
+                if (roomId === "r1") {
+                    return {
+                        history: [{
+                            phase: RightPanelPhases.Widget,
+                            state: {
+                                widgetId: "1",
+                            },
+                        }],
+                        isOpen: true,
+                    };
+                }
+                return null;
+            }
+            return realGetValue(name, roomId);
+        };
+
+        // Run initial render with room 1, and also running lifecycle methods
+        const renderer = TestRenderer.create(<MatrixClientContext.Provider value={cli}>
+            <RightPanel
+                room={r1}
+                resizeNotifier={resizeNotifier}
+            />
+        </MatrixClientContext.Provider>);
+        // Wait for RPS room 1 updates to fire
+        const rpsUpdated1 = waitForRps("r1");
+        dis.dispatch({
+            action: Action.ViewRoom,
+            room_id: "r1",
+        });
+        await rpsUpdated1;
+
+        expect(AppTile.isLive("1", "r1")).toBe(true);
+        expect(AppTile.isLive("1", "r2")).toBe(false);
+
+        SettingsStore.getValue = (name, roomId) => {
+            if (name === "RightPanel.phases") {
+                if (roomId === "r2") {
+                    return {
+                        history: [{
+                            phase: RightPanelPhases.Widget,
+                            state: {
+                                widgetId: "1",
+                            },
+                        }],
+                        isOpen: true,
+                    };
+                }
+                return null;
+            }
+            return realGetValue(name, roomId);
+        };
+        // Wait for RPS room 2 updates to fire
+        const rpsUpdated2 = waitForRps("r2");
+        // Switch to room 2
+        dis.dispatch({
+            action: Action.ViewRoom,
+            room_id: "r2",
+        });
+        renderer.update(<MatrixClientContext.Provider value={cli}>
+            <RightPanel
+                room={r2}
+                resizeNotifier={resizeNotifier}
+            />
+        </MatrixClientContext.Provider>);
+        await rpsUpdated2;
+
+        expect(AppTile.isLive("1", "r1")).toBe(false);
+        expect(AppTile.isLive("1", "r2")).toBe(true);
+
+        SettingsStore.getValue = realGetValue;
+    });
+
     it("preserves non-persisted widget on container move", async () => {
         // Set up widget in top container
         const realGetValue = SettingsStore.getValue;
@@ -187,7 +300,7 @@ describe("AppTile", () => {
             />
         </MatrixClientContext.Provider>);
 
-        expect(AppTile.isLive("1")).toBe(true);
+        expect(AppTile.isLive("1", "r1")).toBe(true);
 
         // We want to verify that as we move the widget to the center container,
         // the widget frame remains running.
@@ -199,11 +312,11 @@ describe("AppTile", () => {
         // Stop mocking settings so that the widget move can take effect
         mockSettings.mockRestore();
         TestRenderer.act(() => {
-            WidgetLayoutStore.instance.moveToContainer(r1, app, Container.Center);
+            WidgetLayoutStore.instance.moveToContainer(r1, app1, Container.Center);
         });
 
         expect(endWidgetActions.mock.calls.length).toBe(0);
-        expect(AppTile.isLive("1")).toBe(true);
+        expect(AppTile.isLive("1", "r1")).toBe(true);
     });
 
     afterAll(async () => {