Closed Bug 475128 Opened 13 years ago Closed 13 years ago
GC old rule trees rather than deleting them
I think we should garbage-collect old rule trees rather than just deleting them. We've seen crashes related to accessing the deleted rule tree because of other bugs show up a bit lately (a fixed example is bug 468645).
Er, I should also delete them in Shutdown, which means we fix bug 462392 as well (and thus I can put in the assertion in FreeToShell that I've had in my tree for ages).
...er, should have said we fix bug 462788 as well.
Comment on attachment 358543 [details] [diff] [review] patch >+++ b/layout/style/nsStyleContext.cpp >- // We don't need to clear out mRoots; NotifyStyleContextDestroyed >- // will, and they're useful in EndReconstruct if they don't get >- // completely cleared out. Why remove that comment? Seems like it might still be good to make it clear that clearing mRoots here would be a _bad_ idea. Even worse than before this patch, no? > nsStyleSet::EndReconstruct() Do we want to assert that mOldRootTrees has length 1 here? Or are we figuring any case when it doesn't is caught by the other asserts? >+nsStyleSet::GCRuleTree() >+ // Mark the style context tree by marking all all style contexts which s/all all/all/ >+++ b/layout/style/nsStyleSet.h >+ void GCRuleTree(); Document? And maybe call it GCRuleTrees? >+ // Old rule tree, which should only be non-empty between s/tree/trees/ ? cause >+ // style contexts to exist to long, may last longer. s/to long/too long/
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Whiteboard: [needs landing]
I landed this earlier today at http://hg.mozilla.org/mozilla-central/rev/24917a339f2e but then realized that it was causing a lot of assertions (I'd noticed earlier, and then forgotten). The assertions were pointing out a real bug; I'd effectively regressed the purpose of the GC. So I backed it out: http://hg.mozilla.org/mozilla-central/rev/0cbd3800749f http://hg.mozilla.org/mozilla-central/rev/f6b5ed8b3824 Here's a fixed patch. The fix is the additional change to nsRuleNode::Sweep. There's also a new nsStyleSet::GetRuleTree method to help it.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 1.9.1 landing]
Target Milestone: --- → mozilla1.9.2a1
Whiteboard: [needs 1.9.1 landing]
Target Milestone: mozilla1.9.2a1 → mozilla1.9.1b3
Attachment #359637 - Flags: approval188.8.131.52? → approval184.108.40.206+
Comment on attachment 359637 [details] [diff] [review] patch Approved for 220.127.116.11, a=dveditz for release-drivers.
Commited to CVS trunk (for 1.9.0.* releases), 2009-02-02 20:17 -0800.
NOTE to anybody doing 1.8.* backports: this patch does not apply to 1.8.* and is not relevant there since we did not do rule tree reconstructs back then; they were new in 1.9.*.
You need to log in before you can comment on or make changes to this bug.