admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fail CreateAnswer if signaling state is not correct

Browse Source 
This changes CreateAnswer to become compliant with the WebRTC 1.0
specification which details that createAnswer should fail if the
PeerConnection is in a state other than 'have-remote-offer' or
'have-local-pranswer'.

Bug: webrtc:8813
Change-Id: I7ca41bdebda1ea163aec8815267c1bbfd7d6d11e
Reviewed-on: https://webrtc-review.googlesource.com/47581
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21923}
This commit is contained in:
Steve Anton 2018-02-06 10:31:29 -08:00 committed by Commit Bot
parent 2870b0a57e
commit dffead8835
3 changed files with 29 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
pc/peerconnection.cc
View File

@ -1679,20 +1679,18 @@ void PeerConnection::CreateAnswer(CreateSessionDescriptionObserver* observer,
return;
}
if (IsClosed()) {
std::string error = "CreateAnswer called when PeerConnection is closed.";
if (!(signaling_state_ == kHaveRemoteOffer ||
signaling_state_ == kHaveLocalPrAnswer)) {
std::string error =
"PeerConnection cannot create an answer in a state other than "
"have-remote-offer or have-local-pranswer.";
RTC_LOG(LS_ERROR) << error;
PostCreateSessionDescriptionFailure(observer, error);
return;
}
if (remote_description() &&
remote_description()->GetType() != SdpType::kOffer) {
std::string error = "CreateAnswer called without remote offer.";
RTC_LOG(LS_ERROR) << error;
PostCreateSessionDescriptionFailure(observer, error);
return;
}
// The remote description should be set if we're in the right state.
RTC_DCHECK(remote_description());
if (IsUnifiedPlan()) {
if (options.offer_to_receive_audio != RTCOfferAnswerOptions::kUndefined) {
@ -3499,18 +3497,15 @@ void PeerConnection::GetOptionsForPlanBAnswer(
rtc::Optional<size_t> audio_index;
rtc::Optional<size_t> video_index;
rtc::Optional<size_t> data_index;
if (remote_description()) {
// The pending remote description should be an offer.
RTC_DCHECK(remote_description()->GetType() == SdpType::kOffer);
// Generate m= sections that match those in the offer.
// Note that mediasession.cc will handle intersection our preferred
// direction with the offered direction.
GenerateMediaDescriptionOptions(
remote_description(),
RtpTransceiverDirectionFromSendRecv(send_audio, recv_audio),
RtpTransceiverDirectionFromSendRecv(send_video, recv_video),
&audio_index, &video_index, &data_index, session_options);
}
// Generate m= sections that match those in the offer.
// Note that mediasession.cc will handle intersection our preferred
// direction with the offered direction.
GenerateMediaDescriptionOptions(
remote_description(),
RtpTransceiverDirectionFromSendRecv(send_audio, recv_audio),
RtpTransceiverDirectionFromSendRecv(send_video, recv_video), &audio_index,
&video_index, &data_index, session_options);
cricket::MediaDescriptionOptions* audio_media_description_options =
!audio_index ? nullptr

2
pc/peerconnection_ice_unittest.cc
View File

@ -303,7 +303,7 @@ TEST_P(PeerConnectionIceTest, AnswerContainsGatheredCandidates) {
EXPECT_TRUE_WAIT(callee->IsIceGatheringDone(), kIceCandidatesTimeout);
auto answer = callee->CreateAnswer();
auto* answer = callee->pc()->local_description();
EXPECT_LT(0u, caller->observer()->GetCandidatesByMline(0).size());
EXPECT_EQ(callee->observer()->GetCandidatesByMline(0).size(),
answer->candidates(0)->count());

46
pc/peerconnection_signaling_unittest.cc
View File

@ -229,18 +229,9 @@ TEST_P(PeerConnectionSignalingStateTest, CreateAnswer) {
} else {
std::string error;
ASSERT_FALSE(wrapper->CreateAnswer(RTCOfferAnswerOptions(), &error));
if (wrapper->signaling_state() == SignalingState::kClosed) {
EXPECT_PRED_FORMAT2(AssertStartsWith, error,
"CreateAnswer called when PeerConnection is closed.");
} else if (wrapper->signaling_state() ==
SignalingState::kHaveRemotePrAnswer) {
EXPECT_PRED_FORMAT2(AssertStartsWith, error,
"CreateAnswer called without remote offer.");
} else {
EXPECT_PRED_FORMAT2(
AssertStartsWith, error,
"CreateAnswer can't be called before SetRemoteDescription.");
}
EXPECT_EQ(error,
"PeerConnection cannot create an answer in a state other than "
"have-remote-offer or have-local-pranswer.");
}
}
@ -369,32 +360,19 @@ INSTANTIATE_TEST_CASE_P(PeerConnectionSignalingTest,
SignalingState::kHaveRemotePrAnswer),
Bool()));
TEST_F(PeerConnectionSignalingTest,
CreateAnswerSucceedsIfStableAndRemoteDescriptionIsOffer) {
// Test that CreateAnswer fails if a round of offer/answer has been done and
// the PeerConnection is in the stable state.
TEST_F(PeerConnectionSignalingTest, CreateAnswerFailsIfStable) {
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
ASSERT_TRUE(
caller->SetRemoteDescription(callee->CreateAnswerAndSetAsLocal()));
ASSERT_EQ(SignalingState::kStable, callee->signaling_state());
EXPECT_TRUE(callee->CreateAnswer());
}
TEST_F(PeerConnectionSignalingTest,
CreateAnswerFailsIfStableButRemoteDescriptionIsAnswer) {
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
ASSERT_TRUE(
caller->SetRemoteDescription(callee->CreateAnswerAndSetAsLocal()));
ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
ASSERT_EQ(SignalingState::kStable, caller->signaling_state());
std::string error;
ASSERT_FALSE(caller->CreateAnswer(RTCOfferAnswerOptions(), &error));
EXPECT_EQ("CreateAnswer called without remote offer.", error);
EXPECT_FALSE(caller->CreateAnswer());
ASSERT_EQ(SignalingState::kStable, callee->signaling_state());
EXPECT_FALSE(callee->CreateAnswer());
}
// According to https://tools.ietf.org/html/rfc3264#section-8, the session id
@ -429,7 +407,7 @@ TEST_F(PeerConnectionSignalingTest,
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
auto original_answer = callee->CreateAnswerAndSetAsLocal();
auto original_answer = callee->CreateAnswer();
const std::string original_id = original_answer->session_id();
const std::string original_version = original_answer->session_version();