Merge pull request #4111 from matrix-org/matthew/dom-leaks

Fix two big DOM leaks which were locking Chrome solid.

src/components/views/messages/EditHistoryMessage.js

@@ -20,7 +20,7 @@ import * as HtmlUtils from '../../../HtmlUtils';
 import { editBodyDiffToHtml } from '../../../utils/MessageDiffUtils';
 import {formatTime} from '../../../DateUtils';
 import {MatrixEvent} from 'matrix-js-sdk';
-import {pillifyLinks} from '../../../utils/pillify';
+import {pillifyLinks, unmountPills} from '../../../utils/pillify';
 import { _t } from '../../../languageHandler';
 import * as sdk from '../../../index';
 import {MatrixClientPeg} from '../../../MatrixClientPeg';
@@ -53,6 +53,7 @@ export default class EditHistoryMessage extends React.PureComponent {
         this.state = {canRedact, sendStatus: event.getAssociatedStatus()};
 
         this._content = createRef();
+        this._pills = [];
     }
 
     _onAssociatedStatusChanged = () => {
@@ -81,7 +82,7 @@ export default class EditHistoryMessage extends React.PureComponent {
     pillifyLinks() {
         // not present for redacted events
         if (this._content.current) {
-            pillifyLinks(this._content.current.children, this.props.mxEvent);
+            pillifyLinks(this._content.current.children, this.props.mxEvent, this._pills);
         }
     }
 
@@ -90,6 +91,7 @@ export default class EditHistoryMessage extends React.PureComponent {
     }
 
     componentWillUnmount() {
+        unmountPills(this._pills);
         const event = this.props.mxEvent;
         if (event.localRedactionEvent()) {
             event.localRedactionEvent().off("status", this._onAssociatedStatusChanged);

src/components/views/messages/TextualBody.js

@@ -30,7 +30,7 @@ import { _t } from '../../../languageHandler';
 import * as ContextMenu from '../../structures/ContextMenu';
 import SettingsStore from "../../../settings/SettingsStore";
 import ReplyThread from "../elements/ReplyThread";
-import {pillifyLinks} from '../../../utils/pillify';
+import {pillifyLinks, unmountPills} from '../../../utils/pillify';
 import {IntegrationManagers} from "../../../integrations/IntegrationManagers";
 import {isPermalinkHost} from "../../../utils/permalinks/Permalinks";
 import {toRightOf} from "../../structures/ContextMenu";
@@ -92,6 +92,7 @@ export default createReactClass({
 
     componentDidMount: function() {
         this._unmounted = false;
+        this._pills = [];
         if (!this.props.editState) {
             this._applyFormatting();
         }
@@ -103,7 +104,7 @@ export default createReactClass({
         // pillifyLinks BEFORE linkifyElement because plain room/user URLs in the composer
         // are still sent as plaintext URLs. If these are ever pillified in the composer,
         // we should be pillify them here by doing the linkifying BEFORE the pillifying.
-        pillifyLinks([this._content.current], this.props.mxEvent);
+        pillifyLinks([this._content.current], this.props.mxEvent, this._pills);
         HtmlUtils.linkifyElement(this._content.current);
         this.calculateUrlPreview();
 
@@ -146,6 +147,7 @@ export default createReactClass({
 
     componentWillUnmount: function() {
         this._unmounted = true;
+        unmountPills(this._pills);
     },
 
     shouldComponentUpdate: function(nextProps, nextState) {

src/components/views/rooms/BasicMessageComposer.js

@@ -490,6 +490,7 @@ export default class BasicMessageEditor extends React.Component {
     }
 
     componentWillUnmount() {
+        document.removeEventListener("selectionchange", this._onSelectionChange);
         this._editorRef.removeEventListener("input", this._onInput, true);
         this._editorRef.removeEventListener("compositionstart", this._onCompositionStart, true);
         this._editorRef.removeEventListener("compositionend", this._onCompositionEnd, true);

src/utils/pillify.js

@@ -1,5 +1,5 @@
 /*
-Copyright 2019 The Matrix.org Foundation C.I.C.
+Copyright 2019, 2020 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.
@@ -21,7 +21,20 @@ import SettingsStore from "../settings/SettingsStore";
 import {PushProcessor} from 'matrix-js-sdk/src/pushprocessor';
 import * as sdk from '../index';
 
-export function pillifyLinks(nodes, mxEvent) {
+/**
+ * Recurses depth-first through a DOM tree, converting matrix.to links
+ * into pills based on the context of a given room.  Returns a list of
+ * the resulting React nodes so they can be unmounted rather than leaking.
+ *
+ * @param {Node[]} nodes - a list of sibling DOM nodes to traverse to try
+ *   to turn into pills.
+ * @param {MatrixEvent} mxEvent - the matrix event which the DOM nodes are
+ *   part of representing.
+ * @param {Node[]} pills: an accumulator of the DOM nodes which contain
+ *   React components which have been mounted as part of this.
+ *   The initial caller should pass in an empty array to seed the accumulator.
+ */
+export function pillifyLinks(nodes, mxEvent, pills) {
     const room = MatrixClientPeg.get().getRoom(mxEvent.getRoomId());
     const shouldShowPillAvatar = SettingsStore.getValue("Pill.shouldShowPillAvatar");
     let node = nodes[0];
@@ -45,6 +58,7 @@ export function pillifyLinks(nodes, mxEvent) {
 
                 ReactDOM.render(pill, pillContainer);
                 node.parentNode.replaceChild(pillContainer, node);
+                pills.push(pillContainer);
                 // Pills within pills aren't going to go well, so move on
                 pillified = true;
 
@@ -102,6 +116,7 @@ export function pillifyLinks(nodes, mxEvent) {
 
                         ReactDOM.render(pill, pillContainer);
                         roomNotifTextNode.parentNode.replaceChild(pillContainer, roomNotifTextNode);
+                        pills.push(pillContainer);
                     }
                     // Nothing else to do for a text node (and we don't need to advance
                     // the loop pointer because we did it above)
@@ -111,9 +126,26 @@ export function pillifyLinks(nodes, mxEvent) {
         }
 
         if (node.childNodes && node.childNodes.length && !pillified) {
-            pillifyLinks(node.childNodes, mxEvent);
+            pillifyLinks(node.childNodes, mxEvent, pills);
         }
 
         node = node.nextSibling;
     }
 }
+
+/**
+ * Unmount all the pill containers from React created by pillifyLinks.
+ *
+ * It's critical to call this after pillifyLinks, otherwise
+ * Pills will leak, leaking entire DOM trees via the event
+ * emitter on BaseAvatar as per
+ * https://github.com/vector-im/riot-web/issues/12417
+ *
+ * @param {Node[]} pills - array of pill containers whose React
+ *   components should be unmounted.
+ */
+export function unmountPills(pills) {
+    for (const pillContainer of pills) {
+        ReactDOM.unmountComponentAtNode(pillContainer);
+    }
+}