Adding remote candidates only after having remote session description. (#478)

Author: jianjunz

talk/owt/sdk/p2p/p2ppeerconnectionchannel.cc

@@ -503,7 +503,17 @@ void P2PPeerConnectionChannel::OnMessageSignal(Json::Value& message) {
                               &sdp_mline_index);
     webrtc::IceCandidateInterface* ice_candidate = webrtc::CreateIceCandidate(
         sdp_mid, sdp_mline_index, candidate, nullptr);
-    peer_connection_->AddIceCandidate(ice_candidate);
+    if(peer_connection_->remote_description()){
+      if (!peer_connection_->AddIceCandidate(ice_candidate)) {
+        RTC_LOG(LS_WARNING) << "Failed to add remote candidate.";
+      }
+    } else{
+      rtc::CritScope cs(&pending_remote_candidates_crit_);
+      pending_remote_candidates_.push_back(
+          std::unique_ptr<webrtc::IceCandidateInterface>(ice_candidate));
+      RTC_LOG(LS_VERBOSE) << "Remote candidate is stored because remote "
+                             "session description is missing.";
+    }
   }
 }
 void P2PPeerConnectionChannel::OnMessageTracksAdded(
@@ -561,44 +571,47 @@ void P2PPeerConnectionChannel::OnMessageTrackSources(
 void P2PPeerConnectionChannel::OnMessageStreamInfo(Json::Value& stream_info) {
   // Stream information is useless in native layer.
 }
+
 void P2PPeerConnectionChannel::OnSignalingChange(
     PeerConnectionInterface::SignalingState new_state) {
   RTC_LOG(LS_INFO) << "Signaling state changed: " << new_state;
-  switch (new_state) {
-    case PeerConnectionInterface::SignalingState::kStable:
-      if (pending_remote_sdp_) {
-        scoped_refptr<FunctionalSetRemoteDescriptionObserver> observer =
-            FunctionalSetRemoteDescriptionObserver::Create(std::bind(
-                &P2PPeerConnectionChannel::OnSetRemoteDescriptionComplete, this,
-                std::placeholders::_1));
-        std::string sdp_string;
-        if (!pending_remote_sdp_->ToString(&sdp_string)) {
-          RTC_LOG(LS_ERROR) << "Error parsing local description.";
-          RTC_DCHECK(false);
-        }
-        std::vector<AudioCodec> audio_codecs;
-        for (auto& audio_enc_param : configuration_.audio) {
-          audio_codecs.push_back(audio_enc_param.codec.name);
-        }
-        sdp_string = SdpUtils::SetPreferAudioCodecs(sdp_string, audio_codecs);
-        std::vector<VideoCodec> video_codecs;
-        for (auto& video_enc_param : configuration_.video) {
-          video_codecs.push_back(video_enc_param.codec.name);
-        }
-        sdp_string = SdpUtils::SetPreferVideoCodecs(sdp_string, video_codecs);
-        std::unique_ptr<webrtc::SessionDescriptionInterface> new_desc(
-            webrtc::CreateSessionDescription(pending_remote_sdp_->type(),
-                                             sdp_string, nullptr));
-        pending_remote_sdp_.reset();
-        peer_connection_->SetRemoteDescription(std::move(new_desc), observer);
-      } else {
-        CheckWaitedList();
+  if (new_state == PeerConnectionInterface::SignalingState::kStable) {
+    if (pending_remote_sdp_) {
+      scoped_refptr<FunctionalSetRemoteDescriptionObserver> observer =
+          FunctionalSetRemoteDescriptionObserver::Create(std::bind(
+              &P2PPeerConnectionChannel::OnSetRemoteDescriptionComplete, this,
+              std::placeholders::_1));
+      std::string sdp_string;
+      if (!pending_remote_sdp_->ToString(&sdp_string)) {
+        RTC_LOG(LS_ERROR) << "Error parsing local description.";
+        RTC_DCHECK(false);
       }
-      break;
-    default:
-      break;
+      std::vector<AudioCodec> audio_codecs;
+      for (auto& audio_enc_param : configuration_.audio) {
+        audio_codecs.push_back(audio_enc_param.codec.name);
+      }
+      sdp_string = SdpUtils::SetPreferAudioCodecs(sdp_string, audio_codecs);
+      std::vector<VideoCodec> video_codecs;
+      for (auto& video_enc_param : configuration_.video) {
+        video_codecs.push_back(video_enc_param.codec.name);
+      }
+      sdp_string = SdpUtils::SetPreferVideoCodecs(sdp_string, video_codecs);
+      std::unique_ptr<webrtc::SessionDescriptionInterface> new_desc(
+          webrtc::CreateSessionDescription(pending_remote_sdp_->type(),
+                                           sdp_string, nullptr));
+      pending_remote_sdp_.reset();
+      peer_connection_->SetRemoteDescription(std::move(new_desc), observer);
+    } else {
+      CheckWaitedList();
+    }
+  }
+
+  if (new_state == PeerConnectionInterface::SignalingState::kStable ||
+      new_state == PeerConnectionInterface::SignalingState::kHaveRemoteOffer) {
+    DrainPendingRemoteCandidates();
   }
 }
+
 void P2PPeerConnectionChannel::OnAddStream(
     rtc::scoped_refptr<MediaStreamInterface> stream) {
   Json::Value stream_tracks;
@@ -1247,6 +1260,17 @@ void P2PPeerConnectionChannel::DrainPendingMessages() {
     pending_messages_.clear();
   }
 }
+
+void P2PPeerConnectionChannel::DrainPendingRemoteCandidates() {
+  rtc::CritScope cs(&pending_remote_candidates_crit_);
+  for (auto& ice_candidate : pending_remote_candidates_) {
+    if (!peer_connection_->AddIceCandidate(ice_candidate.get())) {
+      RTC_LOG(LS_WARNING) << "Failed to add remote candidate.";
+    }
+  }
+  pending_remote_candidates_.clear();
+}
+
 Json::Value P2PPeerConnectionChannel::UaInfo() {
   Json::Value ua;
   // SDK info includes verison and type.

talk/owt/sdk/p2p/p2ppeerconnectionchannel.h

@@ -158,6 +158,7 @@ class P2PPeerConnectionChannel : public P2PSignalingReceiverInterface,
   // Send all messages in pending message list.
   void DrainPendingMessages();
   // Cleans all variables associated with last peerconnection.
+  void DrainPendingRemoteCandidates();
   void CleanLastPeerConnection();
   // Returns user agent info as JSON object.
   Json::Value UaInfo();
@@ -211,6 +212,10 @@ class P2PPeerConnectionChannel : public P2PSignalingReceiverInterface,
       pending_messages_;
   // Protects |pending_messages_|.
   std::mutex pending_messages_mutex_;
+  rtc::CriticalSection pending_remote_candidates_crit_;
+  std::vector<std::unique_ptr<webrtc::IceCandidateInterface>>
+      pending_remote_candidates_
+          RTC_GUARDED_BY(pending_remote_candidates_crit_);
   // Indicates whether remote client supports WebRTC Plan B
   // (https://tools.ietf.org/html/draft-uberti-rtcweb-plan-00).
   // If plan B is not supported, at most one audio/video track is supported.