Fix the Chromium crash when creating an answer without a remote description.
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-Original-Commit-Position: refs/heads/master@{#19589}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 141aacbf0b2df237e3e509ead278b770a2a04ad7
diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
index 6112357..e22a09e 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -1787,18 +1787,19 @@
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
diff --git a/pc/peerconnectioninterface_unittest.cc b/pc/peerconnectioninterface_unittest.cc
index ead950c..c23a79d 100644
--- a/pc/peerconnectioninterface_unittest.cc
+++ b/pc/peerconnectioninterface_unittest.cc
@@ -3771,6 +3771,53 @@
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 @@
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;
-}