From 7f14cecb6320d376210401220ebe407300e2368e Mon Sep 17 00:00:00 2001 From: "hclam@chromium.org" Date: Fri, 24 Jan 2014 13:09:08 +0000 Subject: Fixes for memory and threading issues in cast This change fixes one issue in media/cast code. TickClock is not owned by CastEnvironment TickClock assigned to CastEnvironment is owned by the creator of CastSender/CastReceiver. However CastEnvironment is ref-counted. This means it is possible that TickClock is deleted but deferenced by some objects that still uses CastEnvironment. Fixing this requires a sweeping change in unit tests. And it also affects End2End test. The result is that CastEnvironment is shared between CastSender and CastReceiver. This adds requirements to log filtering because now a CastEnvironment used by CastSender can see receiver events. Tested this patch with valgrind and cast_unittests reports no error. BUG=336887 Review URL: https://codereview.chromium.org/145443005 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@246863 0039d316-1c4b-4281-b951-d872f2087c98 --- media/cast/audio_sender/audio_encoder_unittest.cc | 18 ++++++++++-------- media/cast/audio_sender/audio_sender_unittest.cc | 14 ++++++++------ 2 files changed, 18 insertions(+), 14 deletions(-) (limited to 'media/cast/audio_sender') diff --git a/media/cast/audio_sender/audio_encoder_unittest.cc b/media/cast/audio_sender/audio_encoder_unittest.cc index 95feab4..877f2f2 100644 --- a/media/cast/audio_sender/audio_encoder_unittest.cc +++ b/media/cast/audio_sender/audio_encoder_unittest.cc @@ -90,13 +90,15 @@ class AudioEncoderTest : public ::testing::TestWithParam { public: AudioEncoderTest() { InitializeMediaLibraryForTesting(); - testing_clock_.Advance( + testing_clock_ = new base::SimpleTestTickClock(); + testing_clock_->Advance( base::TimeDelta::FromMilliseconds(kStartMillisecond)); } virtual void SetUp() { - task_runner_ = new test::FakeTaskRunner(&testing_clock_); - cast_environment_ = new CastEnvironment(&testing_clock_, task_runner_, + task_runner_ = new test::FakeTaskRunner(testing_clock_); + cast_environment_ = new CastEnvironment( + scoped_ptr(testing_clock_).Pass(), task_runner_, task_runner_, task_runner_, task_runner_, task_runner_, task_runner_, GetDefaultCastSenderLoggingConfig()); } @@ -110,26 +112,26 @@ class AudioEncoderTest : public ::testing::TestWithParam { CreateObjectsForCodec(codec); - receiver_->SetRecordedTimeLowerBound(testing_clock_.NowTicks()); + receiver_->SetRecordedTimeLowerBound(testing_clock_->NowTicks()); for (size_t i = 0; i < scenario.num_durations; ++i) { const base::TimeDelta duration = base::TimeDelta::FromMilliseconds(scenario.durations_in_ms[i]); receiver_->SetRecordedTimeUpperBound( - testing_clock_.NowTicks() + duration); + testing_clock_->NowTicks() + duration); const scoped_ptr bus( audio_bus_factory_->NextAudioBus(duration)); const int last_count = release_callback_count_; audio_encoder_->InsertAudio( - bus.get(), testing_clock_.NowTicks(), + bus.get(), testing_clock_->NowTicks(), base::Bind(&AudioEncoderTest::IncrementReleaseCallbackCounter, base::Unretained(this))); task_runner_->RunTasks(); EXPECT_EQ(1, release_callback_count_ - last_count) << "Release callback was not invoked once."; - testing_clock_.Advance(duration); + testing_clock_->Advance(duration); } DVLOG(1) << "Received " << receiver_->frames_received() @@ -163,7 +165,7 @@ class AudioEncoderTest : public ::testing::TestWithParam { ++release_callback_count_; } - base::SimpleTestTickClock testing_clock_; + base::SimpleTestTickClock* testing_clock_; // Owned by CastEnvironment. scoped_refptr task_runner_; scoped_ptr audio_bus_factory_; scoped_ptr receiver_; diff --git a/media/cast/audio_sender/audio_sender_unittest.cc b/media/cast/audio_sender/audio_sender_unittest.cc index ecbd910..4fdff91 100644 --- a/media/cast/audio_sender/audio_sender_unittest.cc +++ b/media/cast/audio_sender/audio_sender_unittest.cc @@ -65,11 +65,13 @@ class AudioSenderTest : public ::testing::Test { protected: AudioSenderTest() { InitializeMediaLibraryForTesting(); - testing_clock_.Advance( + testing_clock_ = new base::SimpleTestTickClock(); + testing_clock_->Advance( base::TimeDelta::FromMilliseconds(kStartMillisecond)); - task_runner_ = new test::FakeTaskRunner(&testing_clock_); + task_runner_ = new test::FakeTaskRunner(testing_clock_); cast_environment_ = new CastEnvironment( - &testing_clock_, task_runner_, task_runner_, task_runner_, task_runner_, + scoped_ptr(testing_clock_).Pass(), + task_runner_, task_runner_, task_runner_, task_runner_, task_runner_, task_runner_, GetDefaultCastSenderLoggingConfig()); audio_config_.codec = transport::kOpus; audio_config_.use_external_encoder = false; @@ -82,7 +84,7 @@ class AudioSenderTest : public ::testing::Test { transport_config.audio_rtp_payload_type = 127; transport_config.audio_channels = 2; transport_sender_.reset(new transport::CastTransportSenderImpl( - &testing_clock_, + testing_clock_, transport_config, base::Bind(&UpdateCastTransportStatus), task_runner_)); transport_sender_->InsertFakeTransportForTesting(&transport_); @@ -97,7 +99,7 @@ class AudioSenderTest : public ::testing::Test { EXPECT_EQ(status, transport::TRANSPORT_INITIALIZED); } - base::SimpleTestTickClock testing_clock_; + base::SimpleTestTickClock* testing_clock_; // Owned by CastEnvironment. TestPacketSender transport_; scoped_ptr transport_sender_; scoped_refptr task_runner_; @@ -145,7 +147,7 @@ TEST_F(AudioSenderTest, RtcpTimer) { // Make sure that we send at least one RTCP packet. base::TimeDelta max_rtcp_timeout = base::TimeDelta::FromMilliseconds(1 + kDefaultRtcpIntervalMs * 3 / 2); - testing_clock_.Advance(max_rtcp_timeout); + testing_clock_->Advance(max_rtcp_timeout); task_runner_->RunTasks(); EXPECT_GE(transport_.number_of_rtp_packets(), 1); EXPECT_EQ(transport_.number_of_rtcp_packets(), 1); -- cgit v1.1