Merging BaseSession code into WebRtcSession.

talk/app/webrtc/webrtcsession.cc

Index: talk/app/webrtc/webrtcsession.cc
diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc
index 2ab9a1e6969599983724f9ffdce7aebb91b5d418..3adac45b3b36c031f889f00af46e6a4545ce0f11 100644
--- a/talk/app/webrtc/webrtcsession.cc
+++ b/talk/app/webrtc/webrtcsession.cc
@@ -330,7 +330,11 @@ static bool BadSdp(const std::string& source,
                    const std::string& reason,
                    std::string* err_desc) {
   std::ostringstream desc;
-  desc << "Failed to set " << source << " " << type << " sdp: " << reason;
+  desc << "Failed to set " << source;
+  if (!type.empty()) {
+    desc << " " << type;
+  }
+  desc << " sdp: " << reason;
 
   if (err_desc) {
     *err_desc = desc.str();
@@ -381,30 +385,21 @@ static bool BadAnswerSdp(cricket::ContentSource source,
   return BadSdp(source, SessionDescriptionInterface::kAnswer, reason, err_desc);
 }
 
-#define GET_STRING_OF_STATE(state)  \
-  case cricket::BaseSession::state:  \
-    result = #state;  \
+#define GET_STRING_OF_STATE(state)   \
+  case webrtc::WebRtcSession::state: \
+    result = #state;                 \
     break;
 
-static std::string GetStateString(cricket::BaseSession::State state) {
+static std::string GetStateString(webrtc::WebRtcSession::State state) {
   std::string result;
   switch (state) {
     GET_STRING_OF_STATE(STATE_INIT)
-    GET_STRING_OF_STATE(STATE_SENTINITIATE)
-    GET_STRING_OF_STATE(STATE_RECEIVEDINITIATE)
-    GET_STRING_OF_STATE(STATE_SENTPRACCEPT)
-    GET_STRING_OF_STATE(STATE_SENTACCEPT)
-    GET_STRING_OF_STATE(STATE_RECEIVEDPRACCEPT)
-    GET_STRING_OF_STATE(STATE_RECEIVEDACCEPT)
-    GET_STRING_OF_STATE(STATE_SENTMODIFY)
-    GET_STRING_OF_STATE(STATE_RECEIVEDMODIFY)
-    GET_STRING_OF_STATE(STATE_SENTREJECT)
-    GET_STRING_OF_STATE(STATE_RECEIVEDREJECT)
-    GET_STRING_OF_STATE(STATE_SENTREDIRECT)
-    GET_STRING_OF_STATE(STATE_SENTTERMINATE)
-    GET_STRING_OF_STATE(STATE_RECEIVEDTERMINATE)
+    GET_STRING_OF_STATE(STATE_SENTOFFER)
+    GET_STRING_OF_STATE(STATE_RECEIVEDOFFER)
+    GET_STRING_OF_STATE(STATE_SENTPRANSWER)
+    GET_STRING_OF_STATE(STATE_RECEIVEDPRANSWER)
     GET_STRING_OF_STATE(STATE_INPROGRESS)
-    GET_STRING_OF_STATE(STATE_DEINIT)
+    GET_STRING_OF_STATE(STATE_TERMINATED)
     default:
       ASSERT(false);
       break;
@@ -412,22 +407,19 @@ static std::string GetStateString(cricket::BaseSession::State state) {
   return result;
 }
 
-#define GET_STRING_OF_ERROR_CODE(err)  \
-  case cricket::BaseSession::err:  \
-    result = #err;  \
+#define GET_STRING_OF_ERROR_CODE(err) \
+  case webrtc::WebRtcSession::err:    \
+    result = #err;                    \
     break;
 
-static std::string GetErrorCodeString(cricket::BaseSession::Error err) {
+static std::string GetErrorCodeString(webrtc::WebRtcSession::Error err) {
   std::string result;
   switch (err) {
     GET_STRING_OF_ERROR_CODE(ERROR_NONE)
-    GET_STRING_OF_ERROR_CODE(ERROR_TIME)
-    GET_STRING_OF_ERROR_CODE(ERROR_RESPONSE)
-    GET_STRING_OF_ERROR_CODE(ERROR_NETWORK)
     GET_STRING_OF_ERROR_CODE(ERROR_CONTENT)
     GET_STRING_OF_ERROR_CODE(ERROR_TRANSPORT)
     default:
-      ASSERT(false);
+      RTC_DCHECK(false);
       break;
   }
   return result;
@@ -536,20 +528,21 @@ class IceRestartAnswerLatch {
   bool ice_restart_;
 };
 
-WebRtcSession::WebRtcSession(
-    cricket::ChannelManager* channel_manager,
-    rtc::Thread* signaling_thread,
-    rtc::Thread* worker_thread,
-    cricket::PortAllocator* port_allocator,
-    MediaStreamSignaling* mediastream_signaling)
-    : cricket::BaseSession(signaling_thread,
-                           worker_thread,
-                           port_allocator,
-                           rtc::ToString(rtc::CreateRandomId64() & LLONG_MAX),
-                           false),
+WebRtcSession::WebRtcSession(cricket::ChannelManager* channel_manager,
+                             rtc::Thread* signaling_thread,
+                             rtc::Thread* worker_thread,
+                             cricket::PortAllocator* port_allocator,
+                             MediaStreamSignaling* mediastream_signaling)
+    : signaling_thread_(signaling_thread),
+      worker_thread_(worker_thread),
+      port_allocator_(port_allocator),
       // RFC 3264: The numeric value of the session id and version in the
       // o line MUST be representable with a "64 bit signed integer".
       // Due to this constraint session id |sid_| is max limited to LLONG_MAX.
+      sid_(rtc::ToString(rtc::CreateRandomId64() & LLONG_MAX)),
+      transport_controller_(new cricket::TransportController(signaling_thread,
+                                                             worker_thread,
+                                                             port_allocator)),
       channel_manager_(channel_manager),
       mediastream_signaling_(mediastream_signaling),
       ice_observer_(NULL),
@@ -560,13 +553,14 @@ WebRtcSession::WebRtcSession(
       data_channel_type_(cricket::DCT_NONE),
       ice_restart_latch_(new IceRestartAnswerLatch),
       metrics_observer_(NULL) {
-  transport_controller()->SignalConnectionState.connect(
+  transport_controller_->SetIceRole(cricket::ICEROLE_CONTROLLED);
+  transport_controller_->SignalConnectionState.connect(
       this, &WebRtcSession::OnTransportControllerConnectionState);
-  transport_controller()->SignalReceiving.connect(
+  transport_controller_->SignalReceiving.connect(
       this, &WebRtcSession::OnTransportControllerReceiving);
-  transport_controller()->SignalGatheringState.connect(
+  transport_controller_->SignalGatheringState.connect(
       this, &WebRtcSession::OnTransportControllerGatheringState);
-  transport_controller()->SignalCandidatesGathered.connect(
+  transport_controller_->SignalCandidatesGathered.connect(
       this, &WebRtcSession::OnTransportControllerCandidatesGathered);
 }
 
@@ -586,9 +580,8 @@ WebRtcSession::~WebRtcSession() {
     SignalDataChannelDestroyed();
     channel_manager_->DestroyDataChannel(data_channel_.release());
   }
-  for (size_t i = 0; i < saved_candidates_.size(); ++i) {
-    delete saved_candidates_[i];
-  }
+
+  LOG(LS_INFO) << "Session: " << id() << " is destroyed.";
 }
 
 bool WebRtcSession::Initialize(
@@ -598,7 +591,7 @@ bool WebRtcSession::Initialize(
     const PeerConnectionInterface::RTCConfiguration& rtc_configuration) {
   bundle_policy_ = rtc_configuration.bundle_policy;
   rtcp_mux_policy_ = rtc_configuration.rtcp_mux_policy;
-  transport_controller()->SetSslMaxProtocolVersion(options.ssl_max_version);
+  transport_controller_->SetSslMaxProtocolVersion(options.ssl_max_version);
 
   // Obtain a certificate from RTCConfiguration if any were provided (optional).
   rtc::scoped_refptr<rtc::RTCCertificate> certificate;
@@ -782,17 +775,8 @@ bool WebRtcSession::Initialize(
   return true;
 }
 
-cricket::IceConfig WebRtcSession::ParseIceConfig(
-    const PeerConnectionInterface::RTCConfiguration& config) const {
-  cricket::IceConfig ice_config;
-  ice_config.receiving_timeout_ms = config.ice_connection_receiving_timeout;
-  ice_config.gather_continually = (config.continual_gathering_policy ==
-                                   PeerConnectionInterface::GATHER_CONTINUALLY);
-  return ice_config;
-}
-
 void WebRtcSession::Terminate() {

[pthatcher1, 2015/10/10 00:07:27]

Can we rename this Close()?

[Taylor Brandstetter, 2015/10/10 00:30:15]

Done.

-  SetState(STATE_RECEIVEDTERMINATE);
+  SetState(STATE_TERMINATED);
   RemoveUnusedChannels(NULL);
   ASSERT(!voice_channel_);
   ASSERT(!video_channel_);
@@ -808,13 +792,13 @@ cricket::SecurePolicy WebRtcSession::SdesPolicy() const {
 }
 
 bool WebRtcSession::GetSslRole(rtc::SSLRole* role) {
-  if (local_description() == NULL || remote_description() == NULL) {
+  if (!local_desc_ || !remote_desc_) {
     LOG(LS_INFO) << "Local and Remote descriptions must be applied to get "
                  << "SSL Role of the session.";
     return false;
   }
 
-  return transport_controller()->GetSslRole(role);
+  return transport_controller_->GetSslRole(role);
 }
 
 void WebRtcSession::CreateOffer(
@@ -840,10 +824,11 @@ bool WebRtcSession::SetLocalDescription(SessionDescriptionInterface* desc,
     return false;
   }
 
-  // Update the initiator flag if this session is the initiator.
+  // Update the initial_offerer flag if this session is the initial_offerer.
   Action action = GetAction(desc->type());
   if (state() == STATE_INIT && action == kOffer) {
-    set_initiator(true);
+    initial_offerer_ = true;
+    transport_controller_->SetIceRole(cricket::ICEROLE_CONTROLLING);
   }
 
   cricket::SecurePolicy sdes_policy =
@@ -854,7 +839,6 @@ bool WebRtcSession::SetLocalDescription(SessionDescriptionInterface* desc,
   // Update the MediaContentDescription crypto settings as per the policy set.
   UpdateSessionDescriptionSecurePolicy(crypto_required, desc->description());
 
-  set_local_description(desc->description()->Copy());
   local_desc_.reset(desc_temp.release());
 
   // Transport and Media channels will be created only when offer is set.
@@ -871,15 +855,8 @@ bool WebRtcSession::SetLocalDescription(SessionDescriptionInterface* desc,
     return false;
   }
 
-  if (remote_description()) {
-    // Now that we have a local description, we can push down remote candidates
-    // that we stored, and those from the remote description.
-    if (!saved_candidates_.empty()) {
-      // If there are saved candidates which arrived before the local
-      // description was set, copy those to the remote description.
-      CopySavedCandidates(remote_desc_.get());
-    }
-    // Push remote candidates in remote description to transport channels.
+  if (remote_desc_) {
+    // Now that we have a local description, we can push down remote candidates.
     UseCandidatesInSessionDescription(remote_desc_.get());
   }
 
@@ -891,7 +868,7 @@ bool WebRtcSession::SetLocalDescription(SessionDescriptionInterface* desc,
   if (data_channel_type_ == cricket::DCT_SCTP && GetSslRole(&role)) {
     mediastream_signaling_->OnDtlsRoleReadyForSctp(role);
   }
-  if (error() != cricket::BaseSession::ERROR_NONE) {
+  if (error() != ERROR_NONE) {
     return BadLocalSdp(desc->type(), GetSessionErrorMsg(), err_desc);
   }
   return true;
@@ -909,6 +886,10 @@ bool WebRtcSession::SetRemoteDescription(SessionDescriptionInterface* desc,
     return false;
   }
 
+  rtc::scoped_ptr<SessionDescriptionInterface> old_remote_desc(
+      remote_desc_.release());
+  remote_desc_.reset(desc_temp.release());
+
   // Transport and Media channels will be created only when offer is set.
   Action action = GetAction(desc->type());
   if (action == kOffer && !CreateChannels(desc->description())) {
@@ -922,40 +903,39 @@ bool WebRtcSession::SetRemoteDescription(SessionDescriptionInterface* desc,
 
   // NOTE: Candidates allocation will be initiated only when SetLocalDescription
   // is called.
-  set_remote_description(desc->description()->Copy());
   if (!UpdateSessionState(action, cricket::CS_REMOTE, err_desc)) {
     return false;
   }
 
   // Update remote MediaStreams.
   mediastream_signaling_->OnRemoteDescriptionChanged(desc);
-  if (local_description() && !UseCandidatesInSessionDescription(desc)) {
+
+  if (local_desc_ && !UseCandidatesInSessionDescription(desc)) {
     return BadRemoteSdp(desc->type(), kInvalidCandidates, err_desc);
   }
 
-  // Copy all saved candidates.
-  CopySavedCandidates(desc);
-
   // Check if this new SessionDescription contains new ice ufrag and password
   // that indicates the remote peer requests ice restart.
   bool ice_restart =
-      ice_restart_latch_->CheckForRemoteIceRestart(remote_desc_.get(), desc);
+      ice_restart_latch_->CheckForRemoteIceRestart(old_remote_desc.get(), desc);
   // We retain all received candidates only if ICE is not restarted.
   // When ICE is restarted, all previous candidates belong to an old generation
   // and should not be kept.
+  // TODO(deadbeef): This goes against the W3C spec which says the remote
+  // description should only contain candidates from the last set remote
+  // description plus any candidates added since then. We should remove this
+  // once we're sure it won't break anything.
   if (!ice_restart) {
     WebRtcSessionDescriptionFactory::CopyCandidatesFromSessionDescription(
-        remote_desc_.get(), desc);
+        old_remote_desc.get(), desc);
   }
 
-  remote_desc_.reset(desc_temp.release());
-
   rtc::SSLRole role;
   if (data_channel_type_ == cricket::DCT_SCTP && GetSslRole(&role)) {
     mediastream_signaling_->OnDtlsRoleReadyForSctp(role);
   }
 
-  if (error() != cricket::BaseSession::ERROR_NONE) {
+  if (error() != ERROR_NONE) {
     return BadRemoteSdp(desc->type(), GetSessionErrorMsg(), err_desc);
   }
 
@@ -974,6 +954,29 @@ bool WebRtcSession::SetRemoteDescription(SessionDescriptionInterface* desc,
   return true;
 }
 
+void WebRtcSession::LogState(State old_state, State new_state) {
+  LOG(LS_INFO) << "Session:" << id()
+               << " Old state:" << GetStateString(old_state)
+               << " New state:" << GetStateString(new_state);
+}
+
+void WebRtcSession::SetState(State state) {
+  ASSERT(signaling_thread_->IsCurrent());
+  if (state != state_) {
+    LogState(state_, state);
+    state_ = state;
+    SignalState(this, state_);
+  }
+}
+
+void WebRtcSession::SetError(Error error, const std::string& error_desc) {
+  ASSERT(signaling_thread_->IsCurrent());
+  if (error != error_) {
+    error_ = error;
+    error_desc_ = error_desc;
+  }
+}
+
 bool WebRtcSession::UpdateSessionState(
     Action action, cricket::ContentSource source,
     std::string* err_desc) {
@@ -981,18 +984,18 @@ bool WebRtcSession::UpdateSessionState(
 
   // If there's already a pending error then no state transition should happen.
   // But all call-sites should be verifying this before calling us!
-  ASSERT(error() == cricket::BaseSession::ERROR_NONE);
+  ASSERT(error() == ERROR_NONE);
   std::string td_err;
   if (action == kOffer) {
     if (!PushdownTransportDescription(source, cricket::CA_OFFER, &td_err)) {
       return BadOfferSdp(source, MakeTdErrorString(td_err), err_desc);
     }
-    SetState(source == cricket::CS_LOCAL ?
-        STATE_SENTINITIATE : STATE_RECEIVEDINITIATE);
+    SetState(source == cricket::CS_LOCAL ? STATE_SENTOFFER
+                                         : STATE_RECEIVEDOFFER);
     if (!PushdownMediaDescription(cricket::CA_OFFER, source, err_desc)) {
-      SetError(BaseSession::ERROR_CONTENT, *err_desc);
+      SetError(ERROR_CONTENT, *err_desc);
     }
-    if (error() != cricket::BaseSession::ERROR_NONE) {
+    if (error() != ERROR_NONE) {
       return BadOfferSdp(source, GetSessionErrorMsg(), err_desc);
     }
   } else if (action == kPrAnswer) {
@@ -1000,12 +1003,12 @@ bool WebRtcSession::UpdateSessionState(
       return BadPranswerSdp(source, MakeTdErrorString(td_err), err_desc);
     }
     EnableChannels();
-    SetState(source == cricket::CS_LOCAL ?
-        STATE_SENTPRACCEPT : STATE_RECEIVEDPRACCEPT);
+    SetState(source == cricket::CS_LOCAL ? STATE_SENTPRANSWER
+                                         : STATE_RECEIVEDPRANSWER);
     if (!PushdownMediaDescription(cricket::CA_PRANSWER, source, err_desc)) {
-      SetError(BaseSession::ERROR_CONTENT, *err_desc);
+      SetError(ERROR_CONTENT, *err_desc);
     }
-    if (error() != cricket::BaseSession::ERROR_NONE) {
+    if (error() != ERROR_NONE) {
       return BadPranswerSdp(source, GetSessionErrorMsg(), err_desc);
     }
   } else if (action == kAnswer) {
@@ -1013,11 +1016,9 @@ bool WebRtcSession::UpdateSessionState(
       return BadAnswerSdp(source, MakeTdErrorString(td_err), err_desc);
     }
     const cricket::ContentGroup* local_bundle =
-        BaseSession::local_description()->GetGroupByName(
-            cricket::GROUP_TYPE_BUNDLE);
+        local_desc_->description()->GetGroupByName(cricket::GROUP_TYPE_BUNDLE);
     const cricket::ContentGroup* remote_bundle =
-        BaseSession::remote_description()->GetGroupByName(
-            cricket::GROUP_TYPE_BUNDLE);
+        remote_desc_->description()->GetGroupByName(cricket::GROUP_TYPE_BUNDLE);
     if (local_bundle && remote_bundle) {
       // The answerer decides the transport to bundle on
       const cricket::ContentGroup* answer_bundle =
@@ -1028,18 +1029,29 @@ bool WebRtcSession::UpdateSessionState(
       }
     }
     EnableChannels();
-    SetState(source == cricket::CS_LOCAL ?
-        STATE_SENTACCEPT : STATE_RECEIVEDACCEPT);
+    SetState(STATE_INPROGRESS);
     if (!PushdownMediaDescription(cricket::CA_ANSWER, source, err_desc)) {
-      SetError(BaseSession::ERROR_CONTENT, *err_desc);
+      SetError(ERROR_CONTENT, *err_desc);
     }
-    if (error() != cricket::BaseSession::ERROR_NONE) {
+    if (error() != ERROR_NONE) {
       return BadAnswerSdp(source, GetSessionErrorMsg(), err_desc);
     }
   }
   return true;
 }
 
+WebRtcSession::Action WebRtcSession::GetAction(const std::string& type) {
+  if (type == SessionDescriptionInterface::kOffer) {
+    return WebRtcSession::kOffer;
+  } else if (type == SessionDescriptionInterface::kPrAnswer) {
+    return WebRtcSession::kPrAnswer;
+  } else if (type == SessionDescriptionInterface::kAnswer) {
+    return WebRtcSession::kAnswer;
+  }
+  ASSERT(false && "unknown action type");
+  return WebRtcSession::kOffer;
+}
+
 bool WebRtcSession::PushdownMediaDescription(
     cricket::ContentAction action,
     cricket::ContentSource source,
@@ -1048,11 +1060,11 @@ bool WebRtcSession::PushdownMediaDescription(
     if (!ch) {
       return true;
     } else if (source == cricket::CS_LOCAL) {
-      return ch->PushdownLocalDescription(
-          base_local_description(), action, err);
+      return ch->PushdownLocalDescription(local_desc_->description(), action,
+                                          err);
     } else {
-      return ch->PushdownRemoteDescription(
-          base_remote_description(), action, err);
+      return ch->PushdownRemoteDescription(remote_desc_->description(), action,
+                                           err);
     }
   };
 
@@ -1061,19 +1073,76 @@ bool WebRtcSession::PushdownMediaDescription(
           set_content(data_channel()));
 }
 
-WebRtcSession::Action WebRtcSession::GetAction(const std::string& type) {
-  if (type == SessionDescriptionInterface::kOffer) {
-    return WebRtcSession::kOffer;
-  } else if (type == SessionDescriptionInterface::kPrAnswer) {
-    return WebRtcSession::kPrAnswer;
-  } else if (type == SessionDescriptionInterface::kAnswer) {
-    return WebRtcSession::kAnswer;
+bool WebRtcSession::PushdownTransportDescription(cricket::ContentSource source,
+                                                 cricket::ContentAction action,
+                                                 std::string* error_desc) {
+  RTC_DCHECK(signaling_thread()->IsCurrent());
+
+  if (source == cricket::CS_LOCAL) {
+    return PushdownLocalTransportDescription(local_desc_->description(), action,
+                                             error_desc);
   }
-  ASSERT(false && "unknown action type");
-  return WebRtcSession::kOffer;
+  return PushdownRemoteTransportDescription(remote_desc_->description(), action,
+                                            error_desc);
+}
+
+bool WebRtcSession::PushdownLocalTransportDescription(
+    const SessionDescription* sdesc,
+    cricket::ContentAction action,
+    std::string* err) {
+  RTC_DCHECK(signaling_thread()->IsCurrent());
+
+  if (!sdesc) {
+    return false;
+  }
+
+  for (const TransportInfo& tinfo : sdesc->transport_infos()) {
+    if (!transport_controller_->SetLocalTransportDescription(
+            tinfo.content_name, tinfo.description, action, err)) {
+      return false;
+    }
+  }
+
+  return true;
 }
 
-bool WebRtcSession::GetTransportStats(cricket::SessionStats* stats) {
+bool WebRtcSession::PushdownRemoteTransportDescription(
+    const SessionDescription* sdesc,
+    cricket::ContentAction action,
+    std::string* err) {
+  RTC_DCHECK(signaling_thread()->IsCurrent());
+
+  if (!sdesc) {
+    return false;
+  }
+
+  for (const TransportInfo& tinfo : sdesc->transport_infos()) {
+    if (!transport_controller_->SetRemoteTransportDescription(
+            tinfo.content_name, tinfo.description, action, err)) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
+bool WebRtcSession::GetTransportDescription(
+    const SessionDescription* description,
+    const std::string& content_name,
+    cricket::TransportDescription* tdesc) {
+  if (!description || !tdesc) {
+    return false;
+  }
+  const TransportInfo* transport_info =
+      description->GetTransportInfoByName(content_name);
+  if (!transport_info) {
+    return false;
+  }
+  *tdesc = transport_info->description;
+  return true;
+}
+
+bool WebRtcSession::GetTransportStats(SessionStats* stats) {
   ASSERT(signaling_thread()->IsCurrent());
   return (GetChannelTransportStats(voice_channel(), stats) &&
           GetChannelTransportStats(video_channel(), stats) &&
@@ -1081,7 +1150,7 @@ bool WebRtcSession::GetTransportStats(cricket::SessionStats* stats) {
 }
 
 bool WebRtcSession::GetChannelTransportStats(cricket::BaseChannel* ch,
-                                             cricket::SessionStats* stats) {
+                                             SessionStats* stats) {
   ASSERT(signaling_thread()->IsCurrent());
   if (!ch) {
     // Not using this channel.
@@ -1098,7 +1167,7 @@ bool WebRtcSession::GetChannelTransportStats(cricket::BaseChannel* ch,
   }
 
   cricket::TransportStats tstats;
-  if (!transport_controller()->GetStats(transport_name, &tstats)) {
+  if (!transport_controller_->GetStats(transport_name, &tstats)) {
     return false;
   }
 
@@ -1110,14 +1179,14 @@ bool WebRtcSession::GetLocalCertificate(
     const std::string& transport_name,
     rtc::scoped_refptr<rtc::RTCCertificate>* certificate) {
   ASSERT(signaling_thread()->IsCurrent());
-  return transport_controller()->GetLocalCertificate(transport_name,
-                                                     certificate);
+  return transport_controller_->GetLocalCertificate(transport_name,
+                                                    certificate);
 }
 
 bool WebRtcSession::GetRemoteSSLCertificate(const std::string& transport_name,
                                             rtc::SSLCertificate** cert) {
   ASSERT(signaling_thread()->IsCurrent());
-  return transport_controller()->GetRemoteSSLCertificate(transport_name, cert);
+  return transport_controller_->GetRemoteSSLCertificate(transport_name, cert);
 }
 
 cricket::BaseChannel* WebRtcSession::GetChannel(
@@ -1174,37 +1243,35 @@ bool WebRtcSession::EnableBundle(const cricket::ContentGroup& bundle) {
 }
 
 bool WebRtcSession::ProcessIceMessage(const IceCandidateInterface* candidate) {
-  if (state() == STATE_INIT) {
-     LOG(LS_ERROR) << "ProcessIceMessage: ICE candidates can't be added "
-                   << "without any offer (local or remote) "
-                   << "session description.";
+  if (!remote_desc_) {
+    LOG(LS_ERROR) << "ProcessIceMessage: ICE candidates can't be added "
+                  << "without any remote session description.";
      return false;
   }
 
   if (!candidate) {
-    LOG(LS_ERROR) << "ProcessIceMessage: Candidate is NULL";
+    LOG(LS_ERROR) << "ProcessIceMessage: Candidate is NULL.";
     return false;
   }
 
   bool valid = false;
-  if (!ReadyToUseRemoteCandidate(candidate, NULL, &valid)) {
-    if (valid) {
-      LOG(LS_INFO) << "ProcessIceMessage: Candidate saved";
-      saved_candidates_.push_back(
-          new JsepIceCandidate(candidate->sdp_mid(),
-                               candidate->sdp_mline_index(),
-                               candidate->candidate()));
-    }
-    return valid;
+  bool ready = ReadyToUseRemoteCandidate(candidate, NULL, &valid);
+  if (!valid) {
+    return false;
   }
 
   // Add this candidate to the remote session description.
   if (!remote_desc_->AddCandidate(candidate)) {
-    LOG(LS_ERROR) << "ProcessIceMessage: Candidate cannot be used";
+    LOG(LS_ERROR) << "ProcessIceMessage: Candidate cannot be used.";
     return false;
   }
 
-  return UseCandidate(candidate);
+  if (ready) {
+    return UseCandidate(candidate);
+  } else {
+    LOG(LS_INFO) << "ProcessIceMessage: Not ready to use candidate.";
+    return true;
+  }
 }
 
 bool WebRtcSession::SetIceTransports(
@@ -1213,18 +1280,37 @@ bool WebRtcSession::SetIceTransports(
         ConvertIceTransportTypeToCandidateFilter(type));
 }
 
+cricket::IceConfig WebRtcSession::ParseIceConfig(
+    const PeerConnectionInterface::RTCConfiguration& config) const {
+  cricket::IceConfig ice_config;
+  ice_config.receiving_timeout_ms = config.ice_connection_receiving_timeout;
+  ice_config.gather_continually = (config.continual_gathering_policy ==
+                                   PeerConnectionInterface::GATHER_CONTINUALLY);
+  return ice_config;
+}
+
+void WebRtcSession::SetIceConfig(const cricket::IceConfig& config) {
+  transport_controller_->SetIceConfig(config);
+}
+
+void WebRtcSession::MaybeStartGathering() {
+  transport_controller_->MaybeStartGathering();
+}
+
 bool WebRtcSession::GetLocalTrackIdBySsrc(uint32_t ssrc,
                                           std::string* track_id) {
-  if (!base_local_description())
+  if (!local_desc_) {
     return false;
-  return webrtc::GetTrackIdBySsrc(base_local_description(), ssrc, track_id);
+  }
+  return webrtc::GetTrackIdBySsrc(local_desc_->description(), ssrc, track_id);
 }
 
 bool WebRtcSession::GetRemoteTrackIdBySsrc(uint32_t ssrc,
                                            std::string* track_id) {
-  if (!base_remote_description())
+  if (!remote_desc_) {
     return false;
-  return webrtc::GetTrackIdBySsrc(base_remote_description(), ssrc, track_id);
+  }
+  return webrtc::GetTrackIdBySsrc(remote_desc_->description(), ssrc, track_id);
 }
 
 std::string WebRtcSession::BadStateErrMsg(State state) {
@@ -1342,7 +1428,8 @@ bool WebRtcSession::CanInsertDtmf(const std::string& track_id) {
   uint32_t send_ssrc = 0;
   // The Dtmf is negotiated per channel not ssrc, so we only check if the ssrc
   // exists.
-  if (!GetAudioSsrcByTrackId(base_local_description(), track_id,
+  if (!local_desc_ ||
+      !GetAudioSsrcByTrackId(local_desc_->description(), track_id,
                              &send_ssrc)) {
     LOG(LS_ERROR) << "CanInsertDtmf: Track does not exist: " << track_id;
     return false;
@@ -1358,8 +1445,8 @@ bool WebRtcSession::InsertDtmf(const std::string& track_id,
     return false;
   }
   uint32_t send_ssrc = 0;
-  if (!VERIFY(GetAudioSsrcByTrackId(base_local_description(),
-                                    track_id, &send_ssrc))) {
+  if (!VERIFY(local_desc_ && GetAudioSsrcByTrackId(local_desc_->description(),
+                                                   track_id, &send_ssrc))) {
     LOG(LS_ERROR) << "InsertDtmf: Track does not exist: " << track_id;
     return false;
   }
@@ -1434,7 +1521,7 @@ bool WebRtcSession::ReadyToSendData() const {
 rtc::scoped_refptr<DataChannel> WebRtcSession::CreateDataChannel(
     const std::string& label,
     const InternalDataChannelInit* config) {
-  if (state() == STATE_RECEIVEDTERMINATE) {
+  if (state() == STATE_TERMINATED) {
     return NULL;
   }
   if (data_channel_type_ == cricket::DCT_NONE) {
@@ -1480,7 +1567,7 @@ void WebRtcSession::ResetIceRestartLatch() {
 
 void WebRtcSession::OnCertificateReady(
     const rtc::scoped_refptr<rtc::RTCCertificate>& certificate) {
-  transport_controller()->SetLocalCertificate(certificate);
+  transport_controller_->SetLocalCertificate(certificate);
 }
 
 bool WebRtcSession::waiting_for_certificate_for_testing() const {
@@ -1489,7 +1576,7 @@ bool WebRtcSession::waiting_for_certificate_for_testing() const {
 
 const rtc::scoped_refptr<rtc::RTCCertificate>&
 WebRtcSession::certificate_for_testing() {
-  return transport_controller()->certificate_for_testing();
+  return transport_controller_->certificate_for_testing();
 }
 
 void WebRtcSession::SetIceConnectionState(
@@ -1642,11 +1729,12 @@ void WebRtcSession::EnableChannels() {
 // Returns the media index for a local ice candidate given the content name.
 bool WebRtcSession::GetLocalCandidateMediaIndex(const std::string& content_name,
                                                 int* sdp_mline_index) {
-  if (!base_local_description() || !sdp_mline_index)
+  if (!local_desc_ || !sdp_mline_index) {
     return false;
+  }
 
   bool content_found = false;
-  const ContentInfos& contents = base_local_description()->contents();
+  const ContentInfos& contents = local_desc_->description()->contents();
   for (size_t index = 0; index < contents.size(); ++index) {
     if (contents[index].name == content_name) {
       *sdp_mline_index = static_cast<int>(index);
@@ -1659,28 +1747,27 @@ bool WebRtcSession::GetLocalCandidateMediaIndex(const std::string& content_name,
 
 bool WebRtcSession::UseCandidatesInSessionDescription(
     const SessionDescriptionInterface* remote_desc) {
-  if (!remote_desc)
+  if (!remote_desc) {
     return true;
+  }
   bool ret = true;
 
   for (size_t m = 0; m < remote_desc->number_of_mediasections(); ++m) {
     const IceCandidateCollection* candidates = remote_desc->candidates(m);
-    for  (size_t n = 0; n < candidates->count(); ++n) {
+    for (size_t n = 0; n < candidates->count(); ++n) {
       const IceCandidateInterface* candidate = candidates->at(n);
       bool valid = false;
       if (!ReadyToUseRemoteCandidate(candidate, remote_desc, &valid)) {
         if (valid) {
-          LOG(LS_INFO) << "UseCandidatesInSessionDescription: Candidate saved.";
-          saved_candidates_.push_back(
-              new JsepIceCandidate(candidate->sdp_mid(),
-                                   candidate->sdp_mline_index(),
-                                   candidate->candidate()));
+          LOG(LS_INFO) << "UseCandidatesInSessionDescription: Not ready to use "
+                       << "candidate.";
         }
         continue;
       }
       ret = UseCandidate(candidate);
-      if (!ret)
+      if (!ret) {
         break;
+      }
     }
   }
   return ret;
@@ -1690,7 +1777,7 @@ bool WebRtcSession::UseCandidate(
     const IceCandidateInterface* candidate) {

[pthatcher1, 2015/10/10 00:07:27]

In some of these places where you assume the remote_desc_ is non-null or the
local_desc_ is non-null, can you add an RTC_CHECK(remote_desc_) or
RTC_CHECK(local_desc_)?

I know the logic makes sense, but it's easy for logic to change, and an
RTC_CHECK is better than a null pointer deref.

[Taylor Brandstetter, 2015/10/10 00:30:15]

In an earlier code review, tommi told me that RTC_CHECK'ing something that's
going to be dereferenced anyway is discouraged. So... who's right?

[pthatcher1, 2015/10/10 01:59:20]

I'll defer to Tommi on this one.

 
   size_t mediacontent_index = static_cast<size_t>(candidate->sdp_mline_index());
-  size_t remote_content_size = base_remote_description()->contents().size();
+  size_t remote_content_size = remote_desc_->description()->contents().size();
   if (mediacontent_index >= remote_content_size) {
     LOG(LS_ERROR)
         << "UseRemoteCandidateInSession: Invalid candidate media index.";
@@ -1698,13 +1785,13 @@ bool WebRtcSession::UseCandidate(
   }
 
   cricket::ContentInfo content =
-      base_remote_description()->contents()[mediacontent_index];
+      remote_desc_->description()->contents()[mediacontent_index];
   std::vector<cricket::Candidate> candidates;
   candidates.push_back(candidate->candidate());
   // Invoking BaseSession method to handle remote candidates.
   std::string error;
-  if (transport_controller()->AddRemoteCandidates(content.name, candidates,
-                                                  &error)) {
+  if (transport_controller_->AddRemoteCandidates(content.name, candidates,
+                                                 &error)) {
     // Candidates successfully submitted for checking.
     if (ice_connection_state_ == PeerConnectionInterface::kIceConnectionNew ||
         ice_connection_state_ ==
@@ -1819,7 +1906,7 @@ bool WebRtcSession::CreateChannels(const SessionDescription* desc) {
 
 bool WebRtcSession::CreateVoiceChannel(const cricket::ContentInfo* content) {
   voice_channel_.reset(channel_manager_->CreateVoiceChannel(
-      media_controller_.get(), transport_controller(), content->name, true,
+      media_controller_.get(), transport_controller_.get(), content->name, true,
       audio_options_));
   if (!voice_channel_) {
     return false;
@@ -1832,7 +1919,7 @@ bool WebRtcSession::CreateVoiceChannel(const cricket::ContentInfo* content) {
 
 bool WebRtcSession::CreateVideoChannel(const cricket::ContentInfo* content) {
   video_channel_.reset(channel_manager_->CreateVideoChannel(
-      media_controller_.get(), transport_controller(), content->name, true,
+      media_controller_.get(), transport_controller_.get(), content->name, true,
       video_options_));
   if (!video_channel_) {
     return false;
@@ -1846,7 +1933,7 @@ bool WebRtcSession::CreateVideoChannel(const cricket::ContentInfo* content) {
 bool WebRtcSession::CreateDataChannel(const cricket::ContentInfo* content) {
   bool sctp = (data_channel_type_ == cricket::DCT_SCTP);
   data_channel_.reset(channel_manager_->CreateDataChannel(
-      transport_controller(), content->name, !sctp, data_channel_type_));
+      transport_controller_.get(), content->name, !sctp, data_channel_type_));
   if (!data_channel_) {
     return false;
   }
@@ -1866,21 +1953,8 @@ bool WebRtcSession::CreateDataChannel(const cricket::ContentInfo* content) {
 }
 
 void WebRtcSession::OnDtlsSetupFailure(cricket::BaseChannel*, bool rtcp) {
-  SetError(BaseSession::ERROR_TRANSPORT, rtcp ? kDtlsSetupFailureRtcp :
-           kDtlsSetupFailureRtp);
-}
-
-void WebRtcSession::CopySavedCandidates(
-    SessionDescriptionInterface* dest_desc) {
-  if (!dest_desc) {
-    ASSERT(false);
-    return;
-  }
-  for (size_t i = 0; i < saved_candidates_.size(); ++i) {
-    dest_desc->AddCandidate(saved_candidates_[i]);
-    delete saved_candidates_[i];
-  }
-  saved_candidates_.clear();
+  SetError(ERROR_TRANSPORT,
+           rtcp ? kDtlsSetupFailureRtcp : kDtlsSetupFailureRtp);
 }
 
 void WebRtcSession::OnDataChannelMessageReceived(
@@ -1932,7 +2006,7 @@ bool WebRtcSession::ValidateSessionDescription(
     const SessionDescriptionInterface* sdesc,
     cricket::ContentSource source, std::string* err_desc) {
   std::string type;
-  if (error() != cricket::BaseSession::ERROR_NONE) {
+  if (error() != ERROR_NONE) {
     return BadSdp(source, type, GetSessionErrorMsg(), err_desc);
   }
 
@@ -1970,8 +2044,8 @@ bool WebRtcSession::ValidateSessionDescription(
   // Verify m-lines in Answer when compared against Offer.
   if (action == kAnswer) {
     const cricket::SessionDescription* offer_desc =
-        (source == cricket::CS_LOCAL) ? remote_description()->description() :
-            local_description()->description();
+        (source == cricket::CS_LOCAL) ? remote_desc_->description()
+                                      : local_desc_->description();
     if (!VerifyMediaDescriptions(sdesc->description(), offer_desc)) {
       return BadAnswerSdp(source, kMlineMismatch, err_desc);
     }
@@ -1983,31 +2057,27 @@ bool WebRtcSession::ValidateSessionDescription(
 bool WebRtcSession::ExpectSetLocalDescription(Action action) {
   return ((action == kOffer && state() == STATE_INIT) ||
           // update local offer
-          (action == kOffer && state() == STATE_SENTINITIATE) ||
+          (action == kOffer && state() == STATE_SENTOFFER) ||
           // update the current ongoing session.
-          (action == kOffer && state() == STATE_RECEIVEDACCEPT) ||
-          (action == kOffer && state() == STATE_SENTACCEPT) ||
           (action == kOffer && state() == STATE_INPROGRESS) ||
           // accept remote offer
-          (action == kAnswer && state() == STATE_RECEIVEDINITIATE) ||
-          (action == kAnswer && state() == STATE_SENTPRACCEPT) ||
-          (action == kPrAnswer && state() == STATE_RECEIVEDINITIATE) ||
-          (action == kPrAnswer && state() == STATE_SENTPRACCEPT));
+          (action == kAnswer && state() == STATE_RECEIVEDOFFER) ||
+          (action == kAnswer && state() == STATE_SENTPRANSWER) ||
+          (action == kPrAnswer && state() == STATE_RECEIVEDOFFER) ||
+          (action == kPrAnswer && state() == STATE_SENTPRANSWER));
 }
 
 bool WebRtcSession::ExpectSetRemoteDescription(Action action) {
   return ((action == kOffer && state() == STATE_INIT) ||
           // update remote offer
-          (action == kOffer && state() == STATE_RECEIVEDINITIATE) ||
+          (action == kOffer && state() == STATE_RECEIVEDOFFER) ||
           // update the current ongoing session
-          (action == kOffer && state() == STATE_RECEIVEDACCEPT) ||
-          (action == kOffer && state() == STATE_SENTACCEPT) ||
           (action == kOffer && state() == STATE_INPROGRESS) ||
           // accept local offer
-          (action == kAnswer && state() == STATE_SENTINITIATE) ||
-          (action == kAnswer && state() == STATE_RECEIVEDPRACCEPT) ||
-          (action == kPrAnswer && state() == STATE_SENTINITIATE) ||
-          (action == kPrAnswer && state() == STATE_RECEIVEDPRACCEPT));
+          (action == kAnswer && state() == STATE_SENTOFFER) ||
+          (action == kAnswer && state() == STATE_RECEIVEDPRANSWER) ||
+          (action == kPrAnswer && state() == STATE_SENTOFFER) ||
+          (action == kPrAnswer && state() == STATE_RECEIVEDPRANSWER));
 }
 
 std::string WebRtcSession::GetSessionErrorMsg() {
@@ -2029,10 +2099,11 @@ bool WebRtcSession::ReadyToUseRemoteCandidate(
   *valid = true;;
 
   const SessionDescriptionInterface* current_remote_desc =
-      remote_desc ? remote_desc : remote_description();
+      remote_desc ? remote_desc : remote_desc_.get();
 
-  if (!current_remote_desc)
+  if (!current_remote_desc) {
     return false;
+  }
 
   size_t mediacontent_index =
       static_cast<size_t>(candidate->sdp_mline_index());
@@ -2053,7 +2124,7 @@ bool WebRtcSession::ReadyToUseRemoteCandidate(
     return false;
   }
 
-  return transport_controller()->ReadyForRemoteCandidates(
+  return transport_controller_->ReadyForRemoteCandidates(
       channel->transport_name());
 }
 
@@ -2089,7 +2160,7 @@ void WebRtcSession::ReportTransportStats() {
   }
   for (const auto& name : transport_names) {
     cricket::TransportStats stats;
-    if (transport_controller()->GetStats(name, &stats)) {
+    if (transport_controller_->GetStats(name, &stats)) {
       ReportBestConnectionState(stats);
       ReportNegotiatedCiphers(stats);
     }