Closed
Bug 671960
Opened 13 years ago
Closed 13 years ago
Crash [@ _moz_cairo_surface_get_device_offset ]
Categories
(Core :: Graphics, defect)
Tracking
()
VERIFIED
FIXED
mozilla8
People
(Reporter: nhirata, Assigned: jdm)
Details
(Whiteboard: [inbound])
Crash Data
Attachments
(1 file, 2 obsolete files)
4.99 KB,
patch
|
joe
:
review+
dougt
:
review+
jrmuizel
:
review+
christian
:
approval-mozilla-aurora+
christian
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Frame Module Signature [Expand] Source 0 libxul.so _moz_cairo_surface_get_device_offset gfx/cairo/cairo/src/cairo-surface.c:1168 1 libxul.so gfxASurface::GetDeviceOffset gfx/thebes/gfxASurface.cpp:278 2 libxul.so nsWindow::DrawTo widget/src/android/nsWindow.cpp:941 3 libxul.so nsWindow::OnDraw widget/src/android/nsWindow.cpp:1013 4 libxul.so nsWindow::OnGlobalAndroidEvent widget/src/android/nsWindow.cpp:830 5 libxul.so nsAppShell::ProcessNextNativeEvent widget/src/android/nsAppShell.cpp:410 6 libxul.so nsBaseAppShell::DoProcessNextNativeEvent widget/src/xpwidgets/nsBaseAppShell.cpp:172 7 libxul.so nsBaseAppShell::OnProcessNextEvent widget/src/xpwidgets/nsBaseAppShell.cpp:312 8 libxul.so mozilla::dom::ContentParent::OnProcessNextEvent dom/ipc/ContentParent.cpp:976 9 libxul.so nsThread::ProcessNextEvent nsTArray.h:139 10 libxul.so NS_ProcessNextEvent_P objdir/xpcom/build/nsThreadUtils.cpp:245 11 libxul.so mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:111 12 libxul.so MessageLoop::RunInternal ipc/chromium/src/base/message_loop.cc:219 13 libxul.so MessageLoop::Run ipc/chromium/src/base/message_loop.cc:511 14 libxul.so nsBaseAppShell::Run widget/src/xpwidgets/nsBaseAppShell.cpp:191 15 libxul.so nsAppStartup::Run toolkit/components/startup/nsAppStartup.cpp:223 16 libxul.so XRE_main toolkit/xre/nsAppRunner.cpp:3688 17 libxul.so Java_org_mozilla_gecko_GeckoAppShell_nativeRun toolkit/xre/nsAndroidStartup.cpp:132 18 libmozutils.so libmozutils.so@0x49de 19 libdvm.so libdvm.so@0x11c77 20 dalvik-LinearAlloc (deleted) dalvik-LinearAlloc @0x20a02b 21 dalvik-heap (deleted) dalvik-heap @0x73e527 22 libdvm.so libdvm.so@0x41186 23 data@app@org.mozilla.firefox_beta-1.apk@classes.dex data@app@org.mozilla.firefox_beta-1.apk@classes.dex@0xfd3d 24 libmozutils.so libmozutils.so@0x49c8 25 dalvik-LinearAlloc (deleted) dalvik-LinearAlloc @0x20a02b 26 libdvm.so libdvm.so@0x4113c 27 dalvik-heap (deleted) dalvik-heap @0x73e527 28 libdvm.so libdvm.so@0x46788 29 dalvik-LinearAlloc (deleted) dalvik-LinearAlloc @0x20a02b 30 data@app@org.mozilla.firefox_beta-1.apk@classes.dex data@app@org.mozilla.firefox_beta-1.apk@classes.dex@0x9af9 31 dalvik-heap (deleted) dalvik-heap @0x73e527 32 libdvm.so libdvm.so@0x11e3f 33 libdvm.so libdvm.so@0x16e9f 34 libdvm.so libdvm.so@0x1bd5f 35 libdvm.so libdvm.so@0x1bccf 36 dalvik-LinearAlloc (deleted) dalvik-LinearAlloc @0x20b4d7 37 libdvm.so libdvm.so@0x1ae13 38 libdvm.so libdvm.so@0x16b1b 39 core.odex core.odex@0x10097f 40 dalvik-heap (deleted) dalvik-heap @0x82b717 41 dalvik-LinearAlloc (deleted) dalvik-LinearAlloc @0x20a39f 42 dalvik-mark-stack (deleted) dalvik-mark-stack @0x4dbc86f 43 libdvm.so libdvm.so@0x9ef77 44 libdvm.so libdvm.so@0x16b7f 45 libdvm.so libdvm.so@0x16bf7 46 libdvm.so libdvm.so@0x16a9f 47 libdvm.so libdvm.so@0x16ac7 48 libdvm.so libdvm.so@0x16b1b 49 data@app@org.mozilla.firefox_beta-1.apk@classes.dex data@app@org.mozilla.firefox_beta-1.apk@classes.dex@0x6253 50 data@app@org.mozilla.firefox_beta-1.apk@classes.dex data@app@org.mozilla.firefox_beta-1.apk@classes.dex@0x6253 51 core.odex core.odex@0x155e1b 52 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x4f940 53 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x541d0 54 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x264623 55 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x25f80f 56 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x25f0c6 57 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x6deaad 58 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x25f81d 59 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x266753 60 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x6df6f3 61 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x6dec4b 62 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x6d77ea 63 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x6dec46 64 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x6d77e4 65 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x25f0c1 66 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x6d77e8 67 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x6dec47 68 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x7870d1 69 org.mozilla.firefox_beta-1.apk org.mozilla.firefox_beta-1.apk@0x541d2 70 dalvik-heap (deleted) dalvik-heap @0x552f 71 dalvik-LinearAlloc (deleted) dalvik-LinearAlloc @0x18433 Show/hide other threads More Reports: https://crash-stats.mozilla.com/report/list?range_value=7&range_unit=days&date=2011-07-15%2012%3A00%3A00&signature=_moz_cairo_surface_get_device_offset&version=Fennec%3A6.0 On : Motorola Xoom OS : verizon/trygon/stingray:3.2/HLJ86/137724:user/release-keys Uptime : < 352
Reporter | ||
Updated•13 years ago
|
Severity: normal → critical
Crash Signature: [@ _moz_cairo_surface_get_device_offset ]
OS: Mac OS X → Android
Hardware: x86 → ARM
Updated•13 years ago
|
Component: General → Graphics
Product: Fennec → Core
QA Contact: general → thebes
Updated•13 years ago
|
tracking-firefox6:
--- → ?
Updated•13 years ago
|
tracking-fennec: ? → 6+
Assignee | ||
Comment 2•13 years ago
|
||
This looks like we have a null mSurface in gfxASurface.
Assignee | ||
Comment 3•13 years ago
|
||
>72 void
>73 gfxImageSurface::InitWithData(unsigned char *aData, const gfxIntSize& aSize,
>74 long aStride, gfxImageFormat aFormat)
>75 {
>76 mSize = aSize;
>77 mOwnsData = PR_FALSE;
>78 mData = aData;
>79 mFormat = aFormat;
>80 mStride = aStride;
>81
>82 if (!CheckSurfaceSize(aSize))
>83 return;
If CheckSurfaceSize fails, we are stuck with a NULL mSurface pointer (and mSurfaceValid is false, but we don't appear to check that religiously).
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → josh
Assignee | ||
Comment 4•13 years ago
|
||
Attachment #547199 -
Flags: review?(roc)
Assignee | ||
Comment 5•13 years ago
|
||
Comment on attachment 547199 [details] [diff] [review] Ensure that gfx surfaces contain a valid mSurface pointer even if the requested dimensions are disallowed. Nevermind, I noticed that this check is repeated in a bunch of places. I'll address all of those as well.
Attachment #547199 -
Flags: review?(roc)
Comment 6•13 years ago
|
||
Joe, we're going to want this in Beta(6) and Aurora(7). I'm assuming we're not going to want the refactor patch for the release branches. If my assumption is correct, can we get the jdm's smaller fix reviewed and ready for release branch landing.
Comment 7•13 years ago
|
||
Is it the case that attachment 547199 [details] [diff] [review] will fix this bug? It's not clear from reading comment 5.
Assignee | ||
Comment 8•13 years ago
|
||
A variation of the existing obsolete patch should solve this bug, since it will ensure that all gfxImageSurfaces will contain valid cairo surfaces. I can do a belt-and-suspenders fix as well by adding a check for CairoStatus in the android nsWindow code.
Assignee | ||
Comment 9•13 years ago
|
||
A regression window here would be interesting, at the very least, to figure out why this topcrash started occurring.
Keywords: regressionwindow-wanted
Comment 10•13 years ago
|
||
(In reply to comment #9) > A regression window here would be interesting, at the very least, to figure > out why this topcrash started occurring. First appeared on crast-stats on July 14th: https://crash-stats.mozilla.com/report/index/aa3d7691-757e-44ab-aa49-f036f2110714
Comment 11•13 years ago
|
||
(In reply to comment #10) > (In reply to comment #9) > > A regression window here would be interesting, at the very least, to figure > > out why this topcrash started occurring. > > First appeared on crast-stats on July 14th: > https://crash-stats.mozilla.com/report/index/aa3d7691-757e-44ab-aa49- > f036f2110714 That is a lie
Assignee | ||
Comment 12•13 years ago
|
||
Attachment #547599 -
Flags: review?(joe)
Attachment #547599 -
Flags: review?(doug.turner)
Assignee | ||
Updated•13 years ago
|
Attachment #547199 -
Attachment is obsolete: true
Assignee | ||
Comment 13•13 years ago
|
||
My idea of creating a null cairo surface is too invasive and potentially-behaviour-changing for me to feel comfortable about it. Here's a much more reliable belt-and-suspenders solution that prevents calling cairo functions on null surfaces (some functions had the check, others didn't, so I figured me might as well go all out), as well as checking the CairoStatus of the new surfaces in the Android window impl and avoiding using them if they're not valid.
Assignee | ||
Updated•13 years ago
|
Attachment #547599 -
Attachment is obsolete: true
Attachment #547599 -
Flags: review?(joe)
Attachment #547599 -
Flags: review?(doug.turner)
Assignee | ||
Comment 14•13 years ago
|
||
I accidentally perma-reproduced this by putting Nightly into "zoom mode", where it's zoomed to fill the screen on a Xoom. FWIW.
Comment 16•13 years ago
|
||
(In reply to comment #14) > Created attachment 547600 [details] [diff] [review] [review] > Prevent calling cairo functions on invalid surfaces through gfxASurface. This seems like the wrong approach. If we want to be able to call functions on invalid surfaces we should just initialize the surface and let cairo do the checking. cairo already has the concept of a null surface.
Assignee | ||
Comment 17•13 years ago
|
||
I was seeing test failures when I tried that. For the patch to address the topcrash on Beta, I really want to go with the simples and most reliable option here.
Assignee | ||
Comment 18•13 years ago
|
||
Comment on attachment 547600 [details] [diff] [review] Prevent calling cairo functions on invalid surfaces through gfxASurface. Joe, you're on the hook for gfx/ changes, Doug's in charge of the Android window ones. Try was all green.
Attachment #547600 -
Flags: review?(joe)
Attachment #547600 -
Flags: review?(doug.turner)
Comment 19•13 years ago
|
||
Comment on attachment 547600 [details] [diff] [review] Prevent calling cairo functions on invalid surfaces through gfxASurface. On the android bits: |if (targetSurface && !targetSurface->CairoStatus())| is what we do on windows. No idea if the gfxASurface changes are good. I think you can have a null surface in cairo so, those cairo functions should do the right thing. Maybe joe or jeff know the details.
Attachment #547600 -
Flags: review?(doug.turner) → review+
Comment 20•13 years ago
|
||
Comment on attachment 547600 [details] [diff] [review] Prevent calling cairo functions on invalid surfaces through gfxASurface. Review of attachment 547600 [details] [diff] [review]: ----------------------------------------------------------------- nsWindow parts are obviously good and correct. gfxASurface I want Jeff's input.
Attachment #547600 -
Flags: review?(jmuizelaar)
Updated•13 years ago
|
Attachment #547600 -
Flags: review?(jmuizelaar) → review+
Comment 21•13 years ago
|
||
Comment on attachment 547600 [details] [diff] [review] Prevent calling cairo functions on invalid surfaces through gfxASurface. Review of attachment 547600 [details] [diff] [review]: -----------------------------------------------------------------
Attachment #547600 -
Flags: review?(joe) → review+
Assignee | ||
Comment 22•13 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/cd6bf87809e9
Whiteboard: [inbound]
Comment 23•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/cd6bf87809e9
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
Attachment #547600 -
Flags: approval-mozilla-beta?
Attachment #547600 -
Flags: approval-mozilla-aurora?
Comment 24•13 years ago
|
||
Comment on attachment 547600 [details] [diff] [review] Prevent calling cairo functions on invalid surfaces through gfxASurface. Approved for mozilla-aurora and mozilla-beta. Please land asap
Attachment #547600 -
Flags: approval-mozilla-beta?
Attachment #547600 -
Flags: approval-mozilla-beta+
Attachment #547600 -
Flags: approval-mozilla-aurora?
Attachment #547600 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 25•13 years ago
|
||
http://hg.mozilla.org/releases/mozilla-aurora/rev/6dae6e92d99e http://hg.mozilla.org/releases/mozilla-beta/rev/049e86d2547d
status-firefox6:
--- → fixed
status-firefox7:
--- → fixed
Comment 26•13 years ago
|
||
if anyone can provide reproducible steps, please comment. QA would like to verify this.
Comment 27•13 years ago
|
||
(In reply to comment #26) > if anyone can provide reproducible steps, please comment. QA would like to > verify this. oh, and we do have a ASUS TF101 EEpad Transformer (with keyboard) to hammer on. Since most of the crashes are being reported on that platform
Comment 28•13 years ago
|
||
According to crash stats, there have been no crashes in Fennec 6.0b4 while there were about 15 crashes per day in previous Beta builds. See https://crash-stats.mozilla.com/report/list?range_value=4&range_unit=weeks&signature=_moz_cairo_surface_get_device_offset&version=Fennec%3A6.0 and click the Graph tab.
Status: RESOLVED → VERIFIED
Comment 29•13 years ago
|
||
I was asked to get some URLs for verification purposes, so here they are: 1 http://www.megatv.com/article.asp?catid=14693#toppage 1 http://www.games.co.id/permainan/berpakaian/berpakaian.html 1 http://www.cesgranrio.org.br/pdf/petrobras0111/petrobras0111_edital2.pdf 1 http://mail.yahoo.com/ 1 http://clipat.maktoob.com/video.php?video_id=294884
Updated•12 years ago
|
Keywords: regressionwindow-wanted
You need to log in
before you can comment on or make changes to this bug.
Description
•