Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't assume that widget IDs are unique #8052

Merged
merged 4 commits into from
Mar 15, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/CallHandler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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, {});
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/context_menus/WidgetContextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
45 changes: 26 additions & 19 deletions src/components/views/elements/AppTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
const refs = this.liveTilesById[widgetId] || 0;
this.liveTilesById[widgetId] = refs + 1;
public static addLiveTile(widgetId: string, roomId: string): void {
robintown marked this conversation as resolved.
Show resolved Hide resolved
robintown marked this conversation as resolved.
Show resolved Hide resolved
const uid = WidgetUtils.calcWidgetUid(widgetId, roomId);
const refs = this.liveTilesByUid.get(uid) ?? 0;
this.liveTilesByUid.set(uid, refs + 1);
}

public static removeLiveTile(widgetId: string): void {
const refs = this.liveTilesById[widgetId] || 0;
this.liveTilesById[widgetId] = refs - 1;
public static removeLiveTile(widgetId: string, roomId: string): void {
const uid = WidgetUtils.calcWidgetUid(widgetId, roomId);
const refs = this.liveTilesByUid.get(uid) ?? 0;
this.liveTilesByUid.set(uid, refs - 1);
robintown marked this conversation as resolved.
Show resolved Hide resolved
}

public static isLive(widgetId: string): boolean {
const refs = this.liveTilesById[widgetId] || 0;
public static isLive(widgetId: string, roomId: string): boolean {
const uid = WidgetUtils.calcWidgetUid(widgetId, roomId);
const refs = this.liveTilesByUid.get(uid) ?? 0;
return refs > 0;
}

Expand All @@ -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();
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}
}
}
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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 });
}
Expand Down
25 changes: 13 additions & 12 deletions src/components/views/elements/PersistentApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -26,39 +26,40 @@ import MatrixClientContext from "../../../contexts/MatrixClientContext";

interface IProps {
persistentWidgetId: string;
persistentRoomId: string;
pointerEvents?: string;
}

@replaceableComponent("views.elements.PersistentApp")
export default class PersistentApp extends React.Component<IProps> {
public static contextType = MatrixClientContext;
context: ContextType<typeof MatrixClientContext>;
private room: Room;

private get app(): IApp {
const persistentWidgetInRoomId = ActiveWidgetStore.instance.getRoomId(this.props.persistentWidgetId);
const persistentWidgetInRoom = this.context.getRoom(persistentWidgetInRoomId);
constructor(props: IProps, context: ContextType<typeof MatrixClientContext>) {
super(props, context);
this.room = context.getRoom(this.props.persistentRoomId);
}

private get app(): IApp {
// get the widget data
const appEvent = WidgetUtils.getRoomWidgets(persistentWidgetInRoom).find((ev) => {
return ev.getStateKey() === ActiveWidgetStore.instance.getPersistentWidgetId();
});
const appEvent = WidgetUtils.getRoomWidgets(this.room).find(ev =>
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)}
Expand Down
4 changes: 3 additions & 1 deletion src/components/views/rooms/Stickerpicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
38 changes: 20 additions & 18 deletions src/components/views/voip/PipView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ interface IState {

// widget candidate to be displayed in the pip view.
persistentWidgetId: string;
persistentRoomId: string;
showWidgetInPip: boolean;

moving: boolean;
Expand Down Expand Up @@ -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,
};
}
Expand Down Expand Up @@ -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 => {
Expand All @@ -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 ?? widgetRoomId)) {
if (callRoomId ?? this.state.persistentRoomId) {
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) {
const persistentWidgetInRoomId = ActiveWidgetStore.instance.getRoomId(persistentWidgetId);
const persistentWidgetInRoom = MatrixClientPeg.get().getRoom(persistentWidgetInRoomId);

// Sanity check the room - the widget may have been destroyed between render cycles, and
// thus no room is associated anymore.
if (persistentWidgetInRoom) {
notVisible = !AppTile.isLive(persistentWidgetId);
fromAnotherRoom = this.state.viewedRoomId !== persistentWidgetInRoomId;
}
// Sanity check the room - the widget may have been destroyed between render cycles, and
// thus no room is associated anymore.
if (persistentWidgetId && MatrixClientPeg.get().getRoom(persistentRoomId)) {
notVisible = !AppTile.isLive(persistentWidgetId, persistentRoomId);
fromAnotherRoom = this.state.viewedRoomId !== persistentRoomId;
}

// The widget should only be shown as a persistent app (in a floating
Expand All @@ -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() {
Expand All @@ -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(roomId);
const roomForWidget = MatrixClientPeg.get().getRoom(this.state.persistentRoomId);

pipContent = ({ onStartMoving, _onResize }) =>
<div className={pipViewClasses}>
Expand All @@ -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>;
Expand Down
Loading