diff --git a/webrtc/api/androidtests/src/org/webrtc/PeerConnectionTest.java b/webrtc/api/androidtests/src/org/webrtc/PeerConnectionTest.java index bbaa15287e..ff98d7d96a 100644 --- a/webrtc/api/androidtests/src/org/webrtc/PeerConnectionTest.java +++ b/webrtc/api/androidtests/src/org/webrtc/PeerConnectionTest.java @@ -109,6 +109,9 @@ public class PeerConnectionTest extends ActivityTestCase { } } + @Override + public void onIceCandidatesRemoved(IceCandidate[] candidates) {} + public synchronized void setExpectedResolution(int width, int height) { expectedWidth = width; expectedHeight = height; diff --git a/webrtc/api/java/jni/peerconnection_jni.cc b/webrtc/api/java/jni/peerconnection_jni.cc index 7a95737c9a..6482134df6 100644 --- a/webrtc/api/java/jni/peerconnection_jni.cc +++ b/webrtc/api/java/jni/peerconnection_jni.cc @@ -56,6 +56,7 @@ #include "webrtc/api/rtpreceiverinterface.h" #include "webrtc/api/rtpsenderinterface.h" #include "webrtc/api/videosourceinterface.h" +#include "webrtc/api/webrtcsdp.h" #include "webrtc/base/bind.h" #include "webrtc/base/checks.h" #include "webrtc/base/event_tracer.h" @@ -195,8 +196,8 @@ class PCOJava : public PeerConnectionObserver { "", "(Ljava/lang/String;ILjava/lang/String;)V"); jstring j_mid = JavaStringFromStdString(jni(), candidate->sdp_mid()); jstring j_sdp = JavaStringFromStdString(jni(), sdp); - jobject j_candidate = jni()->NewObject( - candidate_class, ctor, j_mid, candidate->sdp_mline_index(), j_sdp); + jobject j_candidate = jni()->NewObject(candidate_class, ctor, j_mid, + candidate->sdp_mline_index(), j_sdp); CHECK_EXCEPTION(jni()) << "error during NewObject"; jmethodID m = GetMethodID(jni(), *j_observer_class_, "onIceCandidate", "(Lorg/webrtc/IceCandidate;)V"); @@ -204,6 +205,17 @@ class PCOJava : public PeerConnectionObserver { CHECK_EXCEPTION(jni()) << "error during CallVoidMethod"; } + void OnIceCandidatesRemoved( + const std::vector& candidates) { + ScopedLocalRefFrame local_ref_frame(jni()); + jobjectArray candidates_array = ToJavaCandidateArray(jni(), candidates); + jmethodID m = + GetMethodID(jni(), *j_observer_class_, "onIceCandidatesRemoved", + "([Lorg/webrtc/IceCandidate;)V"); + jni()->CallVoidMethod(*j_observer_global_, m, candidates_array); + CHECK_EXCEPTION(jni()) << "Error during CallVoidMethod"; + } + void OnSignalingChange( PeerConnectionInterface::SignalingState new_state) override { ScopedLocalRefFrame local_ref_frame(jni()); @@ -371,6 +383,36 @@ class PCOJava : public PeerConnectionObserver { DeleteGlobalRef(jni(), j_stream); } + jobject ToJavaCandidate(JNIEnv* jni, + jclass* candidate_class, + const cricket::Candidate& candidate) { + std::string sdp = webrtc::SdpSerializeCandidate(candidate); + RTC_CHECK(!sdp.empty()) << "got an empty ICE candidate"; + jmethodID ctor = GetMethodID(jni, *candidate_class, "", + "(Ljava/lang/String;ILjava/lang/String;)V"); + jstring j_mid = JavaStringFromStdString(jni, candidate.transport_name()); + jstring j_sdp = JavaStringFromStdString(jni, sdp); + // sdp_mline_index is not used, pass an invalid value -1. + jobject j_candidate = + jni->NewObject(*candidate_class, ctor, j_mid, -1, j_sdp); + CHECK_EXCEPTION(jni) << "error during Java Candidate NewObject"; + return j_candidate; + } + + jobjectArray ToJavaCandidateArray( + JNIEnv* jni, + const std::vector& candidates) { + jclass candidate_class = FindClass(jni, "org/webrtc/IceCandidate"); + jobjectArray java_candidates = + jni->NewObjectArray(candidates.size(), candidate_class, NULL); + int i = 0; + for (const cricket::Candidate& candidate : candidates) { + jobject j_candidate = ToJavaCandidate(jni, &candidate_class, candidate); + jni->SetObjectArrayElement(java_candidates, i++, j_candidate); + } + return java_candidates; + } + JNIEnv* jni() { return AttachCurrentThreadIfNeeded(); } @@ -1723,6 +1765,35 @@ JOW(jboolean, PeerConnection_nativeAddIceCandidate)( return ExtractNativePC(jni, j_pc)->AddIceCandidate(candidate.get()); } +static cricket::Candidate GetCandidateFromJava(JNIEnv* jni, + jobject j_candidate) { + jclass j_candidate_class = GetObjectClass(jni, j_candidate); + jfieldID j_sdp_mid_id = + GetFieldID(jni, j_candidate_class, "sdpMid", "Ljava/lang/String;"); + std::string sdp_mid = + JavaToStdString(jni, GetStringField(jni, j_candidate, j_sdp_mid_id)); + jfieldID j_sdp_id = + GetFieldID(jni, j_candidate_class, "sdp", "Ljava/lang/String;"); + std::string sdp = + JavaToStdString(jni, GetStringField(jni, j_candidate, j_sdp_id)); + cricket::Candidate candidate; + if (!webrtc::SdpDeserializeCandidate(sdp_mid, sdp, &candidate, NULL)) { + LOG(LS_ERROR) << "SdpDescrializeCandidate failed with sdp " << sdp; + } + return candidate; +} + +JOW(jboolean, PeerConnection_nativeRemoveIceCandidates) +(JNIEnv* jni, jobject j_pc, jobjectArray j_candidates) { + std::vector candidates; + size_t num_candidates = jni->GetArrayLength(j_candidates); + for (size_t i = 0; i < num_candidates; ++i) { + jobject j_candidate = jni->GetObjectArrayElement(j_candidates, i); + candidates.push_back(GetCandidateFromJava(jni, j_candidate)); + } + return ExtractNativePC(jni, j_pc)->RemoveIceCandidates(candidates); +} + JOW(jboolean, PeerConnection_nativeAddLocalStream)( JNIEnv* jni, jobject j_pc, jlong native_stream) { return ExtractNativePC(jni, j_pc)->AddStream( diff --git a/webrtc/api/java/src/org/webrtc/PeerConnection.java b/webrtc/api/java/src/org/webrtc/PeerConnection.java index 3c9fa0ee21..5f526196ee 100644 --- a/webrtc/api/java/src/org/webrtc/PeerConnection.java +++ b/webrtc/api/java/src/org/webrtc/PeerConnection.java @@ -58,6 +58,9 @@ public class PeerConnection { /** Triggered when a new ICE candidate has been found. */ public void onIceCandidate(IceCandidate candidate); + /** Triggered when some ICE candidates have been removed. */ + public void onIceCandidatesRemoved(IceCandidate[] candidates); + /** Triggered when media is received on a new stream from remote peer. */ public void onAddStream(MediaStream stream); @@ -193,6 +196,10 @@ public class PeerConnection { candidate.sdpMid, candidate.sdpMLineIndex, candidate.sdp); } + public boolean removeIceCandidates(final IceCandidate[] candidates) { + return nativeRemoveIceCandidates(candidates); + } + public boolean addStream(MediaStream stream) { boolean ret = nativeAddLocalStream(stream.nativeStream); if (!ret) { @@ -273,6 +280,8 @@ public class PeerConnection { private native boolean nativeAddIceCandidate( String sdpMid, int sdpMLineIndex, String iceCandidateSdp); + private native boolean nativeRemoveIceCandidates(final IceCandidate[] candidates); + private native boolean nativeAddLocalStream(long nativeStream); private native void nativeRemoveLocalStream(long nativeStream); diff --git a/webrtc/api/jsep.h b/webrtc/api/jsep.h index 0673ce128e..6ac1f2d8d1 100644 --- a/webrtc/api/jsep.h +++ b/webrtc/api/jsep.h @@ -20,8 +20,8 @@ #include "webrtc/base/refcount.h" namespace cricket { -class SessionDescription; class Candidate; +class SessionDescription; } // namespace cricket namespace webrtc { @@ -95,6 +95,11 @@ class SessionDescriptionInterface { // Returns false if the session description does not have a media section that // corresponds to the |candidate| label. virtual bool AddCandidate(const IceCandidateInterface* candidate) = 0; + // Removes the candidates from the description. + // Returns the number of candidates removed. + virtual size_t RemoveCandidates( + const std::vector& candidates) = 0; + // Returns the number of m- lines in the session description. virtual size_t number_of_mediasections() const = 0; // Returns a collection of all candidates that belong to a certain m-line diff --git a/webrtc/api/jsepicecandidate.cc b/webrtc/api/jsepicecandidate.cc index 2aabcb8349..cced1b4d6a 100644 --- a/webrtc/api/jsepicecandidate.cc +++ b/webrtc/api/jsepicecandidate.cc @@ -79,4 +79,17 @@ bool JsepCandidateCollection::HasCandidate( return ret; } +size_t JsepCandidateCollection::remove(const cricket::Candidate& candidate) { + auto iter = std::find_if(candidates_.begin(), candidates_.end(), + [candidate](JsepIceCandidate* c) { + return candidate.MatchesForRemoval(c->candidate()); + }); + if (iter != candidates_.end()) { + delete *iter; + candidates_.erase(iter); + return 1; + } + return 0; +} + } // namespace webrtc diff --git a/webrtc/api/jsepicecandidate.h b/webrtc/api/jsepicecandidate.h index 529b2a7756..7e9500bcea 100644 --- a/webrtc/api/jsepicecandidate.h +++ b/webrtc/api/jsepicecandidate.h @@ -70,6 +70,9 @@ class JsepCandidateCollection : public IceCandidateCollection { virtual const IceCandidateInterface* at(size_t index) const { return candidates_[index]; } + // Removes the candidate that has a matching address and protocol. + // Returns the number of candidates that were removed. + size_t remove(const cricket::Candidate& candidate); private: std::vector candidates_; diff --git a/webrtc/api/jsepsessiondescription.cc b/webrtc/api/jsepsessiondescription.cc index b47114b9bc..eb776c86fa 100644 --- a/webrtc/api/jsepsessiondescription.cc +++ b/webrtc/api/jsepsessiondescription.cc @@ -137,6 +137,20 @@ bool JsepSessionDescription::AddCandidate( return true; } +size_t JsepSessionDescription::RemoveCandidates( + const std::vector& candidates) { + size_t num_removed = 0; + for (auto& candidate : candidates) { + int mediasection_index = GetMediasectionIndex(candidate); + if (mediasection_index < 0) { + // Not found. + continue; + } + num_removed += candidate_collection_[mediasection_index].remove(candidate); + } + return num_removed; +} + size_t JsepSessionDescription::number_of_mediasections() const { if (!description_) return 0; @@ -184,4 +198,16 @@ bool JsepSessionDescription::GetMediasectionIndex( return true; } +int JsepSessionDescription::GetMediasectionIndex( + const cricket::Candidate& candidate) { + // Find the description with a matching transport name of the candidate. + const std::string& transport_name = candidate.transport_name(); + for (size_t i = 0; i < description_->contents().size(); ++i) { + if (transport_name == description_->contents().at(i).name) { + return static_cast(i); + } + } + return -1; +} + } // namespace webrtc diff --git a/webrtc/api/jsepsessiondescription.h b/webrtc/api/jsepsessiondescription.h index 244ee19212..9a0d873204 100644 --- a/webrtc/api/jsepsessiondescription.h +++ b/webrtc/api/jsepsessiondescription.h @@ -19,6 +19,7 @@ #include "webrtc/api/jsep.h" #include "webrtc/api/jsepicecandidate.h" #include "webrtc/base/scoped_ptr.h" +#include "webrtc/p2p/base/candidate.h" namespace cricket { class SessionDescription; @@ -57,6 +58,8 @@ class JsepSessionDescription : public SessionDescriptionInterface { // Allow changing the type. Used for testing. void set_type(const std::string& type) { type_ = type; } virtual bool AddCandidate(const IceCandidateInterface* candidate); + virtual size_t RemoveCandidates( + const std::vector& candidates); virtual size_t number_of_mediasections() const; virtual const IceCandidateCollection* candidates( size_t mediasection_index) const; @@ -80,6 +83,7 @@ class JsepSessionDescription : public SessionDescriptionInterface { bool GetMediasectionIndex(const IceCandidateInterface* candidate, size_t* index); + int GetMediasectionIndex(const cricket::Candidate& candidate); RTC_DISALLOW_COPY_AND_ASSIGN(JsepSessionDescription); }; diff --git a/webrtc/api/jsepsessiondescription_unittest.cc b/webrtc/api/jsepsessiondescription_unittest.cc index 7868beaad3..3d87513731 100644 --- a/webrtc/api/jsepsessiondescription_unittest.cc +++ b/webrtc/api/jsepsessiondescription_unittest.cc @@ -109,7 +109,7 @@ TEST_F(JsepSessionDescriptionTest, CheckSessionDescription) { EXPECT_EQ(2u, jsep_desc_->number_of_mediasections()); } -// Test that we can add a candidate to a session description. +// Test that we can add a candidate to a session description without MID. TEST_F(JsepSessionDescriptionTest, AddCandidateWithoutMid) { JsepIceCandidate jsep_candidate("", 0, candidate_); EXPECT_TRUE(jsep_desc_->AddCandidate(&jsep_candidate)); @@ -125,9 +125,12 @@ TEST_F(JsepSessionDescriptionTest, AddCandidateWithoutMid) { EXPECT_EQ(0u, jsep_desc_->candidates(1)->count()); } -TEST_F(JsepSessionDescriptionTest, AddCandidateWithMid) { +// Test that we can add and remove candidates to a session description with +// MID. Removing candidates requires MID (transport_name). +TEST_F(JsepSessionDescriptionTest, AddAndRemoveCandidatesWithMid) { // mid and m-line index don't match, in this case mid is preferred. - JsepIceCandidate jsep_candidate("video", 0, candidate_); + std::string mid = "video"; + JsepIceCandidate jsep_candidate(mid, 0, candidate_); EXPECT_TRUE(jsep_desc_->AddCandidate(&jsep_candidate)); EXPECT_EQ(0u, jsep_desc_->candidates(0)->count()); const IceCandidateCollection* ice_candidates = jsep_desc_->candidates(1); @@ -140,6 +143,12 @@ TEST_F(JsepSessionDescriptionTest, AddCandidateWithMid) { EXPECT_TRUE(ice_candidate->candidate().IsEquivalent(candidate_)); // The mline index should have been updated according to mid. EXPECT_EQ(1, ice_candidate->sdp_mline_index()); + + std::vector candidates(1, candidate_); + candidates[0].set_transport_name(mid); + EXPECT_EQ(1u, jsep_desc_->RemoveCandidates(candidates)); + EXPECT_EQ(0u, jsep_desc_->candidates(0)->count()); + EXPECT_EQ(0u, jsep_desc_->candidates(1)->count()); } TEST_F(JsepSessionDescriptionTest, AddCandidateAlreadyHasUfrag) { diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc index e4ab40e3b2..54e76d46c9 100644 --- a/webrtc/api/peerconnection.cc +++ b/webrtc/api/peerconnection.cc @@ -1205,6 +1205,12 @@ bool PeerConnection::AddIceCandidate( return session_->ProcessIceMessage(ice_candidate); } +bool PeerConnection::RemoveIceCandidates( + const std::vector& candidates) { + TRACE_EVENT0("webrtc", "PeerConnection::RemoveIceCandidates"); + return session_->RemoveRemoteIceCandidates(candidates); +} + void PeerConnection::RegisterUMAObserver(UMAObserver* observer) { TRACE_EVENT0("webrtc", "PeerConnection::RegisterUmaObserver"); uma_observer_ = observer; @@ -1384,6 +1390,12 @@ void PeerConnection::OnIceCandidate(const IceCandidateInterface* candidate) { observer_->OnIceCandidate(candidate); } +void PeerConnection::OnIceCandidatesRemoved( + const std::vector& candidates) { + RTC_DCHECK(signaling_thread()->IsCurrent()); + observer_->OnIceCandidatesRemoved(candidates); +} + void PeerConnection::OnIceConnectionReceivingChange(bool receiving) { RTC_DCHECK(signaling_thread()->IsCurrent()); observer_->OnIceConnectionReceivingChange(receiving); diff --git a/webrtc/api/peerconnection.h b/webrtc/api/peerconnection.h index 3b0d558cfc..d715ecdf73 100644 --- a/webrtc/api/peerconnection.h +++ b/webrtc/api/peerconnection.h @@ -132,6 +132,8 @@ class PeerConnection : public PeerConnectionInterface, bool SetConfiguration( const PeerConnectionInterface::RTCConfiguration& config) override; bool AddIceCandidate(const IceCandidateInterface* candidate) override; + bool RemoveIceCandidates( + const std::vector& candidates) override; void RegisterUMAObserver(UMAObserver* observer) override; @@ -187,6 +189,8 @@ class PeerConnection : public PeerConnectionInterface, void OnIceConnectionChange(IceConnectionState new_state) override; void OnIceGatheringChange(IceGatheringState new_state) override; void OnIceCandidate(const IceCandidateInterface* candidate) override; + void OnIceCandidatesRemoved( + const std::vector& candidates) override; void OnIceConnectionReceivingChange(bool receiving) override; // Signals from WebRtcSession. diff --git a/webrtc/api/peerconnectioninterface.h b/webrtc/api/peerconnectioninterface.h index f6c8cbaf00..22109c4088 100644 --- a/webrtc/api/peerconnectioninterface.h +++ b/webrtc/api/peerconnectioninterface.h @@ -439,6 +439,12 @@ class PeerConnectionInterface : public rtc::RefCountInterface { // take the ownership of the |candidate|. virtual bool AddIceCandidate(const IceCandidateInterface* candidate) = 0; + // Removes a group of remote candidates from the ICE agent. + virtual bool RemoveIceCandidates( + const std::vector& candidates) { + return false; + } + virtual void RegisterUMAObserver(UMAObserver* observer) = 0; // Returns the current SignalingState. @@ -495,6 +501,12 @@ class PeerConnectionObserver { // New Ice candidate have been found. virtual void OnIceCandidate(const IceCandidateInterface* candidate) = 0; + // Ice candidates have been removed. + // TODO(honghaiz): Make this a pure virtual method when all its subclasses + // implement it. + virtual void OnIceCandidatesRemoved( + const std::vector& candidates) {} + // Called when the ICE connection receiving status changes. virtual void OnIceConnectionReceivingChange(bool receiving) {} diff --git a/webrtc/api/peerconnectionproxy.h b/webrtc/api/peerconnectionproxy.h index e47bc965a3..1183e61910 100644 --- a/webrtc/api/peerconnectionproxy.h +++ b/webrtc/api/peerconnectionproxy.h @@ -66,6 +66,9 @@ BEGIN_PROXY_MAP(PeerConnection) SetConfiguration, const PeerConnectionInterface::RTCConfiguration&); PROXY_METHOD1(bool, AddIceCandidate, const IceCandidateInterface*) + PROXY_METHOD1(bool, + RemoveIceCandidates, + const std::vector&); PROXY_METHOD1(void, RegisterUMAObserver, UMAObserver*) PROXY_METHOD0(SignalingState, signaling_state) PROXY_METHOD0(IceState, ice_state) diff --git a/webrtc/api/webrtcsdp.cc b/webrtc/api/webrtcsdp.cc index 54bb0599d4..4a296d9ce2 100644 --- a/webrtc/api/webrtcsdp.cc +++ b/webrtc/api/webrtcsdp.cc @@ -154,8 +154,7 @@ static const char kValueConference[] = "conference"; // Candidate static const char kCandidateHost[] = "host"; static const char kCandidateSrflx[] = "srflx"; -// TODO: How to map the prflx with circket candidate type -// static const char kCandidatePrflx[] = "prflx"; +static const char kCandidatePrflx[] = "prflx"; static const char kCandidateRelay[] = "relay"; static const char kTcpCandidateType[] = "tcptype"; @@ -871,11 +870,14 @@ std::string SdpSerialize(const JsepSessionDescription& jdesc, // Serializes the passed in IceCandidateInterface to a SDP string. // candidate - The candidate to be serialized. -std::string SdpSerializeCandidate( - const IceCandidateInterface& candidate) { +std::string SdpSerializeCandidate(const IceCandidateInterface& candidate) { + return SdpSerializeCandidate(candidate.candidate()); +} + +// Serializes a cricket Candidate. +std::string SdpSerializeCandidate(const cricket::Candidate& candidate) { std::string message; - std::vector candidates; - candidates.push_back(candidate.candidate()); + std::vector candidates(1, candidate); BuildCandidate(candidates, true, &message); // From WebRTC draft section 4.8.1.1 candidate-attribute will be // just candidate: not a=candidate:CRLF @@ -938,6 +940,18 @@ bool SdpDeserializeCandidate(const std::string& message, return true; } +bool SdpDeserializeCandidate(const std::string& transport_name, + const std::string& message, + cricket::Candidate* candidate, + SdpParseError* error) { + ASSERT(candidate != nullptr); + if (!ParseCandidate(message, candidate, error, true)) { + return false; + } + candidate->set_transport_name(transport_name); + return true; +} + bool ParseCandidate(const std::string& message, Candidate* candidate, SdpParseError* error, bool is_raw) { ASSERT(candidate != NULL); @@ -1026,6 +1040,8 @@ bool ParseCandidate(const std::string& message, Candidate* candidate, candidate_type = cricket::STUN_PORT_TYPE; } else if (type == kCandidateRelay) { candidate_type = cricket::RELAY_PORT_TYPE; + } else if (type == kCandidatePrflx) { + candidate_type = cricket::PRFLX_PORT_TYPE; } else { return ParseFailed(first_line, "Unsupported candidate type.", error); } @@ -1761,6 +1777,9 @@ void BuildCandidate(const std::vector& candidates, type = kCandidateSrflx; } else if (it->type() == cricket::RELAY_PORT_TYPE) { type = kCandidateRelay; + } else if (it->type() == cricket::PRFLX_PORT_TYPE) { + type = kCandidatePrflx; + // Peer reflexive candidate may be signaled for being removed. } else { ASSERT(false); // Never write out candidates if we don't know the type. diff --git a/webrtc/api/webrtcsdp.h b/webrtc/api/webrtcsdp.h index 2b22b6249f..e7fdb34d01 100644 --- a/webrtc/api/webrtcsdp.h +++ b/webrtc/api/webrtcsdp.h @@ -22,8 +22,11 @@ #include -namespace webrtc { +namespace cricket { +class Candidate; +} // namespace cricket +namespace webrtc { class IceCandidateInterface; class JsepIceCandidate; class JsepSessionDescription; @@ -42,6 +45,10 @@ std::string SdpSerialize(const JsepSessionDescription& jdesc, // candidate - The candidate to be serialized. std::string SdpSerializeCandidate(const IceCandidateInterface& candidate); +// Serializes a cricket Candidate. +// candidate - The candidate to be serialized. +std::string SdpSerializeCandidate(const cricket::Candidate& candidate); + // Deserializes the passed in SDP string to a JsepSessionDescription. // message - SDP string to be Deserialized. // jdesc - The JsepSessionDescription deserialized from the SDP string. @@ -61,6 +68,20 @@ bool SdpDeserialize(const std::string& message, bool SdpDeserializeCandidate(const std::string& message, JsepIceCandidate* candidate, SdpParseError* error); + +// Deserializes the passed in SDP string to a cricket Candidate. +// The first line must be a=candidate line and only the first line will be +// parsed. +// transport_name - The transport name (MID) of the candidate. +// message - The SDP string to be deserialized. +// candidate - The cricket Candidate from the SDP string. +// error - The detail error information when parsing fails. +// return - true on success, false on failure. +bool SdpDeserializeCandidate(const std::string& transport_name, + const std::string& message, + cricket::Candidate* candidate, + SdpParseError* error); + } // namespace webrtc #endif // WEBRTC_API_WEBRTCSDP_H_ diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc index 2f3c911454..08a46ad276 100644 --- a/webrtc/api/webrtcsession.cc +++ b/webrtc/api/webrtcsession.cc @@ -501,6 +501,8 @@ WebRtcSession::WebRtcSession(webrtc::MediaControllerInterface* media_controller, this, &WebRtcSession::OnTransportControllerGatheringState); transport_controller_->SignalCandidatesGathered.connect( this, &WebRtcSession::OnTransportControllerCandidatesGathered); + transport_controller_->SignalCandidatesRemoved.connect( + this, &WebRtcSession::OnTransportControllerCandidatesRemoved); } WebRtcSession::~WebRtcSession() { @@ -1086,7 +1088,7 @@ bool WebRtcSession::ProcessIceMessage(const IceCandidateInterface* candidate) { if (!remote_desc_) { LOG(LS_ERROR) << "ProcessIceMessage: ICE candidates can't be added " << "without any remote session description."; - return false; + return false; } if (!candidate) { @@ -1114,6 +1116,35 @@ bool WebRtcSession::ProcessIceMessage(const IceCandidateInterface* candidate) { } } +bool WebRtcSession::RemoveRemoteIceCandidates( + const std::vector& candidates) { + if (!remote_desc_) { + LOG(LS_ERROR) << "RemoveRemoteIceCandidates: ICE candidates can't be " + << "removed without any remote session description."; + return false; + } + + if (candidates.empty()) { + LOG(LS_ERROR) << "RemoveRemoteIceCandidates: candidates are empty."; + return false; + } + + size_t number_removed = remote_desc_->RemoveCandidates(candidates); + if (number_removed != candidates.size()) { + LOG(LS_ERROR) << "RemoveRemoteIceCandidates: Failed to remove candidates. " + << "Requested " << candidates.size() << " but only " + << number_removed << " are removed."; + } + + // Remove the candidates from the transport controller. + std::string error; + bool res = transport_controller_->RemoveRemoteCandidates(candidates, &error); + if (!res && !error.empty()) { + LOG(LS_ERROR) << "Error when removing remote candidates: " << error; + } + return true; +} + bool WebRtcSession::SetIceTransports( PeerConnectionInterface::IceTransportsType type) { return port_allocator()->set_candidate_filter( @@ -1523,6 +1554,27 @@ void WebRtcSession::OnTransportControllerCandidatesGathered( } } +void WebRtcSession::OnTransportControllerCandidatesRemoved( + const std::vector& candidates) { + ASSERT(signaling_thread()->IsCurrent()); + // Sanity check. + for (const cricket::Candidate& candidate : candidates) { + if (candidate.transport_name().empty()) { + LOG(LS_ERROR) << "OnTransportControllerCandidatesRemoved: " + << "empty content name in candidate " + << candidate.ToString(); + return; + } + } + + if (local_desc_) { + local_desc_->RemoveCandidates(candidates); + } + if (ice_observer_) { + ice_observer_->OnIceCandidatesRemoved(candidates); + } +} + // Enabling voice and video channel. void WebRtcSession::EnableChannels() { if (voice_channel_ && !voice_channel_->enabled()) @@ -1582,14 +1634,11 @@ bool WebRtcSession::UseCandidatesInSessionDescription( return ret; } -bool WebRtcSession::UseCandidate( - const IceCandidateInterface* candidate) { - +bool WebRtcSession::UseCandidate(const IceCandidateInterface* candidate) { size_t mediacontent_index = static_cast(candidate->sdp_mline_index()); size_t remote_content_size = remote_desc_->description()->contents().size(); if (mediacontent_index >= remote_content_size) { - LOG(LS_ERROR) - << "UseRemoteCandidateInSession: Invalid candidate media index."; + LOG(LS_ERROR) << "UseCandidate: Invalid candidate media index."; return false; } @@ -1930,8 +1979,8 @@ bool WebRtcSession::ReadyToUseRemoteCandidate( 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."; + LOG(LS_ERROR) << "ReadyToUseRemoteCandidate: Invalid candidate media index " + << mediacontent_index; *valid = false; return false; diff --git a/webrtc/api/webrtcsession.h b/webrtc/api/webrtcsession.h index 27472c9136..9495116471 100644 --- a/webrtc/api/webrtcsession.h +++ b/webrtc/api/webrtcsession.h @@ -25,6 +25,7 @@ #include "webrtc/base/sslidentity.h" #include "webrtc/base/thread.h" #include "webrtc/media/base/mediachannel.h" +#include "webrtc/p2p/base/candidate.h" #include "webrtc/p2p/base/transportcontroller.h" #include "webrtc/pc/mediasession.h" @@ -81,6 +82,10 @@ class IceObserver { // New Ice candidate have been found. virtual void OnIceCandidate(const IceCandidateInterface* candidate) = 0; + // Some local ICE candidates have been removed. + virtual void OnIceCandidatesRemoved( + const std::vector& candidates) = 0; + // Called whenever the state changes between receiving and not receiving. virtual void OnIceConnectionReceivingChange(bool receiving) {} @@ -205,6 +210,9 @@ class WebRtcSession : public AudioProviderInterface, std::string* err_desc); bool ProcessIceMessage(const IceCandidateInterface* ice_candidate); + bool RemoveRemoteIceCandidates( + const std::vector& candidates); + bool SetIceTransports(PeerConnectionInterface::IceTransportsType type); cricket::IceConfig ParseIceConfig( @@ -431,7 +439,9 @@ class WebRtcSession : public AudioProviderInterface, void OnTransportControllerGatheringState(cricket::IceGatheringState state); void OnTransportControllerCandidatesGathered( const std::string& transport_name, - const cricket::Candidates& candidates); + const std::vector& candidates); + void OnTransportControllerCandidatesRemoved( + const std::vector& candidates); std::string GetSessionErrorMsg(); diff --git a/webrtc/api/webrtcsession_unittest.cc b/webrtc/api/webrtcsession_unittest.cc index 2c9a99b5e0..c0fff52513 100644 --- a/webrtc/api/webrtcsession_unittest.cc +++ b/webrtc/api/webrtcsession_unittest.cc @@ -189,11 +189,18 @@ class MockIceObserver : public webrtc::IceObserver { EXPECT_NE(PeerConnectionInterface::kIceGatheringNew, ice_gathering_state_); } + // Some local candidates are removed. + void OnIceCandidatesRemoved( + const std::vector& candidates) { + num_candidates_removed_ += candidates.size(); + } + bool oncandidatesready_; std::vector mline_0_candidates_; std::vector mline_1_candidates_; PeerConnectionInterface::IceConnectionState ice_connection_state_; PeerConnectionInterface::IceGatheringState ice_gathering_state_; + size_t num_candidates_removed_ = 0; }; class WebRtcSessionForTest : public webrtc::WebRtcSession { @@ -358,6 +365,9 @@ class WebRtcSessionTest void AddInterface(const SocketAddress& addr) { network_manager_.AddInterface(addr); } + void RemoveInterface(const SocketAddress& addr) { + network_manager_.RemoveInterface(addr); + } // If |dtls_identity_store| != null or |rtc_configuration| contains // |certificates| then DTLS will be enabled unless explicitly disabled by @@ -2106,12 +2116,14 @@ TEST_F(WebRtcSessionTest, TestSetRemoteAnswerWithoutOffer) { "Called in wrong state: STATE_INIT", answer); } -TEST_F(WebRtcSessionTest, TestAddRemoteCandidate) { +// Tests that the remote candidates are added and removed successfully. +TEST_F(WebRtcSessionTest, TestAddAndRemoveRemoteCandidates) { Init(); SendAudioVideoStream1(); - cricket::Candidate candidate; - candidate.set_component(1); + cricket::Candidate candidate(1, "udp", rtc::SocketAddress("1.1.1.1", 5000), 0, + "", "", "host", 0, ""); + candidate.set_transport_name("audio"); JsepIceCandidate ice_candidate1(kMediaContentName0, 0, candidate); // Fail since we have not set a remote description. @@ -2129,6 +2141,7 @@ TEST_F(WebRtcSessionTest, TestAddRemoteCandidate) { EXPECT_TRUE(session_->ProcessIceMessage(&ice_candidate1)); candidate.set_component(2); + candidate.set_address(rtc::SocketAddress("2.2.2.2", 6000)); JsepIceCandidate ice_candidate2(kMediaContentName0, 0, candidate); EXPECT_TRUE(session_->ProcessIceMessage(&ice_candidate2)); @@ -2154,9 +2167,16 @@ TEST_F(WebRtcSessionTest, TestAddRemoteCandidate) { JsepIceCandidate bad_ice_candidate("bad content name", 99, candidate); EXPECT_FALSE(session_->ProcessIceMessage(&bad_ice_candidate)); + + // Remove candidate1 and candidate2 + std::vector remote_candidates; + remote_candidates.push_back(ice_candidate1.candidate()); + remote_candidates.push_back(ice_candidate2.candidate()); + EXPECT_TRUE(session_->RemoveRemoteIceCandidates(remote_candidates)); + EXPECT_EQ(0u, candidates->count()); } -// Test that a remote candidate is added to the remote session description and +// Tests that a remote candidate is added to the remote session description and // that it is retained if the remote session description is changed. TEST_F(WebRtcSessionTest, TestRemoteCandidatesAddedToSessionDescription) { Init(); @@ -2209,8 +2229,11 @@ TEST_F(WebRtcSessionTest, TestRemoteCandidatesAddedToSessionDescription) { } // Test that local candidates are added to the local session description and -// that they are retained if the local session description is changed. -TEST_F(WebRtcSessionTest, TestLocalCandidatesAddedToSessionDescription) { +// that they are retained if the local session description is changed. And if +// continual gathering is enabled, they are removed from the local session +// description when the network is down. +TEST_F(WebRtcSessionTest, + TestLocalCandidatesAddedAndRemovedIfGatherContinually) { AddInterface(rtc::SocketAddress(kClientAddrHost1, kClientAddrPort)); Init(); SendAudioVideoStream1(); @@ -2243,6 +2266,43 @@ TEST_F(WebRtcSessionTest, TestLocalCandidatesAddedToSessionDescription) { candidates = local_desc->candidates(1); ASSERT_TRUE(candidates != NULL); EXPECT_EQ(0u, candidates->count()); + + candidates = local_desc->candidates(kMediaContentIndex0); + size_t num_local_candidates = candidates->count(); + // Enable Continual Gathering + session_->SetIceConfig(cricket::IceConfig(-1, -1, true, false, -1)); + // Bring down the network interface to trigger candidate removals. + RemoveInterface(rtc::SocketAddress(kClientAddrHost1, kClientAddrPort)); + // Verify that all local candidates are removed. + EXPECT_EQ(0, observer_.num_candidates_removed_); + EXPECT_EQ_WAIT(num_local_candidates, observer_.num_candidates_removed_, + kIceCandidatesTimeout); + EXPECT_EQ_WAIT(0u, candidates->count(), kIceCandidatesTimeout); +} + +// Tests that if continual gathering is disabled, local candidates won't be +// removed when the interface is turned down. +TEST_F(WebRtcSessionTest, TestLocalCandidatesNotRemovedIfNotGatherContinually) { + AddInterface(rtc::SocketAddress(kClientAddrHost1, kClientAddrPort)); + Init(); + SendAudioVideoStream1(); + CreateAndSetRemoteOfferAndLocalAnswer(); + + const SessionDescriptionInterface* local_desc = session_->local_description(); + const IceCandidateCollection* candidates = + local_desc->candidates(kMediaContentIndex0); + ASSERT_TRUE(candidates != NULL); + EXPECT_TRUE_WAIT(observer_.oncandidatesready_, kIceCandidatesTimeout); + + size_t num_local_candidates = candidates->count(); + EXPECT_LT(0u, num_local_candidates); + // By default, Continual Gathering is disabled. + // Bring down the network interface. + RemoveInterface(rtc::SocketAddress(kClientAddrHost1, kClientAddrPort)); + // Verify that the local candidates are not removed. + rtc::Thread::Current()->ProcessMessages(1000); + EXPECT_EQ(0, observer_.num_candidates_removed_); + EXPECT_EQ(num_local_candidates, candidates->count()); } // Test that we can set a remote session description with remote candidates. diff --git a/webrtc/examples/androidapp/src/org/appspot/apprtc/AppRTCClient.java b/webrtc/examples/androidapp/src/org/appspot/apprtc/AppRTCClient.java index 195446ab5f..f9cb67bd02 100644 --- a/webrtc/examples/androidapp/src/org/appspot/apprtc/AppRTCClient.java +++ b/webrtc/examples/androidapp/src/org/appspot/apprtc/AppRTCClient.java @@ -58,6 +58,11 @@ public interface AppRTCClient { */ public void sendLocalIceCandidate(final IceCandidate candidate); + /** + * Send removed ICE candidates to the other participant. + */ + public void sendLocalIceCandidateRemovals(final IceCandidate[] candidates); + /** * Disconnect from room. */ @@ -112,6 +117,11 @@ public interface AppRTCClient { */ public void onRemoteIceCandidate(final IceCandidate candidate); + /** + * Callback fired once remote Ice candidate removals are received. + */ + public void onRemoteIceCandidatesRemoved(final IceCandidate[] candidates); + /** * Callback fired once channel is closed. */ diff --git a/webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java b/webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java index e2f130663b..b50935b809 100644 --- a/webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java +++ b/webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java @@ -558,8 +558,7 @@ public class CallActivity extends Activity @Override public void run() { if (peerConnectionClient == null) { - Log.e(TAG, - "Received ICE candidate for non-initilized peer connection."); + Log.e(TAG, "Received ICE candidate for a non-initialized peer connection."); return; } peerConnectionClient.addRemoteIceCandidate(candidate); @@ -567,6 +566,20 @@ public class CallActivity extends Activity }); } + @Override + public void onRemoteIceCandidatesRemoved(final IceCandidate[] candidates) { + runOnUiThread(new Runnable() { + @Override + public void run() { + if (peerConnectionClient == null) { + Log.e(TAG, "Received ICE candidate removals for a non-initialized peer connection."); + return; + } + peerConnectionClient.removeRemoteIceCandidates(candidates); + } + }); + } + @Override public void onChannelClose() { runOnUiThread(new Runnable() { @@ -617,6 +630,18 @@ public class CallActivity extends Activity }); } + @Override + public void onIceCandidatesRemoved(final IceCandidate[] candidates) { + runOnUiThread(new Runnable() { + @Override + public void run() { + if (appRtcClient != null) { + appRtcClient.sendLocalIceCandidateRemovals(candidates); + } + } + }); + } + @Override public void onIceConnected() { final long delta = System.currentTimeMillis() - callStartedTimeMs; diff --git a/webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java b/webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java index eb4d959067..ecd0da4c86 100644 --- a/webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java +++ b/webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java @@ -183,6 +183,11 @@ public class PeerConnectionClient { */ public void onIceCandidate(final IceCandidate candidate); + /** + * Callback fired once local ICE candidates are removed. + */ + public void onIceCandidatesRemoved(final IceCandidate[] candidates); + /** * Callback fired once connection is established (IceConnectionState is * CONNECTED). @@ -655,6 +660,21 @@ public class PeerConnectionClient { }); } + public void removeRemoteIceCandidates(final IceCandidate[] candidates) { + executor.execute(new Runnable() { + @Override + public void run() { + if (peerConnection == null || isError) { + return; + } + // Drain the queued remote candidates if there is any so that + // they are processed in the proper order. + drainCandidates(); + peerConnection.removeIceCandidates(candidates); + } + }); + } + public void setRemoteDescription(final SessionDescription sdp) { executor.execute(new Runnable() { @Override @@ -923,6 +943,16 @@ public class PeerConnectionClient { }); } + @Override + public void onIceCandidatesRemoved(final IceCandidate[] candidates) { + executor.execute(new Runnable() { + @Override + public void run() { + events.onIceCandidatesRemoved(candidates); + } + }); + } + @Override public void onSignalingChange( PeerConnection.SignalingState newState) { diff --git a/webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java b/webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java index ca319abde2..258b22f973 100644 --- a/webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java +++ b/webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java @@ -19,6 +19,7 @@ import org.appspot.apprtc.util.LooperExecutor; import android.util.Log; +import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; import org.webrtc.IceCandidate; @@ -252,6 +253,37 @@ public class WebSocketRTCClient implements AppRTCClient, }); } + // Send removed Ice candidates to the other participant. + @Override + public void sendLocalIceCandidateRemovals(final IceCandidate[] candidates) { + executor.execute(new Runnable() { + @Override + public void run() { + JSONObject json = new JSONObject(); + jsonPut(json, "type", "remove-candidates"); + JSONArray jsonArray = new JSONArray(); + for (final IceCandidate candidate : candidates) { + jsonArray.put(toJsonCandidate(candidate)); + } + jsonPut(json, "candidates", jsonArray); + if (initiator) { + // Call initiator sends ice candidates to GAE server. + if (roomState != ConnectionState.CONNECTED) { + reportError("Sending ICE candidate removals in non connected state."); + return; + } + sendPostMessage(MessageType.MESSAGE, messageUrl, json.toString()); + if (connectionParameters.loopback) { + events.onRemoteIceCandidatesRemoved(candidates); + } + } else { + // Call receiver sends ice candidates to websocket server. + wsClient.send(json.toString()); + } + } + }); + } + // -------------------------------------------------------------------- // WebSocketChannelEvents interface implementation. // All events are called by WebSocketChannelClient on a local looper thread @@ -270,11 +302,14 @@ public class WebSocketRTCClient implements AppRTCClient, json = new JSONObject(msgText); String type = json.optString("type"); if (type.equals("candidate")) { - IceCandidate candidate = new IceCandidate( - json.getString("id"), - json.getInt("label"), - json.getString("candidate")); - events.onRemoteIceCandidate(candidate); + events.onRemoteIceCandidate(toJavaCandidate(json)); + } else if (type.equals("remove-candidates")) { + JSONArray candidateArray = json.getJSONArray("candidates"); + IceCandidate[] candidates = new IceCandidate[candidateArray.length()]; + for (int i =0; i < candidateArray.length(); ++i) { + candidates[i] = toJavaCandidate(candidateArray.getJSONObject(i)); + } + events.onRemoteIceCandidatesRemoved(candidates); } else if (type.equals("answer")) { if (initiator) { SessionDescription sdp = new SessionDescription( @@ -376,4 +411,20 @@ public class WebSocketRTCClient implements AppRTCClient, }); httpConnection.send(); } + + // Converts a Java candidate to a JSONObject. + private JSONObject toJsonCandidate(final IceCandidate candidate) { + JSONObject json = new JSONObject(); + jsonPut(json, "label", candidate.sdpMLineIndex); + jsonPut(json, "id", candidate.sdpMid); + jsonPut(json, "candidate", candidate.sdp); + return json; + } + + // Converts a JSON candidate to a Java object. + IceCandidate toJavaCandidate(JSONObject json) throws JSONException { + return new IceCandidate(json.getString("id"), + json.getInt("label"), + json.getString("candidate")); + } } diff --git a/webrtc/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java b/webrtc/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java index 08f5b91d23..ca7c2f937f 100644 --- a/webrtc/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java +++ b/webrtc/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java @@ -149,6 +149,11 @@ public class PeerConnectionClientTest extends InstrumentationTestCase } } + @Override + public void onIceCandidatesRemoved(final IceCandidate[] candidates) { + // TODO(honghaiz): Add this for tests. + } + @Override public void onIceConnected() { Log.d(TAG, "ICE Connected"); diff --git a/webrtc/p2p/base/candidate.h b/webrtc/p2p/base/candidate.h index afea2a21c7..11481cdb05 100644 --- a/webrtc/p2p/base/candidate.h +++ b/webrtc/p2p/base/candidate.h @@ -169,6 +169,12 @@ class Candidate { tcptype_ = tcptype; } + // The name of the transport channel of this candidate. + const std::string& transport_name() const { return transport_name_; } + void set_transport_name(const std::string& transport_name) { + transport_name_ = transport_name; + } + // Determines whether this candidate is equivalent to the given one. bool IsEquivalent(const Candidate& c) const { // We ignore the network name, since that is just debug information, and @@ -181,6 +187,13 @@ class Candidate { (related_address_ == c.related_address_); } + // Determines whether this candidate can be considered equivalent to the + // given one when looking for a matching candidate to remove. + bool MatchesForRemoval(const Candidate& c) const { + return component_ == c.component_ && protocol_ == c.protocol_ && + address_ == c.address_; + } + std::string ToString() const { return ToStringInternal(false); } @@ -222,10 +235,10 @@ class Candidate { std::ostringstream ost; std::string address = sensitive ? address_.ToSensitiveString() : address_.ToString(); - ost << "Cand[" << foundation_ << ":" << component_ << ":" << protocol_ - << ":" << priority_ << ":" << address << ":" << type_ << ":" - << related_address_ << ":" << username_ << ":" << password_ << ":" - << network_cost_ << "]"; + ost << "Cand[" << transport_name_ << ":" << foundation_ << ":" << component_ + << ":" << protocol_ << ":" << priority_ << ":" << address << ":" + << type_ << ":" << related_address_ << ":" << username_ << ":" + << password_ << ":" << network_cost_ << "]"; return ost.str(); } @@ -245,6 +258,7 @@ class Candidate { rtc::SocketAddress related_address_; std::string tcptype_; uint32_t network_cost_ = 0; + std::string transport_name_; }; // Used during parsing and writing to map component to channel name diff --git a/webrtc/p2p/base/dtlstransportchannel.cc b/webrtc/p2p/base/dtlstransportchannel.cc index 2185fb3aad..bbf3e5c484 100644 --- a/webrtc/p2p/base/dtlstransportchannel.cc +++ b/webrtc/p2p/base/dtlstransportchannel.cc @@ -113,6 +113,8 @@ DtlsTransportChannelWrapper::DtlsTransportChannelWrapper( this, &DtlsTransportChannelWrapper::OnGatheringState); channel_->SignalCandidateGathered.connect( this, &DtlsTransportChannelWrapper::OnCandidateGathered); + channel_->SignalCandidatesRemoved.connect( + this, &DtlsTransportChannelWrapper::OnCandidatesRemoved); channel_->SignalRoleConflict.connect(this, &DtlsTransportChannelWrapper::OnRoleConflict); channel_->SignalRouteChange.connect(this, @@ -613,6 +615,13 @@ void DtlsTransportChannelWrapper::OnCandidateGathered( SignalCandidateGathered(this, c); } +void DtlsTransportChannelWrapper::OnCandidatesRemoved( + TransportChannelImpl* channel, + const Candidates& candidates) { + ASSERT(channel == channel_); + SignalCandidatesRemoved(this, candidates); +} + void DtlsTransportChannelWrapper::OnRoleConflict( TransportChannelImpl* channel) { ASSERT(channel == channel_); diff --git a/webrtc/p2p/base/dtlstransportchannel.h b/webrtc/p2p/base/dtlstransportchannel.h index ad30441a15..a58b1a0f6b 100644 --- a/webrtc/p2p/base/dtlstransportchannel.h +++ b/webrtc/p2p/base/dtlstransportchannel.h @@ -186,6 +186,9 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl { void AddRemoteCandidate(const Candidate& candidate) override { channel_->AddRemoteCandidate(candidate); } + void RemoveRemoteCandidate(const Candidate& candidate) override { + channel_->RemoveRemoteCandidate(candidate); + } void SetIceConfig(const IceConfig& config) override { channel_->SetIceConfig(config); @@ -209,6 +212,8 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl { bool HandleDtlsPacket(const char* data, size_t size); void OnGatheringState(TransportChannelImpl* channel); void OnCandidateGathered(TransportChannelImpl* channel, const Candidate& c); + void OnCandidatesRemoved(TransportChannelImpl* channel, + const Candidates& candidates); void OnRoleConflict(TransportChannelImpl* channel); void OnRouteChange(TransportChannel* channel, const Candidate& candidate); void OnConnectionRemoved(TransportChannelImpl* channel); diff --git a/webrtc/p2p/base/faketransportcontroller.h b/webrtc/p2p/base/faketransportcontroller.h index c5e2afea54..ae5e86669a 100644 --- a/webrtc/p2p/base/faketransportcontroller.h +++ b/webrtc/p2p/base/faketransportcontroller.h @@ -216,6 +216,9 @@ class FakeTransportChannel : public TransportChannelImpl, void AddRemoteCandidate(const Candidate& candidate) override { remote_candidates_.push_back(candidate); } + + void RemoveRemoteCandidate(const Candidate& candidate) override {} + const Candidates& remote_candidates() const { return remote_candidates_; } void OnMessage(rtc::Message* msg) override { diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc index b21d584bf8..cdfcf95643 100644 --- a/webrtc/p2p/base/p2ptransportchannel.cc +++ b/webrtc/p2p/base/p2ptransportchannel.cc @@ -761,6 +761,19 @@ void P2PTransportChannel::AddRemoteCandidate(const Candidate& candidate) { SortConnections(); } +void P2PTransportChannel::RemoveRemoteCandidate( + const Candidate& cand_to_remove) { + auto iter = + std::remove_if(remote_candidates_.begin(), remote_candidates_.end(), + [cand_to_remove](const Candidate& candidate) { + return cand_to_remove.MatchesForRemoval(candidate); + }); + if (iter != remote_candidates_.end()) { + LOG(LS_VERBOSE) << "Removed remote candidate " << cand_to_remove.ToString(); + remote_candidates_.erase(iter, remote_candidates_.end()); + } +} + // Creates connections from all of the ports that we care about to the given // remote candidate. The return value is true if we created a connection from // the origin port. @@ -1449,7 +1462,11 @@ void P2PTransportChannel::OnPortNetworkInactive(PortInterface* port) { ports_.erase(it); LOG(INFO) << "Removed port due to inactive networks: " << ports_.size() << " remaining"; - // TODO(honghaiz): Signal candidate removals to the remote side. + std::vector candidates = port->Candidates(); + for (Candidate& candidate : candidates) { + candidate.set_transport_name(transport_name()); + } + SignalCandidatesRemoved(this, candidates); } // We data is available, let listeners know diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h index 3ca25d4cba..b86389a0bd 100644 --- a/webrtc/p2p/base/p2ptransportchannel.h +++ b/webrtc/p2p/base/p2ptransportchannel.h @@ -92,6 +92,7 @@ class P2PTransportChannel : public TransportChannelImpl, return gathering_state_; } void AddRemoteCandidate(const Candidate& candidate) override; + void RemoveRemoteCandidate(const Candidate& candidate) override; // Sets the parameters in IceConfig. We do not set them blindly. Instead, we // only update the parameter if it is considered set in |config|. For example, // a negative value of receiving_timeout will be considered "not set" and we diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc index c54f054fd8..e083c33f4d 100644 --- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc +++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc @@ -98,9 +98,7 @@ static const char* kIcePwd[4] = {"TESTICEPWD00000000000000", static const uint64_t kTiebreaker1 = 11111; static const uint64_t kTiebreaker2 = 22222; -enum { - MSG_CANDIDATE -}; +enum { MSG_ADD_CANDIDATES, MSG_REMOVE_CANDIDATES }; static cricket::IceConfig CreateIceConfig(int receiving_timeout, bool gather_continually, @@ -216,12 +214,14 @@ class P2PTransportChannelTestBase : public testing::Test, rtc::scoped_ptr ch_; }; - struct CandidateData : public rtc::MessageData { - CandidateData(cricket::TransportChannel* ch, const cricket::Candidate& c) - : channel(ch), candidate(c) { - } + struct CandidatesData : public rtc::MessageData { + CandidatesData(cricket::TransportChannel* ch, const cricket::Candidate& c) + : channel(ch), candidates(1, c) {} + CandidatesData(cricket::TransportChannel* ch, + const std::vector& cc) + : channel(ch), candidates(cc) {} cricket::TransportChannel* channel; - cricket::Candidate candidate; + cricket::Candidates candidates; }; struct Endpoint { @@ -262,7 +262,7 @@ class P2PTransportChannelTestBase : public testing::Test, uint64_t tiebreaker_; bool role_conflict_; bool save_candidates_; - std::vector saved_candidates_; + std::vector saved_candidates_; }; ChannelData* GetChannelData(cricket::TransportChannel* channel) { @@ -310,7 +310,9 @@ class P2PTransportChannelTestBase : public testing::Test, cricket::P2PTransportChannel* channel = new cricket::P2PTransportChannel( "test content name", component, GetAllocator(endpoint)); channel->SignalCandidateGathered.connect( - this, &P2PTransportChannelTestBase::OnCandidate); + this, &P2PTransportChannelTestBase::OnCandidateGathered); + channel->SignalCandidatesRemoved.connect( + this, &P2PTransportChannelTestBase::OnCandidatesRemoved); channel->SignalReadPacket.connect( this, &P2PTransportChannelTestBase::OnReadPacket); channel->SignalRoleConflict.connect( @@ -645,15 +647,15 @@ class P2PTransportChannelTestBase : public testing::Test, } // We pass the candidates directly to the other side. - void OnCandidate(cricket::TransportChannelImpl* ch, - const cricket::Candidate& c) { + void OnCandidateGathered(cricket::TransportChannelImpl* ch, + const cricket::Candidate& c) { if (force_relay_ && c.type() != cricket::RELAY_PORT_TYPE) return; if (GetEndpoint(ch)->save_candidates_) { - GetEndpoint(ch)->saved_candidates_.push_back(new CandidateData(ch, c)); + GetEndpoint(ch)->saved_candidates_.push_back(new CandidatesData(ch, c)); } else { - main_->Post(this, MSG_CANDIDATE, new CandidateData(ch, c)); + main_->Post(this, MSG_ADD_CANDIDATES, new CandidatesData(ch, c)); } } @@ -661,26 +663,35 @@ class P2PTransportChannelTestBase : public testing::Test, GetEndpoint(endpoint)->save_candidates_ = true; } + void OnCandidatesRemoved(cricket::TransportChannelImpl* ch, + const std::vector& candidates) { + // Candidate removals are not paused. + CandidatesData* candidates_data = new CandidatesData(ch, candidates); + main_->Post(this, MSG_REMOVE_CANDIDATES, candidates_data); + } + // Tcp candidate verification has to be done when they are generated. void VerifySavedTcpCandidates(int endpoint, const std::string& tcptype) { for (auto& data : GetEndpoint(endpoint)->saved_candidates_) { - EXPECT_EQ(data->candidate.protocol(), cricket::TCP_PROTOCOL_NAME); - EXPECT_EQ(data->candidate.tcptype(), tcptype); - if (data->candidate.tcptype() == cricket::TCPTYPE_ACTIVE_STR) { - EXPECT_EQ(data->candidate.address().port(), cricket::DISCARD_PORT); - } else if (data->candidate.tcptype() == cricket::TCPTYPE_PASSIVE_STR) { - EXPECT_NE(data->candidate.address().port(), cricket::DISCARD_PORT); - } else { - FAIL() << "Unknown tcptype: " << data->candidate.tcptype(); + for (auto& candidate : data->candidates) { + EXPECT_EQ(candidate.protocol(), cricket::TCP_PROTOCOL_NAME); + EXPECT_EQ(candidate.tcptype(), tcptype); + if (candidate.tcptype() == cricket::TCPTYPE_ACTIVE_STR) { + EXPECT_EQ(candidate.address().port(), cricket::DISCARD_PORT); + } else if (candidate.tcptype() == cricket::TCPTYPE_PASSIVE_STR) { + EXPECT_NE(candidate.address().port(), cricket::DISCARD_PORT); + } else { + FAIL() << "Unknown tcptype: " << candidate.tcptype(); + } } } } void ResumeCandidates(int endpoint) { Endpoint* ed = GetEndpoint(endpoint); - std::vector::iterator it = ed->saved_candidates_.begin(); + std::vector::iterator it = ed->saved_candidates_.begin(); for (; it != ed->saved_candidates_.end(); ++it) { - main_->Post(this, MSG_CANDIDATE, *it); + main_->Post(this, MSG_ADD_CANDIDATES, *it); } ed->saved_candidates_.clear(); ed->save_candidates_ = false; @@ -688,18 +699,29 @@ class P2PTransportChannelTestBase : public testing::Test, void OnMessage(rtc::Message* msg) { switch (msg->message_id) { - case MSG_CANDIDATE: { - rtc::scoped_ptr data( - static_cast(msg->pdata)); + case MSG_ADD_CANDIDATES: { + rtc::scoped_ptr data( + static_cast(msg->pdata)); cricket::P2PTransportChannel* rch = GetRemoteChannel(data->channel); - cricket::Candidate c = data->candidate; - if (clear_remote_candidates_ufrag_pwd_) { - c.set_username(""); - c.set_password(""); + for (auto& c : data->candidates) { + if (clear_remote_candidates_ufrag_pwd_) { + c.set_username(""); + c.set_password(""); + } + LOG(LS_INFO) << "Candidate(" << data->channel->component() << "->" + << rch->component() << "): " << c.ToString(); + rch->AddRemoteCandidate(c); + } + break; + } + case MSG_REMOVE_CANDIDATES: { + rtc::scoped_ptr data( + static_cast(msg->pdata)); + cricket::P2PTransportChannel* rch = GetRemoteChannel(data->channel); + for (cricket::Candidate& c : data->candidates) { + LOG(LS_INFO) << "Removed remote candidate " << c.ToString(); + rch->RemoveRemoteCandidate(c); } - LOG(LS_INFO) << "Candidate(" << data->channel->component() << "->" - << rch->component() << "): " << c.ToString(); - rch->AddRemoteCandidate(c); break; } } @@ -1772,9 +1794,10 @@ TEST_F(P2PTransportChannelMultihomedTest, TestGetState) { ep2_ch1()->GetState(), 1000); } -// Tests that when a network interface becomes inactive, the ports associated -// with that network will be removed from the port list of the channel if -// and only if Continual Gathering is enabled. +// Tests that when a network interface becomes inactive, if and only if +// Continual Gathering is enabled, the ports associated with that network +// will be removed from the port list of the channel, and the respective +// remote candidates on the other participant will be removed eventually. TEST_F(P2PTransportChannelMultihomedTest, TestNetworkBecomesInactive) { AddAddress(0, kPublicAddrs[0]); AddAddress(1, kPublicAddrs[1]); @@ -1793,14 +1816,20 @@ TEST_F(P2PTransportChannelMultihomedTest, TestNetworkBecomesInactive) { // Endpoint 1 enabled continual gathering; the port will be removed // when the interface is removed. RemoveAddress(0, kPublicAddrs[0]); - EXPECT_EQ(0U, ep1_ch1()->ports().size()); + EXPECT_TRUE(ep1_ch1()->ports().empty()); + // The remote candidates will be removed eventually. + EXPECT_TRUE_WAIT(ep2_ch1()->remote_candidates().empty(), 1000); size_t num_ports = ep2_ch1()->ports().size(); EXPECT_LE(1U, num_ports); + size_t num_remote_candidates = ep1_ch1()->remote_candidates().size(); // Endpoint 2 did not enable continual gathering; the port will not be removed - // when the interface is removed. + // when the interface is removed and neither the remote candidates on the + // other participant. RemoveAddress(1, kPublicAddrs[1]); + rtc::Thread::Current()->ProcessMessages(500); EXPECT_EQ(num_ports, ep2_ch1()->ports().size()); + EXPECT_EQ(num_remote_candidates, ep1_ch1()->remote_candidates().size()); } /* diff --git a/webrtc/p2p/base/transport.cc b/webrtc/p2p/base/transport.cc index 17d05adc23..b8d98100cf 100644 --- a/webrtc/p2p/base/transport.cc +++ b/webrtc/p2p/base/transport.cc @@ -294,6 +294,22 @@ bool Transport::VerifyCandidate(const Candidate& cand, std::string* error) { } } + if (!HasChannel(cand.component())) { + *error = "Candidate has an unknown component: " + cand.ToString() + + " for content: " + name(); + return false; + } + + return true; +} + +bool Transport::VerifyCandidates(const Candidates& candidates, + std::string* error) { + for (const Candidate& candidate : candidates) { + if (!VerifyCandidate(candidate, error)) { + return false; + } + } return true; } @@ -318,16 +334,9 @@ bool Transport::GetStats(TransportStats* stats) { bool Transport::AddRemoteCandidates(const std::vector& candidates, std::string* error) { ASSERT(!channels_destroyed_); - // Verify each candidate before passing down to transport layer. - for (const Candidate& cand : candidates) { - if (!VerifyCandidate(cand, error)) { - return false; - } - if (!HasChannel(cand.component())) { - *error = "Candidate has unknown component: " + cand.ToString() + - " for content: " + name(); - return false; - } + // Verify each candidate before passing down to the transport layer. + if (!VerifyCandidates(candidates, error)) { + return false; } for (const Candidate& candidate : candidates) { @@ -339,6 +348,23 @@ bool Transport::AddRemoteCandidates(const std::vector& candidates, return true; } +bool Transport::RemoveRemoteCandidates(const std::vector& candidates, + std::string* error) { + ASSERT(!channels_destroyed_); + // Verify each candidate before passing down to the transport layer. + if (!VerifyCandidates(candidates, error)) { + return false; + } + + for (const Candidate& candidate : candidates) { + TransportChannelImpl* channel = GetChannel(candidate.component()); + if (channel != nullptr) { + channel->RemoveRemoteCandidate(candidate); + } + } + return true; +} + bool Transport::ApplyLocalTransportDescription(TransportChannelImpl* ch, std::string* error_desc) { ch->SetIceCredentials(local_description_->ice_ufrag, diff --git a/webrtc/p2p/base/transport.h b/webrtc/p2p/base/transport.h index 84581c8e7b..66e300a154 100644 --- a/webrtc/p2p/base/transport.h +++ b/webrtc/p2p/base/transport.h @@ -258,11 +258,8 @@ class Transport : public sigslot::has_slots<> { // Called when one or more candidates are ready from the remote peer. bool AddRemoteCandidates(const std::vector& candidates, std::string* error); - - // If candidate is not acceptable, returns false and sets error. - // Call this before calling OnRemoteCandidates. - virtual bool VerifyCandidate(const Candidate& candidate, - std::string* error); + bool RemoveRemoteCandidates(const std::vector& candidates, + std::string* error); virtual bool GetSslRole(rtc::SSLRole* ssl_role) const { return false; } @@ -316,6 +313,11 @@ class Transport : public sigslot::has_slots<> { std::string* error_desc); private: + // If a candidate is not acceptable, returns false and sets error. + // Call this before calling OnRemoteCandidates. + bool VerifyCandidate(const Candidate& candidate, std::string* error); + bool VerifyCandidates(const Candidates& candidates, std::string* error); + // Candidate component => TransportChannelImpl* typedef std::map ChannelMap; diff --git a/webrtc/p2p/base/transportchannelimpl.h b/webrtc/p2p/base/transportchannelimpl.h index 1fa088a30d..904ebf563f 100644 --- a/webrtc/p2p/base/transportchannelimpl.h +++ b/webrtc/p2p/base/transportchannelimpl.h @@ -76,7 +76,10 @@ class TransportChannelImpl : public TransportChannel { // before forwarding. sigslot::signal2 SignalCandidateGathered; + sigslot::signal2 + SignalCandidatesRemoved; virtual void AddRemoteCandidate(const Candidate& candidate) = 0; + virtual void RemoveRemoteCandidate(const Candidate& candidate) = 0; virtual IceGatheringState gathering_state() const = 0; diff --git a/webrtc/p2p/base/transportcontroller.cc b/webrtc/p2p/base/transportcontroller.cc index 053388eeb8..128d2fc656 100644 --- a/webrtc/p2p/base/transportcontroller.cc +++ b/webrtc/p2p/base/transportcontroller.cc @@ -127,6 +127,12 @@ bool TransportController::AddRemoteCandidates(const std::string& transport_name, transport_name, candidates, err)); } +bool TransportController::RemoveRemoteCandidates(const Candidates& candidates, + std::string* err) { + return worker_thread_->Invoke(rtc::Bind( + &TransportController::RemoveRemoteCandidates_w, this, candidates, err)); +} + bool TransportController::ReadyForRemoteCandidates( const std::string& transport_name) { return worker_thread_->Invoke(rtc::Bind( @@ -162,6 +168,8 @@ TransportChannel* TransportController::CreateTransportChannel_w( this, &TransportController::OnChannelGatheringState_w); channel->SignalCandidateGathered.connect( this, &TransportController::OnChannelCandidateGathered_w); + channel->SignalCandidatesRemoved.connect( + this, &TransportController::OnChannelCandidatesRemoved_w); channel->SignalRoleConflict.connect( this, &TransportController::OnChannelRoleConflict_w); channel->SignalConnectionRemoved.connect( @@ -460,6 +468,28 @@ bool TransportController::AddRemoteCandidates_w( return transport->AddRemoteCandidates(candidates, err); } +bool TransportController::RemoveRemoteCandidates_w(const Candidates& candidates, + std::string* err) { + RTC_DCHECK(worker_thread()->IsCurrent()); + std::map candidates_by_transport_name; + for (const Candidate& cand : candidates) { + RTC_DCHECK(!cand.transport_name().empty()); + candidates_by_transport_name[cand.transport_name()].push_back(cand); + } + + bool result = true; + for (auto kv : candidates_by_transport_name) { + Transport* transport = GetTransport_w(kv.first); + if (!transport) { + // If we didn't find a transport, that's not an error; + // it could have been deleted as a result of bundling. + continue; + } + result &= transport->RemoveRemoteCandidates(kv.second, err); + } + return result; +} + bool TransportController::ReadyForRemoteCandidates_w( const std::string& transport_name) { RTC_DCHECK(worker_thread()->IsCurrent()); @@ -518,6 +548,21 @@ void TransportController::OnChannelCandidateGathered_w( signaling_thread_->Post(this, MSG_CANDIDATESGATHERED, data); } +void TransportController::OnChannelCandidatesRemoved_w( + TransportChannelImpl* channel, + const Candidates& candidates) { + invoker_.AsyncInvoke( + signaling_thread_, + rtc::Bind(&TransportController::OnChannelCandidatesRemoved, this, + candidates)); +} + +void TransportController::OnChannelCandidatesRemoved( + const Candidates& candidates) { + RTC_DCHECK(signaling_thread_->IsCurrent()); + SignalCandidatesRemoved(candidates); +} + void TransportController::OnChannelRoleConflict_w( TransportChannelImpl* channel) { RTC_DCHECK(worker_thread_->IsCurrent()); diff --git a/webrtc/p2p/base/transportcontroller.h b/webrtc/p2p/base/transportcontroller.h index 450e6b391f..ed7216033b 100644 --- a/webrtc/p2p/base/transportcontroller.h +++ b/webrtc/p2p/base/transportcontroller.h @@ -15,6 +15,7 @@ #include #include +#include "webrtc/base/asyncinvoker.h" #include "webrtc/base/sigslot.h" #include "webrtc/base/sslstreamadapter.h" #include "webrtc/p2p/base/candidate.h" @@ -74,6 +75,7 @@ class TransportController : public sigslot::has_slots<>, bool AddRemoteCandidates(const std::string& transport_name, const Candidates& candidates, std::string* err); + bool RemoveRemoteCandidates(const Candidates& candidates, std::string* err); bool ReadyForRemoteCandidates(const std::string& transport_name); bool GetStats(const std::string& transport_name, TransportStats* stats); @@ -108,6 +110,8 @@ class TransportController : public sigslot::has_slots<>, sigslot::signal2 SignalCandidatesGathered; + sigslot::signal1 SignalCandidatesRemoved; + // for unit test const rtc::scoped_refptr& certificate_for_testing(); @@ -176,6 +180,7 @@ class TransportController : public sigslot::has_slots<>, bool AddRemoteCandidates_w(const std::string& transport_name, const Candidates& candidates, std::string* err); + bool RemoveRemoteCandidates_w(const Candidates& candidates, std::string* err); bool ReadyForRemoteCandidates_w(const std::string& transport_name); bool GetStats_w(const std::string& transport_name, TransportStats* stats); @@ -185,6 +190,9 @@ class TransportController : public sigslot::has_slots<>, void OnChannelGatheringState_w(TransportChannelImpl* channel); void OnChannelCandidateGathered_w(TransportChannelImpl* channel, const Candidate& candidate); + void OnChannelCandidatesRemoved(const Candidates& candidates); + void OnChannelCandidatesRemoved_w(TransportChannelImpl* channel, + const Candidates& candidates); void OnChannelRoleConflict_w(TransportChannelImpl* channel); void OnChannelConnectionRemoved_w(TransportChannelImpl* channel); @@ -212,6 +220,7 @@ class TransportController : public sigslot::has_slots<>, bool ice_role_switch_ = false; uint64_t ice_tiebreaker_ = rtc::CreateRandomId64(); rtc::scoped_refptr certificate_; + rtc::AsyncInvoker invoker_; }; } // namespace cricket