Bug 1187775 - skip host and reflexive ICE candidates if relay-only. r=bwc
authorJan-Ivar Bruaroey <jib@mozilla.com>
Wed, 05 Aug 2015 08:22:55 -0400
changeset 288470 4ea1373a7a77c1a2c750eef234dcc07b1568a349
parent 288469 1e6ed0e57113a379c8d2adfa6dd7caa5e1f8c3ff
child 288471 25a43f6817f4acf33832ca8543ecf3ba083d9ce2
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc
bugs1187775
milestone42.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 1187775 - skip host and reflexive ICE candidates if relay-only. r=bwc
dom/media/tests/mochitest/mochitest.ini
dom/media/tests/mochitest/pc.js
dom/media/tests/mochitest/test_peerConnection_relayOnly.html
media/mtransport/nricectx.cpp
media/mtransport/third_party/nICEr/src/ice/ice_candidate.c
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_ctx.h
--- a/dom/media/tests/mochitest/mochitest.ini
+++ b/dom/media/tests/mochitest/mochitest.ini
@@ -113,16 +113,18 @@ skip-if = toolkit == 'gonk' # B2G emulat
 skip-if = toolkit == 'gonk' # B2G emulator is too slow to handle a two-way audio call reliably
 [test_peerConnection_offerRequiresReceiveAudio.html]
 [test_peerConnection_offerRequiresReceiveVideo.html]
 skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g(Bug 960442, video support for WebRTC is disabled on b2g)
 [test_peerConnection_offerRequiresReceiveVideoAudio.html]
 skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g(Bug 960442, video support for WebRTC is disabled on b2g)
 [test_peerConnection_promiseSendOnly.html]
 skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g(Bug 960442, video support for WebRTC is disabled on b2g)
+[test_peerConnection_relayOnly.html]
+skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g(Bug 960442, video support for WebRTC is disabled on b2g)
 [test_peerConnection_callbacks.html]
 skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g(Bug 960442, video support for WebRTC is disabled on b2g)
 [test_peerConnection_replaceTrack.html]
 skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g(Bug 960442, video support for WebRTC is disabled on b2g)
 [test_peerConnection_syncSetDescription.html]
 skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g(Bug 960442, video support for WebRTC is disabled on b2g)
 [test_peerConnection_setLocalAnswerInHaveLocalOffer.html]
 [test_peerConnection_setLocalAnswerInStable.html]
--- a/dom/media/tests/mochitest/pc.js
+++ b/dom/media/tests/mochitest/pc.js
@@ -1184,28 +1184,25 @@ PeerConnectionWrapper.prototype = {
    * Registers a callback for the ICE connection state change and
    * reports success (=connected) or failure via the callbacks.
    * States "new" and "checking" are ignored.
    *
    * @returns {Promise}
    *          resolves when connected, rejects on failure
    */
   waitForIceConnected : function() {
-    return new Promise((resolve, reject) => {
-      var iceConnectedChanged = () => {
-        if (this.isIceConnected()) {
-          delete this.ice_connection_callbacks.waitForIceConnected;
-          resolve();
-        } else if (! this.isIceConnectionPending()) {
-          delete this.ice_connection_callbacks.waitForIceConnected;
-          resolve();
-        }
+    return new Promise((resolve, reject) =>
+        this.ice_connection_callbacks.waitForIceConnected = () => {
+      if (this.isIceConnected()) {
+        delete this.ice_connection_callbacks.waitForIceConnected;
+        resolve();
+      } else if (!this.isIceConnectionPending()) {
+        delete this.ice_connection_callbacks.waitForIceConnected;
+        reject(new Error('ICE failed'));
       }
-
-      this.ice_connection_callbacks.waitForIceConnected = iceConnectedChanged;
     });
   },
 
   /**
    * Setup a onicecandidate handler
    *
    * @param {object} test
    *        A PeerConnectionTest object to which the ice candidates gets
new file mode 100644
--- /dev/null
+++ b/dom/media/tests/mochitest/test_peerConnection_relayOnly.html
@@ -0,0 +1,61 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <script type="application/javascript" src="pc.js"></script>
+</head>
+<body>
+<pre id="test">
+<script type="application/javascript">
+createHTML({
+  bug: "1187775",
+  title: "peer connection ICE fails on relay-only without TURN"
+});
+
+function PC_LOCAL_NO_CANDIDATES(test) {
+  var isnt = can => is(can, null, "No candidates: " + JSON.stringify(can));
+  test.pcLocal._pc.addEventListener("icecandidate", e => isnt(e.candidate));
+}
+
+function PC_BOTH_WAIT_FOR_ICE_FAILED(test) {
+  var isFail = (f, reason, msg) =>
+    f().then(() => { throw new Error(msg + " must fail"); },
+             e => is(e.message, reason, msg + " must fail with: " + e.message));
+
+  return Promise.all([
+    isFail(() => waitForIceConnected(test, test.pcLocal), "ICE failed", "Local ICE"),
+    isFail(() => waitForIceConnected(test, test.pcRemote), "ICE failed", "Remote ICE")
+  ])
+  .then(() => ok(true, "ICE on both sides must fail."));
+}
+
+var pushPrefs = (...p) => new Promise(r => SpecialPowers.pushPrefEnv({set: p}, r));
+var test;
+
+runNetworkTest(options =>
+    pushPrefs(['media.peerconnection.ice.stun_client_maximum_transmits', 3],
+              ['media.peerconnection.ice.trickle_grace_period', 5000]).then(() => {
+  options = options || {};
+  options.config_local = options.config_local || {};
+  var servers = options.config_local.iceServers || [];
+  // remove any turn servers
+  options.config_local.iceServers = servers.filter(server =>
+      server.urls.every(u => !u.toLowerCase().startsWith('turn')));
+
+  // Here's the setting we're testing. Comment out and this test should fail:
+  options.config_local.iceTransportPolicy = "relay";
+
+  test = new PeerConnectionTest(options);
+  test.setMediaConstraints([{audio: true}, {video: true}],
+                           [{audio: true}, {video: true}]);
+  test.chain.remove("PC_LOCAL_SETUP_ICE_LOGGER");  // Needed to suppress failing
+  test.chain.remove("PC_REMOTE_SETUP_ICE_LOGGER"); // on ICE-failure.
+  test.chain.insertAfter("PC_LOCAL_SETUP_ICE_HANDLER", PC_LOCAL_NO_CANDIDATES);
+  test.chain.replace("PC_LOCAL_WAIT_FOR_ICE_CONNECTED", PC_BOTH_WAIT_FOR_ICE_FAILED);
+  test.chain.removeAfter("PC_BOTH_WAIT_FOR_ICE_FAILED");
+  test.run();
+}));
+
+</script>
+</pre>
+</body>
+</html>
--- a/media/mtransport/nricectx.cpp
+++ b/media/mtransport/nricectx.cpp
@@ -481,16 +481,19 @@ RefPtr<NrIceCtx> NrIceCtx::Create(const 
   }
 
   // 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;
+  if (policy == ICE_POLICY_RELAY) {
+    flags |= NR_ICE_CTX_FLAGS_RELAY_ONLY;
+  }
 
   r = nr_ice_ctx_create(const_cast<char *>(name.c_str()), flags,
                         &ctx->ctx_);
   if (r) {
     MOZ_MTLOG(ML_ERROR, "Couldn't create ICE ctx for '" << name << "'");
     return nullptr;
   }
 
@@ -715,16 +718,19 @@ abort:
     nr_socket_wrapper_factory_destroy(&wrapper);
     return NS_ERROR_FAILURE;
   }
   return NS_OK;
 }
 
 nsresult NrIceCtx::StartGathering() {
   ASSERT_ON_THREAD(sts_target_);
+  if (policy_ == ICE_POLICY_NONE) {
+    return NS_OK;
+  }
   SetGatheringState(ICE_CTX_GATHER_STARTED);
   // This might start gathering for the first time, or again after
   // renegotiation, or might do nothing at all if gathering has already
   // finished.
   int r = nr_ice_gather(ctx_, &NrIceCtx::gather_cb, this);
 
   if (!r) {
     SetGatheringState(ICE_CTX_GATHER_COMPLETE);
@@ -789,16 +795,21 @@ nsresult NrIceCtx::ParseGlobalAttributes
   }
 
   return NS_OK;
 }
 
 nsresult NrIceCtx::StartChecks() {
   int r;
 
+  if (policy_ == ICE_POLICY_NONE) {
+    MOZ_MTLOG(ML_ERROR, "Couldn't start peer checks because policy == none");
+    SetConnectionState(ICE_CTX_FAILED);
+    return NS_ERROR_FAILURE;
+  }
   r=nr_ice_peer_ctx_pair_candidates(peer_);
   if (r) {
     MOZ_MTLOG(ML_ERROR, "Couldn't pair candidates on "
               << name_ << "'");
     SetConnectionState(ICE_CTX_FAILED);
     return NS_ERROR_FAILURE;
   }
 
--- a/media/mtransport/third_party/nICEr/src/ice/ice_candidate.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_candidate.c
@@ -129,16 +129,17 @@ static int nr_ice_candidate_format_stun_
 
     _status=0;
    abort:
     return(_status);
   }
 
 int nr_ice_candidate_create(nr_ice_ctx *ctx,nr_ice_component *comp,nr_ice_socket *isock, nr_socket *osock, nr_ice_candidate_type ctype, nr_socket_tcp_type tcp_type, nr_ice_stun_server *stun_server, UCHAR component_id, nr_ice_candidate **candp)
   {
+    assert(!(ctx->flags & NR_ICE_CTX_FLAGS_RELAY_ONLY) || ctype == RELAYED);
     nr_ice_candidate *cand=0;
     nr_ice_candidate *tmp=0;
     int r,_status;
     char label[512];
 
     if(!(cand=RCALLOC(sizeof(nr_ice_candidate))))
       ABORT(R_NO_MEMORY);
     cand->state=NR_ICE_CAND_STATE_CREATED;
@@ -916,16 +917,17 @@ static void nr_ice_turn_allocated_cb(NR_
 
 /* Format the candidate attribute as per ICE S 15.1 */
 int nr_ice_format_candidate_attribute(nr_ice_candidate *cand, char *attr, int maxlen)
   {
     int r,_status;
     char addr[64];
     int port;
     int len;
+    nr_transport_addr *raddr;
 
     assert(!strcmp(nr_ice_candidate_type_names[HOST], "host"));
     assert(!strcmp(nr_ice_candidate_type_names[RELAYED], "relay"));
 
     if(r=nr_transport_addr_get_addrstring(&cand->addr,addr,sizeof(addr)))
       ABORT(r);
     if(r=nr_transport_addr_get_port(&cand->addr,&port))
       ABORT(r);
@@ -934,33 +936,36 @@ int nr_ice_format_candidate_attribute(nr
       port=9;
     snprintf(attr,maxlen,"candidate:%s %d %s %u %s %d typ %s",
       cand->foundation, cand->component_id, cand->addr.protocol==IPPROTO_UDP?"UDP":"TCP",cand->priority, addr, port,
       nr_ctype_name(cand->type));
 
     len=strlen(attr); attr+=len; maxlen-=len;
 
     /* raddr, rport */
+    raddr = (cand->stream->ctx->flags & NR_ICE_CTX_FLAGS_RELAY_ONLY) ?
+      &cand->addr : &cand->base;
+
     switch(cand->type){
       case HOST:
         break;
       case SERVER_REFLEXIVE:
       case PEER_REFLEXIVE:
-        if(r=nr_transport_addr_get_addrstring(&cand->base,addr,sizeof(addr)))
+        if(r=nr_transport_addr_get_addrstring(raddr,addr,sizeof(addr)))
           ABORT(r);
-        if(r=nr_transport_addr_get_port(&cand->base,&port))
+        if(r=nr_transport_addr_get_port(raddr,&port))
           ABORT(r);
 
         snprintf(attr,maxlen," raddr %s rport %d",addr,port);
         break;
       case RELAYED:
         // comes from XorMappedAddress via AllocateResponse
-        if(r=nr_transport_addr_get_addrstring(&cand->base,addr,sizeof(addr)))
+        if(r=nr_transport_addr_get_addrstring(raddr,addr,sizeof(addr)))
           ABORT(r);
-        if(r=nr_transport_addr_get_port(&cand->base,&port))
+        if(r=nr_transport_addr_get_port(raddr,&port))
           ABORT(r);
 
         snprintf(attr,maxlen," raddr %s rport %d",addr,port);
         break;
       default:
         assert(0);
         ABORT(R_INTERNAL);
         break;
--- a/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
@@ -255,17 +255,22 @@ static void nr_ice_candidate_pair_stun_c
           while(cand){
             if(!nr_transport_addr_cmp(&cand->addr,&pair->stun_client->results.ice_binding_response.mapped_addr,NR_TRANSPORT_ADDR_CMP_MODE_ALL))
               break;
 
             cand=TAILQ_NEXT(cand,entry_comp);
           }
 
           /* OK, nothing found, must be peer reflexive */
-          if(!cand){
+          if(!cand) {
+            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);
--- a/media/mtransport/third_party/nICEr/src/ice/ice_component.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_component.c
@@ -221,63 +221,69 @@ static int nr_ice_component_initialize_u
       r_log(LOG_ICE,LOG_DEBUG,"ICE(%s): host address %s",ctx->label,addrs[i].addr.as_string);
       if((r=nr_socket_factory_create_socket(ctx->socket_factory,&addrs[i].addr,&sock))){
         r_log(LOG_ICE,LOG_WARNING,"ICE(%s): couldn't create socket for address %s",ctx->label,addrs[i].addr.as_string);
         continue;
       }
 
       if(r=nr_ice_socket_create(ctx,component,sock,NR_ICE_SOCKET_TYPE_DGRAM,&isock))
         ABORT(r);
-      /* Create one host candidate */
-      if(r=nr_ice_candidate_create(ctx,component,isock,sock,HOST,0,0,
-        component->component_id,&cand))
-        ABORT(r);
-
-      TAILQ_INSERT_TAIL(&component->candidates,cand,entry_comp);
-      component->candidate_ct++;
-      cand=0;
 
-      /* And a srvrflx candidate for each STUN server */
-      for(j=0;j<ctx->stun_server_ct;j++){
-        /* Skip non-UDP */
-        if(ctx->stun_servers[j].transport!=IPPROTO_UDP)
-          continue;
+      if (!(ctx->flags & NR_ICE_CTX_FLAGS_RELAY_ONLY)) {
+        /* Create one host candidate */
+        if(r=nr_ice_candidate_create(ctx,component,isock,sock,HOST,0,0,
+          component->component_id,&cand))
+          ABORT(r);
 
-        if(r=nr_ice_candidate_create(ctx,component,
-          isock,sock,SERVER_REFLEXIVE,0,
-          &ctx->stun_servers[j],component->component_id,&cand))
-          ABORT(r);
         TAILQ_INSERT_TAIL(&component->candidates,cand,entry_comp);
         component->candidate_ct++;
         cand=0;
+
+        /* And a srvrflx candidate for each STUN server */
+        for(j=0;j<ctx->stun_server_ct;j++){
+          /* Skip non-UDP */
+          if(ctx->stun_servers[j].transport!=IPPROTO_UDP)
+            continue;
+
+          if(r=nr_ice_candidate_create(ctx,component,
+            isock,sock,SERVER_REFLEXIVE,0,
+            &ctx->stun_servers[j],component->component_id,&cand))
+            ABORT(r);
+          TAILQ_INSERT_TAIL(&component->candidates,cand,entry_comp);
+          component->candidate_ct++;
+          cand=0;
+        }
       }
 
 #ifdef USE_TURN
-      /* And both a srvrflx and relayed candidate for each TURN server */
+      /* And both a srvrflx and relayed candidate for each TURN server (unless
+         we're in relay-only mode, in which case just the relayed one) */
       for(j=0;j<ctx->turn_server_ct;j++){
         nr_socket *turn_sock;
-        nr_ice_candidate *srvflx_cand;
+        nr_ice_candidate *srvflx_cand=0;
 
         /* Skip non-UDP */
         if (ctx->turn_servers[j].turn_server.transport != IPPROTO_UDP)
           continue;
 
-        /* srvrflx */
-        if(r=nr_ice_candidate_create(ctx,component,
-          isock,sock,SERVER_REFLEXIVE,0,
-          &ctx->turn_servers[j].turn_server,component->component_id,&cand))
-          ABORT(r);
-        cand->state=NR_ICE_CAND_STATE_INITIALIZING; /* Don't start */
-        cand->done_cb=nr_ice_gather_finished_cb;
-        cand->cb_arg=cand;
+        if (!(ctx->flags & NR_ICE_CTX_FLAGS_RELAY_ONLY)) {
+          /* srvrflx */
+          if(r=nr_ice_candidate_create(ctx,component,
+            isock,sock,SERVER_REFLEXIVE,0,
+            &ctx->turn_servers[j].turn_server,component->component_id,&cand))
+            ABORT(r);
+          cand->state=NR_ICE_CAND_STATE_INITIALIZING; /* Don't start */
+          cand->done_cb=nr_ice_gather_finished_cb;
+          cand->cb_arg=cand;
 
-        TAILQ_INSERT_TAIL(&component->candidates,cand,entry_comp);
-        component->candidate_ct++;
-        srvflx_cand=cand;
-
+          TAILQ_INSERT_TAIL(&component->candidates,cand,entry_comp);
+          component->candidate_ct++;
+          srvflx_cand=cand;
+          cand=0;
+        }
         /* relayed*/
         if(r=nr_socket_turn_create(sock, &turn_sock))
           ABORT(r);
         if(r=nr_ice_candidate_create(ctx,component,
           isock,turn_sock,RELAYED,0,
           &ctx->turn_servers[j].turn_server,component->component_id,&cand))
            ABORT(r);
         cand->u.relayed.srvflx_candidate=srvflx_cand;
@@ -403,16 +409,19 @@ static int nr_ice_component_initialize_t
       if(r!=R_NOT_FOUND)
         ABORT(r);
     }
 
     if ((r=NR_reg_get_char(NR_ICE_REG_ICE_TCP_DISABLE, &ice_tcp_disabled))) {
       if (r != R_NOT_FOUND)
         ABORT(r);
     }
+    if (ctx->flags & NR_ICE_CTX_FLAGS_RELAY_ONLY) {
+      ice_tcp_disabled = 1;
+    }
 
     for(i=0;i<addr_ct;i++){
       char suppress;
       nr_ice_socket *isock_psv=0;
       nr_ice_socket *isock_so=0;
 
       if(r=NR_reg_get2_char(NR_ICE_REG_SUPPRESS_INTERFACE_PRFX,addrs[i].addr.ifname,&suppress)){
         if(r!=R_NOT_FOUND)
--- a/media/mtransport/third_party/nICEr/src/ice/ice_ctx.h
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_ctx.h
@@ -152,16 +152,17 @@ struct nr_ice_ctx_ {
   void *trickle_cb_arg;
 };
 
 int nr_ice_ctx_create(char *label, UINT4 flags, nr_ice_ctx **ctxp);
 #define NR_ICE_CTX_FLAGS_OFFERER                           1
 #define NR_ICE_CTX_FLAGS_ANSWERER                          (1<<1)
 #define NR_ICE_CTX_FLAGS_AGGRESSIVE_NOMINATION             (1<<2)
 #define NR_ICE_CTX_FLAGS_LITE                              (1<<3)
+#define NR_ICE_CTX_FLAGS_RELAY_ONLY                        (1<<4)
 
 int nr_ice_ctx_destroy(nr_ice_ctx **ctxp);
 int nr_ice_gather(nr_ice_ctx *ctx, NR_async_cb done_cb, void *cb_arg);
 int nr_ice_add_candidate(nr_ice_ctx *ctx, nr_ice_candidate *cand);
 void nr_ice_gather_finished_cb(NR_SOCKET s, int h, void *cb_arg);
 int nr_ice_add_media_stream(nr_ice_ctx *ctx,char *label,int components, nr_ice_media_stream **streamp);
 int nr_ice_remove_media_stream(nr_ice_ctx *ctx,nr_ice_media_stream **streamp);
 int nr_ice_get_global_attributes(nr_ice_ctx *ctx,char ***attrsp, int *attrctp);