Send error reply to GpuVideoDecodeAccelerator::Initialize when not available

Send error reply to GpuVideoDecodeAccelerator::Initialize when not available

BUG=None
TEST=Pepper Flash on x86 with HW decoder on


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=96500

[Ami GONE FROM CHROMIUM, 2011-08-11 04:53:15]

LGTM as far as it goes but I think you can clean up a bug I put in; see comment.

http://codereview.chromium.org/7620003/diff/1/content/common/gpu/media/gpu_vi...
File content/common/gpu/media/gpu_video_decode_accelerator.cc (right):

http://codereview.chromium.org/7620003/diff/1/content/common/gpu/media/gpu_vi...
content/common/gpu/media/gpu_video_decode_accelerator.cc:89: if (!Send(new
AcceleratedVideoDecoderHostMsg_ErrorNotification(
Hmm.   Your change makes me think that if init_done_msg_ is non-NULL this method
should instead do the same thing you're doing in Initialize.
In which case Initialize should just call this method to signal error.  WDYT?

[piman, 2011-08-11 05:08:18]

http://codereview.chromium.org/7620003/diff/1/content/common/gpu/media/gpu_vi...
File content/common/gpu/media/gpu_video_decode_accelerator.cc (right):

http://codereview.chromium.org/7620003/diff/1/content/common/gpu/media/gpu_vi...
content/common/gpu/media/gpu_video_decode_accelerator.cc:89: if (!Send(new
AcceleratedVideoDecoderHostMsg_ErrorNotification(
On 2011/08/11 04:53:19, Ami Fischman wrote:
> Hmm.   Your change makes me think that if init_done_msg_ is non-NULL this
method
> should instead do the same thing you're doing in Initialize.
> In which case Initialize should just call this method to signal error.  WDYT?

I haven't audited in which conditions NotifyError is called, and if it is, if
NotifyInitializeDone will not be called.

If that's the case, then yes, we'll probably need to do that. If we don't send
the reply, the renderer hangs forever.

[Ami GONE FROM CHROMIUM, 2011-08-11 05:12:42]

Yes, NotifiyInitializeDone is XOR NotifyError (only one will be called).
My WDYT was conversational :)  Feel free to submit your CL as-is and I'll do
the other part, or you can do it in your CL if you like.

[piman, 2011-08-11 21:02:01]

http://codereview.chromium.org/7620003/diff/1/content/common/gpu/media/gpu_vi...
File content/common/gpu/media/gpu_video_decode_accelerator.cc (right):

http://codereview.chromium.org/7620003/diff/1/content/common/gpu/media/gpu_vi...
content/common/gpu/media/gpu_video_decode_accelerator.cc:89: if (!Send(new
AcceleratedVideoDecoderHostMsg_ErrorNotification(
On 2011/08/11 05:08:18, piman wrote:
> On 2011/08/11 04:53:19, Ami Fischman wrote:
> > Hmm.   Your change makes me think that if init_done_msg_ is non-NULL this
> method
> > should instead do the same thing you're doing in Initialize.
> > In which case Initialize should just call this method to signal error. 
WDYT?
> 
> I haven't audited in which conditions NotifyError is called, and if it is, if
> NotifyInitializeDone will not be called.
> 
> If that's the case, then yes, we'll probably need to do that. If we don't send
> the reply, the renderer hangs forever.

Done.

[Ami GONE FROM CHROMIUM, 2011-08-11 21:04:10]

LGTM

http://codereview.chromium.org/7620003/diff/6001/content/common/gpu/media/gpu...
File content/common/gpu/media/gpu_video_decode_accelerator.cc (right):

http://codereview.chromium.org/7620003/diff/6001/content/common/gpu/media/gpu...
content/common/gpu/media/gpu_video_decode_accelerator.cc:120:
init_done_msg->set_reply_error();
FWIW, this can now call NotifyError() instead of doing its own set/send.

[piman, 2011-08-11 22:48:48]

http://codereview.chromium.org/7620003/diff/6001/content/common/gpu/media/gpu...
File content/common/gpu/media/gpu_video_decode_accelerator.cc (right):

http://codereview.chromium.org/7620003/diff/6001/content/common/gpu/media/gpu...
content/common/gpu/media/gpu_video_decode_accelerator.cc:120:
init_done_msg->set_reply_error();
On 2011/08/11 21:04:11, Ami Fischman wrote:
> FWIW, this can now call NotifyError() instead of doing its own set/send.

Done.

[Ami GONE FROM CHROMIUM, 2011-08-11 22:56:38]

LGTM
I could LGTM this CL all day!

[commit-bot: I haz the power, 2011-08-12 03:05:45]

Change committed as 96500