Refactor how the TransportChannels are set in the BaseChannel to rely lesson Session, so that in the future we can rely on Transport instead, and also be able to change Transports on the fly for BUNDLE.
Also, remove channel_name. It's no longer needed.
This is a part of the big BUNDLE implementation at https://webrtc-codereview.appspot.com/45519004/
R=decurtis@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/43719004
Cr-Commit-Position: refs/heads/master@{#8741}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8741 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc
index 089ed48..3c2c64f 100644
--- a/talk/session/media/channel.cc
+++ b/talk/session/media/channel.cc
@@ -187,39 +187,28 @@
// the media channel may try to send on the dead transport channel. NULLing
// is not an effective strategy since the sends will come on another thread.
delete media_channel_;
- set_rtcp_transport_channel(NULL);
- if (transport_channel_ != NULL)
- session_->DestroyChannel(content_name_, transport_channel_->component());
+ set_transport_channel(nullptr);
+ set_rtcp_transport_channel(nullptr);
LOG(LS_INFO) << "Destroyed channel";
}
-bool BaseChannel::Init(TransportChannel* transport_channel,
- TransportChannel* rtcp_transport_channel) {
- if (transport_channel == NULL) {
- return false;
- }
- if (rtcp() && rtcp_transport_channel == NULL) {
- return false;
- }
- transport_channel_ = transport_channel;
-
- if (!SetDtlsSrtpCiphers(transport_channel_, false)) {
+bool BaseChannel::Init() {
+ if (!SetTransportChannels(session(), rtcp())) {
return false;
}
- transport_channel_->SignalWritableState.connect(
- this, &BaseChannel::OnWritableState);
- transport_channel_->SignalReadPacket.connect(
- this, &BaseChannel::OnChannelRead);
- transport_channel_->SignalReadyToSend.connect(
- this, &BaseChannel::OnReadyToSend);
+ if (!SetDtlsSrtpCiphers(transport_channel(), false)) {
+ return false;
+ }
+ if (rtcp() && !SetDtlsSrtpCiphers(rtcp_transport_channel(), true)) {
+ return false;
+ }
session_->SignalNewLocalDescription.connect(
this, &BaseChannel::OnNewLocalDescription);
session_->SignalNewRemoteDescription.connect(
this, &BaseChannel::OnNewRemoteDescription);
- set_rtcp_transport_channel(rtcp_transport_channel);
// Both RTP and RTCP channels are set, we can call SetInterface on
// media channel and it can set network options.
media_channel_->SetInterface(this);
@@ -230,6 +219,90 @@
media_channel_->SetInterface(NULL);
}
+bool BaseChannel::SetTransportChannels(BaseSession* session, bool rtcp) {
+ return worker_thread_->Invoke<bool>(Bind(
+ &BaseChannel::SetTransportChannels_w, this, session, rtcp));
+}
+
+bool BaseChannel::SetTransportChannels_w(BaseSession* session, bool rtcp) {
+ ASSERT(worker_thread_ == rtc::Thread::Current());
+
+ set_transport_channel(session->CreateChannel(
+ content_name(), cricket::ICE_CANDIDATE_COMPONENT_RTP));
+ if (!transport_channel()) {
+ return false;
+ }
+ if (rtcp) {
+ set_rtcp_transport_channel(session->CreateChannel(
+ content_name(), cricket::ICE_CANDIDATE_COMPONENT_RTCP));
+ if (!rtcp_transport_channel()) {
+ return false;
+ }
+ } else {
+ set_rtcp_transport_channel(nullptr);
+ }
+
+ return true;
+}
+
+void BaseChannel::set_transport_channel(TransportChannel* new_tc) {
+ ASSERT(worker_thread_ == rtc::Thread::Current());
+
+ TransportChannel* old_tc = transport_channel_;
+
+ if (old_tc == new_tc) {
+ return;
+ }
+ if (old_tc) {
+ DisconnectFromTransportChannel(old_tc);
+ session()->DestroyChannel(
+ content_name(), cricket::ICE_CANDIDATE_COMPONENT_RTP);
+ }
+
+ transport_channel_ = new_tc;
+
+ if (new_tc) {
+ ConnectToTransportChannel(new_tc);
+ }
+}
+
+void BaseChannel::set_rtcp_transport_channel(TransportChannel* new_tc) {
+ ASSERT(worker_thread_ == rtc::Thread::Current());
+
+ TransportChannel* old_tc = rtcp_transport_channel_;
+
+ if (old_tc == new_tc) {
+ return;
+ }
+ if (old_tc) {
+ DisconnectFromTransportChannel(old_tc);
+ session()->DestroyChannel(
+ content_name(), cricket::ICE_CANDIDATE_COMPONENT_RTCP);
+ }
+
+ rtcp_transport_channel_ = new_tc;
+
+ if (new_tc) {
+ ConnectToTransportChannel(new_tc);
+ }
+}
+
+void BaseChannel::ConnectToTransportChannel(TransportChannel* tc) {
+ ASSERT(worker_thread_ == rtc::Thread::Current());
+
+ tc->SignalWritableState.connect(this, &BaseChannel::OnWritableState);
+ tc->SignalReadPacket.connect(this, &BaseChannel::OnChannelRead);
+ tc->SignalReadyToSend.connect(this, &BaseChannel::OnReadyToSend);
+}
+
+void BaseChannel::DisconnectFromTransportChannel(TransportChannel* tc) {
+ ASSERT(worker_thread_ == rtc::Thread::Current());
+
+ tc->SignalWritableState.disconnect(this);
+ tc->SignalReadPacket.disconnect(this);
+ tc->SignalReadyToSend.disconnect(this);
+}
+
bool BaseChannel::Enable(bool enable) {
worker_thread_->Invoke<void>(Bind(
enable ? &BaseChannel::EnableMedia_w : &BaseChannel::DisableMedia_w,
@@ -300,26 +373,6 @@
return transport_channel_->GetStats(infos);
}
-void BaseChannel::set_rtcp_transport_channel(TransportChannel* channel) {
- if (rtcp_transport_channel_ != channel) {
- if (rtcp_transport_channel_) {
- session_->DestroyChannel(
- content_name_, rtcp_transport_channel_->component());
- }
- rtcp_transport_channel_ = channel;
- if (rtcp_transport_channel_) {
- // TODO(juberti): Propagate this error code
- VERIFY(SetDtlsSrtpCiphers(rtcp_transport_channel_, true));
- rtcp_transport_channel_->SignalWritableState.connect(
- this, &BaseChannel::OnWritableState);
- rtcp_transport_channel_->SignalReadPacket.connect(
- this, &BaseChannel::OnChannelRead);
- rtcp_transport_channel_->SignalReadyToSend.connect(
- this, &BaseChannel::OnReadyToSend);
- }
- }
-}
-
bool BaseChannel::IsReadyToReceive() const {
// Receive data if we are enabled and have local content,
return enabled() && IsReceiveContentDirection(local_content_direction_);
@@ -1271,11 +1324,7 @@
}
bool VoiceChannel::Init() {
- TransportChannel* rtcp_channel = rtcp() ? session()->CreateChannel(
- content_name(), "rtcp", ICE_CANDIDATE_COMPONENT_RTCP) : NULL;
- if (!BaseChannel::Init(session()->CreateChannel(
- content_name(), "rtp", ICE_CANDIDATE_COMPONENT_RTP),
- rtcp_channel)) {
+ if (!BaseChannel::Init()) {
return false;
}
media_channel()->SignalMediaError.connect(
@@ -1665,11 +1714,7 @@
}
bool VideoChannel::Init() {
- TransportChannel* rtcp_channel = rtcp() ? session()->CreateChannel(
- content_name(), "video_rtcp", ICE_CANDIDATE_COMPONENT_RTCP) : NULL;
- if (!BaseChannel::Init(session()->CreateChannel(
- content_name(), "video_rtp", ICE_CANDIDATE_COMPONENT_RTP),
- rtcp_channel)) {
+ if (!BaseChannel::Init()) {
return false;
}
media_channel()->SignalMediaError.connect(
@@ -2118,11 +2163,7 @@
}
bool DataChannel::Init() {
- TransportChannel* rtcp_channel = rtcp() ? session()->CreateChannel(
- content_name(), "data_rtcp", ICE_CANDIDATE_COMPONENT_RTCP) : NULL;
- if (!BaseChannel::Init(session()->CreateChannel(
- content_name(), "data_rtp", ICE_CANDIDATE_COMPONENT_RTP),
- rtcp_channel)) {
+ if (!BaseChannel::Init()) {
return false;
}
media_channel()->SignalDataReceived.connect(
diff --git a/talk/session/media/channel.h b/talk/session/media/channel.h
index 7e96fe7..bc4b6f7 100644
--- a/talk/session/media/channel.h
+++ b/talk/session/media/channel.h
@@ -80,8 +80,7 @@
MediaChannel* channel, BaseSession* session,
const std::string& content_name, bool rtcp);
virtual ~BaseChannel();
- bool Init(TransportChannel* transport_channel,
- TransportChannel* rtcp_transport_channel);
+ bool Init();
// Deinit may be called multiple times and is simply ignored if it's alreay
// done.
void Deinit();
@@ -232,6 +231,13 @@
protected:
MediaEngineInterface* media_engine() const { return media_engine_; }
virtual MediaChannel* media_channel() const { return media_channel_; }
+ // Sets the transport_channel_ and rtcp_transport_channel_. If
+ // |rtcp| is false, set rtcp_transport_channel_ is set to NULL. Get
+ // the transport channels from |session|.
+ // TODO(pthatcher): Pass in a Transport instead of a BaseSession.
+ bool SetTransportChannels(BaseSession* session, bool rtcp);
+ bool SetTransportChannels_w(BaseSession* session, bool rtcp);
+ void set_transport_channel(TransportChannel* transport);
void set_rtcp_transport_channel(TransportChannel* transport);
bool was_ever_writable() const { return was_ever_writable_; }
void set_local_content_direction(MediaContentDirection direction) {
@@ -246,6 +252,9 @@
SrtpFilter* srtp_filter() { return &srtp_filter_; }
bool rtcp() const { return rtcp_; }
+ void ConnectToTransportChannel(TransportChannel* tc);
+ void DisconnectFromTransportChannel(TransportChannel* tc);
+
void FlushRtcpMessages();
// NetworkInterface implementation, called by MediaEngine
diff --git a/talk/session/media/channelmanager_unittest.cc b/talk/session/media/channelmanager_unittest.cc
index 8b5b326..f028001 100644
--- a/talk/session/media/channelmanager_unittest.cc
+++ b/talk/session/media/channelmanager_unittest.cc
@@ -172,7 +172,7 @@
// The test is useless unless the session does not fail creating
// cricket::TransportChannel.
ASSERT_TRUE(session_->CreateChannel(
- "audio", "rtp", cricket::ICE_CANDIDATE_COMPONENT_RTP) == NULL);
+ "audio", cricket::ICE_CANDIDATE_COMPONENT_RTP) == NULL);
cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel(
session_, cricket::CN_AUDIO, false);
diff --git a/webrtc/p2p/base/fakesession.h b/webrtc/p2p/base/fakesession.h
index f942487..5486e46 100644
--- a/webrtc/p2p/base/fakesession.h
+++ b/webrtc/p2p/base/fakesession.h
@@ -465,12 +465,11 @@
virtual TransportChannel* CreateChannel(
const std::string& content_name,
- const std::string& channel_name,
int component) {
if (fail_create_channel_) {
return NULL;
}
- return BaseSession::CreateChannel(content_name, channel_name, component);
+ return BaseSession::CreateChannel(content_name, component);
}
void set_fail_channel_creation(bool fail_channel_creation) {
diff --git a/webrtc/p2p/base/session.cc b/webrtc/p2p/base/session.cc
index 690cf13..9078cb4 100644
--- a/webrtc/p2p/base/session.cc
+++ b/webrtc/p2p/base/session.cc
@@ -46,15 +46,14 @@
return GetChannelProxy(component);
}
-TransportChannel* TransportProxy::CreateChannel(const std::string& name,
- int component) {
+TransportChannel* TransportProxy::CreateChannel(int component) {
ASSERT(rtc::Thread::Current() == worker_thread_);
ASSERT(GetChannel(component) == NULL);
ASSERT(!transport_->get()->HasChannel(component));
// We always create a proxy in case we need to change out the transport later.
TransportChannelProxy* channel_proxy =
- new TransportChannelProxy(content_name(), name, component);
+ new TransportChannelProxy(content_name(), component);
channels_[component] = channel_proxy;
// If we're already negotiated, create an impl and hook it up to the proxy
@@ -142,18 +141,6 @@
return (iter != channels_.end()) ? iter->second : NULL;
}
-TransportChannelProxy* TransportProxy::GetChannelProxyByName(
- const std::string& name) const {
- for (ChannelMap::const_iterator iter = channels_.begin();
- iter != channels_.end();
- ++iter) {
- if (iter->second->name() == name) {
- return iter->second;
- }
- }
- return NULL;
-}
-
void TransportProxy::CreateChannelImpl(int component) {
worker_thread_->Invoke<void>(Bind(
&TransportProxy::CreateChannelImpl_w, this, component));
@@ -474,13 +461,12 @@
}
TransportChannel* BaseSession::CreateChannel(const std::string& content_name,
- const std::string& channel_name,
int component) {
// We create the proxy "on demand" here because we need to support
// creating channels at any time, even before we send or receive
// initiate messages, which is before we create the transports.
TransportProxy* transproxy = GetOrCreateTransportProxy(content_name);
- return transproxy->CreateChannel(channel_name, component);
+ return transproxy->CreateChannel(component);
}
TransportChannel* BaseSession::GetChannel(const std::string& content_name,
diff --git a/webrtc/p2p/base/session.h b/webrtc/p2p/base/session.h
index 329ae2a..8dffca7 100644
--- a/webrtc/p2p/base/session.h
+++ b/webrtc/p2p/base/session.h
@@ -81,8 +81,7 @@
}
TransportChannel* GetChannel(int component);
- TransportChannel* CreateChannel(const std::string& channel_name,
- int component);
+ TransportChannel* CreateChannel(int component);
bool HasChannel(int component);
void DestroyChannel(int component);
@@ -130,7 +129,6 @@
private:
TransportChannelProxy* GetChannelProxy(int component) const;
- TransportChannelProxy* GetChannelProxyByName(const std::string& name) const;
// Creates a new channel on the Transport which causes the reference
// count to increment.
@@ -306,7 +304,6 @@
// shouldn't be an issue since the main thread will be blocked in
// Send when doing so.
virtual TransportChannel* CreateChannel(const std::string& content_name,
- const std::string& channel_name,
int component);
// Returns the channel with the given names.
diff --git a/webrtc/p2p/base/transportchannelproxy.cc b/webrtc/p2p/base/transportchannelproxy.cc
index 5118920..7977251 100644
--- a/webrtc/p2p/base/transportchannelproxy.cc
+++ b/webrtc/p2p/base/transportchannelproxy.cc
@@ -22,10 +22,8 @@
};
TransportChannelProxy::TransportChannelProxy(const std::string& content_name,
- const std::string& name,
int component)
: TransportChannel(content_name, component),
- name_(name),
impl_(NULL) {
worker_thread_ = rtc::Thread::Current();
}
diff --git a/webrtc/p2p/base/transportchannelproxy.h b/webrtc/p2p/base/transportchannelproxy.h
index ffdad43..23cd20b 100644
--- a/webrtc/p2p/base/transportchannelproxy.h
+++ b/webrtc/p2p/base/transportchannelproxy.h
@@ -34,11 +34,9 @@
public rtc::MessageHandler {
public:
TransportChannelProxy(const std::string& content_name,
- const std::string& name,
int component);
virtual ~TransportChannelProxy();
- const std::string& name() const { return name_; }
TransportChannelImpl* impl() { return impl_; }
virtual TransportChannelState GetState() const;
@@ -85,7 +83,6 @@
typedef std::pair<rtc::Socket::Option, int> OptionPair;
typedef std::vector<OptionPair> OptionList;
- std::string name_;
rtc::Thread* worker_thread_;
TransportChannelImpl* impl_;
OptionList options_;
