diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp index c6e7134e40e..634ee2384f7 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp @@ -618,7 +618,7 @@ PeerConnectionImpl::Initialize(PeerConnectionObserver& aObserver, 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(), @@ -2040,54 +2040,10 @@ toDomIceGatheringState(NrIceCtx::GatheringState state) { 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 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 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 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. @@ -2097,8 +2053,6 @@ PeerConnectionImpl::CandidateReady_m(const std::string& candidate, FoundIceCandidate(candidate, level); } } - - return NS_OK; } void @@ -2110,7 +2064,7 @@ PeerConnectionImpl::StartTrickle() { } // 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()) { SendLocalIceCandidateToContent(0, "", ""); @@ -2199,33 +2153,35 @@ static bool isFailed(PCImplIceConnectionState state) { } #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); @@ -2233,7 +2189,7 @@ PeerConnectionImpl::IceConnectionStateChange_m(PCImplIceConnectionState aState) } #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... @@ -2269,7 +2225,7 @@ PeerConnectionImpl::IceConnectionStateChange_m(PCImplIceConnectionState aState) nsRefPtr pco = do_QueryObjectReferent(mPCObserver); if (!pco) { - return NS_OK; + return; } WrappableJSErrorResult rv; RUN_ON_THREAD(mThread, @@ -2278,17 +2234,18 @@ PeerConnectionImpl::IceConnectionStateChange_m(PCImplIceConnectionState aState) PCObserverStateType::IceConnectionState, rv, static_cast(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... @@ -2308,7 +2265,7 @@ PeerConnectionImpl::IceGatheringStateChange_m(PCImplIceGatheringState aState) nsRefPtr pco = do_QueryObjectReferent(mPCObserver); if (!pco) { - return NS_OK; + return; } WrappableJSErrorResult rv; RUN_ON_THREAD(mThread, @@ -2322,8 +2279,6 @@ PeerConnectionImpl::IceGatheringStateChange_m(PCImplIceGatheringState aState) mCandidateBuffer.empty()) { SendLocalIceCandidateToContent(0, "", ""); } - - return NS_OK; } #ifdef MOZILLA_INTERNAL_API diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h index ff5b8322735..27f5f1f9049 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h @@ -204,6 +204,12 @@ class RTCStatsQuery { 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, @@ -629,14 +635,7 @@ private: // 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 SendLocalIceCandidateToContent(uint16_t level, const std::string& mid, const std::string& candidate); diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp index 9e120b51d85..e9eaee1e829 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp @@ -240,10 +240,10 @@ nsresult PeerConnectionMedia::Init(const std::vector& stun_serv } 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 @@ -283,7 +283,7 @@ nsresult PeerConnectionMedia::Init(const std::vector& stun_serv 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, @@ -591,16 +591,73 @@ PeerConnectionMedia::AddRemoteStreamHint(int aIndex, bool aIsVideo) 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); } @@ -613,14 +670,11 @@ PeerConnectionMedia::IceStreamReady(NrIceMediaStream *aStream) } 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); } diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h index 7a2f3f34a2b..ee1bdb90e29 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h @@ -415,14 +415,21 @@ class PeerConnectionMedia : public sigslot::has_slots<> { 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