Tear down AppTile using lifecycle tracking (#7833)

This commit is contained in:
J. Ryan Stinnett 2022-02-17 16:30:36 +00:00 committed by GitHub
parent f697301298
commit a939184e10
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 295 additions and 79 deletions

View file

@ -2,7 +2,7 @@
Copyright 2017 Vector Creations Ltd
Copyright 2018 New Vector Ltd
Copyright 2019 Michael Telatynski <7t3chguy@gmail.com>
Copyright 2020 The Matrix.org Foundation C.I.C.
Copyright 2020 - 2022 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
@ -23,7 +23,6 @@ import classNames from 'classnames';
import { MatrixCapabilities } from "matrix-widget-api";
import { Room } from "matrix-js-sdk/src/models/room";
import { logger } from "matrix-js-sdk/src/logger";
import { EventSubscription } from 'fbemitter';
import AccessibleButton from './AccessibleButton';
import { _t } from '../../../languageHandler';
@ -118,17 +117,40 @@ 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.
// 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 } = {};
public static addLiveTile(widgetId: string): void {
const refs = this.liveTilesById[widgetId] || 0;
this.liveTilesById[widgetId] = refs + 1;
}
public static removeLiveTile(widgetId: string): void {
const refs = this.liveTilesById[widgetId] || 0;
this.liveTilesById[widgetId] = refs - 1;
}
public static isLive(widgetId: string): boolean {
const refs = this.liveTilesById[widgetId] || 0;
return refs > 0;
}
private contextMenuButton = createRef<any>();
private iframe: HTMLIFrameElement; // ref to the iframe (callback style)
private allowedWidgetsWatchRef: string;
private persistKey: string;
private sgWidget: StopGapWidget;
private dispatcherRef: string;
private roomStoreToken: EventSubscription;
private unmounted: boolean;
constructor(props: IProps) {
super(props);
AppTile.addLiveTile(this.props.app.id);
// The key used for PersistedElement
this.persistKey = getPersistKey(this.props.app.id);
try {
@ -165,23 +187,6 @@ export default class AppTile extends React.Component<IProps, IState> {
return !!currentlyAllowedWidgets[props.app.eventId];
};
private onWidgetLayoutChange = () => {
const isActiveWidget = ActiveWidgetStore.instance.getWidgetPersistence(this.props.app.id);
const isVisibleOnScreen = WidgetLayoutStore.instance.isVisibleOnScreen(this.props.room, this.props.app.id);
if (!isVisibleOnScreen && !isActiveWidget) {
this.endWidgetActions();
}
};
private onRoomViewStoreUpdate = () => {
if (this.props.room?.roomId === RoomViewStore.getRoomId()) return;
const isActiveWidget = ActiveWidgetStore.instance.getWidgetPersistence(this.props.app.id);
// Stop the widget if it's not the active (persistent) widget and it's not a user widget
if (!isActiveWidget && !this.props.userWidget) {
this.endWidgetActions();
}
};
private onUserLeftRoom() {
const isActiveWidget = ActiveWidgetStore.instance.getWidgetPersistence(this.props.app.id);
if (isActiveWidget) {
@ -263,28 +268,46 @@ export default class AppTile extends React.Component<IProps, IState> {
this.watchUserReady();
if (this.props.room) {
const emitEvent = WidgetLayoutStore.emissionForRoom(this.props.room);
WidgetLayoutStore.instance.on(emitEvent, this.onWidgetLayoutChange);
this.context.on("Room.myMembership", this.onMyMembership);
}
this.roomStoreToken = RoomViewStore.addListener(this.onRoomViewStoreUpdate);
this.allowedWidgetsWatchRef = SettingsStore.watchSetting("allowedWidgets", null, this.onAllowedWidgetsChange);
// Widget action listeners
this.dispatcherRef = dis.register(this.onAction);
}
public componentWillUnmount(): void {
this.unmounted = true;
// It might seem simplest to always tear down the widget itself here,
// and indeed that would be a bit easier to reason about... however, we
// support moving widgets between containers (e.g. top <-> center).
// During such a move, this component will unmount from the old
// container and remount in the new container. By keeping the widget
// iframe loaded across this transition, the widget doesn't notice that
// anything happened, which improves overall widget UX. During this kind
// of movement between containers, the new `AppTile` for the new
// 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);
// 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);
if (!AppTile.isLive(this.props.app.id) && !isActiveWidget) {
this.endWidgetActions();
}
// Widget action listeners
if (this.dispatcherRef) dis.unregister(this.dispatcherRef);
if (this.props.room) {
const emitEvent = WidgetLayoutStore.emissionForRoom(this.props.room);
WidgetLayoutStore.instance.off(emitEvent, this.onWidgetLayoutChange);
this.context.off("Room.myMembership", this.onMyMembership);
}
this.roomStoreToken?.remove();
SettingsStore.unwatchSetting(this.allowedWidgetsWatchRef);
OwnProfileStore.instance.removeListener(UPDATE_EVENT, this.onUserReady);
}
@ -319,6 +342,7 @@ export default class AppTile extends React.Component<IProps, IState> {
private startWidget(): void {
this.sgWidget.prepare().then(() => {
if (this.unmounted) return;
this.setState({ initialising: false });
});
}
@ -333,6 +357,7 @@ export default class AppTile extends React.Component<IProps, IState> {
private iframeRefChange = (ref: HTMLIFrameElement): void => {
this.iframe = ref;
if (this.unmounted) return;
if (ref) {
this.startMessaging();
} else {
@ -668,6 +693,7 @@ export default class AppTile extends React.Component<IProps, IState> {
"mx_AppTileMenuBar_iconButton_maximise": !isMaximised,
});
layoutButtons.push(<AccessibleButton
key="toggleMaximised"
className={maximisedClasses}
title={
isMaximised ? _t("Close") : _t("Maximise")
@ -683,6 +709,7 @@ export default class AppTile extends React.Component<IProps, IState> {
"mx_AppTileMenuBar_iconButton_pin": !isPinned,
});
layoutButtons.push(<AccessibleButton
key="togglePinned"
className={pinnedClasses}
title={
isPinned ? _t("Unpin") : _t("Pin")

View file

@ -17,7 +17,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
import React, { Component, CSSProperties } from 'react';
import React, { CSSProperties } from 'react';
import ReactDOM from 'react-dom';
import classNames from 'classnames';
@ -58,7 +58,6 @@ export interface ITooltipProps {
@replaceableComponent("views.elements.Tooltip")
export default class Tooltip extends React.Component<ITooltipProps> {
private tooltipContainer: HTMLElement;
private tooltip: void | Element | Component<Element, any, any>;
private parent: Element;
// XXX: This is because some components (Field) are unable to `import` the Tooltip class,
@ -178,7 +177,7 @@ export default class Tooltip extends React.Component<ITooltipProps> {
);
// Render the tooltip manually, as we wish it not to be rendered within the parent
this.tooltip = ReactDOM.render<Element>(tooltip, this.tooltipContainer);
ReactDOM.render<Element>(tooltip, this.tooltipContainer);
};
public render() {

View file

@ -33,10 +33,8 @@ import { Action } from "../../../dispatcher/actions";
import { WidgetLayoutStore } from '../../../stores/widgets/WidgetLayoutStore';
import CallViewHeader from './CallView/CallViewHeader';
import ActiveWidgetStore, { ActiveWidgetStoreEvent } from '../../../stores/ActiveWidgetStore';
import { UPDATE_EVENT } from '../../../stores/AsyncStore';
import { RightPanelPhases } from '../../../stores/right-panel/RightPanelStorePhases';
import RightPanelStore from '../../../stores/right-panel/RightPanelStore';
import { ViewRoomPayload } from "../../../dispatcher/payloads/ViewRoomPayload";
import AppTile from '../elements/AppTile';
const SHOW_CALL_IN_STATES = [
CallState.Connected,
@ -63,7 +61,6 @@ interface IState {
// widget candidate to be displayed in the pip view.
persistentWidgetId: string;
showWidgetInPip: boolean;
rightPanelPhase: RightPanelPhases;
moving: boolean;
}
@ -125,7 +122,6 @@ export default class PipView extends React.Component<IProps, IState> {
primaryCall: primaryCall,
secondaryCall: secondaryCalls[0],
persistentWidgetId: ActiveWidgetStore.instance.getPersistentWidgetId(),
rightPanelPhase: RightPanelStore.instance.currentCard.phase,
showWidgetInPip: false,
};
}
@ -139,7 +135,6 @@ export default class PipView extends React.Component<IProps, IState> {
if (room) {
WidgetLayoutStore.instance.on(WidgetLayoutStore.emissionForRoom(room), this.updateCalls);
}
RightPanelStore.instance.on(UPDATE_EVENT, this.onRightPanelStoreUpdate);
ActiveWidgetStore.instance.on(ActiveWidgetStoreEvent.Update, this.onActiveWidgetStoreUpdate);
document.addEventListener("mouseup", this.onEndMoving.bind(this));
}
@ -154,7 +149,6 @@ export default class PipView extends React.Component<IProps, IState> {
if (room) {
WidgetLayoutStore.instance.off(WidgetLayoutStore.emissionForRoom(room), this.updateCalls);
}
RightPanelStore.instance.off(UPDATE_EVENT, this.onRightPanelStoreUpdate);
ActiveWidgetStore.instance.off(ActiveWidgetStoreEvent.Update, this.onActiveWidgetStoreUpdate);
document.removeEventListener("mouseup", this.onEndMoving.bind(this));
}
@ -192,13 +186,6 @@ export default class PipView extends React.Component<IProps, IState> {
this.updateShowWidgetInPip();
};
private onRightPanelStoreUpdate = () => {
this.setState({
rightPanelPhase: RightPanelStore.instance.currentCard.phase,
});
this.updateShowWidgetInPip();
};
private onActiveWidgetStoreUpdate = (): void => {
this.updateShowWidgetInPip(ActiveWidgetStore.instance.getPersistentWidgetId());
};
@ -247,14 +234,15 @@ export default class PipView extends React.Component<IProps, IState> {
// Sanity check the room - the widget may have been destroyed between render cycles, and
// thus no room is associated anymore.
if (persistentWidgetInRoom) {
const wls = WidgetLayoutStore.instance;
notVisible = !wls.isVisibleOnScreen(persistentWidgetInRoom, persistentWidgetId);
notVisible = !AppTile.isLive(persistentWidgetId);
fromAnotherRoom = this.state.viewedRoomId !== persistentWidgetInRoomId;
}
}
// The widget should only be shown as a persistent app (in a floating pip container) if it is not visible on screen
// either, because we are viewing a different room OR because it is in none of the possible containers of the room view.
// The widget should only be shown as a persistent app (in a floating
// pip container) if it is not visible on screen: either because we are
// viewing a different room OR because it is in none of the possible
// containers of the room view.
const showWidgetInPip = fromAnotherRoom || notVisible;
this.setState({ showWidgetInPip, persistentWidgetId });

View file

@ -31,7 +31,6 @@ import {
convertToStorePanel,
IRightPanelForRoom,
} from './RightPanelStoreIPanelState';
import { MatrixClientPeg } from "../../MatrixClientPeg";
import RoomViewStore from '../RoomViewStore';
const GROUP_PHASES = [
@ -71,7 +70,7 @@ export default class RightPanelStore extends ReadyWatchingStore {
protected async onReady(): Promise<any> {
this.isReady = true;
this.roomStoreToken = RoomViewStore.addListener(this.onRoomViewStoreUpdate);
MatrixClientPeg.get().on("crypto.verification.request", this.onVerificationRequestUpdate);
this.matrixClient.on("crypto.verification.request", this.onVerificationRequestUpdate);
this.viewedRoomId = RoomViewStore.getRoomId();
this.loadCacheFromSettings();
this.emitAndUpdateSettings();
@ -85,7 +84,7 @@ export default class RightPanelStore extends ReadyWatchingStore {
protected async onNotReady(): Promise<any> {
this.isReady = false;
MatrixClientPeg.get().off("crypto.verification.request", this.onVerificationRequestUpdate);
this.matrixClient.off("crypto.verification.request", this.onVerificationRequestUpdate);
this.roomStoreToken.remove();
}

View file

@ -1,5 +1,5 @@
/*
* Copyright 2021 The Matrix.org Foundation C.I.C.
* Copyright 2021 - 2022 The Matrix.org Foundation C.I.C.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -28,8 +28,6 @@ import { SettingLevel } from "../../settings/SettingLevel";
import { arrayFastClone } from "../../utils/arrays";
import { UPDATE_EVENT } from "../AsyncStore";
import { compare } from "../../utils/strings";
import RightPanelStore from "../right-panel/RightPanelStore";
import { RightPanelPhases } from "../right-panel/RightPanelStorePhases";
export const WIDGET_LAYOUT_EVENT_TYPE = "io.element.widgets.layout";
@ -354,22 +352,6 @@ export class WidgetLayoutStore extends ReadyWatchingStore {
return this.getContainerWidgets(room, container).some(w => w.id === widget.id);
}
public isVisibleOnScreen(room: Optional<Room>, widgetId: string) {
const wId = widgetId;
const inRightPanel =
(RightPanelStore.instance.currentCard.phase == RightPanelPhases.Widget &&
wId == RightPanelStore.instance.currentCard.state?.widgetId);
const inCenterContainer =
this.getContainerWidgets(room, Container.Center).some((app) => app.id == wId);
const inTopContainer =
this.getContainerWidgets(room, Container.Top).some(app => app.id == wId);
// The widget should only be shown as a persistent app (in a floating pip container) if it is not visible on screen
// either, because we are viewing a different room OR because it is in none of the possible containers of the room view.
const isVisibleOnScreen = (inRightPanel || inCenterContainer || inTopContainer);
return isVisibleOnScreen;
}
public canAddToContainer(room: Room, container: Container): boolean {
switch (container) {
case Container.Top: return this.getContainerWidgets(room, container).length < MAX_PINNED;

View file

@ -18,7 +18,6 @@ import React from "react";
import TestRenderer from "react-test-renderer";
import { jest } from "@jest/globals";
import { Room } from "matrix-js-sdk/src/models/room";
import { SyncState } from "matrix-js-sdk/src/sync";
// We can't use the usual `skinned-sdk`, as it stubs out the RightPanel
import "../../minimal-sdk";
@ -34,6 +33,7 @@ import SettingsStore from "../../../src/settings/SettingsStore";
import { RightPanelPhases } from "../../../src/stores/right-panel/RightPanelStorePhases";
import RightPanelStore from "../../../src/stores/right-panel/RightPanelStore";
import { UPDATE_EVENT } from "../../../src/stores/AsyncStore";
import { WidgetLayoutStore } from "../../../src/stores/widgets/WidgetLayoutStore";
describe("RightPanel", () => {
it("renders info from only one room during room changes", async () => {
@ -72,13 +72,13 @@ describe("RightPanel", () => {
return null;
});
// Wake up any stores waiting for a client
dis.dispatch({
action: "MatrixActions.sync",
prevState: SyncState.Prepared,
state: SyncState.Syncing,
matrixClient: cli,
});
// Wake up various stores we rely on
WidgetLayoutStore.instance.useUnitTestClient(cli);
// @ts-ignore
await WidgetLayoutStore.instance.onReady();
RightPanelStore.instance.useUnitTestClient(cli);
// @ts-ignore
await RightPanelStore.instance.onReady();
const resizeNotifier = new ResizeNotifier();
@ -139,7 +139,11 @@ describe("RightPanel", () => {
await rendered;
});
afterAll(() => {
afterAll(async () => {
// @ts-ignore
await WidgetLayoutStore.instance.onNotReady();
// @ts-ignore
await RightPanelStore.instance.onNotReady();
jest.restoreAllMocks();
});
});

View file

@ -0,0 +1,216 @@
/*
Copyright 2022 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
import React from "react";
import TestRenderer from "react-test-renderer";
import { jest } from "@jest/globals";
import { Room } from "matrix-js-sdk/src/models/room";
import { MatrixWidgetType } from "matrix-widget-api";
// We can't use the usual `skinned-sdk`, as it stubs out the RightPanel
import "../../../minimal-sdk";
import RightPanel from "../../../../src/components/structures/RightPanel";
import { MatrixClientPeg } from "../../../../src/MatrixClientPeg";
import ResizeNotifier from "../../../../src/utils/ResizeNotifier";
import { stubClient } from "../../../test-utils";
import { Action } from "../../../../src/dispatcher/actions";
import dis from "../../../../src/dispatcher/dispatcher";
import DMRoomMap from "../../../../src/utils/DMRoomMap";
import MatrixClientContext from "../../../../src/contexts/MatrixClientContext";
import SettingsStore from "../../../../src/settings/SettingsStore";
import { RightPanelPhases } from "../../../../src/stores/right-panel/RightPanelStorePhases";
import RightPanelStore from "../../../../src/stores/right-panel/RightPanelStore";
import { UPDATE_EVENT } from "../../../../src/stores/AsyncStore";
import WidgetStore, { IApp } from "../../../../src/stores/WidgetStore";
import AppTile from "../../../../src/components/views/elements/AppTile";
import { Container, WidgetLayoutStore } from "../../../../src/stores/widgets/WidgetLayoutStore";
import AppsDrawer from "../../../../src/components/views/rooms/AppsDrawer";
describe("AppTile", () => {
let cli;
let r1;
let r2;
const resizeNotifier = new ResizeNotifier();
let app: IApp;
beforeAll(async () => {
stubClient();
cli = MatrixClientPeg.get();
cli.hasLazyLoadMembersEnabled = () => false;
// Init misc. startup deps
DMRoomMap.makeShared();
r1 = new Room("r1", cli, "@name:example.com");
r2 = new Room("r2", cli, "@name:example.com");
jest.spyOn(cli, "getRoom").mockImplementation(roomId => {
if (roomId === "r1") return r1;
if (roomId === "r2") return r2;
return null;
});
jest.spyOn(cli, "getVisibleRooms").mockImplementation(() => {
return [r1, r2];
});
// Adjust various widget stores to add a mock app
app = {
id: "1",
eventId: "1",
roomId: "r1",
type: MatrixWidgetType.Custom,
url: "https://example.com",
name: "Example",
creatorUserId: cli.getUserId(),
avatar_url: null,
};
jest.spyOn(WidgetStore.instance, "getApps").mockReturnValue([app]);
// Wake up various stores we rely on
WidgetLayoutStore.instance.useUnitTestClient(cli);
// @ts-ignore
await WidgetLayoutStore.instance.onReady();
RightPanelStore.instance.useUnitTestClient(cli);
// @ts-ignore
await RightPanelStore.instance.onReady();
});
it("destroys non-persisted right panel widget on room change", async () => {
// Set up right panel state
const realGetValue = SettingsStore.getValue;
const mockSettings = jest.spyOn(SettingsStore, "getValue").mockImplementation((name, roomId) => {
if (name !== "RightPanel.phases") return realGetValue(name, roomId);
if (roomId === "r1") {
return {
history: [{
phase: RightPanelPhases.Widget,
state: {
widgetId: "1",
},
}],
isOpen: true,
};
}
return null;
});
// 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 rpsUpdated = new Promise<void>(resolve => {
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);
// We want to verify that as we change to room 2, we should close the
// right panel and destroy the widget.
const instance = renderer.root.findByType(AppTile).instance;
const endWidgetActions = jest.spyOn(instance, "endWidgetActions");
console.log("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>);
expect(endWidgetActions.mock.calls.length).toBe(1);
expect(AppTile.isLive("1")).toBe(false);
mockSettings.mockRestore();
});
it("preserves non-persisted widget on container move", async () => {
// Set up widget in top container
const realGetValue = SettingsStore.getValue;
const mockSettings = jest.spyOn(SettingsStore, "getValue").mockImplementation((name, roomId) => {
if (name !== "Widgets.layout") return realGetValue(name, roomId);
if (roomId === "r1") {
return {
widgets: {
1: {
container: Container.Top,
},
},
};
}
return null;
});
TestRenderer.act(() => {
WidgetLayoutStore.instance.recalculateRoom(r1);
});
// Run initial render with room 1, and also running lifecycle methods
const renderer = TestRenderer.create(<MatrixClientContext.Provider value={cli}>
<AppsDrawer
userId={cli.getUserId()}
room={r1}
resizeNotifier={resizeNotifier}
/>
</MatrixClientContext.Provider>);
expect(AppTile.isLive("1")).toBe(true);
// We want to verify that as we move the widget to the center container,
// the widget frame remains running.
const instance = renderer.root.findByType(AppTile).instance;
const endWidgetActions = jest.spyOn(instance, "endWidgetActions");
console.log("Move widget to center");
// Stop mocking settings so that the widget move can take effect
mockSettings.mockRestore();
TestRenderer.act(() => {
WidgetLayoutStore.instance.moveToContainer(r1, app, Container.Center);
});
expect(endWidgetActions.mock.calls.length).toBe(0);
expect(AppTile.isLive("1")).toBe(true);
});
afterAll(async () => {
// @ts-ignore
await WidgetLayoutStore.instance.onNotReady();
// @ts-ignore
await RightPanelStore.instance.onNotReady();
jest.restoreAllMocks();
});
});

View file

@ -56,8 +56,9 @@ export function createTestClient() {
getGroups: jest.fn().mockReturnValue([]),
loginFlows: jest.fn(),
on: eventEmitter.on.bind(eventEmitter),
emit: eventEmitter.emit.bind(eventEmitter),
off: eventEmitter.off.bind(eventEmitter),
removeListener: eventEmitter.removeListener.bind(eventEmitter),
emit: eventEmitter.emit.bind(eventEmitter),
isRoomEncrypted: jest.fn().mockReturnValue(false),
peekInRoom: jest.fn().mockResolvedValue(mkStubRoom()),