Bug 1548318 - pt 1 - get new local addrs when gathering ICE candidates after ICE restart. r=bwc

- Convert to a StunAddrRequestState enum so there is now a pending state,
    rather than just done/not done.  This is to make sure we don't have
    multiple stun addrs requests in flight at the same time.
  - Reset the stun addrs in PeerConnectionMedia from PeerConnectionImpl when
    PeerConnectionImpl::SetSignalingState_m detects ICE restart in an offer.
  - GatherIfReady will now request new stun addrs if none are available.

Differential Revision: https://phabricator.services.mozilla.com/D55883

--HG--
extra : moz-landing-system : lando

media/mtransport/third_party/nICEr/src/ice/ice_ctx.c

@@ -693,10 +693,6 @@ int nr_ice_set_local_addresses(nr_ice_ctx *ctx,
     nr_local_addr default_addrs[2];
     int default_addr_ct = 0;
 
-    if (ctx->local_addrs) {
-      r_log(LOG_ICE,LOG_WARNING,"ICE(%s): local addresses already set, no work to do",ctx->label);
-      ABORT(R_ALREADY);
-    }
     if (!stun_addrs || !stun_addr_ct) {
       r_log(LOG_ICE,LOG_ERR,"ICE(%s): no stun addrs provided",ctx->label);
       ABORT(R_BAD_ARGS);

media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp

@@ -2231,6 +2231,17 @@ void PeerConnectionImpl::SetSignalingState_m(RTCSignalingState aSignalingState,
     return;
   }
 
+  // We'd like to handle this in PeerConnectionMedia::UpdateNetworkState.
+  // Unfortunately, if the WiFi switch happens quickly, we never see
+  // that state change.  We need to detect the ice restart here and
+  // reset the PeerConnectionMedia's stun addresses so they are
+  // regathered when PeerConnectionMedia::GatherIfReady is called.
+  if ((aSignalingState == RTCSignalingState::Have_local_offer ||
+       aSignalingState == RTCSignalingState::Have_remote_offer) &&
+      !rollback && mJsepSession->IsIceRestarting()) {
+    mMedia->ResetStunAddrsForIceRestart();
+  }
+
   if (aSignalingState == RTCSignalingState::Have_local_offer ||
       (aSignalingState == RTCSignalingState::Stable &&
        mSignalingState == RTCSignalingState::Have_remote_offer && !rollback)) {

media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp

@@ -69,7 +69,7 @@ void PeerConnectionMedia::StunAddrsHandler::OnStunAddrsAvailable(
              (int)addrs.Length());
   if (pcm_) {
     pcm_->mStunAddrs = addrs;
-    pcm_->mLocalAddrsCompleted = true;
+    pcm_->mLocalAddrsRequestState = STUN_ADDR_REQUEST_COMPLETE;
     pcm_->FlushIceCtxOperationQueueIfReady();
     // If parent process returns 0 STUN addresses, change ICE connection
     // state to failed.
@@ -88,7 +88,7 @@ PeerConnectionMedia::PeerConnectionMedia(PeerConnectionImpl* parent)
       mSTSThread(mParent->GetSTSThread()),
       mForceProxy(false),
       mStunAddrsRequest(nullptr),
-      mLocalAddrsCompleted(false),
+      mLocalAddrsRequestState(STUN_ADDR_REQUEST_NONE),
       mTargetForDefaultLocalAddressLookupIsSet(false),
       mDestroyed(false) {
   if (XRE_IsContentProcess()) {
@@ -107,10 +107,14 @@ PeerConnectionMedia::~PeerConnectionMedia() {
 }
 
 void PeerConnectionMedia::InitLocalAddrs() {
+  if (mLocalAddrsRequestState == STUN_ADDR_REQUEST_PENDING) {
+    return;
+  }
   if (mStunAddrsRequest) {
+    mLocalAddrsRequestState = STUN_ADDR_REQUEST_PENDING;
     mStunAddrsRequest->SendGetStunAddrs();
   } else {
-    mLocalAddrsCompleted = true;
+    mLocalAddrsRequestState = STUN_ADDR_REQUEST_COMPLETE;
   }
 }
 
@@ -498,6 +502,14 @@ void PeerConnectionMedia::PerformOrEnqueueIceCtxOperation(
 void PeerConnectionMedia::GatherIfReady() {
   ASSERT_ON_THREAD(mMainThread);
 
+  // Init local addrs here so that if we re-gather after an ICE restart
+  // resulting from changing WiFi networks, we get new local addrs.
+  // Otherwise, we would reuse the addrs from the original WiFi network
+  // and the ICE restart will fail.
+  if (!mStunAddrs.Length()) {
+    InitLocalAddrs();
+  }
+
   // If we had previously queued gathering or ICE start, unqueue them
   mQueuedIceCtxOperations.clear();
   nsCOMPtr<nsIRunnable> runnable(WrapRunnable(

media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h

@@ -62,6 +62,8 @@ class PeerConnectionMedia : public sigslot::has_slots<> {
   nsresult UpdateTransports(const JsepSession& aSession,
                             const bool forceIceTcp);
 
+  void ResetStunAddrsForIceRestart() { mStunAddrs.Clear(); }
+
   // Start ICE checks.
   void StartIceChecks(const JsepSession& session);
 
@@ -191,7 +193,9 @@ class PeerConnectionMedia : public sigslot::has_slots<> {
   void OnCandidateFound_m(const std::string& aTransportId,
                           const CandidateInfo& aCandidateInfo);
 
-  bool IsIceCtxReady() const { return mLocalAddrsCompleted; }
+  bool IsIceCtxReady() const {
+    return mLocalAddrsRequestState == STUN_ADDR_REQUEST_COMPLETE;
+  }
 
   // The parent PC
   PeerConnectionImpl* mParent;
@@ -219,8 +223,13 @@ class PeerConnectionMedia : public sigslot::has_slots<> {
   // Used to cancel incoming stun addrs response
   RefPtr<net::StunAddrsRequestChild> mStunAddrsRequest;
 
+  enum StunAddrRequestState {
+    STUN_ADDR_REQUEST_NONE,
+    STUN_ADDR_REQUEST_PENDING,
+    STUN_ADDR_REQUEST_COMPLETE
+  };
   // Used to track the state of the stun addr IPC request
-  bool mLocalAddrsCompleted;
+  StunAddrRequestState mLocalAddrsRequestState;
 
   // Used to store the result of the stun addr IPC request
   nsTArray<NrIceStunAddr> mStunAddrs;