Bug 1391857 - Fix ctx flags for e10s stun addr gathering. r=drno, a=lizzard
authorMichael Froman <mfroman@mozilla.com>
Tue, 22 Aug 2017 10:13:06 -0500
changeset 421455 b20ed3e703900cc9a64ebd17bb026a55a1e3f4ac
parent 421454 e070c1cca1be6f9c51f70ea9ac6f557c6dd76e8f
child 421456 e08568b12c294e87a2b0035cee4d9d174ccac0e8
push id7686
push userryanvm@gmail.com
push dateMon, 28 Aug 2017 18:25:21 +0000
treeherdermozilla-beta@02852b5a5b3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdrno, lizzard
bugs1391857
milestone56.0
Bug 1391857 - Fix ctx flags for e10s stun addr gathering. r=drno, a=lizzard In the case of e10s, the ctx flags for default route only (and less importantly in this case, proxy only) were not set on the ice ctx when SetStunAddrs was called in PeerConnectionMedia. MozReview-Commit-ID: CldUpJfaaH3
media/mtransport/nricectx.cpp
media/mtransport/nricectx.h
media/mtransport/third_party/nICEr/src/ice/ice_ctx.c
media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
--- a/media/mtransport/nricectx.cpp
+++ b/media/mtransport/nricectx.cpp
@@ -937,31 +937,37 @@ abort:
   nr_proxy_tunnel_config_destroy(&config);
   if (_status) {
     nr_socket_wrapper_factory_destroy(&wrapper);
     return NS_ERROR_FAILURE;
   }
   return NS_OK;
 }
 
-nsresult NrIceCtx::StartGathering(bool default_route_only, bool proxy_only) {
+void NrIceCtx::SetCtxFlags(bool default_route_only, bool proxy_only) {
   ASSERT_ON_THREAD(sts_target_);
-  SetGatheringState(ICE_CTX_GATHER_STARTED);
 
   if (default_route_only) {
     nr_ice_ctx_add_flags(ctx_, NR_ICE_CTX_FLAGS_ONLY_DEFAULT_ADDRS);
   } else {
     nr_ice_ctx_remove_flags(ctx_, NR_ICE_CTX_FLAGS_ONLY_DEFAULT_ADDRS);
   }
 
   if (proxy_only) {
     nr_ice_ctx_add_flags(ctx_, NR_ICE_CTX_FLAGS_ONLY_PROXY);
   } else {
     nr_ice_ctx_remove_flags(ctx_, NR_ICE_CTX_FLAGS_ONLY_PROXY);
   }
+}
+
+nsresult NrIceCtx::StartGathering(bool default_route_only, bool proxy_only) {
+  ASSERT_ON_THREAD(sts_target_);
+  SetGatheringState(ICE_CTX_GATHER_STARTED);
+
+  SetCtxFlags(default_route_only, proxy_only);
 
   // 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);
--- a/media/mtransport/nricectx.h
+++ b/media/mtransport/nricectx.h
@@ -319,16 +319,18 @@ class NrIceCtx {
   // Provide the resolution provider. Must be called before
   // StartGathering.
   nsresult SetResolver(nr_resolver *resolver);
 
   // Provide the proxy address. Must be called before
   // StartGathering.
   nsresult SetProxyServer(const NrIceProxyServer& proxy_server);
 
+  void SetCtxFlags(bool default_route_only, bool proxy_only);
+
   // Start ICE gathering
   nsresult StartGathering(bool default_route_only, bool proxy_only);
 
   // Start checking
   nsresult StartChecks(bool offerer);
 
   // Notify that the network has gone online/offline
   void UpdateNetworkState(bool online);
--- a/media/mtransport/third_party/nICEr/src/ice/ice_ctx.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_ctx.c
@@ -741,16 +741,20 @@ int nr_ice_set_local_addresses(nr_ice_ct
             }
           }
           force_addr_ct++;
         }
       }
       addr_ct = force_addr_ct;
     }
 
+    r_log(LOG_ICE, LOG_DEBUG,
+          "ICE(%s): use only default local addresses: %s\n",
+          ctx->label,
+          (char*)(ctx->flags & NR_ICE_CTX_FLAGS_ONLY_DEFAULT_ADDRS?"yes":"no"));
     if ((!addr_ct) || (ctx->flags & NR_ICE_CTX_FLAGS_ONLY_DEFAULT_ADDRS)) {
       /* Get just the default IPv4 and IPv6 addrs */
       if(!nr_ice_get_default_local_address(ctx, NR_IPV4, local_addrs, addr_ct,
                                            &default_addrs[default_addr_ct])) {
         ++default_addr_ct;
       }
       if(!nr_ice_get_default_local_address(ctx, NR_IPV6, local_addrs, addr_ct,
                                            &default_addrs[default_addr_ct])) {
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
@@ -946,16 +946,22 @@ PeerConnectionMedia::EnsureIceGathering_
   if (mProxyServer) {
     mIceCtxHdlr->ctx()->SetProxyServer(*mProxyServer);
   } else if (aProxyOnly) {
     IceGatheringStateChange_s(mIceCtxHdlr->ctx().get(),
                               NrIceCtx::ICE_CTX_GATHER_COMPLETE);
     return;
   }
 
+  // Belt and suspenders - in e10s mode, the call below to SetStunAddrs
+  // needs to have the proper flags set on ice ctx.  For non-e10s,
+  // setting those flags happens in StartGathering.  We could probably
+  // just set them here, and only do it here.
+  mIceCtxHdlr->ctx()->SetCtxFlags(aDefaultRouteOnly, aProxyOnly);
+
   if (mStunAddrs.Length()) {
     mIceCtxHdlr->ctx()->SetStunAddrs(mStunAddrs);
   }
 
   // Start gathering, but only if there are streams
   for (size_t i = 0; i < mIceCtxHdlr->ctx()->GetStreamCount(); ++i) {
     if (mIceCtxHdlr->ctx()->GetStream(i)) {
       mIceCtxHdlr->ctx()->StartGathering(aDefaultRouteOnly, aProxyOnly);