From 814aec6cdd188ac999bcbdb88dc684ea18fb732e Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Thu, 26 Jan 2017 13:18:06 -0500 Subject: [PATCH] Recover CallKit state when remote client fails to hangup Distinguish between localHangup, remoteHangup, and call failure. This allows us to put CallKit in the proper state, ready to receive new calls without having a backlog of phantom calls which haven't been properly removed. Note the "call error" occurs at the point ICE fails, which takes a while. Anecdotally, like 10 seconds, which feels like a long to be talking into the ether. I briefly considered failing at 'disconnected', which happens much sooner, but that's actually a recoverable state. E.g. if you toggle airplane mode you can see that you bounce into `disconnected` and then back to `connected`, so I don't think we'd want to fail the call as long as WebRTC considers it "recoverable". // FREEBIE --- Signal.xcodeproj/project.pbxproj | 4 ++-- Signal/src/call/CallService.swift | 13 +++++++++---- Signal/src/call/NonCallKitCallUIAdaptee.swift | 10 +++++++++- Signal/src/call/PeerConnectionClient.swift | 2 ++ .../call/Speakerbox/CallKitCallManager.swift | 2 +- .../call/Speakerbox/CallKitCallUIAdaptee.swift | 17 ++++++++++++++--- .../src/call/UserInterface/CallUIAdapter.swift | 16 +++++++++++++--- .../view controllers/CallViewController.swift | 2 +- 8 files changed, 51 insertions(+), 15 deletions(-) diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index 06033ff48..5297d1b31 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -105,8 +105,8 @@ 45F170AF1E2F0393003FC1F2 /* CallAudioSessionTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45F170AE1E2F0393003FC1F2 /* CallAudioSessionTest.swift */; }; 45F170BB1E2FC5D3003FC1F2 /* CallAudioService.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45F170BA1E2FC5D3003FC1F2 /* CallAudioService.swift */; }; 45F170BC1E2FC5D3003FC1F2 /* CallAudioService.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45F170BA1E2FC5D3003FC1F2 /* CallAudioService.swift */; }; - 45F170D61E315310003FC1F2 /* Weak.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45F170D51E315310003FC1F2 /* Weak.swift */; }; 45F170CC1E310E22003FC1F2 /* WeakTimer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45F170CB1E310E22003FC1F2 /* WeakTimer.swift */; }; + 45F170D61E315310003FC1F2 /* Weak.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45F170D51E315310003FC1F2 /* Weak.swift */; }; 45F2B1941D9C9F48000D2C69 /* OWSOutgoingMessageCollectionViewCell.m in Sources */ = {isa = PBXBuildFile; fileRef = 45F2B1931D9C9F48000D2C69 /* OWSOutgoingMessageCollectionViewCell.m */; }; 45F2B1971D9CA207000D2C69 /* OWSIncomingMessageCollectionViewCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = 45F2B1951D9CA207000D2C69 /* OWSIncomingMessageCollectionViewCell.xib */; }; 45F2B1981D9CA207000D2C69 /* OWSOutgoingMessageCollectionViewCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = 45F2B1961D9CA207000D2C69 /* OWSOutgoingMessageCollectionViewCell.xib */; }; @@ -704,8 +704,8 @@ 45F170AE1E2F0393003FC1F2 /* CallAudioSessionTest.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = CallAudioSessionTest.swift; path = test/call/CallAudioSessionTest.swift; sourceTree = ""; }; 45F170B31E2F0A6A003FC1F2 /* RTCAudioSession.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RTCAudioSession.h; sourceTree = ""; }; 45F170BA1E2FC5D3003FC1F2 /* CallAudioService.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CallAudioService.swift; sourceTree = ""; }; - 45F170D51E315310003FC1F2 /* Weak.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Weak.swift; sourceTree = ""; }; 45F170CB1E310E22003FC1F2 /* WeakTimer.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = WeakTimer.swift; sourceTree = ""; }; + 45F170D51E315310003FC1F2 /* Weak.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Weak.swift; sourceTree = ""; }; 45F2B1921D9C9F48000D2C69 /* OWSOutgoingMessageCollectionViewCell.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OWSOutgoingMessageCollectionViewCell.h; sourceTree = ""; }; 45F2B1931D9C9F48000D2C69 /* OWSOutgoingMessageCollectionViewCell.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OWSOutgoingMessageCollectionViewCell.m; sourceTree = ""; }; 45F2B1951D9CA207000D2C69 /* OWSIncomingMessageCollectionViewCell.xib */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = file.xib; path = OWSIncomingMessageCollectionViewCell.xib; sourceTree = ""; }; diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index 2b6509d4f..90e98276a 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -499,7 +499,7 @@ fileprivate let timeoutSeconds = 60 call.state = .remoteHangup // Notify UI - callUIAdapter.endCall(call) + callUIAdapter.remoteDidHangupCall(call) // self.call is nil'd in `terminateCall`, so it's important we update it's state *before* calling `terminateCall` terminateCall() @@ -885,9 +885,14 @@ fileprivate let timeoutSeconds = 60 assertOnSignalingQueue() Logger.error("\(TAG) call failed with error: \(error)") - // It's essential to set call.state before terminateCall, because terminateCall nils self.call - call?.error = error - call?.state = .localFailure + if let call = self.call { + // It's essential to set call.state before terminateCall, because terminateCall nils self.call + call.error = error + call.state = .localFailure + callUIAdapter.failCall(call, error: error) + } else { + assertionFailure("\(TAG) in \(#function) but there was no call to fail.") + } terminateCall() } diff --git a/Signal/src/call/NonCallKitCallUIAdaptee.swift b/Signal/src/call/NonCallKitCallUIAdaptee.swift index 9adf7e3b6..c9665be41 100644 --- a/Signal/src/call/NonCallKitCallUIAdaptee.swift +++ b/Signal/src/call/NonCallKitCallUIAdaptee.swift @@ -114,7 +114,7 @@ class NonCallKitCallUIAdaptee: CallUIAdaptee { PeerConnectionClient.startAudioSession() } - func endCall(_ call: SignalCall) { + func localHangupCall(_ call: SignalCall) { CallService.signalingQueue.async { guard call.localId == self.callService.call?.localId else { assertionFailure("\(self.TAG) in \(#function) localId does not match current call") @@ -125,6 +125,14 @@ class NonCallKitCallUIAdaptee: CallUIAdaptee { } } + internal func remoteDidHangupCall(_ call: SignalCall) { + Logger.debug("\(TAG) in \(#function) is no-op") + } + + internal func failCall(_ call: SignalCall, error: CallError) { + Logger.debug("\(TAG) in \(#function) is no-op") + } + func setIsMuted(call: SignalCall, isMuted: Bool) { CallService.signalingQueue.async { guard call.localId == self.callService.call?.localId else { diff --git a/Signal/src/call/PeerConnectionClient.swift b/Signal/src/call/PeerConnectionClient.swift index f264d69a0..0193fbdb2 100644 --- a/Signal/src/call/PeerConnectionClient.swift +++ b/Signal/src/call/PeerConnectionClient.swift @@ -371,6 +371,8 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD case .failed: Logger.warn("\(self.TAG) RTCIceConnection failed.") self.delegate.peerConnectionClientIceFailed(self) + case .disconnected: + Logger.warn("\(self.TAG) RTCIceConnection disconnected.") default: Logger.debug("\(self.TAG) ignoring change IceConnectionState:\(newState.debugDescription)") } diff --git a/Signal/src/call/Speakerbox/CallKitCallManager.swift b/Signal/src/call/Speakerbox/CallKitCallManager.swift index ff75d6af9..45f164bc8 100644 --- a/Signal/src/call/Speakerbox/CallKitCallManager.swift +++ b/Signal/src/call/Speakerbox/CallKitCallManager.swift @@ -33,7 +33,7 @@ final class CallKitCallManager: NSObject { requestTransaction(transaction) } - func end(call: SignalCall) { + func localHangup(call: SignalCall) { let endCallAction = CXEndCallAction(call: call.localId) let transaction = CXTransaction() transaction.addAction(endCallAction) diff --git a/Signal/src/call/Speakerbox/CallKitCallUIAdaptee.swift b/Signal/src/call/Speakerbox/CallKitCallUIAdaptee.swift index 075160cc8..112a6b0db 100644 --- a/Signal/src/call/Speakerbox/CallKitCallUIAdaptee.swift +++ b/Signal/src/call/Speakerbox/CallKitCallUIAdaptee.swift @@ -72,6 +72,12 @@ final class CallKitCallUIAdaptee: NSObject, CallUIAdaptee, CXProviderDelegate { return call } + // Called from CallService after call has ended to clean up any remaining CallKit call state. + func failCall(_ call: SignalCall, error: CallError) { + provider.reportCall(with: call.localId, endedAt: Date(), reason: CXCallEndedReason.failed) + self.callManager.removeCall(call) + } + func reportIncomingCall(_ call: SignalCall, callerName: String) { // Construct a CXCallUpdate describing the incoming call, including the caller. let update = CXCallUpdate() @@ -110,15 +116,20 @@ final class CallKitCallUIAdaptee: NSObject, CallUIAdaptee, CXProviderDelegate { } func declineCall(_ call: SignalCall) { - callManager.end(call: call) + callManager.localHangup(call: call) } func recipientAcceptedCall(_ call: SignalCall) { // no - op + // TODO provider update call connected? + } + + func localHangupCall(_ call: SignalCall) { + callManager.localHangup(call: call) } - func endCall(_ call: SignalCall) { - callManager.end(call: call) + func remoteDidHangupCall(_ call: SignalCall) { + provider.reportCall(with: call.localId, endedAt: nil, reason: CXCallEndedReason.remoteEnded) } func setIsMuted(call: SignalCall, isMuted: Bool) { diff --git a/Signal/src/call/UserInterface/CallUIAdapter.swift b/Signal/src/call/UserInterface/CallUIAdapter.swift index 75bbc6ccc..e14e5f321 100644 --- a/Signal/src/call/UserInterface/CallUIAdapter.swift +++ b/Signal/src/call/UserInterface/CallUIAdapter.swift @@ -19,7 +19,9 @@ protocol CallUIAdaptee { func declineCall(localId: UUID) func declineCall(_ call: SignalCall) func recipientAcceptedCall(_ call: SignalCall) - func endCall(_ call: SignalCall) + func localHangupCall(_ call: SignalCall) + func remoteDidHangupCall(_ call: SignalCall) + func failCall(_ call: SignalCall, error: CallError) func setIsMuted(call: SignalCall, isMuted: Bool) func setHasVideo(call: SignalCall, hasVideo: Bool) func callBack(recipientId: String) @@ -122,8 +124,16 @@ extension CallUIAdaptee { adaptee.recipientAcceptedCall(call) } - internal func endCall(_ call: SignalCall) { - adaptee.endCall(call) + internal func remoteDidHangupCall(_ call: SignalCall) { + adaptee.remoteDidHangupCall(call) + } + + internal func localHangupCall(_ call: SignalCall) { + adaptee.localHangupCall(call) + } + + internal func failCall(_ call: SignalCall, error: CallError) { + adaptee.failCall(call, error: error) } internal func showCall(_ call: SignalCall) { diff --git a/Signal/src/view controllers/CallViewController.swift b/Signal/src/view controllers/CallViewController.swift index ce8da785b..921347007 100644 --- a/Signal/src/view controllers/CallViewController.swift +++ b/Signal/src/view controllers/CallViewController.swift @@ -464,7 +464,7 @@ class CallViewController: UIViewController, CallObserver { func didPressHangup(sender: UIButton) { Logger.info("\(TAG) called \(#function)") if let call = self.call { - callUIAdapter.endCall(call) + callUIAdapter.localHangupCall(call) } else { Logger.warn("\(TAG) hung up, but call was unexpectedly nil") }