diff --git a/dom/media/PeerConnection.js b/dom/media/PeerConnection.js index 1f4dde381aa..c9b4c846959 100644 --- a/dom/media/PeerConnection.js +++ b/dom/media/PeerConnection.js @@ -552,7 +552,7 @@ PeerConnection.prototype = { return Cr.NS_ERROR_NOT_IMPLEMENTED; }, - addIceCandidate: function(cand) { + addIceCandidate: function(cand, onSuccess, onError) { if (!cand) { throw new Error ("NULL candidate passed to addIceCandidate!"); } @@ -561,10 +561,13 @@ PeerConnection.prototype = { throw new Error ("Invalid candidate passed to addIceCandidate!"); } + this._onAddIceCandidateSuccess = onSuccess; + this._onAddIceCandidateError = onError; + this._queueOrRun({ func: this._pc.addIceCandidate, args: [cand.candidate, cand.sdpMid || "", cand.sdpMLineIndex], - wait: false + wait: true }); }, @@ -758,6 +761,22 @@ PeerConnectionObserver.prototype = { this._dompc._executeNext(); }, + onAddIceCandidateSuccess: function(code) { + this._dompc._pendingType = null; + if (this._dompc._onAddIceCandidateSuccess) { + this._dompc._onAddIceCandidateSuccess.onCallback(code); + } + this._dompc._executeNext(); + }, + + onAddIceCandidateError: function(code) { + this._dompc._pendingType = null; + if (this._dompc._onAddIceCandidateError) { + this._dompc._onAddIceCandidateError.onCallback(code); + } + this._dompc._executeNext(); + }, + onStateChange: function(state) { if (state != Ci.IPeerConnectionObserver.kIceState) { return; diff --git a/dom/media/bridge/IPeerConnection.idl b/dom/media/bridge/IPeerConnection.idl index 65298bbcdbf..62d69cd07e6 100644 --- a/dom/media/bridge/IPeerConnection.idl +++ b/dom/media/bridge/IPeerConnection.idl @@ -42,6 +42,8 @@ interface IPeerConnectionObserver : nsISupports void onSetRemoteDescriptionSuccess(in unsigned long code); void onSetLocalDescriptionError(in unsigned long code); void onSetRemoteDescriptionError(in unsigned long code); + void onAddIceCandidateSuccess(in unsigned long code); + void onAddIceCandidateError(in unsigned long code); /* Data channel callbacks */ void notifyDataChannel(in nsIDOMDataChannel channel); diff --git a/dom/media/nsIDOMRTCPeerConnection.idl b/dom/media/nsIDOMRTCPeerConnection.idl index f7be741c24e..b36122a6992 100644 --- a/dom/media/nsIDOMRTCPeerConnection.idl +++ b/dom/media/nsIDOMRTCPeerConnection.idl @@ -59,7 +59,9 @@ interface nsIDOMRTCPeerConnection : nsISupports [optional] in jsval constraints, [optional] in bool restart); - void addIceCandidate(in nsIDOMRTCIceCandidate candidate); + void addIceCandidate(in nsIDOMRTCIceCandidate candidate, + [optional] in RTCPeerConnectionCallback successCallback, + [optional] in RTCPeerConnectionCallback failureCallback); void addStream(in nsIDOMMediaStream stream, [optional] in jsval constraints); diff --git a/dom/media/tests/crashtests/834100.html b/dom/media/tests/crashtests/834100.html new file mode 100644 index 00000000000..08ea2ff8fee --- /dev/null +++ b/dom/media/tests/crashtests/834100.html @@ -0,0 +1,24 @@ + + + + + + + diff --git a/dom/media/tests/crashtests/crashtests.list b/dom/media/tests/crashtests/crashtests.list index 1a3a0f4129f..7bd5222728c 100644 --- a/dom/media/tests/crashtests/crashtests.list +++ b/dom/media/tests/crashtests/crashtests.list @@ -8,4 +8,5 @@ load 799419.html load 801227.html load 802982.html load 812785.html +load 834100.html load 822197.html diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp index 05c5edde5f7..a2a46c2dae3 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp @@ -174,6 +174,14 @@ public: mObserver->OnSetRemoteDescriptionError(mCode); break; + case ADDICECANDIDATE: + mObserver->OnAddIceCandidateSuccess(mCode); + break; + + case ADDICECANDIDATEERROR: + mObserver->OnAddIceCandidateError(mCode); + break; + case REMOTESTREAMADD: { nsDOMMediaStream* stream = nullptr; @@ -200,7 +208,6 @@ public: } case UPDATELOCALDESC: - case UPDATEREMOTEDESC: /* No action necessary */ break; @@ -1183,7 +1190,7 @@ PeerConnectionImpl::onCallEvent(ccapi_call_event_e aCallEvent, break; case SETREMOTEDESC: - case UPDATEREMOTEDESC: + case ADDICECANDIDATE: mRemoteSDP = aInfo->getSDP(); break; diff --git a/media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c b/media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c index bd601d3988a..06e2dcac950 100755 --- a/media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c +++ b/media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c @@ -1319,6 +1319,7 @@ static void ccappUpdateSessionData (session_update_t *sessUpd) sessUpd->eventID == SET_REMOTE_DESC || sessUpd->eventID == UPDATE_LOCAL_DESC || sessUpd->eventID == UPDATE_REMOTE_DESC || + sessUpd->eventID == ICE_CANDIDATE_ADD || sessUpd->eventID == REMOTE_STREAM_ADD ) { CCAPP_DEBUG(DEB_F_PREFIX"CALL_SESSION_CREATED for session id 0x%x event is 0x%x \n", @@ -1361,6 +1362,7 @@ static void ccappUpdateSessionData (session_update_t *sessUpd) sessUpd->eventID == SET_REMOTE_DESC || sessUpd->eventID == UPDATE_LOCAL_DESC || sessUpd->eventID == UPDATE_REMOTE_DESC || + sessUpd->eventID == ICE_CANDIDATE_ADD || sessUpd->eventID == REMOTE_STREAM_ADD ) { data->attr = sessUpd->update.ccSessionUpd.data.state_data.attr; data->inst = sessUpd->update.ccSessionUpd.data.state_data.inst; @@ -1391,6 +1393,7 @@ static void ccappUpdateSessionData (session_update_t *sessUpd) case SET_REMOTE_DESC: case UPDATE_LOCAL_DESC: case UPDATE_REMOTE_DESC: + case ICE_CANDIDATE_ADD: data->sdp = sessUpd->update.ccSessionUpd.data.state_data.sdp; /* Fall through to the next case... */ case REMOTE_STREAM_ADD: @@ -1728,6 +1731,7 @@ static void ccappUpdateSessionData (session_update_t *sessUpd) case SET_REMOTE_DESC: case UPDATE_LOCAL_DESC: case UPDATE_REMOTE_DESC: + case ICE_CANDIDATE_ADD: data->sdp = sessUpd->update.ccSessionUpd.data.state_data.sdp; /* Fall through to the next case... */ case REMOTE_STREAM_ADD: diff --git a/media/webrtc/signaling/src/sipcc/core/common/ui.c b/media/webrtc/signaling/src/sipcc/core/common/ui.c index 1970c5aafd7..ac091718829 100755 --- a/media/webrtc/signaling/src/sipcc/core/common/ui.c +++ b/media/webrtc/signaling/src/sipcc/core/common/ui.c @@ -1670,25 +1670,20 @@ void ui_update_local_description(call_events event, line_t nLine, callid_t nCall } /** - * Let PeerConnection know about an updated remote session description + * Send data from addIceCandidate to the UI * - * @return none + * @return none */ -void ui_update_remote_description(call_events event, line_t nLine, callid_t nCallID, - uint16_t call_instance_id, string_t sdp) +void ui_ice_candidate_add(call_events event, line_t nLine, callid_t nCallID, + uint16_t call_instance_id, string_t sdp) { TNP_DEBUG(DEB_L_C_F_PREFIX"state=%d call_instance=%d\n", - DEB_L_C_F_PREFIX_ARGS(UI_API, nLine, nCallID, __FUNCTION__), - event, call_instance_id); + DEB_L_C_F_PREFIX_ARGS(UI_API, nLine, nCallID, __FUNCTION__), event, call_instance_id); - post_message_helper(UPDATE_REMOTE_DESC, event, nLine, nCallID, - call_instance_id, sdp, PC_OK); - - return; + post_message_helper(ICE_CANDIDATE_ADD, event, nLine, nCallID, call_instance_id, sdp, PC_OK); } - /** * Send Remote Stream data to the UI * diff --git a/media/webrtc/signaling/src/sipcc/core/gsm/fsmdef.c b/media/webrtc/signaling/src/sipcc/core/gsm/fsmdef.c index f62bd94a599..b2e7f293e25 100755 --- a/media/webrtc/signaling/src/sipcc/core/gsm/fsmdef.c +++ b/media/webrtc/signaling/src/sipcc/core/gsm/fsmdef.c @@ -3617,15 +3617,29 @@ fsmdef_ev_addcandidate(sm_event_t *event) { config_get_value(CFGID_SDPMODE, &sdpmode, sizeof(sdpmode)); if (sdpmode == FALSE) { - + ui_ice_candidate_add(evAddIceCandidateError, line, call_id, + dcb->caller_id.call_instance_id, strlib_empty()); return (SM_RC_END); } if (dcb == NULL) { FSM_DEBUG_SM(DEB_F_PREFIX"dcb is NULL.\n", DEB_F_PREFIX_ARGS(FSM, __FUNCTION__)); + ui_ice_candidate_add(evAddIceCandidateError, line, call_id, + dcb->caller_id.call_instance_id, strlib_empty()); return SM_RC_CLEANUP; } + if (!dcb->sdp) { + FSM_DEBUG_SM(DEB_F_PREFIX"dcb->sdp is NULL. Has the " + "remote description been set yet?\n", + DEB_F_PREFIX_ARGS(FSM, __FUNCTION__)); + + ui_ice_candidate_add(evAddIceCandidateError, line, call_id, + dcb->caller_id.call_instance_id, strlib_empty()); + + return SM_RC_END; + } + /* Perform level lookup based on mid value */ /* comment until mid is properly updated cause = gsmsdp_find_level_from_mid(dcb, (const char *)msg->data.candidate.mid, &level); @@ -3665,10 +3679,12 @@ fsmdef_ev_addcandidate(sm_event_t *event) { remote_sdp = sipsdp_write_to_buf(dcb->sdp->dest_sdp, &remote_sdp_len); if (!remote_sdp) { + ui_ice_candidate_add(evAddIceCandidateError, line, call_id, + dcb->caller_id.call_instance_id, strlib_empty()); return (SM_RC_END); } - ui_update_remote_description(evUpdateRemoteDesc, line, call_id, + ui_ice_candidate_add(evAddIceCandidate, line, call_id, dcb->caller_id.call_instance_id, strlib_malloc(remote_sdp,-1)); free(remote_sdp); diff --git a/media/webrtc/signaling/src/sipcc/core/includes/sessionConstants.h b/media/webrtc/signaling/src/sipcc/core/includes/sessionConstants.h index 20fbc1735a0..1ab608b9c4f 100755 --- a/media/webrtc/signaling/src/sipcc/core/includes/sessionConstants.h +++ b/media/webrtc/signaling/src/sipcc/core/includes/sessionConstants.h @@ -222,7 +222,8 @@ typedef enum { SET_REMOTE_DESC, UPDATE_LOCAL_DESC, UPDATE_REMOTE_DESC, - REMOTE_STREAM_ADD + REMOTE_STREAM_ADD, + ICE_CANDIDATE_ADD } group_call_event_t; /* File Player Session Events */ diff --git a/media/webrtc/signaling/src/sipcc/core/includes/uiapi.h b/media/webrtc/signaling/src/sipcc/core/includes/uiapi.h index 238f01627e9..59d52ec864f 100644 --- a/media/webrtc/signaling/src/sipcc/core/includes/uiapi.h +++ b/media/webrtc/signaling/src/sipcc/core/includes/uiapi.h @@ -38,10 +38,11 @@ typedef enum { evSetLocalDesc = SETLOCALDESC, evSetRemoteDesc = SETREMOTEDESC, evUpdateLocalDesc = UPDATELOCALDESC, - evUpdateRemoteDesc = UPDATEREMOTEDESC, evSetLocalDescError = SETLOCALDESCERROR, evSetRemoteDescError = SETREMOTEDESCERROR, evOnRemoteStreamAdd = REMOTESTREAMADD, + evAddIceCandidate = ADDICECANDIDATE, + evAddIceCandidateError = ADDICECANDIDATEERROR, evMaxEvent } call_events; @@ -162,8 +163,9 @@ void ui_set_remote_description(call_events event, line_t nLine, callid_t nCallID void ui_update_local_description(call_events event, line_t nLine, callid_t nCallID, uint16_t call_instance_id, string_t sdp); -void ui_update_remote_description(call_events event, line_t nLine, callid_t nCallID, - uint16_t call_instance_id, string_t sdp); + +void ui_ice_candidate_add(call_events event, line_t nLine, callid_t nCallID, + uint16_t call_instance_id, string_t sdp); void ui_on_remote_stream_added(call_events event, line_t nLine, callid_t nCallID, uint16_t call_instance_id, cc_media_remote_track_table_t media_tracks); diff --git a/media/webrtc/signaling/src/sipcc/include/cc_constants.h b/media/webrtc/signaling/src/sipcc/include/cc_constants.h index cc623d20155..c1e11d1d960 100644 --- a/media/webrtc/signaling/src/sipcc/include/cc_constants.h +++ b/media/webrtc/signaling/src/sipcc/include/cc_constants.h @@ -287,6 +287,8 @@ typedef enum { SETLOCALDESCERROR, SETREMOTEDESCERROR, REMOTESTREAMADD, + ADDICECANDIDATE, + ADDICECANDIDATEERROR, MAX_CALL_STATES } cc_call_state_t; diff --git a/media/webrtc/signaling/src/softphonewrapper/CC_SIPCCCallInfo.cpp b/media/webrtc/signaling/src/softphonewrapper/CC_SIPCCCallInfo.cpp index 23ba7a08f3a..451b0cff925 100644 --- a/media/webrtc/signaling/src/softphonewrapper/CC_SIPCCCallInfo.cpp +++ b/media/webrtc/signaling/src/softphonewrapper/CC_SIPCCCallInfo.cpp @@ -138,9 +138,6 @@ std::string CC_SIPCCCallInfo::callStateToString (cc_call_state_t state) case UPDATELOCALDESC: statestr = "UPDATELOCALDESC"; break; - case UPDATEREMOTEDESC: - statestr = "UPDATEREMOTEDESC"; - break; case SETLOCALDESCERROR: statestr = "SETLOCALDESCERROR"; break; @@ -150,6 +147,12 @@ std::string CC_SIPCCCallInfo::callStateToString (cc_call_state_t state) case REMOTESTREAMADD: statestr = "REMOTESTREAMADD"; break; + case ADDICECANDIDATE: + statestr = "ADDICECANDIDATE"; + break; + case ADDICECANDIDATEERROR: + statestr = "ADDICECANDIDATEERROR"; + break; default: break; } diff --git a/media/webrtc/signaling/test/signaling_unittests.cpp b/media/webrtc/signaling/test/signaling_unittests.cpp index 945dbd78343..fa60f20ba68 100644 --- a/media/webrtc/signaling/test/signaling_unittests.cpp +++ b/media/webrtc/signaling/test/signaling_unittests.cpp @@ -144,7 +144,7 @@ public: }; TestObserver(sipcc::PeerConnectionImpl *peerConnection) : - state(stateNoResponse), + state(stateNoResponse), addIceSuccessCount(0), onAddStreamCalled(false), pc(peerConnection) { } @@ -160,6 +160,7 @@ public: char *lastString; uint32_t lastStatusCode; uint32_t lastStateType; + int addIceSuccessCount; bool onAddStreamCalled; private: @@ -354,6 +355,24 @@ TestObserver::FoundIceCandidate(const char* strCandidate) return NS_OK; } +NS_IMETHODIMP +TestObserver::OnAddIceCandidateSuccess(uint32_t code) +{ + lastStatusCode = code; + state = stateSuccess; + addIceSuccessCount++; + return NS_OK; +} + +NS_IMETHODIMP +TestObserver::OnAddIceCandidateError(uint32_t code) +{ + lastStatusCode = code; + state = stateError; + cout << "onAddIceCandidateError = " << code << endl; + return NS_OK; +} + class ParsedSDP { public: //Line number with the corresponding SDP line. @@ -699,18 +718,24 @@ void CreateAnswer(sipcc::MediaConstraints& constraints, std::string offer, } void DoTrickleIce(ParsedSDP &sdp) { + int expectAddIce = 0; + pObserver->addIceSuccessCount = 0; for (std::multimap::iterator it = sdp.ice_candidates_.begin(); it != sdp.ice_candidates_.end(); ++it) { if ((*it).first != 0) { std::cerr << "Adding trickle ICE candidate " << (*it).second << std::endl; - ASSERT_TRUE(NS_SUCCEEDED(pc->AddIceCandidate((*it).second.c_str(), "", (*it).first))); + expectAddIce++; } } + ASSERT_TRUE_WAIT(pObserver->addIceSuccessCount == expectAddIce, + kDefaultTimeout); } void DoTrickleIceChrome(ParsedSDP &sdp) { + int expectAddIce = 0; + pObserver->addIceSuccessCount = 0; for (std::multimap::iterator it = sdp.ice_candidates_.begin(); it != sdp.ice_candidates_.end(); ++it) { if ((*it).first != 0) { @@ -718,8 +743,11 @@ void CreateAnswer(sipcc::MediaConstraints& constraints, std::string offer, std::cerr << "Adding trickle ICE candidate " << candidate << std::endl; ASSERT_TRUE(NS_SUCCEEDED(pc->AddIceCandidate(candidate.c_str(), "", (*it).first))); + expectAddIce++; } } + ASSERT_TRUE_WAIT(pObserver->addIceSuccessCount == expectAddIce, + kDefaultTimeout); } @@ -729,8 +757,16 @@ void CreateAnswer(sipcc::MediaConstraints& constraints, std::string offer, return state == sipcc::PeerConnectionImpl::kIceConnected; } - void AddIceCandidate(const char* candidate, const char* mid, unsigned short level) { + void AddIceCandidate(const char* candidate, const char* mid, unsigned short level, + bool expectSuccess) { + pObserver->state = TestObserver::stateNoResponse; pc->AddIceCandidate(candidate, mid, level); + ASSERT_TRUE_WAIT(pObserver->state != TestObserver::stateNoResponse, + kDefaultTimeout); + ASSERT_TRUE(pObserver->state == + expectSuccess ? TestObserver::stateSuccess : + TestObserver::stateError + ); } int GetPacketsReceived(int stream) { @@ -1016,7 +1052,12 @@ public: const char * candidate, const char * mid, unsigned short level, uint32_t sdpCheck) { a1_.CreateOffer(constraints, OFFER_AV, sdpCheck); - a1_.AddIceCandidate(candidate, mid, level); + a1_.AddIceCandidate(candidate, mid, level, true); + } + + void AddIceCandidateEarly(const char * candidate, const char * mid, + unsigned short level) { + a1_.AddIceCandidate(candidate, mid, level, false); } protected: @@ -1342,6 +1383,13 @@ TEST_F(SignalingTest, CreateOfferAddCandidate) SHOULD_SENDRECV_AV); } +TEST_F(SignalingTest, AddIceCandidateEarly) +{ + sipcc::MediaConstraints constraints; + AddIceCandidateEarly(strSampleCandidate.c_str(), + strSampleMid.c_str(), nSamplelevel); +} + // XXX adam@nostrum.com -- This test seems questionable; we need to think // through what actually needs to be tested here. TEST_F(SignalingTest, DISABLED_OfferAnswerReNegotiateOfferAnswerDontReceiveVideoNoVideoStream)