Fix crash in renderer initialization when media pipeline is stopped

media/renderers/renderer_impl_unittest.cc

 // Copyright 2014 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 <stdint.h>
 
 #include <memory>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 132 matching lines @@
   void CreateEncryptedVideoStream() { CreateVideoStream(true); }
 
   void CreateAudioAndVideoStream() {
     CreateAudioStream();
     CreateVideoStream();
   }
 
   void InitializeWithAudio() {
     CreateAudioStream();
     SetAudioRendererInitializeExpectations(PIPELINE_OK);
+    // There is a potential race between HTMLMediaElement/WMPI shutdown and
+    // renderers being initialized which might result in DemuxerStreamProvider
+    // GetStream suddenly returning NULL (see crbug.com/668604). So we are going
+    // to check here that GetStream will be invoked exactly 3 times during
+    // RendererImpl initialization to help catch potential issues. Currently the
+    // GetStream is invoked once directly from RendererImpl::Initialize, once
+    // indirectly from RendererImpl::Initialize via HasEncryptedStream and once
+    // from RendererImpl::InitializeAudioRenderer.
+    EXPECT_CALL(*demuxer_, GetStream(DemuxerStream::AUDIO))

[DaleCurtis, 2016/12/12 23:43:09]

For HasEncryptedStream() can we cache the value during InitializeAudioRenderer()
/ InitializeVideoRenderer() or does this change later? +xhwang for clarity.

[xhwang, 2016/12/13 00:07:28]

Not having the big picture of this CL... :)

Today HasEncryptedStream() is called before
InitializeAudioRenderer()/InitializeVideoRenderer(), and the value really
depends on whether we have any encrypted DemuxerStream.

But it's a little bit vague to me when the streams can change.

I assume the streams cannot change when we are pending Renderer::Initialize()?
Then after the initialization succeeded and we are playing, some streams can be
disabled in the form of EOS. Is this correct? If we disable the encrypted
stream, I assume HasEncryptedStream() will become false, but I suppose this
should not happen during Renderer::Initialize()?

FWIW, today RendererImpl::Initialize() will wait for a CDM to be set to start
initialize audio/video renderers if HasEncryptedStream() is true. I am thinking
that we probably should move this logic to DecoderSelector so that we can handle
more dynamic config changes, e.g. switch between clear and encrytped. Will that
do any help here?

+        .Times(3)
+        .WillRepeatedly(Return(audio_stream_.get()));
     InitializeAndExpect(PIPELINE_OK);
   }
 
   void InitializeWithVideo() {
     CreateVideoStream();
     SetVideoRendererInitializeExpectations(PIPELINE_OK);
+    // There is a potential race between HTMLMediaElement/WMPI shutdown and
+    // renderers being initialized which might result in DemuxerStreamProvider
+    // GetStream suddenly returning NULL (see crbug.com/668604). So we are going
+    // to check here that GetStream will be invoked exactly 3 times during
+    // RendererImpl initialization to help catch potential issues. Currently the
+    // GetStream is invoked once directly from RendererImpl::Initialize, once
+    // indirectly from RendererImpl::Initialize via HasEncryptedStream and once
+    // from RendererImpl::InitializeVideoRenderer.
+    EXPECT_CALL(*demuxer_, GetStream(DemuxerStream::VIDEO))
+        .Times(3)
+        .WillRepeatedly(Return(video_stream_.get()));
     InitializeAndExpect(PIPELINE_OK);
   }
 
   void InitializeWithAudioAndVideo() {
     CreateAudioAndVideoStream();
     SetAudioRendererInitializeExpectations(PIPELINE_OK);
     SetVideoRendererInitializeExpectations(PIPELINE_OK);
     InitializeAndExpect(PIPELINE_OK);
   }
 
@@ skipping 567 matching lines @@
   EXPECT_CALL(*audio_renderer_, Flush(_)).WillOnce(RunClosure<0>());
   EXPECT_CALL(*audio_renderer_, StartPlaying()).Times(1);
   audio_stream_status_change_cb.Run(false, time0);
 
   EXPECT_CALL(*video_renderer_, Flush(_)).WillOnce(RunClosure<0>());
   EXPECT_CALL(*video_renderer_, StartPlayingFrom(_)).Times(1);
   video_stream_status_change_cb.Run(false, time0);
 }
 
 }  // namespace media