fix
git-svn-id: http://webrtc.googlecode.com/svn/trunk/talk@6720 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/app/webrtc/webrtcsession.cc b/app/webrtc/webrtcsession.cc
index 9c27be5..ddc8876 100644
--- a/app/webrtc/webrtcsession.cc
+++ b/app/webrtc/webrtcsession.cc
@@ -892,36 +892,16 @@
return false;
}
- cricket::TransportProxy* transport_proxy = NULL;
- if (remote_description()) {
- size_t mediacontent_index =
- static_cast<size_t>(candidate->sdp_mline_index());
- size_t remote_content_size =
- BaseSession::remote_description()->contents().size();
- if (mediacontent_index >= remote_content_size) {
- LOG(LS_ERROR)
- << "ProcessIceMessage: Invalid candidate media index.";
- return false;
+ bool valid = false;
+ if (!ReadyToUseRemoteCandidate(candidate, NULL, &valid)) {
+ if (valid) {
+ LOG(LS_INFO) << "ProcessIceMessage: Candidate saved";
+ saved_candidates_.push_back(
+ new JsepIceCandidate(candidate->sdp_mid(),
+ candidate->sdp_mline_index(),
+ candidate->candidate()));
}
-
- cricket::ContentInfo content =
- BaseSession::remote_description()->contents()[mediacontent_index];
- transport_proxy = GetTransportProxy(content.name);
- }
-
- // We need to check the local/remote description for the Transport instead of
- // the session, because a new Transport added during renegotiation may have
- // them unset while the session has them set from the previou negotiation. Not
- // doing so may trigger the auto generation of transport description and mess
- // up DTLS identity information, ICE credential, etc.
- if (!transport_proxy || !(transport_proxy->local_description_set() &&
- transport_proxy->remote_description_set())) {
- LOG(LS_INFO) << "ProcessIceMessage: Local/Remote description not set "
- << "on the Transport, save the candidate for later use.";
- saved_candidates_.push_back(
- new JsepIceCandidate(candidate->sdp_mid(), candidate->sdp_mline_index(),
- candidate->candidate()));
- return true;
+ return valid;
}
// Add this candidate to the remote session description.
@@ -1386,10 +1366,24 @@
if (!remote_desc)
return true;
bool ret = true;
+
for (size_t m = 0; m < remote_desc->number_of_mediasections(); ++m) {
const IceCandidateCollection* candidates = remote_desc->candidates(m);
for (size_t n = 0; n < candidates->count(); ++n) {
- ret = UseCandidate(candidates->at(n));
+ const IceCandidateInterface* candidate = candidates->at(n);
+ bool valid = false;
+ if (!ReadyToUseRemoteCandidate(candidate, remote_desc, &valid)) {
+ if (valid) {
+ LOG(LS_INFO) << "UseCandidatesInSessionDescription: Candidate saved.";
+ saved_candidates_.push_back(
+ new JsepIceCandidate(candidate->sdp_mid(),
+ candidate->sdp_mline_index(),
+ candidate->candidate()));
+ }
+ continue;
+ }
+
+ ret = UseCandidate(candidate);
if (!ret)
break;
}
@@ -1696,4 +1690,42 @@
return desc.str();
}
+// We need to check the local/remote description for the Transport instead of
+// the session, because a new Transport added during renegotiation may have
+// them unset while the session has them set from the previous negotiation.
+// Not doing so may trigger the auto generation of transport description and
+// mess up DTLS identity information, ICE credential, etc.
+bool WebRtcSession::ReadyToUseRemoteCandidate(
+ const IceCandidateInterface* candidate,
+ const SessionDescriptionInterface* remote_desc,
+ bool* valid) {
+ *valid = true;;
+ cricket::TransportProxy* transport_proxy = NULL;
+
+ const SessionDescriptionInterface* current_remote_desc =
+ remote_desc ? remote_desc : remote_description();
+
+ if (!current_remote_desc)
+ return false;
+
+ size_t mediacontent_index =
+ static_cast<size_t>(candidate->sdp_mline_index());
+ size_t remote_content_size =
+ current_remote_desc->description()->contents().size();
+ if (mediacontent_index >= remote_content_size) {
+ LOG(LS_ERROR)
+ << "ReadyToUseRemoteCandidate: Invalid candidate media index.";
+
+ *valid = false;
+ return false;
+ }
+
+ cricket::ContentInfo content =
+ current_remote_desc->description()->contents()[mediacontent_index];
+ transport_proxy = GetTransportProxy(content.name);
+
+ return transport_proxy && transport_proxy->local_description_set() &&
+ transport_proxy->remote_description_set();
+}
+
} // namespace webrtc
diff --git a/app/webrtc/webrtcsession.h b/app/webrtc/webrtcsession.h
index 3ffea8f..63e0cc4 100644
--- a/app/webrtc/webrtcsession.h
+++ b/app/webrtc/webrtcsession.h
@@ -309,6 +309,14 @@
bool ValidateDtlsSetupAttribute(const cricket::SessionDescription* desc,
Action action);
+ // Returns true if we are ready to push down the remote candidate.
+ // |remote_desc| is the new remote description, or NULL if the current remote
+ // description should be used. Output |valid| is true if the candidate media
+ // index is valid.
+ bool ReadyToUseRemoteCandidate(const IceCandidateInterface* candidate,
+ const SessionDescriptionInterface* remote_desc,
+ bool* valid);
+
std::string GetSessionErrorMsg();
talk_base::scoped_ptr<cricket::VoiceChannel> voice_channel_;
diff --git a/app/webrtc/webrtcsession_unittest.cc b/app/webrtc/webrtcsession_unittest.cc
index 956f8a6..a1d48cf 100644
--- a/app/webrtc/webrtcsession_unittest.cc
+++ b/app/webrtc/webrtcsession_unittest.cc
@@ -3246,6 +3246,66 @@
video_options.suspend_below_min_bitrate.GetWithDefaultIfUnset(false));
}
+// Tests that we can renegotiate new media content with ICE candidates in the
+// new remote SDP.
+TEST_F(WebRtcSessionTest, TestRenegotiateNewMediaWithCandidatesInSdp) {
+ MAYBE_SKIP_TEST(talk_base::SSLStreamAdapter::HaveDtlsSrtp);
+ InitWithDtls();
+ SetFactoryDtlsSrtp();
+
+ mediastream_signaling_.UseOptionsAudioOnly();
+ SessionDescriptionInterface* offer = CreateOffer(NULL);
+ SetLocalDescriptionWithoutError(offer);
+
+ SessionDescriptionInterface* answer = CreateRemoteAnswer(offer);
+ SetRemoteDescriptionWithoutError(answer);
+
+ cricket::MediaSessionOptions options;
+ options.has_video = true;
+ offer = CreateRemoteOffer(options, cricket::SEC_DISABLED);
+
+ cricket::Candidate candidate1;
+ candidate1.set_address(talk_base::SocketAddress("1.1.1.1", 5000));
+ candidate1.set_component(1);
+ JsepIceCandidate ice_candidate(kMediaContentName1, kMediaContentIndex1,
+ candidate1);
+ EXPECT_TRUE(offer->AddCandidate(&ice_candidate));
+ SetRemoteDescriptionWithoutError(offer);
+
+ answer = CreateAnswer(NULL);
+ SetLocalDescriptionWithoutError(answer);
+}
+
+// Tests that we can renegotiate new media content with ICE candidates separated
+// from the remote SDP.
+TEST_F(WebRtcSessionTest, TestRenegotiateNewMediaWithCandidatesSeparated) {
+ MAYBE_SKIP_TEST(talk_base::SSLStreamAdapter::HaveDtlsSrtp);
+ InitWithDtls();
+ SetFactoryDtlsSrtp();
+
+ mediastream_signaling_.UseOptionsAudioOnly();
+ SessionDescriptionInterface* offer = CreateOffer(NULL);
+ SetLocalDescriptionWithoutError(offer);
+
+ SessionDescriptionInterface* answer = CreateRemoteAnswer(offer);
+ SetRemoteDescriptionWithoutError(answer);
+
+ cricket::MediaSessionOptions options;
+ options.has_video = true;
+ offer = CreateRemoteOffer(options, cricket::SEC_DISABLED);
+ SetRemoteDescriptionWithoutError(offer);
+
+ cricket::Candidate candidate1;
+ candidate1.set_address(talk_base::SocketAddress("1.1.1.1", 5000));
+ candidate1.set_component(1);
+ JsepIceCandidate ice_candidate(kMediaContentName1, kMediaContentIndex1,
+ candidate1);
+ EXPECT_TRUE(session_->ProcessIceMessage(&ice_candidate));
+
+ answer = CreateAnswer(NULL);
+ SetLocalDescriptionWithoutError(answer);
+}
+
// TODO(bemasc): Add a TestIceStatesBundle with BUNDLE enabled. That test
// currently fails because upon disconnection and reconnection OnIceComplete is
// called more than once without returning to IceGatheringGathering.
