From 76d295231a251cc68008940fdc65c0a9fb742dc7 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Tue, 30 Jan 2018 14:43:29 +0100 Subject: [PATCH] Don't crash when sender info has been discarded by lower layers. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This happens when pc.close() is called. As a stopgap measure, we return zeroes instead, leading to stats being omitted. Bug: chromium:807174 Change-Id: I36f342adcd038822afb75d8593de808591eb9c4b Reviewed-on: https://webrtc-review.googlesource.com/46161 Reviewed-by: Henrik Boström Commit-Queue: Harald Alvestrand Cr-Commit-Position: refs/heads/master@{#21813} --- pc/rtcstatscollector.cc | 34 +++++++++++++++++++++++--------- pc/rtcstatscollector_unittest.cc | 17 ++++++++++++++++ 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/pc/rtcstatscollector.cc b/pc/rtcstatscollector.cc index bc0c3cf1e6..2e249e1ea2 100644 --- a/pc/rtcstatscollector.cc +++ b/pc/rtcstatscollector.cc @@ -521,12 +521,19 @@ void ProduceSenderMediaTrackStats( // to see from a sender whether it's connected or not. // Related to https://crbug.com/8694 (using ssrc 0 to indicate "none") if (sender->ssrc()) { - voice_sender_info = + // When pc.close is called, sender info is discarded, so + // we generate zeroes instead. Bug: It should be retained. + // https://crbug.com/807174 + auto sender_info = track_media_info_map.GetVoiceSenderInfoBySsrc(sender->ssrc()); + if (sender_info) { + voice_sender_info = sender_info; + } else { + RTC_LOG(LS_INFO) + << "RTCStatsCollector: No voice sender info for sender with ssrc " + << sender->ssrc(); + } } - - RTC_CHECK(voice_sender_info) - << "No voice sender info for sender with ssrc " << sender->ssrc(); std::unique_ptr audio_track_stats = ProduceMediaStreamTrackStatsFromVoiceSenderInfo( timestamp_us, *track, *voice_sender_info, sender->AttachmentId()); @@ -539,12 +546,21 @@ void ProduceSenderMediaTrackStats( cricket::VideoSenderInfo null_sender_info; const cricket::VideoSenderInfo* video_sender_info = &null_sender_info; // TODO(hta): Check on state not ssrc when state is available - // Related to https://crbug.com/8694 (using ssrc 0 to indicate "none") - if (sender->ssrc()) - video_sender_info = + // Related to https://bugs.webrtc.org/8694 (using ssrc 0 to indicate + // "none") + if (sender->ssrc()) { + // When pc.close is called, sender info is discarded, so + // we generate zeroes instead. Bug: It should be retained. + // https://crbug.com/807174 + auto sender_info = track_media_info_map.GetVideoSenderInfoBySsrc(sender->ssrc()); - RTC_CHECK(video_sender_info) - << "No video sender info for sender with ssrc " << sender->ssrc(); + if (sender_info) { + video_sender_info = sender_info; + } else { + RTC_LOG(LS_INFO) << "No video sender info for sender with ssrc " + << sender->ssrc(); + } + } std::unique_ptr video_track_stats = ProduceMediaStreamTrackStatsFromVideoSenderInfo( timestamp_us, *track, *video_sender_info, sender->AttachmentId()); diff --git a/pc/rtcstatscollector_unittest.cc b/pc/rtcstatscollector_unittest.cc index e9adc3ddf5..8c37b77fce 100644 --- a/pc/rtcstatscollector_unittest.cc +++ b/pc/rtcstatscollector_unittest.cc @@ -2413,6 +2413,23 @@ TEST_F(RTCStatsCollectorTest, StatsReportedOnZeroSsrc) { EXPECT_EQ(0, rtp_stream_stats.size()); } +TEST_F(RTCStatsCollectorTest, DoNotCrashOnSsrcChange) { + rtc::scoped_refptr track = + CreateFakeTrack(cricket::MEDIA_TYPE_AUDIO, "audioTrack", + MediaStreamTrackInterface::kLive); + rtc::scoped_refptr sender = + CreateMockSender(track, 4711, 49, {}); + EXPECT_CALL(test_->pc(), GetSenders()) + .WillRepeatedly( + Return(std::vector>( + {rtc::scoped_refptr(sender.get())}))); + // We do not generate any matching voice_sender_info stats. + rtc::scoped_refptr report = GetStatsReport(); + std::vector track_stats = + report->GetStatsOfType(); + EXPECT_EQ(1, track_stats.size()); +} + class RTCStatsCollectorTestWithFakeCollector : public testing::Test { public: RTCStatsCollectorTestWithFakeCollector()