598197 - Don't setup audio when we can't get the JNIEnv

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Michael Wu [:mwu]]

Attachment: Fix

Otherwise we crash. This happens in the child process.

Comment 1

[Matthew Gregan [:kinetik]]

Comment on attachment 476958 [details] [diff] [review]
Fix

I'm not clear when this can happen, but would it make more sense to return SA_ERROR_NO_INIT when GetJNIForThread fails?

Comment 2

[Michael Wu [:mwu]]

(In reply to comment #1)
> Comment on attachment 476958 [details] [diff] [review]
> Fix
> 
> I'm not clear when this can happen, but would it make more sense to return
> SA_ERROR_NO_INIT when GetJNIForThread fails?
Sure. For the first or both cases?

This happens in the child/content process where there is no JVM running.

Comment 3

[Matthew Gregan [:kinetik]]

Actually, looking at the other implementations, SA_ERROR_NO_DEVICE might make slightly more sense.  Just for the first case, I think.  I'm undecided about the second case--if GetJNIForThread fails in destroy, it must've also failed in create, right?  Therefore there's nothing to destroy and no error.

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 476958 [details] [diff] [review]
Fix

SA_ERROR_NOT_SUPPORTED makes more sense to me than SA_ERROR_NO_INIT (which I take to mean that we're not initialized). We don't support playing audio from the unprivileged processes. Though we could potentially if we remoted this.

Comment 5

[Michael Wu [:mwu]]

http://hg.mozilla.org/mozilla-central/rev/e90a6b6e4511

I used SA_ERROR_NO_DEVICE.