Reland "Parameterize PeerConnection signaling tests for Unified Plan"

Original change's description:
> Parameterize PeerConnection signaling tests for Unified Plan
>
> This also changes the behavior of CreateAnswer to fail unless
> the signaling state is kHaveRemoteOffer or kHaveLocalPranswer,
> as per the WebRTC specification.
>
> Bug: webrtc:8765
> Change-Id: I60ac67cd92b17fcbff964afc14d049481e816a28
> Reviewed-on: https://webrtc-review.googlesource.com/41042
> Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
> Commit-Queue: Steve Anton <steveanton@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#21779}

Bug: webrtc:8813
Change-Id: I9f608fcd0b7aca00b4c1092e271dbd9cd710c38a
Reviewed-on: https://webrtc-review.googlesource.com/46861
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21860}
This commit is contained in:
Steve Anton
2018-02-01 11:32:31 -08:00
committed by Commit Bot
parent e831b8c94d
commit 7b581eb1ca
3 changed files with 83 additions and 80 deletions

View File

@ -1674,20 +1674,17 @@ 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 =
"CreateAnswer called when PeerConnection is in a wrong state.";
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) {
@ -3486,18 +3483,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

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());

View File

@ -55,12 +55,14 @@ class PeerConnectionWrapperForSignalingTest : public PeerConnectionWrapper {
}
};
class PeerConnectionSignalingTest : public ::testing::Test {
class PeerConnectionSignalingBaseTest : public ::testing::Test {
protected:
typedef std::unique_ptr<PeerConnectionWrapperForSignalingTest> WrapperPtr;
PeerConnectionSignalingTest()
: vss_(new rtc::VirtualSocketServer()), main_(vss_.get()) {
explicit PeerConnectionSignalingBaseTest(SdpSemantics sdp_semantics)
: vss_(new rtc::VirtualSocketServer()),
main_(vss_.get()),
sdp_semantics_(sdp_semantics) {
#ifdef WEBRTC_ANDROID
InitializeAndroidObjects();
#endif
@ -76,8 +78,10 @@ class PeerConnectionSignalingTest : public ::testing::Test {
WrapperPtr CreatePeerConnection(const RTCConfiguration& config) {
auto observer = rtc::MakeUnique<MockPeerConnectionObserver>();
auto pc = pc_factory_->CreatePeerConnection(config, nullptr, nullptr,
observer.get());
RTCConfiguration modified_config = config;
modified_config.sdp_semantics = sdp_semantics_;
auto pc = pc_factory_->CreatePeerConnection(modified_config, nullptr,
nullptr, observer.get());
if (!pc) {
return nullptr;
}
@ -102,16 +106,24 @@ class PeerConnectionSignalingTest : public ::testing::Test {
std::unique_ptr<rtc::VirtualSocketServer> vss_;
rtc::AutoSocketServerThread main_;
rtc::scoped_refptr<PeerConnectionFactoryInterface> pc_factory_;
const SdpSemantics sdp_semantics_;
};
TEST_F(PeerConnectionSignalingTest, SetLocalOfferTwiceWorks) {
class PeerConnectionSignalingTest
: public PeerConnectionSignalingBaseTest,
public ::testing::WithParamInterface<SdpSemantics> {
protected:
PeerConnectionSignalingTest() : PeerConnectionSignalingBaseTest(GetParam()) {}
};
TEST_P(PeerConnectionSignalingTest, SetLocalOfferTwiceWorks) {
auto caller = CreatePeerConnection();
EXPECT_TRUE(caller->SetLocalDescription(caller->CreateOffer()));
EXPECT_TRUE(caller->SetLocalDescription(caller->CreateOffer()));
}
TEST_F(PeerConnectionSignalingTest, SetRemoteOfferTwiceWorks) {
TEST_P(PeerConnectionSignalingTest, SetRemoteOfferTwiceWorks) {
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
@ -119,14 +131,14 @@ TEST_F(PeerConnectionSignalingTest, SetRemoteOfferTwiceWorks) {
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
}
TEST_F(PeerConnectionSignalingTest, FailToSetNullLocalDescription) {
TEST_P(PeerConnectionSignalingTest, FailToSetNullLocalDescription) {
auto caller = CreatePeerConnection();
std::string error;
ASSERT_FALSE(caller->SetLocalDescription(nullptr, &error));
EXPECT_EQ("SessionDescription is NULL.", error);
}
TEST_F(PeerConnectionSignalingTest, FailToSetNullRemoteDescription) {
TEST_P(PeerConnectionSignalingTest, FailToSetNullRemoteDescription) {
auto caller = CreatePeerConnection();
std::string error;
ASSERT_FALSE(caller->SetRemoteDescription(nullptr, &error));
@ -142,9 +154,15 @@ TEST_F(PeerConnectionSignalingTest, FailToSetNullRemoteDescription) {
// state the PeerConnection was created in before it was closed.
class PeerConnectionSignalingStateTest
: public PeerConnectionSignalingTest,
public ::testing::WithParamInterface<std::tuple<SignalingState, bool>> {
: public PeerConnectionSignalingBaseTest,
public ::testing::WithParamInterface<
std::tuple<SdpSemantics, SignalingState, bool>> {
protected:
PeerConnectionSignalingStateTest()
: PeerConnectionSignalingBaseTest(std::get<0>(GetParam())),
state_under_test_(std::make_tuple(std::get<1>(GetParam()),
std::get<2>(GetParam()))) {}
RTCConfiguration GetConfig() {
RTCConfiguration config;
config.certificates.push_back(
@ -152,6 +170,10 @@ class PeerConnectionSignalingStateTest
return config;
}
WrapperPtr CreatePeerConnectionUnderTest() {
return CreatePeerConnectionInState(state_under_test_);
}
WrapperPtr CreatePeerConnectionInState(SignalingState state) {
return CreatePeerConnectionInState(std::make_tuple(state, false));
}
@ -207,10 +229,12 @@ class PeerConnectionSignalingStateTest
return wrapper;
}
std::tuple<SignalingState, bool> state_under_test_;
};
TEST_P(PeerConnectionSignalingStateTest, CreateOffer) {
auto wrapper = CreatePeerConnectionInState(GetParam());
auto wrapper = CreatePeerConnectionUnderTest();
if (wrapper->signaling_state() != SignalingState::kClosed) {
EXPECT_TRUE(wrapper->CreateOffer());
} else {
@ -222,30 +246,21 @@ TEST_P(PeerConnectionSignalingStateTest, CreateOffer) {
}
TEST_P(PeerConnectionSignalingStateTest, CreateAnswer) {
auto wrapper = CreatePeerConnectionInState(GetParam());
auto wrapper = CreatePeerConnectionUnderTest();
if (wrapper->signaling_state() == SignalingState::kHaveLocalPrAnswer ||
wrapper->signaling_state() == SignalingState::kHaveRemoteOffer) {
EXPECT_TRUE(wrapper->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_PRED_FORMAT2(
AssertStartsWith, error,
"CreateAnswer called when PeerConnection is in a wrong state.");
}
}
TEST_P(PeerConnectionSignalingStateTest, SetLocalOffer) {
auto wrapper = CreatePeerConnectionInState(GetParam());
auto wrapper = CreatePeerConnectionUnderTest();
if (wrapper->signaling_state() == SignalingState::kStable ||
wrapper->signaling_state() == SignalingState::kHaveLocalOffer) {
// Need to call CreateOffer on the PeerConnection under test, otherwise when
@ -273,7 +288,7 @@ TEST_P(PeerConnectionSignalingStateTest, SetLocalPrAnswer) {
auto pranswer =
CloneSessionDescription(wrapper_for_pranswer->pc()->local_description());
auto wrapper = CreatePeerConnectionInState(GetParam());
auto wrapper = CreatePeerConnectionUnderTest();
if (wrapper->signaling_state() == SignalingState::kHaveLocalPrAnswer ||
wrapper->signaling_state() == SignalingState::kHaveRemoteOffer) {
EXPECT_TRUE(wrapper->SetLocalDescription(std::move(pranswer)));
@ -291,7 +306,7 @@ TEST_P(PeerConnectionSignalingStateTest, SetLocalAnswer) {
CreatePeerConnectionInState(SignalingState::kHaveRemoteOffer);
auto answer = wrapper_for_answer->CreateAnswer();
auto wrapper = CreatePeerConnectionInState(GetParam());
auto wrapper = CreatePeerConnectionUnderTest();
if (wrapper->signaling_state() == SignalingState::kHaveLocalPrAnswer ||
wrapper->signaling_state() == SignalingState::kHaveRemoteOffer) {
EXPECT_TRUE(wrapper->SetLocalDescription(std::move(answer)));
@ -310,7 +325,7 @@ TEST_P(PeerConnectionSignalingStateTest, SetRemoteOffer) {
auto offer =
CloneSessionDescription(wrapper_for_offer->pc()->remote_description());
auto wrapper = CreatePeerConnectionInState(GetParam());
auto wrapper = CreatePeerConnectionUnderTest();
if (wrapper->signaling_state() == SignalingState::kStable ||
wrapper->signaling_state() == SignalingState::kHaveRemoteOffer) {
EXPECT_TRUE(wrapper->SetRemoteDescription(std::move(offer)));
@ -329,7 +344,7 @@ TEST_P(PeerConnectionSignalingStateTest, SetRemotePrAnswer) {
auto pranswer =
CloneSessionDescription(wrapper_for_pranswer->pc()->remote_description());
auto wrapper = CreatePeerConnectionInState(GetParam());
auto wrapper = CreatePeerConnectionUnderTest();
if (wrapper->signaling_state() == SignalingState::kHaveLocalOffer ||
wrapper->signaling_state() == SignalingState::kHaveRemotePrAnswer) {
EXPECT_TRUE(wrapper->SetRemoteDescription(std::move(pranswer)));
@ -347,7 +362,7 @@ TEST_P(PeerConnectionSignalingStateTest, SetRemoteAnswer) {
CreatePeerConnectionInState(SignalingState::kHaveRemoteOffer);
auto answer = wrapper_for_answer->CreateAnswer();
auto wrapper = CreatePeerConnectionInState(GetParam());
auto wrapper = CreatePeerConnectionUnderTest();
if (wrapper->signaling_state() == SignalingState::kHaveLocalOffer ||
wrapper->signaling_state() == SignalingState::kHaveRemotePrAnswer) {
EXPECT_TRUE(wrapper->SetRemoteDescription(std::move(answer)));
@ -362,46 +377,35 @@ TEST_P(PeerConnectionSignalingStateTest, SetRemoteAnswer) {
INSTANTIATE_TEST_CASE_P(PeerConnectionSignalingTest,
PeerConnectionSignalingStateTest,
Combine(Values(SignalingState::kStable,
Combine(Values(SdpSemantics::kPlanB,
SdpSemantics::kUnifiedPlan),
Values(SignalingState::kStable,
SignalingState::kHaveLocalOffer,
SignalingState::kHaveLocalPrAnswer,
SignalingState::kHaveRemoteOffer,
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_P(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
// stays the same but the version must be incremented if a later, different
// session description is generated. These two tests verify that is the case for
// both offers and answers.
TEST_F(PeerConnectionSignalingTest,
TEST_P(PeerConnectionSignalingTest,
SessionVersionIncrementedInSubsequentDifferentOffer) {
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
@ -422,14 +426,14 @@ TEST_F(PeerConnectionSignalingTest,
EXPECT_LT(rtc::FromString<uint64_t>(original_version),
rtc::FromString<uint64_t>(later_offer->session_version()));
}
TEST_F(PeerConnectionSignalingTest,
TEST_P(PeerConnectionSignalingTest,
SessionVersionIncrementedInSubsequentDifferentAnswer) {
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
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();
@ -443,7 +447,7 @@ TEST_F(PeerConnectionSignalingTest,
rtc::FromString<uint64_t>(later_answer->session_version()));
}
TEST_F(PeerConnectionSignalingTest, InitiatorFlagSetOnCallerAndNotOnCallee) {
TEST_P(PeerConnectionSignalingTest, InitiatorFlagSetOnCallerAndNotOnCallee) {
auto caller = CreatePeerConnectionWithAudioVideo();
auto callee = CreatePeerConnectionWithAudioVideo();
@ -466,7 +470,7 @@ TEST_F(PeerConnectionSignalingTest, InitiatorFlagSetOnCallerAndNotOnCallee) {
// PeerConnection and make sure we get success/failure callbacks for all of the
// requests.
// Background: crbug.com/507307
TEST_F(PeerConnectionSignalingTest, CreateOffersAndShutdown) {
TEST_P(PeerConnectionSignalingTest, CreateOffersAndShutdown) {
auto caller = CreatePeerConnection();
RTCOfferAnswerOptions options;
@ -491,4 +495,9 @@ TEST_F(PeerConnectionSignalingTest, CreateOffersAndShutdown) {
}
}
INSTANTIATE_TEST_CASE_P(PeerConnectionSignalingTest,
PeerConnectionSignalingTest,
Values(SdpSemantics::kPlanB,
SdpSemantics::kUnifiedPlan));
} // namespace webrtc