Closed Bug 492480 Opened 16 years ago Closed 16 years ago

Simplify use of UnregisterPlaceholderFrame

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: roc, Assigned: roc)

Details

Attachments

(1 file)

Attached patch fixSplinter Review
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.
Attachment #376843 - Flags: superreview?(dbaron)
Attachment #376843 - Flags: review?(dbaron)
Comment on attachment 376843 [details] [diff] [review] fix r+sr=dbaron
Attachment #376843 - Flags: superreview?(dbaron)
Attachment #376843 - Flags: superreview+
Attachment #376843 - Flags: review?(dbaron)
Attachment #376843 - Flags: review+
Status: NEW → RESOLVED
Closed: 16 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: