From b415b6142f656c72378078e0fd186fd5c7a065f0 Mon Sep 17 00:00:00 2001
From: Matthew Chen <charlesmchen@gmail.com>
Date: Wed, 1 Feb 2017 12:26:10 -0500
Subject: [PATCH] Respond to CR, mainly by fixing broken tests.
// FREEBIE
---
Signal/src/call/CallService.swift | 24 ++++++-------
Signal/src/call/PeerConnectionClient.swift | 32 +++++++++++++++++
.../test/call/PeerConnectionClientTest.swift | 35 ++++++++++++++++---
3 files changed, 75 insertions(+), 16 deletions(-)
diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift
index 2e3ed4663..caee436d1 100644
--- a/Signal/src/call/CallService.swift
+++ b/Signal/src/call/CallService.swift
@@ -241,7 +241,7 @@ protocol CallServiceObserver: class {
return Promise(error: CallError.assertionError(description: errorDescription))
}
- return getIceServers().then(on: DispatchQueue.main) { iceServers -> Promise<HardenedRTCSessionDescription> in
+ return getIceServers().then() { iceServers -> Promise<HardenedRTCSessionDescription> in
Logger.debug("\(self.TAG) got ice servers:\(iceServers)")
let peerConnectionClient = PeerConnectionClient(iceServers: iceServers, delegate: self)
@@ -253,13 +253,13 @@ protocol CallServiceObserver: class {
self.peerConnectionClient = peerConnectionClient
return self.peerConnectionClient!.createOffer()
- }.then(on: DispatchQueue.main) { (sessionDescription: HardenedRTCSessionDescription) -> Promise<Void> in
- return self.peerConnectionClient!.setLocalSessionDescription(sessionDescription).then(on: DispatchQueue.main) {
+ }.then() { (sessionDescription: HardenedRTCSessionDescription) -> Promise<Void> in
+ return self.peerConnectionClient!.setLocalSessionDescription(sessionDescription).then() {
let offerMessage = OWSCallOfferMessage(callId: call.signalingId, sessionDescription: sessionDescription.sdp)
let callMessage = OWSOutgoingCallMessage(thread: thread, offerMessage: offerMessage)
return self.messageSender.sendCallMessage(callMessage)
}
- }.catch(on: DispatchQueue.main) { error in
+ }.catch() { error in
Logger.error("\(self.TAG) placing call failed with error: \(error)")
if let callError = error as? CallError {
@@ -307,7 +307,7 @@ protocol CallServiceObserver: class {
let sessionDescription = RTCSessionDescription(type: .answer, sdp: sessionDescription)
_ = peerConnectionClient.setRemoteSessionDescription(sessionDescription).then {
Logger.debug("\(self.TAG) successfully set remote description")
- }.catch(on: DispatchQueue.main) { error in
+ }.catch() { error in
if let callError = error as? CallError {
self.handleFailedCall(error: callError)
} else {
@@ -392,7 +392,7 @@ protocol CallServiceObserver: class {
incomingCallPromise = firstly {
return getIceServers()
- }.then(on: DispatchQueue.main) { (iceServers: [RTCIceServer]) -> Promise<HardenedRTCSessionDescription> in
+ }.then() { (iceServers: [RTCIceServer]) -> Promise<HardenedRTCSessionDescription> 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.
self.peerConnectionClient = PeerConnectionClient(iceServers: iceServers, delegate: self)
@@ -402,14 +402,14 @@ protocol CallServiceObserver: class {
// Find a sessionDescription compatible with my constraints and the remote sessionDescription
return self.peerConnectionClient!.negotiateSessionDescription(remoteDescription: offerSessionDescription, constraints: constraints)
- }.then(on: DispatchQueue.main) { (negotiatedSessionDescription: HardenedRTCSessionDescription) in
+ }.then() { (negotiatedSessionDescription: HardenedRTCSessionDescription) in
Logger.debug("\(self.TAG) set the remote description")
let answerMessage = OWSCallAnswerMessage(callId: newCall.signalingId, sessionDescription: negotiatedSessionDescription.sdp)
let callAnswerMessage = OWSOutgoingCallMessage(thread: thread, answerMessage: answerMessage)
return self.messageSender.sendCallMessage(callAnswerMessage)
- }.then(on: DispatchQueue.main) {
+ }.then() {
Logger.debug("\(self.TAG) successfully sent callAnswerMessage")
let (promise, fulfill, _) = Promise<Void>.pending()
@@ -423,7 +423,7 @@ protocol CallServiceObserver: class {
self.fulfillCallConnectedPromise = fulfill
return race(promise, timeout)
- }.catch(on: DispatchQueue.main) { error in
+ }.catch() { error in
if let callError = error as? CallError {
self.handleFailedCall(error: callError)
} else {
@@ -738,9 +738,9 @@ protocol CallServiceObserver: class {
// If the call hasn't started yet, we don't have a data channel to communicate the hang up. Use Signal Service Message.
let hangupMessage = OWSCallHangupMessage(callId: call.signalingId)
let callMessage = OWSOutgoingCallMessage(thread: thread, hangupMessage: hangupMessage)
- _ = self.messageSender.sendCallMessage(callMessage).then(on: DispatchQueue.main) {
+ _ = self.messageSender.sendCallMessage(callMessage).then() {
Logger.debug("\(self.TAG) successfully sent hangup call message to \(thread)")
- }.catch(on: DispatchQueue.main) { error in
+ }.catch() { error in
Logger.error("\(self.TAG) failed to send hangup call message to \(thread) with error: \(error)")
}
@@ -946,7 +946,7 @@ protocol CallServiceObserver: class {
return firstly {
return accountManager.getTurnServerInfo()
- }.then(on: DispatchQueue.main) { turnServerInfo -> [RTCIceServer] in
+ }.then() { turnServerInfo -> [RTCIceServer] in
Logger.debug("\(self.TAG) got turn server urls: \(turnServerInfo.urls)")
return turnServerInfo.urls.map { url in
diff --git a/Signal/src/call/PeerConnectionClient.swift b/Signal/src/call/PeerConnectionClient.swift
index 3ff27d3d7..46aab4679 100644
--- a/Signal/src/call/PeerConnectionClient.swift
+++ b/Signal/src/call/PeerConnectionClient.swift
@@ -577,6 +577,38 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD
// Skipping check on <iOS10, since syntax is different and it's just a development convenience.
}
}
+
+ // MARK: Test-only accessors
+
+ internal func peerConnectionForTests() -> RTCPeerConnection {
+ AssertIsOnMainThread()
+
+ var result: RTCPeerConnection? = nil
+ PeerConnectionClient.signalingQueue.sync {
+ result = peerConnection
+ Logger.info("\(self.TAG) called \(#function)")
+ }
+ return result!
+ }
+
+ internal func dataChannelForTests() -> RTCDataChannel {
+ AssertIsOnMainThread()
+
+ var result: RTCDataChannel? = nil
+ PeerConnectionClient.signalingQueue.sync {
+ result = dataChannel
+ Logger.info("\(self.TAG) called \(#function)")
+ }
+ return result!
+ }
+
+ internal func flushSignalingQueueForTests() {
+ AssertIsOnMainThread()
+
+ PeerConnectionClient.signalingQueue.sync {
+ // Noop.
+ }
+ }
}
/**
diff --git a/Signal/test/call/PeerConnectionClientTest.swift b/Signal/test/call/PeerConnectionClientTest.swift
index 4b2a4486f..85e615e1f 100644
--- a/Signal/test/call/PeerConnectionClientTest.swift
+++ b/Signal/test/call/PeerConnectionClientTest.swift
@@ -1,5 +1,6 @@
-// Created by Michael Kirk on 1/11/17.
-// Copyright © 2017 Open Whisper Systems. All rights reserved.
+//
+// Copyright (c) 2017 Open Whisper Systems. All rights reserved.
+//
import XCTest
import WebRTC
@@ -32,6 +33,14 @@ class FakePeerConnectionClientDelegate: PeerConnectionClientDelegate {
internal func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, received dataChannelMessage: OWSWebRTCProtosData) {
dataChannelMessages.append(dataChannelMessage)
}
+
+ internal func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, didUpdateLocal videoTrack: RTCVideoTrack?) {
+
+ }
+
+ internal func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, didUpdateRemote videoTrack: RTCVideoTrack?) {
+
+ }
}
class PeerConnectionClientTest: XCTestCase {
@@ -47,9 +56,9 @@ class PeerConnectionClientTest: XCTestCase {
let iceServers = [RTCIceServer]()
clientDelegate = FakePeerConnectionClientDelegate()
client = PeerConnectionClient(iceServers: iceServers, delegate: clientDelegate)
- peerConnection = client.peerConnection
+ peerConnection = client.peerConnectionForTests()
client.createSignalingDataChannel()
- dataChannel = client.dataChannel!
+ dataChannel = client.dataChannelForTests()
}
override func tearDown() {
@@ -62,12 +71,15 @@ class PeerConnectionClientTest: XCTestCase {
XCTAssertNil(clientDelegate.connectionState)
client.peerConnection(peerConnection, didChange: RTCIceConnectionState.connected)
+ waitForPeerConnectionClient()
XCTAssertEqual(FakePeerConnectionClientDelegate.ConnectionState.connected, clientDelegate.connectionState)
client.peerConnection(peerConnection, didChange: RTCIceConnectionState.completed)
+ waitForPeerConnectionClient()
XCTAssertEqual(FakePeerConnectionClientDelegate.ConnectionState.connected, clientDelegate.connectionState)
client.peerConnection(peerConnection, didChange: RTCIceConnectionState.failed)
+ waitForPeerConnectionClient()
XCTAssertEqual(FakePeerConnectionClientDelegate.ConnectionState.failed, clientDelegate.connectionState)
}
@@ -82,9 +94,22 @@ class PeerConnectionClientTest: XCTestCase {
client.peerConnection(peerConnection, didGenerate: candidate2)
client.peerConnection(peerConnection, didGenerate: candidate3)
+ waitForPeerConnectionClient()
+
XCTAssertEqual(3, clientDelegate.localIceCandidates.count)
}
+ func waitForPeerConnectionClient() {
+ // PeerConnectionClient processes RTCPeerConnectionDelegate invocations first on the signaling queue...
+ client.flushSignalingQueueForTests()
+ // ...then on the main queue.
+ let expectation = self.expectation(description: "Wait for PeerConnectionClient to call delegate method on main queue")
+ DispatchQueue.main.async {
+ expectation.fulfill()
+ }
+ self.waitForExpectations(timeout: 1.0, handler: nil)
+ }
+
func testDataChannelMessage() {
XCTAssertEqual(0, clientDelegate.dataChannelMessages.count)
@@ -92,6 +117,8 @@ class PeerConnectionClientTest: XCTestCase {
let hangupBuffer = RTCDataBuffer(data: hangup.asData(), isBinary: false)
client.dataChannel(dataChannel, didReceiveMessageWith: hangupBuffer)
+ waitForPeerConnectionClient()
+
XCTAssertEqual(1, clientDelegate.dataChannelMessages.count)
let dataChannelMessageProto = clientDelegate.dataChannelMessages[0]