Closed
Bug 1177627
Opened 9 years ago
Closed 9 years ago
Child with null parent during unlinking
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla42
People
(Reporter: mccr8, Assigned: mccr8)
References
Details
Attachments
(1 file)
(deleted),
patch
|
smaug
:
review+
ritu
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Apparently we're crashing regularly in some B2G test that adds and removes a bunch of DOM nodes. See bug 1165469 comment 754. From the stack, we unlink a document, which calls UnbindFromTree on a child which is an HTMLSharedElement, which calls UnbindFromTree on a child which is an HTMLSharedElement, which calls UnbindFromTree on a child that is an nsGenericDOMDataNode, and that child has a null mParent, causing us to crash on this check:
if (aNullParent || !mParent->IsInShadowTree()) {
This is a regression from my patch that changes how the content unbinder is implemented (bug 866681). I'm assuming that a node that is a child should never have a null mParent? I'm not sure how this could be happening. I'll have to read over the content unbinder code again, and also figure out what my patch could have done to break this besides change when the ContentUnbinder runs. It is also strange that only this one B2G test is triggering it.
(I'm filing separate from bug 1165469 because that one is so spammy.)
Assignee | ||
Comment 1•9 years ago
|
||
FWIW, I don't see anything with UnbindFromTree in the signature in the top 300 crashes on Nightly.
Comment 2•9 years ago
|
||
I was looking at the code too, and mParent really shouldn't be null there.
And given that it is b2g my guess is that it might be something shadow DOM related.
(but even then, I don't understand how mParent could be null.)
I'll re-read the code tomorrow.
Comment 3•9 years ago
|
||
Wondering if you had the time to look at this in this post-Whistler week :)
Bug 1165469 has more than 1000 comments now :D
Assignee | ||
Comment 4•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → continuation
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8629019 [details] [diff] [review]
Back out most of 7fcf6bf43eda for causing Gaia unit test crashes.
Review of attachment 8629019 [details] [diff] [review]:
-----------------------------------------------------------------
I'm not sure what the failure rate is, but the try push looks quite green.
The one change here is that I didn't add back the content unbinder telemetry, because I delete that in another bug. I don't think it is a big deal as we didn't look at it too much.
Attachment #8629019 -
Flags: review?(bugs)
Comment 6•9 years ago
|
||
And that telemetry was reporting rather small numbers.
Updated•9 years ago
|
Attachment #8629019 -
Flags: review?(bugs) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Keywords: checkin-needed
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8629019 [details] [diff] [review]
Back out most of 7fcf6bf43eda for causing Gaia unit test crashes.
Approval Request Comment
[Feature/regressing bug #]: bug 866681
[User impact if declined]: A gaia unit test is crashing very frequently due tos this (bug 1165469). It is also possible this could affect regular Firefox users, too, as there's nothing Gaia specific here.
[Describe test coverage new/current, TreeHerder]: this code is heavily tested
[Risks and why]: low, this just backs out a patch to restore existing behavior.
[String/UUID change made/needed]: none
Attachment #8629019 -
Flags: approval-mozilla-aurora?
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Comment on attachment 8629019 [details] [diff] [review]
Back out most of 7fcf6bf43eda for causing Gaia unit test crashes.
Approving for uplift as this is a patch backout.
Attachment #8629019 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 11•9 years ago
|
||
Updated•9 years ago
|
blocking-b2g: 2.2? → ---
tracking-b2g:
backlog → ---
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•