Add ICE candidate event handlers

Add OnICECandidate and OnICEGatheringStateChange methods to
PeerConnection. The main goal of this change is to improve API
compatibility with the JavaScript/Wasm bindings. It does not actually
add trickle ICE support or change the ICE candidate gathering process,
which is still synchronous in the Go implementation. Rather, it fires
the appropriate events similar to they way they would be fired in a true
trickle ICE process.

Remove unused OnNegotiationNeeded event handler. This handler is not
required for most applications and would be difficult to implement in
Go. This commit removes the handler from the JavaScript/Wasm bindings,
which leads to a more similar API for Go and JavaScript/Wasm.

Add OnICEGatheringStateChange to the JavaScript/Wasm bindings. Also
changes the Go implementation so that the function signatures match.

datachannel_go_test.go

@@ -158,6 +158,12 @@ func TestDataChannelParamters_Go(t *testing.T) {
 		}
 
 		answerPC.OnDataChannel(func(d *DataChannel) {
+			// Make sure this is the data channel we were looking for. (Not the one
+			// created in signalPair).
+			if d.Label() != expectedLabel {
+				return
+			}
+
 			// Check if parameters are correctly set
 			assert.True(t, d.ordered, "Ordered should be set to true")
 			if assert.NotNil(t, d.maxPacketLifeTime, "should not be nil") {

examples/data-channels/jsfiddle/main.go

@@ -39,24 +39,26 @@ func main() {
 		log(fmt.Sprintf("Message from DataChannel %s payload %s", sendChannel.Label(), string(msg.Data)))
 	})
 
+	// Create offer
+	offer, err := pc.CreateOffer(nil)
+	if err != nil {
+		handleError(err)
+	}
+	if err := pc.SetLocalDescription(offer); err != nil {
+		handleError(err)
+	}
+
 	// Add handlers for setting up the connection.
 	pc.OnICEConnectionStateChange(func(state webrtc.ICEConnectionState) {
 		log(fmt.Sprint(state))
 	})
-	pc.OnICECandidate(func(candidate *string) {
+	pc.OnICECandidate(func(candidate *webrtc.ICECandidate) {
 		if candidate != nil {
 			encodedDescr := signal.Encode(pc.LocalDescription())
 			el := getElementByID("localSessionDescription")
 			el.Set("value", encodedDescr)
 		}
 	})
-	pc.OnNegotiationNeeded(func() {
-		offer, err := pc.CreateOffer(nil)
-		if err != nil {
-			handleError(err)
-		}
-		pc.SetLocalDescription(offer)
-	})
 
 	// Set up global callbacks which will be triggered on button clicks.
 	js.Global().Set("sendMessage", js.FuncOf(func(_ js.Value, _ []js.Value) interface{} {

examples/data-channels/main.go

@@ -5,7 +5,6 @@ import (
 	"time"
 
 	"github.com/pions/webrtc"
-
 	"github.com/pions/webrtc/examples/internal/signal"
 )
 

js_utils.go

@@ -92,6 +92,20 @@ func valueToUint8OrZero(val js.Value) uint8 {
 	return uint8(val.Int())
 }
 
+func valueToUint16OrZero(val js.Value) uint16 {
+	if val == js.Null() || val == js.Undefined() {
+		return 0
+	}
+	return uint16(val.Int())
+}
+
+func valueToUint32OrZero(val js.Value) uint32 {
+	if val == js.Null() || val == js.Undefined() {
+		return 0
+	}
+	return uint32(val.Int())
+}
+
 func valueToStrings(val js.Value) []string {
 	result := make([]string, val.Length())
 	for i := 0; i < val.Length(); i++ {

peerconnection.go

@@ -35,7 +35,7 @@ type PeerConnection struct {
 	currentRemoteDescription *SessionDescription
 	pendingRemoteDescription *SessionDescription
 	signalingState           SignalingState
-	iceGatheringState        ICEGatheringState // FIXME NOT-USED
+	iceGatheringState        ICEGatheringState
 	iceConnectionState       ICEConnectionState
 	connectionState          PeerConnectionState
 
@@ -53,16 +53,16 @@ type PeerConnection struct {
 	dataChannels map[uint16]*DataChannel
 
 	// OnNegotiationNeeded        func() // FIXME NOT-USED
-	// OnICECandidate             func() // FIXME NOT-USED
 	// OnICECandidateError        func() // FIXME NOT-USED
 
-	// OnICEGatheringStateChange  func() // FIXME NOT-USED
 	// OnConnectionStateChange    func() // FIXME NOT-USED
 
 	onSignalingStateChangeHandler     func(SignalingState)
 	onICEConnectionStateChangeHandler func(ICEConnectionState)
 	onTrackHandler                    func(*Track, *RTPReceiver)
 	onDataChannelHandler              func(*DataChannel)
+	onICECandidateHandler             func(*ICECandidate)
+	onICEGatheringStateChangeHandler  func()
 
 	iceGatherer   *ICEGatherer
 	iceTransport  *ICETransport
@@ -238,6 +238,61 @@ func (pc *PeerConnection) OnDataChannel(f func(*DataChannel)) {
 	pc.onDataChannelHandler = f
 }
 
+// OnICECandidate sets an event handler which is invoked when a new ICE
+// candidate is found.
+// BUG: trickle ICE is not supported so this event is triggered immediately when
+// SetLocalDescription is called. Typically, you only need to use this method
+// if you want API compatibility with the JavaScript/Wasm bindings.
+func (pc *PeerConnection) OnICECandidate(f func(*ICECandidate)) {
+	pc.mu.Lock()
+	defer pc.mu.Unlock()
+	pc.onICECandidateHandler = f
+}
+
+// OnICEGatheringStateChange sets an event handler which is invoked when the
+// ICE candidate gathering state has changed.
+// BUG: trickle ICE is not supported so this event is triggered immediately when
+// SetLocalDescription is called. Typically, you only need to use this method
+// if you want API compatibility with the JavaScript/Wasm bindings.
+func (pc *PeerConnection) OnICEGatheringStateChange(f func()) {
+	pc.mu.Lock()
+	defer pc.mu.Unlock()
+	pc.onICEGatheringStateChangeHandler = f
+}
+
+// signalICECandidateGatheringComplete should be called after ICE candidate
+// gathering is complete. It triggers the appropriate event handlers in order to
+// emulate a trickle ICE process.
+func (pc *PeerConnection) signalICECandidateGatheringComplete() error {
+	pc.mu.Lock()
+	defer pc.mu.Unlock()
+
+	// Call onICECandidateHandler for all candidates.
+	if pc.onICECandidateHandler != nil {
+		candidates, err := pc.iceGatherer.GetLocalCandidates()
+		if err != nil {
+			return err
+		}
+		for i := range candidates {
+			go pc.onICECandidateHandler(&candidates[i])
+		}
+		// Call the handler one last time with nil. This is a signal that candidate
+		// gathering is complete.
+		go pc.onICECandidateHandler(nil)
+	}
+
+	pc.iceGatheringState = ICEGatheringStateComplete
+
+	// Also trigger the onICEGatheringStateChangeHandler
+	if pc.onICEGatheringStateChangeHandler != nil {
+		// Note: Gathering is already done at this point, but some clients might
+		// still expect the state change handler to be triggered.
+		go pc.onICEGatheringStateChangeHandler()
+	}
+
+	return nil
+}
+
 // OnTrack sets an event handler which is called when remote track
 // arrives from a remote peer.
 func (pc *PeerConnection) OnTrack(f func(*Track, *RTPReceiver)) {
@@ -687,7 +742,18 @@ func (pc *PeerConnection) SetLocalDescription(desc SessionDescription) error {
 	if err := desc.parsed.Unmarshal([]byte(desc.SDP)); err != nil {
 		return err
 	}
-	return pc.setDescription(&desc, stateChangeOpSetLocal)
+	if err := pc.setDescription(&desc, stateChangeOpSetLocal); err != nil {
+		return err
+	}
+
+	// Call the appropriate event handlers to signal that ICE candidate gathering
+	// is complete. In reality it completed a while ago, but triggering these
+	// events helps maintain API compatibility with the JavaScript/Wasm bindings.
+	if err := pc.signalICECandidateGatheringComplete(); err != nil {
+		return err
+	}
+
+	return nil
 }
 
 // LocalDescription returns pendingLocalDescription if it is not null and
@@ -1510,7 +1576,6 @@ func (pc *PeerConnection) SignalingState() SignalingState {
 
 // ICEGatheringState attribute returns the ICE gathering state of the
 // PeerConnection instance.
-// FIXME NOT-USED
 func (pc *PeerConnection) ICEGatheringState() ICEGatheringState {
 	return pc.iceGatheringState
 }

peerconnection_go_test.go

@@ -34,39 +34,6 @@ func (api *API) newPair() (pcOffer *PeerConnection, pcAnswer *PeerConnection, er
 
 	return pca, pcb, nil
 }
-
-func signalPair(pcOffer *PeerConnection, pcAnswer *PeerConnection) error {
-	offer, err := pcOffer.CreateOffer(nil)
-	if err != nil {
-		return err
-	}
-
-	if err = pcOffer.SetLocalDescription(offer); err != nil {
-		return err
-	}
-
-	err = pcAnswer.SetRemoteDescription(offer)
-	if err != nil {
-		return err
-	}
-
-	answer, err := pcAnswer.CreateAnswer(nil)
-	if err != nil {
-		return err
-	}
-
-	if err = pcAnswer.SetLocalDescription(answer); err != nil {
-		return err
-	}
-
-	err = pcOffer.SetRemoteDescription(answer)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
 func TestNew_Go(t *testing.T) {
 	api := NewAPI()
 	t.Run("Success", func(t *testing.T) {

peerconnection_js.go

@@ -22,7 +22,7 @@ type PeerConnection struct {
 	onDataChannelHandler             *js.Func
 	onICEConectionStateChangeHandler *js.Func
 	onICECandidateHandler            *js.Func
-	onNegotiationNeededHandler       *js.Func
+	onICEGatheringStateChangeHandler *js.Func
 }
 
 // NewPeerConnection creates a peerconnection with the default
@@ -276,36 +276,35 @@ func (pc *PeerConnection) ICEConnectionState() ICEConnectionState {
 	return newICEConnectionState(pc.underlying.Get("iceConnectionState").String())
 }
 
-// TODO(albrow): This function doesn't exist in the Go implementation.
-// TODO(albrow): Follow the spec more closely. Handler should accept
-// RTCPeerConnectionIceEvent instead of *string.
-// https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/onicecandidate
-func (pc *PeerConnection) OnICECandidate(f func(candidate *string)) {
+// OnICECandidate sets an event handler which is invoked when a new ICE
+// candidate is found.
+func (pc *PeerConnection) OnICECandidate(f func(candidate *ICECandidate)) {
 	if pc.onICECandidateHandler != nil {
 		oldHandler := pc.onICECandidateHandler
 		defer oldHandler.Release()
 	}
 	onICECandidateHandler := js.FuncOf(func(this js.Value, args []js.Value) interface{} {
-		candidate := valueToStringPointer(args[0].Get("candidate"))
+		candidate := valueToICECandidate(args[0].Get("candidate"))
 		go f(candidate)
 		return js.Undefined()
 	})
 	pc.onICECandidateHandler = &onICECandidateHandler
 	pc.underlying.Set("onicecandidate", onICECandidateHandler)
 }
 
-// TODO(albrow): This function doesn't exist in the Go implementation.
-func (pc *PeerConnection) OnNegotiationNeeded(f func()) {
-	if pc.onNegotiationNeededHandler != nil {
-		oldHandler := pc.onNegotiationNeededHandler
+// OnICEGatheringStateChange sets an event handler which is invoked when the
+// ICE candidate gathering state has changed.
+func (pc *PeerConnection) OnICEGatheringStateChange(f func()) {
+	if pc.onICEGatheringStateChangeHandler != nil {
+		oldHandler := pc.onICEGatheringStateChangeHandler
 		defer oldHandler.Release()
 	}
-	onNegotiationNeededHandler := js.FuncOf(func(this js.Value, args []js.Value) interface{} {
+	onICEGatheringStateChangeHandler := js.FuncOf(func(this js.Value, args []js.Value) interface{} {
 		go f()
 		return js.Undefined()
 	})
-	pc.onNegotiationNeededHandler = &onNegotiationNeededHandler
-	pc.underlying.Set("onnegotiationneeded", onNegotiationNeededHandler)
+	pc.onICEGatheringStateChangeHandler = &onICEGatheringStateChangeHandler
+	pc.underlying.Set("onicegatheringstatechange", onICEGatheringStateChangeHandler)
 }
 
 // // GetSenders returns the RTPSender that are currently attached to this PeerConnection
@@ -384,8 +383,8 @@ func (pc *PeerConnection) Close() (err error) {
 	if pc.onICECandidateHandler != nil {
 		pc.onICECandidateHandler.Release()
 	}
-	if pc.onNegotiationNeededHandler != nil {
-		pc.onNegotiationNeededHandler.Release()
+	if pc.onICEGatheringStateChangeHandler != nil {
+		pc.onICEGatheringStateChangeHandler.Release()
 	}
 
 	return nil
@@ -529,6 +528,36 @@ func valueToICEServer(iceServerValue js.Value) ICEServer {
 	}
 }
 
+func valueToICECandidate(val js.Value) *ICECandidate {
+	if val == js.Null() || val == js.Undefined() {
+		return nil
+	}
+	protocol, _ := newICEProtocol(val.Get("protocol").String())
+	candidateType, _ := newICECandidateType(val.Get("type").String())
+	return &ICECandidate{
+		Foundation:     val.Get("foundation").String(),
+		Priority:       valueToUint32OrZero(val.Get("priority")),
+		IP:             val.Get("ip").String(),
+		Protocol:       protocol,
+		Port:           valueToUint16OrZero(val.Get("port")),
+		Typ:            candidateType,
+		Component:      stringToComponentIDOrZero(val.Get("component").String()),
+		RelatedAddress: val.Get("relatedAddress").String(),
+		RelatedPort:    valueToUint16OrZero(val.Get("relatedPort")),
+	}
+}
+
+func stringToComponentIDOrZero(val string) uint16 {
+	// See: https://developer.mozilla.org/en-US/docs/Web/API/RTCIceComponent
+	switch val {
+	case "rtp":
+		return 1
+	case "rtcp":
+		return 2
+	}
+	return 0
+}
+
 func sessionDescriptionToValue(desc *SessionDescription) js.Value {
 	if desc == nil {
 		return js.Undefined()