Remove candidates when doing continual gathering

webrtc/api/webrtcsession.cc

Index: webrtc/api/webrtcsession.cc
diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc
index c693cb1077ee6e1d072bfd9a1308ad1175b6350e..595b9cef2aad75b5026f8ffcfb18ddb9872a0493 100644
--- a/webrtc/api/webrtcsession.cc
+++ b/webrtc/api/webrtcsession.cc
@@ -554,6 +554,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() {
@@ -1151,7 +1153,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) {
@@ -1179,6 +1181,28 @@ bool WebRtcSession::ProcessIceMessage(const IceCandidateInterface* candidate) {
   }
 }
 
+bool WebRtcSession::RemoveRemoteIceCandidates(
+    const std::vector<IceCandidateInterfaceRefPtr>& 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 is empty.";
+    return false;
+  }
+
+  int number_removed = remote_desc_->RemoveCandidates(candidates);
+  if (number_removed != candidates.size()) {
+    LOG(LS_ERROR) << "RemoveRemoteIceCandidates: Failed to remove candidates. "
+                  << "Requested " << candidates.size() << " only "
+                  << number_removed << " are removed.";
+  }
+  return CeaseRemoteCandidates(candidates);
+}
+
 bool WebRtcSession::SetIceTransports(
     PeerConnectionInterface::IceTransportsType type) {
   return port_allocator()->set_candidate_filter(
@@ -1589,6 +1613,33 @@ void WebRtcSession::OnTransportControllerCandidatesGathered(
   }
 }
 
+void WebRtcSession::OnTransportControllerCandidatesRemoved(
+    const std::string& transport_name,
+    const cricket::Candidates& candidates) {
+  ASSERT(signaling_thread()->IsCurrent());
+  int sdp_mline_index;
+  if (!GetLocalCandidateMediaIndex(transport_name, &sdp_mline_index)) {
+    LOG(LS_ERROR) << "OnTransportControllerCandidatesRemoved: content name "
+                  << transport_name << " not found";
+    return;
+  }
+
+  std::vector<IceCandidateInterfaceRefPtr> ice_candidates;
+  for (const cricket::Candidate& cand : candidates) {
+    // Use transport_name as the candidate media id.
+    JsepIceCandidate* candidate =
+        new JsepIceCandidate(transport_name, sdp_mline_index, cand);
+    ice_candidates.push_back(candidate);
+  }
+
+  if (local_desc_) {
+    local_desc_->RemoveCandidates(ice_candidates);
+  }
+  if (ice_observer_) {
+    ice_observer_->OnIceCandidatesRemoved(ice_candidates);
+  }
+}
+
 // Enabling voice and video channel.
 void WebRtcSession::EnableChannels() {
   if (voice_channel_ && !voice_channel_->enabled())
@@ -1648,24 +1699,26 @@ bool WebRtcSession::UseCandidatesInSessionDescription(
   return ret;
 }
 
-bool WebRtcSession::UseCandidate(
-    const IceCandidateInterface* candidate) {
-
+const cricket::ContentInfo* WebRtcSession::GetRemoteMediaContent(
+    const IceCandidateInterface* candidate) const {
   size_t mediacontent_index = static_cast<size_t>(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.";
-    return false;
+    return nullptr;
   }
+  return &(remote_desc_->description()->contents()[mediacontent_index]);
+}
 
-  cricket::ContentInfo content =
-      remote_desc_->description()->contents()[mediacontent_index];
-  std::vector<cricket::Candidate> candidates;
-  candidates.push_back(candidate->candidate());
+bool WebRtcSession::UseCandidate(const IceCandidateInterface* candidate) {
+  const cricket::ContentInfo* content = GetRemoteMediaContent(candidate);
+  if (content == nullptr) {
+    LOG(LS_ERROR) << "UseCandidate: media content not found";
+    return false;
+  }
+  std::vector<cricket::Candidate> candidates(1, candidate->candidate());
   // Invoking BaseSession method to handle remote candidates.
   std::string error;
-  if (transport_controller_->AddRemoteCandidates(content.name, candidates,
+  if (transport_controller_->AddRemoteCandidates(content->name, candidates,
                                                  &error)) {
     // Candidates successfully submitted for checking.
     if (ice_connection_state_ == PeerConnectionInterface::kIceConnectionNew ||
@@ -1690,6 +1743,34 @@ bool WebRtcSession::UseCandidate(
   return true;
 }
 
+bool WebRtcSession::CeaseRemoteCandidates(

[pthatcher1, 2016/03/01 23:51:54]

Why is this a separate method from RemoveRemoteCandidates?  It seems weird to
have both Cease and Remove.  Looking at this method, it seems it should be
called RemoveRemoteCandidates, but we've already got one of those.

If you *have* to keep this separate, perhaps call it
RemoteRemoteCandidatesFromTransportController.

[honghaiz3, 2016/03/02 19:06:40]

I tried to make it symmetric to the process of adding candidates. 
CeaseRemoteCandidates is opposite to UseCandidates. 
UseCandidate means the underlying stack (TransportController,
p2ptransportchannel) starts to use the candidates to create connections. 
CeaseRemoteCandidate means that the transport channels/controllers stop using
them, while RemoveRemoteCandidates including both removing them from the
description and stop using them. 

[pthatcher1, 2016/03/02 20:37:58]

Then please call it RemoveRemoteCandidatesFromTransportController

[honghaiz3, 2016/03/03 01:17:40]

Done.

+    const std::vector<IceCandidateInterfaceRefPtr>& ice_candidates) {
+  const cricket::ContentInfo* content = nullptr;
+  cricket::Candidates candidates;
+  for (auto ice_candidate : ice_candidates) {
+    const cricket::ContentInfo* new_content =
+        GetRemoteMediaContent(ice_candidate);
+    if (!new_content) {
+      LOG(LS_ERROR) << "CeaseRemoteCandidates: media content not found";
+      return false;
+    }
+    if (!content) {
+      content = new_content;
+    } else if (content != new_content) {
+      LOG(LS_ERROR) << "CeaseRemoteCandidates: media content is different";
+      return false;
+    }
+    candidates.push_back(ice_candidate->candidate());
+  }
+  std::string error;
+  bool res = transport_controller_->RemoveRemoteCandidates(content->name,
+                                                           candidates, &error);
+  if (!res) {
+    LOG(LS_WARNING) << "Error when Removing remote candidate: " << error;
+  }
+  return res;
+}
+
 void WebRtcSession::RemoveUnusedChannels(const SessionDescription* desc) {
   // Destroy video_channel_ first since it may have a pointer to the
   // voice_channel_.