From cfa4fc76ee8bc799bac237305f53a99af26e9698 Mon Sep 17 00:00:00 2001
From: Sean DuBois <sean@siobud.com>
Date: Sat, 1 Jun 2019 00:45:05 -0700
Subject: [PATCH] Minor checklist cleanup

Add helper function to add localCandidates. Brings down the duplication
and make sure we have a properly formed checklist when we are doing
trickle.

When comparing candidates in findPair do by value, and not address.
Before some candidates were failing to be found because of this.
---
 agent.go  | 35 +++++++++++++++++++++--------------
 gather.go | 12 +++---------
 2 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/agent.go b/agent.go
index c18e032..ea1bc81 100644
--- a/agent.go
+++ b/agent.go
@@ -228,6 +228,7 @@ func NewAgent(config *AgentConfig) (*Agent, error) {
 		localCandidates:        make(map[NetworkType][]Candidate),
 		remoteCandidates:       make(map[NetworkType][]Candidate),
 		pendingBindingRequests: make([]bindingRequest, 0, maxPendingBindingRequests),
+		checklist:              make([]*candidatePair, 0),
 		urls:                   config.Urls,
 		networkTypes:           config.NetworkTypes,
 
@@ -370,19 +371,6 @@ func (a *Agent) startConnectivityChecks(isControlling bool, remoteUfrag, remoteP
 		agent.remoteUfrag = remoteUfrag
 		agent.remotePwd = remotePwd
 
-		a.checklist = make([]*candidatePair, 0)
-		for networkType, localCandidates := range a.localCandidates {
-			if remoteCandidates, ok := a.remoteCandidates[networkType]; ok {
-
-				for _, localCandidate := range localCandidates {
-					for _, remoteCandidate := range remoteCandidates {
-						a.addPair(localCandidate, remoteCandidate)
-					}
-				}
-
-			}
-		}
-
 		if isControlling {
 			a.selector = &controllingSelector{agent: a, log: a.log}
 		} else {
@@ -481,7 +469,7 @@ func (a *Agent) addPair(local, remote Candidate) *candidatePair {
 
 func (a *Agent) findPair(local, remote Candidate) *candidatePair {
 	for _, p := range a.checklist {
-		if p.local == local && p.remote == remote {
+		if p.local.Equal(local) && p.remote.Equal(remote) {
 			return p
 		}
 	}
@@ -600,6 +588,25 @@ func (a *Agent) addRemoteCandidate(c Candidate) {
 	}
 }
 
+// addCandidate assumes you are holding the lock (must be execute using a.run)
+func (a *Agent) addCandidate(c Candidate) {
+	set := a.localCandidates[c.NetworkType()]
+	for _, candidate := range set {
+		if candidate.Equal(c) {
+			return
+		}
+	}
+
+	set = append(set, c)
+	a.localCandidates[c.NetworkType()] = set
+
+	if remoteCandidates, ok := a.remoteCandidates[c.NetworkType()]; ok {
+		for _, remoteCandidate := range remoteCandidates {
+			a.addPair(c, remoteCandidate)
+		}
+	}
+}
+
 // GetLocalCandidates returns the local candidates
 func (a *Agent) GetLocalCandidates() ([]Candidate, error) {
 	res := make(chan []Candidate)
diff --git a/gather.go b/gather.go
index ea4f15c..3deed31 100644
--- a/gather.go
+++ b/gather.go
@@ -180,9 +180,7 @@ func (a *Agent) gatherCandidatesLocal(networkTypes []NetworkType) {
 				}
 
 				if err := a.run(func(agent *Agent) {
-					set := a.localCandidates[c.NetworkType()]
-					set = append(set, c)
-					a.localCandidates[c.NetworkType()] = set
+					a.addCandidate(c)
 				}); err != nil {
 					a.log.Warnf("Failed to append to localCandidates: %v\n", err)
 					return
@@ -248,9 +246,7 @@ func (a *Agent) gatherCandidatesSrflx(urls []*URL, networkTypes []NetworkType) {
 					}
 
 					if err := a.run(func(agent *Agent) {
-						set := a.localCandidates[c.NetworkType()]
-						set = append(set, c)
-						a.localCandidates[c.NetworkType()] = set
+						a.addCandidate(c)
 					}); err != nil {
 						a.log.Warnf("Failed to append to localCandidates: %v\n", err)
 						return
@@ -317,9 +313,7 @@ func (a *Agent) gatherCandidatesRelay(urls []*URL) error {
 		}
 		candidate.setAllocation(allocation)
 
-		set := a.localCandidates[candidate.NetworkType()]
-		set = append(set, candidate)
-		a.localCandidates[candidate.NetworkType()] = set
+		a.addCandidate(candidate)
 		candidate.start(a, nil)
 	}