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

Fix the Chromium crash when creating an answer without a remote description.

Browse Source 
Replacing the RTC_DCHECK with an if condition so that the method GetOptionsForAnswer
won't crash when there is no remote description. WebRtcSessionDescriptionFactory
will handle the null remote description.

TBR=deadbeef@webrtc.org
BUG=chromium:757830

Review-Url: https://codereview.webrtc.org/3006723002
Cr-Commit-Position: refs/heads/master@{#19589}
This commit is contained in:
zhihuang 2017-08-29 13:23:53 -07:00 committed by Commit Bot
parent 1dca9d513a
commit 141aacbf0b
2 changed files with 60 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
webrtc/pc/peerconnection.cc
View File

@ -1787,18 +1787,19 @@ void PeerConnection::GetOptionsForAnswer(
rtc::Optional<size_t> audio_index;
rtc::Optional<size_t> video_index;
rtc::Optional<size_t> data_index;
// There should be a pending remote description that's an offer...
RTC_DCHECK(session_->remote_description());
RTC_DCHECK(session_->remote_description()->type() ==
SessionDescriptionInterface::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(
session_->remote_description(),
cricket::RtpTransceiverDirection(send_audio, recv_audio),
cricket::RtpTransceiverDirection(send_video, recv_video), &audio_index,
&video_index, &data_index, session_options);
if (session_->remote_description()) {
// The pending remote description should be an offer.
RTC_DCHECK(session_->remote_description()->type() ==
SessionDescriptionInterface::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(
session_->remote_description(),
cricket::RtpTransceiverDirection(send_audio, recv_audio),
cricket::RtpTransceiverDirection(send_video, recv_video), &audio_index,
&video_index, &data_index, session_options);
}
cricket::MediaDescriptionOptions* audio_media_description_options =
!audio_index ? nullptr

81
webrtc/pc/peerconnectioninterface_unittest.cc
View File

@ -3771,6 +3771,53 @@ TEST_F(PeerConnectionInterfaceTest,
EXPECT_TRUE(video_content->rejected);
}
// This test ensures OnRenegotiationNeeded is called when we add track with
// MediaStream -> AddTrack in the same way it is called when we add track with
// PeerConnection -> AddTrack.
// The test can be removed once addStream is rewritten in terms of addTrack
// https://bugs.chromium.org/p/webrtc/issues/detail?id=7815
TEST_F(PeerConnectionInterfaceTest, MediaStreamAddTrackRemoveTrackRenegotiate) {
CreatePeerConnectionWithoutDtls();
rtc::scoped_refptr<MediaStreamInterface> stream(
pc_factory_->CreateLocalMediaStream(kStreamLabel1));
pc_->AddStream(stream);
rtc::scoped_refptr<AudioTrackInterface> audio_track(
pc_factory_->CreateAudioTrack("audio_track", nullptr));
rtc::scoped_refptr<VideoTrackInterface> video_track(
pc_factory_->CreateVideoTrack(
"video_track", pc_factory_->CreateVideoSource(
std::unique_ptr<cricket::VideoCapturer>(
new cricket::FakeVideoCapturer()))));
stream->AddTrack(audio_track);
EXPECT_TRUE_WAIT(observer_.renegotiation_needed_, kTimeout);
observer_.renegotiation_needed_ = false;
stream->AddTrack(video_track);
EXPECT_TRUE_WAIT(observer_.renegotiation_needed_, kTimeout);
observer_.renegotiation_needed_ = false;
stream->RemoveTrack(audio_track);
EXPECT_TRUE_WAIT(observer_.renegotiation_needed_, kTimeout);
observer_.renegotiation_needed_ = false;
stream->RemoveTrack(video_track);
EXPECT_TRUE_WAIT(observer_.renegotiation_needed_, kTimeout);
observer_.renegotiation_needed_ = false;
}
// Tests that creating answer would fail gracefully without being crashed if the
// remote description is unset.
TEST_F(PeerConnectionInterfaceTest, CreateAnswerWithoutRemoteDescription) {
CreatePeerConnection();
// Creating answer fails because the remote description is unset.
std::unique_ptr<SessionDescriptionInterface> answer;
EXPECT_FALSE(DoCreateAnswer(&answer, nullptr));
// Createing answer succeeds when the remote description is set.
CreateOfferAsRemoteDescription();
EXPECT_TRUE(DoCreateAnswer(&answer, nullptr));
}
class PeerConnectionMediaConfigTest : public testing::Test {
protected:
void SetUp() override {
@ -3891,37 +3938,3 @@ TEST(RTCConfigurationTest, ComparisonOperators) {
PeerConnectionInterface::RTCConfigurationType::kAggressive);
EXPECT_NE(a, h);
}
// This test ensures OnRenegotiationNeeded is called when we add track with
// MediaStream -> AddTrack in the same way it is called when we add track with
// PeerConnection -> AddTrack.
// The test can be removed once addStream is rewritten in terms of addTrack
// https://bugs.chromium.org/p/webrtc/issues/detail?id=7815
TEST_F(PeerConnectionInterfaceTest, MediaStreamAddTrackRemoveTrackRenegotiate) {
CreatePeerConnectionWithoutDtls();
rtc::scoped_refptr<MediaStreamInterface> stream(
pc_factory_->CreateLocalMediaStream(kStreamLabel1));
pc_->AddStream(stream);
rtc::scoped_refptr<AudioTrackInterface> audio_track(
pc_factory_->CreateAudioTrack("audio_track", nullptr));
rtc::scoped_refptr<VideoTrackInterface> video_track(
pc_factory_->CreateVideoTrack(
"video_track", pc_factory_->CreateVideoSource(
std::unique_ptr<cricket::VideoCapturer>(
new cricket::FakeVideoCapturer()))));
stream->AddTrack(audio_track);
EXPECT_TRUE_WAIT(observer_.renegotiation_needed_, kTimeout);
observer_.renegotiation_needed_ = false;
stream->AddTrack(video_track);
EXPECT_TRUE_WAIT(observer_.renegotiation_needed_, kTimeout);
observer_.renegotiation_needed_ = false;
stream->RemoveTrack(audio_track);
EXPECT_TRUE_WAIT(observer_.renegotiation_needed_, kTimeout);
observer_.renegotiation_needed_ = false;
stream->RemoveTrack(video_track);
EXPECT_TRUE_WAIT(observer_.renegotiation_needed_, kTimeout);
observer_.renegotiation_needed_ = false;
}