Fix 99596.

media/filters/ffmpeg_demuxer_unittest.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <deque>
 
 #include "base/file_path.h"
 #include "base/path_service.h"
 #include "base/threading/thread.h"
 #include "media/base/filters.h"
@@ skipping 13 matching lines @@
 using ::testing::NotNull;
 using ::testing::Return;
 using ::testing::SaveArg;
 using ::testing::SetArgPointee;
 using ::testing::StrictMock;
 using ::testing::WithArgs;
 using ::testing::_;
 
 namespace media {
 
+MATCHER(IsEndOfStreamBuffer,
+        std::string(negation ? "isn't" : "is") + " end of stream") {
+  return arg->IsEndOfStream();
+}
+
 // Fixture class to facilitate writing tests.  Takes care of setting up the
 // FFmpeg, pipeline and filter host mocks.
 class FFmpegDemuxerTest : public testing::Test {
  protected:
 
   FFmpegDemuxerTest() {
     // Create an FFmpegDemuxer.
     demuxer_ = new FFmpegDemuxer(&message_loop_);
     demuxer_->disable_first_seek_hack_for_testing();
 
     // Inject a filter host and message loop and prepare a data source.
     demuxer_->set_host(&host_);
 
     EXPECT_CALL(host_, SetTotalBytes(_)).Times(AnyNumber());
     EXPECT_CALL(host_, SetBufferedBytes(_)).Times(AnyNumber());
     EXPECT_CALL(host_, SetCurrentReadPosition(_))
         .WillRepeatedly(SaveArg<0>(&current_read_position_));
   }
 
   virtual ~FFmpegDemuxerTest() {
-    // Call Stop() to shut down internal threads.
-    demuxer_->Stop(NewExpectedClosure());
+    if (demuxer_)

[scherkus (not reviewing), 2011/10/15 01:32:14]

could you add {} around here

[ddorwin, 2011/10/15 01:38:16]

Done.

+      // Call Stop() to shut down internal threads.
+      demuxer_->Stop(NewExpectedClosure());
 
     // Finish up any remaining tasks.
     message_loop_.RunAllPending();
     // Release the reference to the demuxer.
     demuxer_ = NULL;
   }
 
   scoped_refptr<FileDataSource> CreateDataSource(const std::string& name) {
     return CreateDataSource(name, false);
   }
@@ skipping 303 matching lines @@
   }
 
   MOCK_METHOD0(OnDelete, void());
   MOCK_METHOD1(Run, void(Buffer* buffer));
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockReadCallback);
 };
 
 TEST_F(FFmpegDemuxerTest, Stop) {
-  // Tests that calling Read() on a stopped demuxer immediately deletes the
-  // callback.
+  // Tests that calling Read() on a stopped demuxer stream immediately deletes
+  // the callback.
   InitializeDemuxer(CreateDataSource("bear-320x240.webm"));
 
   // Get our stream.
   scoped_refptr<DemuxerStream> audio =
       demuxer_->GetStream(DemuxerStream::AUDIO);
   ASSERT_TRUE(audio);
 
   demuxer_->Stop(NewExpectedClosure());
 
   // Expect all calls in sequence.
   InSequence s;
 
   // Create our mocked callback. The Callback created by base::Bind() will take
   // ownership of this pointer.
   StrictMock<MockReadCallback>* callback = new StrictMock<MockReadCallback>();
 
   // The callback should be immediately deleted.  We'll use a checkpoint to
   // verify that it has indeed been deleted.
+  EXPECT_CALL(*callback, Run(NotNull()));
   EXPECT_CALL(*callback, OnDelete());
   EXPECT_CALL(*this, CheckPoint(1));
 
   // Attempt the read...
   audio->Read(base::Bind(&MockReadCallback::Run, callback));
 
   message_loop_.RunAllPending();
 
   // ...and verify that |callback| was deleted.
   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) {
+  InitializeDemuxer(CreateDataSource("bear-320x240.webm"));
+
+  // Get our stream.
+  scoped_refptr<DemuxerStream> audio =
+      demuxer_->GetStream(DemuxerStream::AUDIO);
+  ASSERT_TRUE(audio);
+
+  demuxer_->Stop(NewExpectedClosure());
+
+  // Finish up any remaining tasks.
+  message_loop_.RunAllPending();
+
+  // Expect all calls in sequence.
+  InSequence s;
+
+  // Create our mocked callback. The Callback created by base::Bind() will take
+  // ownership of this pointer.
+  StrictMock<MockReadCallback>* callback = new StrictMock<MockReadCallback>();
+
+  // The callback should be immediately deleted.  We'll use a checkpoint to
+  // verify that it has indeed been deleted.
+  EXPECT_CALL(*callback, Run(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(&MockReadCallback::Run, callback));
+
+  message_loop_.RunAllPending();
+
+  // ...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.
   // 2. Send a "disable audio stream" message to the demuxer.
   // 3. Demuxer will free audio packets even if audio stream was initialized.
   InitializeDemuxer(CreateDataSource("bear-320x240.webm"));
 
   // Submit a "disable audio stream" message to the demuxer.
   demuxer_->OnAudioRendererDisabled();
   message_loop_.RunAllPending();
@@ skipping 211 matching lines @@
   message_loop_.RunAllPending();
   EXPECT_TRUE(reader->called());
   ValidateBuffer(FROM_HERE, reader->buffer(), 1740, 2436000);
 
   // Manually release the last reference to the buffer and verify it was freed.
   reader->Reset();
   message_loop_.RunAllPending();
 }
 
 }  // namespace media