Merge pull request #504 from matrix-org/dbkr/fix_member_events_timeline_reset_2

Fix member events breaking on timeline reset, 2

spec/unit/room.spec.js

@@ -404,7 +404,7 @@ describe("Room", function() {
         it("should copy state from previous timeline", function() {
             room.addLiveEvents([events[0], events[1]]);
             expect(room.getLiveTimeline().getEvents().length).toEqual(2);
-            room.resetLiveTimeline();
+            room.resetLiveTimeline('sometoken', 'someothertoken');
 
             room.addLiveEvents([events[2]]);
             const oldState = room.getLiveTimeline().getState(EventTimeline.BACKWARDS);
@@ -417,7 +417,7 @@ describe("Room", function() {
         it("should reset the legacy timeline fields", function() {
             room.addLiveEvents([events[0], events[1]]);
             expect(room.timeline.length).toEqual(2);
-            room.resetLiveTimeline();
+            room.resetLiveTimeline('sometoken', 'someothertoken');
 
             room.addLiveEvents([events[2]]);
             const newLiveTimeline = room.getLiveTimeline();
@@ -451,7 +451,7 @@ describe("Room", function() {
             room.addLiveEvents([events[0]]);
             expect(room.timeline.length).toEqual(1);
             const firstLiveTimeline = room.getLiveTimeline();
-            room.resetLiveTimeline();
+            room.resetLiveTimeline('sometoken', 'someothertoken');
 
             const tl = room.getTimelineForEvent(events[0].getId());
             expect(tl).toBe(timelineSupport ? firstLiveTimeline : null);

src/client.js

@@ -2135,7 +2135,7 @@ MatrixClient.prototype.resetNotifTimelineSet = function() {
     // know about /notifications, so we have no choice but to start paginating
     // from the current point in time.  This may well overlap with historical
     // notifs which are then inserted into the timeline by /sync responses.
-    this._notifTimelineSet.resetLiveTimeline('end', true);
+    this._notifTimelineSet.resetLiveTimeline('end', null);
 
     // we could try to paginate a single event at this point in order to get
     // a more valid pagination token, but it just ends up with an out of order

src/models/event-timeline-set.js

@@ -149,13 +149,24 @@ EventTimelineSet.prototype.replaceEventId = function(oldEventId, newEventId) {
  * <p>This is used when /sync returns a 'limited' timeline.
  *
  * @param {string=} backPaginationToken   token for back-paginating the new timeline
- * @param {?bool} flush  Whether to flush the non-live timelines too.
+ * @param {string=} forwardPaginationToken token for forward-paginating the old live timeline,
+ * if absent or null, all timelines are reset.
  *
  * @fires module:client~MatrixClient#event:"Room.timelineReset"
  */
-EventTimelineSet.prototype.resetLiveTimeline = function(backPaginationToken, flush) {
+EventTimelineSet.prototype.resetLiveTimeline = function(
+    backPaginationToken, forwardPaginationToken,
+) {
+    // Each EventTimeline has RoomState objects tracking the state at the start
+    // and end of that timeline. The copies at the end of the live timeline are
+    // special because they will have listeners attached to monitor changes to
+    // the current room state, so we move this RoomState from the end of the
+    // current live timeline to the end of the new one and, if necessary,
+    // replace it with a newly created one. We also make a copy for the start
+    // of the new timeline.
+
     // if timeline support is disabled, forget about the old timelines
-    const resetAllTimelines = !this._timelineSupport || flush;
+    const resetAllTimelines = !this._timelineSupport || !forwardPaginationToken;
 
     let newTimeline;
     if (resetAllTimelines) {
@@ -166,8 +177,10 @@ EventTimelineSet.prototype.resetLiveTimeline = function(backPaginationToken, flu
         newTimeline = this.addTimeline();
     }
 
-    // initialise the state in the new timeline from our last known state
+    const oldTimeline = this._liveTimeline;
-    const evMap = this._liveTimeline.getState(EventTimeline.FORWARDS).events;
+
+    // Collect the state events from the old timeline
+    const evMap = oldTimeline.getState(EventTimeline.FORWARDS).events;
     const events = [];
     for (const evtype in evMap) {
         if (!evMap.hasOwnProperty(evtype)) {
@@ -180,13 +193,37 @@ EventTimelineSet.prototype.resetLiveTimeline = function(backPaginationToken, flu
             events.push(evMap[evtype][stateKey]);
         }
     }
+
+    // Use those events to initialise the state of the new live timeline
     newTimeline.initialiseState(events);
 
+    const freshEndState = newTimeline._endState;
+    // Now clobber the end state of the new live timeline with that from the
+    // previous live timeline. It will be identical except that we'll keep
+    // using the same RoomMember objects for the 'live' set of members with any
+    // listeners still attached
+    newTimeline._endState = oldTimeline._endState;
+
+    // If we're not resetting all timelines, we need to fix up the old live timeline
+    if (!resetAllTimelines) {
+        // Firstly, we just stole the old timeline's end state, so it needs a new one.
+        // Just swap them around and give it the one we just generated for the
+        // new live timeline.
+        oldTimeline._endState = freshEndState;
+
+        // Now set the forward pagination token on the old live timeline
+        // so it can be forward-paginated.
+        oldTimeline.setPaginationToken(
+            forwardPaginationToken, EventTimeline.FORWARDS,
+        );
+    }
+
     // make sure we set the pagination token before firing timelineReset,
     // otherwise clients which start back-paginating will fail, and then get
     // stuck without realising that they *can* back-paginate.
     newTimeline.setPaginationToken(backPaginationToken, EventTimeline.BACKWARDS);
 
+    // Now we can swap the live timeline to the new one.
     this._liveTimeline = newTimeline;
     this.emit("Room.timelineReset", this.room, this, resetAllTimelines);
 };

src/models/room.js

@@ -205,12 +205,16 @@ Room.prototype.getLiveTimeline = function() {
  * <p>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
+ * @param {string=} forwardPaginationToken token for forward-paginating the old live timeline,
- * the live timeline is reset.
+ * if absent or null, all timelines are reset, removing old ones (including the previous live
+ * timeline which would otherwise be unable to paginate forwards without this token).
+ * Removing just the old live timeline whilst preserving previous ones is not supported.
  */
-Room.prototype.resetLiveTimeline = function(backPaginationToken, flush) {
+Room.prototype.resetLiveTimeline = function(backPaginationToken, forwardPaginationToken) {
     for (let i = 0; i < this._timelineSets.length; i++) {
-        this._timelineSets[i].resetLiveTimeline(backPaginationToken, flush);
+        this._timelineSets[i].resetLiveTimeline(
+            backPaginationToken, forwardPaginationToken,
+        );
     }
 
     this._fixUpLegacyTimelineFields();

src/sync.js

@@ -885,14 +885,10 @@ SyncApi.prototype._processSyncResponse = function(syncToken, data) {
             }
 
             if (limited) {
-                // save the old 'next_batch' token as the
-                // forward-pagination token for the previously-active
-                // timeline.
-                room.currentState.paginationToken = syncToken;
                 self._deregisterStateListeners(room);
                 room.resetLiveTimeline(
                     joinObj.timeline.prev_batch,
-                    self.opts.canResetEntireTimeline(room.roomId),
+                    self.opts.canResetEntireTimeline(room.roomId) ? null : syncToken,
                 );
 
                 // We have to assume any gap in any timeline is