Remove BaseSession::SignalNewDescription. It was only used by GTP and now just clutters the code.

R=pbos@webrtc.org

Review URL: https://codereview.webrtc.org/1228203002 .

Cr-Commit-Position: refs/heads/master@{#9564}
This commit is contained in:
Peter Thatcher
2015-07-09 21:26:36 -07:00
parent 4d9d097217
commit a6d2444c84
5 changed files with 64 additions and 182 deletions

View File

@ -258,19 +258,6 @@ class WebRtcSession : public cricket::BaseSession,
metrics_observer_ = metrics_observer; metrics_observer_ = metrics_observer;
} }
protected:
// Don't fire a new description. The only thing it's used for is to
// push new media descriptions to the BaseChannels. But in
// WebRtcSession, we just push to the BaseChannels directly, so we
// don't need this (and it would cause the descriptions to be pushed
// down twice).
// TODO(pthatcher): Remove this method and signal completely from
// BaseSession once all the subclasses of BaseSession push to
// BaseChannels directly rather than relying on the signal, or once
// BaseChannel no longer listens to the event and requires
// descriptions to be pushed down.
virtual void SignalNewDescription() override {}
private: private:
// Indicates the type of SessionDescription in a call to SetLocalDescription // Indicates the type of SessionDescription in a call to SetLocalDescription
// and SetRemoteDescription. // and SetRemoteDescription.

View File

@ -204,11 +204,6 @@ bool BaseChannel::Init() {
return false; return false;
} }
session_->SignalNewLocalDescription.connect(
this, &BaseChannel::OnNewLocalDescription);
session_->SignalNewRemoteDescription.connect(
this, &BaseChannel::OnNewRemoteDescription);
// Both RTP and RTCP channels are set, we can call SetInterface on // Both RTP and RTCP channels are set, we can call SetInterface on
// media channel and it can set network options. // media channel and it can set network options.
media_channel_->SetInterface(this); media_channel_->SetInterface(this);
@ -663,24 +658,6 @@ void BaseChannel::HandlePacket(bool rtcp, rtc::Buffer* packet,
} }
} }
void BaseChannel::OnNewLocalDescription(
BaseSession* session, ContentAction action) {
std::string error_desc;
if (!PushdownLocalDescription(
session->local_description(), action, &error_desc)) {
SetSessionError(session_, BaseSession::ERROR_CONTENT, error_desc);
}
}
void BaseChannel::OnNewRemoteDescription(
BaseSession* session, ContentAction action) {
std::string error_desc;
if (!PushdownRemoteDescription(
session->remote_description(), action, &error_desc)) {
SetSessionError(session_, BaseSession::ERROR_CONTENT, error_desc);
}
}
bool BaseChannel::PushdownLocalDescription( bool BaseChannel::PushdownLocalDescription(
const SessionDescription* local_desc, ContentAction action, const SessionDescription* local_desc, ContentAction action,
std::string* error_desc) { std::string* error_desc) {

View File

@ -1559,62 +1559,42 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> {
void TestSetContentFailure() { void TestSetContentFailure() {
CreateChannels(0, 0); CreateChannels(0, 0);
typename T::Content content;
cricket::SessionDescription* sdesc_loc = new cricket::SessionDescription();
cricket::SessionDescription* sdesc_rem = new cricket::SessionDescription();
// Set up the session description. auto sdesc = cricket::SessionDescription();
CreateContent(0, kPcmuCodec, kH264Codec, &content); sdesc.AddContent(cricket::CN_AUDIO, cricket::NS_JINGLE_RTP,
sdesc_loc->AddContent(cricket::CN_AUDIO, cricket::NS_JINGLE_RTP,
new cricket::AudioContentDescription()); new cricket::AudioContentDescription());
sdesc_loc->AddContent(cricket::CN_VIDEO, cricket::NS_JINGLE_RTP, sdesc.AddContent(cricket::CN_VIDEO, cricket::NS_JINGLE_RTP,
new cricket::VideoContentDescription()); new cricket::VideoContentDescription());
session1_.set_local_description(sdesc_loc);
sdesc_rem->AddContent(cricket::CN_AUDIO, cricket::NS_JINGLE_RTP,
new cricket::AudioContentDescription());
sdesc_rem->AddContent(cricket::CN_VIDEO, cricket::NS_JINGLE_RTP,
new cricket::VideoContentDescription());
session1_.set_remote_description(sdesc_rem);
// Test failures in SetLocalContent. std::string err;
media_channel1_->set_fail_set_recv_codecs(true); media_channel1_->set_fail_set_recv_codecs(true);
session1_.SetError(cricket::BaseSession::ERROR_NONE, ""); EXPECT_FALSE(channel1_->PushdownLocalDescription(
session1_.SetState(cricket::BaseSession::STATE_SENTINITIATE); &sdesc, cricket::CA_OFFER, &err));
EXPECT_EQ(cricket::BaseSession::ERROR_CONTENT, session1_.error()); EXPECT_FALSE(channel1_->PushdownLocalDescription(
media_channel1_->set_fail_set_recv_codecs(true); &sdesc, cricket::CA_ANSWER, &err));
session1_.SetError(cricket::BaseSession::ERROR_NONE, "");
session1_.SetState(cricket::BaseSession::STATE_SENTACCEPT);
EXPECT_EQ(cricket::BaseSession::ERROR_CONTENT, session1_.error());
// Test failures in SetRemoteContent.
media_channel1_->set_fail_set_send_codecs(true); media_channel1_->set_fail_set_send_codecs(true);
session1_.SetError(cricket::BaseSession::ERROR_NONE, ""); EXPECT_FALSE(channel1_->PushdownRemoteDescription(
session1_.SetState(cricket::BaseSession::STATE_RECEIVEDINITIATE); &sdesc, cricket::CA_OFFER, &err));
EXPECT_EQ(cricket::BaseSession::ERROR_CONTENT, session1_.error());
media_channel1_->set_fail_set_send_codecs(true); media_channel1_->set_fail_set_send_codecs(true);
session1_.SetError(cricket::BaseSession::ERROR_NONE, ""); EXPECT_FALSE(channel1_->PushdownRemoteDescription(
session1_.SetState(cricket::BaseSession::STATE_RECEIVEDACCEPT); &sdesc, cricket::CA_ANSWER, &err));
EXPECT_EQ(cricket::BaseSession::ERROR_CONTENT, session1_.error());
} }
void TestSendTwoOffers() { void TestSendTwoOffers() {
CreateChannels(0, 0); CreateChannels(0, 0);
// Set up the initial session description. std::string err;
cricket::SessionDescription* sdesc = CreateSessionDescriptionWithStream(1); rtc::scoped_ptr<cricket::SessionDescription> sdesc1(
session1_.set_local_description(sdesc); CreateSessionDescriptionWithStream(1));
EXPECT_TRUE(channel1_->PushdownLocalDescription(
session1_.SetError(cricket::BaseSession::ERROR_NONE, ""); sdesc1.get(), cricket::CA_OFFER, &err));
session1_.SetState(cricket::BaseSession::STATE_SENTINITIATE);
EXPECT_EQ(cricket::BaseSession::ERROR_NONE, session1_.error());
EXPECT_TRUE(media_channel1_->HasSendStream(1)); EXPECT_TRUE(media_channel1_->HasSendStream(1));
// Update the local description and set the state again. rtc::scoped_ptr<cricket::SessionDescription> sdesc2(
sdesc = CreateSessionDescriptionWithStream(2); CreateSessionDescriptionWithStream(2));
session1_.set_local_description(sdesc); EXPECT_TRUE(channel1_->PushdownLocalDescription(
sdesc2.get(), cricket::CA_OFFER, &err));
session1_.SetState(cricket::BaseSession::STATE_SENTINITIATE);
EXPECT_EQ(cricket::BaseSession::ERROR_NONE, session1_.error());
EXPECT_FALSE(media_channel1_->HasSendStream(1)); EXPECT_FALSE(media_channel1_->HasSendStream(1));
EXPECT_TRUE(media_channel1_->HasSendStream(2)); EXPECT_TRUE(media_channel1_->HasSendStream(2));
} }
@ -1622,19 +1602,17 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> {
void TestReceiveTwoOffers() { void TestReceiveTwoOffers() {
CreateChannels(0, 0); CreateChannels(0, 0);
// Set up the initial session description. std::string err;
cricket::SessionDescription* sdesc = CreateSessionDescriptionWithStream(1); rtc::scoped_ptr<cricket::SessionDescription> sdesc1(
session1_.set_remote_description(sdesc); CreateSessionDescriptionWithStream(1));
EXPECT_TRUE(channel1_->PushdownRemoteDescription(
session1_.SetError(cricket::BaseSession::ERROR_NONE, ""); sdesc1.get(), cricket::CA_OFFER, &err));
session1_.SetState(cricket::BaseSession::STATE_RECEIVEDINITIATE);
EXPECT_EQ(cricket::BaseSession::ERROR_NONE, session1_.error());
EXPECT_TRUE(media_channel1_->HasRecvStream(1)); EXPECT_TRUE(media_channel1_->HasRecvStream(1));
sdesc = CreateSessionDescriptionWithStream(2); rtc::scoped_ptr<cricket::SessionDescription> sdesc2(
session1_.set_remote_description(sdesc); CreateSessionDescriptionWithStream(2));
session1_.SetState(cricket::BaseSession::STATE_RECEIVEDINITIATE); EXPECT_TRUE(channel1_->PushdownRemoteDescription(
EXPECT_EQ(cricket::BaseSession::ERROR_NONE, session1_.error()); sdesc2.get(), cricket::CA_OFFER, &err));
EXPECT_FALSE(media_channel1_->HasRecvStream(1)); EXPECT_FALSE(media_channel1_->HasRecvStream(1));
EXPECT_TRUE(media_channel1_->HasRecvStream(2)); EXPECT_TRUE(media_channel1_->HasRecvStream(2));
} }
@ -1642,30 +1620,27 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> {
void TestSendPrAnswer() { void TestSendPrAnswer() {
CreateChannels(0, 0); CreateChannels(0, 0);
// Set up the initial session description. std::string err;
cricket::SessionDescription* sdesc = CreateSessionDescriptionWithStream(1); // Receive offer
session1_.set_remote_description(sdesc); rtc::scoped_ptr<cricket::SessionDescription> sdesc1(
CreateSessionDescriptionWithStream(1));
session1_.SetError(cricket::BaseSession::ERROR_NONE, ""); EXPECT_TRUE(channel1_->PushdownRemoteDescription(
session1_.SetState(cricket::BaseSession::STATE_RECEIVEDINITIATE); sdesc1.get(), cricket::CA_OFFER, &err));
EXPECT_EQ(cricket::BaseSession::ERROR_NONE, session1_.error());
EXPECT_TRUE(media_channel1_->HasRecvStream(1)); EXPECT_TRUE(media_channel1_->HasRecvStream(1));
// Send PRANSWER // Send PR answer
sdesc = CreateSessionDescriptionWithStream(2); rtc::scoped_ptr<cricket::SessionDescription> sdesc2(
session1_.set_local_description(sdesc); CreateSessionDescriptionWithStream(2));
EXPECT_TRUE(channel1_->PushdownLocalDescription(
session1_.SetState(cricket::BaseSession::STATE_SENTPRACCEPT); sdesc2.get(), cricket::CA_PRANSWER, &err));
EXPECT_EQ(cricket::BaseSession::ERROR_NONE, session1_.error());
EXPECT_TRUE(media_channel1_->HasRecvStream(1)); EXPECT_TRUE(media_channel1_->HasRecvStream(1));
EXPECT_TRUE(media_channel1_->HasSendStream(2)); EXPECT_TRUE(media_channel1_->HasSendStream(2));
// Send ACCEPT // Send answer
sdesc = CreateSessionDescriptionWithStream(3); rtc::scoped_ptr<cricket::SessionDescription> sdesc3(
session1_.set_local_description(sdesc); CreateSessionDescriptionWithStream(3));
EXPECT_TRUE(channel1_->PushdownLocalDescription(
session1_.SetState(cricket::BaseSession::STATE_SENTACCEPT); sdesc3.get(), cricket::CA_ANSWER, &err));
EXPECT_EQ(cricket::BaseSession::ERROR_NONE, session1_.error());
EXPECT_TRUE(media_channel1_->HasRecvStream(1)); EXPECT_TRUE(media_channel1_->HasRecvStream(1));
EXPECT_FALSE(media_channel1_->HasSendStream(2)); EXPECT_FALSE(media_channel1_->HasSendStream(2));
EXPECT_TRUE(media_channel1_->HasSendStream(3)); EXPECT_TRUE(media_channel1_->HasSendStream(3));
@ -1674,30 +1649,27 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> {
void TestReceivePrAnswer() { void TestReceivePrAnswer() {
CreateChannels(0, 0); CreateChannels(0, 0);
// Set up the initial session description. std::string err;
cricket::SessionDescription* sdesc = CreateSessionDescriptionWithStream(1); // Send offer
session1_.set_local_description(sdesc); rtc::scoped_ptr<cricket::SessionDescription> sdesc1(
CreateSessionDescriptionWithStream(1));
session1_.SetError(cricket::BaseSession::ERROR_NONE, ""); EXPECT_TRUE(channel1_->PushdownLocalDescription(
session1_.SetState(cricket::BaseSession::STATE_SENTINITIATE); sdesc1.get(), cricket::CA_OFFER, &err));
EXPECT_EQ(cricket::BaseSession::ERROR_NONE, session1_.error());
EXPECT_TRUE(media_channel1_->HasSendStream(1)); EXPECT_TRUE(media_channel1_->HasSendStream(1));
// Receive PRANSWER // Receive PR answer
sdesc = CreateSessionDescriptionWithStream(2); rtc::scoped_ptr<cricket::SessionDescription> sdesc2(
session1_.set_remote_description(sdesc); CreateSessionDescriptionWithStream(2));
EXPECT_TRUE(channel1_->PushdownRemoteDescription(
session1_.SetState(cricket::BaseSession::STATE_RECEIVEDPRACCEPT); sdesc2.get(), cricket::CA_PRANSWER, &err));
EXPECT_EQ(cricket::BaseSession::ERROR_NONE, session1_.error());
EXPECT_TRUE(media_channel1_->HasSendStream(1)); EXPECT_TRUE(media_channel1_->HasSendStream(1));
EXPECT_TRUE(media_channel1_->HasRecvStream(2)); EXPECT_TRUE(media_channel1_->HasRecvStream(2));
// Receive ACCEPT // Receive answer
sdesc = CreateSessionDescriptionWithStream(3); rtc::scoped_ptr<cricket::SessionDescription> sdesc3(
session1_.set_remote_description(sdesc); CreateSessionDescriptionWithStream(3));
EXPECT_TRUE(channel1_->PushdownRemoteDescription(
session1_.SetState(cricket::BaseSession::STATE_RECEIVEDACCEPT); sdesc3.get(), cricket::CA_ANSWER, &err));
EXPECT_EQ(cricket::BaseSession::ERROR_NONE, session1_.error());
EXPECT_TRUE(media_channel1_->HasSendStream(1)); EXPECT_TRUE(media_channel1_->HasSendStream(1));
EXPECT_FALSE(media_channel1_->HasRecvStream(2)); EXPECT_FALSE(media_channel1_->HasRecvStream(2));
EXPECT_TRUE(media_channel1_->HasRecvStream(3)); EXPECT_TRUE(media_channel1_->HasRecvStream(3));

View File

@ -582,7 +582,6 @@ void BaseSession::SetState(State state) {
SignalState(this, state_); SignalState(this, state_);
signaling_thread_->Post(this, MSG_STATE); signaling_thread_->Post(this, MSG_STATE);
} }
SignalNewDescription();
} }
void BaseSession::SetError(Error error, const std::string& error_desc) { void BaseSession::SetError(Error error, const std::string& error_desc) {
@ -787,54 +786,6 @@ bool BaseSession::GetTransportDescription(const SessionDescription* description,
return true; return true;
} }
void BaseSession::SignalNewDescription() {
ContentAction action;
ContentSource source;
if (!GetContentAction(&action, &source)) {
return;
}
if (source == CS_LOCAL) {
SignalNewLocalDescription(this, action);
} else {
SignalNewRemoteDescription(this, action);
}
}
bool BaseSession::GetContentAction(ContentAction* action,
ContentSource* source) {
switch (state_) {
// new local description
case STATE_SENTINITIATE:
*action = CA_OFFER;
*source = CS_LOCAL;
break;
case STATE_SENTPRACCEPT:
*action = CA_PRANSWER;
*source = CS_LOCAL;
break;
case STATE_SENTACCEPT:
*action = CA_ANSWER;
*source = CS_LOCAL;
break;
// new remote description
case STATE_RECEIVEDINITIATE:
*action = CA_OFFER;
*source = CS_REMOTE;
break;
case STATE_RECEIVEDPRACCEPT:
*action = CA_PRANSWER;
*source = CS_REMOTE;
break;
case STATE_RECEIVEDACCEPT:
*action = CA_ANSWER;
*source = CS_REMOTE;
break;
default:
return false;
}
return true;
}
void BaseSession::OnMessage(rtc::Message *pmsg) { void BaseSession::OnMessage(rtc::Message *pmsg) {
switch (pmsg->message_id) { switch (pmsg->message_id) {
case MSG_TIMEOUT: case MSG_TIMEOUT:

View File

@ -412,8 +412,6 @@ class BaseSession : public sigslot::has_slots<>,
Error error_; Error error_;
std::string error_desc_; std::string error_desc_;
// Fires the new description signal according to the current state.
virtual void SignalNewDescription();
// This method will delete the Transport and TransportChannelImpls // This method will delete the Transport and TransportChannelImpls
// and replace those with the Transport object of the first // and replace those with the Transport object of the first
// MediaContent in bundle_group. // MediaContent in bundle_group.
@ -439,9 +437,6 @@ class BaseSession : public sigslot::has_slots<>,
const std::string& content_name, const std::string& content_name,
TransportDescription* info); TransportDescription* info);
// Gets the ContentAction and ContentSource according to the session state.
bool GetContentAction(ContentAction* action, ContentSource* source);
rtc::Thread* const signaling_thread_; rtc::Thread* const signaling_thread_;
rtc::Thread* const worker_thread_; rtc::Thread* const worker_thread_;
PortAllocator* const port_allocator_; PortAllocator* const port_allocator_;