800538 - Add MOZ_ASSERT for mVoiceEngine in MediaEngineWebRTCAudioSource/etc constructors

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P3)

Description

[Randell Jesup [:jesup] (needinfo me)]

Belt-and-suspenders safety

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Adds two MOZ_ASSERTs

Note that I took the "etc" in the comment as license to add an equivalent MOZ_ASSERT to the mVideoEngine in MediaEngineWebRTCVideoSource as well.

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 686386 [details] [diff] [review]
Adds two MOZ_ASSERTs

Review of attachment 686386 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/webrtc/MediaEngineWebRTC.h
@@ +62,5 @@
>    // ViEExternalRenderer.
>    virtual int FrameSizeChange(unsigned int, unsigned int, unsigned int);
>    virtual int DeliverFrame(unsigned char*, int, uint32_t, int64_t);
>  
> +  MediaEngineWebRTCVideoSource(webrtc::VideoEngine* videoEngine,

Mozilla prefers aFoo for parameters, mFoo for member vars, foo for locals.  Put this back to aVideoEnginePtr (or aVideoEngine might be better), and change the AudioSource from voiceEngine to aVoiceEngine

Comment 3

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

I tried to make it symmetrical with the Audio code. Should I change the Audio code to the preferred syntax or leave both alone?

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Adds two MOZ_ASSERTs

Addressing comments from review, carrying forward r=jesup.

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

 https://tbpl.mozilla.org/?tree=Try&rev=9ee1b88c7b7f

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3d26dff1f3e9

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3d26dff1f3e9