The Static Analysis tool Coverity detected that a null pointer deference can happen in the following instance: >> 786 MediaFormatReader::MediaFormatReader(AbstractMediaDecoder* aDecoder, >> 787 MediaDataDemuxer* aDemuxer, >> 788 VideoFrameContainer* aVideoFrameContainer) >> 789 : MediaDecoderReader(aDecoder) >> 790 , mAudio(this, MediaData::AUDIO_DATA, >> 791 Preferences::GetUint("media.audio-max-decode-error", 3)) >> 792 , mVideo(this, MediaData::VIDEO_DATA, >> 793 Preferences::GetUint("media.video-max-decode-error", 2)) >> 794 , mDemuxer(new DemuxerProxy(aDemuxer, aDecoder->AbstractMainThread())) >> 795 , mDemuxerInitDone(false) >> 796 , mLastReportedNumDecodedFrames(0) >> 797 , mPreviousDecodedKeyframeTime_us(sNoPreviousDecodedKeyframe) >> 798 , mInitDone(false) >> 799 , mTrackDemuxersMayBlock(false) >> 800 , mSeekScheduled(false) >> 801 , mVideoFrameContainer(aVideoFrameContainer) >> 802 , mDecoderFactory(new DecoderFactory(this)) >> 803 { >> 804 MOZ_ASSERT(aDemuxer); >> 805 MOZ_COUNT_CTOR(MediaFormatReader); >> 806 >> 807 if (aDecoder && aDecoder->CompositorUpdatedEvent()) { Since the |aDecoder| is null checked on line 807 the analyser thinks that a possible null value is accepting thus it asserts that a null pointer dereference can happen in the initialiser list on line 794. Looking on how the ctor is called each the parameter that is passed as |aDeoder| is always checked against null, thus preventing the nullness to be passed. But still in order to silence the checker and to prevent future uses where we don't protect the call against null cases i would propose the attached patch.

Comment on attachment 8832014 [details] Bug 1335390 - prevent nul pointer dereference in parameter initialisation. https://reviewboard.mozilla.org/r/108444/#review109840 ::: dom/media/MediaFormatReader.cpp:808 (Diff revision 1) > { > MOZ_ASSERT(aDemuxer); > MOZ_COUNT_CTOR(MediaFormatReader); > > if (aDecoder && aDecoder->CompositorUpdatedEvent()) { > + mDemuxer = MakeUnique<DemuxerProxy>(aDemuxer, Here you are changing the logic. This should be: if (aDecoder) { mDemuxer = MakeUnique<DemuxerProxy>(aDemuxer, aDecoder->AbstractMainThread()); if (aDecoder->CompositorUpdatedEvent()) { mCompositorUpdatedListener = aDecoder->CompositorUpdatedEvent()->Connect( mTaskQueue, this, &MediaFormatReader::NotifyCompositorUpdated); } }

Pushed by bpostelnicu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/dd429b170253 prevent nul pointer dereference in parameter initialisation. r=baku

Keeping in touch with the code maintainers would avoid conflicts and time wasted. And this code is wrong too, now mDemuxer won't be initialised without the parent decoder being defined

Using the correct component helps too! ;-) Also, please check and update the CID issue (on the Coverity website) to claim the issue and reference the corresponding Bugzilla bug -- if I had seen that, I wouldn't have opened another bug. Thanks!