Bug 1298991: only create candidate pairs from STUN response when needed. r=bwc
MozReview-Commit-ID: KpqNyi0FVb3
--- a/media/mtransport/test/ice_unittest.cpp
+++ b/media/mtransport/test/ice_unittest.cpp
@@ -758,17 +758,17 @@ class IceTestPeer : public sigslot::has_
RefPtr<NrIceMediaStream> aStream = ice_ctx_->ctx()->GetStream(i);
if (!aStream || aStream->HasParsedAttributes()) {
continue;
}
std::vector<std::string> candidates =
remote->GetCandidates(i);
for (size_t j=0; j<candidates.size(); ++j) {
- std::cerr << name_ << " Candidate: " + candidates[j] << std::endl;
+ std::cerr << name_ << " Adding remote candidate: " + candidates[j] << std::endl;
}
res = aStream->ParseAttributes(candidates);
ASSERT_TRUE(NS_SUCCEEDED(res));
}
} else {
// Parse empty attributes and then trickle them out later
for (size_t i=0; i<ice_ctx_->ctx()->GetStreamCount(); ++i) {
RefPtr<NrIceMediaStream> aStream = ice_ctx_->ctx()->GetStream(i);
--- a/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
@@ -176,17 +176,18 @@ int nr_ice_candidate_pair_unfreeze(nr_ic
nr_ice_candidate_pair_set_state(pctx,pair,NR_ICE_PAIR_STATE_WAITING);
return(0);
}
static void nr_ice_candidate_pair_stun_cb(NR_SOCKET s, int how, void *cb_arg)
{
int r,_status;
- nr_ice_cand_pair *pair=cb_arg,*orig_pair;
+ nr_ice_cand_pair *pair=cb_arg;
+ nr_ice_cand_pair *actual_pair=0;
nr_ice_candidate *cand=0;
nr_stun_message *sres;
nr_transport_addr *request_src;
nr_transport_addr *request_dst;
nr_transport_addr *response_src;
nr_transport_addr response_dst;
nr_stun_message_attribute *attr;
@@ -251,58 +252,65 @@ static void nr_ice_candidate_pair_stun_c
nr_ice_candidate_pair_set_state(pair->pctx,pair,NR_ICE_PAIR_STATE_SUCCEEDED);
}
else if(pair->stun_client->state == NR_STUN_CLIENT_STATE_DONE) {
/* OK, this didn't correspond to a pair on the check list, but
it probably matches one of our candidates */
cand=TAILQ_FIRST(&pair->local->component->candidates);
while(cand){
- if(!nr_transport_addr_cmp(&cand->addr,&pair->stun_client->results.ice_binding_response.mapped_addr,NR_TRANSPORT_ADDR_CMP_MODE_ALL))
+ if(!nr_transport_addr_cmp(&cand->addr,&pair->stun_client->results.ice_binding_response.mapped_addr,NR_TRANSPORT_ADDR_CMP_MODE_ALL)) {
+ r_log(LOG_ICE,LOG_DEBUG,"ICE-PEER(%s): found pre-existing local candidate of type %d for mapped address %s", pair->pctx->label,cand->type,cand->addr.as_string);
+ assert(cand->type != HOST);
break;
+ }
cand=TAILQ_NEXT(cand,entry_comp);
}
- /* OK, nothing found, must be peer reflexive */
if(!cand) {
+ /* OK, nothing found, must be a new peer reflexive */
if (pair->pctx->ctx->flags & NR_ICE_CTX_FLAGS_RELAY_ONLY) {
/* Any STUN response with a reflexive address in it is unwanted
when we'll send on relay only. Bail since cand is used below. */
goto done;
}
if(r=nr_ice_candidate_create(pair->pctx->ctx,
pair->local->component,pair->local->isock,pair->local->osock,
PEER_REFLEXIVE,pair->local->tcp_type,0,pair->local->component->component_id,&cand))
ABORT(r);
if(r=nr_transport_addr_copy(&cand->addr,&pair->stun_client->results.ice_binding_response.mapped_addr))
ABORT(r);
cand->state=NR_ICE_CAND_STATE_INITIALIZED;
TAILQ_INSERT_TAIL(&pair->local->component->candidates,cand,entry_comp);
+ } else {
+ /* Check if we have a pair for this candidate already. */
+ if(r=nr_ice_media_stream_find_pair(pair->remote->stream, cand, pair->remote, &actual_pair)) {
+ r_log(LOG_ICE,LOG_DEBUG,"ICE-PEER(%s): no pair exists for %s and %s", pair->pctx->label,cand->addr.as_string, pair->remote->addr.as_string);
+ }
}
- /* Note: we stomp the existing pair! */
- orig_pair=pair;
- if(r=nr_ice_candidate_pair_create(pair->pctx,cand,pair->remote,
- &pair))
- ABORT(r);
+ if(!actual_pair) {
+ if(r=nr_ice_candidate_pair_create(pair->pctx,cand,pair->remote, &actual_pair))
+ ABORT(r);
- nr_ice_candidate_pair_set_state(pair->pctx,pair,NR_ICE_PAIR_STATE_SUCCEEDED);
+ if(r=nr_ice_component_insert_pair(actual_pair->remote->component,actual_pair))
+ ABORT(r);
- if(r=nr_ice_component_insert_pair(pair->remote->component,pair))
- ABORT(r);
+ /* If the original pair was nominated, make us nominated too. */
+ if(pair->peer_nominated)
+ actual_pair->peer_nominated=1;
- /* If the original pair was nominated, make us nominated,
- since we replace him*/
- if(orig_pair->peer_nominated)
- pair->peer_nominated=1;
+ /* Now mark the orig pair failed */
+ nr_ice_candidate_pair_set_state(pair->pctx,pair,NR_ICE_PAIR_STATE_FAILED);
+ }
-
- /* Now mark the orig pair failed */
- nr_ice_candidate_pair_set_state(orig_pair->pctx,orig_pair,NR_ICE_PAIR_STATE_FAILED);
+ assert(actual_pair);
+ nr_ice_candidate_pair_set_state(actual_pair->pctx,actual_pair,NR_ICE_PAIR_STATE_SUCCEEDED);
+ pair=actual_pair;
}
/* Should we set nominated? */
if(pair->pctx->controlling){
if(pair->pctx->ctx->flags & NR_ICE_CTX_FLAGS_AGGRESSIVE_NOMINATION)
pair->nominated=1;
}
--- a/media/mtransport/third_party/nICEr/src/ice/ice_media_stream.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_media_stream.c
@@ -908,8 +908,26 @@ void nr_ice_media_stream_role_change(nr_
/* Re-insert into the check list */
TAILQ_FOREACH_SAFE(pair,&old_checklist,check_queue_entry,temp_pair) {
TAILQ_REMOVE(&old_checklist,pair,check_queue_entry);
nr_ice_candidate_pair_role_change(pair);
nr_ice_candidate_pair_insert(&stream->check_list,pair);
}
}
+int nr_ice_media_stream_find_pair(nr_ice_media_stream *str, nr_ice_candidate *lcand, nr_ice_candidate *rcand, nr_ice_cand_pair **pair)
+ {
+ nr_ice_cand_pair_head *head = &str->check_list;
+ nr_ice_cand_pair *c1;
+
+ c1=TAILQ_FIRST(head);
+ while(c1){
+ if(c1->local == lcand &&
+ c1->remote == rcand) {
+ *pair=c1;
+ return(0);
+ }
+
+ c1=TAILQ_NEXT(c1,check_queue_entry);
+ }
+
+ return(R_NOT_FOUND);
+ }
--- a/media/mtransport/third_party/nICEr/src/ice/ice_media_stream.h
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_media_stream.h
@@ -87,16 +87,17 @@ int nr_ice_media_stream_unfreeze_pairs_f
int nr_ice_media_stream_dump_state(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream,FILE *out);
int nr_ice_media_stream_component_nominated(nr_ice_media_stream *stream,nr_ice_component *component);
int nr_ice_media_stream_component_failed(nr_ice_media_stream *stream,nr_ice_component *component);
int nr_ice_media_stream_set_state(nr_ice_media_stream *str, int state);
int nr_ice_media_stream_get_best_candidate(nr_ice_media_stream *str, int component, nr_ice_candidate **candp);
int nr_ice_media_stream_send(nr_ice_peer_ctx *pctx, nr_ice_media_stream *str, int component, UCHAR *data, int len);
int nr_ice_media_stream_get_active(nr_ice_peer_ctx *pctx, nr_ice_media_stream *str, int component, nr_ice_candidate **local, nr_ice_candidate **remote);
int nr_ice_media_stream_find_component(nr_ice_media_stream *str, int comp_id, nr_ice_component **compp);
+int nr_ice_media_stream_find_pair(nr_ice_media_stream *str, nr_ice_candidate *local, nr_ice_candidate *remote, nr_ice_cand_pair **pair);
int nr_ice_media_stream_addrs(nr_ice_peer_ctx *pctx, nr_ice_media_stream *str, int component, nr_transport_addr *local, nr_transport_addr *remote);
int
nr_ice_peer_ctx_parse_media_stream_attribute(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream, char *attr);
int nr_ice_media_stream_get_consent_status(nr_ice_media_stream *stream, int component_id, int *can_send, struct timeval *ts);
int nr_ice_media_stream_disable_component(nr_ice_media_stream *stream, int component_id);
int nr_ice_media_stream_pair_new_trickle_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_stream *pstream, nr_ice_candidate *cand);
void nr_ice_media_stream_role_change(nr_ice_media_stream *stream);