Closed Bug 381621 Opened 13 years ago Closed 13 years ago

Get rid of nsBoxFrame::AddRef/Release

Categories

(Core :: Layout, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: sharparrow1, Assigned: sharparrow1)

References

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
Getting rid of nsBoxFrame AddRef/Release plus some trivial cleanups.
Attachment #265693 - Flags: review?(roc)
Comment on attachment 265693 [details] [diff] [review]
Patch

> nsresult 
> nsBoxFrame::GetContentOf(nsIContent** aContent)
> {
>     // If we don't have a content node find a parent that does.
>     nsIFrame *frame = this;
>     while (frame) {
>       *aContent = frame->GetContent();
>       if (*aContent) {
>         NS_ADDREF(*aContent);
>+        NS_ASSERTION(*aContent == GetContent(), "Can't happen");
>         return NS_OK;
>       }
> 
>       frame = frame->GetParent();
>     }

Are you planning to rewrite this on the assumption that the assertion never fires?  (Though it would be clearer to put an NS_NOTREACHED after the if {}, since all you're really doing is asserting that we never loop.)

r+sr=dbaron
Attachment #265693 - Flags: superreview+
Attachment #265693 - Flags: review?(roc)
Attachment #265693 - Flags: review+
(In reply to comment #1)
> Are you planning to rewrite this on the assumption that the assertion never
> fires?  (Though it would be clearer to put an NS_NOTREACHED after the if {},
> since all you're really doing is asserting that we never loop.)

I'm essentially asserting that GetContentOf is just a fancy way of calling GetContent().  That's true because every frame except the root frames have content pointers.  I'll file a followup on removing it.
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
(In reply to comment #2)
> I'm essentially asserting that GetContentOf is just a fancy way of calling
> GetContent().  That's true because every frame except the root frames have
> content pointers.  I'll file a followup on removing it.

I know.  I just think an NS_NOTREACHED right before the loop loops would be a clearer way of asserting that.
Depends on: 385286
Product: Core → Core Graveyard
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.