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
This commit is contained in:
pthatcher@webrtc.org
2015-03-16 19:34:23 +00:00
parent c04a97f054
commit 4eeef584a7
4 changed files with 60 additions and 34 deletions

View File

@@ -76,6 +76,10 @@ const char kSdpWithoutIceUfragPwd[] =
"Called with SDP without ice-ufrag and ice-pwd."; "Called with SDP without ice-ufrag and ice-pwd.";
const char kSessionError[] = "Session error code: "; const char kSessionError[] = "Session error code: ";
const char kSessionErrorDesc[] = "Session error description: "; 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; const int kMaxUnsignalledRecvStreams = 20;
// Compares |answer| against |offer|. Comparision is done // Compares |answer| against |offer|. Comparision is done
@@ -495,15 +499,15 @@ WebRtcSession::WebRtcSession(
WebRtcSession::~WebRtcSession() { WebRtcSession::~WebRtcSession() {
// Destroy video_channel_ first since it may have a pointer to the // Destroy video_channel_ first since it may have a pointer to the
// voice_channel_. // voice_channel_.
if (video_channel_.get()) { if (video_channel_) {
SignalVideoChannelDestroyed(); SignalVideoChannelDestroyed();
channel_manager_->DestroyVideoChannel(video_channel_.release()); channel_manager_->DestroyVideoChannel(video_channel_.release());
} }
if (voice_channel_.get()) { if (voice_channel_) {
SignalVoiceChannelDestroyed(); SignalVoiceChannelDestroyed();
channel_manager_->DestroyVoiceChannel(voice_channel_.release()); channel_manager_->DestroyVoiceChannel(voice_channel_.release());
} }
if (data_channel_.get()) { if (data_channel_) {
SignalDataChannelDestroyed(); SignalDataChannelDestroyed();
channel_manager_->DestroyDataChannel(data_channel_.release()); channel_manager_->DestroyDataChannel(data_channel_.release());
} }
@@ -670,9 +674,9 @@ bool WebRtcSession::Initialize(
void WebRtcSession::Terminate() { void WebRtcSession::Terminate() {
SetState(STATE_RECEIVEDTERMINATE); SetState(STATE_RECEIVEDTERMINATE);
RemoveUnusedChannelsAndTransports(NULL); RemoveUnusedChannelsAndTransports(NULL);
ASSERT(voice_channel_.get() == NULL); ASSERT(!voice_channel_);
ASSERT(video_channel_.get() == NULL); ASSERT(!video_channel_);
ASSERT(data_channel_.get() == NULL); ASSERT(!data_channel_);
} }
bool WebRtcSession::StartCandidatesAllocation() { bool WebRtcSession::StartCandidatesAllocation() {
@@ -1061,7 +1065,7 @@ bool WebRtcSession::SetCaptureDevice(uint32 ssrc,
cricket::VideoCapturer* camera) { cricket::VideoCapturer* camera) {
ASSERT(signaling_thread()->IsCurrent()); ASSERT(signaling_thread()->IsCurrent());
if (!video_channel_.get()) { if (!video_channel_) {
// |video_channel_| doesnt't exist. Probably because the remote end doesnt't // |video_channel_| doesnt't exist. Probably because the remote end doesnt't
// support video. // support video.
LOG(LS_WARNING) << "Video not used in this call."; LOG(LS_WARNING) << "Video not used in this call.";
@@ -1156,7 +1160,7 @@ sigslot::signal0<>* WebRtcSession::GetOnDestroyedSignal() {
bool WebRtcSession::SendData(const cricket::SendDataParams& params, bool WebRtcSession::SendData(const cricket::SendDataParams& params,
const rtc::Buffer& payload, const rtc::Buffer& payload,
cricket::SendDataResult* result) { cricket::SendDataResult* result) {
if (!data_channel_.get()) { if (!data_channel_) {
LOG(LS_ERROR) << "SendData called when data_channel_ is NULL."; LOG(LS_ERROR) << "SendData called when data_channel_ is NULL.";
return false; return false;
} }
@@ -1164,7 +1168,7 @@ bool WebRtcSession::SendData(const cricket::SendDataParams& params,
} }
bool WebRtcSession::ConnectDataChannel(DataChannel* webrtc_data_channel) { bool WebRtcSession::ConnectDataChannel(DataChannel* webrtc_data_channel) {
if (!data_channel_.get()) { if (!data_channel_) {
LOG(LS_ERROR) << "ConnectDataChannel called when data_channel_ is NULL."; LOG(LS_ERROR) << "ConnectDataChannel called when data_channel_ is NULL.";
return false; return false;
} }
@@ -1176,7 +1180,7 @@ bool WebRtcSession::ConnectDataChannel(DataChannel* webrtc_data_channel) {
} }
void WebRtcSession::DisconnectDataChannel(DataChannel* webrtc_data_channel) { void WebRtcSession::DisconnectDataChannel(DataChannel* webrtc_data_channel) {
if (!data_channel_.get()) { if (!data_channel_) {
LOG(LS_ERROR) << "DisconnectDataChannel called when data_channel_ is NULL."; LOG(LS_ERROR) << "DisconnectDataChannel called when data_channel_ is NULL.";
return; return;
} }
@@ -1185,7 +1189,7 @@ void WebRtcSession::DisconnectDataChannel(DataChannel* webrtc_data_channel) {
} }
void WebRtcSession::AddSctpDataStream(int sid) { void WebRtcSession::AddSctpDataStream(int sid) {
if (!data_channel_.get()) { if (!data_channel_) {
LOG(LS_ERROR) << "AddDataChannelStreams called when data_channel_ is NULL."; LOG(LS_ERROR) << "AddDataChannelStreams called when data_channel_ is NULL.";
return; return;
} }
@@ -1196,7 +1200,7 @@ void WebRtcSession::AddSctpDataStream(int sid) {
void WebRtcSession::RemoveSctpDataStream(int sid) { void WebRtcSession::RemoveSctpDataStream(int sid) {
mediastream_signaling_->RemoveSctpDataChannel(sid); mediastream_signaling_->RemoveSctpDataChannel(sid);
if (!data_channel_.get()) { if (!data_channel_) {
LOG(LS_ERROR) << "RemoveDataChannelStreams called when data_channel_ is " LOG(LS_ERROR) << "RemoveDataChannelStreams called when data_channel_ is "
<< "NULL."; << "NULL.";
return; return;
@@ -1206,7 +1210,7 @@ void WebRtcSession::RemoveSctpDataStream(int sid) {
} }
bool WebRtcSession::ReadyToSendData() const { 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( rtc::scoped_refptr<DataChannel> WebRtcSession::CreateDataChannel(
@@ -1385,7 +1389,7 @@ void WebRtcSession::EnableChannels() {
if (video_channel_ && !video_channel_->enabled()) if (video_channel_ && !video_channel_->enabled())
video_channel_->Enable(true); video_channel_->Enable(true);
if (data_channel_.get() && !data_channel_->enabled()) if (data_channel_ && !data_channel_->enabled())
data_channel_->Enable(true); data_channel_->Enable(true);
} }
@@ -1566,7 +1570,7 @@ bool WebRtcSession::CreateChannels(const SessionDescription* desc) {
const cricket::ContentInfo* data = cricket::GetFirstDataContent(desc); const cricket::ContentInfo* data = cricket::GetFirstDataContent(desc);
if (data_channel_type_ != cricket::DCT_NONE && if (data_channel_type_ != cricket::DCT_NONE &&
data && !data->rejected && !data_channel_.get()) { data && !data->rejected && !data_channel_) {
if (!CreateDataChannel(data)) { if (!CreateDataChannel(data)) {
LOG(LS_ERROR) << "Failed to create data channel."; LOG(LS_ERROR) << "Failed to create data channel.";
return false; return false;
@@ -1579,26 +1583,36 @@ bool WebRtcSession::CreateChannels(const SessionDescription* desc) {
bool WebRtcSession::CreateVoiceChannel(const cricket::ContentInfo* content) { bool WebRtcSession::CreateVoiceChannel(const cricket::ContentInfo* content) {
voice_channel_.reset(channel_manager_->CreateVoiceChannel( voice_channel_.reset(channel_manager_->CreateVoiceChannel(
this, content->name, true)); this, content->name, true));
if (!voice_channel_.get()) if (!voice_channel_) {
return false; return false;
}
voice_channel_->SetChannelOptions(audio_options_); voice_channel_->SetChannelOptions(audio_options_);
voice_channel_->SignalDtlsSetupFailure.connect(
this, &WebRtcSession::OnDtlsSetupFailure);
return true; return true;
} }
bool WebRtcSession::CreateVideoChannel(const cricket::ContentInfo* content) { bool WebRtcSession::CreateVideoChannel(const cricket::ContentInfo* content) {
video_channel_.reset(channel_manager_->CreateVideoChannel( video_channel_.reset(channel_manager_->CreateVideoChannel(
this, content->name, true, video_options_, voice_channel_.get())); 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 WebRtcSession::CreateDataChannel(const cricket::ContentInfo* content) {
bool sctp = (data_channel_type_ == cricket::DCT_SCTP); bool sctp = (data_channel_type_ == cricket::DCT_SCTP);
data_channel_.reset(channel_manager_->CreateDataChannel( data_channel_.reset(channel_manager_->CreateDataChannel(
this, content->name, !sctp, data_channel_type_)); this, content->name, !sctp, data_channel_type_));
if (!data_channel_.get()) { if (!data_channel_) {
return false; return false;
} }
if (sctp) { if (sctp) {
mediastream_signaling_->OnDataTransportCreatedForSctp(); mediastream_signaling_->OnDataTransportCreatedForSctp();
data_channel_->SignalDataReceived.connect( data_channel_->SignalDataReceived.connect(
@@ -1607,9 +1621,17 @@ bool WebRtcSession::CreateDataChannel(const cricket::ContentInfo* content) {
mediastream_signaling_, mediastream_signaling_,
&MediaStreamSignaling::OnRemoteSctpDataChannelClosed); &MediaStreamSignaling::OnRemoteSctpDataChannelClosed);
} }
data_channel_->SignalDtlsSetupFailure.connect(
this, &WebRtcSession::OnDtlsSetupFailure);
return true; return true;
} }
void WebRtcSession::OnDtlsSetupFailure(cricket::BaseChannel*, bool rtcp) {
SetError(BaseSession::ERROR_TRANSPORT, rtcp ? kDtlsSetupFailureRtcp :
kDtlsSetupFailureRtp);
}
void WebRtcSession::CopySavedCandidates( void WebRtcSession::CopySavedCandidates(
SessionDescriptionInterface* dest_desc) { SessionDescriptionInterface* dest_desc) {
if (!dest_desc) { if (!dest_desc) {

View File

@@ -73,6 +73,8 @@ extern const char kSdpWithoutIceUfragPwd[];
extern const char kSdpWithoutSdesAndDtlsDisabled[]; extern const char kSdpWithoutSdesAndDtlsDisabled[];
extern const char kSessionError[]; extern const char kSessionError[];
extern const char kSessionErrorDesc[]; extern const char kSessionErrorDesc[];
extern const char kDtlsSetupFailureRtp[];
extern const char kDtlsSetupFailureRtcp[];
// Maximum number of received video streams that will be processed by webrtc // Maximum number of received video streams that will be processed by webrtc
// even if they are not signalled beforehand. // even if they are not signalled beforehand.
extern const int kMaxUnsignalledRecvStreams; extern const int kMaxUnsignalledRecvStreams;
@@ -232,6 +234,7 @@ class WebRtcSession : public cricket::BaseSession,
// Called when an SSLIdentity is generated or retrieved by // Called when an SSLIdentity is generated or retrieved by
// WebRTCSessionDescriptionFactory. Should happen before setLocalDescription. // WebRTCSessionDescriptionFactory. Should happen before setLocalDescription.
void OnIdentityReady(rtc::SSLIdentity* identity); void OnIdentityReady(rtc::SSLIdentity* identity);
void OnDtlsSetupFailure(cricket::BaseChannel*, bool rtcp);
// For unit test. // For unit test.
bool waiting_for_identity() const; bool waiting_for_identity() const;

View File

@@ -722,27 +722,13 @@ void BaseChannel::ChannelWritable_w() {
// If we're doing DTLS-SRTP, now is the time. // If we're doing DTLS-SRTP, now is the time.
if (!was_ever_writable_ && ShouldSetupDtlsSrtp()) { if (!was_ever_writable_ && ShouldSetupDtlsSrtp()) {
if (!SetupDtlsSrtp(false)) { if (!SetupDtlsSrtp(false)) {
const std::string error_desc = SignalDtlsSetupFailure(this, false);
"Couldn't set up DTLS-SRTP on RTP channel.";
// Sent synchronously.
signaling_thread()->Invoke<void>(Bind(
&SetSessionError,
session_,
BaseSession::ERROR_TRANSPORT,
error_desc));
return; return;
} }
if (rtcp_transport_channel_) { if (rtcp_transport_channel_) {
if (!SetupDtlsSrtp(true)) { if (!SetupDtlsSrtp(true)) {
const std::string error_desc = SignalDtlsSetupFailure(this, true);
"Couldn't set up DTLS-SRTP on RTCP channel";
// Sent synchronously.
signaling_thread()->Invoke<void>(Bind(
&SetSessionError,
session_,
BaseSession::ERROR_TRANSPORT,
error_desc));
return; return;
} }
} }
@@ -753,6 +739,17 @@ void BaseChannel::ChannelWritable_w() {
ChangeState(); 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) { bool BaseChannel::SetDtlsSrtpCiphers(TransportChannel *tc, bool rtcp) {
std::vector<std::string> ciphers; std::vector<std::string> ciphers;
// We always use the default SRTP ciphers for RTCP, but we may use different // We always use the default SRTP ciphers for RTCP, but we may use different

View File

@@ -213,6 +213,10 @@ class BaseChannel
return remote_streams_; return remote_streams_;
} }
sigslot::signal2<BaseChannel*, bool> SignalDtlsSetupFailure;
void SignalDtlsSetupFailure_w(bool rtcp);
void SignalDtlsSetupFailure_s(bool rtcp);
// Used for latency measurements. // Used for latency measurements.
sigslot::signal1<BaseChannel*> SignalFirstPacketReceived; sigslot::signal1<BaseChannel*> SignalFirstPacketReceived;