TransportController refactoring

talk/app/webrtc/webrtcsession.cc

 /*
  * libjingle
  * Copyright 2012 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 13 matching lines @@
  * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
  * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "talk/app/webrtc/webrtcsession.h"
 
 #include <limits.h>
 
 #include <algorithm>
 #include <vector>
+#include <set>
 
 #include "talk/app/webrtc/jsepicecandidate.h"
 #include "talk/app/webrtc/jsepsessiondescription.h"
 #include "talk/app/webrtc/mediaconstraintsinterface.h"
 #include "talk/app/webrtc/mediastreamsignaling.h"
 #include "talk/app/webrtc/peerconnectioninterface.h"
 #include "talk/app/webrtc/webrtcsessiondescriptionfactory.h"
 #include "talk/media/base/constants.h"
 #include "talk/media/base/videocapturer.h"
 #include "talk/session/media/channel.h"
@@ skipping 30 matching lines @@
 const char kSdpWithoutSdesCrypto[] =
     "Called with SDP without SDES crypto.";
 const char kSdpWithoutIceUfragPwd[] =
     "Called with SDP without ice-ufrag and ice-pwd.";
 const char kSessionError[] = "Session error code: ";
 const char kSessionErrorDesc[] = "Session error description: ";
 const char kDtlsSetupFailureRtp[] =
     "Couldn't set up DTLS-SRTP on RTP channel.";
 const char kDtlsSetupFailureRtcp[] =
     "Couldn't set up DTLS-SRTP on RTCP channel.";
+const char kEnableBundleFailed[] = "Failed to enable BUNDLE.";
 const int kMaxUnsignalledRecvStreams = 20;
 
 // Compares |answer| against |offer|. Comparision is done
 // for number of m-lines in answer against offer. If matches true will be
 // returned otherwise false.
 static bool VerifyMediaDescriptions(
     const SessionDescription* answer, const SessionDescription* offer) {
   if (offer->contents().size() != answer->contents().size())
     return false;
 
@@ skipping 380 matching lines @@
 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),
-                           cricket::NS_JINGLE_RTP,
                            false),
       // 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.
       channel_manager_(channel_manager),
       mediastream_signaling_(mediastream_signaling),
       ice_observer_(NULL),
       ice_connection_state_(PeerConnectionInterface::kIceConnectionNew),
       ice_connection_receiving_(true),
       older_version_remote_peer_(false),
       dtls_enabled_(false),
       data_channel_type_(cricket::DCT_NONE),
       ice_restart_latch_(new IceRestartAnswerLatch),
       metrics_observer_(NULL) {
+  transport_controller()->SignalConnectionStateChanged.connect(
+      this, &WebRtcSession::OnTransportControllerConnectionStateChanged);
+  transport_controller()->SignalReceiving.connect(
+      this, &WebRtcSession::OnTransportControllerReceiving);
+  transport_controller()->SignalCandidatesAllocationStarted.connect(
+      this, &WebRtcSession::OnTransportControllerCandidatesAllocationStarted);
+  transport_controller()->SignalCandidatesAllocationDone.connect(
+      this, &WebRtcSession::OnTransportControllerCandidatesAllocationDone);
+  transport_controller()->SignalCandidatesReady.connect(
+      this, &WebRtcSession::OnTransportControllerCandidatesReady);
 }
 
 WebRtcSession::~WebRtcSession() {
   ASSERT(signaling_thread()->IsCurrent());
   // Destroy video_channel_ first since it may have a pointer to the
   // voice_channel_.
   if (video_channel_) {
     SignalVideoChannelDestroyed();
     channel_manager_->DestroyVideoChannel(video_channel_.release());
   }
   if (voice_channel_) {
     SignalVoiceChannelDestroyed();
     channel_manager_->DestroyVoiceChannel(voice_channel_.release(), nullptr);
   }
   if (data_channel_) {
     SignalDataChannelDestroyed();
     channel_manager_->DestroyDataChannel(data_channel_.release());
   }
   for (size_t i = 0; i < saved_candidates_.size(); ++i) {
     delete saved_candidates_[i];
   }
-  delete identity();
 }
 
 bool WebRtcSession::Initialize(
     const PeerConnectionFactoryInterface::Options& options,
     const MediaConstraintsInterface*  constraints,
     DTLSIdentityServiceInterface* dtls_identity_service,
     const PeerConnectionInterface::RTCConfiguration& rtc_configuration) {
   bundle_policy_ = rtc_configuration.bundle_policy;
   rtcp_mux_policy_ = rtc_configuration.rtcp_mux_policy;
-  SetSslMaxProtocolVersion(options.ssl_max_version);
+  transport_controller()->SetSslMaxProtocolVersion(options.ssl_max_version);
 
   // TODO(perkj): Take |constraints| into consideration. Return false if not all
   // mandatory constraints can be fulfilled. Note that |constraints|
   // can be null.
   bool value;
 
   if (options.disable_encryption) {
     dtls_enabled_ = false;
   } else {
     // Enable DTLS by default if |dtls_identity_service| is valid.
@@ skipping 135 matching lines @@
   if (options.disable_encryption) {
     webrtc_session_desc_factory_->SetSdesPolicy(cricket::SEC_DISABLED);
   }
   port_allocator()->set_candidate_filter(
       ConvertIceTransportTypeToCandidateFilter(rtc_configuration.type));
   return true;
 }
 
 void WebRtcSession::Terminate() {
   SetState(STATE_RECEIVEDTERMINATE);
-  RemoveUnusedChannelsAndTransports(NULL);
+  RemoveUnusedChannels(NULL);
   ASSERT(!voice_channel_);
   ASSERT(!video_channel_);
   ASSERT(!data_channel_);
 }
 
 bool WebRtcSession::StartCandidatesAllocation() {

[pthatcher1, 2015/08/10 20:40:16]

And while we're in here, we might as well call this StartGatheringCandidates.

[Taylor Brandstetter, 2015/08/11 01:20:07]

Done.

-  // SpeculativelyConnectTransportChannels, will call ConnectChannels method
-  // from TransportProxy to start gathering ice candidates.
-  SpeculativelyConnectAllTransportChannels();
   if (!saved_candidates_.empty()) {
     // If there are saved candidates which arrived before local description is
     // set, copy those to remote description.
     CopySavedCandidates(remote_desc_.get());
   }
   // Push remote candidates present in remote description to transport channels.
   UseCandidatesInSessionDescription(remote_desc_.get());
   return true;
 }
 
 void WebRtcSession::SetSdesPolicy(cricket::SecurePolicy secure_policy) {
   webrtc_session_desc_factory_->SetSdesPolicy(secure_policy);
 }
 
 cricket::SecurePolicy WebRtcSession::SdesPolicy() const {
   return webrtc_session_desc_factory_->SdesPolicy();
 }
 
 bool WebRtcSession::GetSslRole(rtc::SSLRole* role) {
   if (local_description() == NULL || remote_description() == NULL) {
     LOG(LS_INFO) << "Local and Remote descriptions must be applied to get "
                  << "SSL Role of the session.";
     return false;
   }
 
-  // TODO(mallinath) - Return role of each transport, as role may differ from
-  // one another.
-  // In current implementaion we just return the role of first transport in the
-  // transport map.
-  for (cricket::TransportMap::const_iterator iter = transport_proxies().begin();
-       iter != transport_proxies().end(); ++iter) {
-    if (iter->second->impl()) {
-      return iter->second->impl()->GetSslRole(role);
-    }
-  }
-  return false;
+  return transport_controller()->GetSslRole(role);

[pthatcher1, 2015/08/10 20:40:16]

We should probably still leave the TODO about the different transports possibly
having different roles.

[Taylor Brandstetter, 2015/08/11 01:20:07]

I left it, it just moved to transportcontroller.cc.

 }
 
 void WebRtcSession::CreateOffer(
     CreateSessionDescriptionObserver* observer,
     const PeerConnectionInterface::RTCOfferAnswerOptions& options) {
   webrtc_session_desc_factory_->CreateOffer(observer, options);
 }
 
 void WebRtcSession::CreateAnswer(CreateSessionDescriptionObserver* observer,
                                  const MediaConstraintsInterface* constraints) {
   webrtc_session_desc_factory_->CreateAnswer(observer, constraints);
 }
 
 bool WebRtcSession::SetLocalDescription(SessionDescriptionInterface* desc,
                                         std::string* err_desc) {
+  ASSERT(signaling_thread()->IsCurrent());
+
   // Takes the ownership of |desc| regardless of the result.
   rtc::scoped_ptr<SessionDescriptionInterface> desc_temp(desc);
 
   // Validate SDP.
   if (!ValidateSessionDescription(desc, cricket::CS_LOCAL, err_desc)) {
     return false;
   }
 
   // Update the initiator flag if this session is the initiator.
   Action action = GetAction(desc->type());
@@ skipping 12 matching lines @@
   set_local_description(desc->description()->Copy());
   local_desc_.reset(desc_temp.release());
 
   // Transport and Media channels will be created only when offer is set.
   if (action == kOffer && !CreateChannels(local_desc_->description())) {
     // TODO(mallinath) - Handle CreateChannel failure, as new local description
     // is applied. Restore back to old description.
     return BadLocalSdp(desc->type(), kCreateChannelFailed, err_desc);
   }
 
-  // Remove channel and transport proxies, if MediaContentDescription is
-  // rejected.
-  RemoveUnusedChannelsAndTransports(local_desc_->description());
+  // Enable BUNDLE before when kMaxBundle policy is in effect
+  if (bundle_policy_ == PeerConnectionInterface::kBundlePolicyMaxBundle) {
+    const cricket::ContentGroup* local_bundle =
+        BaseSession::local_description()->GetGroupByName(
+            cricket::GROUP_TYPE_BUNDLE);
+    if (!local_bundle) {
+      LOG(LS_WARNING) << "max-bundle specified without BUNDLE specified";
+      return false;
+    }
+    if (!EnableBundle(*local_bundle)) {
+      LOG(LS_WARNING) << "max-bundle failed to enable bundling.";
+      return false;
+    }
+  }
+
+  // Remove transport channels if MediaContentDescription is rejected.
+  RemoveUnusedChannels(local_desc_->description());
 
   if (!UpdateSessionState(action, cricket::CS_LOCAL, err_desc)) {
     return false;
   }
 
   // Kick starting the ice candidates allocation.
   StartCandidatesAllocation();
 
   // Update state and SSRC of local MediaStreams and DataChannels based on the
   // local session description.
   mediastream_signaling_->OnLocalDescriptionChanged(local_desc_.get());
 
   rtc::SSLRole role;
   if (data_channel_type_ == cricket::DCT_SCTP && GetSslRole(&role)) {
     mediastream_signaling_->OnDtlsRoleReadyForSctp(role);
   }
   if (error() != cricket::BaseSession::ERROR_NONE) {
     return BadLocalSdp(desc->type(), GetSessionErrorMsg(), err_desc);
   }
   return true;
 }
 
 bool WebRtcSession::SetRemoteDescription(SessionDescriptionInterface* desc,
                                          std::string* err_desc) {
+  ASSERT(signaling_thread()->IsCurrent());
+
   // Takes the ownership of |desc| regardless of the result.
   rtc::scoped_ptr<SessionDescriptionInterface> desc_temp(desc);
 
   // Validate SDP.
   if (!ValidateSessionDescription(desc, cricket::CS_REMOTE, err_desc)) {
     return false;
   }
 
   // Transport and Media channels will be created only when offer is set.
   Action action = GetAction(desc->type());
   if (action == kOffer && !CreateChannels(desc->description())) {
     // TODO(mallinath) - Handle CreateChannel failure, as new local description
     // is applied. Restore back to old description.
     return BadRemoteSdp(desc->type(), kCreateChannelFailed, err_desc);
   }
 
-  // Remove channel and transport proxies, if MediaContentDescription is
-  // rejected.
-  RemoveUnusedChannelsAndTransports(desc->description());
+  // Remove transport channels if MediaContentDescription is rejected.

[pthatcher1, 2015/08/10 20:40:16]

I think you mean Remove channels, not transport channels.

[Taylor Brandstetter, 2015/08/11 01:20:07]

Done.

+  RemoveUnusedChannels(desc->description());
 
   // 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);
@@ skipping 38 matching lines @@
   if (desc->type() != SessionDescriptionInterface::kOffer &&
       ice_connection_state_ == PeerConnectionInterface::kIceConnectionNew) {
     SetIceConnectionState(PeerConnectionInterface::kIceConnectionChecking);
   }
   return true;
 }
 
 bool WebRtcSession::UpdateSessionState(
     Action action, cricket::ContentSource source,
     std::string* err_desc) {
+  ASSERT(signaling_thread()->IsCurrent());
+
   // 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);
   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);
@@ skipping 13 matching lines @@
     if (!PushdownMediaDescription(cricket::CA_PRANSWER, source, err_desc)) {
       SetError(BaseSession::ERROR_CONTENT, *err_desc);
     }
     if (error() != cricket::BaseSession::ERROR_NONE) {
       return BadPranswerSdp(source, GetSessionErrorMsg(), err_desc);
     }
   } else if (action == kAnswer) {
     if (!PushdownTransportDescription(source, cricket::CA_ANSWER, &td_err)) {
       return BadAnswerSdp(source, MakeTdErrorString(td_err), err_desc);
     }
-    MaybeEnableMuxingSupport();
+    const cricket::ContentGroup* local_bundle =
+        BaseSession::local_description()->GetGroupByName(
+            cricket::GROUP_TYPE_BUNDLE);
+    const cricket::ContentGroup* remote_bundle =
+        BaseSession::remote_description()->GetGroupByName(
+            cricket::GROUP_TYPE_BUNDLE);
+    if (local_bundle && remote_bundle) {
+      // The answerer decides the transport to bundle on
+      const cricket::ContentGroup* answer_bundle = (
+          source == cricket::CS_LOCAL ? local_bundle : remote_bundle);
+      if (!EnableBundle(*answer_bundle)) {
+        LOG(LS_WARNING) << "Failed to enable BUNDLE.";
+        return BadAnswerSdp(source, kEnableBundleFailed, err_desc);
+      }
+    }
     EnableChannels();
     SetState(source == cricket::CS_LOCAL ?
         STATE_SENTACCEPT : STATE_RECEIVEDACCEPT);
     if (!PushdownMediaDescription(cricket::CA_ANSWER, source, err_desc)) {
       SetError(BaseSession::ERROR_CONTENT, *err_desc);
     }
     if (error() != cricket::BaseSession::ERROR_NONE) {
       return BadAnswerSdp(source, GetSessionErrorMsg(), err_desc);
     }
   }
@@ skipping 28 matching lines @@
     return WebRtcSession::kPrAnswer;
   } else if (type == SessionDescriptionInterface::kAnswer) {
     return WebRtcSession::kAnswer;
   }
   ASSERT(false && "unknown action type");
   return WebRtcSession::kOffer;
 }
 
 bool WebRtcSession::GetTransportStats(cricket::SessionStats* stats) {
   ASSERT(signaling_thread()->IsCurrent());
+  return (GetChannelTransportStats(voice_channel(), stats) &&
+          GetChannelTransportStats(video_channel(), stats) &&
+          GetChannelTransportStats(data_channel(), stats));
+}
 
-  const auto get_transport_stats = [stats](const std::string& content_name,
-                                           cricket::Transport* transport) {
-    const std::string& transport_id = transport->content_name();
-    stats->proxy_to_transport[content_name] = transport_id;
-    if (stats->transport_stats.find(transport_id)
-        != stats->transport_stats.end()) {
-      // Transport stats already done for this transport.
+bool WebRtcSession::GetChannelTransportStats(cricket::BaseChannel* ch,
+                                             cricket::SessionStats* stats) {
+  ASSERT(signaling_thread()->IsCurrent());
+  if (!ch) {
+    // Not using this channel.
+    return true;
+  }
+
+  const std::string& content_name = ch->content_name();
+  const std::string& transport_name = ch->transport_name();
+  stats->proxy_to_transport[content_name] = transport_name;
+  if (stats->transport_stats.find(transport_name)
+      != stats->transport_stats.end()) {
+    // Transport stats already done for this transport.
+    return true;
+  }
+
+  cricket::TransportStats tstats;
+  if (!transport_controller()->GetStats(transport_name, &tstats)) {
+    return false;
+  }
+
+  stats->transport_stats[transport_name] = tstats;
+  return true;
+}
+
+bool WebRtcSession::GetIdentity(const std::string& content_name,
+                                rtc::SSLIdentity** identity) {
+  ASSERT(signaling_thread()->IsCurrent());
+
+  cricket::BaseChannel* ch = GetChannel(content_name);
+  if (!ch) {
+    return false;
+  }
+
+  return transport_controller()->GetIdentity(ch->transport_name(), identity);
+}
+
+bool WebRtcSession::GetRemoteCertificate(const std::string& content_name,
+                                         rtc::SSLCertificate** cert) {
+  ASSERT(signaling_thread()->IsCurrent());
+
+  cricket::BaseChannel* ch = GetChannel(content_name);
+  if (!ch) {
+    return false;
+  }
+
+  return transport_controller()->GetRemoteCertificate(ch->transport_name(),
+                                                      cert);
+}
+
+cricket::BaseChannel* WebRtcSession::GetChannel(
+    const std::string& content_name) {
+  if (voice_channel() && voice_channel()->content_name() == content_name) {
+    return voice_channel();
+  }
+  if (video_channel() && video_channel()->content_name() == content_name) {
+    return video_channel();
+  }
+  if (data_channel() && data_channel()->content_name() == content_name) {
+    return data_channel();
+  }
+  return nullptr;
+}
+
+bool WebRtcSession::EnableBundle(const cricket::ContentGroup& bundle) {
+  const std::string* first_content_name = bundle.FirstContentName();
+  if (!first_content_name) {
+    LOG(LS_WARNING) << "Tried to BUNDLE with no contents.";
+    return false;
+  }
+  const std::string& transport_name = *first_content_name;
+  cricket::BaseChannel* first_channel = GetChannel(transport_name);
+
+  auto maybe_set_transport = [this, bundle, transport_name, first_channel](
+      cricket::BaseChannel* ch) {
+    if (!ch || !bundle.HasContentName(ch->content_name())) {
       return true;
     }
 
-    cricket::TransportStats tstats;
-    if (!transport->GetStats(&tstats)) {
+    if (ch->transport_name() == transport_name) {
+      LOG(LS_INFO) << "BUNDLE already enabled for " << ch->content_name()
+                   << " on " << transport_name << ".";
+      return true;
+    }
+
+    if (!ch->SetTransport(transport_name)) {
+      LOG(LS_WARNING) << "Failed to enable BUNDLE for " << ch->content_name();
       return false;
     }
-
-    stats->transport_stats[transport_id] = tstats;
+    LOG(LS_INFO) << "Enabled BUNDLE for " << ch->content_name()
+                 << " on " << transport_name << ".";
     return true;
   };
 
-  for (const auto& kv : transport_proxies()) {
-    cricket::Transport* transport = kv.second->impl();
-    if (transport && !get_transport_stats(kv.first, transport)) {
-      return false;
-    }
+  if (!maybe_set_transport(voice_channel()) ||
+      !maybe_set_transport(video_channel()) ||
+      !maybe_set_transport(data_channel())) {
+    return false;
   }
+
   return true;
 }
 
 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.";
      return false;
   }
@@ skipping 297 matching lines @@
 
 bool WebRtcSession::IceRestartPending() const {
   return ice_restart_latch_->Get();
 }
 
 void WebRtcSession::ResetIceRestartLatch() {
   ice_restart_latch_->Reset();
 }
 
 void WebRtcSession::OnIdentityReady(rtc::SSLIdentity* identity) {
-  SetIdentity(identity);
+  transport_controller()->SetIdentity(identity);
 }
 
 bool WebRtcSession::waiting_for_identity() const {
   return webrtc_session_desc_factory_->waiting_for_identity();
 }
 
 void WebRtcSession::SetIceConnectionState(
       PeerConnectionInterface::IceConnectionState state) {
   if (ice_connection_state_ == state) {
     return;
   }
 
   // ASSERT that the requested transition is allowed.  Note that
   // WebRtcSession does not implement "kIceConnectionClosed" (that is handled
   // within PeerConnection).  This switch statement should compile away when
   // ASSERTs are disabled.
+  LOG(LS_INFO) << "Changing IceConnectionState " << ice_connection_state_
+               << " => " << state;
   switch (ice_connection_state_) {
     case PeerConnectionInterface::kIceConnectionNew:
       ASSERT(state == PeerConnectionInterface::kIceConnectionChecking);
       break;
     case PeerConnectionInterface::kIceConnectionChecking:
       ASSERT(state == PeerConnectionInterface::kIceConnectionFailed ||
              state == PeerConnectionInterface::kIceConnectionConnected);
       break;
     case PeerConnectionInterface::kIceConnectionConnected:
       ASSERT(state == PeerConnectionInterface::kIceConnectionDisconnected ||
@@ skipping 20 matching lines @@
       ASSERT(false);
       break;
   }
 
   ice_connection_state_ = state;
   if (ice_observer_) {
     ice_observer_->OnIceConnectionChange(ice_connection_state_);
   }
 }
 
-void WebRtcSession::OnTransportRequestSignaling(
-    cricket::Transport* transport) {
-  ASSERT(signaling_thread()->IsCurrent());
-  transport->OnSignalingReady();
-  if (ice_observer_) {
-    ice_observer_->OnIceGatheringChange(
-      PeerConnectionInterface::kIceGatheringGathering);
+void WebRtcSession::OnTransportControllerConnectionStateChanged(
+    cricket::TransportController::ConnectionState state) {

[pthatcher1, 2015/08/10 20:40:16]

It seems like this enum type should still be a TransportChannel ConnectionState,
with the understanding that the TransportController is aggregating them.  Why do
we have the enum on TransportController?

[Taylor Brandstetter, 2015/08/11 01:20:07]

Because it has a slightly different meaning than the state in TransportChannel,
and has an additional value ("Connected"). However, I at least moved it to the
"cricket" scope in transport.h, along with the gathering state. Note that once
the "talk" and "webrtc" trees are combined, we can probably just get rid of
those enums and use the ones in PeerConnectionInterface.

+  switch (state) {
+    case cricket::TransportController::kConnecting:
+      // If the current state is Connected or Completed, then there were
+      // writable channels but now there are not, so the next state must
+      // be Disconnected.
+      if (ice_connection_state_ ==
+              PeerConnectionInterface::kIceConnectionConnected ||
+          ice_connection_state_ ==
+              PeerConnectionInterface::kIceConnectionCompleted) {
+        SetIceConnectionState(
+            PeerConnectionInterface::kIceConnectionDisconnected);

[pthatcher1, 2015/08/10 20:40:16]

Should it not go to checking, or maybe disconnected then checking?

[Taylor Brandstetter, 2015/08/11 01:20:07]

This is the behavior we had before so I kept it. And it seems to match the W3C
spec.

+      }
+      break;
+    case cricket::TransportController::kFailed:
+      SetIceConnectionState(PeerConnectionInterface::kIceConnectionFailed);
+      break;
+    case cricket::TransportController::kConnected:
+      LOG(LS_INFO) << "Changing to ICE connected state because "
+                   << "all transports are writable.";
+      SetIceConnectionState(PeerConnectionInterface::kIceConnectionConnected);
+      break;
+    case cricket::TransportController::kCompleted:
+      LOG(LS_INFO) << "Changing to ICE completed state because "
+                   << "all transports are complete.";
+      if (ice_connection_state_ !=
+          PeerConnectionInterface::kIceConnectionConnected) {
+        // If jumping directly from "checking" to "connected",
+        // signal "connected" first.
+        SetIceConnectionState(PeerConnectionInterface::kIceConnectionConnected);
+      }
+      SetIceConnectionState(PeerConnectionInterface::kIceConnectionCompleted);
+      if (metrics_observer_) {
+        // Once ICE connection is completed, report stats for all transports
+        // in use.
+        std::set<std::string> transport_names;

[pthatcher1, 2015/08/10 20:40:16]

Could just be a vector instead of a set.

Actually, this pattern comes up so often in the code, can we just add a method
called channels() that returns all the non-null channels so that we can do
something like this?

for (BaseChannel* channel: channels()) {
  cricket::TransportStats stats;
  if (transport_controller()->GetStats(channel->transport_name(), &stats)) {
    ReportBestConnectionState(stats);
    ReportNegotiatedCiphers(stats);
  }
}

[Taylor Brandstetter, 2015/08/11 01:20:07]

If BUNDLE is enabled, I assumed we wouldn't want to call 'GetStats("audio",
&stats)' three times, hence the use of a set. Or DO we want that behavior?

+        if (voice_channel()) {
+          transport_names.insert(voice_channel()->transport_name());
+        }
+        if (video_channel()) {
+          transport_names.insert(video_channel()->transport_name());
+        }
+        if (data_channel()) {
+          transport_names.insert(data_channel()->transport_name());
+        }
+        for (auto name : transport_names) {
+          cricket::TransportStats stats;
+          if (transport_controller()->GetStats(name, &stats)) {
+            ReportBestConnectionState(stats);
+            ReportNegotiatedCiphers(stats);
+          }
+        }
+      }
+      break;
+    default:
+      ASSERT(false);
   }
 }
 
-void WebRtcSession::OnTransportConnecting(cricket::Transport* transport) {
-  ASSERT(signaling_thread()->IsCurrent());
-  // start monitoring for the write state of the transport.
-  OnTransportWritable(transport);
-}
-
-void WebRtcSession::OnTransportWritable(cricket::Transport* transport) {
-  ASSERT(signaling_thread()->IsCurrent());
-  if (transport->all_channels_writable()) {
-    SetIceConnectionState(PeerConnectionInterface::kIceConnectionConnected);
-  } else if (transport->HasChannels()) {
-    // If the current state is Connected or Completed, then there were writable
-    // channels but now there are not, so the next state must be Disconnected.
-    if (ice_connection_state_ ==
-            PeerConnectionInterface::kIceConnectionConnected ||
-        ice_connection_state_ ==
-            PeerConnectionInterface::kIceConnectionCompleted) {
-      SetIceConnectionState(
-          PeerConnectionInterface::kIceConnectionDisconnected);
-    }
-  }
-}
-
-void WebRtcSession::OnTransportCompleted(cricket::Transport* transport) {
-  ASSERT(signaling_thread()->IsCurrent());
-  PeerConnectionInterface::IceConnectionState old_state = ice_connection_state_;
-  SetIceConnectionState(PeerConnectionInterface::kIceConnectionCompleted);
-  // Only report once when Ice connection is completed.
-  if (old_state != PeerConnectionInterface::kIceConnectionCompleted) {
-    cricket::TransportStats stats;
-    if (metrics_observer_ && transport->GetStats(&stats)) {
-      ReportBestConnectionState(stats);
-      ReportNegotiatedCiphers(stats);
-    }
-  }
-}
-
-void WebRtcSession::OnTransportFailed(cricket::Transport* transport) {
-  ASSERT(signaling_thread()->IsCurrent());
-  SetIceConnectionState(PeerConnectionInterface::kIceConnectionFailed);
-}
-
-void WebRtcSession::OnTransportReceiving(cricket::Transport* transport) {
-  ASSERT(signaling_thread()->IsCurrent());
-  // The ice connection is considered receiving if at least one transport is
-  // receiving on any channels.
-  bool receiving = false;
-  for (const auto& kv : transport_proxies()) {
-    cricket::Transport* transport = kv.second->impl();
-    if (transport && transport->any_channel_receiving()) {
-      receiving = true;
-      break;
-    }
-  }
+void WebRtcSession::OnTransportControllerReceiving(bool receiving) {
   SetIceConnectionReceiving(receiving);
 }
 
 void WebRtcSession::SetIceConnectionReceiving(bool receiving) {
   if (ice_connection_receiving_ == receiving) {
     return;
   }
   ice_connection_receiving_ = receiving;
   if (ice_observer_) {
     ice_observer_->OnIceConnectionReceivingChange(receiving);
   }
 }
 
-void WebRtcSession::OnTransportProxyCandidatesReady(
-    cricket::TransportProxy* proxy, const cricket::Candidates& candidates) {
+void WebRtcSession::OnTransportControllerCandidatesReady(
+    const std::string& transport_name, const cricket::Candidates& candidates) {
   ASSERT(signaling_thread()->IsCurrent());
-  ProcessNewLocalCandidate(proxy->content_name(), candidates);
-}
-
-void WebRtcSession::OnCandidatesAllocationDone() {
-  ASSERT(signaling_thread()->IsCurrent());
-  if (ice_observer_) {
-    ice_observer_->OnIceGatheringChange(
-      PeerConnectionInterface::kIceGatheringComplete);
-    ice_observer_->OnIceComplete();
-  }
+  ProcessNewLocalCandidate(transport_name, candidates);

[pthatcher1, 2015/08/10 20:40:16]

Could we just put the ProcessNewLocalCandidate code right here?  Why have a
separate method?

[Taylor Brandstetter, 2015/08/11 01:20:07]

Done.

 }
 
 // Enabling voice and video channel.
 void WebRtcSession::EnableChannels() {
   if (voice_channel_ && !voice_channel_->enabled())
     voice_channel_->Enable(true);
 
   if (video_channel_ && !video_channel_->enabled())
     video_channel_->Enable(true);
 
@@ skipping 77 matching lines @@
   size_t mediacontent_index = static_cast<size_t>(candidate->sdp_mline_index());
   size_t remote_content_size = base_remote_description()->contents().size();
   if (mediacontent_index >= remote_content_size) {
     LOG(LS_ERROR)
         << "UseRemoteCandidateInSession: Invalid candidate media index.";
     return false;
   }
 
   cricket::ContentInfo content =
       base_remote_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 (OnRemoteCandidates(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_ ==
             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.
       SetIceConnectionState(PeerConnectionInterface::kIceConnectionChecking);
     }
     // TODO(bemasc): If state is Completed, go back to Connected.
   } else {
     if (!error.empty()) {
       LOG(LS_WARNING) << error;
     }
   }
   return true;
 }
 
-void WebRtcSession::RemoveUnusedChannelsAndTransports(
+void WebRtcSession::RemoveUnusedChannels(
     const SessionDescription* desc) {
   // Destroy video_channel_ first since it may have a pointer to the
   // voice_channel_.
   const cricket::ContentInfo* video_info =
       cricket::GetFirstVideoContent(desc);
   if ((!video_info || video_info->rejected) && video_channel_) {
     mediastream_signaling_->OnVideoChannelClose();
     SignalVideoChannelDestroyed();
     const std::string content_name = video_channel_->content_name();
     channel_manager_->DestroyVideoChannel(video_channel_.release());
-    DestroyTransportProxy(content_name);
   }
 
   const cricket::ContentInfo* voice_info =
       cricket::GetFirstAudioContent(desc);
   if ((!voice_info || voice_info->rejected) && voice_channel_) {
     mediastream_signaling_->OnAudioChannelClose();
     SignalVoiceChannelDestroyed();
     const std::string content_name = voice_channel_->content_name();
     channel_manager_->DestroyVoiceChannel(voice_channel_.release(),
                                           video_channel_.get());
-    DestroyTransportProxy(content_name);
   }
 
   const cricket::ContentInfo* data_info =
       cricket::GetFirstDataContent(desc);
   if ((!data_info || data_info->rejected) && data_channel_) {
     mediastream_signaling_->OnDataChannelClose();
     SignalDataChannelDestroyed();
     const std::string content_name = data_channel_->content_name();
     channel_manager_->DestroyDataChannel(data_channel_.release());
-    DestroyTransportProxy(content_name);
   }
 }
 
 // TODO(mallinath) - Add a correct error code if the channels are not creatued
 // due to BUNDLE is enabled but rtcp-mux is disabled.
 bool WebRtcSession::CreateChannels(const SessionDescription* desc) {
   // Creating the media channels and transport proxies.
   const cricket::ContentInfo* voice = cricket::GetFirstAudioContent(desc);
   if (voice && !voice->rejected && !voice_channel_) {
     if (!CreateVoiceChannel(voice)) {
@@ skipping 32 matching lines @@
   }
 
   // Enable bundle before when kMaxBundle policy is in effect.
   if (bundle_policy_ == PeerConnectionInterface::kBundlePolicyMaxBundle) {
     const cricket::ContentGroup* bundle_group = desc->GetGroupByName(
         cricket::GROUP_TYPE_BUNDLE);
     if (!bundle_group) {
       LOG(LS_WARNING) << "max-bundle specified without BUNDLE specified";
       return false;
     }
-    if (!BaseSession::BundleContentGroup(bundle_group)) {
+    if (!EnableBundle(*bundle_group)) {
       LOG(LS_WARNING) << "max-bundle failed to enable bundling.";
       return false;
     }
   }
 
   return true;
 }
 
 bool WebRtcSession::CreateVoiceChannel(const cricket::ContentInfo* content) {
   voice_channel_.reset(channel_manager_->CreateVoiceChannel(
-      this, content->name, true, audio_options_));
+      transport_controller(), content->name, true, audio_options_));
   if (!voice_channel_) {
     return false;
   }
 
   voice_channel_->SignalDtlsSetupFailure.connect(
       this, &WebRtcSession::OnDtlsSetupFailure);
   return true;
 }
 
 bool WebRtcSession::CreateVideoChannel(const cricket::ContentInfo* content) {
   video_channel_.reset(channel_manager_->CreateVideoChannel(
-      this, content->name, true, video_options_, voice_channel_.get()));
+      transport_controller(), content->name, true,
+      video_options_, voice_channel_.get()));
   if (!video_channel_) {
     return false;
   }
 
   video_channel_->SignalDtlsSetupFailure.connect(
       this, &WebRtcSession::OnDtlsSetupFailure);
   return true;
 }
 
 bool WebRtcSession::CreateDataChannel(const cricket::ContentInfo* content) {
   bool sctp = (data_channel_type_ == cricket::DCT_SCTP);
   data_channel_.reset(channel_manager_->CreateDataChannel(
-      this, content->name, !sctp, data_channel_type_));
+      transport_controller(), content->name, !sctp, data_channel_type_));
   if (!data_channel_) {
     return false;
   }
 
   if (sctp) {
     mediastream_signaling_->OnDataTransportCreatedForSctp();
     data_channel_->SignalDataReceived.connect(
         this, &WebRtcSession::OnDataChannelMessageReceived);
     data_channel_->SignalStreamClosedRemotely.connect(
         mediastream_signaling_,
@@ skipping 160 matching lines @@
 // We need to check the local/remote description for the Transport instead of
 // the session, because a new Transport added during renegotiation may have
 // them unset while the session has them set from the previous negotiation.
 // Not doing so may trigger the auto generation of transport description and
 // mess up DTLS identity information, ICE credential, etc.
 bool WebRtcSession::ReadyToUseRemoteCandidate(
     const IceCandidateInterface* candidate,
     const SessionDescriptionInterface* remote_desc,
     bool* valid) {
   *valid = true;;
-  cricket::TransportProxy* transport_proxy = NULL;
 
   const SessionDescriptionInterface* current_remote_desc =
       remote_desc ? remote_desc : remote_description();
 
   if (!current_remote_desc)
     return false;
 
   size_t mediacontent_index =
       static_cast<size_t>(candidate->sdp_mline_index());
   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.";
 
     *valid = false;
     return false;
   }
 
   cricket::ContentInfo content =
       current_remote_desc->description()->contents()[mediacontent_index];
-  transport_proxy = GetTransportProxy(content.name);
+  cricket::BaseChannel* channel = GetChannel(content.name);
+  if (!channel) {
+    return false;
+  }
 
-  return transport_proxy && transport_proxy->local_description_set() &&
-      transport_proxy->remote_description_set();
+  return transport_controller()->ReadyForRemoteCandidates(
+      channel->transport_name());
+}
+
+void WebRtcSession::OnTransportControllerCandidatesAllocationStarted() {
+  ASSERT(signaling_thread()->IsCurrent());
+  if (ice_observer_) {
+    ice_observer_->OnIceGatheringChange(
+      PeerConnectionInterface::kIceGatheringGathering);
+  }
+}
+
+void WebRtcSession::OnTransportControllerCandidatesAllocationDone() {
+  ASSERT(signaling_thread()->IsCurrent());
+  if (ice_observer_) {
+    ice_observer_->OnIceGatheringChange(
+      PeerConnectionInterface::kIceGatheringComplete);
+    ice_observer_->OnIceComplete();
+  }
 }
 
 // Walk through the ConnectionInfos to gather best connection usage
 // for IPv4 and IPv6.
 void WebRtcSession::ReportBestConnectionState(
     const cricket::TransportStats& stats) {
   DCHECK(metrics_observer_ != NULL);
   for (cricket::TransportChannelStatsList::const_iterator it =
          stats.channel_stats.begin();
        it != stats.channel_stats.end(); ++it) {
@@ skipping 47 matching lines @@
 
   if (!srtp_cipher.empty()) {
     metrics_observer_->AddHistogramSample(srtp_name, srtp_cipher);
   }
   if (!ssl_cipher.empty()) {
     metrics_observer_->AddHistogramSample(ssl_name, ssl_cipher);
   }
 }
 
 }  // namespace webrtc