Merge pull request #612 from matrix-org/dbkr/dont_clone_events

Stop cloning events when adding to state

spec/unit/room.spec.js

@@ -383,22 +383,25 @@ describe("Room", function() {
     });
 
     const resetTimelineTests = function(timelineSupport) {
-        const events = [
-            utils.mkMessage({
-                room: roomId, user: userA, msg: "A message", event: true,
-            }),
-            utils.mkEvent({
-                type: "m.room.name", room: roomId, user: userA, event: true,
-                content: { name: "New Room Name" },
-            }),
-            utils.mkEvent({
-                type: "m.room.name", room: roomId, user: userA, event: true,
-                content: { name: "Another New Name" },
-            }),
-        ];
+        let events = null;
 
         beforeEach(function() {
             room = new Room(roomId, {timelineSupport: timelineSupport});
+            // set events each time to avoid resusing Event objects (which
+            // doesn't work because they get frozen)
+            events = [
+                utils.mkMessage({
+                    room: roomId, user: userA, msg: "A message", event: true,
+                }),
+                utils.mkEvent({
+                    type: "m.room.name", room: roomId, user: userA, event: true,
+                    content: { name: "New Room Name" },
+                }),
+                utils.mkEvent({
+                    type: "m.room.name", room: roomId, user: userA, event: true,
+                    content: { name: "Another New Name" },
+                }),
+            ];
         });
 
         it("should copy state from previous timeline", function() {

src/models/event-timeline.js

@@ -20,8 +20,6 @@ limitations under the License.
  */
 
 const RoomState = require("./room-state");
-const utils = require("../utils");
-const MatrixEvent = require("./event").MatrixEvent;
 
 /**
  * Construct a new EventTimeline
@@ -88,19 +86,23 @@ EventTimeline.prototype.initialiseState = function(stateEvents) {
         throw new Error("Cannot initialise state after events are added");
     }
 
-    // we deep-copy the events here, in case they get changed later - we don't
-    // want changes to the start state leaking through to the end state.
-    const oldStateEvents = utils.map(
-        utils.deepCopy(
-            stateEvents.map(function(mxEvent) {
-                return mxEvent.event;
-            }),
-        ),
-    function(ev) {
-        return new MatrixEvent(ev);
-    });
-
-    this._startState.setStateEvents(oldStateEvents);
+    // We previously deep copied events here and used different copies in
+    // the oldState and state events: this decision seems to date back
+    // quite a way and was apparently made to fix a bug where modifications
+    // made to the start state leaked through to the end state.
+    // This really shouldn't be possible though: the events themselves should
+    // not change. Duplicating the events uses a lot of extra memory,
+    // so we now no longer do it. To assert that they really do never change,
+    // freeze them! Note that we can't do this for events in general:
+    // although it looks like the only things preventing us are the
+    // 'status' flag, forwardLooking (which is only set once when adding to the
+    // timeline) and possibly the sender (which seems like it should never be
+    // reset but in practice causes a lot of the tests to break).
+    for (const e of stateEvents) {
+        Object.freeze(e);
+    }
+
+    this._startState.setStateEvents(stateEvents);
     this._endState.setStateEvents(stateEvents);
 };