Bug 1794456 Part 3 - Support range-based for loop for nsFrameList::Slice. r=emilio

Since Slice::mEnd is the first frame that is NOT in the slice, so we only
support forward iteration, and it's sufficient to replace existing usages of
nsFrameList::Enumerator.

For those for loops iterating TableColFrame and TableColGroupFrame, we need to
explicitly check the validity of the iterator because they modify the frame list
while iterating. nsFrameList::Enumerator::End() has a hack for this.
https://searchfox.org/mozilla-central/rev/ffa4d00965c5281def6d3ddcbcdf6259d38c9b9a/layout/generic/nsFrameList.h#402-407

Differential Revision: https://phabricator.services.mozilla.com/D158986

layout/generic/nsBlockFrame.cpp

@@ -5953,8 +5953,7 @@ void nsBlockFrame::AddFrames(nsFrameList& aFrameList, nsIFrame* aPrevSibling,
 
   // Walk through the new frames being added and update the line data
   // structures to fit.
-  for (nsFrameList::Enumerator e(newFrames); !e.AtEnd(); e.Next()) {
-    nsIFrame* newFrame = e.get();
+  for (nsIFrame* newFrame : newFrames) {
     NS_ASSERTION(!aPrevSibling || aPrevSibling->GetNextSibling() == newFrame,
                  "Unexpected aPrevSibling");
     NS_ASSERTION(
@@ -8166,8 +8165,8 @@ void nsBlockFrame::VerifyOverflowSituation() {
   // Pushed floats must not have a next-in-flow in mFloats or mFrames.
   oofs = GetPushedFloats();
   if (oofs) {
-    for (nsFrameList::Enumerator e(*oofs); !e.AtEnd(); e.Next()) {
-      nsIFrame* nif = e.get()->GetNextInFlow();
+    for (nsIFrame* f : *oofs) {
+      nsIFrame* nif = f->GetNextInFlow();
       MOZ_ASSERT(!nif ||
                  (!mFloats.ContainsFrame(nif) && !mFrames.ContainsFrame(nif)));
     }

layout/generic/nsFrameList.h

@@ -377,10 +377,17 @@ class nsFrameList {
         : mStart(aList.FirstChild()), mEnd(nullptr) {}
     Slice(nsIFrame* aStart, nsIFrame* aEnd) : mStart(aStart), mEnd(aEnd) {}
 
+    iterator begin() const { return iterator(mStart); }
+    const_iterator cbegin() const { return begin(); }
+    iterator end() const { return iterator(mEnd); }
+    const_iterator cend() const { return end(); }
+
    private:
-    nsIFrame* const mStart;      // our starting frame
-    const nsIFrame* const mEnd;  // The first frame that is NOT in the slice.
-                                 // May be null.
+    // Our starting frame.
+    nsIFrame* const mStart;
+
+    // The first frame that is NOT in the slice. May be null.
+    nsIFrame* const mEnd;
   };
 
   class Enumerator {

layout/generic/nsInlineFrame.cpp

@@ -260,10 +260,10 @@ static void ReparentChildListStyle(nsPresContext* aPresContext,
                                    nsIFrame* aParentFrame) {
   RestyleManager* restyleManager = aPresContext->RestyleManager();
 
-  for (nsFrameList::Enumerator e(aFrames); !e.AtEnd(); e.Next()) {
-    NS_ASSERTION(e.get()->GetParent() == aParentFrame, "Bogus parentage");
-    restyleManager->ReparentComputedStyleForFirstLine(e.get());
-    nsLayoutUtils::MarkDescendantsDirty(e.get());
+  for (nsIFrame* f : aFrames) {
+    NS_ASSERTION(f->GetParent() == aParentFrame, "Bogus parentage");
+    restyleManager->ReparentComputedStyleForFirstLine(f);
+    nsLayoutUtils::MarkDescendantsDirty(f);
   }
 }
 

layout/tables/nsTableColGroupFrame.cpp

@@ -69,17 +69,23 @@ nsresult nsTableColGroupFrame::AddColsToTable(int32_t aFirstColIndex,
 
   // set the col indices of the col frames and and add col info to the table
   int32_t colIndex = aFirstColIndex;
-  nsFrameList::Enumerator e(aCols);
-  for (; !e.AtEnd(); e.Next()) {
-    ((nsTableColFrame*)e.get())->SetColIndex(colIndex);
+
+  // XXX: We cannot use range-based for loop because InsertCol() can destroy the
+  // nsTableColFrame in the slice we're traversing! Need to check the validity
+  // of *colIter.
+  auto colIter = aCols.begin();
+  for (auto colIterEnd = aCols.end(); *colIter && colIter != colIterEnd;
+       ++colIter) {
+    auto* colFrame = static_cast<nsTableColFrame*>(*colIter);
+    colFrame->SetColIndex(colIndex);
     mColCount++;
-    tableFrame->InsertCol((nsTableColFrame&)*e.get(), colIndex);
+    tableFrame->InsertCol(*colFrame, colIndex);
     colIndex++;
   }
 
-  for (nsFrameList::Enumerator eTail = e.GetUnlimitedEnumerator();
-       !eTail.AtEnd(); eTail.Next()) {
-    ((nsTableColFrame*)eTail.get())->SetColIndex(colIndex);
+  for (; *colIter; ++colIter) {
+    auto* colFrame = static_cast<nsTableColFrame*>(*colIter);
+    colFrame->SetColIndex(colIndex);
     colIndex++;
   }
 

layout/tables/nsTableFrame.cpp

@@ -476,17 +476,15 @@ void nsTableFrame::ResetRowIndices(
   OrderRowGroups(rowGroups);
 
   nsTHashSet<nsTableRowGroupFrame*> excludeRowGroups;
-  nsFrameList::Enumerator excludeRowGroupsEnumerator(aRowGroupsToExclude);
-  while (!excludeRowGroupsEnumerator.AtEnd()) {
+  for (nsIFrame* excludeRowGroup : aRowGroupsToExclude) {
     excludeRowGroups.Insert(
-        static_cast<nsTableRowGroupFrame*>(excludeRowGroupsEnumerator.get()));
+        static_cast<nsTableRowGroupFrame*>(excludeRowGroup));
 #ifdef DEBUG
     {
       // Check to make sure that the row indices of all rows in excluded row
       // groups are '0' (i.e. the initial value since they haven't been added
       // yet)
-      const nsFrameList& rowFrames =
-          excludeRowGroupsEnumerator.get()->PrincipalChildList();
+      const nsFrameList& rowFrames = excludeRowGroup->PrincipalChildList();
       for (nsIFrame* r : rowFrames) {
         auto* row = static_cast<nsTableRowFrame*>(r);
         MOZ_ASSERT(row->GetRowIndex() == 0,
@@ -495,7 +493,6 @@ void nsTableFrame::ResetRowIndices(
       }
     }
 #endif
-    excludeRowGroupsEnumerator.Next();
   }
 
   int32_t rowIndex = 0;
@@ -517,32 +514,23 @@ void nsTableFrame::ResetRowIndices(
 void nsTableFrame::InsertColGroups(int32_t aStartColIndex,
                                    const nsFrameList::Slice& aColGroups) {
   int32_t colIndex = aStartColIndex;
-  nsFrameList::Enumerator colGroups(aColGroups);
-  for (; !colGroups.AtEnd(); colGroups.Next()) {
-    MOZ_ASSERT(colGroups.get()->IsTableColGroupFrame());
-    nsTableColGroupFrame* cgFrame =
-        static_cast<nsTableColGroupFrame*>(colGroups.get());
-    cgFrame->SetStartColumnIndex(colIndex);
-    // XXXbz this sucks.  AddColsToTable will actually remove colgroups from
-    // the list we're traversing!  Need to fix things here.  :( I guess this is
-    // why the old code used pointer-to-last-frame as opposed to
-    // pointer-to-frame-after-last....
 
-    // How about dealing with this by storing a const reference to the
-    // mNextSibling of the framelist's last frame, instead of storing a pointer
-    // to the first-after-next frame?  Will involve making nsFrameList friend
-    // of nsIFrame, but it's time for that anyway.
-    cgFrame->AddColsToTable(colIndex, false,
-                            colGroups.get()->PrincipalChildList());
+  // XXX: We cannot use range-based for loop because AddColsToTable() can
+  // destroy the nsTableColGroupFrame in the slice we're traversing! Need to
+  // check the validity of *colGroupIter.
+  auto colGroupIter = aColGroups.begin();
+  for (auto colGroupIterEnd = aColGroups.end();
+       *colGroupIter && colGroupIter != colGroupIterEnd; ++colGroupIter) {
+    MOZ_ASSERT((*colGroupIter)->IsTableColGroupFrame());
+    auto* cgFrame = static_cast<nsTableColGroupFrame*>(*colGroupIter);
+    cgFrame->SetStartColumnIndex(colIndex);
+    cgFrame->AddColsToTable(colIndex, false, cgFrame->PrincipalChildList());
     int32_t numCols = cgFrame->GetColCount();
     colIndex += numCols;
   }
 
-  nsFrameList::Enumerator remainingColgroups =
-      colGroups.GetUnlimitedEnumerator();
-  if (!remainingColgroups.AtEnd()) {
-    nsTableColGroupFrame::ResetColIndices(
-        static_cast<nsTableColGroupFrame*>(remainingColgroups.get()), colIndex);
+  for (; *colGroupIter; ++colGroupIter) {
+    nsTableColGroupFrame::ResetColIndices(*colGroupIter, colIndex);
   }
 }
 
@@ -1052,9 +1040,8 @@ void nsTableFrame::InsertRowGroups(const nsFrameList::Slice& aRowGroups) {
     // and M is number of rowgroups we have!
     uint32_t rgIndex;
     for (rgIndex = 0; rgIndex < orderedRowGroups.Length(); rgIndex++) {
-      for (nsFrameList::Enumerator rowgroups(aRowGroups); !rowgroups.AtEnd();
-           rowgroups.Next()) {
-        if (orderedRowGroups[rgIndex] == rowgroups.get()) {
+      for (nsIFrame* rowGroup : aRowGroups) {
+        if (orderedRowGroups[rgIndex] == rowGroup) {
           nsTableRowGroupFrame* priorRG =
               (0 == rgIndex) ? nullptr : orderedRowGroups[rgIndex - 1];
           // create and add the cell map for the row group
@@ -1069,13 +1056,12 @@ void nsTableFrame::InsertRowGroups(const nsFrameList::Slice& aRowGroups) {
 
     // now that the cellmaps are reordered too insert the rows
     for (rgIndex = 0; rgIndex < orderedRowGroups.Length(); rgIndex++) {
-      for (nsFrameList::Enumerator rowgroups(aRowGroups); !rowgroups.AtEnd();
-           rowgroups.Next()) {
-        if (orderedRowGroups[rgIndex] == rowgroups.get()) {
+      for (nsIFrame* rowGroup : aRowGroups) {
+        if (orderedRowGroups[rgIndex] == rowGroup) {
           nsTableRowGroupFrame* priorRG =
               (0 == rgIndex) ? nullptr : orderedRowGroups[rgIndex - 1];
           // collect the new row frames in an array and add them to the table
-          int32_t numRows = CollectRows(rowgroups.get(), rows);
+          int32_t numRows = CollectRows(rowGroup, rows);
           if (numRows > 0) {
             int32_t rowIndex = 0;
             if (priorRG) {

layout/tables/nsTableRowFrame.cpp

@@ -215,8 +215,7 @@ void nsTableRowFrame::AppendFrames(ChildListID aListID,
 
   // Add the new cell frames to the table
   nsTableFrame* tableFrame = GetTableFrame();
-  for (nsFrameList::Enumerator e(newCells); !e.AtEnd(); e.Next()) {
-    nsIFrame* childFrame = e.get();
+  for (nsIFrame* childFrame : newCells) {
     NS_ASSERTION(childFrame->IsTableCellFrame(),
                  "Not a table cell frame/pseudo frame construction failure");
     tableFrame->AppendCell(static_cast<nsTableCellFrame&>(*childFrame),
@@ -251,8 +250,7 @@ void nsTableRowFrame::InsertFrames(ChildListID aListID, nsIFrame* aPrevFrame,
       static_cast<nsTableCellFrame*>(nsTableFrame::GetFrameAtOrBefore(
           this, aPrevFrame, LayoutFrameType::TableCell));
   nsTArray<nsTableCellFrame*> cellChildren;
-  for (nsFrameList::Enumerator e(newCells); !e.AtEnd(); e.Next()) {
-    nsIFrame* childFrame = e.get();
+  for (nsIFrame* childFrame : newCells) {
     NS_ASSERTION(childFrame->IsTableCellFrame(),
                  "Not a table cell frame/pseudo frame construction failure");
     cellChildren.AppendElement(static_cast<nsTableCellFrame*>(childFrame));