Closed Bug 1176105 Opened 5 years ago Closed 5 years ago

Remove helper-class nsTableIterator


(Core :: Layout: Tables, defect)

Not set



Tracking Status
firefox41 --- fixed


(Reporter: dholbert, Assigned: jfkthame)




(1 file)

Looks like attachment 8624178 [details] [diff] [review] (on bug 1174700) is removing all the interesting parts of nsTableIterator (the "interesting parts" being special cases to handle RTL tables), because now we're going to be supporting RTL tables more directly via logical coordinates, passing around WritingMode structs, etc.

I suspect this means we can & should get rid of nsTableIterator entirely, since it won't really be doing anything special/interesting anymore (I think).  We can probably replace its usages with iteration over a nsFrameList.
Assignee: nobody → jfkthame
Comment on attachment 8625345 [details] [diff] [review]
Remove the (largely gutted) nsTableIterator class, and replace with simple frame-list iteration

r=me with the below:

>+++ b/layout/tables/nsTableFrame.cpp
>@@ -1424,28 +1424,24 @@ nsTableFrame::SetColumnDimensions(nscoor
>                                   nscoord aContainerWidth)
> {
>-    nsTableIterator iterCol(*colGroupFrame);
>-    for (nsIFrame* colFrame = iterCol.First(); colFrame;
>-         colFrame = iterCol.Next()) {
>+    for (nsIFrame* colFrame : colGroupFrame->PrincipalChildList()) {
>-    for (nsIFrame* colFrame = iterCol.First(); colFrame;
>-         colFrame = iterCol.Next()) {
>+    for (nsIFrame* colFrame : colGroupFrame->PrincipalChildList()) {

Here, rather than calling colGroupFrame->PrincipalChildList() twice, just call it once and cache that in a local 'const nsFrameList' variable (and use that in each of these loops).

(The function PrincipalChildList() involves a virtual function call, to GetChildList().  Might as well avoid introducing a new virtual function call as part of this cleanup. (unless there was a risk that one of these loops actually modified the list; but neither of the loops seem to have any risk of that.)

>+++ b/layout/tables/nsTableRowFrame.cpp
> void
> nsTableRowFrame::DidResize()
> {
>   // Resize and re-align the cell frames based on our row bsize
>   nsTableFrame* tableFrame = GetTableFrame();
>-  nsTableIterator iter(*this);
>-  nsIFrame* childFrame = iter.First();
>-  while (childFrame) {
>+  for (nsIFrame* childFrame : PrincipalChildList()) {

Let's just use "mFrames" here.  "PrincipalChildList()" is unnecessarily abstract, for accessing our own child list.

This applies to pretty much all of the chunks in this file (nsTableRowFrame.cpp)
Attachment #8625345 - Flags: review?(dholbert) → review+
Summary: Consider removing class nsTableIterator → Remove helper-class nsTableIterator
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.