474377 - "ASSERTION: must be in the same rule tree as parent: 'r1 == r2'" with MathML, {ib}, :after

Status: RESOLVED WORKSFORME

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase (triggers a fatal assertion when loaded)

Loading this testcase triggers a fatal assertion that was added in bug 473871:

###!!! ABORT: must be in the same rule tree as parent: 'r1 == r2', file /Users/jruderman/central/layout/style/nsStyleContext.cpp, line 89

Initially security-sensitive because in bug 473871, dbaron said something about crashes due to dangling pointers.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The testcase seems somewhat familiar, but I can't find anything.  (I looked at bug 454276, bug 469432, bug 454736.)

This does seem pretty bad, though.

Comment 2

[Mats Palmgren (inactive)]

Attachment: Frame dump

The blue frame (0x177b100) is going to be re-framed so we skip processing
its children, and continue with it's next sibling 0x177a8e8 (yellow),
which contains 0x177abe8 (red) which is a special sibling to the child
we skipped (green) which is the style context provider for (red).

Comment 3

[Mats Palmgren (inactive)]

Attachment: wip

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'm wondering if we should just remove the optimization of skipping things that are going to be reframed.  It's also potentially problematic when reframing fails.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The only case I can think of where that could cause performance problems is toggling display:none on something high in the tree.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Or just something with lots of kids (e.g. a big table)?

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Yeah, toggling display:none on something with large numbers of descendants.

Comment 8

[Boris Zbarsky [:bzbarsky]]

That might not be that uncommon, esp. in code that tries to optimize things by toggling display, doing DOM manipulation, then toggling back...

Comment 9

[Jesse Ruderman]

Bug 475128 downgraded this abort to an assertion.  The testcase in this bug still triggers it.

Comment 10

[Jesse Ruderman]

Still happens on trunk.  This bug has been in [sg:investigate] for quite a while :(

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Post-bug 475128, I think this no longer involves dangling pointers.

Comment 12

[Jesse Ruderman]

Is it safe to make this bug public, thne?

Comment 13

[Honza Bambas (:mayhemer)]

I may have a STR for this but not sure it's consistent.  Will report.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Testcase works without issue now.

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f47ec3c4a51c

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f47ec3c4a51c