Issue #1355 - Do less work for columns not in the desired set in PaintRowGroupBackgroundByColIdx

Do less work in PaintRowGroupBackgroundByColIdx for cells that are not in our desired set of columns.

crash fix:

Guard against empty column groups when building a display list for a table.

1 files changed, 26 insertions, 12 deletions

diff --git a/layout/tables/nsTableFrame.cpp b/layout/tables/nsTableFrame.cpp
index 0e54fd093e..868f83bda2 100644
--- a/layout/tables/nsTableFrame.cpp
+++ b/layout/tables/nsTableFrame.cpp
@@ -1237,19 +1237,29 @@ PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup,
                                 const nsTArray<int32_t>& aColIdx,
                                 const nsPoint& aOffset)
 {
+  MOZ_DIAGNOSTIC_ASSERT(!aColIdx.IsEmpty(),
+                        "Must be painting backgrounds for something");
   for (nsTableRowFrame* row = aRowGroup->GetFirstRow(); row; row = row->GetNextRow()) {
     auto rowPos = row->GetNormalPosition() + aOffset;
     if (!aDirtyRect.Intersects(nsRect(rowPos, row->GetSize()))) {
       continue;
     }
     for (nsTableCellFrame* cell = row->GetFirstCell(); cell; cell = cell->GetNextCell()) {
+
+      int32_t curColIdx;
+      cell->GetColIndex(curColIdx);
+      if (!aColIdx.Contains(curColIdx)) {
+        if (curColIdx > aColIdx.LastElement()) {
+          // We can just stop looking at this row.
+          break;
+        }
+        continue;
+      }
+
       if (!cell->ShouldPaintBackground(aBuilder)) {
         continue;
       }
 
-      int32_t curColIdx;
-      cell->GetColIndex(curColIdx);
-      if (aColIdx.Contains(curColIdx)) {
         auto cellPos = cell->GetNormalPosition() + rowPos;
         auto cellRect = nsRect(cellPos, cell->GetSize());
         if (!aDirtyRect.Intersects(cellRect)) {
@@ -1260,7 +1270,6 @@ PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup,
                                                              true, nullptr,
                                                              aFrame->GetRectRelativeToSelf(),
                                                              cell);
-      }
     }
   }
 }
@@ -1311,18 +1320,23 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder,
     // Collecting column index.
     AutoTArray<int32_t, 1> colIdx;
     for (nsTableColFrame* col = colGroup->GetFirstColumn(); col; col = col->GetNextCol()) {
+      MOZ_ASSERT(colIdx.IsEmpty() ||
+                 col->GetColIndex() > colIdx.LastElement());
       colIdx.AppendElement(col->GetColIndex());
     }
 
-    nsTableFrame* table = colGroup->GetTableFrame();
-    RowGroupArray rowGroups;
-    table->OrderRowGroups(rowGroups);
-    for (nsTableRowGroupFrame* rowGroup : rowGroups) {
-      auto offset = rowGroup->GetNormalPosition() - colGroup->GetNormalPosition();
-      if (!aDirtyRect.Intersects(nsRect(offset, rowGroup->GetSize()))) {
-        continue;
-      }
+    if (!colIdx.IsEmpty()) {
+      // We have some actual cells that live inside this rowgroup.
+      nsTableFrame* table = colGroup->GetTableFrame();
+      RowGroupArray rowGroups;
+      table->OrderRowGroups(rowGroups);
+      for (nsTableRowGroupFrame* rowGroup : rowGroups) {
+        auto offset = rowGroup->GetNormalPosition() - colGroup->GetNormalPosition();
+        if (!aDirtyRect.Intersects(nsRect(offset, rowGroup->GetSize()))) {
+          continue;
+        }
       PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, aDirtyRect, colIdx, offset);
+      }
     }
   } else if (aFrame->GetType() == nsGkAtoms::tableColFrame) {
     // Compute background rect by iterating all cell frame.