Bug 942188 - Various fixes to role-conflict resolution and some test-cases. r=abr
authorByron Campen [:bwc] <docfaraday@gmail.com>
Fri, 13 Dec 2013 20:18:24 -0800
changeset 218976 9488dace08651b9285826e3ad964de37ce9baa4c
parent 218975 8ba5b2460e826e7a2b9ce673d5cd3b46040d90d5
child 218977 3e0ffdf2da545bdfa18eda356560f74d683f412b
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersabr
bugs942188
milestone34.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 942188 - Various fixes to role-conflict resolution and some test-cases. r=abr
media/mtransport/test/ice_unittest.cpp
media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.h
media/mtransport/third_party/nICEr/src/ice/ice_component.c
media/mtransport/third_party/nICEr/src/ice/ice_media_stream.c
media/mtransport/third_party/nICEr/src/ice/ice_media_stream.h
media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.c
media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.h
--- a/media/mtransport/test/ice_unittest.cpp
+++ b/media/mtransport/test/ice_unittest.cpp
@@ -36,16 +36,20 @@
 #include "runnable_utils.h"
 #include "stunserver.h"
 // TODO(bcampen@mozilla.com): Big fat hack since the build system doesn't give
 // us a clean way to add object files to a single executable.
 #include "stunserver.cpp"
 #include "stun_udp_socket_filter.h"
 #include "mozilla/net/DNS.h"
 
+#include "ice_ctx.h"
+#include "ice_peer_ctx.h"
+#include "ice_media_stream.h"
+
 #define GTEST_HAS_RTTI 0
 #include "gtest/gtest.h"
 #include "gtest_utils.h"
 
 using namespace mozilla;
 MtransportTestUtils *test_utils;
 
 bool stream_added = false;
@@ -227,17 +231,18 @@ class IceTestPeer : public sigslot::has_
       fake_resolver_(),
       dns_resolver_(new NrIceResolver()),
       remote_(nullptr),
       candidate_filter_(nullptr),
       expected_local_type_(NrIceCandidate::ICE_HOST),
       expected_local_transport_(kNrIceTransportUdp),
       expected_remote_type_(NrIceCandidate::ICE_HOST),
       trickle_mode_(TRICKLE_NONE),
-      trickled_(0) {
+      trickled_(0),
+      simulate_ice_lite_(false) {
     ice_ctx_->SignalGatheringStateChange.connect(
         this,
         &IceTestPeer::GatheringStateChange);
     ice_ctx_->SignalConnectionStateChange.connect(
         this,
         &IceTestPeer::ConnectionStateChange);
   }
 
@@ -324,17 +329,21 @@ class IceTestPeer : public sigslot::has_
         WrapRunnableRet(ice_ctx_, &NrIceCtx::StartGathering, &res),
         NS_DISPATCH_SYNC);
 
     ASSERT_TRUE(NS_SUCCEEDED(res));
   }
 
   // Get various pieces of state
   std::vector<std::string> GetGlobalAttributes() {
-    return ice_ctx_->GetGlobalAttributes();
+    std::vector<std::string> attrs(ice_ctx_->GetGlobalAttributes());
+    if (simulate_ice_lite_) {
+      attrs.push_back("ice-lite");
+    }
+    return attrs;
   }
 
    std::vector<std::string> GetCandidates(size_t stream) {
     std::vector<std::string> v;
 
     RUN_ON_THREAD(
         test_utils->sts_target(),
         WrapRunnableRet(this, &IceTestPeer::GetCandidates_s, stream, &v));
@@ -787,16 +796,44 @@ class IceTestPeer : public sigslot::has_
   void DisableComponent(size_t stream, int component_id) {
     ASSERT_LT(stream, streams_.size());
     nsresult res = streams_[stream]->DisableComponent(component_id);
     ASSERT_TRUE(NS_SUCCEEDED(res));
   }
 
   int trickled() { return trickled_; }
 
+  void SetControlling(NrIceCtx::Controlling controlling) {
+    nsresult res;
+    test_utils->sts_target()->Dispatch(
+        WrapRunnableRet(ice_ctx_,
+                        &NrIceCtx::SetControlling,
+                        controlling,
+                        &res),
+        NS_DISPATCH_SYNC);
+    ASSERT_TRUE(NS_SUCCEEDED(res));
+  }
+
+  void SetTiebreaker(uint64_t tiebreaker) {
+    test_utils->sts_target()->Dispatch(
+        WrapRunnable(this,
+                     &IceTestPeer::SetTiebreaker_s,
+                     tiebreaker),
+        NS_DISPATCH_SYNC);
+  }
+
+  void SetTiebreaker_s(uint64_t tiebreaker) {
+    ice_ctx_->peer()->tiebreaker = tiebreaker;
+  }
+
+  void SimulateIceLite() {
+    simulate_ice_lite_ = true;
+    SetControlling(NrIceCtx::ICE_CONTROLLED);
+  }
+
  private:
   std::string name_;
   nsRefPtr<NrIceCtx> ice_ctx_;
   std::vector<mozilla::RefPtr<NrIceMediaStream> > streams_;
   std::map<std::string, std::vector<std::string> > candidates_;
   // Maps from stream id to list of remote trickle candidates
   std::map<size_t, std::vector<SchedulableTrickleCandidate*> >
     controlled_trickle_candidates_;
@@ -809,16 +846,17 @@ class IceTestPeer : public sigslot::has_
   nsRefPtr<NrIceResolver> dns_resolver_;
   IceTestPeer *remote_;
   CandidateFilter candidate_filter_;
   NrIceCandidate::Type expected_local_type_;
   std::string expected_local_transport_;
   NrIceCandidate::Type expected_remote_type_;
   TrickleMode trickle_mode_;
   int trickled_;
+  bool simulate_ice_lite_;
 };
 
 void SchedulableTrickleCandidate::Trickle() {
   timer_handle_ = nullptr;
   nsresult res = peer_->TrickleCandidate_s(candidate_, stream_);
   ASSERT_TRUE(NS_SUCCEEDED(res));
 }
 
@@ -927,18 +965,22 @@ class IceConnectTest : public ::testing:
   void SetCandidateFilter(CandidateFilter filter, bool both=true) {
     p1_->SetCandidateFilter(filter);
     if (both) {
       p2_->SetCandidateFilter(filter);
     }
   }
 
   void Connect() {
+    // IceTestPeer::Connect grabs attributes from the first arg, and gives them
+    // to |this|, meaning that p2_->Connect(p1_, ...) simulates p1 sending an
+    // offer to p2. Order matters here because it determines which peer is
+    // controlling.
+    p2_->Connect(p1_, TRICKLE_NONE);
     p1_->Connect(p2_, TRICKLE_NONE);
-    p2_->Connect(p1_, TRICKLE_NONE);
 
     ASSERT_TRUE_WAIT(p1_->ready_ct() == 1 && p2_->ready_ct() == 1,
                      kDefaultTimeout);
     ASSERT_TRUE_WAIT(p1_->ice_complete() && p2_->ice_complete(),
                      kDefaultTimeout);
 
     p1_->DumpAndCheckActiveCandidates();
     p2_->DumpAndCheckActiveCandidates();
@@ -972,18 +1014,18 @@ class IceConnectTest : public ::testing:
   }
 
   void WaitForGather() {
     ASSERT_TRUE_WAIT(p1_->gathering_complete(), kDefaultTimeout);
     ASSERT_TRUE_WAIT(p2_->gathering_complete(), kDefaultTimeout);
   }
 
   void ConnectTrickle(TrickleMode trickle = TRICKLE_SIMULATE) {
+    p2_->Connect(p1_, trickle);
     p1_->Connect(p2_, trickle);
-    p2_->Connect(p1_, trickle);
   }
 
   void SimulateTrickle(size_t stream) {
     p1_->SimulateTrickle(stream);
     p2_->SimulateTrickle(stream);
     ASSERT_TRUE_WAIT(p1_->is_ready(stream), kDefaultTimeout);
     ASSERT_TRUE_WAIT(p2_->is_ready(stream), kDefaultTimeout);
   }
@@ -999,18 +1041,18 @@ class IceConnectTest : public ::testing:
   void VerifyConnected() {
   }
 
   void CloseP1() {
     p1_->Close();
   }
 
   void ConnectThenDelete() {
+    p2_->Connect(p1_, TRICKLE_NONE, false);
     p1_->Connect(p2_, TRICKLE_NONE, true);
-    p2_->Connect(p1_, TRICKLE_NONE, false);
     test_utils->sts_target()->Dispatch(WrapRunnable(this,
                                                     &IceConnectTest::CloseP1),
                                        NS_DISPATCH_SYNC);
     p2_->StartChecks();
 
     // Wait to see if we crash
     PR_Sleep(PR_MillisecondsToInterval(kDefaultTimeout));
   }
@@ -1256,16 +1298,79 @@ TEST_F(IceConnectTest, TestGatherAutoPri
 
 
 TEST_F(IceConnectTest, TestConnect) {
   AddStream("first", 1);
   ASSERT_TRUE(Gather());
   Connect();
 }
 
+TEST_F(IceConnectTest, TestConnectBothControllingP1Wins) {
+  AddStream("first", 1);
+  p1_->SetTiebreaker(1);
+  p2_->SetTiebreaker(0);
+  ASSERT_TRUE(Gather());
+  p1_->SetControlling(NrIceCtx::ICE_CONTROLLING);
+  p2_->SetControlling(NrIceCtx::ICE_CONTROLLING);
+  Connect();
+}
+
+TEST_F(IceConnectTest, TestConnectBothControllingP2Wins) {
+  AddStream("first", 1);
+  p1_->SetTiebreaker(0);
+  p2_->SetTiebreaker(1);
+  ASSERT_TRUE(Gather());
+  p1_->SetControlling(NrIceCtx::ICE_CONTROLLING);
+  p2_->SetControlling(NrIceCtx::ICE_CONTROLLING);
+  Connect();
+}
+
+TEST_F(IceConnectTest, TestConnectIceLiteOfferer) {
+  AddStream("first", 1);
+  ASSERT_TRUE(Gather());
+  p1_->SimulateIceLite();
+  Connect();
+}
+
+TEST_F(IceConnectTest, TestTrickleBothControllingP1Wins) {
+  AddStream("first", 1);
+  p1_->SetTiebreaker(1);
+  p2_->SetTiebreaker(0);
+  ASSERT_TRUE(Gather());
+  p1_->SetControlling(NrIceCtx::ICE_CONTROLLING);
+  p2_->SetControlling(NrIceCtx::ICE_CONTROLLING);
+  ConnectTrickle();
+  SimulateTrickle(0);
+  ASSERT_TRUE_WAIT(p1_->ice_complete(), 1000);
+  ASSERT_TRUE_WAIT(p2_->ice_complete(), 1000);
+}
+
+TEST_F(IceConnectTest, TestTrickleBothControllingP2Wins) {
+  AddStream("first", 1);
+  p1_->SetTiebreaker(0);
+  p2_->SetTiebreaker(1);
+  ASSERT_TRUE(Gather());
+  p1_->SetControlling(NrIceCtx::ICE_CONTROLLING);
+  p2_->SetControlling(NrIceCtx::ICE_CONTROLLING);
+  ConnectTrickle();
+  SimulateTrickle(0);
+  ASSERT_TRUE_WAIT(p1_->ice_complete(), 1000);
+  ASSERT_TRUE_WAIT(p2_->ice_complete(), 1000);
+}
+
+TEST_F(IceConnectTest, TestTrickleIceLiteOfferer) {
+  AddStream("first", 1);
+  ASSERT_TRUE(Gather());
+  p1_->SimulateIceLite();
+  ConnectTrickle();
+  SimulateTrickle(0);
+  ASSERT_TRUE_WAIT(p1_->ice_complete(), 1000);
+  ASSERT_TRUE_WAIT(p2_->ice_complete(), 1000);
+}
+
 TEST_F(IceConnectTest, TestConnectTwoComponents) {
   AddStream("first", 2);
   ASSERT_TRUE(Gather());
   Connect();
 }
 
 TEST_F(IceConnectTest, TestConnectTwoComponentsDisableSecond) {
   AddStream("first", 2);
@@ -1596,18 +1701,18 @@ TEST_F(IceConnectTest, TestPollCandPairs
   ASSERT_TRUE(p2_->CandidatePairsPriorityDescending(pairs));
   ASSERT_TRUE(ContainsSucceededPair(pairs));
 }
 
 TEST_F(IceConnectTest, TestPollCandPairsDuringConnect) {
   AddStream("first", 1);
   ASSERT_TRUE(Gather());
 
+  p2_->Connect(p1_, TRICKLE_NONE, false);
   p1_->Connect(p2_, TRICKLE_NONE, false);
-  p2_->Connect(p1_, TRICKLE_NONE, false);
 
   std::vector<NrIceCandidatePair> pairs1;
   std::vector<NrIceCandidatePair> pairs2;
 
   p1_->StartChecks();
   p1_->UpdateAndValidateCandidatePairs(0, &pairs1);
   p2_->UpdateAndValidateCandidatePairs(0, &pairs2);
 
@@ -1622,18 +1727,18 @@ TEST_F(IceConnectTest, TestPollCandPairs
   ASSERT_TRUE(ContainsSucceededPair(pairs2));
 }
 
 TEST_F(IceConnectTest, TestRLogRingBuffer) {
   RLogRingBuffer::CreateInstance();
   AddStream("first", 1);
   ASSERT_TRUE(Gather());
 
+  p2_->Connect(p1_, TRICKLE_NONE, false);
   p1_->Connect(p2_, TRICKLE_NONE, false);
-  p2_->Connect(p1_, TRICKLE_NONE, false);
 
   std::vector<NrIceCandidatePair> pairs1;
   std::vector<NrIceCandidatePair> pairs2;
 
   p1_->StartChecks();
   p1_->UpdateAndValidateCandidatePairs(0, &pairs1);
   p2_->UpdateAndValidateCandidatePairs(0, &pairs2);
 
--- a/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
@@ -40,17 +40,17 @@ static char *RCSSTRING __UNUSED__="$Id: 
 #include "async_timer.h"
 #include "ice_ctx.h"
 #include "ice_util.h"
 #include "ice_codeword.h"
 #include "stun.h"
 
 static char *nr_ice_cand_pair_states[]={"UNKNOWN","FROZEN","WAITING","IN_PROGRESS","FAILED","SUCCEEDED","CANCELLED"};
 
-static void nr_ice_candidate_pair_restart_stun_controlled_cb(NR_SOCKET s, int how, void *cb_arg);
+static void nr_ice_candidate_pair_restart_stun_role_change_cb(NR_SOCKET s, int how, void *cb_arg);
 static void nr_ice_candidate_pair_compute_codeword(nr_ice_cand_pair *pair,
   nr_ice_candidate *lcand, nr_ice_candidate *rcand);
 
 int nr_ice_candidate_pair_create(nr_ice_peer_ctx *pctx, nr_ice_candidate *lcand,nr_ice_candidate *rcand,nr_ice_cand_pair **pairp)
   {
     nr_ice_cand_pair *pair=0;
     UINT8 o_priority, a_priority;
     int r,_status;
@@ -154,17 +154,17 @@ int nr_ice_candidate_pair_destroy(nr_ice
     nr_ice_socket_deregister(pair->local->isock,pair->stun_client_handle);
     if (pair->stun_client) {
       RFREE(pair->stun_client->params.ice_binding_request.username);
       RFREE(pair->stun_client->params.ice_binding_request.password.data);
       nr_stun_client_ctx_destroy(&pair->stun_client);
     }
 
     NR_async_timer_cancel(pair->stun_cb_timer);
-    NR_async_timer_cancel(pair->restart_controlled_cb_timer);
+    NR_async_timer_cancel(pair->restart_role_change_cb_timer);
     NR_async_timer_cancel(pair->restart_nominated_cb_timer);
 
     RFREE(pair);
     return(0);
   }
 
 int nr_ice_candidate_pair_unfreeze(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair)
   {
@@ -197,23 +197,22 @@ static void nr_ice_candidate_pair_stun_c
        Just bail out */
     if(pair->state==NR_ICE_PAIR_STATE_SUCCEEDED)
       goto done;
 
     switch(pair->stun_client->state){
       case NR_STUN_CLIENT_STATE_FAILED:
         sres=pair->stun_client->response;
         if(sres && nr_stun_message_has_attribute(sres,NR_STUN_ATTR_ERROR_CODE,&attr)&&attr->u.error_code.number==487){
-          r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s)/CAND-PAIR(%s): detected role conflict. Switching to controlled",pair->pctx->label,pair->codeword);
 
-          pair->pctx->controlling=0;
-
-          /* Only restart if we haven't tried this already */
-          if(!pair->restart_controlled_cb_timer)
-            NR_ASYNC_TIMER_SET(0,nr_ice_candidate_pair_restart_stun_controlled_cb,pair,&pair->restart_controlled_cb_timer);
+          /*
+           * Flip the controlling bit; subsequent 487s for other pairs will be
+           * ignored, since we abandon their STUN transactions.
+           */
+          nr_ice_peer_ctx_switch_controlling_role(pair->pctx);
 
           return;
         }
         /* Fall through */
       case NR_STUN_CLIENT_STATE_TIMED_OUT:
         nr_ice_candidate_pair_set_state(pair->pctx,pair,NR_ICE_PAIR_STATE_FAILED);
         break;
       case NR_STUN_CLIENT_STATE_DONE:
@@ -335,33 +334,31 @@ static void nr_ice_candidate_pair_stun_c
     }
 
   done:
     _status=0;
   abort:
     return;
   }
 
-int nr_ice_candidate_pair_start(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair)
+static void nr_ice_candidate_pair_restart(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair)
   {
     int r,_status;
     UINT4 mode;
 
     nr_ice_candidate_pair_set_state(pctx,pair,NR_ICE_PAIR_STATE_IN_PROGRESS);
 
-    /* Register the stun ctx for when responses come in*/
-    if(r=nr_ice_socket_register_stun_client(pair->local->isock,pair->stun_client,&pair->stun_client_handle))
-      ABORT(r);
-
     /* Start STUN */
     if(pair->pctx->controlling && (pair->pctx->ctx->flags & NR_ICE_CTX_FLAGS_AGGRESSIVE_NOMINATION))
       mode=NR_ICE_CLIENT_MODE_USE_CANDIDATE;
     else
       mode=NR_ICE_CLIENT_MODE_BINDING_REQUEST;
 
+    nr_stun_client_reset(pair->stun_client);
+
     if(r=nr_stun_client_start(pair->stun_client,mode,nr_ice_candidate_pair_stun_cb,pair))
       ABORT(r);
 
     if ((r=nr_ice_ctx_remember_id(pair->pctx->ctx, pair->stun_client->request))) {
       /* ignore if this fails (which it shouldn't) because it's only an
        * optimization and the cleanup routines are not going to do the right
        * thing if this fails */
       assert(0);
@@ -370,16 +367,31 @@ int nr_ice_candidate_pair_start(nr_ice_p
     _status=0;
   abort:
     if(_status){
       /* Don't fire the CB, but schedule it to fire ASAP */
       assert(!pair->stun_cb_timer);
       NR_ASYNC_TIMER_SET(0,nr_ice_candidate_pair_stun_cb,pair, &pair->stun_cb_timer);
       _status=0;
     }
+  }
+
+int nr_ice_candidate_pair_start(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair)
+  {
+    int r,_status;
+    UINT4 mode;
+
+    /* Register the stun ctx for when responses come in*/
+    if(r=nr_ice_socket_register_stun_client(pair->local->isock,pair->stun_client,&pair->stun_client_handle))
+      ABORT(r);
+
+    nr_ice_candidate_pair_restart(pctx, pair);
+
+    _status=0;
+  abort:
     return(_status);
   }
 
 
 int nr_ice_candidate_pair_do_triggered_check(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair)
   {
     int r,_status;
 
@@ -543,40 +555,39 @@ void nr_ice_candidate_pair_restart_stun_
     if(r=nr_ice_ctx_remember_id(pair->pctx->ctx, pair->stun_client->request))
       ABORT(r);
 
     _status=0;
   abort:
     return;
   }
 
-static void nr_ice_candidate_pair_restart_stun_controlled_cb(NR_SOCKET s, int how, void *cb_arg)
-  {
+static void nr_ice_candidate_pair_restart_stun_role_change_cb(NR_SOCKET s, int how, void *cb_arg)
+ {
     nr_ice_cand_pair *pair=cb_arg;
-    int r,_status;
-
-    pair->restart_controlled_cb_timer=0;
-
-    r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s)/STREAM(%s)/CAND-PAIR(%s):COMP(%d): Restarting pair as CONTROLLED: %s",pair->pctx->label,pair->local->stream->label,pair->codeword,pair->remote->component->component_id,pair->as_string);
 
-    nr_stun_client_reset(pair->stun_client);
-    pair->stun_client->params.ice_binding_request.control=NR_ICE_CONTROLLED;
-
-    if(r=nr_stun_client_start(pair->stun_client,NR_ICE_CLIENT_MODE_BINDING_REQUEST,nr_ice_candidate_pair_stun_cb,pair))
-      ABORT(r);
+    pair->restart_role_change_cb_timer=0;
 
-    if(r=nr_ice_ctx_remember_id(pair->pctx->ctx, pair->stun_client->request))
-      ABORT(r);
+    r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s)/STREAM(%s)/CAND-PAIR(%s):COMP(%d): Restarting pair as %s: %s",pair->pctx->label,pair->local->stream->label,pair->codeword,pair->remote->component->component_id,pair->pctx->controlling ? "CONTROLLING" : "CONTROLLED",pair->as_string);
 
-    _status=0;
-  abort:
-    return;
+    nr_ice_candidate_pair_restart(pair->pctx, pair);
   }
 
+void nr_ice_candidate_pair_role_change(nr_ice_cand_pair *pair)
+  {
+    pair->stun_client->params.ice_binding_request.control = pair->pctx->controlling ? NR_ICE_CONTROLLING : NR_ICE_CONTROLLED;
 
+    if(pair->state == NR_ICE_PAIR_STATE_IN_PROGRESS) {
+      /* We could try only restarting in-progress pairs when they receive their
+       * 487, but this ends up being simpler, because any extra 487 are dropped.
+       */
+      if(!pair->restart_role_change_cb_timer)
+        NR_ASYNC_TIMER_SET(0,nr_ice_candidate_pair_restart_stun_role_change_cb,pair,&pair->restart_role_change_cb_timer);
+    }
+  }
 
 static void nr_ice_candidate_pair_compute_codeword(nr_ice_cand_pair *pair,
   nr_ice_candidate *lcand, nr_ice_candidate *rcand)
   {
     char as_string[2048];
 
     snprintf(as_string,
              sizeof(as_string),
--- a/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.h
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.h
@@ -60,31 +60,32 @@ struct nr_ice_cand_pair_ {
   nr_ice_candidate *local;            /* The local candidate */
   nr_ice_candidate *remote;           /* The remote candidate */
   char *foundation;                   /* The combined foundations */
 
   nr_stun_client_ctx *stun_client;    /* STUN context when acting as a client */
   void *stun_client_handle;
 
   void *stun_cb_timer;
-  void *restart_controlled_cb_timer;
+  void *restart_role_change_cb_timer;
   void *restart_nominated_cb_timer;
 
   TAILQ_ENTRY(nr_ice_cand_pair_) entry;
 };
 
 int nr_ice_candidate_pair_create(nr_ice_peer_ctx *pctx, nr_ice_candidate *lcand,nr_ice_candidate *rcand,nr_ice_cand_pair **pairp);
 int nr_ice_candidate_pair_unfreeze(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair);
 int nr_ice_candidate_pair_start(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair);
 int nr_ice_candidate_pair_set_state(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair, int state);
 int nr_ice_candidate_pair_dump_state(nr_ice_cand_pair *pair, FILE *out);
 int nr_ice_candidate_pair_cancel(nr_ice_peer_ctx *pctx,nr_ice_cand_pair *pair);
 int nr_ice_candidate_pair_select(nr_ice_cand_pair *pair);
 int nr_ice_candidate_pair_do_triggered_check(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair);
 int nr_ice_candidate_pair_insert(nr_ice_cand_pair_head *head,nr_ice_cand_pair *pair);
 void nr_ice_candidate_pair_restart_stun_nominated_cb(NR_SOCKET s, int how, void *cb_arg);
 int nr_ice_candidate_pair_destroy(nr_ice_cand_pair **pairp);
+void nr_ice_candidate_pair_role_change(nr_ice_cand_pair *pair);
 
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
 #endif
 
--- a/media/mtransport/third_party/nICEr/src/ice/ice_component.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_component.c
@@ -524,40 +524,38 @@ static int nr_ice_component_process_inco
 
     /* Check for role conficts (7.2.1.1) */
     if(comp->stream->pctx->controlling){
       if(nr_stun_message_has_attribute(sreq,NR_STUN_ATTR_ICE_CONTROLLING,&attr)){
         /* OK, there is a conflict. Who's right? */
         r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s): role conflict, both controlling",comp->stream->pctx->label);
 
         if(attr->u.ice_controlling > comp->stream->pctx->tiebreaker){
-          /* They are: switch */
-          r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s): switching to controlled",comp->stream->pctx->label);
-
-          comp->stream->pctx->controlling=0;
+          /* Update the peer ctx. This will propagate to all candidate pairs
+             in the context. */
+          nr_ice_peer_ctx_switch_controlling_role(comp->stream->pctx);
         }
         else {
           /* We are: throw an error */
           r_log(LOG_ICE,LOG_WARNING,"ICE-PEER(%s): returning 487 role conflict",comp->stream->pctx->label);
 
           *error=487;
           ABORT(R_REJECTED);
         }
       }
     }
     else{
       if(nr_stun_message_has_attribute(sreq,NR_STUN_ATTR_ICE_CONTROLLED,&attr)){
         /* OK, there is a conflict. Who's right? */
         r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s): role conflict, both controlled",comp->stream->pctx->label);
 
         if(attr->u.ice_controlling < comp->stream->pctx->tiebreaker){
-          r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s): switching to controlling",comp->stream->pctx->label);
-
-          /* They are: switch */
-          comp->stream->pctx->controlling=1;
+          /* Update the peer ctx. This will propagate to all candidate pairs
+             in the context. */
+          nr_ice_peer_ctx_switch_controlling_role(comp->stream->pctx);
         }
         else {
           /* We are: throw an error */
           r_log(LOG_ICE,LOG_WARNING,"ICE-PEER(%s): returning 487 role conflict",comp->stream->pctx->label);
 
           *error=487;
           ABORT(R_REJECTED);
         }
--- a/media/mtransport/third_party/nICEr/src/ice/ice_media_stream.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_media_stream.c
@@ -533,30 +533,30 @@ int nr_ice_media_stream_unfreeze_pairs_f
           default:
             break;
         }
       }
 
       str=STAILQ_NEXT(str,entry);
     }
 
-//    nr_ice_media_stream_dump_state(stream->pctx,stream,stderr);
+/*    nr_ice_media_stream_dump_state(stream->pctx,stream,stderr); */
 
 
     _status=0;
   abort:
     return(_status);
   }
 
 
 int nr_ice_media_stream_dump_state(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream,FILE *out)
   {
     nr_ice_cand_pair *pair;
 
-    //r_log(LOG_ICE,LOG_DEBUG,"MEDIA-STREAM(%s): state dump", stream->label);
+    /* r_log(LOG_ICE,LOG_DEBUG,"MEDIA-STREAM(%s): state dump", stream->label); */
     pair=TAILQ_FIRST(&stream->check_list);
     while(pair){
       nr_ice_candidate_pair_dump_state(pair,out);
 
       pair=TAILQ_NEXT(pair,entry);
     }
 
     return(0);
@@ -869,8 +869,20 @@ int nr_ice_media_stream_disable_componen
 
     comp->state = NR_ICE_COMPONENT_DISABLED;
 
     _status=0;
  abort:
     return(_status);
   }
 
+void nr_ice_media_stream_role_change(nr_ice_media_stream *stream)
+  {
+    nr_ice_cand_pair *pair;
+    assert(stream->ice_state != NR_ICE_MEDIA_STREAM_UNPAIRED);
+
+    pair=TAILQ_FIRST(&stream->check_list);
+    while(pair){
+      nr_ice_candidate_pair_role_change(pair);
+      pair=TAILQ_NEXT(pair,entry);
+    }
+  }
+
--- a/media/mtransport/third_party/nICEr/src/ice/ice_media_stream.h
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_media_stream.h
@@ -60,17 +60,17 @@ struct nr_ice_media_stream_ {
 #define NR_ICE_MEDIA_STREAM_CHECKS_FROZEN      2
 #define NR_ICE_MEDIA_STREAM_CHECKS_ACTIVE      3
 #define NR_ICE_MEDIA_STREAM_CHECKS_COMPLETED   4
 #define NR_ICE_MEDIA_STREAM_CHECKS_FAILED      5
 
   nr_ice_cand_pair_head check_list;
   void *timer;  /* Check list periodic timer */
 
-//  nr_ice_cand_pair_head valid_list;
+/*  nr_ice_cand_pair_head valid_list; */
 
   STAILQ_ENTRY(nr_ice_media_stream_) entry;
 };
 
 typedef STAILQ_HEAD(nr_ice_media_stream_head_,nr_ice_media_stream_) nr_ice_media_stream_head;
 
 int nr_ice_media_stream_create(struct nr_ice_ctx_ *ctx,char *label, int components, nr_ice_media_stream **streamp);
 int nr_ice_media_stream_destroy(nr_ice_media_stream **streamp);
@@ -91,14 +91,15 @@ int nr_ice_media_stream_get_best_candida
 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_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_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);
 
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
 #endif
 
--- a/media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.c
@@ -63,25 +63,20 @@ int nr_ice_peer_ctx_create(nr_ice_ctx *c
     if(!(pctx->label=r_strdup(label)))
       ABORT(R_NO_MEMORY);
 
     pctx->ctx=ctx;
     pctx->handler=handler;
 
     /* Decide controlling vs. controlled */
     if(ctx->flags & NR_ICE_CTX_FLAGS_LITE){
-      if(pctx->peer_lite){
-        r_log(LOG_ICE,LOG_ERR,"Both sides are ICE-Lite");
-        ABORT(R_BAD_DATA);
-      }
-
       pctx->controlling=0;
     }
     else{
-      if(pctx->peer_lite || (ctx->flags & NR_ICE_CTX_FLAGS_OFFERER))
+      if(ctx->flags & NR_ICE_CTX_FLAGS_OFFERER)
         pctx->controlling=1;
       else if(ctx->flags & NR_ICE_CTX_FLAGS_ANSWERER)
         pctx->controlling=0;
     }
     if(r=nr_crypto_random_bytes((UCHAR *)&pctx->tiebreaker,8))
       ABORT(r);
 
     STAILQ_INIT(&pctx->peer_streams);
@@ -359,16 +354,23 @@ static void nr_ice_peer_ctx_start_trickl
     }
   }
 
 int nr_ice_peer_ctx_pair_candidates(nr_ice_peer_ctx *pctx)
   {
     nr_ice_media_stream *stream;
     int r,_status;
 
+    if(pctx->peer_lite && !pctx->controlling) {
+      if(pctx->ctx->flags & NR_ICE_CTX_FLAGS_LITE){
+        r_log(LOG_ICE,LOG_ERR,"Both sides are ICE-Lite");
+        ABORT(R_BAD_DATA);
+      }
+      nr_ice_peer_ctx_switch_controlling_role(pctx);
+    }
 
     r_log(LOG_ICE,LOG_DEBUG,"ICE(%s): peer (%s) pairing candidates",pctx->ctx->label,pctx->label);
 
     if(STAILQ_EMPTY(&pctx->peer_streams)) {
         r_log(LOG_ICE,LOG_ERR,"ICE(%s): peer (%s) received no media stream attributes",pctx->ctx->label,pctx->label);
         ABORT(R_FAILED);
     }
 
@@ -718,9 +720,36 @@ int nr_ice_peer_ctx_deliver_packet_maybe
         pctx,comp->stream,comp->component_id,data,len);
     }
 
     _status=0;
   abort:
     return(_status);
   }
 
+void nr_ice_peer_ctx_switch_controlling_role(nr_ice_peer_ctx *pctx)
+  {
+    int controlling = !(pctx->controlling);
+    if(pctx->controlling_conflict_resolved) {
+      r_log(LOG_ICE,LOG_WARNING,"ICE(%s): peer (%s) %s called more than once; "
+            "this probably means the peer is confused. Not switching roles.",
+            pctx->ctx->label,pctx->label,__FUNCTION__);
+      return;
+    }
 
+    r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s): detected "
+          "role conflict. Switching to %s",
+          pctx->label,
+          controlling ? "controlling" : "controlled");
+
+    pctx->controlling = controlling;
+    pctx->controlling_conflict_resolved = 1;
+
+    if(pctx->state == NR_ICE_PEER_STATE_PAIRED) {
+      /*  We have formed candidate pairs. We need to inform them. */
+      nr_ice_media_stream *pstream=STAILQ_FIRST(&pctx->peer_streams);
+      while(pstream) {
+        nr_ice_media_stream_role_change(pstream);
+        pstream = STAILQ_NEXT(pstream, entry);
+      }
+    }
+  }
+
--- a/media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.h
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.h
@@ -44,16 +44,17 @@ struct nr_ice_peer_ctx_ {
 #define NR_ICE_PEER_STATE_UNPAIRED 1
 #define NR_ICE_PEER_STATE_PAIRED   2
 
   char *label;
   nr_ice_ctx *ctx;
   nr_ice_handler *handler;
 
   UCHAR controlling; /* 1 for controlling, 0 for controlled */
+  UCHAR controlling_conflict_resolved;
   UINT8 tiebreaker;
 
   char *peer_ufrag;
   char *peer_pwd;
   int peer_lite;
   int peer_ice_mismatch;
 
   nr_ice_media_stream_head peer_streams;
@@ -81,14 +82,15 @@ int nr_ice_peer_ctx_start_checks(nr_ice_
 int nr_ice_peer_ctx_start_checks2(nr_ice_peer_ctx *pctx, int allow_non_first);
 int nr_ice_peer_ctx_dump_state(nr_ice_peer_ctx *pctx,FILE *out);
 int nr_ice_peer_ctx_log_state(nr_ice_peer_ctx *pctx);
 int nr_ice_peer_ctx_stream_done(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream);
 int nr_ice_peer_ctx_find_component(nr_ice_peer_ctx *pctx, nr_ice_media_stream *str, int component_id, nr_ice_component **compp);
 int nr_ice_peer_ctx_deliver_packet_maybe(nr_ice_peer_ctx *pctx, nr_ice_component *comp, nr_transport_addr *source_addr, UCHAR *data, int len);
 int nr_ice_peer_ctx_disable_component(nr_ice_peer_ctx *pctx, nr_ice_media_stream *lstream, int component_id);
 int nr_ice_peer_ctx_pair_new_trickle_candidate(nr_ice_ctx *ctx, nr_ice_peer_ctx *pctx, nr_ice_candidate *cand);
+void nr_ice_peer_ctx_switch_controlling_role(nr_ice_peer_ctx *pctx);
 
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
 #endif