Simplify use of UnregisterPlaceholderFrame

RESOLVED FIXED

Status

()

RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: roc, Assigned: roc)

Tracking

Trunk
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Created attachment 376843 [details] [diff] [review]
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.
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+
http://hg.mozilla.org/mozilla-central/rev/0208904c1413
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.