Remove reference counting from media::DemuxerStream and friends.

media/filters/ffmpeg_demuxer_unittest.cc

Index: media/filters/ffmpeg_demuxer_unittest.cc
diff --git a/media/filters/ffmpeg_demuxer_unittest.cc b/media/filters/ffmpeg_demuxer_unittest.cc
index 35c950d42dfdf774e86ffa497e729537c81d990f..266f2219d62afe9df2d3a843fcdabbdf1c24c8d8 100644
--- a/media/filters/ffmpeg_demuxer_unittest.cc
+++ b/media/filters/ffmpeg_demuxer_unittest.cc
@@ -66,8 +66,9 @@ class FFmpegDemuxerTest : public testing::Test {
 
   virtual ~FFmpegDemuxerTest() {
     if (demuxer_) {
-      demuxer_->Stop(MessageLoop::QuitWhenIdleClosure());
-      message_loop_.Run();
+      WaitableMessageLoopEvent event;
+      demuxer_->Stop(event.GetClosure());
+      event.RunAndWait();
     }
   }
 
@@ -160,8 +161,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;
@@ -231,8 +231,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());
 
@@ -281,8 +280,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());
@@ -314,8 +312,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();
@@ -330,8 +327,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();
@@ -346,10 +342,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));
@@ -386,10 +380,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);
 
@@ -448,8 +440,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());
@@ -476,47 +467,6 @@ TEST_F(FFmpegDemuxerTest, Stop) {
   CheckPoint(1);
 }
 
-// The streams can outlive the demuxer because the streams may still be in use
-// 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) {

[acolwell GONE FROM CHROMIUM, 2013/04/23 03:20:42]

I feel like this test is still useful w/o destruction. Is it possible for
Read()s to occur after Stop() is called on the demuxer?

[scherkus (not reviewing), 2013/04/23 18:43:40]

Yes and that is covered by the test above (FFmpegDemuxerTest.Stop)

This test covered the case where we do a Read() but due to refcounting the
stream is kept alive but the demuxer was destroyed.

Although looking at the test again it seems silly that we're testing that the
callback is deleted ... it seems we should only care about running the callback
(whether the callback is deleted is an implementation detail)

-  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));
-
-  // Destroy the demuxer. |audio| now has a demuxer_ pointer to invalid memory.
-  demuxer_.reset();
-
-  // 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.
@@ -530,10 +480,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);
 
@@ -560,10 +508,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);