From 84430da6817ce69c53bfad088be5c9df8b420f01 Mon Sep 17 00:00:00 2001 From: honghaiz Date: Fri, 11 Mar 2016 13:28:09 -0800 Subject: [PATCH] When doing candidate re-gathering in the same generation, Remove the existing local candidate on the same network and signaling the remote side to remove its remote candidate by setting the candidate priority to 0. BUG= Review URL: https://codereview.webrtc.org/1648813004 Cr-Commit-Position: refs/heads/master@{#11958} --- .../src/org/webrtc/PeerConnectionTest.java | 3 + webrtc/api/java/jni/peerconnection_jni.cc | 75 +++++++++++- .../java/src/org/webrtc/PeerConnection.java | 9 ++ webrtc/api/jsep.h | 7 +- webrtc/api/jsepicecandidate.cc | 13 +++ webrtc/api/jsepicecandidate.h | 3 + webrtc/api/jsepsessiondescription.cc | 26 +++++ webrtc/api/jsepsessiondescription.h | 4 + webrtc/api/jsepsessiondescription_unittest.cc | 15 ++- webrtc/api/peerconnection.cc | 12 ++ webrtc/api/peerconnection.h | 4 + webrtc/api/peerconnectioninterface.h | 12 ++ webrtc/api/peerconnectionproxy.h | 3 + webrtc/api/webrtcsdp.cc | 31 ++++- webrtc/api/webrtcsdp.h | 23 +++- webrtc/api/webrtcsession.cc | 65 +++++++++-- webrtc/api/webrtcsession.h | 12 +- webrtc/api/webrtcsession_unittest.cc | 72 +++++++++++- .../src/org/appspot/apprtc/AppRTCClient.java | 10 ++ .../src/org/appspot/apprtc/CallActivity.java | 29 ++++- .../appspot/apprtc/PeerConnectionClient.java | 30 +++++ .../appspot/apprtc/WebSocketRTCClient.java | 61 +++++++++- .../apprtc/test/PeerConnectionClientTest.java | 5 + webrtc/p2p/base/candidate.h | 22 +++- webrtc/p2p/base/dtlstransportchannel.cc | 9 ++ webrtc/p2p/base/dtlstransportchannel.h | 5 + webrtc/p2p/base/faketransportcontroller.h | 3 + webrtc/p2p/base/p2ptransportchannel.cc | 19 +++- webrtc/p2p/base/p2ptransportchannel.h | 1 + .../p2p/base/p2ptransportchannel_unittest.cc | 107 +++++++++++------- webrtc/p2p/base/transport.cc | 46 ++++++-- webrtc/p2p/base/transport.h | 12 +- webrtc/p2p/base/transportchannelimpl.h | 3 + webrtc/p2p/base/transportcontroller.cc | 45 ++++++++ webrtc/p2p/base/transportcontroller.h | 9 ++ 35 files changed, 711 insertions(+), 94 deletions(-) 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