492480 - Simplify use of UnregisterPlaceholderFrame

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Currently, when an out-of-flow frame is destroyed we require someone to first call frameManager->UnregisterPlaceholderFrame on its placeholder. We also require someone to call SetOutOfFlowFrame(nsnull) on the placeholder. Both of these are to avoid dangling references to the out-of-flow frame that will be deleted.

However, not all code does this. In particular, DoDeletingFrameSubtree does not call SetOutOfFlowFrame(nsnull):
         // Don't SetOutOfFlowFrame(nsnull) here because the float cache depends
         // on it when the float is removed later on, see bug 348688 comment 6.
So nsBlockFrame::RemoveFloat has to do that just before it destroys the float frame. However, nsAbsoluteContainingBlock::RemoveFrame doesn't do this, so in some cases we leave a dangling pointer around in the abs-pos frame's placeholder frame.

I'm not aware of any actual bugs due to this, but it seems to me we can simplify this and make it safer by just making nsFrame::Destroy for out-of-flow frames responsible for frameManager->UnregisterPlaceholderFrame and placeholder->SetOutOfFlowFrame(nsnull). The attached patch does this.

In this patch I've removed frameManager->UnregisterPlaceholderFrame calls that are just before a RemoveFrame that will trigger the new nsFrame::Destroy code. There are some remaining calls to UnregisterPlaceholderFrame that I haven't removed, they'd need more analysis.

Comment 1

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

Comment on attachment 376843 [details] [diff] [review]
fix

r+sr=dbaron

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/0208904c1413