Fix DCHECK when track status changes in non-playing state

media/renderers/renderer_impl.cc

Index: media/renderers/renderer_impl.cc
diff --git a/media/renderers/renderer_impl.cc b/media/renderers/renderer_impl.cc
index c4fbf53d3ffa3d1badacd0621677c3aaab570ade..a087603f9bab15e43cbd327b826a7631237bdc7a 100644
--- a/media/renderers/renderer_impl.cc
+++ b/media/renderers/renderer_impl.cc
@@ -229,7 +229,7 @@ void RendererImpl::RestartStreamPlayback(DemuxerStream* stream,
   bool video = (stream->type() == DemuxerStream::VIDEO);
   DVLOG(1) << __func__ << (video ? " video" : " audio") << " stream=" << stream
            << " enabled=" << stream->enabled() << " time=" << time.InSecondsF();
-  if (state_ != STATE_PLAYING)
+  if ((state_ != STATE_PLAYING) || (audio_ended_ && video_ended_))

[chcunningham, 2016/08/24 22:16:53]

This seems to be a separate issue from relaxing that DCHECK? Can you explain the
circumstances here?

[servolk, 2016/08/24 22:46:32]

Yes, somewhat separate, but related. First I've simply updated the DCHECK
condition (see patchset #1). But then I decided to add a unit test that would
test changing media track status while the pipeline is not in the kPlaying state
(previously it would trip on that DCHECK). But when I added a unit test that
waits for playback to end (so that pipeline is in the stopped state) and then
changes tracks, it would hit a different DCHECK (here
https://cs.chromium.org/chromium/src/media/base/pipeline_impl.cc?rcl=0&l=552).
It turns out that RendererImpl stays in the STATE_PLAYING even after both
renderers are ended. So we would get here and would try to call
RestartA/VRenderer below even though the playback of both streams has already
ended. By adding this additional check we'll ensure that media track status
changes when the RendererImpl has been ended, we won't try to restart renderers.
Alternatively I guess we could try to set RendererImpl::state_ to something else
besides STATE_PLAYING in RendererImpl::RunEndedCallbackIfNeeded when we detect
that playback has actually ended. But none of the other RendererImpl::State enum
values seem to be fitting this situation, and I wanted to keep this change small
and safe. So I've chose to modify the check here, rather than mess with state_
or introduce a new State enum value.

     return;
   if (stream->type() == DemuxerStream::VIDEO) {
     DCHECK(video_renderer_);