Move RTCDataChannelDelegate to PeerConnectionClient

- minimizes CallService exposure to WebRTC

// FREEBIE

Signal/src/call/CallService.swift

@@ -75,7 +75,7 @@ enum CallError: Error {
 // FIXME TODO do we need to timeout?
 fileprivate let timeoutSeconds = 60
 
-@objc class CallService: NSObject, PeerConnectionClientDelegate, RTCDataChannelDelegate {
+@objc class CallService: NSObject, PeerConnectionClientDelegate {
 
     // MARK: - Properties
 
@@ -168,10 +168,12 @@ fileprivate let timeoutSeconds = 60
         return getIceServers().then(on: CallService.signalingQueue) { iceServers -> Promise<HardenedRTCSessionDescription> in
             Logger.debug("\(self.TAG) got ice servers:\(iceServers)")
             let peerConnectionClient = PeerConnectionClient(iceServers: iceServers, delegate: self)
-            self.peerConnectionClient = peerConnectionClient
 
-            // When calling, it's our responsibility to create the DataChannel. Receivers will not have to do this explicitly.
+            // When placing an outgoing call, it's our responsibility to create the DataChannel. Recipient will not have
-            self.peerConnectionClient!.createSignalingDataChannel(delegate: self)
+            // to do this explicitly.
+            peerConnectionClient.createSignalingDataChannel()
+
+            self.peerConnectionClient = peerConnectionClient
 
             return self.peerConnectionClient!.createOffer()
         }.then(on: CallService.signalingQueue) { (sessionDescription: HardenedRTCSessionDescription) -> Promise<Void> in
@@ -788,7 +790,7 @@ fileprivate let timeoutSeconds = 60
     /**
      * The connection has been established. The clients can now communicate.
      */
-    internal func peerConnectionClientIceConnected(_ peerconnectionClient: PeerConnectionClient) {
+    func peerConnectionClientIceConnected(_ peerconnectionClient: PeerConnectionClient) {
         CallService.signalingQueue.async {
             self.handleIceConnected()
         }
@@ -797,7 +799,7 @@ fileprivate let timeoutSeconds = 60
     /**
      * The connection failed to establish. The clients will not be able to communicate.
      */
-    internal func peerConnectionClientIceFailed(_ peerconnectionClient: PeerConnectionClient) {
+    func peerConnectionClientIceFailed(_ peerconnectionClient: PeerConnectionClient) {
         CallService.signalingQueue.async {
             self.handleFailedCall(error: CallError.disconnected)
         }
@@ -808,12 +810,21 @@ fileprivate let timeoutSeconds = 60
      * reach the local client via the internet. The delegate must shuttle these IceCandates to the other (remote) client
      * out of band, as part of establishing a connection over WebRTC.
      */
-    internal func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, addedLocalIceCandidate iceCandidate: RTCIceCandidate) {
+    func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, addedLocalIceCandidate iceCandidate: RTCIceCandidate) {
         CallService.signalingQueue.async {
             self.handleLocalAddedIceCandidate(iceCandidate)
         }
     }
 
+    /**
+     * Once the peerconnection is established, we can receive messages via the data channel, and notify the delegate.
+     */
+    func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, received dataChannelMessage: OWSWebRTCProtosData) {
+        CallService.signalingQueue.async {
+            self.handleDataChannelMessage(dataChannelMessage)
+        }
+    }
+
     // MARK: Helpers
 
     /**
@@ -883,34 +894,6 @@ fileprivate let timeoutSeconds = 60
         sendIceUpdatesImmediately = true
         pendingIceUpdateMessages = []
     }
-
-    // MARK: - RTCDataChannelDelegate
-    // TODO move `RTCDataChannelDelegate` stuff into peerConnectionClient and add a method to peerConnectionClientDelegate `receiveDataChannelMssage(_ message:OWSWebRTCProtos)
-
-    /** The data channel state changed. */
-    public func dataChannelDidChangeState(_ dataChannel: RTCDataChannel) {
-        Logger.debug("\(TAG) dataChannelDidChangeState: \(dataChannel)")
-    }
-
-    /** The data channel successfully received a data buffer. */
-    public func dataChannel(_ dataChannel: RTCDataChannel, didReceiveMessageWith buffer: RTCDataBuffer) {
-        Logger.debug("\(TAG) dataChannel didReceiveMessageWith buffer:\(buffer)")
-
-        guard let dataChannelMessage = OWSWebRTCProtosData.parse(from:buffer.data) else {
-            // TODO can't proto parsings throw an exception? Is it just being lost in the Objc->Swift?
-            Logger.error("\(TAG) failed to parse dataProto")
-            return
-        }
-
-        CallService.signalingQueue.async {
-            self.handleDataChannelMessage(dataChannelMessage)
-        }
-    }
-
-    /** The data channel's |bufferedAmount| changed. */
-    public func dataChannel(_ dataChannel: RTCDataChannel, didChangeBufferedAmount amount: UInt64) {
-        Logger.debug("\(TAG) didChangeBufferedAmount: \(amount)")
-    }
 }
 
 fileprivate extension MessageSender {

Signal/src/call/PeerConnectionClient.swift

@@ -29,6 +29,11 @@ protocol PeerConnectionClientDelegate: class {
      * out of band, as part of establishing a connection over WebRTC.
      */
     func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, addedLocalIceCandidate iceCandidate: RTCIceCandidate)
+
+    /**
+     * Once the peerconnection is established, we can receive messages via the data channel, and notify the delegate.
+     */
+    func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, received dataChannelMessage: OWSWebRTCProtosData)
 }
 
 /**
@@ -37,7 +42,7 @@ protocol PeerConnectionClientDelegate: class {
  * It is primarily a wrapper around `RTCPeerConnection`, which is responsible for sending and receiving our call data 
  * including audio, video, and some post-connected signaling (hangup, add video)
  */
-class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate {
+class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelDelegate {
 
     let TAG = "[PeerConnectionClient]"
     enum Identifiers: String {
@@ -102,10 +107,10 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate {
 
     // MARK: - Media Streams
 
-    public func createSignalingDataChannel(delegate: RTCDataChannelDelegate) {
+    public func createSignalingDataChannel() {
         let dataChannel = peerConnection.dataChannel(forLabel: Identifiers.dataChannelSignaling.rawValue,
                                                      configuration: RTCDataChannelConfiguration())
-        dataChannel.delegate = delegate
+        dataChannel.delegate = self
 
         self.dataChannel = dataChannel
     }
@@ -293,7 +298,7 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate {
         peerConnection.close()
     }
 
-    // MARK: Data Channel
+    // MARK: - Data Channel
 
     func sendDataChannelMessage(data: Data) -> Bool {
         guard let dataChannel = self.dataChannel else {
@@ -305,6 +310,31 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate {
         return dataChannel.sendData(buffer)
     }
 
+    // MARK: RTCDataChannelDelegate
+
+    /** The data channel state changed. */
+    public func dataChannelDidChangeState(_ dataChannel: RTCDataChannel) {
+        Logger.debug("\(TAG) dataChannelDidChangeState: \(dataChannel)")
+    }
+
+    /** The data channel successfully received a data buffer. */
+    public func dataChannel(_ dataChannel: RTCDataChannel, didReceiveMessageWith buffer: RTCDataBuffer) {
+        Logger.debug("\(TAG) dataChannel didReceiveMessageWith buffer:\(buffer)")
+
+        guard let dataChannelMessage = OWSWebRTCProtosData.parse(from:buffer.data) else {
+            // TODO can't proto parsings throw an exception? Is it just being lost in the Objc->Swift?
+            Logger.error("\(TAG) failed to parse dataProto")
+            return
+        }
+
+        delegate.peerConnectionClient(self, received: dataChannelMessage)
+    }
+
+    /** The data channel's |bufferedAmount| changed. */
+    public func dataChannel(_ dataChannel: RTCDataChannel, didChangeBufferedAmount amount: UInt64) {
+        Logger.debug("\(TAG) didChangeBufferedAmount: \(amount)")
+    }
+
     // MARK: - RTCPeerConnectionDelegate
 
     /** Called when the SignalingState changed. */

Signal/test/call/PeerConnectionClientTest.swift

@@ -15,6 +15,7 @@ class FakePeerConnectionClientDelegate: PeerConnectionClientDelegate {
 
     var connectionState: ConnectionState?
     var localIceCandidates = [RTCIceCandidate]()
+    var dataChannelMessages = [OWSWebRTCProtosData]()
 
     internal func peerConnectionClientIceConnected(_ peerconnectionClient: PeerConnectionClient) {
         connectionState = .connected
@@ -27,6 +28,10 @@ class FakePeerConnectionClientDelegate: PeerConnectionClientDelegate {
     internal func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, addedLocalIceCandidate iceCandidate: RTCIceCandidate) {
         localIceCandidates.append(iceCandidate)
     }
+
+    internal func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, received dataChannelMessage: OWSWebRTCProtosData) {
+        dataChannelMessages.append(dataChannelMessage)
+    }
 }
 
 class PeerConnectionClientTest: XCTestCase {
@@ -34,6 +39,7 @@ class PeerConnectionClientTest: XCTestCase {
     var client: PeerConnectionClient!
     var clientDelegate: FakePeerConnectionClientDelegate!
     var peerConnection: RTCPeerConnection!
+    var dataChannel: RTCDataChannel!
 
     override func setUp() {
         super.setUp()
@@ -42,6 +48,8 @@ class PeerConnectionClientTest: XCTestCase {
         clientDelegate = FakePeerConnectionClientDelegate()
         client = PeerConnectionClient(iceServers: iceServers, delegate: clientDelegate)
         peerConnection = client.peerConnection
+        client.createSignalingDataChannel()
+        dataChannel = client.dataChannel!
     }
 
     override func tearDown() {
@@ -77,4 +85,19 @@ class PeerConnectionClientTest: XCTestCase {
         XCTAssertEqual(3, clientDelegate.localIceCandidates.count)
     }
 
+    func testDataChannelMessage() {
+        XCTAssertEqual(0, clientDelegate.dataChannelMessages.count)
+
+        let hangup = DataChannelMessage.forHangup(callId: 123)
+        let hangupBuffer = RTCDataBuffer(data: hangup.asData(), isBinary: false)
+        client.dataChannel(dataChannel, didReceiveMessageWith: hangupBuffer)
+
+        XCTAssertEqual(1, clientDelegate.dataChannelMessages.count)
+
+        let dataChannelMessageProto = clientDelegate.dataChannelMessages[0]
+        XCTAssert(dataChannelMessageProto.hasHangup())
+
+        let hangupProto = dataChannelMessageProto.hangup!
+        XCTAssertEqual(123, hangupProto.id)
+    }
 }