970096 - crash in mozilla::gl::GLContextProviderEGL::CreateForWindow(nsIWidget*) - mostly on Sony ICS devices

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Toolbar, defect)

Description

[Kevin Brosnan [Ex-Mozilla]]

This bug was filed from the Socorro interface and is 
report bp-847b7405-1e51-4eae-aa4f-84be02140208.
=============================================================

Some sort of startup crash.

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Do we have any STR here? I've never seen this myself, AFAIK.

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

Seems to be common on Sony Ericsson ICS phones. There is a long tail of other manufactures. This can be seen on the "Signature Summary" page -> "Mobile Devices" at https://crash-stats.mozilla.com/report/list?product=FennecAndroid&signature=mozilla%3A%3Agl%3A%3AGLContextProviderEGL%3A%3ACreateForWindow(nsIWidget*) 

Sony Ericsson LT18i
Sony Ericsson MT11i
Sony Ericsson SK17i
Sony Ericsson WT19a
Sony Ericsson ST15i
Sony Ericsson WT19i
Sony Ericsson MT25i
Sony Ericsson MT15i
Sony Ericsson MK16i
Sony Ericsson ST18i

We don't have these phones, likely would require scouring eBay or similar.

Comments do not contain anything useful nor do URLs.

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

So this appears to be due to a MOZ_CRASH() that gets hit when we are unable to find a usable EGLConfig. It looks like if the screen depth is 24bit, we only try to get a 24bit EGLConfig. Maybe these devices can only support a 16bit one?

QA, can we try to get one of these devices please? In the mean time, I guess we could try a speculative patch.

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Try a 16 bit OpenGL context as a last result on Android

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

Since, in the fallback case, it seems that we're willing to continue with whichever R/G/B channels depth we'll get, why bother specifying them in the attribs list at all?

Comment 6

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 8380817 [details] [diff] [review]
Try a 16 bit OpenGL context as a last result on Android

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

but, if you need a 2-line patch to land to see if this fixes crash-stat, then sure.

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2f0a52a545af

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Benoit Jacob [:bjacob] from comment #5)
> Since, in the fallback case, it seems that we're willing to continue with
> whichever R/G/B channels depth we'll get, why bother specifying them in the
> attribs list at all?

It's not clear to me that the configs would be sorted in any meaningful way if we leave those out entirely. So we'd have to go through and figure out which one is best, which is pretty ugh. I could be wrong, though? If we just request 565, will it return 888 if that's available?

Comment 9

[Benoit Jacob [:bjacob] (mostly away)]

It is "pretty ugh" indeed, but it is what we are supposed to be doing; maybe that's what we should be fixing!

I don't know at the moment is the size attributes require == or >= matching.

Comment 10

[James Willcox (:snorp) ([e-mail]) (he/him)]

http://www.khronos.org/registry/egl/sdk/docs/man/xhtml/eglChooseConfig.html

The comments there about sort order seem to indicate that maybe it's >=, but then how would our 16bit thing ever work right now if a driver supports 24? Changing stuff like this makes me very nervous, because it's so fragile and seemingly device-dependent.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2f0a52a545af

Comment 12

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8380817 [details] [diff] [review]
Try a 16 bit OpenGL context as a last result on Android

There are no crashes on m-c from this bug, so we'll need to uplift it to see if it fixes things or not.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: Startup crashes on some phones
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): None
String or IDL/UUID changes made by this patch: None

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4fc176db9f50
https://hg.mozilla.org/releases/mozilla-beta/rev/3d2e069fb893

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/3d2e069fb893