Closed Bug 861490 Opened 12 years ago Closed 12 years ago

crash in nsChildView::MaybeDrawRoundedBottomCorners with OMTC

Categories

(Core :: Widget: Cocoa, defect)

23 Branch
x86_64
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla23
Tracking Status
firefox22 --- unaffected
firefox23 --- verified

People

(Reporter: scoobidiver, Assigned: nrc)

References

Details

(Keywords: crash, regression, Whiteboard: [startupcrash])

Crash Data

Attachments

(1 file, 1 obsolete file)

It first showed up in 23.0a1/20130412 and seems an OMTC regression based on comments. The regression range might be (setting different from its default value because of http://featherweightmusings.blogspot.co.at/2013/04/the-layers-refactoring-has-landed.html): http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2949e808ed33&tochange=7b8ed29c6bc0 It might be a regression from bug 858550. Signature nsChildView::MaybeDrawRoundedBottomCorners(mozilla::layers::LayerManagerOGL*, nsIntRect) More Reports Search UUID 3da9ac5c-a7a7-4dfd-86ba-eba862130413 Date Processed 2013-04-13 06:51:00 Uptime 1 Last Crash 22 seconds before submission Install Age 11.1 hours since version was first installed. Install Time 2013-04-12 19:42:11 Product Firefox Version 23.0a1 Build ID 20130412030828 Release Channel nightly OS Mac OS X OS Version 10.8.3 12D78 Build Architecture amd64 Build Architecture Info family 6 model 58 stepping 9 Crash Reason EXC_BAD_ACCESS / KERN_INVALID_ADDRESS Crash Address 0x0 User Comments Testing: "layers.offmainthreadcomposition.enabled" set to true as per: http://featherweightmusings.blogspot.co.at/2013/04/the-layers-refactoring-has-landed.html nope… It's dead now :D App Notes AdapterVendorID: 0x8086, AdapterDeviceID: 0x 166GL Layers! GL Context? GL Context+ GL Layers+ Processor Notes sp-processor02.phx1.mozilla.com_20236:2012; exploitability tool failed: 127 EMCheckCompatibility True Adapter Vendor ID 0x8086 Adapter Device ID 0x 166 Frame Module Signature Source 0 XUL nsChildView::MaybeDrawRoundedBottomCorners widget/cocoa/nsChildView.mm:1990 1 libobjc.A.dylib lookUpMethod 2 XUL nsChildView::DrawWindowOverlay widget/cocoa/nsChildView.mm:1870 3 XUL XUL@0x1599480 4 XUL mozilla::layers::LayerManagerComposite::Render LayerManagerComposite.cpp:290 5 XUL mozilla::layers::LayerManager::GetScrollableLayers obj-firefox/x86_64/dist/include/nsTArray-inl.h:234 6 XUL XUL@0x1599480 7 XUL mozilla::layers::LayerManagerComposite::EndTransaction LayerManagerComposite.cpp:185 8 XUL mozilla::layers::LayerManagerComposite::EndEmptyTransaction LayerManagerComposite.cpp:150 9 XUL mozilla::layers::CompositorParent::Composite gfx/layers/ipc/CompositorParent.cpp:570 10 XUL MessageLoop::DeferOrRunPendingTask ipc/chromium/src/base/message_loop.cc:334 11 XUL MessageLoop::DoWork ipc/chromium/src/base/message_loop.cc:442 12 XUL base::MessagePumpDefault::Run message_pump_default.cc:23 13 XUL MessageLoop::Run ipc/chromium/src/base/message_loop.cc:216 14 XUL base::Thread::ThreadMain thread.cc:156 More reports at: https://crash-stats.mozilla.com/report/list?signature=nsChildView%3A%3AMaybeDrawRoundedBottomCorners%28mozilla%3A%3Alayers%3A%3ALayerManagerOGL*%2C+nsIntRect%29
Whiteboard: [startupcrash]
(In reply to Scoobidiver from comment #0) > It first showed up in 23.0a1/20130412 Seen earlier in 23.0a1/20130410065939.
Summary: crash in nsChildView::MaybeDrawRoundedBottomCorners → crash in nsChildView::MaybeDrawRoundedBottomCorners with OMTC
I expect this is because we expect aManager to be LayerManagerOGL, but it is now a LayerManagerComposite.
Blocks: 756601
What a horror show. There is no nice fix for this. If we ever get GL to be OMTC only, then this problem will go away. For now I think the only solution is large amounts of duplicated code. (FWIW, the nice fix is to have LayerManagerOGL backed by CompositorOGL which was working nicely on the graphics branch for a while, but which was deemed too risky to land, probably right about the risk though :-( ).
Assignee: nobody → ncameron
I'm surprised that many people are using mac OMTC. It was only designed to speed up development of other features to bridge the gap ofpoor devtools on mobile which has been largely addressed now.
(In reply to Benoit Girard (:BenWa) from comment #4) > I'm surprised that many people are using mac OMTC. It was only designed to > speed up development of other features to bridge the gap ofpoor devtools on > mobile which has been largely addressed now. I doubt it is many. I made a request on my blog for help testing the layers refactoring, seems like at least one person on a mac answered the call.
Attached patch patch (obsolete) (deleted) — Splinter Review
There is another use of LayerManagerOGL in nsChildView.nm which I am suspicious of - in DrawUsingOpenGL, but I didn't seem to hit that path when I was testing and since I didn't understand when it should be hit I was wary of changing it, but perhaps that also needs fixing up.
Attachment #737818 - Flags: review?(matt.woodrow)
Attachment #737818 - Flags: review?(matt.woodrow)
(In reply to Nick Cameron [:nrc] from comment #7) > Created attachment 737818 [details] [diff] [review] > patch > > There is another use of LayerManagerOGL in nsChildView.nm which I am > suspicious of - in DrawUsingOpenGL, but I didn't seem to hit that path when > I was testing and since I didn't understand when it should be hit I was wary > of changing it, but perhaps that also needs fixing up. Seems to leak :-(
Attached patch patch (deleted) — Splinter Review
still need to decide what to do about the potential race in ShowsResizeIndicator
Attachment #737818 - Attachment is obsolete: true
Attachment #737890 - Flags: review?(matt.woodrow)
Attachment #737890 - Flags: review?(matt.woodrow) → review+
Could we trigger code to run every time that the result of ShowsResizeIndicator might change? Then we could compute this and cache it on the main thread, and just read it from the compositor thread.
(In reply to Matt Woodrow (:mattwoodrow) from comment #11) > Could we trigger code to run every time that the result of > ShowsResizeIndicator might change? > > Then we could compute this and cache it on the main thread, and just read it > from the compositor thread. I think we should file this as a follow up bug - we only want to stop crashing here, and Mac OMTC is still very experimental - this patch just takes us back to how we were before the refactoring. We should block Mac OMTC on it though. This might get complicated and I don't want to wait for that to land this. Is that OK with you?
Absolutely, that was the intent of giving the patch r+ as is.
(In reply to Matt Woodrow (:mattwoodrow) from comment #13) > Absolutely, that was the intent of giving the patch r+ as is. bug 862556
Unfortunately this has created a shutdown crash with debug builds (and OMTC enabled). The TextureImage object we cache on nsChildView is created from the compositor thread, but released from the main thread. We don't have threadsafe refcounting for this type (and probably shouldn't), so this hits an assertion.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Is there anything QA can do to verify this is fixed?
Checking the crash stats I see no crashes for the last 4 weeks. Marking verified fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: