From 62cf34b58cb554324fda7cb11e130c353d954ca6 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 8 Jan 2016 12:03:45 +0000 Subject: [PATCH] Fix some races due to promises completing after we've switched rooms Add a few isMounted() checks to promise handlers so that we don't end up throwing NPEs. --- src/components/structures/RoomView.js | 13 ++++++++----- src/components/structures/ScrollPanel.js | 10 ++++++++++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 9ab99d9cba..c91be22a47 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -353,11 +353,14 @@ module.exports = React.createClass({ _paginateCompleted: function() { debuglog("paginate complete"); - this.setState({ - room: MatrixClientPeg.get().getRoom(this.props.roomId) - }); + // we might have switched rooms since the paginate started - just bin + // the results if so. + if (!this.isMounted()) return; - this.setState({paginating: false}); + this.setState({ + room: MatrixClientPeg.get().getRoom(this.props.roomId), + paginating: false, + }); }, onSearchResultsFillRequest: function(backwards) { @@ -535,7 +538,7 @@ module.exports = React.createClass({ return searchPromise.then(function(results) { debuglog("search complete"); - if (!self.state.searching || self.searchId != localSearchId) { + if (!this.isMounted() || !self.state.searching || self.searchId != localSearchId) { console.error("Discarding stale search results"); return; } diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 042458717d..fc628b5a5e 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -158,6 +158,10 @@ module.exports = React.createClass({ // check the scroll state and send out backfill requests if necessary. checkFillState: function() { + if (!this.isMounted()) { + return; + } + var sn = this._getScrollNode(); // if there is less than a screenful of messages above or below the @@ -346,6 +350,12 @@ module.exports = React.createClass({ * message panel. */ _getScrollNode: function() { + if (!this.isMounted()) { + // this shouldn't happen, but when it does, turn the NPE into + // something more meaningful. + throw new Error("ScrollPanel._getScrollNode called when unmounted"); + } + var panel = ReactDOM.findDOMNode(this.refs.geminiPanel); // If the gemini scrollbar is doing its thing, this will be a div within