diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index 0cf4cb6a5..5f39c0d72 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -123,7 +123,7 @@ protocol CallServiceObserver: class { didSet { AssertIsOnMainThread() - Logger.debug("\(self.TAG) .peerConnectionClient setter: \(oldValue != nil) -> \(peerConnectionClient != nil)") + Logger.debug("\(self.TAG) .peerConnectionClient setter: \(oldValue != nil) -> \(peerConnectionClient != nil) \(peerConnectionClient)") } } @@ -138,6 +138,8 @@ protocol CallServiceObserver: class { updateIsVideoEnabled() + Logger.debug("\(self.TAG) .call setter: \(oldValue != nil) -> \(call != nil) \(call)") + for observer in observers { observer.value?.didUpdateCall(call:call) } @@ -283,11 +285,7 @@ protocol CallServiceObserver: class { return getIceServers().then { iceServers -> Promise in Logger.debug("\(self.TAG) got ice servers:\(iceServers)") - let peerConnectionClient = PeerConnectionClient(iceServers: iceServers, delegate: self) - - // When placing an outgoing call, it's our responsibility to create the DataChannel. Recipient will not have - // to do this explicitly. - peerConnectionClient.createSignalingDataChannel() + let peerConnectionClient = PeerConnectionClient(iceServers: iceServers, delegate: self, callType: .outgoing) assert(self.peerConnectionClient == nil, "Unexpected PeerConnectionClient instance") Logger.debug("\(self.TAG) setting peerConnectionClient in \(#function)") @@ -428,6 +426,9 @@ protocol CallServiceObserver: class { let backgroundTask = UIApplication.shared.beginBackgroundTask { let timeout = CallError.timeout(description: "background task time ran out before call connected.") DispatchQueue.main.async { + guard self.call == newCall else { + return + } self.handleFailedCall(error: timeout) } } @@ -437,9 +438,12 @@ protocol CallServiceObserver: class { }.then { (iceServers: [RTCIceServer]) -> Promise in // FIXME for first time call recipients I think we'll see mic/camera permission requests here, // even though, from the users perspective, no incoming call is yet visible. + guard self.call == newCall else { + throw CallError.assertionError(description: "getIceServers() response for obsolete call") + } assert(self.peerConnectionClient == nil, "Unexpected PeerConnectionClient instance") Logger.debug("\(self.self.TAG) setting peerConnectionClient in \(#function)") - self.peerConnectionClient = PeerConnectionClient(iceServers: iceServers, delegate: self) + self.peerConnectionClient = PeerConnectionClient(iceServers: iceServers, delegate: self, callType: .incoming) let offerSessionDescription = RTCSessionDescription(type: .offer, sdp: callerSessionDescription) let constraints = RTCMediaConstraints(mandatoryConstraints: nil, optionalConstraints: nil) @@ -447,6 +451,9 @@ protocol CallServiceObserver: class { // Find a sessionDescription compatible with my constraints and the remote sessionDescription return self.peerConnectionClient!.negotiateSessionDescription(remoteDescription: offerSessionDescription, constraints: constraints) }.then { (negotiatedSessionDescription: HardenedRTCSessionDescription) in + guard self.call == newCall else { + throw CallError.assertionError(description: "negotiateSessionDescription() response for obsolete call") + } Logger.debug("\(self.TAG) set the remote description") let answerMessage = OWSCallAnswerMessage(callId: newCall.signalingId, sessionDescription: negotiatedSessionDescription.sdp) @@ -454,6 +461,9 @@ protocol CallServiceObserver: class { return self.messageSender.sendCallMessage(callAnswerMessage) }.then { + guard self.call == newCall else { + throw CallError.assertionError(description: "sendCallMessage() response for obsolete call") + } Logger.debug("\(self.TAG) successfully sent callAnswerMessage") let (promise, fulfill, _) = Promise.pending() @@ -468,6 +478,10 @@ protocol CallServiceObserver: class { return race(promise, timeout) }.catch { error in + guard self.call == newCall else { + Logger.debug("\(self.TAG) error for obsolete call: \(error)") + return + } if let callError = error as? CallError { self.handleFailedCall(error: callError) } else { @@ -996,7 +1010,6 @@ protocol CallServiceObserver: class { } self.localVideoTrack = videoTrack - self.fireDidUpdateVideoTracks() } internal func peerConnectionClient(_ peerConnectionClient: PeerConnectionClient, didUpdateRemote videoTrack: RTCVideoTrack?) { @@ -1008,7 +1021,6 @@ protocol CallServiceObserver: class { } self.remoteVideoTrack = videoTrack - self.fireDidUpdateVideoTracks() } // MARK: Helpers diff --git a/Signal/src/call/PeerConnectionClient.swift b/Signal/src/call/PeerConnectionClient.swift index 1a6d3dcdc..8d9cc4234 100644 --- a/Signal/src/call/PeerConnectionClient.swift +++ b/Signal/src/call/PeerConnectionClient.swift @@ -64,6 +64,11 @@ protocol PeerConnectionClientDelegate: class { */ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelDelegate { + enum CallType { + case incoming + case outgoing + } + let TAG = "[PeerConnectionClient]" enum Identifiers: String { case mediaStream = "ARDAMS", @@ -110,10 +115,10 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD private var videoSender: RTCRtpSender? private var localVideoTrack: RTCVideoTrack? - private var remoteVideoTrack: RTCVideoTrack? + private weak var remoteVideoTrack: RTCVideoTrack? private var cameraConstraints: RTCMediaConstraints - init(iceServers: [RTCIceServer], delegate: PeerConnectionClientDelegate) { + init(iceServers: [RTCIceServer], delegate: PeerConnectionClientDelegate, callType: CallType) { AssertIsOnMainThread() self.iceServers = iceServers @@ -139,21 +144,25 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD delegate: self) createAudioSender() createVideoSender() + + if callType == .outgoing { + // When placing an outgoing call, it's our responsibility to create the DataChannel. + // Recipient will not have to do this explicitly. + createSignalingDataChannel() + } } // MARK: - Media Streams - public func createSignalingDataChannel() { + private func createSignalingDataChannel() { AssertIsOnMainThread() - PeerConnectionClient.signalingQueue.sync { - let dataChannel = peerConnection.dataChannel(forLabel: Identifiers.dataChannelSignaling.rawValue, - configuration: RTCDataChannelConfiguration()) - dataChannel.delegate = self + let dataChannel = peerConnection.dataChannel(forLabel: Identifiers.dataChannelSignaling.rawValue, + configuration: RTCDataChannelConfiguration()) + dataChannel.delegate = self - assert(self.dataChannel == nil) - self.dataChannel = dataChannel - } + assert(self.dataChannel == nil) + self.dataChannel = dataChannel } // MARK: Video @@ -536,22 +545,23 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD /** Called when media is received on a new stream from remote peer. */ internal func peerConnection(_ peerConnection: RTCPeerConnection, didAdd stream: RTCMediaStream) { + guard stream.videoTracks.count > 0 else { + return + } + weak var remoteVideoTrack = stream.videoTracks[0] + Logger.debug("\(self.TAG) didAdd stream:\(stream) video tracks: \(stream.videoTracks.count) audio tracks: \(stream.audioTracks.count)") + PeerConnectionClient.signalingQueue.async { guard self.peerConnection != nil else { Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client") return } - Logger.debug("\(self.TAG) didAdd stream:\(stream) video tracks: \(stream.videoTracks.count) audio tracks: \(stream.audioTracks.count)") - if stream.videoTracks.count > 0 { - self.remoteVideoTrack = stream.videoTracks[0] - if let delegate = self.delegate { - let remoteVideoTrack = self.remoteVideoTrack - DispatchQueue.main.async { [weak self, weak remoteVideoTrack] in - guard let strongSelf = self else { return } - guard let strongRemoteVideoTrack = remoteVideoTrack else { return } - delegate.peerConnectionClient(strongSelf, didUpdateRemote: strongRemoteVideoTrack) - } + self.remoteVideoTrack = remoteVideoTrack + if let delegate = self.delegate { + DispatchQueue.main.async { [weak self] in + guard let strongSelf = self else { return } + delegate.peerConnectionClient(strongSelf, didUpdateRemote: remoteVideoTrack) } } } diff --git a/Signal/test/call/PeerConnectionClientTest.swift b/Signal/test/call/PeerConnectionClientTest.swift index 85e615e1f..a977a9d2b 100644 --- a/Signal/test/call/PeerConnectionClientTest.swift +++ b/Signal/test/call/PeerConnectionClientTest.swift @@ -55,9 +55,8 @@ class PeerConnectionClientTest: XCTestCase { let iceServers = [RTCIceServer]() clientDelegate = FakePeerConnectionClientDelegate() - client = PeerConnectionClient(iceServers: iceServers, delegate: clientDelegate) + client = PeerConnectionClient(iceServers: iceServers, delegate: clientDelegate, callType: .outgoing) peerConnection = client.peerConnectionForTests() - client.createSignalingDataChannel() dataChannel = client.dataChannelForTests() }