GpuVideoDecoder not to use DemuxerStream after it's stopped.

GpuVideoDecoder not to use DemuxerStream after it's stopped.

BUG=225846
TEST=Will test on Lumpy.


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=193317

[xhwang, 2013-04-04 01:25:51]

I am not able to repro the crash either with or without this CL :( I'll try
more. But I think this CL fixes the issue. PTAL.

[Ami GONE FROM CHROMIUM, 2013-04-04 03:43:22]

https://codereview.chromium.org/13585003/diff/2001/media/base/video_decoder.h
File media/base/video_decoder.h (right):

https://codereview.chromium.org/13585003/diff/2001/media/base/video_decoder.h...
media/base/video_decoder.h:62: // been stopped. A VideoDecoder should not access
the DemuxerStream after it
This seems like a commentary on the lifecycle of the demuxerstream arg to
Initialize.

https://codereview.chromium.org/13585003/diff/2001/media/filters/gpu_video_de...
File media/filters/gpu_video_decoder.cc (right):

https://codereview.chromium.org/13585003/diff/2001/media/filters/gpu_video_de...
media/filters/gpu_video_decoder.cc:576: &GpuVideoDecoder::ReadFromDemuxerStream,
this));
Do you see a reason not to simply run the Read() here, instead of posting it?

[xhwang, 2013-04-04 04:15:22]

Comment only.

https://codereview.chromium.org/13585003/diff/2001/media/filters/gpu_video_de...
File media/filters/gpu_video_decoder.cc (right):

https://codereview.chromium.org/13585003/diff/2001/media/filters/gpu_video_de...
media/filters/gpu_video_decoder.cc:576: &GpuVideoDecoder::ReadFromDemuxerStream,
this));
On 2013/04/04 03:43:22, Ami Fischman wrote:
> Do you see a reason not to simply run the Read() here, instead of posting it?

I think there are two reasons why we always post:
1) DemuxerStream can return the ReadCB synchronously (See
http://crbug.com/168270). 
2) RequestBufferDecode() calls EnsureDemuxOrDecode() directly. If we don't post,
we'd reentry the DS. 

Now (1) is already fixed. (2) seems can be solved by posting
ReadFromDemuxerStream instead of calling it directly. I'll try it out tomorrow
and update this CL.

[xhwang, 2013-04-04 17:05:33]

PTAL

https://codereview.chromium.org/13585003/diff/2001/media/base/video_decoder.h
File media/base/video_decoder.h (right):

https://codereview.chromium.org/13585003/diff/2001/media/base/video_decoder.h...
media/base/video_decoder.h:62: // been stopped. A VideoDecoder should not access
the DemuxerStream after it
On 2013/04/04 03:43:22, Ami Fischman wrote:
> This seems like a commentary on the lifecycle of the demuxerstream arg to
> Initialize.

Done.

https://codereview.chromium.org/13585003/diff/2001/media/filters/gpu_video_de...
File media/filters/gpu_video_decoder.cc (right):

https://codereview.chromium.org/13585003/diff/2001/media/filters/gpu_video_de...
media/filters/gpu_video_decoder.cc:576: &GpuVideoDecoder::ReadFromDemuxerStream,
this));
On 2013/04/04 03:43:22, Ami Fischman wrote:
> Do you see a reason not to simply run the Read() here, instead of posting it?

Done.

[Ami GONE FROM CHROMIUM, 2013-04-04 17:39:13]

LGTM % typo and confirming that this actually fixes the issue!
(repro the problem without this fix)

https://codereview.chromium.org/13585003/diff/9001/media/filters/gpu_video_de...
File media/filters/gpu_video_decoder.cc (right):

https://codereview.chromium.org/13585003/diff/9001/media/filters/gpu_video_de...
media/filters/gpu_video_decoder.cc:308: // Force post here to prevent reentrency
into DemuxerStream.
s/reentrency/reentrancy/

[scherkus (not reviewing), 2013-04-04 17:46:17]

lgtm

[xhwang, 2013-04-09 23:31:48]

https://codereview.chromium.org/13585003/diff/9001/media/filters/gpu_video_de...
File media/filters/gpu_video_decoder.cc (right):

https://codereview.chromium.org/13585003/diff/9001/media/filters/gpu_video_de...
media/filters/gpu_video_decoder.cc:308: // Force post here to prevent reentrency
into DemuxerStream.
On 2013/04/04 17:39:13, Ami Fischman wrote:
> s/reentrency/reentrancy/

Done.

[commit-bot: I haz the power, 2013-04-09 23:41:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/13585003/16002

[commit-bot: I haz the power, 2013-04-10 04:17:34]

Change committed as 193317