crash in gfxContext::gfxContext(mozilla::gfx::DrawTarget*)

RESOLVED FIXED in Firefox 30

Status

()

defect
--
major
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: gcp, Assigned: mattwoodrow)

Tracking

(5 keywords)

29 Branch
mozilla32
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox26 wontfix, firefox27 wontfix, firefox28+ wontfix, firefox29+ wontfix, firefox30+ verified, firefox31+ verified, firefox32+ verified, b2g-v1.4 fixed, b2g-v2.0 fixed, fennec30+)

Details

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

Attachments

(5 attachments)

Reporter

Description

6 years ago
This bug was filed from the Socorro interface and is 
report bp-04e6ef64-4d0e-4431-add4-c5f9d2131221.
=============================================================
maybe dupe of 914134?
Component: Graphics, Panning and Zooming → Graphics
Product: Firefox for Android → Core
Version: Firefox 29 → 29 Branch
Reporter

Comment 2

6 years ago
It's crashing because the argument to that function is null. Looking at the call stack, the caller one level up is different. So maybe not, though I'm not really familiar with this code.

Updated

6 years ago
Crash Signature: [@ gfxContext::gfxContext(mozilla::gfx::DrawTarget*)] → [@ gfxContext::gfxContext(mozilla::gfx::DrawTarget*)]
Reporter

Comment 3

6 years ago
I'm now hitting this once a day on my 2013 Nexus 7. No solid STR yet though.
Keywords: regression
Reporter

Comment 4

6 years ago
And it's now the nr. 1 topcrasher on Android Nightly.
Keywords: topcrash
Looks like CreateOffscreenContentDrawTarget is returning null?
(In reply to Gian-Carlo Pascutto (:gcp) from comment #3)
> I'm now hitting this once a day on my 2013 Nexus 7. No solid STR yet though.

If I, on my Fennec nightly on HTC One do the following:

1. Load http://uverseonline.att.net/live
2. Pan, zoom, click buttons

...I'll crash in about 5-8 seconds, typically, with the following incident:

https://crash-stats.mozilla.com/report/index/7c04159e-5aaf-4c16-a061-c70cb2131227
This crash is affecting desktop Firefox as well. There reports filed against 27betas, 28 and 29 nightlies.

#23 on 27.0b topcrash list.
OS: Android → All
Definitely a topcrash on Desktop in Aurora and Beta but strangely not Nightly:
 * Firefox 29: N/A (only 5 crashes reported in 7 days)
 * Firefox 28: #6 @ 1.63%
 * Firefox 27: #19 @ 0.45%
 * Firefox 26: N/A (only 3 crashes reported in 7 days)

Is it possible this was inadvertently fixed on Nightly?
Reporter

Comment 9

6 years ago
>Is it possible this was inadvertently fixed on Nightly?

It would have to be fixed after 2014-1-11 then because my Nightly just crashed with this signature.
(In reply to Gian-Carlo Pascutto (:gcp) from comment #9)
> >Is it possible this was inadvertently fixed on Nightly?
> 
> It would have to be fixed after 2014-1-11 then because my Nightly just
> crashed with this signature.

Well, to be clear I was more referring to the prevalence of the crash (ie. going from hundreds of crashes a day to 5 crashes a week). Do you have a testcase which reliably reproduces this in Desktop Nightly?
Reporter

Comment 11

6 years ago
Nope, no easy/reliable STR.
Updating the status flags to reflect comment 8.
Duplicate of this bug: 914134
I think before we can make any progress on this bug we'll need reliable steps to reproduce, either on Android or Desktop. Right now this seems to be way more prevalent on Android. I'm not sure how we typically handle signatures that affect Android and Desktop though -- should this be split into two bugs?
Keywords: steps-wanted
I've been trying quite a bit today, and what was dead simple to cause a crash in comment 6 didn't do that for me, now.
Hit this crash three times today by zooming in on the moz Stumbler map

Updated

6 years ago
Duplicate of this bug: 956093
Firefox 27 releases tomorrow so this is wontfix for Firefox 26.
I have been able to crash nightly by scrolling down http://fontwalk.de/03/ when I get to the broadcast antenna part Firefox crashes.
I have reproduced this on two different Nexus 5 phones one using 27 release and one using nightly. Both crash in the same spot.
Milan, can we get an assignee?
Flags: needinfo?(milan)
I can also crash Firefox on Android by clicking around in Google maps street view. https://crash-stats.mozilla.com/report/index/5752b621-00a1-4516-9ecf-6c70e2140225
I'm sorry everybody, I just realized I didn't immediately reply to this. No assignee from the graphics team until we clear 3/17 date.  We can either look at it during Aurora, or Brad can try and find somebody on his team to look at it.
Anything you can look into Snorp?
Flags: needinfo?(snorp)
Flags: needinfo?(blassey.bugs)
I will try to look. Chris, if you have cycles, that would be good too.
Flags: needinfo?(snorp) → needinfo?(chrislord.net)
Assignee: nobody → chrislord.net
Flags: needinfo?(chrislord.net)
Flags: needinfo?(blassey.bugs)
As long as Chris is on tiling, and tiling hasn't landed, this waits.  See comment 23.
Assignee: chrislord.net → nobody
Dan, can you take a look when you're short of B2G bugs.
Flags: needinfo?(milan) → needinfo?(dglastonbury)
Flags: needinfo?(milan)
Benoit, can you look at this as well, then hand it off to Dan afterwards.
Flags: needinfo?(bjacob)
We've missed the FF28 cutoff, leaving tracking for 29/30 to keep this on our radar, it would be a great fix to have in the next release if ready in time.
Following STR in Comment 16, I got a crash in an approx. week old Fennec that I had on my Nexus 4. When I built a fresh debug and opt version to run in the debugger, I can't repo with the same steps. Will keep investigating.
Flags: needinfo?(dglastonbury)
Performing a quick code review, Kats is correct in #Comment 5, if CreateOffscreenContentDrawTarget returns NULL, then this code is going to crash. I can patch the gfxContext constructor to not crash but then BasicTiledLayerBuffer::PaintThebes() will crash because it doesn't handle CreateOffscreenContentDrawTarget failure either.
I am on vacation for a week, and Dan already commented on it above, so I believe that the needinfo can be cleared for now :)
Flags: needinfo?(bjacob)
(In reply to Dan Glastonbury :djg :kamidphish from comment #31)
> Performing a quick code review, Kats is correct in #Comment 5, if
> CreateOffscreenContentDrawTarget returns NULL, then this code is going to
> crash. I can patch the gfxContext constructor to not crash but then
> BasicTiledLayerBuffer::PaintThebes() will crash because it doesn't handle
> CreateOffscreenContentDrawTarget failure either.

Hmm. So it looks like this may be just another OOM class of bugs? Ugh.
If it is OOM, :snorp, what's our precedent?  Just protect against it up the stack as in comment 31?
Assignee: nobody → dglastonbury
Flags: needinfo?(milan)
Crash Signature: [@ gfxContext::gfxContext(mozilla::gfx::DrawTarget*)] → [@ gfxContext::gfxContext(mozilla::gfx::DrawTarget*)] [@ gfxContext::gfxContext(mozilla::gfx::DrawTarget*, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits> const&)]
Duplicate of this bug: 978492

Comment 36

5 years ago
STR Suggestion: I have managed to get OOMs using Google Street View (GSV) and Browsermark.

I just caused a crash in "mozalloc_abort(char const*)" but Socorro has yet to crossreference it to here. 

Currently it is: FennecAndroid 31.0a1 Crash Report [@ mozalloc_abort(char const*) | mozalloc_handle_oom(unsigned int) | moz_xrealloc | nsStreamLoader::WriteSegmentFun(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*) ] that is bp-1ccc5870-ae10-4ecc-bb7e-ae1282140326 .

In Bug 758654 Comment 8 I mentioned that I could reproduce using GSV and referenced these BPs: bp-9f773aba-3573-4999-91c8-81b722140319 and bp-f28a55a6-841d-4b56-b9f6-943002140316 .

.

After the last Update (to Nightly, on Android) GSV is so badly broken that I could not get it to crash, but Browsermark was very cooperative (that sounds funny).

It you can get GSV (Desktop Mode) to run on Android or use http://browsermark.rightware.com/ then you should get a GL or OOM crash fairly quickly.


PS: GSV is entirely unusable after that last Update, trying to walk the trails in the Grand Canyon is impossible (my CellPhone has a 1080P Screen so I am not crazy to try) and Google is not letting us get zoomed in so close the we can jump into Street View (nor is 'the little Man' showing up).

It looks like they are locking us out based on either Browser Version or Headers.

Thus, your ability to get a crash using GSV is greatly impaired (but it worked fine a few days ago). BrowserMark is working well (or crashing easily) so I suggest you try that STR.
FYI, there is a bit more than a week for a potential fix in 29.
Do we have any update here?
Status: NEW → ASSIGNED
I am trying to work on a regression range but it is slow.
I'm not really the best person to work on this bug. Is it OK to assign to Snorp?
Flags: needinfo?(milan)
James, can you give us a hand with this one?  Chris is off for a couple of weeks, want to see if this can be looked at before the trains change...
Flags: needinfo?(milan) → needinfo?(snorp)
On trunk this also seems to show up as [@ libxul.so@$ADDRESS | jsimd_ycc_rgb_convert]
Crash Signature: [@ gfxContext::gfxContext(mozilla::gfx::DrawTarget*)] [@ gfxContext::gfxContext(mozilla::gfx::DrawTarget*, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits> const&)] → [@ gfxContext::gfxContext(mozilla::gfx::DrawTarget*)] [@ gfxContext::gfxContext(mozilla::gfx::DrawTarget*, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits> const&)] [@ libxul.so@0xe00338 | jsimd_ycc_rgb_convert] [@ libxul.so@0xe042f8 | jsimd_ycc_rg…
Yeah, I can look at it.
Flags: needinfo?(snorp)
(In reply to Kevin Brosnan [:kbrosnan] from comment #43)
> On trunk this also seems to show up as [@ libxul.so@$ADDRESS |
> jsimd_ycc_rgb_convert]

Wait, what? How is that the same as this?
I got this to crash with a local build while wildly zooming around the mozstumbler map (https://location.services.mozilla.com/map#2/15.0/10.0).

We tried to allocate a 6144x8192 surface via gfxPlatform::CreateOffscreenSurface(). That's 200MB or there abouts, so probably going to fail (and it did in this case).

This dimension comes from the passed-in paint region, which in this case was {x1 = -4816, y1 = -3136, x2 = 1328, y2 = 5056}. It seems like that needs to be clipped to the displayport maybe? I am not sure how those bits work.
BenWa/Matt can you suggest a fix here? I don't really know anything about this tiling stuff and Chris is out for a while. We're getting here[1] and the invalidRegion (and mVisibleRegion) is gigantic. Shouldn't that be clipped? Where? To what?

[1] http://dxr.mozilla.org/mozilla-central/source/gfx/layers/client/ClientTiledThebesLayer.cpp#234
Flags: needinfo?(matt.woodrow)
Flags: needinfo?(bgirard)
It sounds like we're building a layer that's just too big. The next step is to look inside FrameLayerBuilder to understand why we're requesting a layer that big.
Flags: needinfo?(bgirard)
Can you check if the patch I posted on bug 993554 fixes this?
Flags: needinfo?(snorp)
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #49)
> Can you check if the patch I posted on bug 993554 fixes this?

Yup, I'll try it.
Flags: needinfo?(snorp)
Wontfix for 29 as we've already shipped this several times and it's not worth the risk of taking something like this in a final beta with no real-world testing.  Nominate for uplift to 30 if the patch in comment 49 works and the patch is low risk.
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #50)
> (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #49)
> > Can you check if the patch I posted on bug 993554 fixes this?
> 
> Yup, I'll try it.

Didn't seem to help.
This looks interesting:

I/Gecko   (27186): Painting --- retained layer tree:
I/Gecko   (27186): ClientLayerManager (0x85895100)
I/Gecko   (27186):   ClientContainerLayer (0x84c26400) [visible=< (x=0, y=0, w=768, h=1134); >] [metrics={ viewport=(x=0.000000, y=0.000000, w=384.000000, h=567.000000) viewportScroll=(x=0.000000, y=0.000000) displayport=(x=0.000000, y=0.000000, w=0.000000, h=0.000000) scrollableRect=(x=0.000000, y=0.000000, w=384.000000, h=567.000000) scrollId=0 }]
I/Gecko   (27186):     ClientContainerLayer (0x891e3800) [clip=(x=0, y=0, w=768, h=1134)] [visible=< (x=0, y=-338, w=768, h=2021); >] [opaqueContent] [metrics={ viewport=(x=0.000000, y=0.000000, w=384.000000, h=567.000000) viewportScroll=(x=0.000000, y=169.000000) displayport=(x=0.000000, y=-169.000000, w=384.000000, h=1010.500000) scrollableRect=(x=0.000000, y=0.000000, w=384.000000, h=1010.500000) scrollId=3 }]
I/Gecko   (27186):       ClientThebesLayer (0x891e3c00) [transform=[ 1 0; 0 1; 0 -338; ]] [visible=< (x=0, y=0, w=768, h=2021); >] [opaqueContent] [valid=< (x=0, y=0, w=768, h=2021); >]
I/Gecko   (27186):       ClientContainerLayer (0x8b4eb400) [clip=(x=20, y=448, w=600, h=600)] [transform=[ 1 0; 0 1; 20 448; ]] [visible=< (x=0, y=0, w=600, h=600); >]
I/Gecko   (27186):         ClientContainerLayer (0x8adf9c00) [transform=[ 5.54308 0; 0 5.54308; -969.013 -1458.71; ]] [visible=< (x=-96, y=-1008, w=6144, h=6144); >] [preScale=0.125, 0.125]
I/Gecko   (27186):           ClientThebesLayer (0x8adfa000) [visible=< (x=1398, y=2105, w=867, h=867); >] [valid=< (x=1398, y=2105, w=867, h=867); >]
I/Gecko   (27186):         ClientContainerLayer (0x8b0de400) [transform=[ 5.54308 0; 0 5.54308; -969.013 -1458.71; ]] [visible=< (x=-96, y=-1008, w=6144, h=6144); >] [preScale=0.125, 0.125]
I/Gecko   (27186):           ClientThebesLayer (0x8b0de800) [visible=< (x=1398, y=2105, w=867, h=867); >] [valid=< (x=1398, y=2105, w=867, h=867); >]
I/Gecko   (27186):         ClientContainerLayer (0x8a6dc800) [transform=[ 5.54308 0; 0 5.54308; -969.013 -1458.71; ]] [visible=< (x=-96, y=-1008, w=6144, h=6144); >] [preScale=0.125, 0.125]
I/Gecko   (27186):           ClientThebesLayer (0x8a6dcc00) [visible=< (x=1398, y=2105, w=867, h=867); >] [valid=< (x=1398, y=2105, w=867, h=867); >]
I/Gecko   (27186):       ClientThebesLayer (0x8a6e1800) [transform=[ 1 0; 0 1; 0 -338; ]] [visible=< (x=40, y=806, w=56, h=108); (x=548, y=1314, w=52, h=14); (x=30, y=1328, w=570, h=38); (x=30, y=1366, w=188, h=10); >] [componentAlpha] [valid=< (x=40, y=806, w=56, h=108); (x=548, y=1314, w=52, h=14); (x=30, y=1328, w=570, h=38); (x=30, y=1366, w=188, h=10); >]
I/Gecko   (27186):
I/Gecko   (27186): ---- PAINT END ----

There are several container layers where the visible region is 6144x6144, but the underlying thebes layer appears to have sane bounds. Somehow it eventually gets set to that of the parent, though (flattening?)
If you also dump the display items it could help.
I have it MOZ_CRASH()ing when it tries to allocate anything larger than 4096x4096, so that's what you see in this log.
Assignee

Comment 57

5 years ago
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #53)
> This looks interesting:

> I/Gecko   (27186):         ClientContainerLayer (0x8adf9c00) [transform=[
> 5.54308 0; 0 5.54308; -969.013 -1458.71; ]] [visible=< (x=-96, y=-1008,
> w=6144, h=6144); >] [preScale=0.125, 0.125]
> I/Gecko   (27186):           ClientThebesLayer (0x8adfa000) [visible=<
> (x=1398, y=2105, w=867, h=867); >] [valid=< (x=1398, y=2105, w=867, h=867);

> 
> There are several container layers where the visible region is 6144x6144,
> but the underlying thebes layer appears to have sane bounds. Somehow it
> eventually gets set to that of the parent, though (flattening?)

Because the ContainerLayer is scaling up (by a factor of around 5.5) we fold that down into the child and draw it at a higher resolution.

The resolution change isn't printed here, but we can see the 'pre scale' we apply to cancel out the resolution change on the child. preScale of 0.125 = 8x increased resolution on the ThebesLayer.

So we'd be looking to allocate 6936x6936 as the buffer for our ThebesLayer. Not good.

What's weird, is that we should still limit the visible regions (and thus drawing/allocation) to what will actually be visible on the screen/display port. I'm pretty sure 6144x6144 does not fit that criteria.

The only exception to that should be here: http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDisplayList.cpp#4807

But that should only kick on for transform items that are at most 1/8th bigger than the size of the screen.

Figuring out why the ContainerLayer has such a big visible region would be useful.

We also probably shouldn't crash if the allocation fails, and just skip drawing instead.
Flags: needinfo?(matt.woodrow)

Updated

5 years ago
Duplicate of this bug: 1001475
Assignee: dglastonbury → snorp
AFAICT, the gigantic size is coming from a display list item, but I don't understand this stuff enough to understand why it isn't being clipped. Matt is probably going to have a much easier time fixing this.

I agree that we should just not draw if we fail to allocate the surface, though, so I can at least put together a patch for that.
Assignee: snorp → matt.woodrow
Assignee

Comment 60

5 years ago
Took me a while to figure out that display dump.

There's a hidden 2x scale on the root ContainerLayer for the current zoom/resolution. It's hidden because it gets folded down into the children, and ThebesLayers don't print what resolution factor gets applied to them.

In the dump *before* the crash we can see preScale=0.25, so the inverse of that has been applied to the children. An 8x scale is double what we'd otherwise expect since the transform scale is 2.2 (which gets rounded to 4x).

I think we're deciding to pre-render this content because the untransformed size of the content is smaller than the viewport [1]. We also try to check the post-transform size of the content in dev pixels [2] (which was added for bug 717521) but this isn't going to see the extra resolution on the parent document.

So we decide to pre-render the entire transform, not realizing how big that actually might be once we factor the current zoom in. Then we get ridiculous sized ThebesLayers and all goes downhill.

I'll think about the best way to fix this.

[1] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDisplayList.cpp#4653
[2] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDisplayList.cpp#4656
Assignee

Comment 61

5 years ago
Posted file Ugly Testcase
Turns out it's not only resolutions that throw this check off, parent transforms do too.

This test case is trying to allocate a 25k x 20k ThebesLayer on my macbook pro.

Looks like we handle this 'gracefully' for the non-tiled case by not crashing when the allocation fails and just fail to draw anything.
Assignee

Updated

5 years ago
Attachment #8415159 - Attachment is patch: false
Attachment #8415159 - Attachment mime type: text/plain → text/html
(In reply to Matt Woodrow (:mattwoodrow) from comment #61)
> Created attachment 8415159 [details]
> Ugly Testcase
> 
> Turns out it's not only resolutions that throw this check off, parent
> transforms do too.
> 
> This test case is trying to allocate a 25k x 20k ThebesLayer on my macbook
> pro.
> 
> Looks like we handle this 'gracefully' for the non-tiled case by not
> crashing when the allocation fails and just fail to draw anything.

How long would this layer typically hang around? Until the transform is removed? Another terrible case here is when the allocation actually succeeds and we have this boat anchor of a layer weighing us down.
Assignee

Comment 63

5 years ago
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #62)
 
> How long would this layer typically hang around? Until the transform is
> removed? Another terrible case here is when the allocation actually succeeds
> and we have this boat anchor of a layer weighing us down.

Yes exactly.

I think we need to do three separate things here:

* Not crash if the allocation fails. Even with reasonable sized surfaces, it's still can be a large allocation so we shouldn't need to crash in low memory situations. We can wait until much smaller allocations start failing before throwing in the towel.

* Have ThebesLayers not respect their requested resolution if it's going result in an excessively large surface. This is a quality/memory usage tradeoff.

* Be smarter about when we pre-render transformed layers. Agreeing to pre-render 6k x 6k of content is not ok.
Assignee

Comment 64

5 years ago
Posted patch Don't crashSplinter Review
The first one of my suggestions.

This should bring mobile's behaviour in line with what we do on desktop.
Attachment #8415645 - Flags: review?(nical.bugzilla)
Assignee

Comment 65

5 years ago
I think this makes the most sense. We usually attempt to draw content at it's final resolution (or larger even), so we want to decide to prerender based on it's size in screen coordinate space.

This shouldn't break the original intent of this code (B2G homescreen) since that's only using a translation not scaling.
Attachment #8415694 - Flags: review?(tnikkel)
Attachment #8415694 - Flags: review?(tnikkel) → review+
Attachment #8415645 - Flags: review?(nical.bugzilla) → review+
Assignee

Comment 67

5 years ago
I've decided not to bother with the second idea for now since we should no longer request anything excessively large to be pre-rendered.

It's possible in the future we might change that to intentionally pre-render huge areas, and then we'd need the FrameLayerBuilder change to choose a manageable resolution to respect that.
https://hg.mozilla.org/mozilla-central/rev/29349a955780
https://hg.mozilla.org/mozilla-central/rev/660cc013d558
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
There are no reports of this crash after May-2. This would be a candidate for b2 which goes to build Monday, this crash accounts for 22% of all beta 1 crashes on Android.
tracking-fennec: --- → 28+

Comment 70

5 years ago
Given we tracked this on the previous trains and the fix working on 32, let's get this on the 31 radar as well. :)

Matt, Kevin, awesome to hear the fix works!

Matt, can we get approval requests for uplift?
Flags: needinfo?(matt.woodrow)
tracking-fennec: 28+ → ?
Assignee

Comment 71

5 years ago
Comment on attachment 8415645 [details] [diff] [review]
Don't crash

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Unknown.
User impact if declined: Crashes when zooming
Testing completed (on m-c, etc.): Been on m-c for a week, verified crash rate has dropped.
Risk to taking this patch (and alternatives if risky): The first patch is just a null check and will replace the crash with broken rendering. Very low risk. The second patch changes our prerendering logic to avoid getting into this situation at all. Still should be low risk, but we could take only the first patch if necessary. 
String or IDL/UUID changes made by this patch: None
Attachment #8415645 - Flags: approval-mozilla-beta?
Attachment #8415645 - Flags: approval-mozilla-aurora?
Flags: needinfo?(matt.woodrow)
Attachment #8415645 - Flags: approval-mozilla-beta?
Attachment #8415645 - Flags: approval-mozilla-beta+
Attachment #8415645 - Flags: approval-mozilla-aurora?
Attachment #8415645 - Flags: approval-mozilla-aurora+
tracking-fennec: ? → 30+
matt, this doesn't appear to be fixed on Desktop beta despite the fix landing on beta prior to going to build for 30b3.  When I say *this*, I mean the main signature - gfxContext::PushClipsToDT(mozilla::gfx::DrawTarget*)  

Should we reopen this bug or file a new one?
Flags: needinfo?(matt.woodrow)
Tracy, I don't see PushClipsToDT mentioned anywhere on this bug. I thought that was bug 805406 and bug 974656.
Monday morning brain fart, not sure how I got those signatures crossed.

I am not seeing the reported crash signature in beta3.  The secondary signature is still present as bsmedberg has shown.
Assignee

Comment 78

5 years ago
The remaining crashes appear to be a different callstack, coming from ClippedImage.cpp.

That should be filed as a new bug, and cc :seth.
Flags: needinfo?(matt.woodrow)
No crashes on FF 32 in the crashstats in the last month.
The testcase seems to work ok too, 32.0a1 (2014-05-12) Win 7 x64, OS X 10.8.5.
Verified fixed.
Status: RESOLVED → VERIFIED
This should still be verified on 31 and 30, and on Android. 
Aaron is there anything you can add?
Flags: needinfo?(aaron.train)
Crash stats data should detail verification.
Flags: needinfo?(aaron.train)
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #82)
> Some of the reports show this happening with builds as recent as yesterday's
> Nightly. Based on data I think it's safe to say this has been mitigated but
> I don't think it has been fixed. Should we mark this verified and clone this
> bug to deal with the lingering crashes?

That's from a new call stack, see comment #78.

Comment 84

5 years ago
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #82)
> Some of the reports show this happening with builds as recent as yesterday's
> Nightly. Based on data I think it's safe to say this has been mitigated but
> I don't think it has been fixed. Should we mark this verified and clone this
> bug to deal with the lingering crashes?

Anthony, this fix was for the Android case (see this being marked as an Android topcrash), there may be other cases with this signature, esp. on other platforms.
Please file a new bug.
Flags: needinfo?(matt.woodrow)

Comment 87

5 years ago
(In reply to Alexandra Lucinet, QA Mentor [:adalucinet] from comment #85)
> There are 9 crashes for 30 beta 7 (Build ID: 20140522105902) and 3 for
> Aurora 31.0a1 (from 2014-05-23) - as far as I can tell, all of them are
> coming from ClippedImage.cpp (as per comment 78).

Please verify this based on ANDROID. We should have desktop crashes tracked by a different bug.
Whiteboard: [native-crash]
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #87)
> Please verify this based on ANDROID. We should have desktop crashes tracked
> by a different bug.

Right, I rechecked this and there are 0 crashes across all Android branches. I'll clone this bug for the Windows-specific crashes.
Keywords: verifyme
OS: All → Android

Updated

5 years ago
Status: VERIFIED → REOPENED
Resolution: FIXED → ---

Comment 89

5 years ago
Looks like this is causing bug 1028216 and we need a fix or a backout.
Depends on: 1028216
There are two patches on this bug - one that fixes that crash, and one that makes things better.  It's only the second one that is currently identified as a cause of 1028216, but I'm asking in that bug exactly what the fallout is and will act on this once we have a bit more detail.  I'm not sure this bug should be reopened though, none of the issues are reproducible.  If we do decide to back it out, together with the crash fix as well, then we should reopen?

Comment 91

5 years ago
I do not think we should back out the crash fix, unless we find it's doing something really completely wrong. This was a really high-volume crash on Firefox for Android, and was uplifted to the Gecko 30 train but we have no reports of any B2G issues with that train that would be caused by this fix. The only thing in question is the second patch.
Assignee

Comment 92

5 years ago
I'll look into this tomorrow.
This bug is back to fixed.  The backout of the second patch will happen in bug 1028216, but the crash stays fixed, so can this bug.
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Duplicate of this bug: 1035399
You need to log in before you can comment on or make changes to this bug.