Send ICE updates immediately after sending CallOffer for faster call

connection.

For legacy reasons, the call sender used to wait until after receiving
the call answer before sending the ICE updates. The primary motivation
was that if the receiving user hadn't accepted a new identity change,
rather than just seeing one "Tap to Accept New Safety Number" messages
for a call, they'd see one for the call offer and then a dozen more as
ICE updates trickled in.

We changed that behavior long ago, and effectively all clients will
avoid that case, while sending ICE updates immediately will allow calls
to connect without having to wait for an additional serialized round
trip between the caller and call recipient.

// FREEBIE
pull/1/head
Michael Kirk 7 years ago
parent 76aee05817
commit 5f305f844f

@ -163,17 +163,6 @@ protocol CallServiceObserver: class {
private var fulfillCallConnectedPromise: (() -> Void)? private var fulfillCallConnectedPromise: (() -> Void)?
private var rejectCallConnectedPromise: ((Error) -> Void)? private var rejectCallConnectedPromise: ((Error) -> Void)?
/**
* In the process of establishing a connection between the clients (ICE process) we must exchange ICE updates.
* Because this happens via Signal Service it's possible the callee user has not accepted any change in the caller's
* identity. In which case *each* ICE update would cause an "identity change" warning on the callee's device. Since
* this could be several messages, the caller stores all ICE updates until receiving positive confirmation that the
* callee has received a message from us. This positive confirmation comes in the form of the callees `CallAnswer`
* message.
*/
var sendIceUpdatesImmediately = true
var pendingIceUpdateMessages = [OWSCallIceUpdateMessage]()
// Used by waitForPeerConnectionClient to make sure any received // Used by waitForPeerConnectionClient to make sure any received
// ICE messages wait until the peer connection client is set up. // ICE messages wait until the peer connection client is set up.
private var fulfillPeerConnectionClientPromise: (() -> Void)? private var fulfillPeerConnectionClientPromise: (() -> Void)?
@ -283,9 +272,6 @@ protocol CallServiceObserver: class {
self.call = call self.call = call
sendIceUpdatesImmediately = false
pendingIceUpdateMessages = []
let callRecord = TSCall(timestamp: NSDate.ows_millisecondTimeStamp(), withCallNumber: call.remotePhoneNumber, callType: RPRecentCallTypeOutgoingIncomplete, in: call.thread) let callRecord = TSCall(timestamp: NSDate.ows_millisecondTimeStamp(), withCallNumber: call.remotePhoneNumber, callType: RPRecentCallTypeOutgoingIncomplete, in: call.thread)
callRecord.save() callRecord.save()
call.callRecord = callRecord call.callRecord = callRecord
@ -405,19 +391,6 @@ protocol CallServiceObserver: class {
return return
} }
// Now that we know the recipient trusts our identity, we no longer need to enqueue ICE updates.
self.sendIceUpdatesImmediately = true
if pendingIceUpdateMessages.count > 0 {
Logger.error("\(self.logTag) Sending \(pendingIceUpdateMessages.count) pendingIceUpdateMessages")
let callMessage = OWSOutgoingCallMessage(thread: thread, iceUpdateMessages: pendingIceUpdateMessages)
let sendPromise = messageSender.sendPromise(message: callMessage).catch { error in
Logger.error("\(self.logTag) failed to send ice updates in \(#function) with error: \(error)")
}
sendPromise.retainUntilComplete()
}
guard let peerConnectionClient = self.peerConnectionClient else { guard let peerConnectionClient = self.peerConnectionClient else {
OWSProdError(OWSAnalyticsEvents.callServicePeerConnectionMissing(), file: #file, function: #function, line: #line) OWSProdError(OWSAnalyticsEvents.callServicePeerConnectionMissing(), file: #file, function: #function, line: #line)
handleFailedCall(failedCall: call, error: CallError.assertionError(description: "peerConnectionClient was unexpectedly nil in \(#function)")) handleFailedCall(failedCall: call, error: CallError.assertionError(description: "peerConnectionClient was unexpectedly nil in \(#function)"))
@ -696,7 +669,7 @@ protocol CallServiceObserver: class {
AssertIsOnMainThread() AssertIsOnMainThread()
guard let call = self.call else { guard let call = self.call else {
Logger.warn("ignoring remote ice update for thread: \(thread.uniqueId) since there is no current call. Call already ended?") Logger.warn("ignoring remote ice update for thread: \(String(describing: thread.uniqueId)) since there is no current call. Call already ended?")
return return
} }
@ -706,12 +679,12 @@ protocol CallServiceObserver: class {
} }
guard thread.contactIdentifier() == call.thread.contactIdentifier() else { guard thread.contactIdentifier() == call.thread.contactIdentifier() else {
Logger.warn("ignoring remote ice update for thread: \(thread.uniqueId) due to thread mismatch. Call already ended?") Logger.warn("ignoring remote ice update for thread: \(String(describing: thread.uniqueId)) due to thread mismatch. Call already ended?")
return return
} }
guard let peerConnectionClient = self.peerConnectionClient else { guard let peerConnectionClient = self.peerConnectionClient else {
Logger.warn("ignoring remote ice update for thread: \(thread.uniqueId) since there is no current peerConnectionClient. Call already ended?") Logger.warn("ignoring remote ice update for thread: \(String(describing: thread.uniqueId)) since there is no current peerConnectionClient. Call already ended?")
return return
} }
@ -753,19 +726,10 @@ protocol CallServiceObserver: class {
let iceUpdateMessage = OWSCallIceUpdateMessage(callId: call.signalingId, sdp: iceCandidate.sdp, sdpMLineIndex: iceCandidate.sdpMLineIndex, sdpMid: iceCandidate.sdpMid) let iceUpdateMessage = OWSCallIceUpdateMessage(callId: call.signalingId, sdp: iceCandidate.sdp, sdpMLineIndex: iceCandidate.sdpMLineIndex, sdpMid: iceCandidate.sdpMid)
if self.sendIceUpdatesImmediately { Logger.info("\(self.logTag) in \(#function) sending ICE Candidate.")
Logger.info("\(self.logTag) in \(#function). Sending immediately.") let callMessage = OWSOutgoingCallMessage(thread: call.thread, iceUpdateMessage: iceUpdateMessage)
let callMessage = OWSOutgoingCallMessage(thread: call.thread, iceUpdateMessage: iceUpdateMessage) let sendPromise = self.messageSender.sendPromise(message: callMessage)
let sendPromise = self.messageSender.sendPromise(message: callMessage) sendPromise.retainUntilComplete()
sendPromise.retainUntilComplete()
} else {
// For outgoing messages, we wait to send ice updates until we're sure client received our call message.
// e.g. if the client has blocked our message due to an identity change, we'd otherwise
// bombard them with a bunch *more* undecipherable messages.
Logger.info("\(self.logTag) in \(#function). Enqueing for later.")
self.pendingIceUpdateMessages.append(iceUpdateMessage)
return
}
}.catch { error in }.catch { error in
OWSProdInfo(OWSAnalyticsEvents.callServiceErrorHandleLocalAddedIceCandidate(), file: #file, function: #function, line: #line) OWSProdInfo(OWSAnalyticsEvents.callServiceErrorHandleLocalAddedIceCandidate(), file: #file, function: #function, line: #line)
Logger.error("\(self.logTag) in \(#function) waitUntilReadyToSendIceUpdates failed with error: \(error)") Logger.error("\(self.logTag) in \(#function) waitUntilReadyToSendIceUpdates failed with error: \(error)")
@ -1503,9 +1467,6 @@ protocol CallServiceObserver: class {
self.callUIAdapter.didTerminateCall(self.call) self.callUIAdapter.didTerminateCall(self.call)
self.call = nil self.call = nil
self.sendIceUpdatesImmediately = true
Logger.info("\(self.logTag) clearing pendingIceUpdateMessages")
self.pendingIceUpdateMessages = []
self.fulfillCallConnectedPromise = nil self.fulfillCallConnectedPromise = nil
// In case we're still waiting on this promise somewhere, we need to reject it to avoid a memory leak. // In case we're still waiting on this promise somewhere, we need to reject it to avoid a memory leak.

@ -1,5 +1,5 @@
// //
// Copyright (c) 2017 Open Whisper Systems. All rights reserved. // Copyright (c) 2018 Open Whisper Systems. All rights reserved.
// //
import Foundation import Foundation
@ -657,7 +657,7 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD
Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client") Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client")
return return
} }
Logger.debug("\(self.TAG) didChange IceConnectionState:\(newState.debugDescription)") Logger.info("\(self.TAG) didChange IceConnectionState:\(newState.debugDescription)")
switch newState { switch newState {
case .connected, .completed: case .connected, .completed:
if let delegate = self.delegate { if let delegate = self.delegate {
@ -684,7 +684,7 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD
/** Called any time the IceGatheringState changes. */ /** Called any time the IceGatheringState changes. */
internal func peerConnection(_ peerConnection: RTCPeerConnection, didChange newState: RTCIceGatheringState) { internal func peerConnection(_ peerConnection: RTCPeerConnection, didChange newState: RTCIceGatheringState) {
Logger.debug("\(TAG) didChange IceGatheringState:\(newState.debugDescription)") Logger.info("\(TAG) didChange IceGatheringState:\(newState.debugDescription)")
} }
/** New ice candidate has been found. */ /** New ice candidate has been found. */
@ -716,7 +716,7 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD
Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client") Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client")
return return
} }
Logger.debug("\(self.TAG) didOpen dataChannel:\(dataChannel)") Logger.info("\(self.TAG) didOpen dataChannel:\(dataChannel)")
assert(self.dataChannel == nil) assert(self.dataChannel == nil)
self.dataChannel = dataChannel self.dataChannel = dataChannel
dataChannel.delegate = self dataChannel.delegate = self

Loading…
Cancel
Save