1312332 - Bad `stopVideo` check in GUMCallbackMediaStreamListener::NotifyChromeOfTrackStops

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect, P2)

Description

[Andreas Pehrson [:pehrsons]]

gerald pinged me about an issue that Coverity reported for this method:

> >>>     CID 1374148:  Control flow issues  (DEADCODE)
> >>>    Execution cannot reach the expression "this->mAudioDevice.get()" inside > this statement: "<storage from new>->MediaOp...".
> 3640       RefPtr<MediaOperationTask> mediaOperation =
> 3641         new MediaOperationTask(MEDIA_STOP_TRACK,
> 3642                                this, nullptr, nullptr,
> 3643                                stopAudio ? mAudioDevice.get() : nullptr,
> 3644                                stopVideo ? mVideoDevice.get() : nullptr,
> 3645                                false , mWindowID, nullptr);


He also points out that a check above this looks bad:
> 3630:  if ((stopAudio || mAudioStopped || !mAudioDevice) &&
> 3631:      (stopAudio || mVideoStopped || !mVideoDevice)) {


And indeed, that second line should check `stopVideo` instead. Good thing that this is only on 52 and we can easily fix it.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Thank you for taking care of this.

Comment 2

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1312332 - Use stopVideo as intended.

Review commit: https://reviewboard.mozilla.org/r/87914/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/87914/

Comment 3

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

Comment on attachment 8803770 [details]
Bug 1312332 - Use stopVideo as intended.

https://reviewboard.mozilla.org/r/87914/#review87136

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/267476747553
Use stopVideo as intended. r=jib

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/267476747553