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

RESOLVED FIXED in Firefox 28, Firefox OS v1.3

Status

()

Core
Graphics
--
critical
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: kbrosnan, Assigned: bjacob)

Tracking

({crash, verifyme})

Trunk
mozilla29
All
Android
crash, verifyme
Points:
---

Firefox Tracking Flags

(firefox27 unaffected, firefox28+ fixed, firefox29+ fixed, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed, fennec28+)

Details

(Whiteboard: [native-crash][leave open], crash signature)

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
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)
Keywords: regressionwindow-wanted, steps-wanted
(Reporter)

Comment 1

5 years ago
We can't get a regression window till we have steps. The First appeared info might give a close approximation.
Keywords: regressionwindow-wanted
(Assignee)

Comment 2

5 years ago
Yeah, don't worry about that. Pretty clear what caused this, and what is going on here. I have a patch coming up.
tracking-firefox28: ? → +
tracking-firefox29: ? → +
Assignee: nobody → bjacob
tracking-fennec: ? → 28+
(Assignee)

Comment 3

5 years ago
Created attachment 8357923 [details] [diff] [review]
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
Attachment #8357923 - Flags: review?(jmuizelaar)
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.
Attachment #8357923 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 5

5 years ago
Created attachment 8357952 [details] [diff] [review]
patch for landing (r=jrmuizel)
Attachment #8357952 - Flags: review+
(Assignee)

Comment 6

5 years ago
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.
Whiteboard: [native-crash] → [native-crash][leave open]
Target Milestone: --- → mozilla29
Did the landed patch work? Can we mark 29 fixed and get uplift to beta if so?
Flags: needinfo?(bjacob)
Keywords: verifyme
(Reporter)

Comment 9

5 years ago
From crash stats it looks like it did. There are no crashes in 29a2 were as there were 11 crashes on 28a2.
Thanks for checking crash stats.
Flags: needinfo?(bjacob)
Let's get this on Beta then, if it's low risk enough for uplift - Benoit can you nominate?
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-firefox29: affected → fixed
Flags: needinfo?(bjacob)
Resolution: --- → FIXED
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
Attachment #8357952 - Flags: approval-mozilla-beta?
Flags: needinfo?(bjacob)
Comment on attachment 8357952 [details] [diff] [review]
patch for landing (r=jrmuizel)

Thanks!
Attachment #8357952 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Should I land myself?
status-b2g-v1.3: --- → fixed
status-b2g-v1.4: --- → fixed
status-b2g-v1.3T: --- → fixed
Issue is resolved - clearing old keywords - qa-wanted clean-up
Keywords: steps-wanted
You need to log in before you can comment on or make changes to this bug.