Remove a hacky dependency of BaseChannel on BaseSession by moving the handling of DTLS setup failure into a signal on BaseChannel rather than a method call on BaseSession.
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/47589004
Cr-Commit-Position: refs/heads/master@{#8740}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8740 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc
index fb18576..c2ce02c 100644
--- a/talk/app/webrtc/webrtcsession.cc
+++ b/talk/app/webrtc/webrtcsession.cc
@@ -76,6 +76,10 @@
"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 int kMaxUnsignalledRecvStreams = 20;
// Compares |answer| against |offer|. Comparision is done
@@ -495,15 +499,15 @@
WebRtcSession::~WebRtcSession() {
// Destroy video_channel_ first since it may have a pointer to the
// voice_channel_.
- if (video_channel_.get()) {
+ if (video_channel_) {
SignalVideoChannelDestroyed();
channel_manager_->DestroyVideoChannel(video_channel_.release());
}
- if (voice_channel_.get()) {
+ if (voice_channel_) {
SignalVoiceChannelDestroyed();
channel_manager_->DestroyVoiceChannel(voice_channel_.release());
}
- if (data_channel_.get()) {
+ if (data_channel_) {
SignalDataChannelDestroyed();
channel_manager_->DestroyDataChannel(data_channel_.release());
}
@@ -670,9 +674,9 @@
void WebRtcSession::Terminate() {
SetState(STATE_RECEIVEDTERMINATE);
RemoveUnusedChannelsAndTransports(NULL);
- ASSERT(voice_channel_.get() == NULL);
- ASSERT(video_channel_.get() == NULL);
- ASSERT(data_channel_.get() == NULL);
+ ASSERT(!voice_channel_);
+ ASSERT(!video_channel_);
+ ASSERT(!data_channel_);
}
bool WebRtcSession::StartCandidatesAllocation() {
@@ -1061,7 +1065,7 @@
cricket::VideoCapturer* camera) {
ASSERT(signaling_thread()->IsCurrent());
- if (!video_channel_.get()) {
+ if (!video_channel_) {
// |video_channel_| doesnt't exist. Probably because the remote end doesnt't
// support video.
LOG(LS_WARNING) << "Video not used in this call.";
@@ -1156,7 +1160,7 @@
bool WebRtcSession::SendData(const cricket::SendDataParams& params,
const rtc::Buffer& payload,
cricket::SendDataResult* result) {
- if (!data_channel_.get()) {
+ if (!data_channel_) {
LOG(LS_ERROR) << "SendData called when data_channel_ is NULL.";
return false;
}
@@ -1164,7 +1168,7 @@
}
bool WebRtcSession::ConnectDataChannel(DataChannel* webrtc_data_channel) {
- if (!data_channel_.get()) {
+ if (!data_channel_) {
LOG(LS_ERROR) << "ConnectDataChannel called when data_channel_ is NULL.";
return false;
}
@@ -1176,7 +1180,7 @@
}
void WebRtcSession::DisconnectDataChannel(DataChannel* webrtc_data_channel) {
- if (!data_channel_.get()) {
+ if (!data_channel_) {
LOG(LS_ERROR) << "DisconnectDataChannel called when data_channel_ is NULL.";
return;
}
@@ -1185,7 +1189,7 @@
}
void WebRtcSession::AddSctpDataStream(int sid) {
- if (!data_channel_.get()) {
+ if (!data_channel_) {
LOG(LS_ERROR) << "AddDataChannelStreams called when data_channel_ is NULL.";
return;
}
@@ -1196,7 +1200,7 @@
void WebRtcSession::RemoveSctpDataStream(int sid) {
mediastream_signaling_->RemoveSctpDataChannel(sid);
- if (!data_channel_.get()) {
+ if (!data_channel_) {
LOG(LS_ERROR) << "RemoveDataChannelStreams called when data_channel_ is "
<< "NULL.";
return;
@@ -1206,7 +1210,7 @@
}
bool WebRtcSession::ReadyToSendData() const {
- return data_channel_.get() && data_channel_->ready_to_send_data();
+ return data_channel_ && data_channel_->ready_to_send_data();
}
rtc::scoped_refptr<DataChannel> WebRtcSession::CreateDataChannel(
@@ -1385,7 +1389,7 @@
if (video_channel_ && !video_channel_->enabled())
video_channel_->Enable(true);
- if (data_channel_.get() && !data_channel_->enabled())
+ if (data_channel_ && !data_channel_->enabled())
data_channel_->Enable(true);
}
@@ -1566,7 +1570,7 @@
const cricket::ContentInfo* data = cricket::GetFirstDataContent(desc);
if (data_channel_type_ != cricket::DCT_NONE &&
- data && !data->rejected && !data_channel_.get()) {
+ data && !data->rejected && !data_channel_) {
if (!CreateDataChannel(data)) {
LOG(LS_ERROR) << "Failed to create data channel.";
return false;
@@ -1579,26 +1583,36 @@
bool WebRtcSession::CreateVoiceChannel(const cricket::ContentInfo* content) {
voice_channel_.reset(channel_manager_->CreateVoiceChannel(
this, content->name, true));
- if (!voice_channel_.get())
+ if (!voice_channel_) {
return false;
+ }
voice_channel_->SetChannelOptions(audio_options_);
+ 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()));
- return video_channel_.get() != NULL;
+ 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_));
- if (!data_channel_.get()) {
+ if (!data_channel_) {
return false;
}
+
if (sctp) {
mediastream_signaling_->OnDataTransportCreatedForSctp();
data_channel_->SignalDataReceived.connect(
@@ -1607,9 +1621,17 @@
mediastream_signaling_,
&MediaStreamSignaling::OnRemoteSctpDataChannelClosed);
}
+
+ data_channel_->SignalDtlsSetupFailure.connect(
+ this, &WebRtcSession::OnDtlsSetupFailure);
return true;
}
+void WebRtcSession::OnDtlsSetupFailure(cricket::BaseChannel*, bool rtcp) {
+ SetError(BaseSession::ERROR_TRANSPORT, rtcp ? kDtlsSetupFailureRtcp :
+ kDtlsSetupFailureRtp);
+}
+
void WebRtcSession::CopySavedCandidates(
SessionDescriptionInterface* dest_desc) {
if (!dest_desc) {
diff --git a/talk/app/webrtc/webrtcsession.h b/talk/app/webrtc/webrtcsession.h
index 01b352e..14e4414 100644
--- a/talk/app/webrtc/webrtcsession.h
+++ b/talk/app/webrtc/webrtcsession.h
@@ -73,6 +73,8 @@
extern const char kSdpWithoutSdesAndDtlsDisabled[];
extern const char kSessionError[];
extern const char kSessionErrorDesc[];
+extern const char kDtlsSetupFailureRtp[];
+extern const char kDtlsSetupFailureRtcp[];
// Maximum number of received video streams that will be processed by webrtc
// even if they are not signalled beforehand.
extern const int kMaxUnsignalledRecvStreams;
@@ -232,6 +234,7 @@
// Called when an SSLIdentity is generated or retrieved by
// WebRTCSessionDescriptionFactory. Should happen before setLocalDescription.
void OnIdentityReady(rtc::SSLIdentity* identity);
+ void OnDtlsSetupFailure(cricket::BaseChannel*, bool rtcp);
// For unit test.
bool waiting_for_identity() const;
diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc
index 025a6ad..089ed48 100644
--- a/talk/session/media/channel.cc
+++ b/talk/session/media/channel.cc
@@ -722,27 +722,13 @@
// If we're doing DTLS-SRTP, now is the time.
if (!was_ever_writable_ && ShouldSetupDtlsSrtp()) {
if (!SetupDtlsSrtp(false)) {
- const std::string error_desc =
- "Couldn't set up DTLS-SRTP on RTP channel.";
- // Sent synchronously.
- signaling_thread()->Invoke<void>(Bind(
- &SetSessionError,
- session_,
- BaseSession::ERROR_TRANSPORT,
- error_desc));
+ SignalDtlsSetupFailure(this, false);
return;
}
if (rtcp_transport_channel_) {
if (!SetupDtlsSrtp(true)) {
- const std::string error_desc =
- "Couldn't set up DTLS-SRTP on RTCP channel";
- // Sent synchronously.
- signaling_thread()->Invoke<void>(Bind(
- &SetSessionError,
- session_,
- BaseSession::ERROR_TRANSPORT,
- error_desc));
+ SignalDtlsSetupFailure(this, true);
return;
}
}
@@ -753,6 +739,17 @@
ChangeState();
}
+void BaseChannel::SignalDtlsSetupFailure_w(bool rtcp) {
+ ASSERT(worker_thread() == rtc::Thread::Current());
+ signaling_thread()->Invoke<void>(Bind(
+ &BaseChannel::SignalDtlsSetupFailure_s, this, rtcp));
+}
+
+void BaseChannel::SignalDtlsSetupFailure_s(bool rtcp) {
+ ASSERT(signaling_thread() == rtc::Thread::Current());
+ SignalDtlsSetupFailure(this, rtcp);
+}
+
bool BaseChannel::SetDtlsSrtpCiphers(TransportChannel *tc, bool rtcp) {
std::vector<std::string> ciphers;
// We always use the default SRTP ciphers for RTCP, but we may use different
diff --git a/talk/session/media/channel.h b/talk/session/media/channel.h
index bb88c2f..7e96fe7 100644
--- a/talk/session/media/channel.h
+++ b/talk/session/media/channel.h
@@ -213,6 +213,10 @@
return remote_streams_;
}
+ sigslot::signal2<BaseChannel*, bool> SignalDtlsSetupFailure;
+ void SignalDtlsSetupFailure_w(bool rtcp);
+ void SignalDtlsSetupFailure_s(bool rtcp);
+
// Used for latency measurements.
sigslot::signal1<BaseChannel*> SignalFirstPacketReceived;