381621 - Get rid of nsBoxFrame::AddRef/Release

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Eli Friedman]

Attachment: Patch

Getting rid of nsBoxFrame AddRef/Release plus some trivial cleanups.

Comment 1

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

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

Comment 2

[Eli Friedman]

(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.

Comment 3

[Eli Friedman]

Checked in.

Comment 4

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

(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.