957437 - crash in mozilla::widget::GfxInfo::GLStrings::EnsureInitialized()

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Kevin Brosnan [Ex-Mozilla]]

This bug was filed from the Socorro interface and is 
report bp-ba455d76-fd2c-4ef7-8892-cb7102140106.
=============================================================

First appeared 2013-12-22

This has crashed on x86 and ARM Android devices. Various Android versions. Only URL is http://www.deviantart.com/messages/notes/#1_0 at this time. 

samsung   Nexus S               16 (REL)
HTC       Desire HD             10 (REL)
unknown   AN10G2                 9 (REL)
samsung   GT-I9000              18 (REL)
unknown   android                9 (REL)
HTC       HTC One               18 (REL)
HTC       HTC Desire HD A9191   10 (REL)

Comment 1

[Kevin Brosnan [Ex-Mozilla]]

We can't get a regression window till we have steps. The First appeared info might give a close approximation.

Comment 2

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

Yeah, don't worry about that. Pretty clear what caused this, and what is going on here. I have a patch coming up.

Comment 3

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

Attachment: Fix EnsureInitialized

Two things:
 - Size 1x1 might be what causes issues here; switching to size 16x16 to match what we do in GLContextProvider::CreateOffscreen
 - Handle gracefully the case where GLContext creation fails and gl is null

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8357923 [details] [diff] [review]
Fix EnsureInitialized

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

::: widget/android/GfxInfo.cpp
@@ +81,5 @@
> +      // failed (and bug 957437 shows that that can happen) then it might
> +      // fail again in the future, and we don't want to keep retrying.
> +      // So we just carry on with empty strings.
> +      // In the future we might decide to blacklist everything GL-related
> +      // (aside from GL layers) in that case. For now, it doesn't matter much.

I think we should do this now. If we don't know anything about the system we're on we should blacklist everything we can.

It would also be good to add a comment about a future solution of having the compositor update gfxInfo with proper strings once it gets a glcontext.

Comment 5

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

Attachment: patch for landing (r=jrmuizel)

Comment 6

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/f138d20db7d3

Let's leave this open until we get a confirmation that this fixes real-world crashiness. There is a lot of finger-crossing involved here.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Did the landed patch work? Can we mark 29 fixed and get uplift to beta if so?

Comment 9

[Kevin Brosnan [Ex-Mozilla]]

From crash stats it looks like it did. There are no crashes in 29a2 were as there were 11 crashes on 28a2.

Comment 10

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

Thanks for checking crash stats.

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

Let's get this on Beta then, if it's low risk enough for uplift - Benoit can you nominate?

Comment 12

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

Comment on attachment 8357952 [details] [diff] [review]
patch for landing (r=jrmuizel)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 937204
User impact if declined: crashes
Testing completed (on m-c, etc.): already on aurora for a while
Risk to taking this patch (and alternatives if risky): low risk, well tested already
String or IDL/UUID changes made by this patch: none

Comment 13

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 8357952 [details] [diff] [review]
patch for landing (r=jrmuizel)

Thanks!

Comment 14

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

Should I land myself?

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/90ed118bdf5f

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/90ed118bdf5f

Comment 17

[Joshua Mitchell (Inactive)]

Issue is resolved - clearing old keywords - qa-wanted clean-up