836143 - MediaDecoder should call its own OnDecodeThread for clearer error detection

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Steve Workman [:sworkman] (INACTIVE)]

I had an intermittent crash on try (unreproducible locally), in which mDecoderStateMachine was null in MediaDecoder::NotifyBytesConsumed. Specifically, there was an nsCOMPtr crash due to a null mDecoderStateMachine in the MOZ_ASSERT which calls mDecoderStateMachine->OnDecodeThread. The stack trace and error message could have been clearer.

Since I can't reproduce it locally, I'd like to change the error checks to be a little more explicit next time.
-- MOZ_ASSERT should call MediaDecoder::OnDecodeThread, which has a null check for mDecoderStateMachine right before the call.
-- Add an NS_WARN_IF_FALSE to OnDecodeThread to clearly state when mDecoderStateMachine is null.

This way, a crash caused by null mDecoderStateMachine will be caught as such, and not via a null nsCOMPtr crash. Note: The rest of the functions in MediaDecoder call MediaDecoder::OnDecodeThread, not mDecoderStateMachine::OnDecodeThread.

Comment 1

[Steve Workman [:sworkman] (INACTIVE)]

Attachment: v1.0 MediaDecoder should call its own OnDecodeThread for clearer error detection

Comment 2

[Steve Workman [:sworkman] (INACTIVE)]

Thanks Chris!

https://hg.mozilla.org/integration/mozilla-inbound/rev/47354ad87a98

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/47354ad87a98