Closed Bug 1443525 Opened 2 years ago Closed 2 years ago

Add a test to ensure we're not crashing if cubeb_init fails

Categories

(Core :: Audio/Video: cubeb, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox60 --- wontfix
firefox61 --- fixed

People

(Reporter: padenot, Assigned: bryce)

References

Details

Attachments

(2 files)

It's probably enough to have a Gecko pref, that, when set, forces `GetCubebContextUnlocked` to return `nullptr`.
Rank: 25
Priority: -- → P3
Assignee: nobody → bvandyk
Status: NEW → ASSIGNED
Comment on attachment 8960836 [details]
Bug 1443525 - Add hidden pref to force CubebUtils to return null context.

https://reviewboard.mozilla.org/r/229588/#review235418

Very cool. Hopefully with that and your new logging we'll catch things early next time.
Attachment #8960836 - Flags: review?(padenot) → review+
Comment on attachment 8960837 [details]
Bug 1443525 - Add mochitests to test gUM when a cubeb context cannot be obtained.

https://reviewboard.mozilla.org/r/229590/#review235420
Attachment #8960837 - Flags: review?(padenot) → review+
Was seeing some unexpected failures for these and believe what's happening is that if we nullify cubeb after a device enumeration has already taken place, then we can end up using the previous list when we do a new device enumeration. This is because the device list is maintained and we don't unmap the IDs if we failed to grab cubeb during the next enumeration[0].

In the above push[1] I've changed the order so that devices are unmapped before checking if cubeb exists. Which fixes the tests. It would also mean if cubeb died on us in the wild, we wouldn't maintain a list with potentially stale IDs.

I know there is a larger effort towards reworking device enumeration with bug 1404977. :padenot, would that bandaid fix here[1] be worth doing in the meantime? Or is there something else in the works, or something more I could push along which would be worth doing?

[0]: https://searchfox.org/mozilla-central/source/dom/media/webrtc/MediaEngineWebRTC.cpp#48
[1]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f0b4d9e6cb167258aefcf0d6cc4b05e02a31edf9
Flags: needinfo?(padenot)
I've stopped working on the larger cleanup of this (it was mostly ready, but we have other issues), so I think it's good if you land this one. Good find in any case!
Flags: needinfo?(padenot)
Depends on: 1449178
Pushed by bvandyk@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a659624435e3
Add hidden pref to force CubebUtils to return null context. r=padenot
https://hg.mozilla.org/integration/autoland/rev/1dd761da1344
Add mochitests to test gUM when a cubeb context cannot be obtained. r=padenot
https://hg.mozilla.org/mozilla-central/rev/a659624435e3
https://hg.mozilla.org/mozilla-central/rev/1dd761da1344
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.