Closed
Bug 861490
Opened 12 years ago
Closed 12 years ago
crash in nsChildView::MaybeDrawRoundedBottomCorners with OMTC
Categories
(Core :: Widget: Cocoa, defect)
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)
(deleted),
patch
|
mattwoodrow
:
review+
|
Details | Diff | Splinter Review |
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
Reporter | ||
Updated•12 years ago
|
Whiteboard: [startupcrash]
Reporter | ||
Comment 1•12 years ago
|
||
(In reply to Scoobidiver from comment #0)
> It first showed up in 23.0a1/20130412
Seen earlier in 23.0a1/20130410065939.
Reporter | ||
Updated•12 years ago
|
Blocks: layers-refactoring
Summary: crash in nsChildView::MaybeDrawRoundedBottomCorners → crash in nsChildView::MaybeDrawRoundedBottomCorners with OMTC
Assignee | ||
Comment 2•12 years ago
|
||
I expect this is because we expect aManager to be LayerManagerOGL, but it is now a LayerManagerComposite.
Assignee | ||
Comment 3•12 years ago
|
||
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 | ||
Updated•12 years ago
|
Assignee: nobody → ncameron
Comment 4•12 years ago
|
||
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.
Assignee | ||
Comment 5•12 years ago
|
||
Assignee | ||
Comment 6•12 years ago
|
||
(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.
Assignee | ||
Comment 7•12 years ago
|
||
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)
Assignee | ||
Updated•12 years ago
|
Attachment #737818 -
Flags: review?(matt.woodrow)
Assignee | ||
Comment 8•12 years ago
|
||
(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 :-(
Assignee | ||
Comment 9•12 years ago
|
||
Ah, much better try push: https://tbpl.mozilla.org/?tree=Try&rev=260e6adc1f2f
Assignee | ||
Comment 10•12 years ago
|
||
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)
Updated•12 years ago
|
Attachment #737890 -
Flags: review?(matt.woodrow) → review+
Comment 11•12 years ago
|
||
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.
Assignee | ||
Comment 12•12 years ago
|
||
(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?
Comment 13•12 years ago
|
||
Absolutely, that was the intent of giving the patch r+ as is.
Assignee | ||
Comment 14•12 years ago
|
||
(In reply to Matt Woodrow (:mattwoodrow) from comment #13)
> Absolutely, that was the intent of giving the patch r+ as is.
bug 862556
Assignee | ||
Comment 15•12 years ago
|
||
Comment 16•12 years ago
|
||
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.
Comment 17•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Reporter | ||
Updated•12 years ago
|
Comment 18•11 years ago
|
||
Is there anything QA can do to verify this is fixed?
Comment 19•11 years ago
|
||
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.
Description
•