Closed
Bug 950930
Opened 11 years ago
Closed 11 years ago
Crash in nsXBLProtoImplAnonymousMethod::Execute()
Categories
(Core :: XBL, defect)
Core
XBL
Tracking
()
VERIFIED
FIXED
mozilla29
People
(Reporter: mccr8, Assigned: smaug)
References
Details
(Keywords: crash, regression, topcrash)
Crash Data
Attachments
(1 file)
(deleted),
patch
|
bzbarsky
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
This is currently the top crash on Windows by a huge margin, with about 8% of all crashes. I'm not sure why nothing is filed yet for it. All near-null crashes. The comments mostly talk about shutdown.
Comment 1•11 years ago
|
||
This is happening on Linux too, btw. And the stacks are definitely shutdown: we're trying to run XBL dtors from the pagehide when closing a XULWindow.
Do we have anything like a regression range?
Do we have any idea which is null here? mBoundElement?
Reporter | ||
Comment 2•11 years ago
|
||
Looking at the "table" tab in crash stats, it looks like this showed up in the Dec 14 build.
Assignee | ||
Comment 3•11 years ago
|
||
When a node is adopted we seem to clear mBoundElement, but not really tear down it properly.
// If we're adopting a node that's not in a document, it might still
// have a binding applied. Remove the binding from the element now
// that it's getting adopted into a new document.
// TODO Fully tear down the binding.
adoptedNode->AsContent()->SetXBLBinding(nullptr);
Assignee | ||
Comment 4•11 years ago
|
||
But I still think this.
http://hg.mozilla.org/mozilla-central/rev/78b36c1db4e3
Assignee | ||
Comment 5•11 years ago
|
||
Yes, that actually had the null check for mBoundElement
Reporter | ||
Updated•11 years ago
|
status-firefox28:
--- → unaffected
status-firefox29:
--- → affected
tracking-firefox29:
--- → ?
Keywords: crash,
regression
Comment 6•11 years ago
|
||
Yeah, that would make sense. We used to just skip running script altogether when !mBoundElement. We should consider restoring that.
Assignee | ||
Comment 7•11 years ago
|
||
Hmm, I didn't upload this last night.
Assignee: nobody → bugs
Attachment #8348705 -
Flags: review?(bzbarsky)
Comment 8•11 years ago
|
||
Comment on attachment 8348705 [details] [diff] [review]
xbl_crash.diff
r=me
Attachment #8348705 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 9•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d58545aaeab9
Spinning new nightlies as well.
Status: NEW → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Updated•11 years ago
|
Comment 10•11 years ago
|
||
This needs an Aurora nom so bug 944407 can be uplifted.
Flags: needinfo?(bugs)
Assignee | ||
Comment 11•11 years ago
|
||
Comment on attachment 8348705 [details] [diff] [review]
xbl_crash.diff
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 944407
User impact if declined: Crashes
Testing completed (on m-c, etc.): landed m-c 2013-12-17
The last crashes with this stack are from
20131217030203 nightly
Risk to taking this patch (and alternatives if risky): This is just a null pointer check (bringing back the old behavior)
String or IDL/UUID changes made by this patch: NA
Attachment #8348705 -
Flags: approval-mozilla-aurora?
Flags: needinfo?(bugs)
Updated•11 years ago
|
Attachment #8348705 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•11 years ago
|
Comment 12•11 years ago
|
||
Comment 13•11 years ago
|
||
Marking this verified fixed based on crashstats. There are no crashes with a Firefox 29 build after 2013-12-17 nor with a Firefox 28 build after 2014-01-02.
You need to log in
before you can comment on or make changes to this bug.
Description
•