v9.2.0-rc.1

Implement call holding functionality

CHANGELOG.md

@@ -1,3 +1,43 @@
+Changes in [9.2.0-rc.1](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v9.2.0-rc.1) (2020-11-18)
+==========================================================================================================
+[Full Changelog](https://github.com/matrix-org/matrix-js-sdk/compare/v9.1.0...v9.2.0-rc.1)
+
+ * Implement call holding functionality
+   [\#1532](https://github.com/matrix-org/matrix-js-sdk/pull/1532)
+ * Support awaitable one-time dehydration
+   [\#1537](https://github.com/matrix-org/matrix-js-sdk/pull/1537)
+ * Client set profile methods update own user
+   [\#1534](https://github.com/matrix-org/matrix-js-sdk/pull/1534)
+
+Changes in [9.1.0](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v9.1.0) (2020-11-09)
+================================================================================================
+[Full Changelog](https://github.com/matrix-org/matrix-js-sdk/compare/v9.1.0-rc.1...v9.1.0)
+
+* No changes since rc.1
+
+Changes in [9.1.0-rc.1](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v9.1.0-rc.1) (2020-11-04)
+==========================================================================================================
+[Full Changelog](https://github.com/matrix-org/matrix-js-sdk/compare/v9.0.1...v9.1.0-rc.1)
+
+ * Fix spelling error in the server ACL event type
+   [\#1535](https://github.com/matrix-org/matrix-js-sdk/pull/1535)
+ * await idb operations from crypto store for dehydration
+   [\#1533](https://github.com/matrix-org/matrix-js-sdk/pull/1533)
+ * Fix stuck never-sending messages
+   [\#1531](https://github.com/matrix-org/matrix-js-sdk/pull/1531)
+ * Await key cache check to avoid prompts
+   [\#1529](https://github.com/matrix-org/matrix-js-sdk/pull/1529)
+ * Improve ICE candidate batching
+   [\#1524](https://github.com/matrix-org/matrix-js-sdk/pull/1524)
+ * Convert logger to typescript
+   [\#1527](https://github.com/matrix-org/matrix-js-sdk/pull/1527)
+ * Fix logger typo
+   [\#1525](https://github.com/matrix-org/matrix-js-sdk/pull/1525)
+ * bind online listener to window instead of document
+   [\#1523](https://github.com/matrix-org/matrix-js-sdk/pull/1523)
+ * Support m.call.select_answer
+   [\#1522](https://github.com/matrix-org/matrix-js-sdk/pull/1522)
+
 Changes in [9.0.1](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v9.0.1) (2020-10-28)
 ================================================================================================
 [Full Changelog](https://github.com/matrix-org/matrix-js-sdk/compare/v9.0.0...v9.0.1)

package.json

@@ -1,6 +1,6 @@
 {
   "name": "matrix-js-sdk",
-  "version": "9.0.1",
+  "version": "9.2.0-rc.1",
   "description": "Matrix Client-Server SDK for Javascript",
   "scripts": {
     "prepare": "yarn build",

spec/unit/webrtc/call.spec.ts

@@ -15,7 +15,7 @@ limitations under the License.
 */
 
 import {TestClient} from '../../TestClient';
-import {MatrixCall} from '../../../src/webrtc/call';
+import {MatrixCall, CallErrorCode} from '../../../src/webrtc/call';
 
 const DUMMY_SDP = (
     "v=0\r\n" +
@@ -78,6 +78,7 @@ class MockRTCPeerConnection {
     setLocalDescription() {
         return Promise.resolve();
     }
+    close() {}
 }
 
 describe('Call', function() {
@@ -118,6 +119,9 @@ describe('Call', function() {
         global.document = {};
 
         client = new TestClient("@alice:foo", "somedevice", "token", undefined, {});
+        // We just stub out sendEvent: we're not interested in testing the client's
+        // event sending code here
+        client.client.sendEvent = () => {};
         call = new MatrixCall({
             client: client.client,
             roomId: '!foo:bar',
@@ -135,12 +139,16 @@ describe('Call', function() {
 
     it('should ignore candidate events from non-matching party ID', async function() {
         await call.placeVoiceCall();
-        await call.receivedAnswer({
-            version: 0,
-            call_id: call.callId,
-            party_id: 'the_correct_party_id',
-            answer: {
-                sdp: DUMMY_SDP,
+        await call.onAnswerReceived({
+            getContent: () => {
+                return {
+                    version: 0,
+                    call_id: call.callId,
+                    party_id: 'the_correct_party_id',
+                    answer: {
+                        sdp: DUMMY_SDP,
+                    },
+                };
             },
         });
 
@@ -178,5 +186,8 @@ describe('Call', function() {
             },
         });
         expect(call.peerConn.addIceCandidate.mock.calls.length).toBe(1);
+
+        // Hangup to stop timers
+        call.hangup(CallErrorCode.UserHangup, true);
     });
 });

src/@types/event.ts

@@ -29,7 +29,7 @@ export enum EventType {
     RoomEncryption = "m.room.encryption",
     RoomHistoryVisibility = "m.room.history_visibility",
     RoomGuestAccess = "m.room.guest_access",
-    RoomServerAcl = "m.room_server_acl",
+    RoomServerAcl = "m.room.server_acl",
     RoomTombstone = "m.room.tombstone",
     /**
      * @deprecated Should not be used.
@@ -46,6 +46,8 @@ export enum EventType {
     CallAnswer = "m.call.answer",
     CallHangup = "m.call.hangup",
     CallReject = "m.call.reject",
+    CallSelectAnswer = "m.call.select_answer",
+    CallNegotiate = "m.call.negotiate",
     KeyVerificationRequest = "m.key.verification.request",
     KeyVerificationStart = "m.key.verification.start",
     KeyVerificationCancel = "m.key.verification.cancel",

src/client.js

@@ -577,11 +577,33 @@ MatrixClient.prototype.setDehydrationKey = async function(
         logger.warn('not dehydrating device if crypto is not enabled');
         return;
     }
-    return await this._crypto._dehydrationManager.setDehydrationKey(
+    return await this._crypto._dehydrationManager.setKeyAndQueue(
         key, keyInfo, deviceDisplayName,
     );
 };
 
+/**
+ * Creates a new dehydrated device (without queuing periodic dehydration)
+ * @param {Uint8Array} key the dehydration key
+ * @param {object} [keyInfo] Information about the key.  Primarily for
+ *     information about how to generate the key from a passphrase.
+ * @param {string} [deviceDisplayName] The device display name for the
+ *     dehydrated device.
+ * @return {Promise<String>} the device id of the newly created dehydrated device
+ */
+MatrixClient.prototype.createDehydratedDevice = async function(
+    key, keyInfo = {}, deviceDisplayName = undefined,
+) {
+    if (!(this._crypto)) {
+        logger.warn('not dehydrating device if crypto is not enabled');
+        return;
+    }
+    await this._crypto._dehydrationManager.setKey(
+        key, keyInfo, deviceDisplayName,
+    );
+    return await this._crypto._dehydrationManager.dehydrateDevice();
+};
+
 MatrixClient.prototype.exportDevice = async function() {
     if (!(this._crypto)) {
         logger.warn('not exporting device if crypto is not enabled');
@@ -3699,25 +3721,39 @@ MatrixClient.prototype.setProfileInfo = function(info, data, callback) {
 /**
  * @param {string} name
  * @param {module:client.callback} callback Optional.
- * @return {Promise} Resolves: TODO
+ * @return {Promise} Resolves: {} an empty object.
  * @return {module:http-api.MatrixError} Rejects: with an error response.
  */
-MatrixClient.prototype.setDisplayName = function(name, callback) {
-    return this.setProfileInfo(
+MatrixClient.prototype.setDisplayName = async function(name, callback) {
+    const prom = await this.setProfileInfo(
         "displayname", { displayname: name }, callback,
     );
+    // XXX: synthesise a profile update for ourselves because Synapse is broken and won't
+    const user = this.getUser(this.getUserId());
+    if (user) {
+        user.displayName = name;
+        user.emit("User.displayName", user.events.presence, user);
+    }
+    return prom;
 };
 
 /**
  * @param {string} url
  * @param {module:client.callback} callback Optional.
- * @return {Promise} Resolves: TODO
+ * @return {Promise} Resolves: {} an empty object.
  * @return {module:http-api.MatrixError} Rejects: with an error response.
  */
-MatrixClient.prototype.setAvatarUrl = function(url, callback) {
-    return this.setProfileInfo(
+MatrixClient.prototype.setAvatarUrl = async function(url, callback) {
+    const prom = await this.setProfileInfo(
         "avatar_url", { avatar_url: url }, callback,
     );
+    // XXX: synthesise a profile update for ourselves because Synapse is broken and won't
+    const user = this.getUser(this.getUserId());
+    if (user) {
+        user.avatarUrl = url;
+        user.emit("User.avatarUrl", user.events.presence, user);
+    }
+    return prom;
 };
 
 /**

src/crypto/dehydration.ts

@@ -51,7 +51,7 @@ export class DehydrationManager {
         this.getDehydrationKeyFromCache();
     }
     async getDehydrationKeyFromCache(): Promise<void> {
-        return this.crypto._cryptoStore.doTxn(
+        return await this.crypto._cryptoStore.doTxn(
             'readonly',
             [IndexedDBCryptoStore.STORE_ACCOUNT],
             (txn) => {
@@ -77,10 +77,23 @@ export class DehydrationManager {
             },
         );
     }
-    async setDehydrationKey(
+
+    /** set the key, and queue periodic dehydration to the server in the background */
+    async setKeyAndQueueDehydration(
         key: Uint8Array, keyInfo: {[props: string]: any} = {},
         deviceDisplayName: string = undefined,
     ): Promise<void> {
+        const matches = await this.setKey(key, keyInfo, deviceDisplayName);
+        if (!matches) {
+            // start dehydration in the background
+            this.dehydrateDevice();
+        }
+    }
+
+    async setKey(
+        key: Uint8Array, keyInfo: {[props: string]: any} = {},
+        deviceDisplayName: string = undefined,
+    ): Promise<boolean> {
         if (!key) {
             // unsetting the key -- cancel any pending dehydration task
             if (this.timeoutId) {
@@ -88,7 +101,7 @@ export class DehydrationManager {
                 this.timeoutId = undefined;
             }
             // clear storage
-            this.crypto._cryptoStore.doTxn(
+            await this.crypto._cryptoStore.doTxn(
                 'readwrite',
                 [IndexedDBCryptoStore.STORE_ACCOUNT],
                 (txn) => {
@@ -104,7 +117,7 @@ export class DehydrationManager {
 
         // Check to see if it's the same key as before.  If it's different,
         // dehydrate a new device.  If it's the same, we can keep the same
-        // device.  (Assume that keyInfo and deviceDisplayNamme will be the
+        // device.  (Assume that keyInfo and deviceDisplayName will be the
         // same if the key is the same.)
         let matches: boolean = this.key && key.length == this.key.length;
         for (let i = 0; matches && i < key.length; i++) {
@@ -116,11 +129,12 @@ export class DehydrationManager {
             this.key = key;
             this.keyInfo = keyInfo;
             this.deviceDisplayName = deviceDisplayName;
-            // start dehydration in the background
-            this.dehydrateDevice();
         }
+        return matches;
     }
-    private async dehydrateDevice(): Promise<void> {
+
+    /** returns the device id of the newly created dehydrated device */
+    async dehydrateDevice(): Promise<string> {
         if (this.inProgress) {
             logger.log("Dehydration already in progress -- not starting new dehydration");
             return;
@@ -135,7 +149,7 @@ export class DehydrationManager {
 
             // update the crypto store with the timestamp
             const key = await encryptAES(encodeBase64(this.key), pickleKey, DEHYDRATION_ALGORITHM);
-            this.crypto._cryptoStore.doTxn(
+            await this.crypto._cryptoStore.doTxn(
                 'readwrite',
                 [IndexedDBCryptoStore.STORE_ACCOUNT],
                 (txn) => {
@@ -258,8 +272,17 @@ export class DehydrationManager {
             this.timeoutId = global.setTimeout(
                 this.dehydrateDevice.bind(this), oneweek,
             );
+
+            return deviceId;
         } finally {
             this.inProgress = false;
         }
     }
+
+    private stop() {
+        if (this.timeoutId) {
+            global.clearTimeout(this.timeoutId);
+            this.timeoutId = undefined;
+        }
+    }
 }

src/crypto/index.js

@@ -1752,6 +1752,7 @@ Crypto.prototype.start = function() {
 Crypto.prototype.stop = function() {
     this._outgoingRoomKeyRequestManager.stop();
     this._deviceList.stop();
+    this._dehydrationManager.stop();
 };
 
 /**
@@ -2638,7 +2639,10 @@ Crypto.prototype.trackRoomDevices = function(roomId) {
     let promise = this._roomDeviceTrackingState[roomId];
     if (!promise) {
         promise = trackMembers();
-        this._roomDeviceTrackingState[roomId] = promise;
+        this._roomDeviceTrackingState[roomId] = promise.catch(err => {
+            this._roomDeviceTrackingState[roomId] = null;
+            throw err;
+        });
     }
     return promise;
 };

src/sync.js

@@ -475,9 +475,9 @@ SyncApi.prototype.sync = function() {
 
     this._running = true;
 
-    if (global.document) {
+    if (global.window) {
         this._onOnlineBound = this._onOnline.bind(this);
-        global.document.addEventListener("online", this._onOnlineBound, false);
+        global.window.addEventListener("online", this._onOnlineBound, false);
     }
 
     let savedSyncPromise = Promise.resolve();
@@ -643,8 +643,8 @@ SyncApi.prototype.sync = function() {
  */
 SyncApi.prototype.stop = function() {
     debuglog("SyncApi.stop");
-    if (global.document) {
-        global.document.removeEventListener("online", this._onOnlineBound, false);
+    if (global.window) {
+        global.window.removeEventListener("online", this._onOnlineBound, false);
         this._onOnlineBound = undefined;
     }
     this._running = false;

src/webrtc/call.ts

@@ -22,9 +22,10 @@ limitations under the License.
  */
 
 import {logger} from '../logger';
-import {EventEmitter} from "events";
-import * as utils from "../utils";
-import MatrixEvent from "../models/event"
+import {EventEmitter} from 'events';
+import * as utils from '../utils';
+import MatrixEvent from '../models/event';
+import {EventType} from '../@types/event';
 
 // events: hangup, error(err), replaced(call), state(state, oldState)
 
@@ -90,6 +91,9 @@ export enum CallEvent {
     State = 'state',
     Error = 'error',
     Replaced = 'replaced',
+
+    // The value of isLocalOnHold() has changed
+    HoldUnhold = 'hold_unhold',
 }
 
 enum MediaQueueId {
@@ -162,6 +166,11 @@ export enum CallErrorCode {
      * The call was replaced by another call
      */
     Replaced = 'replaced',
+
+    /**
+     * Signalling for the call could not be sent (other than the initial invite)
+     */
+    SignallingFailed = 'signalling_timeout',
 }
 
 /**
@@ -208,6 +217,7 @@ export class MatrixCall extends EventEmitter {
     hangupParty: CallParty;
     hangupReason: string;
     direction: CallDirection;
+    ourPartyId: string;
 
     private client: any; // Fix when client is TSified
     private forceTURN: boolean;
@@ -223,7 +233,7 @@ export class MatrixCall extends EventEmitter {
     private screenSharingStream: MediaStream;
     private remoteStream: MediaStream;
     private localAVStream: MediaStream;
-    private answerContent: object;
+    private inviteOrAnswerSent: boolean;
     private waitForLocalAVStream: boolean;
     // XXX: This is either the invite or answer from remote...
     private msg: any;
@@ -231,10 +241,20 @@ export class MatrixCall extends EventEmitter {
     private config: MediaStreamConstraints;
     private successor: MatrixCall;
     private opponentVersion: number;
-    private ourPartyId: string;
     // The party ID of the other side: undefined if we haven't chosen a partner
     // yet, null if we have but they didn't send a party ID.
     private opponentPartyId: string;
+    private inviteTimeout: NodeJS.Timeout; // in the browser it's 'number'
+
+    // The logic of when & if a call is on hold is nontrivial and explained in is*OnHold
+    // This flag represents whether we want the other party to be on hold
+    private remoteOnHold;
+    private micMuted;
+    private vidMuted;
+
+    // Perfect negotiation state: https://www.w3.org/TR/webrtc/#perfect-negotiation-example
+    private makingOffer: boolean;
+    private ignoreOffer: boolean;
 
     constructor(opts: CallOpts) {
         super();
@@ -270,6 +290,12 @@ export class MatrixCall extends EventEmitter {
         this.mediaPromises = Object.create(null);
 
         this.sentEndOfCandidates = false;
+        this.inviteOrAnswerSent = false;
+        this.makingOffer = false;
+
+        this.remoteOnHold = false;
+        this.micMuted = false;
+        this.vidMuted = false;
     }
 
     /**
@@ -418,7 +444,7 @@ export class MatrixCall extends EventEmitter {
     async setRemoteAudioElement(element: HTMLAudioElement) {
         if (element === this.remoteAudioElement) return;
 
-        this.remoteVideoElement.muted = true;
+        if (this.remoteVideoElement) this.remoteVideoElement.muted = true;
         this.remoteAudioElement = element;
         this.remoteAudioElement.muted = false;
 
@@ -488,20 +514,21 @@ export class MatrixCall extends EventEmitter {
      * Answer a call.
      */
     async answer() {
-        logger.debug(`Answering call ${this.callId} of type ${this.type}`);
-
-        if (this.answerContent) {
-            this.sendAnswer();
+        if (this.inviteOrAnswerSent) {
             return;
         }
 
+        logger.debug(`Answering call ${this.callId} of type ${this.type}`);
+
         if (!this.localAVStream && !this.waitForLocalAVStream) {
             const constraints = getUserMediaVideoContraints(this.type);
             logger.log("Getting user media with constraints", constraints);
             this.setState(CallState.WaitLocalMedia);
+            this.waitForLocalAVStream = true;
 
             try {
                 const mediaStream = await navigator.mediaDevices.getUserMedia(constraints);
+                this.waitForLocalAVStream = false;
                 this.gotUserMediaForAnswer(mediaStream);
             } catch (e) {
                 this.getUserMediaFailed(e);
@@ -547,7 +574,7 @@ export class MatrixCall extends EventEmitter {
      * @param {boolean} suppressEvent True to suppress emitting an event.
      */
     hangup(reason: CallErrorCode, suppressEvent: boolean) {
-        if (this.state === CallState.Ended) return;
+        if (this.callHasEnded()) return;
 
         logger.debug("Ending call " + this.callId);
         this.terminate(CallParty.Local, reason, !suppressEvent);
@@ -555,7 +582,7 @@ export class MatrixCall extends EventEmitter {
         // Continue to send no reason for user hangups temporarily, until
         // clients understand the user_hangup reason (voip v1)
         if (reason !== CallErrorCode.UserHangup) content['reason'] = reason;
-        this.sendVoipEvent('m.call.hangup', {});
+        this.sendVoipEvent(EventType.CallHangup, {});
     }
 
     /**
@@ -578,18 +605,16 @@ export class MatrixCall extends EventEmitter {
 
         logger.debug("Rejecting call: " + this.callId);
         this.terminate(CallParty.Local, CallErrorCode.UserHangup, true);
-        this.sendVoipEvent('m.call.reject', {});
+        this.sendVoipEvent(EventType.CallReject, {});
     }
 
     /**
-     * Set whether the local video preview should be muted or not.
-     * @param {boolean} muted True to mute the local video.
+     * Set whether our outbound video should be muted or not.
+     * @param {boolean} muted True to mute the outbound video.
      */
     setLocalVideoMuted(muted: boolean) {
-        if (!this.localAVStream) {
-            return;
-        }
-        setTracksEnabled(this.localAVStream.getVideoTracks(), !muted);
+        this.vidMuted = muted;
+        this.updateMuteStatus();
     }
 
     /**
@@ -602,10 +627,7 @@ export class MatrixCall extends EventEmitter {
      * (including if the call is not set up yet).
      */
     isLocalVideoMuted(): boolean {
-        if (!this.localAVStream) {
-            return false;
-        }
-        return !isTracksEnabled(this.localAVStream.getVideoTracks());
+        return this.vidMuted;
     }
 
     /**
@@ -613,10 +635,8 @@ export class MatrixCall extends EventEmitter {
      * @param {boolean} muted True to mute the mic.
      */
     setMicrophoneMuted(muted: boolean) {
-        if (!this.localAVStream) {
-            return;
-        }
-        setTracksEnabled(this.localAVStream.getAudioTracks(), !muted);
+        this.micMuted = muted;
+        this.updateMuteStatus();
     }
 
     /**
@@ -629,10 +649,63 @@ export class MatrixCall extends EventEmitter {
      * is not set up yet).
      */
     isMicrophoneMuted(): boolean {
-        if (!this.localAVStream) {
-            return false;
+        return this.micMuted;
+    }
+
+    /**
+     * @returns true if we have put the party on the other side of the call on hold
+     * (that is, we are signalling to them that we are not listening)
+     */
+    isRemoteOnHold(): boolean {
+        return this.remoteOnHold;
+    }
+
+    setRemoteOnHold(onHold: boolean) {
+        if (this.isRemoteOnHold() === onHold) return;
+        this.remoteOnHold = onHold;
+
+        for (const tranceiver of this.peerConn.getTransceivers()) {
+            // We set 'inactive' rather than 'sendonly' because we're not planning on
+            // playing music etc. to the other side.
+            tranceiver.direction = onHold ? 'inactive' : 'sendrecv';
         }
-        return !isTracksEnabled(this.localAVStream.getAudioTracks());
+        this.updateMuteStatus();
+    }
+
+    /**
+     * Indicates whether we are 'on hold' to the remote party (ie. if true,
+     * they cannot hear us). Note that this will return true when we put the
+     * remote on hold too due to the way hold is implemented (since we don't
+     * wish to play hold music when we put a call on hold, we use 'inactive'
+     * rather than 'sendonly')
+     * @returns true if the other party has put us on hold
+     */
+    isLocalOnHold(): boolean {
+        if (this.state !== CallState.Connected) return false;
+
+        let callOnHold = true;
+
+        // We consider a call to be on hold only if *all* the tracks are on hold
+        // (is this the right thing to do?)
+        for (const tranceiver of this.peerConn.getTransceivers()) {
+            const trackOnHold = ['inactive', 'recvonly'].includes(tranceiver.currentDirection);
+
+            if (!trackOnHold) callOnHold = false;
+        }
+
+        return callOnHold;
+    }
+
+    private updateMuteStatus() {
+        if (!this.localAVStream) {
+            return;
+        }
+
+        const micShouldBeMuted = this.micMuted || this.remoteOnHold;
+        setTracksEnabled(this.localAVStream.getAudioTracks(), !micShouldBeMuted);
+
+        const vidShouldBeMuted = this.vidMuted || this.remoteOnHold;
+        setTracksEnabled(this.localAVStream.getVideoTracks(), !vidShouldBeMuted);
     }
 
     /**
@@ -644,9 +717,12 @@ export class MatrixCall extends EventEmitter {
             this.successor.gotUserMediaForAnswer(stream);
             return;
         }
-        if (this.state === CallState.Ended) {
+        if (this.callHasEnded()) {
             return;
         }
+
+        this.setState(CallState.CreateOffer);
+
         logger.debug("gotUserMediaForInvite -> " + this.type);
 
         const videoEl = this.getLocalVideoElement();
@@ -668,32 +744,42 @@ export class MatrixCall extends EventEmitter {
         }
 
         this.localAVStream = stream;
+        logger.info("Got local AV stream with id " + this.localAVStream.id);
         // why do we enable audio (and only audio) tracks here? -- matthew
         setTracksEnabled(stream.getAudioTracks(), true);
         this.peerConn = this.createPeerConnection();
 
         for (const audioTrack of stream.getAudioTracks()) {
+            logger.info("Adding audio track with id " + audioTrack.id);
             this.peerConn.addTrack(audioTrack, stream);
         }
         for (const videoTrack of (this.screenSharingStream || stream).getVideoTracks()) {
+            logger.info("Adding audio track with id " + videoTrack.id);
             this.peerConn.addTrack(videoTrack, stream);
         }
 
-        try {
-            const myOffer = await this.peerConn.createOffer();
-            this.gotLocalOffer(myOffer);
-        } catch (e) {
-            this.getLocalOfferFailed(e);
-            return;
-        }
-        this.setState(CallState.CreateOffer);
+        // Now we wait for the negotiationneeded event
     };
 
     private sendAnswer() {
-        this.setState(CallState.Connecting);
-        this.sendVoipEvent('m.call.answer', this.answerContent).then(() => {
+        const answerContent = {
+            answer: {
+                sdp: this.peerConn.localDescription.sdp,
+                // type is now deprecated as of Matrix VoIP v1, but
+                // required to still be sent for backwards compat
+                type: this.peerConn.localDescription.type,
+            },
+        };
+        // We have just taken the local description from the peerconnection which will
+        // contain all the local candidates added so far, so we can discard any candidates
+        // we had queued up because they'll be in the answer.
+        logger.info(`Discarding ${this.candidateSendQueue.length} candidates that will be sent in answer`);
+        this.candidateSendQueue = [];
+
+        this.sendVoipEvent(EventType.CallAnswer, answerContent).then(() => {
             // If this isn't the first time we've tried to send the answer,
             // we may have candidates queued up, so send them now.
+            this.inviteOrAnswerSent = true;
             this.sendCandidateQueue();
         }).catch((error) => {
             // We've failed to answer: back to the ringing state
@@ -712,7 +798,7 @@ export class MatrixCall extends EventEmitter {
     }
 
     private gotUserMediaForAnswer = async (stream: MediaStream) => {
-        if (this.state === CallState.Ended) {
+        if (this.callHasEnded()) {
             return;
         }
 
@@ -729,6 +815,7 @@ export class MatrixCall extends EventEmitter {
         }
 
         this.localAVStream = stream;
+        logger.info("Got local AV stream with id " + this.localAVStream.id);
         setTracksEnabled(stream.getAudioTracks(), true);
         for (const track of stream.getTracks()) {
             this.peerConn.addTrack(track, stream);
@@ -747,15 +834,13 @@ export class MatrixCall extends EventEmitter {
 
         try {
             await this.peerConn.setLocalDescription(myAnswer);
+            this.setState(CallState.Connecting);
+
+            // Allow a short time for initial candidates to be gathered
+            await new Promise(resolve => {
+                setTimeout(resolve, 200);
+            });
 
-            this.answerContent = {
-                answer: {
-                    sdp: this.peerConn.localDescription.sdp,
-                    // type is now deprecated as of Matrix VoIP v1, but
-                    // required to still be sent for backwards compat
-                    type: this.peerConn.localDescription.type,
-                },
-            };
             this.sendAnswer();
         } catch (err) {
             logger.debug("Error setting local description!", err);
@@ -775,12 +860,12 @@ export class MatrixCall extends EventEmitter {
                 event.candidate.candidate,
             );
 
-            if (this.state == CallState.Ended) return;
+            if (this.callHasEnded()) return;
 
             // As with the offer, note we need to make a copy of this object, not
             // pass the original: that broke in Chrome ~m43.
             if (event.candidate.candidate !== '' || !this.sentEndOfCandidates) {
-                this.sendCandidate(event.candidate);
+                this.queueCandidate(event.candidate);
 
                 if (event.candidate.candidate === '') this.sentEndOfCandidates = true;
             }
@@ -800,13 +885,13 @@ export class MatrixCall extends EventEmitter {
             const c = {
                 candidate: '',
             } as RTCIceCandidate;
-            this.sendCandidate(c);
+            this.queueCandidate(c);
             this.sentEndOfCandidates = true;
         }
     };
 
     onRemoteIceCandidatesReceived(ev: MatrixEvent) {
-        if (this.state == CallState.Ended) {
+        if (this.callHasEnded()) {
             //debuglog("Ignoring remote ICE candidate because call has ended");
             return;
         }
@@ -834,7 +919,13 @@ export class MatrixCall extends EventEmitter {
                 return;
             }
             logger.debug("Got remote ICE " + cand.sdpMid + " candidate: " + cand.candidate);
-            this.peerConn.addIceCandidate(cand);
+            try {
+                this.peerConn.addIceCandidate(cand);
+            } catch (err) {
+                if (!this.ignoreOffer) {
+                    logger.info("Failed to add remore ICE candidate", err);
+                }
+            }
         }
     }
 
@@ -842,37 +933,126 @@ export class MatrixCall extends EventEmitter {
      * Used by MatrixClient.
      * @param {Object} msg
      */
-    async receivedAnswer(msg: MatrixEvent) {
-        if (this.state === CallState.Ended) {
+    async onAnswerReceived(event: MatrixEvent) {
+        if (this.callHasEnded()) {
             return;
         }
 
         if (this.opponentPartyId !== undefined) {
             logger.info(
-                `Ignoring answer from party ID ${msg.party_id}: ` +
+                `Ignoring answer from party ID ${event.getContent().party_id}: ` +
                 `we already have an answer/reject from ${this.opponentPartyId}`,
             );
             return;
         }
 
-        this.opponentVersion = msg.version;
-        this.opponentPartyId = msg.party_id || null;
+        this.opponentVersion = event.getContent().version;
+        this.opponentPartyId = event.getContent().party_id || null;
+
+        this.setState(CallState.Connecting);
 
         try {
-            await this.peerConn.setRemoteDescription(msg.answer);
+            await this.peerConn.setRemoteDescription(event.getContent().answer);
         } catch (e) {
             logger.debug("Failed to set remote description", e);
             this.terminate(CallParty.Local, CallErrorCode.SetRemoteDescription, false);
             return;
         }
 
-        this.setState(CallState.Connecting);
+        // If the answer we selected has a party_id, send a select_answer event
+        // We do this after setting the remote description since otherwise we'd block
+        // call setup on it
+        if (this.opponentPartyId !== null) {
+            try {
+                await this.sendVoipEvent(EventType.CallSelectAnswer, {
+                    selected_party_id: this.opponentPartyId,
+                });
+            } catch (err) {
+                // This isn't fatal, and will just mean that if another party has raced to answer
+                // the call, they won't know they got rejected, so we carry on & don't retry.
+                logger.warn("Failed to send select_answer event", err);
+            }
+        }
+    }
+
+    async onSelectAnswerReceived(event: MatrixEvent) {
+        if (this.direction !== CallDirection.Inbound) {
+            logger.warn("Got select_answer for an outbound call: ignoring");
+            return;
+        }
+
+        const selectedPartyId = event.getContent().selected_party_id;
+
+        if (selectedPartyId === undefined || selectedPartyId === null) {
+            logger.warn("Got nonsensical select_answer with null/undefined selected_party_id: ignoring");
+            return;
+        }
+
+        if (selectedPartyId !== this.ourPartyId) {
+            logger.info(`Got select_answer for party ID ${selectedPartyId}: we are party ID ${this.ourPartyId}.`);
+            // The other party has picked somebody else's answer
+            this.terminate(CallParty.Remote, CallErrorCode.AnsweredElsewhere, true);
+        }
+    }
+
+    async onNegotiateReceived(event: MatrixEvent) {
+        const description = event.getContent().description;
+        if (!description || !description.sdp || !description.type) {
+            logger.info("Ignoring invalid m.call.negotiate event");
+            return;
+        }
+        // Politeness always follows the direction of the call: in a glare situation,
+        // we pick either the inbound or outbound call, so one side will always be
+        // inbound and one outbound
+        const polite = this.direction === CallDirection.Inbound;
+
+        // Here we follow the perfect negotiation logic from
+        // https://developer.mozilla.org/en-US/docs/Web/API/WebRTC_API/Perfect_negotiation
+        const offerCollision = (
+            (description.type == 'offer') &&
+            (this.makingOffer || this.peerConn.signalingState != 'stable')
+        );
+
+        this.ignoreOffer = !polite && offerCollision;
+        if (this.ignoreOffer) {
+            logger.info("Ignoring colliding negotiate event because we're impolite");
+            return;
+        }
+
+        const prevOnHold = this.isLocalOnHold();
+
+        try {
+            await this.peerConn.setRemoteDescription(description);
+
+            if (description.type === 'offer') {
+                const localDescription = await this.peerConn.createAnswer();
+                await this.peerConn.setLocalDescription(localDescription);
+
+                this.sendVoipEvent(EventType.CallNegotiate, {
+                    description: this.peerConn.localDescription,
+                });
+            }
+        } catch (err) {
+            logger.warn("Failed to complete negotiation", err);
+        }
+
+        const nowOnHold = this.isLocalOnHold();
+        if (prevOnHold !== nowOnHold) {
+            this.emit(CallEvent.HoldUnhold, nowOnHold);
+        }
+    }
+
+    private callHasEnded() : boolean {
+        // This exists as workaround to typescript trying to be clever and erroring
+        // when putting if (this.state === CallState.Ended) return; twice in the same
+        // function, even though that function is async.
+        return this.state === CallState.Ended;
     }
 
     private gotLocalOffer = async (description: RTCSessionDescriptionInit) => {
         logger.debug("Created offer: ", description);
 
-        if (this.state === CallState.Ended) {
+        if (this.callHasEnded()) {
             logger.debug("Ignoring newly created offer on call ID " + this.callId +
                 " because the call has ended");
             return;
@@ -886,50 +1066,63 @@ export class MatrixCall extends EventEmitter {
             return
         }
 
+        if (this.peerConn.iceGatheringState === 'gathering') {
+            // Allow a short time for initial candidates to be gathered
+            await new Promise(resolve => {
+                setTimeout(resolve, 200);
+            });
+        }
+
+        if (this.callHasEnded()) return;
+
+        const keyName = this.state === CallState.CreateOffer ? 'offer' : 'description';
+        const eventType = this.state === CallState.CreateOffer ? EventType.CallInvite : EventType.CallNegotiate;
+
         const content = {
-            // OpenWebRTC appears to add extra stuff (like the DTLS fingerprint)
-            // to the description when setting it on the peerconnection.
-            // According to the spec it should only add ICE
-            // candidates. Any ICE candidates that have already been generated
-            // at this point will probably be sent both in the offer and separately.
-            // Also, note that we have to make a new object here, copying the
-            // type and sdp properties.
-            // Passing the RTCSessionDescription object as-is doesn't work in
-            // Chrome (as of about m43).
-            offer: {
-                sdp: this.peerConn.localDescription.sdp,
-                // type now deprecated in Matrix VoIP v1, but
-                // required to still be sent for backwards compat
-                type: this.peerConn.localDescription.type,
-            },
+            [keyName]: this.peerConn.localDescription,
             lifetime: CALL_TIMEOUT_MS,
         };
+
+        // Get rid of any candidates waiting to be sent: they'll be included in the local
+        // description we just got and will send in the offer.
+        logger.info(`Discarding ${this.candidateSendQueue.length} candidates that will be sent in offer`);
+        this.candidateSendQueue = [];
+
         try {
-            await this.sendVoipEvent('m.call.invite', content);
-            this.setState(CallState.InviteSent);
-            setTimeout(() => {
-                if (this.state === CallState.InviteSent) {
-                    this.hangup(CallErrorCode.InviteTimeout, false);
-                }
-            }, CALL_TIMEOUT_MS);
+            await this.sendVoipEvent(eventType, content);
+            this.sendCandidateQueue();
+            if (this.state === CallState.CreateOffer) {
+                this.inviteOrAnswerSent = true;
+                this.setState(CallState.InviteSent);
+                this.inviteTimeout = setTimeout(() => {
+                    this.inviteTimeout = null;
+                    if (this.state === CallState.InviteSent) {
+                        this.hangup(CallErrorCode.InviteTimeout, false);
+                    }
+                }, CALL_TIMEOUT_MS);
+            }
         } catch (error) {
-            let code = CallErrorCode.SendInvite;
-            let message = "Failed to send invite";
+            this.client.cancelPendingEvent(error.event);
+
+            let code = CallErrorCode.SignallingFailed;
+            let message = "Signalling failed";
+            if (this.state === CallState.CreateOffer) {
+                code = CallErrorCode.SendInvite;
+                message = "Failed to send invite";
+            }
             if (error.name == 'UnknownDeviceError') {
                 code = CallErrorCode.UnknownDevices;
                 message = "Unknown devices present in the room";
             }
 
-            this.client.cancelPendingEvent(error.event);
-            this.terminate(CallParty.Local, code, false);
             this.emit(CallEvent.Error, new CallError(code, message, error));
+            this.terminate(CallParty.Local, code, false);
         }
     };
 
     private getLocalOfferFailed = (err: Error) => {
         logger.error("Failed to get local offer", err);
 
-        this.terminate(CallParty.Local, CallErrorCode.LocalOfferFailed, false);
         this.emit(
             CallEvent.Error,
             new CallError(
@@ -937,6 +1130,7 @@ export class MatrixCall extends EventEmitter {
                 "Failed to get local offer!", err,
             ),
         );
+        this.terminate(CallParty.Local, CallErrorCode.LocalOfferFailed, false);
     };
 
     private getUserMediaFailed = (err: Error) => {
@@ -945,7 +1139,8 @@ export class MatrixCall extends EventEmitter {
             return;
         }
 
-        this.terminate(CallParty.Local, CallErrorCode.NoUserMedia, false);
+        logger.warn("Failed to get user media - ending call", err);
+
         this.emit(
             CallEvent.Error,
             new CallError(
@@ -954,10 +1149,11 @@ export class MatrixCall extends EventEmitter {
                 "does this app have permission?", err,
             ),
         );
+        this.terminate(CallParty.Local, CallErrorCode.NoUserMedia, false);
     };
 
     onIceConnectionStateChanged = () => {
-        if (this.state === CallState.Ended) {
+        if (this.callHasEnded()) {
             return; // because ICE can still complete as we're ending the call
         }
         logger.debug(
@@ -980,41 +1176,30 @@ export class MatrixCall extends EventEmitter {
     };
 
     private onTrack = (ev: RTCTrackEvent) => {
-        logger.debug(`Track id ${ev.track.id} of kind ${ev.track.kind} added`);
-
-        // This is relatively complex as we may get any number of tracks that may
-        // be in any number of streams, or not in streams at all, etc.
-        // I'm not entirely sure how this API is supposed to be used: it would
-        // be nice to know when the browser is finished telling us about a bunch
-        // of tracks so we could go & figure out which ones to use in which streams,
-        // but it doesn't. There was an 'addstream' event, but that is now deprecated.
-
-        // The base case is that there will be one stream with one audio track, or in
-        // the case of a video call, and audio and video track.
-
-        // This algorithm is not perfect and will fail in edge cases such as a streamless
-        // track being added first, followed by a normal audio + video stream.
-
-        const haveStream = this.remoteStream !== undefined;
-        if (!haveStream) {
-            // If we don't currently have a stream, use one this track is already in
-            if (ev.streams.length > 0) {
-                this.remoteStream = ev.streams[0];
-            } else {
-                // ...unless it's a streamless track, in which case we'll need to make
-                // our own stream.
-                this.remoteStream = new MediaStream();
-            }
+        if (ev.streams.length === 0) {
+            logger.warn(`Streamless ${ev.track.kind} found: ignoring.`);
+            return;
+        }
+        // If we already have a stream, check this track is from the same one
+        if (this.remoteStream && ev.streams[0].id !== this.remoteStream.id) {
+            logger.warn(
+                `Ignoring new stream ID ${ev.streams[0].id}: we already have stream ID ${this.remoteStream.id}`,
+            );
+            return;
         }
 
-        // if this track isn't in a stream, add it to the one we have.
-        // This basically assumes all the tracks are streamless, otherwise it
-        // will end up adding the track to a stream provided by the RTCPeerConnection,
-        // which would be weird.
-        if (ev.streams.length === 0) this.remoteStream.addTrack(ev.track);
+        if (!this.remoteStream) {
+            logger.info("Got remote stream with id " + ev.streams[0].id);
+        }
 
-        // If we've just gained our stream, wire it up to the media object
-        if (!haveStream) {
+        // Note that we check by ID above and always set the remote stream: Chrome appears
+        // to make new stream objects when tranciever directionality is changed and the 'active'
+        // status of streams change
+        this.remoteStream = ev.streams[0];
+
+        logger.debug(`Track id ${ev.track.id} of kind ${ev.track.kind} added`);
+
+        if (ev.track.kind === 'video') {
             if (this.remoteVideoElement) {
                 this.queueMediaOperation(MediaQueueId.RemoteVideo, async () => {
                     this.remoteVideoElement.srcObject = this.remoteStream;
@@ -1025,10 +1210,28 @@ export class MatrixCall extends EventEmitter {
                     }
                 });
             }
+        } else {
+            if (this.remoteAudioElement) this.playRemoteAudio();
+        }
+    };
 
-            if (this.remoteAudioElement) {
-                this.playRemoteAudio();
-            }
+    onNegotiationNeeded = async () => {
+        logger.info("Negotation is needed!");
+
+        if (this.state !== CallState.CreateOffer && this.opponentVersion === 0) {
+            logger.info("Opponent does not support renegotiation: ignoring negotiationneeded event");
+            return;
+        }
+
+        this.makingOffer = true;
+        try {
+            const myOffer = await this.peerConn.createOffer();
+            await this.gotLocalOffer(myOffer);
+        } catch (e) {
+            this.getLocalOfferFailed(e);
+            return;
+        } finally {
+            this.makingOffer = false;
         }
     };
 
@@ -1110,7 +1313,7 @@ export class MatrixCall extends EventEmitter {
         }));
     }
 
-    sendCandidate(content: RTCIceCandidate) {
+    queueCandidate(content: RTCIceCandidate) {
         // Sends candidates with are sent in a special way because we try to amalgamate
         // them into one message
         this.candidateSendQueue.push(content);
@@ -1119,17 +1322,28 @@ export class MatrixCall extends EventEmitter {
         // means we tried to pick (ie. started generating candidates) and then failed to
         // send the answer and went back to the ringing state. Queue up the candidates
         // to send if we sucessfully send the answer.
-        if (this.state === CallState.Ringing) return;
+        // Equally don't send if we haven't yet sent the answer because we can send the
+        // first batch of candidates along with the answer
+        if (this.state === CallState.Ringing || !this.inviteOrAnswerSent) return;
+
+        // MSC2746 reccomends these values (can be quite long when calling because the
+        // callee will need a while to answer the call)
+        const delay = this.direction === CallDirection.Inbound ? 500 : 2000;
 
         if (this.candidateSendTries === 0) {
             setTimeout(() => {
                 this.sendCandidateQueue();
-            }, 100);
+            }, delay);
         }
     }
 
     private terminate(hangupParty: CallParty, hangupReason: CallErrorCode, shouldEmit: boolean) {
-        if (this.state === CallState.Ended) return;
+        if (this.callHasEnded()) return;
+
+        if (this.inviteTimeout) {
+            clearTimeout(this.inviteTimeout);
+            this.inviteTimeout = null;
+        }
 
         const remoteVid = this.getRemoteVideoElement();
         const remoteAud = this.getRemoteAudioElement();
@@ -1168,7 +1382,7 @@ export class MatrixCall extends EventEmitter {
             this.peerConn.close();
         }
         if (shouldEmit) {
-            this.emit(CallEvent.Hangup, self);
+            this.emit(CallEvent.Hangup, this);
         }
     }
 
@@ -1212,7 +1426,7 @@ export class MatrixCall extends EventEmitter {
             candidates: cands,
         };
         logger.debug("Attempting to send " + cands.length + " candidates");
-        this.sendVoipEvent('m.call.candidates', content).then(() => {
+        this.sendVoipEvent(EventType.CallCandidates, content).then(() => {
             this.candidateSendTries = 0;
             this.sendCandidateQueue();
         }, (error) => {
@@ -1245,6 +1459,11 @@ export class MatrixCall extends EventEmitter {
         this.setState(CallState.WaitLocalMedia);
         this.direction = CallDirection.Outbound;
         this.config = constraints;
+        // It would be really nice if we could start gathering candidates at this point
+        // so the ICE agent could be gathering while we open our media devices: we already
+        // know the type of the call and therefore what tracks we want to send.
+        // Perhaps we could do this by making fake tracks now and then using replaceTrack()
+        // once we have the actual tracks? (Can we make fake tracks?)
         try {
             const mediaStream = await navigator.mediaDevices.getUserMedia(constraints);
             this.gotUserMediaForInvite(mediaStream);
@@ -1266,6 +1485,7 @@ export class MatrixCall extends EventEmitter {
         pc.addEventListener('icecandidate', this.gotLocalIceCandidate);
         pc.addEventListener('icegatheringstatechange', this.onIceGatheringStateChange);
         pc.addEventListener('track', this.onTrack);
+        pc.addEventListener('negotiationneeded', this.onNegotiationNeeded);
 
         return pc;
     }
@@ -1283,15 +1503,6 @@ function setTracksEnabled(tracks: Array<MediaStreamTrack>, enabled: boolean) {
     }
 }
 
-function isTracksEnabled(tracks: Array<MediaStreamTrack>) {
-    for (let i = 0; i < tracks.length; i++) {
-        if (tracks[i].enabled) {
-            return true; // at least one track is enabled
-        }
-    }
-    return false;
-}
-
 function getUserMediaVideoContraints(callType: CallType) {
     const isWebkit = !!navigator.webkitGetUserMedia;
 

src/webrtc/callEventHandler.ts

@@ -213,7 +213,7 @@ export class CallEventHandler {
                     call.onAnsweredElsewhere(content);
                 }
             } else {
-                call.receivedAnswer(content);
+                call.onAnswerReceived(event);
             }
         } else if (event.getType() === EventType.CallCandidates) {
             if (event.getSender() === this.client.credentials.userId) {
@@ -251,6 +251,24 @@ export class CallEventHandler {
                     this.calls.delete(content.call_id);
                 }
             }
+        } else if (event.getType() === EventType.CallSelectAnswer) {
+            if (!call) return;
+
+            if (event.getContent().party_id === call.ourPartyId) {
+                // Ignore remote echo
+                return;
+            }
+
+            call.onSelectAnswerReceived(event);
+        } else if (event.getType() === EventType.CallNegotiate) {
+            if (!call) return;
+
+            if (event.getContent().party_id === call.ourPartyId) {
+                // Ignore remote echo
+                return;
+            }
+
+            call.onNegotiateReceived(event);
         }
     }
 }