Skip to content

Commit

Permalink
Use ICETransport private fields for PeerConnection
Browse files Browse the repository at this point in the history
PeerConnection used the public OnConnectionStateChange to track the
status of the ICETransport. This was incorrect because a user can
override this value at anytime.

Add a new internalOnConnectionStateChangeHandler that is set directly by
the PeerConnection and not accessible to the user.
  • Loading branch information
Sean-Der committed Apr 23, 2022
1 parent a698156 commit 2cc6ea0
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 15 deletions.
11 changes: 7 additions & 4 deletions icetransport.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ type ICETransport struct {

role ICERole

onConnectionStateChangeHandler atomic.Value // func(ICETransportState)
onSelectedCandidatePairChangeHandler atomic.Value // func(*ICECandidatePair)
onConnectionStateChangeHandler atomic.Value // func(ICETransportState)
internalOnConnectionStateChangeHandler atomic.Value // func(ICETransportState)
onSelectedCandidatePairChangeHandler atomic.Value // func(*ICECandidatePair)

state atomic.Value // ICETransportState

Expand Down Expand Up @@ -220,8 +221,10 @@ func (t *ICETransport) OnConnectionStateChange(f func(ICETransportState)) {
}

func (t *ICETransport) onConnectionStateChange(state ICETransportState) {
handler := t.onConnectionStateChangeHandler.Load()
if handler != nil {
if handler := t.onConnectionStateChangeHandler.Load(); handler != nil {
handler.(func(ICETransportState))(state)
}
if handler := t.internalOnConnectionStateChangeHandler.Load(); handler != nil {
handler.(func(ICETransportState))(state)
}
}
Expand Down
30 changes: 20 additions & 10 deletions icetransport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package webrtc

import (
"sync"
"sync/atomic"
"testing"
"time"
Expand All @@ -22,23 +23,32 @@ func TestICETransport_OnConnectionStateChange(t *testing.T) {
pcOffer, pcAnswer, err := newPair()
assert.NoError(t, err)

iceOfferComplete := make(chan struct{})
iceAnswerComplete := make(chan struct{})
var (
iceComplete sync.WaitGroup
peerConnectionConnected sync.WaitGroup
)
iceComplete.Add(2)
peerConnectionConnected.Add(2)

pcOffer.SCTP().Transport().ICETransport().OnConnectionStateChange(func(s ICETransportState) {
onIceComplete := func(s ICETransportState) {
if s == ICETransportStateConnected {
close(iceOfferComplete)
iceComplete.Done()
}
})
}
pcOffer.SCTP().Transport().ICETransport().OnConnectionStateChange(onIceComplete)
pcAnswer.SCTP().Transport().ICETransport().OnConnectionStateChange(onIceComplete)

pcAnswer.SCTP().Transport().ICETransport().OnConnectionStateChange(func(s ICETransportState) {
if s == ICETransportStateConnected {
close(iceAnswerComplete)
onConnected := func(s PeerConnectionState) {
if s == PeerConnectionStateConnected {
peerConnectionConnected.Done()
}
})
}
pcOffer.OnConnectionStateChange(onConnected)
pcAnswer.OnConnectionStateChange(onConnected)

assert.NoError(t, signalPair(pcOffer, pcAnswer))
<-iceOfferComplete
iceComplete.Wait()
peerConnectionConnected.Wait()

closePairNow(t, pcOffer, pcAnswer)
}
Expand Down
2 changes: 1 addition & 1 deletion peerconnection.go
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ func (pc *PeerConnection) updateConnectionState(iceConnectionState ICEConnection

func (pc *PeerConnection) createICETransport() *ICETransport {
t := pc.api.NewICETransport(pc.iceGatherer)
t.OnConnectionStateChange(func(state ICETransportState) {
t.internalOnConnectionStateChangeHandler.Store(func(state ICETransportState) {
var cs ICEConnectionState
switch state {
case ICETransportStateNew:
Expand Down

0 comments on commit 2cc6ea0

Please sign in to comment.