Bug 1004530 - Part 1: Allow a grace period for trickle candidates to arrive when all candidate pairs have failed. r=drno, r=ekr
authorByron Campen [:bwc] <docfaraday@gmail.com>
Thu, 01 May 2014 14:07:54 -0700
changeset 206593 2f00e356e52b686a43453787095434401278dfe3
parent 206592 553bf8d3404cd2b8ded27786daef295354c733f4
child 206594 ba3a341fe5b880026ccfab61570eb6f449f91063
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdrno, ekr
bugs1004530
milestone32.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1004530 - Part 1: Allow a grace period for trickle candidates to arrive when all candidate pairs have failed. r=drno, r=ekr
media/mtransport/nricectx.cpp
media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
media/mtransport/third_party/nICEr/src/ice/ice_component.c
media/mtransport/third_party/nICEr/src/ice/ice_component.h
media/mtransport/third_party/nICEr/src/ice/ice_media_stream.c
media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.c
media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.h
media/mtransport/third_party/nICEr/src/ice/ice_reg.h
--- a/media/mtransport/nricectx.cpp
+++ b/media/mtransport/nricectx.cpp
@@ -414,16 +414,17 @@ RefPtr<NrIceCtx> NrIceCtx::Create(const 
       NR_reg_set_uchar((char *)"ice.pref.interface.vmnet6", 236);
       NR_reg_set_uchar((char *)"ice.pref.interface.vmnet7", 235);
       NR_reg_set_uchar((char *)"ice.pref.interface.vmnet8", 234);
       NR_reg_set_uchar((char *)"ice.pref.interface.virbr0", 233);
       NR_reg_set_uchar((char *)"ice.pref.interface.wlan0", 232);
     }
 
     NR_reg_set_uint4((char *)"stun.client.maximum_transmits",4);
+    NR_reg_set_uint4((char *)NR_ICE_REG_TRICKLE_GRACE_PERIOD, 5000);
   }
 
   // Create the ICE context
   int r;
 
   UINT4 flags = offerer ? NR_ICE_CTX_FLAGS_OFFERER:
       NR_ICE_CTX_FLAGS_ANSWERER;
   flags |= NR_ICE_CTX_FLAGS_AGGRESSIVE_NOMINATION;
--- a/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
@@ -275,17 +275,17 @@ static void nr_ice_candidate_pair_stun_c
           /* Note: we stomp the existing pair! */
           orig_pair=pair;
           if(r=nr_ice_candidate_pair_create(pair->pctx,cand,pair->remote,
             &pair))
             ABORT(r);
 
           nr_ice_candidate_pair_set_state(pair->pctx,pair,NR_ICE_PAIR_STATE_SUCCEEDED);
 
-          if(r=nr_ice_candidate_pair_insert(&pair->remote->stream->check_list,pair))
+          if(r=nr_ice_component_insert_pair(pair->remote->component,pair))
             ABORT(r);
 
           /* If the original pair was nominated, make us nominated,
              since we replace him*/
           if(orig_pair->peer_nominated)
             pair->peer_nominated=1;
 
 
--- a/media/mtransport/third_party/nICEr/src/ice/ice_component.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_component.c
@@ -636,17 +636,17 @@ static int nr_ice_component_process_inco
 
         if(r=nr_ice_candidate_pair_create(comp->stream->pctx,cand,pcand,
              &pair)) {
           *error=(r==R_NO_MEMORY)?500:400;
           ABORT(r);
         }
         nr_ice_candidate_pair_set_state(pair->pctx,pair,NR_ICE_PAIR_STATE_FROZEN);
 
-        if(r=nr_ice_candidate_pair_insert(&comp->stream->check_list,pair)) {
+        if(r=nr_ice_component_insert_pair(comp,pair)) {
           *error=(r==R_NO_MEMORY)?500:400;
           nr_ice_candidate_pair_destroy(&pair);
           ABORT(r);
         }
 
         /* Do this last, since any call to ABORT will destroy pcand */
         TAILQ_INSERT_TAIL(&comp->candidates,pcand,entry_comp);
         pcand=0;
@@ -800,18 +800,17 @@ int nr_ice_component_pair_candidate(nr_i
           assert (pcand->state == NR_ICE_CAND_PEER_CANDIDATE_PAIRED);
 
         nr_ice_compute_codeword(pcand->label,strlen(pcand->label),codeword);
         r_log(LOG_ICE,LOG_DEBUG,"ICE-PEER(%s)/CAND(%s): Pairing with peer candidate %s", pctx->label, codeword, pcand->label);
 
         if(r=nr_ice_candidate_pair_create(pctx,lcand,pcand,&pair))
           ABORT(r);
 
-        if(r=nr_ice_candidate_pair_insert(&pcomp->stream->check_list,
-                                          pair))
+        if(r=nr_ice_component_insert_pair(pcomp, pair))
           ABORT(r);
       }
 
       pcand=TAILQ_NEXT(pcand,entry_comp);
     }
 
    done:
     _status = 0;
@@ -937,61 +936,64 @@ int nr_ice_component_nominated_pair(nr_i
     if(r=nr_ice_media_stream_component_nominated(comp->stream,comp))
       ABORT(r);
 
     _status=0;
   abort:
     return(_status);
   }
 
-int nr_ice_component_failed_pair(nr_ice_component *comp, nr_ice_cand_pair *pair)
+static int nr_ice_component_have_all_pairs_failed(nr_ice_component *comp)
   {
-    int r,_status;
     nr_ice_cand_pair *p2;
 
-    assert(pair->state == NR_ICE_PAIR_STATE_FAILED);
-
     p2=TAILQ_FIRST(&comp->stream->check_list);
     while(p2){
       if(comp->component_id==p2->local->component_id){
         switch(p2->state){
         case NR_ICE_PAIR_STATE_FROZEN:
         case NR_ICE_PAIR_STATE_WAITING:
         case NR_ICE_PAIR_STATE_IN_PROGRESS:
-            /* answer component status cannot be determined yet */
-            goto done;
-            break;
         case NR_ICE_PAIR_STATE_SUCCEEDED:
-            /* the component will succeed */
-            goto done;
-            break;
+            return(0);
         case NR_ICE_PAIR_STATE_FAILED:
         case NR_ICE_PAIR_STATE_CANCELLED:
             /* states that will never be recovered from */
             break;
         default:
             assert(0);
             break;
         }
       }
 
       p2=TAILQ_NEXT(p2,entry);
     }
 
-    /* all the pairs in the component are in their final states with
-     * none of them being SUCCEEDED, so the component fails entirely,
-     * tell the media stream that this component has failed */
+    return(1);
+  }
+
+int nr_ice_component_failed_pair(nr_ice_component *comp, nr_ice_cand_pair *pair)
+  {
+    return nr_ice_component_check_if_failed(comp);
+  }
 
-    if(r=nr_ice_media_stream_component_failed(comp->stream,comp))
-      ABORT(r);
+int nr_ice_component_check_if_failed(nr_ice_component *comp)
+  {
+    if (comp->state == NR_ICE_COMPONENT_RUNNING) {
+      /* Don't do anything to streams that aren't currently running */
+      r_log(LOG_ICE,LOG_DEBUG,"ICE-PEER(%s)/STREAM(%s)/COMP(%d): Checking whether component needs to be marked failed.",comp->stream->pctx->label,comp->stream->label,comp->component_id);
 
-  done:
-    _status=0;
-  abort:
-    return(_status);
+      if (!comp->stream->pctx->trickle_grace_period_timer &&
+          nr_ice_component_have_all_pairs_failed(comp)) {
+        r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s)/STREAM(%s)/COMP(%d): All pairs are failed, and grace period has elapsed. Marking component as failed.",comp->stream->pctx->label,comp->stream->label,comp->component_id);
+        return nr_ice_media_stream_component_failed(comp->stream,comp);
+      }
+    }
+
+    return(0);
   }
 
 int nr_ice_component_select_pair(nr_ice_peer_ctx *pctx, nr_ice_component *comp)
   {
     nr_ice_cand_pair **pairs=0;
     int ct=0;
     nr_ice_cand_pair *pair;
     int r,_status;
@@ -1037,17 +1039,16 @@ static void nr_ice_component_keepalive_c
     UINT4 keepalive_timeout;
 
     assert(comp->keepalive_ctx);
 
     if(NR_reg_get_uint4(NR_ICE_REG_KEEPALIVE_TIMER,&keepalive_timeout)){
       keepalive_timeout=15000; /* Default */
     }
 
-
     if(comp->keepalive_needed)
       nr_stun_client_force_retransmit(comp->keepalive_ctx);
 
     comp->keepalive_needed=1;
     NR_ASYNC_TIMER_SET(keepalive_timeout,nr_ice_component_keepalive_cb,cb_arg,&comp->keepalive_timer);
   }
 
 
@@ -1079,8 +1080,40 @@ int nr_ice_component_finalize(nr_ice_com
     nr_ice_component_keepalive_cb(0,0,rcomp);
 
 
     _status=0;
   abort:
 
     return(_status);
   }
+
+
+int nr_ice_component_insert_pair(nr_ice_component *pcomp, nr_ice_cand_pair *pair)
+  {
+    int r,_status;
+
+    /* Pairs for peer reflexive are marked SUCCEEDED immediately */
+    if (pair->state != NR_ICE_PAIR_STATE_FROZEN &&
+        pair->state != NR_ICE_PAIR_STATE_SUCCEEDED){
+      assert(0);
+      ABORT(R_BAD_ARGS);
+    }
+
+    if(r=nr_ice_candidate_pair_insert(&pair->remote->stream->check_list,pair))
+      ABORT(r);
+
+    /* Make sure the check timer is running, if the stream was previously
+     * started. We will not start streams just because a pair was created. */
+    r_log(LOG_ICE,LOG_DEBUG,"ICE-PEER(%s)/CAND-PAIR(%s): Ensure that check timer is running for new pair %s.",pair->remote->stream->pctx->label, pair->codeword, pair->as_string);
+
+    if(pair->remote->stream->ice_state == NR_ICE_MEDIA_STREAM_CHECKS_ACTIVE){
+      if(nr_ice_media_stream_start_checks(pair->remote->stream->pctx, pair->remote->stream)) {
+        r_log(LOG_ICE,LOG_WARNING,"ICE-PEER(%s)/CAND-PAIR(%s): Could not restart checks for new pair %s.",pair->remote->stream->pctx->label, pair->codeword, pair->as_string);
+        ABORT(R_INTERNAL);
+      }
+    }
+
+    _status=0;
+  abort:
+    return(_status);
+  }
+
--- a/media/mtransport/third_party/nICEr/src/ice/ice_component.h
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_component.h
@@ -83,16 +83,18 @@ int nr_ice_component_create(struct nr_ic
 int nr_ice_component_destroy(nr_ice_component **componentp);
 int nr_ice_component_initialize(struct nr_ice_ctx_ *ctx,nr_ice_component *component);
 int nr_ice_component_maybe_prune_candidate(nr_ice_ctx *ctx, nr_ice_component *comp, nr_ice_candidate *c1, int *was_pruned);
 int nr_ice_component_pair_candidate(nr_ice_peer_ctx *pctx, nr_ice_component *pcomp, nr_ice_candidate *lcand, int pair_all_remote);
 int nr_ice_component_pair_candidates(nr_ice_peer_ctx *pctx, nr_ice_component *lcomp, nr_ice_component *pcomp);
 int nr_ice_component_service_pre_answer_requests(nr_ice_peer_ctx *pctx, nr_ice_component *pcomp, char *username, int *serviced);
 int nr_ice_component_nominated_pair(nr_ice_component *comp, nr_ice_cand_pair *pair);
 int nr_ice_component_failed_pair(nr_ice_component *comp, nr_ice_cand_pair *pair);
+int nr_ice_component_check_if_failed(nr_ice_component *comp);
 int nr_ice_component_select_pair(nr_ice_peer_ctx *pctx, nr_ice_component *comp);
 int nr_ice_component_set_failed(nr_ice_component *comp);
 int nr_ice_component_finalize(nr_ice_component *lcomp, nr_ice_component *rcomp);
+int nr_ice_component_insert_pair(nr_ice_component *pcomp, nr_ice_cand_pair *pair);
 
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
 #endif
--- a/media/mtransport/third_party/nICEr/src/ice/ice_media_stream.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_media_stream.c
@@ -389,20 +389,35 @@ static void nr_ice_media_stream_check_ti
   abort:
     return;
   }
 
 
 /* Start checks for this media stream (aka check list) */
 int nr_ice_media_stream_start_checks(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream)
   {
-    nr_ice_media_stream_set_state(stream,NR_ICE_MEDIA_STREAM_CHECKS_ACTIVE);
-    nr_ice_media_stream_check_timer_cb(0,0,stream);
+    int r,_status;
+
+    /* Don't start the check timer if the stream is done (failed/completed) */
+    if (stream->ice_state > NR_ICE_MEDIA_STREAM_CHECKS_ACTIVE) {
+      assert(0);
+      ABORT(R_INTERNAL);
+    }
 
-    return(0);
+    if(r=nr_ice_media_stream_set_state(stream,NR_ICE_MEDIA_STREAM_CHECKS_ACTIVE))
+      ABORT(r);
+
+    if (!stream->timer) {
+      r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s)/ICE-STREAM(%s): Starting check timer for stream.",pctx->label,stream->label);
+      nr_ice_media_stream_check_timer_cb(0,0,stream);
+    }
+
+    _status=0;
+  abort:
+    return(_status);
   }
 
 /* Start checks for this media stream (aka check list) S 5.7 */
 int nr_ice_media_stream_unfreeze_pairs(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream)
   {
     int r,_status;
     r_assoc *assoc=0;
     nr_ice_cand_pair *pair=0;
--- a/media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.c
@@ -31,23 +31,25 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 */
 
 
 
 static char *RCSSTRING __UNUSED__="$Id: ice_peer_ctx.c,v 1.2 2008/04/28 17:59:01 ekr Exp $";
 
 #include <string.h>
 #include <assert.h>
+#include <registry.h>
 #include <nr_api.h>
 #include "ice_ctx.h"
 #include "ice_peer_ctx.h"
 #include "ice_media_stream.h"
 #include "ice_util.h"
 #include "nr_crypto.h"
 #include "async_timer.h"
+#include "ice_reg.h"
 
 static void nr_ice_peer_ctx_destroy_cb(NR_SOCKET s, int how, void *cb_arg);
 static int nr_ice_peer_ctx_parse_stream_attributes_int(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream, nr_ice_media_stream *pstream, char **attrs, int attr_ct);
 static int nr_ice_ctx_parse_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_stream *pstream, char *candidate);
 
 int nr_ice_peer_ctx_create(nr_ice_ctx *ctx, nr_ice_handler *handler,char *label, nr_ice_peer_ctx **pctxp)
   {
     int r,_status;
@@ -318,16 +320,50 @@ int nr_ice_peer_ctx_parse_trickle_candid
 
     _status=0;
  abort:
     return(_status);
 
   }
 
 
+static void nr_ice_peer_ctx_trickle_wait_cb(NR_SOCKET s, int how, void *cb_arg)
+  {
+    nr_ice_peer_ctx *pctx=cb_arg;
+    nr_ice_media_stream *stream;
+    nr_ice_component *comp;
+
+    pctx->trickle_grace_period_timer=0;
+
+    r_log(LOG_ICE,LOG_INFO,"ICE(%s): peer (%s) Trickle grace period is over; marking every component with only failed pairs as failed.",pctx->ctx->label,pctx->label);
+
+    stream=STAILQ_FIRST(&pctx->peer_streams);
+    while(stream){
+      comp=STAILQ_FIRST(&stream->components);
+      while(comp){
+        nr_ice_component_check_if_failed(comp);
+        comp=STAILQ_NEXT(comp,entry);
+      }
+      stream=STAILQ_NEXT(stream,entry);
+    }
+  }
+
+static void nr_ice_peer_ctx_start_trickle_timer(nr_ice_peer_ctx *pctx)
+  {
+    UINT4 grace_period_timeout=0;
+
+    NR_reg_get_uint4(NR_ICE_REG_TRICKLE_GRACE_PERIOD,&grace_period_timeout);
+
+    if (grace_period_timeout) {
+      /* If we're doing trickle, we need to allow a grace period for new
+       * trickle candidates to arrive in case the pairs we have fail quickly. */
+       NR_ASYNC_TIMER_SET(grace_period_timeout,nr_ice_peer_ctx_trickle_wait_cb,pctx,&pctx->trickle_grace_period_timer);
+    }
+  }
+
 int nr_ice_peer_ctx_pair_candidates(nr_ice_peer_ctx *pctx)
   {
     nr_ice_media_stream *stream;
     int r,_status;
 
 
     r_log(LOG_ICE,LOG_DEBUG,"ICE(%s): peer (%s) pairing candidates",pctx->ctx->label,pctx->label);
 
@@ -335,16 +371,19 @@ int nr_ice_peer_ctx_pair_candidates(nr_i
         r_log(LOG_ICE,LOG_ERR,"ICE(%s): peer (%s) received no media stream attributes",pctx->ctx->label,pctx->label);
         ABORT(R_FAILED);
     }
 
     /* Set this first; if we fail partway through, we do not want to end
      * up in UNPAIRED after creating some pairs. */
     pctx->state = NR_ICE_PEER_STATE_PAIRED;
 
+    /* Start grace period timer for incoming trickle candidates */
+    nr_ice_peer_ctx_start_trickle_timer(pctx);
+
     stream=STAILQ_FIRST(&pctx->peer_streams);
     while(stream){
       if(r=nr_ice_media_stream_pair_candidates(pctx, stream->local_stream,
         stream))
         ABORT(r);
 
       stream=STAILQ_NEXT(stream,entry);
     }
@@ -410,16 +449,21 @@ static void nr_ice_peer_ctx_destroy_cb(N
     STAILQ_FOREACH_SAFE(str1, &pctx->peer_streams, entry, str2){
       STAILQ_REMOVE(&pctx->peer_streams,str1,nr_ice_media_stream_,entry);
       nr_ice_media_stream_destroy(&str1);
     }
     assert(pctx->ctx);
     if (pctx->ctx)
       STAILQ_REMOVE(&pctx->ctx->peers, pctx, nr_ice_peer_ctx_, entry);
 
+    if(pctx->trickle_grace_period_timer) {
+      NR_async_timer_cancel(pctx->trickle_grace_period_timer);
+      pctx->trickle_grace_period_timer=0;
+    }
+
     RFREE(pctx);
   }
 
 int nr_ice_peer_ctx_destroy(nr_ice_peer_ctx **pctxp)
   {
 
     if(!pctxp || !*pctxp)
       return(0);
--- a/media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.h
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.h
@@ -57,16 +57,17 @@ struct nr_ice_peer_ctx_ {
   int peer_ice_mismatch;
 
   nr_ice_media_stream_head peer_streams;
   int active_streams;
   int waiting_pairs;
 
   void *done_cb_timer;
   UCHAR reported_done;
+  void *trickle_grace_period_timer;
 
   STAILQ_ENTRY(nr_ice_peer_ctx_) entry;
 };
 
 typedef STAILQ_HEAD(nr_ice_peer_ctx_head_, nr_ice_peer_ctx_) nr_ice_peer_ctx_head;
 
 int nr_ice_peer_ctx_create(nr_ice_ctx *ctx, nr_ice_handler *handler,char *label, nr_ice_peer_ctx **pctxp);
 int nr_ice_peer_ctx_destroy(nr_ice_peer_ctx **pctxp);
--- a/media/mtransport/third_party/nICEr/src/ice/ice_reg.h
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_reg.h
@@ -57,13 +57,14 @@ extern "C" {
 #define NR_ICE_REG_TURN_SRV_PORT        "port"
 #define NR_ICE_REG_TURN_SRV_BANDWIDTH   "bandwidth"
 #define NR_ICE_REG_TURN_SRV_LIFETIME    "lifetime"
 #define NR_ICE_REG_TURN_SRV_USERNAME    "username"
 #define NR_ICE_REG_TURN_SRV_PASSWORD    "password"
 
 #define NR_ICE_REG_KEEPALIVE_TIMER      "ice.keepalive_timer"
 
+#define NR_ICE_REG_TRICKLE_GRACE_PERIOD "ice.trickle_grace_period"
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
 #endif