Bug 1072044: Fire ICE signals on main. (aurora backport) r=mt a=abillings
authorByron Campen [:bwc] <docfaraday@gmail.com>
Wed, 01 Oct 2014 20:09:34 -0700
changeset 217959 b0aee943388490bd64bf04b18eacee9bf886d581
parent 217958 33ccccf7550bd810adcecf6f2961e54da5ef4bf4
child 217960 baaa0c3ab8fd682d8828c83422cb2e0cea82f6f5
push id7002
push userbcampen@mozilla.com
push dateThu, 02 Oct 2014 03:10:55 +0000
treeherdermozilla-aurora@b0aee9433884 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmt, abillings
bugs1072044
milestone34.0a2
Bug 1072044: Fire ICE signals on main. (aurora backport) r=mt a=abillings
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ -876,17 +876,17 @@ PeerConnectionImpl::Initialize(PeerConne
   // Connect ICE slots.
   mMedia->SignalIceGatheringStateChange.connect(
       this,
       &PeerConnectionImpl::IceGatheringStateChange);
   mMedia->SignalIceConnectionStateChange.connect(
       this,
       &PeerConnectionImpl::IceConnectionStateChange);
 
-  mMedia->SignalCandidate.connect(this, &PeerConnectionImpl::CandidateReady_s);
+  mMedia->SignalCandidate.connect(this, &PeerConnectionImpl::CandidateReady);
 
   // Initialize the media object.
   res = mMedia->Init(aConfiguration->getStunServers(),
                      aConfiguration->getTurnServers());
   if (NS_FAILED(res)) {
     CSFLogError(logTag, "%s: Couldn't initialize media object", __FUNCTION__);
     return res;
   }
@@ -2085,87 +2085,41 @@ toDomIceGatheringState(NrIceCtx::Gatheri
     case NrIceCtx::ICE_CTX_GATHER_STARTED:
       return PCImplIceGatheringState::Gathering;
     case NrIceCtx::ICE_CTX_GATHER_COMPLETE:
       return PCImplIceGatheringState::Complete;
   }
   MOZ_CRASH();
 }
 
-// This is called from the STS thread and so we need to thunk
-// to the main thread.
-void PeerConnectionImpl::IceConnectionStateChange(
-    NrIceCtx* ctx,
-    NrIceCtx::ConnectionState state) {
-  (void)ctx;
-  // Do an async call here to unwind the stack. refptr keeps the PC alive.
-  nsRefPtr<PeerConnectionImpl> pc(this);
-  RUN_ON_THREAD(mThread,
-                WrapRunnable(pc,
-                             &PeerConnectionImpl::IceConnectionStateChange_m,
-                             toDomIceConnectionState(state)),
-                NS_DISPATCH_NORMAL);
-}
-
 void
-PeerConnectionImpl::IceGatheringStateChange(
-    NrIceCtx* ctx,
-    NrIceCtx::GatheringState state)
-{
-  (void)ctx;
-  // Do an async call here to unwind the stack. refptr keeps the PC alive.
-  nsRefPtr<PeerConnectionImpl> pc(this);
-  RUN_ON_THREAD(mThread,
-                WrapRunnable(pc,
-                             &PeerConnectionImpl::IceGatheringStateChange_m,
-                             toDomIceGatheringState(state)),
-                NS_DISPATCH_NORMAL);
-}
-
-void
-PeerConnectionImpl::CandidateReady_s(const std::string& candidate,
-                                     uint16_t level)
-{
-  ASSERT_ON_THREAD(mSTSThread);
-  nsRefPtr<PeerConnectionImpl> pc(this);
-  RUN_ON_THREAD(mThread,
-                WrapRunnable(pc,
-                             &PeerConnectionImpl::CandidateReady_m,
-                             candidate,
-                             level),
-                NS_DISPATCH_NORMAL);
-}
-
-nsresult
-PeerConnectionImpl::CandidateReady_m(const std::string& candidate,
-                                     uint16_t level) {
-  PC_AUTO_ENTER_API_CALL(false);
+PeerConnectionImpl::CandidateReady(const std::string& candidate,
+                                   uint16_t level) {
+  PC_AUTO_ENTER_API_CALL_VOID_RETURN(false);
 
   if (mLocalSDP.empty()) {
     // It is not appropriate to trickle yet; buffer.
     mCandidateBuffer.push_back(std::make_pair(candidate, level));
   } else {
     if (level <= mNumMlines) {
       mInternal->mCall->foundICECandidate(candidate, "", level, nullptr);
     }
   }
-
-  return NS_OK;
 }
 
 void
 PeerConnectionImpl::StartTrickle() {
   for (auto it = mCandidateBuffer.begin(); it != mCandidateBuffer.end(); ++it) {
     if (it->second <= mNumMlines) {
       mInternal->mCall->foundICECandidate(it->first, "", it->second, nullptr);
     }
   }
 
   // If the buffer was empty to begin with, we have already sent the
-  // end-of-candidates event in IceGatheringStateChange_m.
+  // end-of-candidates event in IceGatheringStateChange.
   if (mIceGatheringState == PCImplIceGatheringState::Complete &&
       !mCandidateBuffer.empty()) {
     SendEndOfCandidates();
   }
 
   mCandidateBuffer.clear();
 }
 
@@ -2201,51 +2155,53 @@ static bool isSucceeded(PCImplIceConnect
 }
 
 static bool isFailed(PCImplIceConnectionState state) {
   return state == PCImplIceConnectionState::Failed ||
          state == PCImplIceConnectionState::Disconnected;
 }
 #endif
 
-nsresult
-PeerConnectionImpl::IceConnectionStateChange_m(PCImplIceConnectionState aState)
-{
-  PC_AUTO_ENTER_API_CALL(false);
+void PeerConnectionImpl::IceConnectionStateChange(
+    NrIceCtx* ctx,
+    NrIceCtx::ConnectionState state) {
+  PC_AUTO_ENTER_API_CALL_VOID_RETURN(false);
 
   CSFLogDebug(logTag, "%s", __FUNCTION__);
 
+  auto domState = toDomIceConnectionState(state);
+
 #ifdef MOZILLA_INTERNAL_API
-  if (!isDone(mIceConnectionState) && isDone(aState)) {
+  if (!isDone(mIceConnectionState) && isDone(domState)) {
     // mIceStartTime can be null if going directly from New to Closed, in which
     // case we don't count it as a success or a failure.
     if (!mIceStartTime.IsNull()){
       TimeDuration timeDelta = TimeStamp::Now() - mIceStartTime;
-      if (isSucceeded(aState)) {
+      if (isSucceeded(domState)) {
         Telemetry::Accumulate(Telemetry::WEBRTC_ICE_SUCCESS_TIME,
                               timeDelta.ToMilliseconds());
-      } else if (isFailed(aState)) {
+      } else if (isFailed(domState)) {
         Telemetry::Accumulate(Telemetry::WEBRTC_ICE_FAILURE_TIME,
                               timeDelta.ToMilliseconds());
       }
     }
 
-    if (isSucceeded(aState)) {
+    if (isSucceeded(domState)) {
       Telemetry::Accumulate(
           Telemetry::WEBRTC_ICE_ADD_CANDIDATE_ERRORS_GIVEN_SUCCESS,
           mAddCandidateErrorCount);
-    } else if (isFailed(aState)) {
+    } else if (isFailed(domState)) {
       Telemetry::Accumulate(
           Telemetry::WEBRTC_ICE_ADD_CANDIDATE_ERRORS_GIVEN_FAILURE,
           mAddCandidateErrorCount);
     }
   }
 #endif
 
-  mIceConnectionState = aState;
+  mIceConnectionState = domState;
 
   // Would be nice if we had a means of converting one of these dom enums
   // to a string that wasn't almost as much text as this switch statement...
   switch (mIceConnectionState) {
     case PCImplIceConnectionState::New:
       STAMP_TIMECARD(mTimeCard, "Ice state: new");
       break;
     case PCImplIceConnectionState::Checking:
@@ -2271,36 +2227,37 @@ PeerConnectionImpl::IceConnectionStateCh
       STAMP_TIMECARD(mTimeCard, "Ice state: closed");
       break;
     default:
       MOZ_ASSERT_UNREACHABLE("Unexpected mIceConnectionState!");
   }
 
   nsRefPtr<PeerConnectionObserver> pco = do_QueryObjectReferent(mPCObserver);
   if (!pco) {
-    return NS_OK;
+    return;
   }
   WrappableJSErrorResult rv;
   RUN_ON_THREAD(mThread,
                 WrapRunnable(pco,
                              &PeerConnectionObserver::OnStateChange,
                              PCObserverStateType::IceConnectionState,
                              rv, static_cast<JSCompartment*>(nullptr)),
                 NS_DISPATCH_NORMAL);
-  return NS_OK;
 }
 
-nsresult
-PeerConnectionImpl::IceGatheringStateChange_m(PCImplIceGatheringState aState)
+void
+PeerConnectionImpl::IceGatheringStateChange(
+    NrIceCtx* ctx,
+    NrIceCtx::GatheringState state)
 {
-  PC_AUTO_ENTER_API_CALL(false);
+  PC_AUTO_ENTER_API_CALL_VOID_RETURN(false);
 
   CSFLogDebug(logTag, "%s", __FUNCTION__);
 
-  mIceGatheringState = aState;
+  mIceGatheringState = toDomIceGatheringState(state);
 
   // Would be nice if we had a means of converting one of these dom enums
   // to a string that wasn't almost as much text as this switch statement...
   switch (mIceGatheringState) {
     case PCImplIceGatheringState::New:
       STAMP_TIMECARD(mTimeCard, "Ice gathering state: new");
       break;
     case PCImplIceGatheringState::Gathering:
@@ -2310,32 +2267,30 @@ PeerConnectionImpl::IceGatheringStateCha
       STAMP_TIMECARD(mTimeCard, "Ice gathering state: complete");
       break;
     default:
       MOZ_ASSERT_UNREACHABLE("Unexpected mIceGatheringState!");
   }
 
   nsRefPtr<PeerConnectionObserver> pco = do_QueryObjectReferent(mPCObserver);
   if (!pco) {
-    return NS_OK;
+    return;
   }
   WrappableJSErrorResult rv;
   RUN_ON_THREAD(mThread,
                 WrapRunnable(pco,
                              &PeerConnectionObserver::OnStateChange,
                              PCObserverStateType::IceGatheringState,
                              rv, static_cast<JSCompartment*>(nullptr)),
                 NS_DISPATCH_NORMAL);
 
   if (mIceGatheringState == PCImplIceGatheringState::Complete &&
       mCandidateBuffer.empty()) {
     SendEndOfCandidates();
   }
-
-  return NS_OK;
 }
 
 #ifdef MOZILLA_INTERNAL_API
 nsresult
 PeerConnectionImpl::BuildStatsQuery_m(
     mozilla::dom::MediaStreamTrack *aSelector,
     RTCStatsQuery *query) {
 
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
@@ -198,16 +198,22 @@ class RTCStatsQuery {
 // Enter an API call and check that the state is OK,
 // the PC isn't closed, etc.
 #define PC_AUTO_ENTER_API_CALL(assert_ice_ready) \
     do { \
       /* do/while prevents res from conflicting with locals */    \
       nsresult res = CheckApiState(assert_ice_ready);             \
       if (NS_FAILED(res)) return res; \
     } while(0)
+#define PC_AUTO_ENTER_API_CALL_VOID_RETURN(assert_ice_ready) \
+    do { \
+      /* do/while prevents res from conflicting with locals */    \
+      nsresult res = CheckApiState(assert_ice_ready);             \
+      if (NS_FAILED(res)) return; \
+    } while(0)
 #define PC_AUTO_ENTER_API_CALL_NO_CHECK() CheckThread()
 
 class PeerConnectionImpl MOZ_FINAL : public nsISupports,
 #ifdef MOZILLA_INTERNAL_API
                                      public mozilla::DataChannelConnection::DataConnectionListener,
                                      public nsNSSShutDownObject,
                                      public DOMMediaStream::PrincipalChangeObserver,
 #endif
@@ -621,24 +627,17 @@ private:
   void virtualDestroyNSSReference() MOZ_FINAL;
   void destructorSafeDestroyNSSReference();
   nsresult GetTimeSinceEpoch(DOMHighResTimeStamp *result);
 #endif
 
   // Shut down media - called on main thread only
   void ShutdownMedia();
 
-  // ICE callbacks run on the right thread.
-  nsresult IceConnectionStateChange_m(
-      mozilla::dom::PCImplIceConnectionState aState);
-  nsresult IceGatheringStateChange_m(
-      mozilla::dom::PCImplIceGatheringState aState);
-
-  void CandidateReady_s(const std::string& candidate, uint16_t level);
-  nsresult CandidateReady_m(const std::string& candidate, uint16_t level);
+  void CandidateReady(const std::string& candidate, uint16_t level);
   void SendEndOfCandidates();
 
   NS_IMETHOD FingerprintSplitHelper(
       std::string& fingerprint, size_t& spaceIdx) const;
 
 
 #ifdef MOZILLA_INTERNAL_API
   static void GetStatsForPCObserver_s(
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
@@ -235,20 +235,20 @@ nsresult PeerConnectionMedia::Init(const
     return rv;
   }
   if (NS_FAILED(rv = mIceCtx->SetResolver(mDNSResolver->AllocateResolver()))) {
     CSFLogError(logTag, "%s: Failed to get dns resolver", __FUNCTION__);
     return rv;
   }
   mIceCtx->SignalGatheringStateChange.connect(
       this,
-      &PeerConnectionMedia::IceGatheringStateChange);
+      &PeerConnectionMedia::IceGatheringStateChange_s);
   mIceCtx->SignalConnectionStateChange.connect(
       this,
-      &PeerConnectionMedia::IceConnectionStateChange);
+      &PeerConnectionMedia::IceConnectionStateChange_s);
 
   // Create three streams to start with.
   // One each for audio, video and DataChannel
   // TODO: this will be re-visited
   RefPtr<NrIceMediaStream> audioStream =
     mIceCtx->CreateStream((mParent->GetName()+": stream1/audio").c_str(), 2);
   RefPtr<NrIceMediaStream> videoStream =
     mIceCtx->CreateStream((mParent->GetName()+": stream2/video").c_str(), 2);
@@ -278,17 +278,17 @@ nsresult PeerConnectionMedia::Init(const
 
   // TODO(ekr@rtfm.com): This is not connected to the PCCimpl.
   // Will need to do that later.
   for (std::size_t i=0; i<mIceStreams.size(); i++) {
     mIceStreams[i]->SetLevel(i + 1);
     mIceStreams[i]->SignalReady.connect(this, &PeerConnectionMedia::IceStreamReady);
     mIceStreams[i]->SignalCandidate.connect(
         this,
-        &PeerConnectionMedia::OnCandidateFound);
+        &PeerConnectionMedia::OnCandidateFound_s);
   }
 
   // TODO(ekr@rtfm.com): When we have a generic error reporting mechanism,
   // figure out how to report that StartGathering failed. Bug 827982.
   RUN_ON_THREAD(mIceCtx->thread(),
                 WrapRunnable(mIceCtx, &NrIceCtx::StartGathering), NS_DISPATCH_NORMAL);
 
   return NS_OK;
@@ -586,46 +586,100 @@ PeerConnectionMedia::AddRemoteStreamHint
     pInfo->mTrackTypeHints |= DOMMediaStream::HINT_CONTENTS_AUDIO;
   }
 
   return NS_OK;
 }
 
 
 void
-PeerConnectionMedia::IceGatheringStateChange(NrIceCtx* ctx,
-                                             NrIceCtx::GatheringState state)
+PeerConnectionMedia::IceGatheringStateChange_s(NrIceCtx* ctx,
+                                               NrIceCtx::GatheringState state)
+{
+  ASSERT_ON_THREAD(mSTSThread);
+  // ShutdownMediaTransport_s has not run yet because it unhooks this function
+  // from its signal, which means that SelfDestruct_m has not been dispatched
+  // yet either, so this PCMedia will still be around when this dispatch reaches
+  // main.
+  GetMainThread()->Dispatch(
+    WrapRunnable(this,
+                 &PeerConnectionMedia::IceGatheringStateChange_m,
+                 ctx,
+                 state),
+    NS_DISPATCH_NORMAL);
+}
+
+void
+PeerConnectionMedia::IceConnectionStateChange_s(NrIceCtx* ctx,
+                                                NrIceCtx::ConnectionState state)
 {
+  ASSERT_ON_THREAD(mSTSThread);
+  // ShutdownMediaTransport_s has not run yet because it unhooks this function
+  // from its signal, which means that SelfDestruct_m has not been dispatched
+  // yet either, so this PCMedia will still be around when this dispatch reaches
+  // main.
+  GetMainThread()->Dispatch(
+    WrapRunnable(this,
+                 &PeerConnectionMedia::IceConnectionStateChange_m,
+                 ctx,
+                 state),
+    NS_DISPATCH_NORMAL);
+}
+
+void
+PeerConnectionMedia::OnCandidateFound_s(NrIceMediaStream *aStream,
+                                        const std::string &candidate)
+{
+  ASSERT_ON_THREAD(mSTSThread);
+  MOZ_ASSERT(aStream);
+
+  CSFLogDebug(logTag, "%s: %s", __FUNCTION__, aStream->name().c_str());
+
+  // ShutdownMediaTransport_s has not run yet because it unhooks this function
+  // from its signal, which means that SelfDestruct_m has not been dispatched
+  // yet either, so this PCMedia will still be around when this dispatch reaches
+  // main.
+  GetMainThread()->Dispatch(
+    WrapRunnable(this,
+                 &PeerConnectionMedia::OnCandidateFound_m,
+                 candidate,
+                 aStream->GetLevel()),
+    NS_DISPATCH_NORMAL);
+}
+
+void
+PeerConnectionMedia::IceGatheringStateChange_m(NrIceCtx* ctx,
+                                               NrIceCtx::GatheringState state)
+{
+  ASSERT_ON_THREAD(mMainThread);
   SignalIceGatheringStateChange(ctx, state);
 }
 
 void
-PeerConnectionMedia::IceConnectionStateChange(NrIceCtx* ctx,
-                                              NrIceCtx::ConnectionState state)
+PeerConnectionMedia::IceConnectionStateChange_m(NrIceCtx* ctx,
+                                                NrIceCtx::ConnectionState state)
 {
+  ASSERT_ON_THREAD(mMainThread);
   SignalIceConnectionStateChange(ctx, state);
 }
 
 void
 PeerConnectionMedia::IceStreamReady(NrIceMediaStream *aStream)
 {
   MOZ_ASSERT(aStream);
 
   CSFLogDebug(logTag, "%s: %s", __FUNCTION__, aStream->name().c_str());
 }
 
 void
-PeerConnectionMedia::OnCandidateFound(NrIceMediaStream *aStream,
-                                      const std::string &candidate)
+PeerConnectionMedia::OnCandidateFound_m(const std::string &candidate,
+                                        uint16_t level)
 {
-  MOZ_ASSERT(aStream);
-
-  CSFLogDebug(logTag, "%s: %s", __FUNCTION__, aStream->name().c_str());
-
-  SignalCandidate(candidate, aStream->GetLevel());
+  ASSERT_ON_THREAD(mMainThread);
+  SignalCandidate(candidate, level);
 }
 
 
 
 void
 PeerConnectionMedia::DtlsConnected_s(TransportLayer *dtlsLayer,
                                      TransportLayer::State state)
 {
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h
@@ -410,24 +410,31 @@ class PeerConnectionMedia : public sigsl
   // Shutdown media transport. Must be called on STS thread.
   void ShutdownMediaTransport_s();
 
   // Final destruction of the media stream. Must be called on the main
   // thread.
   void SelfDestruct_m();
 
   // ICE events
-  void IceGatheringStateChange(mozilla::NrIceCtx* ctx,
+  void IceGatheringStateChange_s(mozilla::NrIceCtx* ctx,
                                mozilla::NrIceCtx::GatheringState state);
-  void IceConnectionStateChange(mozilla::NrIceCtx* ctx,
+  void IceConnectionStateChange_s(mozilla::NrIceCtx* ctx,
                                 mozilla::NrIceCtx::ConnectionState state);
   void IceStreamReady(mozilla::NrIceMediaStream *aStream);
-  void OnCandidateFound(mozilla::NrIceMediaStream *aStream,
+  void OnCandidateFound_s(mozilla::NrIceMediaStream *aStream,
                         const std::string &candidate);
 
+  void IceGatheringStateChange_m(mozilla::NrIceCtx* ctx,
+                                 mozilla::NrIceCtx::GatheringState state);
+  void IceConnectionStateChange_m(mozilla::NrIceCtx* ctx,
+                                  mozilla::NrIceCtx::ConnectionState state);
+  void OnCandidateFound_m(const std::string &candidate, uint16_t level);
+
+
   // The parent PC
   PeerConnectionImpl *mParent;
   // and a loose handle on it for event driven stuff
   std::string mParentHandle;
 
   // A list of streams returned from GetUserMedia
   // This is only accessed on the main thread (with one special exception)
   nsTArray<nsRefPtr<LocalSourceStreamInfo> > mLocalSourceStreams;