462788 - Bug 462392 testcase causes rule trees to leak

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Jesse Ruderman]

The testcase in bug 462392 triggers this assertion:

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

Rule trees, rather than frames, are leaking here.  See bug 462392 comment 7 through 9.

This isn't exploitable, but in the future it could prevent me from using the assertion to look for exploitable bugs.  I say "in the future" because four other bugs (tracked in bug 334514's dependency list) already prevent me from using the assertion in this way.

Comment 1

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

It's not actually a leak.  What happens is that nsPresContext::FreeToShell is called after nsPresShell::Destroy.  (FreeToShell should probably have an assertion about mShell being null.)  But the pres shell arena isn't destroyed until ~nsPresContext.  So we're running destructors but not calling the FrameArena::FreeFrame, and then almost immediately afterwards destroying the arena.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Can we just tear down the ruletree when we tear down the frame tree?

Comment 3

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

I think the objects in question were actually part of mOldRuleTree.  We could free mOldRuleTree where we free mRuleTree; I think that would fix this.

Comment 4

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

Fixed by checkin of bug 475128.