Fix status notifications for FFmpegDemuxerStream with a pending read

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 1c84ca97a2718a0b68334a19e9f3bed058bcbe7e..6d066ebd5c8dcdd3938db27b99f68bbcffd46e24 100644
--- a/media/filters/ffmpeg_demuxer_unittest.cc
+++ b/media/filters/ffmpeg_demuxer_unittest.cc
@@ -54,6 +54,40 @@ MATCHER(IsEndOfStreamBuffer,
   return arg->end_of_stream();
 }
 
+namespace {
+void OnStreamStatusChanged(base::WaitableEvent* event,
+                           DemuxerStream* stream,
+                           bool enabled,
+                           base::TimeDelta) {
+  event->Signal();
+}
+
+void CheckStreamStatusNotifications(MediaResource* media_resource,
+                                    FFmpegDemuxerStream* stream) {
+  base::WaitableEvent event(base::WaitableEvent::ResetPolicy::AUTOMATIC,
+                            base::WaitableEvent::InitialState::NOT_SIGNALED);
+
+  ASSERT_TRUE(stream->IsEnabled());
+  media_resource->SetStreamStatusChangeCB(
+      base::Bind(&OnStreamStatusChanged, base::Unretained(&event)));
+
+  stream->SetEnabled(false, base::TimeDelta());
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(event.IsSignaled());
+
+  event.Reset();
+  stream->SetEnabled(true, base::TimeDelta());
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(event.IsSignaled());
+}
+
+void OnReadDone_ExpectEos(DemuxerStream::Status status,
+                          const scoped_refptr<DecoderBuffer>& buffer) {
+  EXPECT_EQ(status, DemuxerStream::kOk);
+  EXPECT_TRUE(buffer->end_of_stream());
+}
+}
+
 const uint8_t kEncryptedMediaInitData[] = {
     0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
     0x38, 0x39, 0x30, 0x31, 0x32, 0x33, 0x34, 0x35,
@@ -1598,4 +1632,28 @@ TEST_F(FFmpegDemuxerTest, Seek_FallbackToDisabledAudioStream) {
   EXPECT_EQ(astream, preferred_seeking_stream(base::TimeDelta()));
 }
 
+TEST_F(FFmpegDemuxerTest, StreamStatusNotifications) {
+  CreateDemuxer("bear-320x240.webm");
+  InitializeDemuxer();
+  FFmpegDemuxerStream* audio_stream =
+      static_cast<FFmpegDemuxerStream*>(GetStream(DemuxerStream::AUDIO));
+  EXPECT_NE(nullptr, audio_stream);
+  FFmpegDemuxerStream* video_stream =
+      static_cast<FFmpegDemuxerStream*>(GetStream(DemuxerStream::VIDEO));
+  EXPECT_NE(nullptr, video_stream);
+
+  // Verify stream status notifications delivery without pending read first.
+  CheckStreamStatusNotifications(demuxer_.get(), audio_stream);
+  CheckStreamStatusNotifications(demuxer_.get(), video_stream);
+
+  // Verify that stream notifications are delivered properly when stream status
+  // changes with a pending read.
+  audio_stream->FlushBuffers();

[chcunningham, 2017/04/19 01:01:34]

What is the FlushBuffers needed for?

[servolk, 2017/04/19 01:05:33]

To ensure there's no prepared frames that are ready to be returned as soon as we
call stream->Read(). In this case we want the Read to not be satisfied
immediately, so that the stream status changes performed by the
CheckStreamStatusNotifications are done while there is a pending read (i.e. the
Stream::read_cb_ is non-null).

[chcunningham, 2017/04/20 01:00:25]

Makes good sense. Maybe a comment to explain that part. 

[servolk, 2017/04/20 01:04:19]

Done.

+  audio_stream->Read(base::Bind(&media::OnReadDone_ExpectEos));
+  CheckStreamStatusNotifications(demuxer_.get(), audio_stream);
+  video_stream->FlushBuffers();
+  video_stream->Read(base::Bind(&media::OnReadDone_ExpectEos));
+  CheckStreamStatusNotifications(demuxer_.get(), video_stream);
+}
+
 }  // namespace media