Closed Bug 1050412 Opened 8 years ago Closed 8 years ago

nsSimplePageSequenceFrame.cpp should use a frame enumerator during reflow

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: dholbert, Assigned: dholbert)

Details

Attachments

(2 files)

During nsSimplePageSequence's reflow method, it walks over its child frames like this:

> 204   for (nsIFrame* kidFrame = mFrames.FirstChild(); nullptr != kidFrame; ) {
> [...]
> 249     kidFrame = kidFrame->GetNextSibling();
> }

http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsSimplePageSequenceFrame.cpp?rev=05eb059f70bf#204

I don't think there's any good reason that the "kidFrame = kidFrame->GetNextSibling()" line is outside of the "for" statement. It should move up into the "for" statement.

Better, though, we should just use a nsFrameList::Enumerator, since IIUC that's what we're trying to use for iterating over frame lists.
This second part fixes up some broken code that I noticed in one of the loops. 

In particular, this patch:
 (1) makes us assert before static_casting
 (2) drops a useless null-check of child-frame. (The loop ensures that the pointer must be non-null.  nsFrameList is a linked-list, which means it's impossible to have a null pointer in the middle of the list.)  I suspect the original author might've mistakenly thought that this null-check was testing the validity of the static_cast.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #8469491 - Flags: review?(tnikkel)
Attachment #8469486 - Attachment description: fix v1 → part 1: Use nsFrameList::Enumerator (plus nsFrameList::GetLength() in one spot)
Attachment #8469486 - Flags: review?(tnikkel) → review+
Attachment #8469491 - Flags: review?(tnikkel) → review+
https://hg.mozilla.org/mozilla-central/rev/b6c812537814
https://hg.mozilla.org/mozilla-central/rev/3ac1b309271d
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.