Bug 1550177 - Part 2: Remove a stack-unwind, and simplify signal hand…

…ling considerably. r=mjf

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

dom/media/PeerConnection.jsm

@@ -1587,6 +1587,10 @@ class RTCPeerConnection {
     this._iceGatheringState = state;
     _globalPCList.notifyLifecycleObservers(this, "icegatheringstatechange");
     this.dispatchEvent(new this._win.Event("icegatheringstatechange"));
+    if (this.iceGatheringState === "complete") {
+      this.dispatchEvent(new this._win.RTCPeerConnectionIceEvent(
+        "icecandidate", { candidate: null }));
+    }
   }
 
   changeIceConnectionState(state) {
@@ -1785,8 +1789,6 @@ class PeerConnectionObserver {
         this._dompc._iceGatheredRelayCandidates = true;
       }
       candidate = new win.RTCIceCandidate({ candidate, sdpMid, sdpMLineIndex, usernameFragment });
-    } else {
-      candidate = null;
     }
     this.dispatchEvent(new win.RTCPeerConnectionIceEvent("icecandidate",
                                                          { candidate }));

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

@@ -513,15 +513,6 @@ nsresult PeerConnectionImpl::Initialize(PeerConnectionObserver& aObserver,
     return res;
   }
 
-  // Connect ICE slots.
-  mMedia->SignalIceGatheringStateChange.connect(
-      this, &PeerConnectionImpl::IceGatheringStateChange);
-  mMedia->SignalUpdateDefaultCandidate.connect(
-      this, &PeerConnectionImpl::UpdateDefaultCandidate);
-  mMedia->SignalIceConnectionStateChange.connect(
-      this, &PeerConnectionImpl::IceConnectionStateChange);
-  mMedia->SignalCandidate.connect(this, &PeerConnectionImpl::CandidateReady);
-
   PeerConnectionCtx::GetInstance()->mPeerConnections[mHandle] = this;
 
   return NS_OK;
@@ -2413,28 +2404,15 @@ void PeerConnectionImpl::CandidateReady(const std::string& candidate,
   SendLocalIceCandidateToContent(level, mid, candidate, ufrag);
 }
 
-static void SendLocalIceCandidateToContentImpl(
-    const RefPtr<PeerConnectionObserver>& aPCObserver, uint16_t level,
-    const std::string& mid, const std::string& candidate,
+void PeerConnectionImpl::SendLocalIceCandidateToContent(
+    uint16_t level, const std::string& mid, const std::string& candidate,
     const std::string& ufrag) {
   JSErrorResult rv;
-  aPCObserver->OnIceCandidate(level, ObString(mid.c_str()),
+  mPCObserver->OnIceCandidate(level, ObString(mid.c_str()),
                               ObString(candidate.c_str()),
                               ObString(ufrag.c_str()), rv);
 }
 
-void PeerConnectionImpl::SendLocalIceCandidateToContent(
-    uint16_t level, const std::string& mid, const std::string& candidate,
-    const std::string& ufrag) {
-  // We dispatch this because OnSetLocalDescriptionSuccess does a setTimeout(0)
-  // to unwind the stack, but the event handlers don't. We need to ensure that
-  // the candidates do not skip ahead of the callback.
-  NS_DispatchToMainThread(
-      WrapRunnableNM(&SendLocalIceCandidateToContentImpl, mPCObserver, level,
-                     mid, candidate, ufrag),
-      NS_DISPATCH_NORMAL);
-}
-
 static bool isDone(RTCIceConnectionState state) {
   return state != RTCIceConnectionState::Checking &&
          state != RTCIceConnectionState::New;
@@ -2516,11 +2494,26 @@ void PeerConnectionImpl::IceConnectionStateChange(
   mPCObserver->OnStateChange(PCObserverStateType::IceConnectionState, rv);
 }
 
+void PeerConnectionImpl::OnCandidateFound(const std::string& aTransportId,
+                                          const CandidateInfo& aCandidateInfo) {
+  if (!aCandidateInfo.mDefaultHostRtp.empty()) {
+    UpdateDefaultCandidate(aCandidateInfo.mDefaultHostRtp,
+                           aCandidateInfo.mDefaultPortRtp,
+                           aCandidateInfo.mDefaultHostRtcp,
+                           aCandidateInfo.mDefaultPortRtcp, aTransportId);
+  }
+  CandidateReady(aCandidateInfo.mCandidate, aTransportId,
+                 aCandidateInfo.mUfrag);
+}
+
 void PeerConnectionImpl::IceGatheringStateChange(
     dom::RTCIceGatheringState state) {
   PC_AUTO_ENTER_API_CALL_VOID_RETURN(false);
 
-  CSFLogDebug(LOGTAG, "%s", __FUNCTION__);
+  CSFLogDebug(LOGTAG, "%s %d", __FUNCTION__, static_cast<int>(state));
+  if (mIceGatheringState == state) {
+    return;
+  }
 
   mIceGatheringState = state;
 
@@ -2540,16 +2533,8 @@ void PeerConnectionImpl::IceGatheringStateChange(
       MOZ_ASSERT_UNREACHABLE("Unexpected mIceGatheringState!");
   }
 
-  WrappableJSErrorResult rv;
-  mThread->Dispatch(
-      WrapRunnable(mPCObserver, &PeerConnectionObserver::OnStateChange,
-                   PCObserverStateType::IceGatheringState, rv,
-                   static_cast<JS::Realm*>(nullptr)),
-      NS_DISPATCH_NORMAL);
-
-  if (mIceGatheringState == RTCIceGatheringState::Complete) {
-    SendLocalIceCandidateToContent(0, "", "", "");
-  }
+  JSErrorResult rv;
+  mPCObserver->OnStateChange(PCObserverStateType::IceGatheringState, rv);
 }
 
 void PeerConnectionImpl::UpdateDefaultCandidate(

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

@@ -15,7 +15,6 @@
 #include "mozilla/RefPtr.h"
 #include "nsAutoPtr.h"
 #include "IPeerConnection.h"
-#include "sigslot.h"
 #include "nsComponentManagerUtils.h"
 #include "nsPIDOMWindow.h"
 #include "nsIUUIDGenerator.h"
@@ -55,6 +54,7 @@ class AFakePCObserver;
 class nsDOMDataChannel;
 
 namespace mozilla {
+struct CandidateInfo;
 class DataChannel;
 class DtlsIdentity;
 class MediaPipeline;
@@ -164,8 +164,7 @@ typedef MozPromise<UniquePtr<RTCStatsQuery>, nsresult, true>
 class PeerConnectionImpl final
     : public nsISupports,
       public mozilla::DataChannelConnection::DataConnectionListener,
-      public dom::PrincipalChangeObserver<dom::MediaStreamTrack>,
-      public sigslot::has_slots<> {
+      public dom::PrincipalChangeObserver<dom::MediaStreamTrack> {
   struct Internal;  // Avoid exposing c includes to bindings
 
  public:
@@ -207,6 +206,8 @@ class PeerConnectionImpl final
   // ICE events
   void IceConnectionStateChange(dom::RTCIceConnectionState state);
   void IceGatheringStateChange(dom::RTCIceGatheringState state);
+  void OnCandidateFound(const std::string& aTransportId,
+                        const CandidateInfo& aCandidateInfo);
   void UpdateDefaultCandidate(const std::string& defaultAddr,
                               uint16_t defaultPort,
                               const std::string& defaultRtcpAddr,
@@ -218,7 +219,6 @@ class PeerConnectionImpl final
 
   // Get the main thread
   nsCOMPtr<nsIThread> GetMainThread() {
-    PC_AUTO_ENTER_API_CALL_NO_CHECK();
     return mThread;
   }
 
@@ -432,9 +432,7 @@ class PeerConnectionImpl final
   NS_IMETHODIMP IceGatheringState(mozilla::dom::RTCIceGatheringState* aState);
 
   mozilla::dom::RTCIceGatheringState IceGatheringState() {
-    mozilla::dom::RTCIceGatheringState state;
-    IceGatheringState(&state);
-    return state;
+    return mIceGatheringState;
   }
 
   NS_IMETHODIMP Close();

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

@@ -96,7 +96,7 @@ void PeerConnectionMedia::StunAddrsHandler::OnStunAddrsAvailable(
     // If parent process returns 0 STUN addresses, change ICE connection
     // state to failed.
     if (!pcm_->mStunAddrs.Length()) {
-      pcm_->SignalIceConnectionStateChange(dom::RTCIceConnectionState::Failed);
+      pcm_->IceConnectionStateChange_m(dom::RTCIceConnectionState::Failed);
     }
 
     pcm_ = nullptr;
@@ -505,6 +505,7 @@ void PeerConnectionMedia::SelfDestruct() {
       mSTSThread,
       WrapRunnable(this, &PeerConnectionMedia::ShutdownMediaTransport_s),
       NS_DISPATCH_NORMAL);
+  mParent = nullptr;
 
   CSFLogDebug(LOGTAG, "%s: Media shut down", __FUNCTION__);
 }
@@ -677,41 +678,36 @@ void PeerConnectionMedia::OnCandidateFound_s(
 void PeerConnectionMedia::IceGatheringStateChange_m(
     dom::RTCIceGatheringState aState) {
   ASSERT_ON_THREAD(mMainThread);
-  SignalIceGatheringStateChange(aState);
+  if (mParent) {
+    mParent->IceGatheringStateChange(aState);
+  }
 }
 
 void PeerConnectionMedia::IceConnectionStateChange_m(
     dom::RTCIceConnectionState aState) {
   ASSERT_ON_THREAD(mMainThread);
-  SignalIceConnectionStateChange(aState);
+  if (mParent) {
+    mParent->IceConnectionStateChange(aState);
+  }
 }
 
 void PeerConnectionMedia::OnCandidateFound_m(
     const std::string& aTransportId, const CandidateInfo& aCandidateInfo) {
   ASSERT_ON_THREAD(mMainThread);
-  if (!aCandidateInfo.mDefaultHostRtp.empty()) {
-    SignalUpdateDefaultCandidate(aCandidateInfo.mDefaultHostRtp,
-                                 aCandidateInfo.mDefaultPortRtp,
-                                 aCandidateInfo.mDefaultHostRtcp,
-                                 aCandidateInfo.mDefaultPortRtcp, aTransportId);
+  if (mParent) {
+    mParent->OnCandidateFound(aTransportId, aCandidateInfo);
   }
-  SignalCandidate(aCandidateInfo.mCandidate, aTransportId,
-                  aCandidateInfo.mUfrag);
 }
 
 void PeerConnectionMedia::AlpnNegotiated_s(const std::string& aAlpn) {
   GetMainThread()->Dispatch(
-      WrapRunnableNM(&PeerConnectionMedia::AlpnNegotiated_m, mParentHandle,
-                     aAlpn),
+      WrapRunnable(this, &PeerConnectionMedia::AlpnNegotiated_m, aAlpn),
       NS_DISPATCH_NORMAL);
 }
 
-void PeerConnectionMedia::AlpnNegotiated_m(const std::string& aParentHandle,
-                                           const std::string& aAlpn) {
-  PeerConnectionWrapper pcWrapper(aParentHandle);
-  PeerConnectionImpl* pc = pcWrapper.impl();
-  if (pc) {
-    pc->OnAlpnNegotiated(aAlpn);
+void PeerConnectionMedia::AlpnNegotiated_m(const std::string& aAlpn) {
+  if (mParent) {
+    mParent->OnAlpnNegotiated(aAlpn);
   }
 }
 

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

@@ -127,22 +127,7 @@ class PeerConnectionMedia : public sigslot::has_slots<> {
   nsPIDOMWindowInner* GetWindow() const;
 
   void AlpnNegotiated_s(const std::string& aAlpn);
-  static void AlpnNegotiated_m(const std::string& aParentHandle,
-                               const std::string& aAlpn);
-
-  // ICE state signals
-  sigslot::signal1<mozilla::dom::RTCIceGatheringState>
-      SignalIceGatheringStateChange;
-  sigslot::signal1<mozilla::dom::RTCIceConnectionState>
-      SignalIceConnectionStateChange;
-  // This passes a candidate:... attribute, transport id, and ufrag
-  // end-of-candidates is signaled with the empty string
-  sigslot::signal3<const std::string&, const std::string&, const std::string&>
-      SignalCandidate;
-  // This passes address, port, transport id of the default candidate.
-  sigslot::signal5<const std::string&, uint16_t, const std::string&, uint16_t,
-                   const std::string&>
-      SignalUpdateDefaultCandidate;
+  void AlpnNegotiated_m(const std::string& aAlpn);
 
   // TODO: Move to PeerConnectionImpl
   RefPtr<WebRtcCallWrapper> mCall;