From 1e05e0d6f8c86a6a0f23d593db90ec1b23e25bab Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Wed, 22 Mar 2017 11:56:10 +0000 Subject: [PATCH] Review comments --- src/client.js | 15 +++++++++++---- src/models/event-timeline-set.js | 4 +++- src/models/room.js | 11 ++++------- src/sync.js | 15 ++++++++++++++- 4 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/client.js b/src/client.js index eaafeb90a..ae0e21a6a 100644 --- a/src/client.js +++ b/src/client.js @@ -2733,10 +2733,10 @@ MatrixClient.prototype.getTurnServers = function() { /** - * High level helper method to call initialSync, emit the resulting events, - * and then start polling the eventStream for new events. To listen for these + * High level helper method to begin syncing and poll for new events. To listen for these * events, add a listener for {@link module:client~MatrixClient#event:"event"} - * via {@link module:client~MatrixClient#on}. + * via {@link module:client~MatrixClient#on}. Alternatively, listen for specific + * state change events. * @param {Object=} opts Options to apply when syncing. * @param {Number=} opts.initialSyncLimit The event limit= to apply * to initial sync. Default: 8. @@ -2753,11 +2753,18 @@ MatrixClient.prototype.getTurnServers = function() { * accessbile via {@link module:models/room#getPendingEvents}. Default: * "chronological". * - * @param {Number=} opts.pollTimeout The number of milliseconds to wait on /events. + * @param {Number=} opts.pollTimeout The number of milliseconds to wait on /sync. * Default: 30000 (30 seconds). * * @param {Filter=} opts.filter The filter to apply to /sync calls. This will override * the opts.initialSyncLimit, which would normally result in a timeline limit filter. + * + * @param {Function=} opts.canResetEntireTimeline A function which is called when /sync + * returns a 'limited' response. It is called with a room ID and returns a boolean. + * It should return 'true' if the SDK can SAFELY remove events from this room. It may + * not be safe to remove events if there are other references to the timelines for this + * room, e.g because the client is actively viewing events in this room. + * Default: returns false. */ MatrixClient.prototype.startClient = function(opts) { if (this.clientRunning) { diff --git a/src/models/event-timeline-set.js b/src/models/event-timeline-set.js index 927049e13..7fba58c1f 100644 --- a/src/models/event-timeline-set.js +++ b/src/models/event-timeline-set.js @@ -187,7 +187,7 @@ EventTimelineSet.prototype.resetLiveTimeline = function(backPaginationToken, flu newTimeline.setPaginationToken(backPaginationToken, EventTimeline.BACKWARDS); this._liveTimeline = newTimeline; - this.emit("Room.timelineReset", this.room, this); + this.emit("Room.timelineReset", this.room, this, flush); }; /** @@ -655,4 +655,6 @@ module.exports = EventTimelineSet; * @event module:client~MatrixClient#"Room.timelineReset" * @param {Room} room The room whose live timeline was reset, if any * @param {EventTimelineSet} timelineSet timelineSet room whose live timeline was reset + * @param {boolean} resetAll The return value for canResetEntireTimeline() for this room. + * If true, ALL timeline sets for this room will be reset. */ diff --git a/src/models/room.js b/src/models/room.js index 93bebf004..2447268a0 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -204,15 +204,12 @@ Room.prototype.getLiveTimeline = function() { *

This is used when /sync returns a 'limited' timeline. * * @param {string=} backPaginationToken token for back-paginating the new timeline + * @param {boolean=} flush True to remove all events in all timelines. If false, only + * the live timeline is reset. */ -Room.prototype.resetLiveTimeline = function(backPaginationToken) { +Room.prototype.resetLiveTimeline = function(backPaginationToken, flush) { for (let i = 0; i < this._timelineSets.length; i++) { - // Flush out non-live timelines. We do this to reduce the amount of memory - // used, as storing multiple distinct copies of room state (each of which - // can be 10s of MBs) for each timeline is expensive. This is particularly - // noticeable when the client unsleeps and there are a lot of 'limited: true' - // responses. https://github.com/vector-im/riot-web/issues/3307#issuecomment-282895568 - this._timelineSets[i].resetLiveTimeline(backPaginationToken, true); + this._timelineSets[i].resetLiveTimeline(backPaginationToken, flush); } this._fixUpLegacyTimelineFields(); diff --git a/src/sync.js b/src/sync.js index 8eef2fbf8..94e99e5e0 100644 --- a/src/sync.js +++ b/src/sync.js @@ -63,6 +63,11 @@ function debuglog() { * @param {SyncAccumulator=} opts.syncAccumulator An accumulator which will be * kept up-to-date. If one is supplied, the response to getJSON() will be used * initially. + * @param {Function=} opts.canResetEntireTimeline A function which is called + * with a room ID and returns a boolean. It should return 'true' if the SDK can + * SAFELY remove events from this room. It may not be safe to remove events if + * there are other references to the timelines for this room. + * Default: returns false. */ function SyncApi(client, opts) { this.client = client; @@ -73,6 +78,11 @@ function SyncApi(client, opts) { opts.resolveInvitesToProfiles = opts.resolveInvitesToProfiles || false; opts.pollTimeout = opts.pollTimeout || (30 * 1000); opts.pendingEventOrdering = opts.pendingEventOrdering || "chronological"; + if (!opts.canResetEntireTimeline) { + opts.canResetEntireTimeline = function(roomId) { + return false; + }; + } this.opts = opts; this._peekRoomId = null; this._currentSyncRequest = null; @@ -848,7 +858,10 @@ SyncApi.prototype._processSyncResponse = function(syncToken, data) { // timeline. room.currentState.paginationToken = syncToken; self._deregisterStateListeners(room); - room.resetLiveTimeline(joinObj.timeline.prev_batch); + room.resetLiveTimeline( + joinObj.timeline.prev_batch, + self.opts.canResetEntireTimeline(room.roomId) + ); // We have to assume any gap in any timeline is // reason to stop incrementally tracking notifications and