Merge branch 'develop' into kegan/indexeddb

jenkins.sh

@@ -16,7 +16,7 @@ function fail {
 }
 
 # don't use last time's test reports
-rm -rf reports || exit $?
+rm -rf reports coverage || exit $?
 
 npm test || fail "npm test finished with return code $?"
 

package.json

@@ -5,9 +5,9 @@
   "main": "index.js",
   "scripts": {
     "test:build": "babel -s -d specbuild spec",
-    "test:run": "mocha --recursive specbuild --colors --reporter mocha-jenkins-reporter --reporter-options junit_report_path=reports/test-results.xml",
-    "test": "npm run test:build && istanbul cover --report cobertura --config .istanbul.yml -i \"lib/**/*.js\" npm run test:run",
-    "check": "npm run test:build && npm run test:run",
+    "test:run": "istanbul cover --report text --report cobertura --config .istanbul.yml -i \"lib/**/*.js\" _mocha -- --recursive specbuild --colors --reporter mocha-jenkins-reporter --reporter-options junit_report_path=reports/test-results.xml",
+    "test": "npm run test:build && npm run test:run",
+    "check": "npm run test:build && _mocha --recursive specbuild --colors",
     "gendoc": "jsdoc -r lib -P package.json -R README.md -d .jsdoc",
     "start": "babel -s -w -d lib src",
     "build": "babel -s -d lib src && rimraf dist && mkdir dist && browserify -d browser-index.js | exorcist dist/browser-matrix.js.map > dist/browser-matrix.js && uglifyjs -c -m -o dist/browser-matrix.min.js --source-map dist/browser-matrix.min.js.map --in-source-map dist/browser-matrix.js.map dist/browser-matrix.js",

spec/integ/matrix-client-crypto.spec.js

@@ -130,7 +130,12 @@ function expectAliClaimKeys() {
         return {one_time_keys: result};
     });
 
-    return aliTestClient.httpBackend.flush("/keys/claim", 1);
+    // it can take a while to process the key query, so give it some extra
+    // time, and make sure the claim actually happens rather than ploughing on
+    // confusingly.
+    return aliTestClient.httpBackend.flush("/keys/claim", 1, 20).then((r) => {
+        expect(r).toEqual(1);
+    });
 }
 
 
@@ -263,16 +268,16 @@ function sendMessage(client) {
 
 function expectSendMessageRequest(httpBackend) {
     const path = "/send/m.room.encrypted/";
-    let sent;
+    const deferred = q.defer();
     httpBackend.when("PUT", path).respond(200, function(path, content) {
-        sent = content;
+        deferred.resolve(content);
         return {
             event_id: "asdfgh",
         };
     });
-    return httpBackend.flush(path, 1).then(function() {
-        return sent;
-    });
+
+    // it can take a while to process the key query, so give it 20ms
+    return httpBackend.flush(path, 1, 20).then(() => deferred.promise);
 }
 
 function aliRecvMessage() {

spec/integ/matrix-client-room-timeline.spec.js

@@ -123,7 +123,7 @@ describe("MatrixClient room timelines", function() {
         client.startClient();
         httpBackend.flush("/pushrules").then(function() {
             return httpBackend.flush("/filter");
-        }).done(done);
+        }).nodeify(done);
     });
 
     afterEach(function() {

spec/integ/megolm.spec.js

@@ -771,6 +771,20 @@ describe("megolm", function() {
 
             return aliceTestClient.httpBackend.flush();
         }).then(function() {
+            // start out with the device unknown - the send should be rejected.
+            return q.all([
+                aliceTestClient.client.sendTextMessage(ROOM_ID, 'test').then(() => {
+                    throw new Error("sendTextMessage failed on an unknown device");
+                }, (e) => {
+                    expect(e.name).toEqual("UnknownDeviceError");
+                    expect(Object.keys(e.devices)).toEqual([aliceTestClient.userId]);
+                    expect(Object.keys(e.devices[aliceTestClient.userId])).
+                        toEqual(['DEVICE_ID']);
+                }),
+                aliceTestClient.httpBackend.flush(),
+            ]);
+        }).then(function() {
+            // mark the device as known, and resend.
             aliceTestClient.client.setDeviceKnown(aliceTestClient.userId, 'DEVICE_ID');
             aliceTestClient.httpBackend.when('POST', '/keys/claim').respond(
                 200, function(path, content) {
@@ -900,6 +914,144 @@ describe("megolm", function() {
     });
 
 
+    it("We should not get confused by out-of-order device query responses",
+       () => {
+           // https://github.com/vector-im/riot-web/issues/3126
+           return aliceTestClient.start().then(() => {
+               aliceTestClient.httpBackend.when('GET', '/sync').respond(
+                   200, getSyncResponse(['@bob:xyz', '@chris:abc']));
+               return aliceTestClient.httpBackend.flush('/sync', 1);
+           }).then(() => {
+               // to make sure the initial device queries are flushed out, we
+               // attempt to send a message.
+
+               aliceTestClient.httpBackend.when('POST', '/keys/query').respond(
+                   200, {
+                       device_keys: {
+                           '@bob:xyz': {},
+                           '@chris:abc': {},
+                       },
+                   },
+               );
+
+               aliceTestClient.httpBackend.when('PUT', '/send/').respond(
+                   200, {event_id: '$event1'});
+
+               return q.all([
+                   aliceTestClient.client.sendTextMessage(ROOM_ID, 'test'),
+                   aliceTestClient.httpBackend.flush('/keys/query', 1).then(
+                       () => aliceTestClient.httpBackend.flush('/send/', 1, 20),
+                   ),
+               ]);
+           }).then(() => {
+               expect(aliceTestClient.storage.getEndToEndDeviceSyncToken()).toEqual(1);
+
+               // invalidate bob's and chris's device lists in separate syncs
+               aliceTestClient.httpBackend.when('GET', '/sync').respond(200, {
+                   next_batch: '2',
+                   device_lists: {
+                       changed: ['@bob:xyz'],
+                   },
+               });
+               aliceTestClient.httpBackend.when('GET', '/sync').respond(200, {
+                   next_batch: '3',
+                   device_lists: {
+                       changed: ['@chris:abc'],
+                   },
+               });
+               return aliceTestClient.httpBackend.flush('/sync', 2);
+           }).then(() => {
+               // check that we don't yet have a request for chris's devices.
+               aliceTestClient.httpBackend.when('POST', '/keys/query', {
+                   device_keys: {
+                       '@chris:abc': {},
+                   },
+                   token: '3',
+               }).respond(200, {
+                   device_keys: {'@chris:abc': {}},
+               });
+               return aliceTestClient.httpBackend.flush('/keys/query', 1);
+           }).then((flushed) => {
+               expect(flushed).toEqual(0);
+               expect(aliceTestClient.storage.getEndToEndDeviceSyncToken()).toEqual(1);
+
+               // now add an expectation for a query for bob's devices, and let
+               // it complete.
+               aliceTestClient.httpBackend.when('POST', '/keys/query', {
+                   device_keys: {
+                       '@bob:xyz': {},
+                   },
+                   token: '2',
+               }).respond(200, {
+                   device_keys: {'@bob:xyz': {}},
+               });
+               return aliceTestClient.httpBackend.flush('/keys/query', 1);
+           }).then((flushed) => {
+               expect(flushed).toEqual(1);
+
+               // wait for the client to stop processing the response
+               return aliceTestClient.client.downloadKeys(['@bob:xyz']);
+           }).then(() => {
+               expect(aliceTestClient.storage.getEndToEndDeviceSyncToken()).toEqual(2);
+
+               // now let the query for chris's devices complete.
+               return aliceTestClient.httpBackend.flush('/keys/query', 1);
+           }).then((flushed) => {
+               expect(flushed).toEqual(1);
+
+               // wait for the client to stop processing the response
+               return aliceTestClient.client.downloadKeys(['@chris:abc']);
+           }).then(() => {
+               expect(aliceTestClient.storage.getEndToEndDeviceSyncToken()).toEqual(3);
+           });
+       });
+
+    it("Device list downloads before /changes shouldn't affect sync token",
+        () => {
+            // https://github.com/vector-im/riot-web/issues/3126#issuecomment-279374939
+            aliceTestClient.storage.storeEndToEndDeviceSyncToken(0);
+            aliceTestClient.storage.storeEndToEndRoom(ROOM_ID, {
+                algorithm: 'm.megolm.v1.aes-sha2',
+            });
+
+            return aliceTestClient.start().then(() => {
+               aliceTestClient.httpBackend.when('GET', '/sync').respond(
+                   200, getSyncResponse([aliceTestClient.userId, '@bob:xyz']));
+               return aliceTestClient.httpBackend.flush('/sync', 1);
+            }).then(() => {
+                aliceTestClient.httpBackend.when('POST', '/keys/query').respond(
+                    200, {device_keys: {'@bob:xyz': {}}},
+                );
+                return q.all([
+                    aliceTestClient.client.downloadKeys(['@bob:xyz']),
+                    aliceTestClient.httpBackend.flush('/keys/query', 1),
+                ]);
+           }).then(() => {
+               expect(aliceTestClient.storage.getEndToEndDeviceSyncToken()).toEqual(0);
+
+               aliceTestClient.httpBackend.when(
+                   'GET', '/keys/changes',
+               ).check((req) => {
+                   expect(req.queryParams.from).toEqual(0);
+                   expect(req.queryParams.to).toEqual(1);
+               }).respond(200, {changed: ['@bob:xyz']});
+
+               return aliceTestClient.httpBackend.flush('/keys/changes');
+           }).then((flushed) => {
+               aliceTestClient.httpBackend.when('POST', '/keys/query').respond(
+                   200, {device_keys: {'@bob:xyz': {}}},
+               );
+               return aliceTestClient.httpBackend.flush('/keys/query');
+           }).then((flushed) => {
+               expect(flushed).toEqual(1);
+
+               // let the client finish processing the keys
+               return q.delay(10);
+           }).then(() => {
+               expect(aliceTestClient.storage.getEndToEndDeviceSyncToken()).toEqual(1);
+           });
+        });
+
     it("Alice exports megolm keys and imports them to a new device", function(done) {
         let messageEncrypted;
 

spec/mock-request.js

@@ -36,15 +36,23 @@ HttpBackend.prototype = {
      * Respond to all of the requests (flush the queue).
      * @param {string} path The path to flush (optional) default: all.
      * @param {integer} numToFlush The number of things to flush (optional), default: all.
-     * @return {Promise} resolved when there is nothing left to flush.
+     * @param {integer=} waitTime  The time (in ms) to wait for a request to happen.
+     *    default: 5
+     *
+     * @return {Promise} resolves when there is nothing left to flush, with the
+     *    number of requests flushed
      */
-    flush: function(path, numToFlush) {
+    flush: function(path, numToFlush, waitTime) {
         const defer = q.defer();
         const self = this;
         let flushed = 0;
         let triedWaiting = false;
+        if (waitTime === undefined) {
+            waitTime = 5;
+        }
         console.log(
-            "HTTP backend flushing... (path=%s  numToFlush=%s)", path, numToFlush,
+            "HTTP backend flushing... (path=%s numToFlush=%s waitTime=%s)",
+            path, numToFlush, waitTime,
         );
         const tryFlush = function() {
             // if there's more real requests and more expected requests, flush 'em.
@@ -57,7 +65,7 @@ HttpBackend.prototype = {
                 flushed += 1;
                 if (numToFlush && flushed === numToFlush) {
                     console.log("  Flushed assigned amount: %s", numToFlush);
-                    defer.resolve();
+                    defer.resolve(flushed);
                 } else {
                     console.log("  flushed. Trying for more.");
                     setTimeout(tryFlush, 0);
@@ -65,11 +73,11 @@ HttpBackend.prototype = {
             } else if (flushed === 0 && !triedWaiting) {
                 // we may not have made the request yet, wait a generous amount of
                 // time before giving up.
-                setTimeout(tryFlush, 5);
+                setTimeout(tryFlush, waitTime);
                 triedWaiting = true;
             } else {
                 console.log("  no more flushes.");
-                defer.resolve();
+                defer.resolve(flushed);
             }
         };
 

src/client.js

@@ -1383,13 +1383,23 @@ MatrixClient.prototype.forget = function(roomId, deleteRoom, callback) {
  * @param {string} roomId
  * @param {string} userId
  * @param {module:client.callback} callback Optional.
- * @return {module:client.Promise} Resolves: TODO
+ * @return {module:client.Promise} Resolves: Object (currently empty)
  * @return {module:http-api.MatrixError} Rejects: with an error response.
  */
 MatrixClient.prototype.unban = function(roomId, userId, callback) {
-    // unbanning = set their state to leave
-    return _setMembershipState(
-        this, roomId, userId, "leave", undefined, callback,
+    // unbanning != set their state to leave: this used to be
+    // the case, but was then changed so that leaving was always
+    // a revoking of priviledge, otherwise two people racing to
+    // kick / ban someone could end up banning and then un-banning
+    // them.
+    const path = utils.encodeUri("/rooms/$roomId/unban", {
+        $roomId: roomId,
+    });
+    const data = {
+        user_id: userId,
+    };
+    return this._http.authedRequest(
+        callback, "POST", path, undefined, data,
     );
 };
 

src/crypto/DeviceList.js

@@ -39,9 +39,22 @@ export default class DeviceList {
         // userId -> true
         this._pendingUsersWithNewDevices = {};
 
-        // userId -> promise
+        // userId -> true
         this._keyDownloadsInProgressByUser = {};
 
+        // deferred which is resolved when the current device query resolves.
+        // (null if there is no current request).
+        this._currentQueryDeferred = null;
+
+        // deferred which is resolved when the *next* device query resolves.
+        //
+        // Normally it is meaningless for this to be non-null when
+        // _currentQueryDeferred is null, but it can happen if the previous
+        // query has finished but the next one has not yet started (because the
+        // previous query failed, in which case we deliberately delay starting
+        // the next query to avoid tight-looping).
+        this._queuedQueryDeferred = null;
+
         this.lastKnownSyncToken = null;
     }
 
@@ -55,35 +68,43 @@ export default class DeviceList {
      * module:crypto/deviceinfo|DeviceInfo}.
      */
     downloadKeys(userIds, forceDownload) {
-        // promises we need to wait for while the download happens
-        const promises = [];
-
         let needsRefresh = false;
+        let waitForCurrentQuery = false;
+
         userIds.forEach((u) => {
-            if (this._keyDownloadsInProgressByUser[u]) {
-                // just wait for the existing download to complete
-                promises.push(this._keyDownloadsInProgressByUser[u]);
-            } else {
-                if (forceDownload) {
-                    console.log("Invalidating device list for " + u +
-                                " for forceDownload");
-                    this.invalidateUserDeviceList(u);
-                } else if (!this.getStoredDevicesForUser(u)) {
-                    console.log("Invalidating device list for " + u +
-                                " due to empty cache");
-                    this.invalidateUserDeviceList(u);
-                }
-                if (this._pendingUsersWithNewDevices[u]) {
-                    needsRefresh = true;
-                }
+            if (this._pendingUsersWithNewDevices[u]) {
+                // we already know this user's devices are outdated
+                needsRefresh = true;
+            } else if (this._keyDownloadsInProgressByUser[u]) {
+                // already a download in progress - just wait for it.
+                // (even if forceDownload is true)
+                waitForCurrentQuery = true;
+            } else if (forceDownload) {
+                console.log("Invalidating device list for " + u +
+                            " for forceDownload");
+                this.invalidateUserDeviceList(u);
+                needsRefresh = true;
+            } else if (!this.getStoredDevicesForUser(u)) {
+                console.log("Invalidating device list for " + u +
+                            " due to empty cache");
+                this.invalidateUserDeviceList(u);
+                needsRefresh = true;
             }
         });
 
+        let promise;
         if (needsRefresh) {
-            promises.push(this.refreshOutdatedDeviceLists(true));
+            console.log("downloadKeys: waiting for next key query");
+            promise = this._startOrQueueDeviceQuery();
+        } else if(waitForCurrentQuery) {
+            console.log("downloadKeys: waiting for in-flight query to complete");
+            promise = this._currentQueryDeferred.promise;
+        } else {
+            // we're all up-to-date.
+            promise = q();
         }
 
-        return q.all(promises).then(() => {
+        return promise.then(() => {
             return this._getDevicesFromStore(userIds);
         });
     }
@@ -217,75 +238,104 @@ export default class DeviceList {
     }
 
     /**
-     * Start device queries for any users with outdated device lists
-     *
-     * We tolerate multiple concurrent device queries, but only one query per
-     * user.
-     *
-     * If any users already have downloads in progress, they are ignored - they
-     * will be refreshed when the current download completes anyway, so
-     * each user with outdated device lists will be updated eventually.
-     *
-     * The returned promise resolves immediately if there are no users with
-     * outdated device lists, or if all users with outdated device lists already
-     * have a query in progress.
-     *
-     * Otherwise, a new query request is made, and the promise resolves
-     * once that query completes. If the query fails, the promise will reject
-     * if rejectOnFailure was truthy, otherwise it will still resolve.
-     *
-     * @param {Boolean?} rejectOnFailure  true to make the returned promise
-     *   reject if the device list query fails.
+     * If there is not already a device list query in progress, and we have
+     * users who have outdated device lists, start a query now.
+     */
+    refreshOutdatedDeviceLists() {
+        if (this._currentQueryDeferred) {
+            // request already in progress - do nothing. (We will automatically
+            // make another request if there are more users with outdated
+            // device lists when the current request completes).
+            return;
+        }
+
+        this._startDeviceQuery();
+    }
+
+    /**
+     * If there is currently a device list query in progress, returns a promise
+     * which will resolve when the *next* query completes. Otherwise, starts
+     * a new query, and returns a promise which resolves when it completes.
      *
      * @return {Promise}
      */
-    refreshOutdatedDeviceLists(rejectOnFailure) {
-        const users = Object.keys(this._pendingUsersWithNewDevices).filter(
-            (u) => !this._keyDownloadsInProgressByUser[u],
-        );
+    _startOrQueueDeviceQuery() {
+        if (!this._currentQueryDeferred) {
+            this._startDeviceQuery();
+            if (!this._currentQueryDeferred) {
+                return q();
+            }
 
-        if (users.length === 0) {
-            return q();
+            return this._currentQueryDeferred.promise;
         }
 
-        let prom = this._doKeyDownloadForUsers(users).then(() => {
+        if (!this._queuedQueryDeferred) {
+            this._queuedQueryDeferred = q.defer();
+        }
+
+        return this._queuedQueryDeferred.promise;
+    }
+
+    /**
+     * kick off a new device query
+     *
+     * Throws if there is already a query in progress.
+     */
+    _startDeviceQuery() {
+        if (this._currentQueryDeferred) {
+            throw new Error("DeviceList._startDeviceQuery called with request active");
+        }
+
+        this._currentQueryDeferred = this._queuedQueryDeferred || q.defer();
+        this._queuedQueryDeferred = null;
+
+        const users = Object.keys(this._pendingUsersWithNewDevices);
+        if (users.length === 0) {
+            // nothing to do
+            this._currentQueryDeferred.resolve();
+            this._currentQueryDeferred = null;
+
+            // that means we're up-to-date with the lastKnownSyncToken.
+            const token = this.lastKnownSyncToken;
+            if (token !== null) {
+                this._sessionStore.storeEndToEndDeviceSyncToken(token);
+            }
+
+            return;
+        }
+
+        this._doKeyDownloadForUsers(users).done(() => {
             users.forEach((u) => {
                 delete this._keyDownloadsInProgressByUser[u];
             });
 
+            this._currentQueryDeferred.resolve();
+            this._currentQueryDeferred = null;
+
             // flush out any more requests that were blocked up while that
-            // was going on, but let the initial promise complete now.
-            //
-            this.refreshOutdatedDeviceLists().done();
+            // was going on.
+            this._startDeviceQuery();
         }, (e) => {
             console.error(
                 'Error updating device key cache for ' + users + ":", e,
             );
 
             // reinstate the pending flags on any users which failed; this will
-            // mean that we will do another download in the future, but won't
-            // tight-loop.
-            //
+            // mean that we will do another download in the future (actually on
+            // the next /sync).
             users.forEach((u) => {
                 delete this._keyDownloadsInProgressByUser[u];
                 this._pendingUsersWithNewDevices[u] = true;
             });
 
-            // TODO: schedule a retry.
-            throw e;
+            this._currentQueryDeferred.reject(e);
+            this._currentQueryDeferred = null;
         });
 
         users.forEach((u) => {
             delete this._pendingUsersWithNewDevices[u];
-            this._keyDownloadsInProgressByUser[u] = prom;
+            this._keyDownloadsInProgressByUser[u] = true;
         });
-
-        if (!rejectOnFailure) {
-            // normally we just want to swallow the exception - we've already
-            // logged it futher up.
-            prom = prom.catch((e) => {});
-        }
-        return prom;
     }
 
     /**
@@ -306,41 +356,57 @@ export default class DeviceList {
         ).then((res) => {
             const dk = res.device_keys || {};
 
+            // do each user in a separate promise, to avoid wedging the CPU
+            // (https://github.com/vector-im/riot-web/issues/3158)
+            //
+            // of course we ought to do this in a web worker or similar, but
+            // this serves as an easy solution for now.
+            let prom = q();
             for (const userId of downloadUsers) {
-                console.log('got keys for ' + userId + ':', dk[userId]);
-
-                // map from deviceid -> deviceinfo for this user
-                const userStore = {};
-                const devs = this._sessionStore.getEndToEndDevicesForUser(userId);
-                if (devs) {
-                    Object.keys(devs).forEach((deviceId) => {
-                        const d = DeviceInfo.fromStorage(devs[deviceId], deviceId);
-                        userStore[deviceId] = d;
-                    });
-                }
-
-                _updateStoredDeviceKeysForUser(
-                    this._olmDevice, userId, userStore, dk[userId] || {},
-                );
-
-                // update the session store
-                const storage = {};
-                Object.keys(userStore).forEach((deviceId) => {
-                    storage[deviceId] = userStore[deviceId].toStorage();
+                prom = prom.delay(5).then(() => {
+                    this._processQueryResponseForUser(userId, dk[userId]);
                 });
-
-                this._sessionStore.storeEndToEndDevicesForUser(
-                    userId, storage,
-                );
-
-                if (token) {
-                    this._sessionStore.storeEndToEndDeviceSyncToken(token);
-                }
             }
+
+            return prom;
+        }).then(() => {
+            if (token !== null) {
+                this._sessionStore.storeEndToEndDeviceSyncToken(token);
+            }
+            console.log('Completed key download for ' + downloadUsers);
         });
     }
+
+    _processQueryResponseForUser(userId, response) {
+        console.log('got keys for ' + userId + ':', response);
+
+        // map from deviceid -> deviceinfo for this user
+        const userStore = {};
+        const devs = this._sessionStore.getEndToEndDevicesForUser(userId);
+        if (devs) {
+            Object.keys(devs).forEach((deviceId) => {
+                const d = DeviceInfo.fromStorage(devs[deviceId], deviceId);
+                userStore[deviceId] = d;
+            });
+        }
+
+        _updateStoredDeviceKeysForUser(
+            this._olmDevice, userId, userStore, response || {},
+        );
+
+        // update the session store
+        const storage = {};
+        Object.keys(userStore).forEach((deviceId) => {
+            storage[deviceId] = userStore[deviceId].toStorage();
+        });
+
+        this._sessionStore.storeEndToEndDevicesForUser(
+            userId, storage,
+        );
+    }
 }
 
+
 function _updateStoredDeviceKeysForUser(_olmDevice, userId, userStore,
         userResult) {
     let updated = false;

src/crypto/index.js

@@ -61,6 +61,7 @@ function Crypto(baseApis, eventEmitter, sessionStore, userId, deviceId,
 
     this._olmDevice = new OlmDevice(sessionStore);
     this._deviceList = new DeviceList(baseApis, sessionStore, this._olmDevice);
+    this._initialDeviceListInvalidationDone = false;
 
     // EncryptionAlgorithm instance for each room
     this._roomEncryptors = {};
@@ -764,7 +765,7 @@ Crypto.prototype._onCryptoEvent = function(event) {
  * @param {Object} syncData  the data from the 'MatrixClient.sync' event
  */
 Crypto.prototype._onSyncCompleted = function(syncData) {
-    this._deviceList.lastKnownSyncToken = syncData.nextSyncToken;
+    const nextSyncToken = syncData.nextSyncToken;
 
     if (!syncData.oldSyncToken) {
         // an initialsync.
@@ -773,24 +774,37 @@ Crypto.prototype._onSyncCompleted = function(syncData) {
         // if we have a deviceSyncToken, we can tell the deviceList to
         // invalidate devices which have changed since then.
         const oldSyncToken = this._sessionStore.getEndToEndDeviceSyncToken();
-        if (oldSyncToken) {
-            this._invalidateDeviceListsSince(oldSyncToken).catch((e) => {
+        if (oldSyncToken !== null) {
+            this._invalidateDeviceListsSince(
+                oldSyncToken, nextSyncToken,
+            ).catch((e) => {
                 // if that failed, we fall back to invalidating everyone.
                 console.warn("Error fetching changed device list", e);
                 this._invalidateDeviceListForAllActiveUsers();
-                return this._deviceList.refreshOutdatedDeviceLists();
-            }).done();
+            }).done(() => {
+                this._initialDeviceListInvalidationDone = true;
+                this._deviceList.lastKnownSyncToken = nextSyncToken;
+                this._deviceList.refreshOutdatedDeviceLists();
+            });
         } else {
             // otherwise, we have to invalidate all devices for all users we
             // share a room with.
             console.log("Completed first initialsync; invalidating all " +
                         "device list caches");
             this._invalidateDeviceListForAllActiveUsers();
+            this._initialDeviceListInvalidationDone = true;
         }
     }
 
-    // catch up on any new devices we got told about during the sync.
-    this._deviceList.refreshOutdatedDeviceLists().done();
+    if (this._initialDeviceListInvalidationDone) {
+        // if we've got an up-to-date list of users with outdated device lists,
+        // tell the device list about the new sync token (but not otherwise, because
+        // otherwise we'll start thinking we're more in sync than we are.)
+        this._deviceList.lastKnownSyncToken = nextSyncToken;
+
+        // catch up on any new devices we got told about during the sync.
+        this._deviceList.refreshOutdatedDeviceLists();
+    }
 };
 
 /**
@@ -846,16 +860,19 @@ Crypto.prototype._sendNewDeviceEvents = function() {
 
 /**
  * Ask the server which users have new devices since a given token,
- * invalidate them, and start an update query.
+ * and invalidate them
  *
  * @param {String} oldSyncToken
+ * @param {String} lastKnownSyncToken
  *
  * @returns {Promise} resolves once the query is complete. Rejects if the
  *   keyChange query fails.
  */
-Crypto.prototype._invalidateDeviceListsSince = function(oldSyncToken) {
+Crypto.prototype._invalidateDeviceListsSince = function(
+    oldSyncToken, lastKnownSyncToken,
+) {
     return this._baseApis.getKeyChanges(
-        oldSyncToken, this.lastKnownSyncToken,
+        oldSyncToken, lastKnownSyncToken,
     ).then((r) => {
         if (!r.changed || !Array.isArray(r.changed)) {
             return;
@@ -869,7 +886,6 @@ Crypto.prototype._invalidateDeviceListsSince = function(oldSyncToken) {
                 this._deviceList.invalidateUserDeviceList(u);
             }
         });
-        return this._deviceList.refreshOutdatedDeviceLists();
     });
 };
 

src/models/event.js

@@ -61,7 +61,9 @@ module.exports.EventStatus = {
  * from changes to event JSON between Matrix versions.
  *
  * @prop {RoomMember} sender The room member who sent this event, or null e.g.
- * this is a presence event.
+ * this is a presence event. This is only guaranteed to be set for events that
+ * appear in a timeline, ie. do not guarantee that it will be set on state
+ * events.
  * @prop {RoomMember} target The room member who is the target of this event, e.g.
  * the invitee, the person being banned, etc.
  * @prop {EventStatus} status The sending status of the event.