Revert "Revert "Lazy loading: don't block on setting up room crypto""

This reverts commit 5cf2ebea4f.

spec/unit/room.spec.js

@@ -1318,6 +1318,9 @@ describe("Room", function() {
                     // events should already be MatrixEvents
                     return function(event) {return event;};
                 },
+                isRoomEncrypted: function() {
+                    return false;
+                },
                 _http: {
                     serverResponse,
                     authedRequest: function() {

src/crypto/RoomList.js

@@ -71,6 +71,9 @@ export default class RoomList {
     }
 
     async setRoomEncryption(roomId, roomInfo) {
+        // important that this happens before calling into the store
+        // as it prevents the Crypto::setRoomEncryption for calling
+        // this twice for consecutive m.room.encryption events
         this._roomEncryption[roomId] = roomInfo;
         await this._cryptoStore.doTxn(
             'readwrite', [IndexedDBCryptoStore.STORE_ROOMS], (txn) => {

src/crypto/index.js

@@ -106,6 +106,10 @@ function Crypto(baseApis, sessionStore, userId, deviceId,
     this._receivedRoomKeyRequestCancellations = [];
     // true if we are currently processing received room key requests
     this._processingRoomKeyRequests = false;
+    // track if an initial tracking of all the room members
+    // has happened for a given room. This is delayed
+    // to avoid loading room members as long as possible.
+    this._roomDeviceTrackingState = {};
 }
 utils.inherits(Crypto, EventEmitter);
 
@@ -612,9 +616,6 @@ Crypto.prototype.getEventSenderDeviceInfo = function(event) {
  * @param {string} roomId The room ID to enable encryption in.
  *
  * @param {object} config The encryption config for the room.
- *
- * @param {boolean=} inhibitDeviceQuery true to suppress device list query for
- *   users in the room (for now)
  */
 Crypto.prototype.setRoomEncryption = async function(roomId, config, inhibitDeviceQuery) {
     // if state is being replayed from storage, we might already have a configuration
@@ -670,25 +671,42 @@ Crypto.prototype.setRoomEncryption = async function(roomId, config, inhibitDevic
     if (storeConfigPromise) {
         await storeConfigPromise;
     }
-
+    console.log("Enabling encryption in " + roomId);
-    // make sure we are tracking the device lists for all users in this room.
-    console.log("Enabling encryption in " + roomId + "; " +
-                "starting to track device lists for all users therein");
-    const room = this._clientStore.getRoom(roomId);
-    if (!room) {
-        throw new Error(`Unable to enable encryption in unknown room ${roomId}`);
-    }
-
-    const members = await room.getEncryptionTargetMembers();
-    members.forEach((m) => {
-        this._deviceList.startTrackingDeviceList(m.userId);
-    });
-    if (!inhibitDeviceQuery) {
-        this._deviceList.refreshOutdatedDeviceLists();
-    }
 };
 
 
+/**
+ * Make sure we are tracking the device lists for all users in this room.
+ *
+ * @param {string} roomId The room ID to start tracking devices in.
+ * @returns {Promise} when all devices for the room have been fetched and marked to track
+ */
+Crypto.prototype.trackRoomDevices = function(roomId) {
+    const trackAndRefresh = async () => {
+        // not an encrypted room
+        if (!this._roomEncryptors[roomId]) {
+            return;
+        }
+        const room = this._clientStore.getRoom(roomId);
+        if (!room) {
+            throw new Error(`Unable to start tracking devices in unknown room ${roomId}`);
+        }
+        console.log(`Starting to track devices for room ${roomId} ...`);
+        const members = await room.getEncryptionTargetMembers();
+        members.forEach((m) => {
+            this._deviceList.startTrackingDeviceList(m.userId);
+        });
+        return this._deviceList.refreshOutdatedDeviceLists();
+    };
+
+    let promise = this._roomDeviceTrackingState[roomId];
+    if (!promise) {
+        promise = trackAndRefresh();
+        this._roomDeviceTrackingState[roomId] = promise;
+    }
+    return promise;
+};
+
 /**
  * @typedef {Object} module:crypto~OlmSessionResult
  * @property {module:crypto/deviceinfo} device  device info
@@ -779,7 +797,7 @@ Crypto.prototype.importRoomKeys = function(keys) {
         },
     );
 };
-
+/* eslint-disable valid-jsdoc */    //https://github.com/eslint/eslint/issues/7307
 /**
  * Encrypt an event according to the configuration of the room.
  *
@@ -790,7 +808,8 @@ Crypto.prototype.importRoomKeys = function(keys) {
  * @return {module:client.Promise?} Promise which resolves when the event has been
  *     encrypted, or null if nothing was needed
  */
-Crypto.prototype.encryptEvent = function(event, room) {
+/* eslint-enable valid-jsdoc */
+Crypto.prototype.encryptEvent = async function(event, room) {
     if (!room) {
         throw new Error("Cannot send encrypted messages in unknown rooms");
     }
@@ -808,6 +827,12 @@ Crypto.prototype.encryptEvent = function(event, room) {
         );
     }
 
+    if (!this._roomDeviceTrackingState[roomId]) {
+        this.trackRoomDevices(roomId);
+    }
+    // wait for all the room devices to be loaded
+    await this._roomDeviceTrackingState[roomId];
+
     let content = event.getContent();
     // If event has an m.relates_to then we need
     // to put this on the wrapping event instead
@@ -818,20 +843,19 @@ Crypto.prototype.encryptEvent = function(event, room) {
         delete content['m.relates_to'];
     }
 
-    return alg.encryptMessage(
+    const encryptedContent = await alg.encryptMessage(
-        room, event.getType(), content,
+        room, event.getType(), content);
-    ).then((encryptedContent) => {
-        if (mRelatesTo) {
-            encryptedContent['m.relates_to'] = mRelatesTo;
-        }
 
-        event.makeEncrypted(
+    if (mRelatesTo) {
-            "m.room.encrypted",
+        encryptedContent['m.relates_to'] = mRelatesTo;
-            encryptedContent,
+    }
-            this._olmDevice.deviceCurve25519Key,
+
-            this._olmDevice.deviceEd25519Key,
+    event.makeEncrypted(
-        );
+        "m.room.encrypted",
-    });
+        encryptedContent,
+        this._olmDevice.deviceCurve25519Key,
+        this._olmDevice.deviceEd25519Key,
+    );
 };
 
 /**
@@ -924,9 +948,7 @@ Crypto.prototype.onCryptoEvent = async function(event) {
     const content = event.getContent();
 
     try {
-        // inhibit the device list refresh for now - it will happen once we've
+        await this.setRoomEncryption(roomId, content);
-        // finished processing the sync, in onSyncCompleted.
-        await this.setRoomEncryption(roomId, content, true);
     } catch (e) {
         console.error("Error configuring encryption in room " + roomId +
                       ":", e);
@@ -946,6 +968,7 @@ Crypto.prototype.onSyncWillProcess = async function(syncData) {
         // at which point we'll start tracking all the users of that room.
         console.log("Initial sync performed - resetting device tracking state");
         this._deviceList.stopTrackingAllDeviceLists();
+        this._roomDeviceTrackingState = {};
     }
 };
 
@@ -991,11 +1014,12 @@ Crypto.prototype._evalDeviceListChanges = async function(deviceLists) {
         });
     }
 
-    if (deviceLists.left && Array.isArray(deviceLists.left)) {
+    if (deviceLists.left && Array.isArray(deviceLists.left) &&
+        deviceLists.left.length) {
         // Check we really don't share any rooms with these users
         // any more: the server isn't required to give us the
         // exact correct set.
-        const e2eUserIds = new Set(await this._getE2eUsers());
+        const e2eUserIds = new Set(await this._getTrackedE2eUsers());
 
         deviceLists.left.forEach((u) => {
             if (!e2eUserIds.has(u)) {
@@ -1007,12 +1031,13 @@ Crypto.prototype._evalDeviceListChanges = async function(deviceLists) {
 
 /**
  * Get a list of all the IDs of users we share an e2e room with
+ * for which we are tracking devices already
  *
  * @returns {string[]} List of user IDs
  */
-Crypto.prototype._getE2eUsers = async function() {
+Crypto.prototype._getTrackedE2eUsers = async function() {
     const e2eUserIds = [];
-    for (const room of this._getE2eRooms()) {
+    for (const room of this._getTrackedE2eRooms()) {
         const members = await room.getEncryptionTargetMembers();
         for (const member of members) {
             e2eUserIds.push(member.userId);
@@ -1022,17 +1047,21 @@ Crypto.prototype._getE2eUsers = async function() {
 };
 
 /**
- * Get a list of the e2e-enabled rooms we are members of
+ * Get a list of the e2e-enabled rooms we are members of,
+ * and for which we are already tracking the devices
  *
  * @returns {module:models.Room[]}
  */
-Crypto.prototype._getE2eRooms = function() {
+Crypto.prototype._getTrackedE2eRooms = function() {
     return this._clientStore.getRooms().filter((room) => {
         // check for rooms with encryption enabled
         const alg = this._roomEncryptors[room.roomId];
         if (!alg) {
             return false;
         }
+        if (!this._roomDeviceTrackingState[room.roomId]) {
+            return false;
+        }
 
         // ignore any rooms which we have left
         const myMembership = room.getMyMembership();
@@ -1101,15 +1130,20 @@ Crypto.prototype._onRoomMembership = function(event, member, oldMembership) {
         // not encrypting in this room
         return;
     }
-
+    // only mark users in this room as tracked if we already started tracking in this room
-    if (member.membership == 'join') {
+    // this way we don't start device queries after sync on behalf of this room which we won't use
-        console.log('Join event for ' + member.userId + ' in ' + roomId);
+    // the result of anyway, as we'll need to do a query again once all the members are fetched
-        // make sure we are tracking the deviceList for this user
+    // by calling _trackRoomDevices
-        this._deviceList.startTrackingDeviceList(member.userId);
+    if (this._roomDeviceTrackingState[roomId]) {
-    } else if (member.membership == 'invite' &&
+        if (member.membership == 'join') {
-             this._clientStore.getRoom(roomId).shouldEncryptForInvitedMembers()) {
+            console.log('Join event for ' + member.userId + ' in ' + roomId);
-        console.log('Invite event for ' + member.userId + ' in ' + roomId);
+            // make sure we are tracking the deviceList for this user
-        this._deviceList.startTrackingDeviceList(member.userId);
+            this._deviceList.startTrackingDeviceList(member.userId);
+        } else if (member.membership == 'invite' &&
+                 this._clientStore.getRoom(roomId).shouldEncryptForInvitedMembers()) {
+            console.log('Invite event for ' + member.userId + ' in ' + roomId);
+            this._deviceList.startTrackingDeviceList(member.userId);
+        }
     }
 
     alg.onRoomMembership(event, member, oldMembership);

src/models/room.js

@@ -368,6 +368,11 @@ Room.prototype.loadMembersIfNeeded = function() {
     });
 
     this._membersPromise = promise;
+    // now the members are loaded, start to track the e2e devices if needed
+    if (this._client.isRoomEncrypted(this.roomId)) {
+        this._client._crypto.trackRoomDevices(this.roomId);
+    }
+
     return this._membersPromise;
 };
 

src/sync.js

@@ -1085,15 +1085,16 @@ SyncApi.prototype._processSyncResponse = async function(
 
         self._processEventsForNotifs(room, timelineEvents);
 
-        async function processRoomEvent(e) {
+        function processRoomEvent(e) {
             client.emit("event", e);
             if (e.isState() && e.getType() == "m.room.encryption" && self.opts.crypto) {
-                await self.opts.crypto.onCryptoEvent(e);
+                self.opts.crypto.onCryptoEvent(e);
             }
         }
 
-        await Promise.mapSeries(stateEvents, processRoomEvent);
+        stateEvents.forEach(processRoomEvent);
-        await Promise.mapSeries(timelineEvents, processRoomEvent);
+        timelineEvents.forEach(processRoomEvent);
+
         ephemeralEvents.forEach(function(e) {
             client.emit("event", e);
         });