AddRemoteCandidate on the network thread
SdpOfferAnswerHandler now hands over most of the work of adding a
remote candidate over to PeerConnection where the work will be
carried out asynchronously on the network thread (was
synchronous/blocking).
Once added, reporting (ReportRemoteIceCandidateAdded) continues on the
signaling thread as before. The difference is though that we don't
block the UseCandidate() operation which is a part of applying the
local and remote descriptions.
Besides now being asynchronous, there's one behavioural change:
Before starting the 'add' operation, the validity of the candidate
instance to be added, is checked. Previously if such an error occurred,
the error was silently ignored.
Bug: webrtc:9987
Change-Id: Ic1bfb8e27670fc81038b6ccec95ff36c65d12262
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/206063
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33230}
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 8588ca8..b0a594b 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -4468,40 +4468,21 @@
bool SdpOfferAnswerHandler::UseCandidate(
const IceCandidateInterface* candidate) {
RTC_DCHECK_RUN_ON(signaling_thread());
+
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
RTCErrorOr<const cricket::ContentInfo*> result =
FindContentInfo(remote_description(), candidate);
- if (!result.ok()) {
- RTC_LOG(LS_ERROR) << "UseCandidate: Invalid candidate. "
- << result.error().message();
+ if (!result.ok())
return false;
- }
- std::vector<cricket::Candidate> candidates;
- candidates.push_back(candidate->candidate());
- // Invoking BaseSession method to handle remote candidates.
- RTCError error = transport_controller()->AddRemoteCandidates(
- result.value()->name, candidates);
- if (error.ok()) {
- ReportRemoteIceCandidateAdded(candidate->candidate());
- // Candidates successfully submitted for checking.
- if (pc_->ice_connection_state() ==
- PeerConnectionInterface::kIceConnectionNew ||
- pc_->ice_connection_state() ==
- PeerConnectionInterface::kIceConnectionDisconnected) {
- // If state is New, then the session has just gotten its first remote ICE
- // candidates, so go to Checking.
- // If state is Disconnected, the session is re-using old candidates or
- // receiving additional ones, so go to Checking.
- // If state is Connected, stay Connected.
- // TODO(bemasc): If state is Connected, and the new candidates are for a
- // newly added transport, then the state actually _should_ move to
- // checking. Add a way to distinguish that case.
- pc_->SetIceConnectionState(
- PeerConnectionInterface::kIceConnectionChecking);
- }
- // TODO(bemasc): If state is Completed, go back to Connected.
- } else {
- RTC_LOG(LS_WARNING) << error.message();
- }
+
+ const cricket::Candidate& c = candidate->candidate();
+ RTCError error = cricket::VerifyCandidate(c);
+ if (!error.ok())
+ return false;
+
+ pc_->AddRemoteCandidate(result.value()->name, c);
+
return true;
}
@@ -4546,20 +4527,6 @@
return has_transport;
}
-void SdpOfferAnswerHandler::ReportRemoteIceCandidateAdded(
- const cricket::Candidate& candidate) {
- pc_->NoteUsageEvent(UsageEvent::REMOTE_CANDIDATE_ADDED);
- if (candidate.address().IsPrivateIP()) {
- pc_->NoteUsageEvent(UsageEvent::REMOTE_PRIVATE_CANDIDATE_ADDED);
- }
- if (candidate.address().IsUnresolvedIP()) {
- pc_->NoteUsageEvent(UsageEvent::REMOTE_MDNS_CANDIDATE_ADDED);
- }
- if (candidate.address().family() == AF_INET6) {
- pc_->NoteUsageEvent(UsageEvent::REMOTE_IPV6_CANDIDATE_ADDED);
- }
-}
-
RTCErrorOr<const cricket::ContentInfo*> SdpOfferAnswerHandler::FindContentInfo(
const SessionDescriptionInterface* description,
const IceCandidateInterface* candidate) {
