Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1957)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: media/filters/ffmpeg_demuxer_unittest.cc

Issue 13813016: Remove reference counting from media::Demuxer and friends. (Closed) Base URL: http://git.chromium.org/chromium/src.git@vd_scoped
Patch Set: Created 7 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« media/filters/ffmpeg_demuxer.cc ('K') | « media/filters/ffmpeg_demuxer.cc ('k') | media/filters/ffmpeg_video_decoder.h » ('j') | media/filters/pipeline_integration_test.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: media/filters/ffmpeg_demuxer_unittest.cc
diff --git a/media/filters/ffmpeg_demuxer_unittest.cc b/media/filters/ffmpeg_demuxer_unittest.cc
index 4fccfb97b90b12c7a403b18135a029afdddc6f32..64525747c218c39f654379645a52772d4a0a82f1 100644
--- a/media/filters/ffmpeg_demuxer_unittest.cc
+++ b/media/filters/ffmpeg_demuxer_unittest.cc
@@ -66,11 +66,12 @@ class FFmpegDemuxerTest : public testing::Test {
virtual ~FFmpegDemuxerTest() {
if (demuxer_) {
- demuxer_->Stop(MessageLoop::QuitWhenIdleClosure());
- message_loop_.Run();
+ WaitableMessageLoopEvent event;
+ demuxer_->Stop(event.GetClosure());
+ event.RunAndWait();
}
- demuxer_ = NULL;
+ demuxer_.reset();
}
void CreateDemuxer(const std::string& name) {
@@ -84,9 +85,9 @@ class FFmpegDemuxerTest : public testing::Test {
media::FFmpegNeedKeyCB need_key_cb =
base::Bind(&FFmpegDemuxerTest::NeedKeyCB, base::Unretained(this));
- demuxer_ = new FFmpegDemuxer(message_loop_.message_loop_proxy(),
- data_source_,
- need_key_cb);
+ demuxer_.reset(new FFmpegDemuxer(message_loop_.message_loop_proxy(),
+ data_source_,
+ need_key_cb));
}
MOCK_METHOD1(CheckPoint, void(int v));
@@ -153,7 +154,7 @@ class FFmpegDemuxerTest : public testing::Test {
// Fixture members.
scoped_refptr<FileDataSource> data_source_;
- scoped_refptr<FFmpegDemuxer> demuxer_;
+ scoped_ptr<FFmpegDemuxer> demuxer_;
StrictMock<MockDemuxerHost> host_;
MessageLoop message_loop_;
@@ -163,8 +164,7 @@ class FFmpegDemuxerTest : public testing::Test {
void ReadUntilEndOfStream() {
// We should expect an end of stream buffer.
- scoped_refptr<DemuxerStream> audio =
- demuxer_->GetStream(DemuxerStream::AUDIO);
+ DemuxerStream* audio = demuxer_->GetStream(DemuxerStream::AUDIO);
bool got_eos_buffer = false;
const int kMaxBuffers = 170;
@@ -234,8 +234,7 @@ TEST_F(FFmpegDemuxerTest, Initialize_Successful) {
InitializeDemuxer();
// Video stream should be present.
- scoped_refptr<DemuxerStream> stream =
- demuxer_->GetStream(DemuxerStream::VIDEO);
+ DemuxerStream* stream = demuxer_->GetStream(DemuxerStream::VIDEO);
ASSERT_TRUE(stream);
EXPECT_EQ(DemuxerStream::VIDEO, stream->type());
@@ -284,8 +283,7 @@ TEST_F(FFmpegDemuxerTest, Initialize_Multitrack) {
InitializeDemuxer();
// Video stream should be VP8.
- scoped_refptr<DemuxerStream> stream =
- demuxer_->GetStream(DemuxerStream::VIDEO);
+ DemuxerStream* stream = demuxer_->GetStream(DemuxerStream::VIDEO);
ASSERT_TRUE(stream);
EXPECT_EQ(DemuxerStream::VIDEO, stream->type());
EXPECT_EQ(kCodecVP8, stream->video_decoder_config().codec());
@@ -317,8 +315,7 @@ TEST_F(FFmpegDemuxerTest, Read_Audio) {
InitializeDemuxer();
// Attempt a read from the audio stream and run the message loop until done.
- scoped_refptr<DemuxerStream> audio =
- demuxer_->GetStream(DemuxerStream::AUDIO);
+ DemuxerStream* audio = demuxer_->GetStream(DemuxerStream::AUDIO);
audio->Read(NewReadCB(FROM_HERE, 29, 0));
message_loop_.Run();
@@ -333,8 +330,7 @@ TEST_F(FFmpegDemuxerTest, Read_Video) {
InitializeDemuxer();
// Attempt a read from the video stream and run the message loop until done.
- scoped_refptr<DemuxerStream> video =
- demuxer_->GetStream(DemuxerStream::VIDEO);
+ DemuxerStream* video = demuxer_->GetStream(DemuxerStream::VIDEO);
video->Read(NewReadCB(FROM_HERE, 22084, 0));
message_loop_.Run();
@@ -349,10 +345,8 @@ TEST_F(FFmpegDemuxerTest, Read_VideoNonZeroStart) {
InitializeDemuxer();
// Attempt a read from the video stream and run the message loop until done.
- scoped_refptr<DemuxerStream> video =
- demuxer_->GetStream(DemuxerStream::VIDEO);
- scoped_refptr<DemuxerStream> audio =
- demuxer_->GetStream(DemuxerStream::AUDIO);
+ DemuxerStream* video = demuxer_->GetStream(DemuxerStream::VIDEO);
+ DemuxerStream* audio = demuxer_->GetStream(DemuxerStream::AUDIO);
// Check first buffer in video stream.
video->Read(NewReadCB(FROM_HERE, 5636, 400000));
@@ -389,10 +383,8 @@ TEST_F(FFmpegDemuxerTest, Seek) {
InitializeDemuxer();
// Get our streams.
- scoped_refptr<DemuxerStream> video =
- demuxer_->GetStream(DemuxerStream::VIDEO);
- scoped_refptr<DemuxerStream> audio =
- demuxer_->GetStream(DemuxerStream::AUDIO);
+ DemuxerStream* video = demuxer_->GetStream(DemuxerStream::VIDEO);
+ DemuxerStream* audio = demuxer_->GetStream(DemuxerStream::AUDIO);
ASSERT_TRUE(video);
ASSERT_TRUE(audio);
@@ -451,8 +443,7 @@ TEST_F(FFmpegDemuxerTest, Stop) {
InitializeDemuxer();
// Get our stream.
- scoped_refptr<DemuxerStream> audio =
- demuxer_->GetStream(DemuxerStream::AUDIO);
+ DemuxerStream* audio = demuxer_->GetStream(DemuxerStream::AUDIO);
ASSERT_TRUE(audio);
demuxer_->Stop(NewExpectedClosure());
@@ -479,48 +470,6 @@ TEST_F(FFmpegDemuxerTest, Stop) {
CheckPoint(1);
}
-// The streams can outlive the demuxer because the streams may still be in use
scherkus (not reviewing) 2013/04/17 17:21:59 this is no longer possible as it would be a UAF to
acolwell GONE FROM CHROMIUM 2013/04/17 20:24:53 What about just removing the demuxer_ = NULL line?
scherkus (not reviewing) 2013/04/19 01:07:22 test added back (it's even more critical now that
-// by the decoder when the demuxer is destroyed.
-// This test verifies that DemuxerStream::Read() does not use an invalid demuxer
-// pointer (no crash occurs) and calls the callback with an EndOfStream buffer.
-TEST_F(FFmpegDemuxerTest, StreamReadAfterStopAndDemuxerDestruction) {
- CreateDemuxer("bear-320x240.webm");
- InitializeDemuxer();
-
- // Get our stream.
- scoped_refptr<DemuxerStream> audio =
- demuxer_->GetStream(DemuxerStream::AUDIO);
- ASSERT_TRUE(audio);
-
- demuxer_->Stop(MessageLoop::QuitWhenIdleClosure());
- message_loop_.Run();
-
- // Expect all calls in sequence.
- InSequence s;
-
- // Create our mocked callback. The Callback created by base::Bind() will take
- // ownership of this pointer.
- StrictMock<MockReadCB>* callback = new StrictMock<MockReadCB>();
-
- // The callback should be immediately deleted. We'll use a checkpoint to
- // verify that it has indeed been deleted.
- EXPECT_CALL(*callback, Run(DemuxerStream::kOk, IsEndOfStreamBuffer()));
- EXPECT_CALL(*callback, OnDelete());
- EXPECT_CALL(*this, CheckPoint(1));
-
- // Release the reference to the demuxer. This should also destroy it.
- demuxer_ = NULL;
- // |audio| now has a demuxer_ pointer to invalid memory.
-
- // Attempt the read...
- audio->Read(base::Bind(&MockReadCB::Run, callback));
-
- message_loop_.RunUntilIdle();
-
- // ...and verify that |callback| was deleted.
- CheckPoint(1);
-}
-
TEST_F(FFmpegDemuxerTest, DisableAudioStream) {
// We are doing the following things here:
// 1. Initialize the demuxer with audio and video stream.
@@ -534,10 +483,8 @@ TEST_F(FFmpegDemuxerTest, DisableAudioStream) {
message_loop_.RunUntilIdle();
// Get our streams.
- scoped_refptr<DemuxerStream> video =
- demuxer_->GetStream(DemuxerStream::VIDEO);
- scoped_refptr<DemuxerStream> audio =
- demuxer_->GetStream(DemuxerStream::AUDIO);
+ DemuxerStream* video = demuxer_->GetStream(DemuxerStream::VIDEO);
+ DemuxerStream* audio = demuxer_->GetStream(DemuxerStream::AUDIO);
ASSERT_TRUE(video);
ASSERT_TRUE(audio);
@@ -564,10 +511,8 @@ TEST_F(FFmpegDemuxerTest, SeekWithCuesBeforeFirstCluster) {
InitializeDemuxer();
// Get our streams.
- scoped_refptr<DemuxerStream> video =
- demuxer_->GetStream(DemuxerStream::VIDEO);
- scoped_refptr<DemuxerStream> audio =
- demuxer_->GetStream(DemuxerStream::AUDIO);
+ DemuxerStream* video = demuxer_->GetStream(DemuxerStream::VIDEO);
+ DemuxerStream* audio = demuxer_->GetStream(DemuxerStream::AUDIO);
ASSERT_TRUE(video);
ASSERT_TRUE(audio);
« media/filters/ffmpeg_demuxer.cc ('K') | « media/filters/ffmpeg_demuxer.cc ('k') | media/filters/ffmpeg_video_decoder.h » ('j') | media/filters/pipeline_integration_test.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698