508919 - Leak (style objects?) with display:table-cell, -moz-border-left-colors:inherit

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Reloading triggers:

###!!! ASSERTION: Some objects allocated with AllocateFrame were not freed: 'mFrameCount == 0', file /Users/jruderman/central/layout/base/nsPresShell.cpp, line 683

Comment 1

[Boris Zbarsky [:bzbarsky]]

This is definitely leaking an nsStyleBorder (verified by adding ctor/dtor logging to all the style structs).  We're landing in nsRuleNode::ComputeBorderData with:

(gdb) p aCanStoreInRuleTree
$9 = 1
(gdb) p aRuleDetail
$10 = nsRuleNode::eRulePartialInherited

and at the end of the function we have:

(gdb) p canStoreInRuleTree
$11 = 1

Which is, of course, broken.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix (plus logging and asserts to make this easier to debug in the future)

Comment 3

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

Comment on attachment 393131 [details] [diff] [review]
Fix (plus logging and asserts to make this easier to debug in the future)

r=dbaron

Can we add assertions in COMPUTE_END_* that if the rule detail is *Inherit* or *Mixed* (or also, for inherited structs *Partial* or *None*) that canStoreInRuleTree is false?  Or are the rule details not that accurate?  I suspect they might not be...

Also, did you check that your new assertions aren't firing during layout/style mochitests?  It's probably worth doing.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> Or are the rule details not that accurate?

Hmm.  I think they should be for the most part, except:

1)  border-color: inherit on the root does not set canStoreInRuleTree to false.
    Maybe it should?
2)  If we have a property that is in CSS_PROP_something but not actually
    computed, it could affect the detail but not canStoreInRuleTree.  That would
    be a bug, though, so I'm temped to say we should not take it into account.

I'm going to try fixing item 1 above, adding those asserts and running the mochitests; see what happens.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Added the asserts per comment 4.  They don't fire during tests; pushed http://hg.mozilla.org/mozilla-central/rev/45cee94d82f3