267085 - [FIX]Leaking reflow commands in PresShell::AppendReflowCommand?

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[rbs]

|PresShell::AppendReflowCommand| has an early return when there hasn't been the
initial reflow yet.

Further down the code, any reflow command that isn't added to the queue is deleted.

The question is: why doesn't this apply to the first early return?!?


===
Another separate thought: how common are reflow commands rejected? If these are
too common, maybe the signature of AppendReflowCommand could be changed so that
commands are not created just to be deleted immediately. This means doing the
creation inside the function itself. Of course, it is not worth the hassle if it
is rare.

Comment 1

[Boris Zbarsky [:bzbarsky]]

> The question is: why doesn't this apply to the first early return?!?

That's a bug, indeed.

> maybe the signature of AppendReflowCommand could be changed

I think it's worth doing this anyway....  There's no reason to burden every
single caller with having to call NS_NewReflowCommand, check the rv, etc.

David, if you don't have any objections, I'll go ahead and do just that.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

This moves the reflow command creation into presshell and cleans up a bunch of
the code.  I went ahead and removed some unused members of the reflow command
(some were write-only, and some were always null and never read).

Comment 3

[rbs]

Comment on attachment 171331 [details] [diff] [review]
Patch

r=rbs

Also update the description of |GetChildListName()|, as what is left there is
outdated and makes little sense.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 171331 [details] [diff] [review]
Patch

rbs, would you mind doing r+sr?  David said that's OK with him.

Comment 5

[rbs]

Comment on attachment 171331 [details] [diff] [review]
Patch

sr=rbs

Comment 6

[Boris Zbarsky [:bzbarsky]]

Fixed.  Note that I didn't check in quite the patch attached to this bug,
because I had to merge to the prescontext arg removal for
Append/Remove/ReplaceChild.