Close peer connection if the connection state is 'disconnected' for 15s

Previously, we relied on the ICE connection state going into 'failed'
(or 'closed') eventually. However, with continuous gathering, there
is no guarantee this ever happens. Furthermore, this may take an
arbitrary amount of time. Recently, libwebrtc has turned off
dispatching of the 'failed' state completely, see:
https://bugs.chromium.org/p/chromium/issues/detail?id=982793

Also, a bit of cleanup (sorry).

src/services/peerconnection.ts

@@ -20,50 +20,56 @@ import {Logger} from 'ts-log';
 
 import {ConfidentialIceCandidate} from '../helpers/confidential';
 import {LogService} from './log';
+import {TimeoutService} from './timeout';
 
 /**
  * Wrapper around the WebRTC PeerConnection.
  */
 export class PeerConnectionHelper {
+    private static readonly CONNECTION_FAILED_TIMEOUT_MS = 15000;
+
     // Angular services
-    private log: Logger;
-    private $q: ng.IQService;
-    private $timeout: ng.ITimeoutService;
-    private $rootScope: ng.IRootScopeService;
+    private readonly log: Logger;
+    private readonly $q: ng.IQService;
+    private readonly $rootScope: ng.IRootScopeService;
+
+    // Custom services
+    private readonly timeoutService: TimeoutService;
 
-    // WebRTC
-    private pc: RTCPeerConnection;
-    private webrtcTask: saltyrtc.tasks.webrtc.WebRTCTask;
+        // WebRTC
+    private readonly pc: RTCPeerConnection;
+    private readonly webrtcTask: saltyrtc.tasks.webrtc.WebRTCTask;
+    private connectionFailedTimer: ng.IPromise<void> | null = null;
 
     // Calculated connection state
     public connectionState: TaskConnectionState = TaskConnectionState.New;
     public onConnectionStateChange: (state: TaskConnectionState) => void = null;
 
-    constructor($q: ng.IQService, $timeout: ng.ITimeoutService, $rootScope: ng.IRootScopeService,
-                logService: LogService, webrtcTask: saltyrtc.tasks.webrtc.WebRTCTask, iceServers: RTCIceServer[]) {
+    constructor($q: ng.IQService, $rootScope: ng.IRootScopeService,
+                logService: LogService, timeoutService: TimeoutService,
+                webrtcTask: saltyrtc.tasks.webrtc.WebRTCTask, iceServers: RTCIceServer[]) {
         this.log = logService.getLogger('PeerConnection', 'color: #fff; background-color: #3333ff');
         this.log.info('Initialize WebRTC PeerConnection');
         this.log.debug('ICE servers used:', [].concat(...iceServers.map((c) => c.urls)));
         this.$q = $q;
-        this.$timeout = $timeout;
         this.$rootScope = $rootScope;
 
+        this.timeoutService = timeoutService;
         this.webrtcTask = webrtcTask;
 
         // Set up peer connection
         this.pc = new RTCPeerConnection({iceServers: iceServers});
-        this.pc.onnegotiationneeded = (e: Event) => {
+        this.pc.onnegotiationneeded = () => {
             this.log.debug('RTCPeerConnection: negotiation needed');
-            this.initiatorFlow().then(
-                (_) => this.log.debug('Initiator flow done'),
-            );
+            this.initiatorFlow()
+                .then(() => this.log.debug('Initiator flow done'));
         };
 
         // Handle state changes
-        this.pc.onconnectionstatechange = (e: Event) => {
+        this.pc.onconnectionstatechange = () => {
             this.log.debug('Connection state change:', this.pc.connectionState);
         };
-        this.pc.onsignalingstatechange = (e: Event) => {
+        this.pc.onsignalingstatechange = () => {
             this.log.debug('Signaling state change:', this.pc.signalingState);
         };
 
@@ -103,9 +109,16 @@ export class PeerConnectionHelper {
         this.pc.onicecandidateerror = (e: RTCPeerConnectionIceErrorEvent) => {
             this.log.error('ICE candidate error:', e);
         };
-        this.pc.oniceconnectionstatechange = (e: Event) => {
+        this.pc.oniceconnectionstatechange = () => {
             this.log.debug('ICE connection state change:', this.pc.iceConnectionState);
             this.$rootScope.$apply(() => {
+                // Cancel connection failed timer
+                if (this.connectionFailedTimer !== null) {
+                    this.timeoutService.cancel(this.connectionFailedTimer);
+                    this.connectionFailedTimer = null;
+                }
+
+                // Handle state
                 switch (this.pc.iceConnectionState) {
                     case 'new':
                         this.setConnectionState(TaskConnectionState.New);
@@ -113,6 +126,17 @@ export class PeerConnectionHelper {
                     case 'checking':
                     case 'disconnected':
                         this.setConnectionState(TaskConnectionState.Connecting);
+
+                        // Setup connection failed timer
+                        // Note: There is no guarantee that we will end up in the 'failed' state, so we need to set up
+                        // our own timer as well.
+                        this.connectionFailedTimer = this.timeoutService.register(() => {
+                            // Closing the peer connection to prevent "SURPRISE, the connection works after all!"
+                            // situations which certainly would lead to ugly race conditions.
+                            this.connectionFailedTimer = null;
+                            this.log.debug('ICE connection considered failed');
+                            this.pc.close();
+                        }, PeerConnectionHelper.CONNECTION_FAILED_TIMEOUT_MS, true, 'connectionFailedTimer');
                         break;
                     case 'connected':
                     case 'completed':
@@ -128,7 +152,7 @@ export class PeerConnectionHelper {
                 }
             });
         };
-        this.pc.onicegatheringstatechange = (e: Event) => {
+        this.pc.onicegatheringstatechange = () => {
             this.log.debug('ICE gathering state change:', this.pc.iceGatheringState);
         };
         this.webrtcTask.on('candidates', (e: saltyrtc.tasks.webrtc.CandidatesEvent) => {
@@ -139,7 +163,8 @@ export class PeerConnectionHelper {
                 } else {
                     this.log.debug('No more remote ICE candidates');
                 }
-                this.pc.addIceCandidate(candidateInit);
+                this.pc.addIceCandidate(candidateInit)
+                    .catch((error) => this.log.warn('Unable to add ice candidate:', error));
             }
         });
     }
@@ -180,7 +205,7 @@ export class PeerConnectionHelper {
         if (state !== this.connectionState) {
             this.connectionState = state;
             if (this.onConnectionStateChange !== null) {
-                this.$timeout(() => this.onConnectionStateChange(state), 0);
+                this.onConnectionStateChange(state);
             }
         }
     }

src/services/webclient.ts

@@ -862,10 +862,9 @@ export class WebClientService {
             }
 
             this.pcHelper = new PeerConnectionHelper(
-                this.$q, this.$timeout, this.$rootScope,
-                this.logService,
-                this.webrtcTask,
-                this.config.ICE_SERVERS);
+                this.$q, this.$rootScope,
+                this.logService, this.timeoutService,
+                this.webrtcTask, this.config.ICE_SERVERS);
 
             // On state changes in the PeerConnectionHelper class, let state service know about it
             this.pcHelper.onConnectionStateChange = (state: threema.TaskConnectionState) => {