From 7666db79fa269c6688651008edd8cf88276c0671 Mon Sep 17 00:00:00 2001 From: "henrike@webrtc.org" Date: Thu, 22 Aug 2013 14:45:42 +0000 Subject: [PATCH] Update talk to 51242664. TBR=wu@webrtc.org Review URL: https://webrtc-codereview.appspot.com/2090005 git-svn-id: http://webrtc.googlecode.com/svn/trunk@4594 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/app/webrtc/mediastreamsignaling.cc | 28 ++++++++--- .../webrtc/mediastreamsignaling_unittest.cc | 49 +++++++++++++++++++ talk/app/webrtc/webrtcsession_unittest.cc | 20 +++----- talk/media/webrtc/fakewebrtcvideoengine.h | 3 ++ .../webrtc/webrtcvoiceengine_unittest.cc | 2 +- 5 files changed, 79 insertions(+), 23 deletions(-) diff --git a/talk/app/webrtc/mediastreamsignaling.cc b/talk/app/webrtc/mediastreamsignaling.cc index c199f1e9c7..4996983969 100644 --- a/talk/app/webrtc/mediastreamsignaling.cc +++ b/talk/app/webrtc/mediastreamsignaling.cc @@ -599,15 +599,19 @@ void MediaStreamSignaling::OnRemoteTrackRemoved( if (media_type == cricket::MEDIA_TYPE_AUDIO) { talk_base::scoped_refptr audio_track = stream->FindAudioTrack(track_id); - audio_track->set_state(webrtc::MediaStreamTrackInterface::kEnded); - stream->RemoveTrack(audio_track); - stream_observer_->OnRemoveRemoteAudioTrack(stream, audio_track); + if (audio_track) { + audio_track->set_state(webrtc::MediaStreamTrackInterface::kEnded); + stream->RemoveTrack(audio_track); + stream_observer_->OnRemoveRemoteAudioTrack(stream, audio_track); + } } else if (media_type == cricket::MEDIA_TYPE_VIDEO) { talk_base::scoped_refptr video_track = stream->FindVideoTrack(track_id); - video_track->set_state(webrtc::MediaStreamTrackInterface::kEnded); - stream->RemoveTrack(video_track); - stream_observer_->OnRemoveRemoteVideoTrack(stream, video_track); + if (video_track) { + video_track->set_state(webrtc::MediaStreamTrackInterface::kEnded); + stream->RemoveTrack(video_track); + stream_observer_->OnRemoveRemoteVideoTrack(stream, video_track); + } } else { ASSERT(false && "Invalid media type"); } @@ -621,11 +625,19 @@ void MediaStreamSignaling::RejectRemoteTracks(cricket::MediaType media_type) { MediaStreamInterface* stream = remote_streams_->find(info.stream_label); if (media_type == cricket::MEDIA_TYPE_AUDIO) { AudioTrackInterface* track = stream->FindAudioTrack(info.track_id); - track->set_state(webrtc::MediaStreamTrackInterface::kEnded); + // There's no guarantee the track is still available, e.g. the track may + // have been removed from the stream by javascript. + if (track) { + track->set_state(webrtc::MediaStreamTrackInterface::kEnded); + } } if (media_type == cricket::MEDIA_TYPE_VIDEO) { VideoTrackInterface* track = stream->FindVideoTrack(info.track_id); - track->set_state(webrtc::MediaStreamTrackInterface::kEnded); + // There's no guarantee the track is still available, e.g. the track may + // have been removed from the stream by javascript. + if (track) { + track->set_state(webrtc::MediaStreamTrackInterface::kEnded); + } } } } diff --git a/talk/app/webrtc/mediastreamsignaling_unittest.cc b/talk/app/webrtc/mediastreamsignaling_unittest.cc index 8b3dacb52f..d2dc19e844 100644 --- a/talk/app/webrtc/mediastreamsignaling_unittest.cc +++ b/talk/app/webrtc/mediastreamsignaling_unittest.cc @@ -778,6 +778,33 @@ TEST_F(MediaStreamSignalingTest, RejectMediaContent) { EXPECT_EQ(webrtc::MediaStreamTrackInterface::kEnded, remote_audio->state()); } +// This test that it won't crash if the remote track as been removed outside +// of MediaStreamSignaling and then MediaStreamSignaling tries to reject +// this track. +TEST_F(MediaStreamSignalingTest, RemoveTrackThenRejectMediaContent) { + talk_base::scoped_ptr desc( + webrtc::CreateSessionDescription(SessionDescriptionInterface::kOffer, + kSdpStringWithStream1, NULL)); + EXPECT_TRUE(desc != NULL); + signaling_->OnRemoteDescriptionChanged(desc.get()); + + MediaStreamInterface* remote_stream = observer_->remote_streams()->at(0); + remote_stream->RemoveTrack(remote_stream->GetVideoTracks()[0]); + remote_stream->RemoveTrack(remote_stream->GetAudioTracks()[0]); + + cricket::ContentInfo* video_info = + desc->description()->GetContentByName("video"); + video_info->rejected = true; + signaling_->OnLocalDescriptionChanged(desc.get()); + + cricket::ContentInfo* audio_info = + desc->description()->GetContentByName("audio"); + audio_info->rejected = true; + signaling_->OnLocalDescriptionChanged(desc.get()); + + // No crash is a pass. +} + // This tests that a default MediaStream is created if a remote session // description doesn't contain any streams and no MSID support. // It also tests that the default stream is updated if a video m-line is added @@ -812,6 +839,28 @@ TEST_F(MediaStreamSignalingTest, SdpWithoutMsidCreatesDefaultStream) { observer_->VerifyRemoteVideoTrack("default", "defaultv0", 0); } +// This tests that it won't crash when MediaStreamSignaling tries to remove +// a remote track that as already been removed from the mediastream. +TEST_F(MediaStreamSignalingTest, RemoveAlreadyGoneRemoteStream) { + talk_base::scoped_ptr desc_audio_only( + webrtc::CreateSessionDescription(SessionDescriptionInterface::kOffer, + kSdpStringWithoutStreams, + NULL)); + ASSERT_TRUE(desc_audio_only != NULL); + signaling_->OnRemoteDescriptionChanged(desc_audio_only.get()); + MediaStreamInterface* remote_stream = observer_->remote_streams()->at(0); + remote_stream->RemoveTrack(remote_stream->GetAudioTracks()[0]); + remote_stream->RemoveTrack(remote_stream->GetVideoTracks()[0]); + + talk_base::scoped_ptr desc( + webrtc::CreateSessionDescription(SessionDescriptionInterface::kOffer, + kSdpStringWithoutStreams, NULL)); + ASSERT_TRUE(desc != NULL); + signaling_->OnRemoteDescriptionChanged(desc.get()); + + // No crash is a pass. +} + // This tests that a default MediaStream is created if the remote session // description doesn't contain any streams and don't contain an indication if // MSID is supported. diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index 37f8c5b2e5..053bcc218c 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -313,15 +313,7 @@ class WebRtcSessionTest : public testing::Test { Init(NULL); } - void InitWithDtls() { - constraints_.reset(new FakeConstraints()); - constraints_->AddOptional( - webrtc::MediaConstraintsInterface::kEnableDtlsSrtp, true); - - Init(NULL); - } - - void InitWithAsyncDtls(bool identity_request_should_fail) { + void InitWithDtls(bool identity_request_should_fail = false) { constraints_.reset(new FakeConstraints()); constraints_->AddOptional( webrtc::MediaConstraintsInterface::kEnableDtlsSrtp, true); @@ -819,7 +811,7 @@ class WebRtcSessionTest : public testing::Test { void VerifyMultipleAsyncCreateDescription( bool success, CreateSessionDescriptionRequest::Type type) { - InitWithAsyncDtls(!success); + InitWithDtls(!success); if (type == CreateSessionDescriptionRequest::kAnswer) { cricket::MediaSessionOptions options; @@ -2526,7 +2518,7 @@ TEST_F(WebRtcSessionTest, TestSctpDataChannelWithDtls) { // identity generation is finished. TEST_F(WebRtcSessionTest, TestCreateOfferBeforeIdentityRequestReturnSuccess) { MAYBE_SKIP_TEST(talk_base::SSLStreamAdapter::HaveDtlsSrtp); - InitWithAsyncDtls(false); + InitWithDtls(false); EXPECT_TRUE(session_->waiting_for_identity()); talk_base::scoped_ptr offer(CreateOffer(NULL)); @@ -2537,7 +2529,7 @@ TEST_F(WebRtcSessionTest, TestCreateOfferBeforeIdentityRequestReturnSuccess) { // identity generation is finished. TEST_F(WebRtcSessionTest, TestCreateAnswerBeforeIdentityRequestReturnSuccess) { MAYBE_SKIP_TEST(talk_base::SSLStreamAdapter::HaveDtlsSrtp); - InitWithAsyncDtls(false); + InitWithDtls(false); cricket::MediaSessionOptions options; scoped_ptr offer( @@ -2553,7 +2545,7 @@ TEST_F(WebRtcSessionTest, TestCreateAnswerBeforeIdentityRequestReturnSuccess) { // identity generation is finished. TEST_F(WebRtcSessionTest, TestCreateOfferAfterIdentityRequestReturnSuccess) { MAYBE_SKIP_TEST(talk_base::SSLStreamAdapter::HaveDtlsSrtp); - InitWithAsyncDtls(false); + InitWithDtls(false); EXPECT_TRUE_WAIT(!session_->waiting_for_identity(), 1000); talk_base::scoped_ptr offer(CreateOffer(NULL)); @@ -2564,7 +2556,7 @@ TEST_F(WebRtcSessionTest, TestCreateOfferAfterIdentityRequestReturnSuccess) { // identity generation fails. TEST_F(WebRtcSessionTest, TestCreateOfferAfterIdentityRequestReturnFailure) { MAYBE_SKIP_TEST(talk_base::SSLStreamAdapter::HaveDtlsSrtp); - InitWithAsyncDtls(true); + InitWithDtls(true); EXPECT_TRUE_WAIT(!session_->waiting_for_identity(), 1000); talk_base::scoped_ptr offer(CreateOffer(NULL)); diff --git a/talk/media/webrtc/fakewebrtcvideoengine.h b/talk/media/webrtc/fakewebrtcvideoengine.h index 3113c9c187..bfd880f103 100644 --- a/talk/media/webrtc/fakewebrtcvideoengine.h +++ b/talk/media/webrtc/fakewebrtcvideoengine.h @@ -714,6 +714,9 @@ class FakeWebRtcVideoEngine WEBRTC_STUB(DeregisterDecoderObserver, (const int)); WEBRTC_STUB(SendKeyFrame, (const int)); WEBRTC_STUB(WaitForFirstKeyFrame, (const int, const bool)); +#ifdef USE_WEBRTC_DEV_BRANCH + WEBRTC_STUB(SetDecodeErrorMode, (const int, const ViEDecodeErrorMode)); +#endif WEBRTC_STUB(StartDebugRecording, (int, const char*)); WEBRTC_STUB(StopDebugRecording, (int)); diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc index 31596cd726..051a026f29 100644 --- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc +++ b/talk/media/webrtc/webrtcvoiceengine_unittest.cc @@ -125,7 +125,7 @@ class WebRtcVoiceEngineTestFake : public testing::Test { EXPECT_TRUE(SetupEngine()); // Remove stream added in Setup, which is corresponding to default channel. int default_channel_num = voe_.GetLastChannel(); - uint32 default_send_ssrc; + uint32 default_send_ssrc = 0u; EXPECT_EQ(0, voe_.GetLocalSSRC(default_channel_num, default_send_ssrc)); EXPECT_EQ(kSsrc1, default_send_ssrc); EXPECT_TRUE(channel_->RemoveSendStream(default_send_ssrc));