Skip to content

Commit

Permalink
Don't assume that widget IDs are unique (#8052)
Browse files Browse the repository at this point in the history 
* 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>
  • Loading branch information
@robintown
robintown committed Mar 15, 2022
1 parent bc8fdac commit 744eeb5
Show file tree
Hide file tree
Showing 13 changed files with 276 additions and 159 deletions.
2 changes: 1 addition & 1 deletion src/CallHandler.tsx
View file
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
View file
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
View file
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 {
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);
if (refs) this.liveTilesByUid.set(uid, refs - 1);
}

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
View file
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
View file
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
View file
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

0 comments on commit 744eeb5

Please sign in to comment.