Bug 1409114 - Part 2: Get rid of generic table painting code, and handle each class separately. r=dbaron
authorMatt Woodrow <mwoodrow@mozilla.com>
Mon, 20 May 2019 23:14:45 +0000
changeset 474628 c0aa0d405e3c38c2b63b57eac0843ec7e3a0f0b3
parent 474627 a8bd87bfc775789e75c9c8721ffb681121c4f100
child 474629 1782b76bbc228f32d9056415a831fdba1034ca5a
push id36042
push userdvarga@mozilla.com
push dateTue, 21 May 2019 04:19:40 +0000
treeherdermozilla-central@ca560ff55451 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs1409114
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1409114 - Part 2: Get rid of generic table painting code, and handle each class separately. r=dbaron Most of the code in DisplayGenericTablePart was all within a per-class if statement, so it doesn't add much value, and makes the control flow harder to understand. Differential Revision: https://phabricator.services.mozilla.com/D29273
layout/generic/nsFrame.cpp
layout/tables/nsTableColFrame.cpp
layout/tables/nsTableColGroupFrame.cpp
layout/tables/nsTableFrame.cpp
layout/tables/nsTableFrame.h
layout/tables/nsTableRowFrame.cpp
layout/tables/nsTableRowFrame.h
layout/tables/nsTableRowGroupFrame.cpp
layout/tables/nsTableRowGroupFrame.h
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -2428,17 +2428,19 @@ void nsFrame::DisplayBorderBackgroundOut
 
   if (effects->HasBoxShadowWithInset(true)) {
     aLists.BorderBackground()->AppendNewToTop<nsDisplayBoxShadowInner>(aBuilder,
                                                                        this);
   }
 
   // If there's a themed background, we should not create a border item.
   // It won't be rendered.
-  if (!bgIsThemed && StyleBorder()->HasBorder()) {
+  // Don't paint borders for tables here, since they paint them in a different
+  // order.
+  if (!bgIsThemed && StyleBorder()->HasBorder() && !IsTableFrame()) {
     aLists.BorderBackground()->AppendNewToTop<nsDisplayBorder>(aBuilder, this);
   }
 
   DisplayOutlineUnconditional(aBuilder, aLists);
 }
 
 inline static bool IsSVGContentWithCSSClip(const nsIFrame* aFrame) {
   // The CSS spec says that the 'clip' property only applies to absolutely
--- a/layout/tables/nsTableColFrame.cpp
+++ b/layout/tables/nsTableColFrame.cpp
@@ -109,17 +109,63 @@ void nsTableColFrame::Reflow(nsPresConte
   if (collapseCol) {
     GetTableFrame()->SetNeedToCollapse(true);
   }
   NS_FRAME_SET_TRUNCATION(aStatus, aReflowInput, aDesiredSize);
 }
 
 void nsTableColFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
                                        const nsDisplayListSet& aLists) {
-  nsTableFrame::DisplayGenericTablePart(aBuilder, this, aLists);
+  if (IsVisibleForPainting()) {
+    // XXXbz should box-shadow for rows/rowgroups/columns/colgroups get painted
+    // just because we're visible?  Or should it depend on the cell visibility
+    // when we're not the whole table?
+
+    // Paint the outset box-shadows for the table frames
+    if (StyleEffects()->mBoxShadow) {
+      aLists.BorderBackground()->AppendNewToTop<nsDisplayBoxShadowOuter>(
+          aBuilder, this);
+    }
+  }
+
+  // Compute background rect by iterating all cell frame.
+  AutoTArray<uint32_t, 1> colIdx;
+  colIdx.AppendElement(GetColIndex());
+
+  nsTableFrame* table = GetTableFrame();
+  nsTableFrame::RowGroupArray rowGroups;
+  table->OrderRowGroups(rowGroups);
+  for (nsTableRowGroupFrame* rowGroup : rowGroups) {
+    auto offset = rowGroup->GetNormalPosition() - GetNormalPosition() -
+                  GetTableColGroupFrame()->GetNormalPosition();
+    if (!aBuilder->GetDirtyRect().Intersects(
+            nsRect(offset, rowGroup->GetSize()))) {
+      continue;
+    }
+    rowGroup->PaintCellBackgroundsForColumns(this, aBuilder, aLists, colIdx,
+                                             offset);
+  }
+
+  if (IsVisibleForPainting()) {
+    // XXXbz should box-shadow for rows/rowgroups/columns/colgroups get painted
+    // just because we're visible?  Or should it depend on the cell visibility
+    // when we're not the whole table?
+
+    // Paint the inset box-shadows for the table frames
+    if (StyleEffects()->mBoxShadow) {
+      aLists.BorderBackground()->AppendNewToTop<nsDisplayBoxShadowInner>(
+          aBuilder, this);
+    }
+  }
+
+  DisplayOutline(aBuilder, aLists);
+
+  for (nsIFrame* kid : PrincipalChildList()) {
+    BuildDisplayListForChild(aBuilder, kid, aLists);
+  }
 }
 
 int32_t nsTableColFrame::GetSpan() { return StyleTable()->mSpan; }
 
 #ifdef DEBUG
 void nsTableColFrame::Dump(int32_t aIndent) {
   char* indent = new char[aIndent + 1];
   if (!indent) return;
--- a/layout/tables/nsTableColGroupFrame.cpp
+++ b/layout/tables/nsTableColGroupFrame.cpp
@@ -347,17 +347,69 @@ void nsTableColGroupFrame::Reflow(nsPres
   }
 
   aDesiredSize.ClearSize();
   NS_FRAME_SET_TRUNCATION(aStatus, aReflowInput, aDesiredSize);
 }
 
 void nsTableColGroupFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
                                             const nsDisplayListSet& aLists) {
-  nsTableFrame::DisplayGenericTablePart(aBuilder, this, aLists);
+  if (IsVisibleForPainting()) {
+    // XXXbz should box-shadow for rows/rowgroups/columns/colgroups get painted
+    // just because we're visible?  Or should it depend on the cell visibility
+    // when we're not the whole table?
+
+    // Paint the outset box-shadows for the table frames
+    if (StyleEffects()->mBoxShadow) {
+      aLists.BorderBackground()->AppendNewToTop<nsDisplayBoxShadowOuter>(
+          aBuilder, this);
+    }
+  }
+
+  // Collecting column index.
+  AutoTArray<uint32_t, 1> colIdx;
+  for (nsTableColFrame* col = GetFirstColumn(); col; col = col->GetNextCol()) {
+    MOZ_ASSERT(colIdx.IsEmpty() || static_cast<uint32_t>(col->GetColIndex()) >
+                                       colIdx.LastElement());
+    colIdx.AppendElement(col->GetColIndex());
+  }
+
+  if (!colIdx.IsEmpty()) {
+    // We have some actual cells that live inside this rowgroup.
+    nsTableFrame* table = GetTableFrame();
+    nsTableFrame::RowGroupArray rowGroups;
+    table->OrderRowGroups(rowGroups);
+    for (nsTableRowGroupFrame* rowGroup : rowGroups) {
+      auto offset = rowGroup->GetNormalPosition() - GetNormalPosition();
+      if (!aBuilder->GetDirtyRect().Intersects(
+              nsRect(offset, rowGroup->GetSize()))) {
+        continue;
+      }
+      rowGroup->PaintCellBackgroundsForColumns(this, aBuilder, aLists, colIdx,
+                                               offset);
+    }
+  }
+
+  if (IsVisibleForPainting()) {
+    // XXXbz should box-shadow for rows/rowgroups/columns/colgroups get painted
+    // just because we're visible?  Or should it depend on the cell visibility
+    // when we're not the whole table?
+
+    // Paint the inset box-shadows for the table frames
+    if (StyleEffects()->mBoxShadow) {
+      aLists.BorderBackground()->AppendNewToTop<nsDisplayBoxShadowInner>(
+          aBuilder, this);
+    }
+  }
+
+  DisplayOutline(aBuilder, aLists);
+
+  for (nsIFrame* kid : PrincipalChildList()) {
+    BuildDisplayListForChild(aBuilder, kid, aLists);
+  }
 }
 
 nsTableColFrame* nsTableColGroupFrame::GetFirstColumn() {
   return GetNextColumn(nullptr);
 }
 
 nsTableColFrame* nsTableColGroupFrame::GetNextColumn(nsIFrame* aChildFrame) {
   nsTableColFrame* result = nullptr;
--- a/layout/tables/nsTableFrame.cpp
+++ b/layout/tables/nsTableFrame.cpp
@@ -1210,116 +1210,16 @@ bool nsDisplayTableBorderCollapse::Creat
     const StackingContextHelper& aSc,
     mozilla::layers::RenderRootStateManager* aManager,
     nsDisplayListBuilder* aDisplayListBuilder) {
   static_cast<nsTableFrame*>(mFrame)->CreateWebRenderCommandsForBCBorders(
       aBuilder, aSc, GetPaintRect(), ToReferenceFrame());
   return true;
 }
 
-/* static */
-void nsTableFrame::GenericTraversal(nsDisplayListBuilder* aBuilder,
-                                    nsFrame* aFrame,
-                                    const nsDisplayListSet& aLists) {
-  // This is similar to what
-  // nsContainerFrame::BuildDisplayListForNonBlockChildren does, except that we
-  // allow the children's background and borders to go in our BorderBackground
-  // list. This doesn't really affect background painting --- the children won't
-  // actually draw their own backgrounds because the nsTableFrame already drew
-  // them, unless a child has its own stacking context, in which case the child
-  // won't use its passed-in BorderBackground list anyway. It does affect cell
-  // borders though; this lets us get cell borders into the nsTableFrame's
-  // BorderBackground list.
-  for (nsIFrame* kid : aFrame->GetChildList(kColGroupList)) {
-    aFrame->BuildDisplayListForChild(aBuilder, kid, aLists);
-  }
-
-  for (nsIFrame* kid : aFrame->PrincipalChildList()) {
-    aFrame->BuildDisplayListForChild(aBuilder, kid, aLists);
-  }
-}
-
-static void PaintRowBackground(nsTableRowFrame* aRow, nsIFrame* aFrame,
-                               nsDisplayListBuilder* aBuilder,
-                               const nsDisplayListSet& aLists,
-                               const nsPoint& aOffset = nsPoint()) {
-  // Compute background rect by iterating all cell frame.
-  for (nsTableCellFrame* cell = aRow->GetFirstCell(); cell;
-       cell = cell->GetNextCell()) {
-    if (!cell->ShouldPaintBackground(aBuilder)) {
-      continue;
-    }
-
-    auto cellRect =
-        cell->GetRectRelativeToSelf() + cell->GetNormalPosition() + aOffset;
-    if (!aBuilder->GetDirtyRect().Intersects(cellRect)) {
-      continue;
-    }
-    nsDisplayBackgroundImage::AppendBackgroundItemsToTop(
-        aBuilder, aFrame, cellRect, aLists.BorderBackground(), true, nullptr,
-        aFrame->GetRectRelativeToSelf(), cell);
-  }
-}
-
-static void PaintRowGroupBackground(nsTableRowGroupFrame* aRowGroup,
-                                    nsIFrame* aFrame,
-                                    nsDisplayListBuilder* aBuilder,
-                                    const nsDisplayListSet& aLists) {
-  for (nsTableRowFrame* row = aRowGroup->GetFirstRow(); row;
-       row = row->GetNextRow()) {
-    if (!aBuilder->GetDirtyRect().Intersects(
-            nsRect(row->GetNormalPosition(), row->GetSize()))) {
-      continue;
-    }
-    PaintRowBackground(row, aFrame, aBuilder, aLists, row->GetNormalPosition());
-  }
-}
-
-static void PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup,
-                                            nsIFrame* aFrame,
-                                            nsDisplayListBuilder* aBuilder,
-                                            const nsDisplayListSet& aLists,
-                                            const nsTArray<uint32_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 (!aBuilder->GetDirtyRect().Intersects(nsRect(rowPos, row->GetSize()))) {
-      continue;
-    }
-    for (nsTableCellFrame* cell = row->GetFirstCell(); cell;
-         cell = cell->GetNextCell()) {
-      uint32_t curColIdx = cell->ColIndex();
-      if (!aColIdx.ContainsSorted(curColIdx)) {
-        if (curColIdx > aColIdx.LastElement()) {
-          // We can just stop looking at this row.
-          break;
-        }
-        continue;
-      }
-
-      if (!cell->ShouldPaintBackground(aBuilder)) {
-        continue;
-      }
-
-      auto cellPos = cell->GetNormalPosition() + rowPos;
-      auto cellRect = nsRect(cellPos, cell->GetSize());
-      if (!aBuilder->GetDirtyRect().Intersects(cellRect)) {
-        continue;
-      }
-      nsDisplayBackgroundImage::AppendBackgroundItemsToTop(
-          aBuilder, aFrame, cellRect, aLists.BorderBackground(), false, nullptr,
-          aFrame->GetRectRelativeToSelf(), cell);
-    }
-  }
-}
-
 static inline bool FrameHasBorder(nsIFrame* f) {
   if (!f->StyleVisibility()->IsVisible()) {
     return false;
   }
 
   if (f->StyleBorder()->HasBorder()) {
     return true;
   }
@@ -1379,140 +1279,56 @@ void nsTableFrame::CalcHasBCBorders() {
         }
       }
     }
   }
 
   SetHasBCBorders(false);
 }
 
-/* static */
-void nsTableFrame::DisplayGenericTablePart(
-    nsDisplayListBuilder* aBuilder, nsFrame* aFrame,
-    const nsDisplayListSet& aLists,
-    DisplayGenericTablePartTraversal aTraversal) {
-  bool isVisible = aFrame->IsVisibleForPainting();
-  bool isTable = aFrame->IsTableFrame();
-
-  if (isVisible) {
-    // XXXbz should box-shadow for rows/rowgroups/columns/colgroups get painted
-    // just because we're visible?  Or should it depend on the cell visibility
-    // when we're not the whole table?
-
-    // Paint the outset box-shadows for the table frames
-    if (!aFrame->StyleEffects()->mBoxShadow.IsEmpty()) {
-      aLists.BorderBackground()->AppendNewToTop<nsDisplayBoxShadowOuter>(
-          aBuilder, aFrame);
-    }
-  }
-
-  // Background visibility for rows, rowgroups, columns, colgroups depends on
-  // the visibility of the _cell_, not of the row/col(group).
-  if (aFrame->IsTableRowGroupFrame()) {
-    nsTableRowGroupFrame* rowGroup = static_cast<nsTableRowGroupFrame*>(aFrame);
-    PaintRowGroupBackground(rowGroup, aFrame, aBuilder, aLists);
-  } else if (aFrame->IsTableRowFrame()) {
-    nsTableRowFrame* row = static_cast<nsTableRowFrame*>(aFrame);
-    PaintRowBackground(row, aFrame, aBuilder, aLists);
-  } else if (aFrame->IsTableColGroupFrame()) {
-    // Compute background rect by iterating all cell frame.
-    nsTableColGroupFrame* colGroup = static_cast<nsTableColGroupFrame*>(aFrame);
-    // Collecting column index.
-    AutoTArray<uint32_t, 1> colIdx;
-    for (nsTableColFrame* col = colGroup->GetFirstColumn(); col;
-         col = col->GetNextCol()) {
-      MOZ_ASSERT(colIdx.IsEmpty() || static_cast<uint32_t>(col->GetColIndex()) >
-                                         colIdx.LastElement());
-      colIdx.AppendElement(col->GetColIndex());
-    }
-
-    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 (!aBuilder->GetDirtyRect().Intersects(
-                nsRect(offset, rowGroup->GetSize()))) {
-          continue;
-        }
-        PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists,
-                                        colIdx, offset);
-      }
-    }
-  } else if (aFrame->IsTableColFrame()) {
-    // Compute background rect by iterating all cell frame.
-    nsTableColFrame* col = static_cast<nsTableColFrame*>(aFrame);
-    AutoTArray<uint32_t, 1> colIdx;
-    colIdx.AppendElement(col->GetColIndex());
-
-    nsTableFrame* table = col->GetTableFrame();
-    RowGroupArray rowGroups;
-    table->OrderRowGroups(rowGroups);
-    for (nsTableRowGroupFrame* rowGroup : rowGroups) {
-      auto offset = rowGroup->GetNormalPosition() - col->GetNormalPosition() -
-                    col->GetTableColGroupFrame()->GetNormalPosition();
-      if (!aBuilder->GetDirtyRect().Intersects(
-              nsRect(offset, rowGroup->GetSize()))) {
-        continue;
-      }
-      PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists,
-                                      colIdx, offset);
-    }
-  } else if (isVisible) {
-    nsDisplayBackgroundImage::AppendBackgroundItemsToTop(
-        aBuilder, aFrame, aFrame->GetRectRelativeToSelf(),
-        aLists.BorderBackground());
-  }
-
-  if (isVisible) {
-    // XXXbz should box-shadow for rows/rowgroups/columns/colgroups get painted
-    // just because we're visible?  Or should it depend on the cell visibility
-    // when we're not the whole table?
-
-    // Paint the inset box-shadows for the table frames
-    if (!aFrame->StyleEffects()->mBoxShadow.IsEmpty()) {
-      aLists.BorderBackground()->AppendNewToTop<nsDisplayBoxShadowInner>(
-          aBuilder, aFrame);
-    }
-  }
-
-  aFrame->DisplayOutline(aBuilder, aLists);
-
-  aTraversal(aBuilder, aFrame, aLists);
-
-  if (isVisible) {
-    if (isTable) {
-      nsTableFrame* table = static_cast<nsTableFrame*>(aFrame);
-      // In the collapsed border model, overlay all collapsed borders.
-      if (table->IsBorderCollapse()) {
-        if (table->HasBCBorders()) {
-          aLists.BorderBackground()
-              ->AppendNewToTop<nsDisplayTableBorderCollapse>(aBuilder, table);
-        }
-      } else {
-        const nsStyleBorder* borderStyle = aFrame->StyleBorder();
-        if (borderStyle->HasBorder()) {
-          aLists.BorderBackground()->AppendNewToTop<nsDisplayBorder>(aBuilder,
-                                                                     table);
-        }
-      }
-    }
-  }
-}
-
 // table paint code is concerned primarily with borders and bg color
 // SEC: TODO: adjust the rect for captions
 void nsTableFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
                                     const nsDisplayListSet& aLists) {
   DO_GLOBAL_REFLOW_COUNT_DSP_COLOR("nsTableFrame", NS_RGB(255, 128, 255));
 
-  DisplayGenericTablePart(aBuilder, this, aLists);
+  DisplayBorderBackgroundOutline(aBuilder, aLists);
+
+  // This is similar to what
+  // nsContainerFrame::BuildDisplayListForNonBlockChildren does, except that we
+  // allow the children's background and borders to go in our BorderBackground
+  // list. This doesn't really affect background painting --- the children won't
+  // actually draw their own backgrounds because the nsTableFrame already drew
+  // them, unless a child has its own stacking context, in which case the child
+  // won't use its passed-in BorderBackground list anyway. It does affect cell
+  // borders though; this lets us get cell borders into the nsTableFrame's
+  // BorderBackground list.
+  for (nsIFrame* kid : GetChildList(kColGroupList)) {
+    BuildDisplayListForChild(aBuilder, kid, aLists);
+  }
+
+  for (nsIFrame* kid : PrincipalChildList()) {
+    BuildDisplayListForChild(aBuilder, kid, aLists);
+  }
+
+  if (IsVisibleForPainting()) {
+    // In the collapsed border model, overlay all collapsed borders.
+    if (IsBorderCollapse()) {
+      if (HasBCBorders()) {
+        aLists.BorderBackground()->AppendNewToTop<nsDisplayTableBorderCollapse>(
+            aBuilder, this);
+      }
+    } else {
+      const nsStyleBorder* borderStyle = StyleBorder();
+      if (borderStyle->HasBorder()) {
+        aLists.BorderBackground()->AppendNewToTop<nsDisplayBorder>(aBuilder,
+                                                                   this);
+      }
+    }
+  }
 }
 
 nsMargin nsTableFrame::GetDeflationForBackground(
     nsPresContext* aPresContext) const {
   if (eCompatibility_NavQuirks != aPresContext->CompatibilityMode() ||
       !IsBorderCollapse())
     return nsMargin(0, 0, 0, 0);
 
--- a/layout/tables/nsTableFrame.h
+++ b/layout/tables/nsTableFrame.h
@@ -181,38 +181,16 @@ class nsTableFrame : public nsContainerF
 
   /* Like GetTableFrame, but will set *aDidPassThrough to false if we don't
    * pass through aMustPassThrough on the way to the table.
    */
   static nsTableFrame* GetTableFramePassingThrough(nsIFrame* aMustPassThrough,
                                                    nsIFrame* aSourceFrame,
                                                    bool* aDidPassThrough);
 
-  typedef void (*DisplayGenericTablePartTraversal)(
-      nsDisplayListBuilder* aBuilder, nsFrame* aFrame,
-      const nsDisplayListSet& aLists);
-  static void GenericTraversal(nsDisplayListBuilder* aBuilder, nsFrame* aFrame,
-                               const nsDisplayListSet& aLists);
-
-  /**
-   * Helper method to handle display common to table frames, rowgroup frames
-   * and row frames. It creates a background display item for handling events
-   * if necessary, an outline display item if necessary, and displays
-   * all the the frame's children.
-   * @param aDisplayItem the display item created for this part, or null
-   * if this part's border/background painting is delegated to an ancestor
-   * @param aTraversal a function that gets called to traverse the table
-   * part's child frames and add their display list items to a
-   * display list set.
-   */
-  static void DisplayGenericTablePart(
-      nsDisplayListBuilder* aBuilder, nsFrame* aFrame,
-      const nsDisplayListSet& aLists,
-      DisplayGenericTablePartTraversal aTraversal = GenericTraversal);
-
   // Return the closest sibling of aPriorChildFrame (including aPriroChildFrame)
   // of type aChildType.
   static nsIFrame* GetFrameAtOrBefore(nsIFrame* aParentFrame,
                                       nsIFrame* aPriorChildFrame,
                                       mozilla::LayoutFrameType aChildType);
   bool IsAutoBSize(mozilla::WritingMode aWM);
 
   /** @return true if aDisplayType represents a rowgroup of any sort
--- a/layout/tables/nsTableRowFrame.cpp
+++ b/layout/tables/nsTableRowFrame.cpp
@@ -527,19 +527,70 @@ nscoord nsTableRowFrame::CalcBSize(const
         ascent = cellFrame->GetCellBaseline();
       nscoord descent = desSize.BSize(wm) - ascent;
       UpdateBSize(desSize.BSize(wm), ascent, descent, tableFrame, cellFrame);
     }
   }
   return GetInitialBSize();
 }
 
+void nsTableRowFrame::PaintCellBackgroundsForFrame(
+    nsIFrame* aFrame, nsDisplayListBuilder* aBuilder,
+    const nsDisplayListSet& aLists, const nsPoint& aOffset) {
+  // Compute background rect by iterating all cell frame.
+  for (nsTableCellFrame* cell = GetFirstCell(); cell;
+       cell = cell->GetNextCell()) {
+    if (!cell->ShouldPaintBackground(aBuilder)) {
+      continue;
+    }
+
+    auto cellRect =
+        cell->GetRectRelativeToSelf() + cell->GetNormalPosition() + aOffset;
+    if (!aBuilder->GetDirtyRect().Intersects(cellRect)) {
+      continue;
+    }
+    nsDisplayBackgroundImage::AppendBackgroundItemsToTop(
+        aBuilder, aFrame, cellRect, aLists.BorderBackground(), true, nullptr,
+        aFrame->GetRectRelativeToSelf(), cell);
+  }
+}
+
 void nsTableRowFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
                                        const nsDisplayListSet& aLists) {
-  nsTableFrame::DisplayGenericTablePart(aBuilder, this, aLists);
+  if (IsVisibleForPainting()) {
+    // XXXbz should box-shadow for rows/rowgroups/columns/colgroups get painted
+    // just because we're visible?  Or should it depend on the cell visibility
+    // when we're not the whole table?
+
+    // Paint the outset box-shadows for the table frames
+    if (StyleEffects()->mBoxShadow) {
+      aLists.BorderBackground()->AppendNewToTop<nsDisplayBoxShadowOuter>(
+          aBuilder, this);
+    }
+  }
+
+  PaintCellBackgroundsForFrame(this, aBuilder, aLists);
+
+  if (IsVisibleForPainting()) {
+    // XXXbz should box-shadow for rows/rowgroups/columns/colgroups get painted
+    // just because we're visible?  Or should it depend on the cell visibility
+    // when we're not the whole table?
+
+    // Paint the inset box-shadows for the table frames
+    if (StyleEffects()->mBoxShadow) {
+      aLists.BorderBackground()->AppendNewToTop<nsDisplayBoxShadowInner>(
+          aBuilder, this);
+    }
+  }
+
+  DisplayOutline(aBuilder, aLists);
+
+  for (nsIFrame* kid : PrincipalChildList()) {
+    BuildDisplayListForChild(aBuilder, kid, aLists);
+  }
 }
 
 nsIFrame::LogicalSides nsTableRowFrame::GetLogicalSkipSides(
     const ReflowInput* aReflowInput) const {
   if (MOZ_UNLIKELY(StyleBorder()->mBoxDecorationBreak ==
                    StyleBoxDecorationBreak::Clone)) {
     return LogicalSides();
   }
--- a/layout/tables/nsTableRowFrame.h
+++ b/layout/tables/nsTableRowFrame.h
@@ -71,16 +71,21 @@ class nsTableRowFrame : public nsContain
 
   virtual nsMargin GetUsedMargin() const override;
   virtual nsMargin GetUsedBorder() const override;
   virtual nsMargin GetUsedPadding() const override;
 
   virtual void BuildDisplayList(nsDisplayListBuilder* aBuilder,
                                 const nsDisplayListSet& aLists) override;
 
+  void PaintCellBackgroundsForFrame(nsIFrame* aFrame,
+                                    nsDisplayListBuilder* aBuilder,
+                                    const nsDisplayListSet& aLists,
+                                    const nsPoint& aOffset = nsPoint());
+
   // Implemented in nsTableCellFrame.h, because it needs to know about the
   // nsTableCellFrame class, but we can't include nsTableCellFrame.h here.
   inline nsTableCellFrame* GetFirstCell() const;
 
   /** calls Reflow for all of its child cells.
    *
    * Cells with rowspan=1 are all set to the same height and stacked
    * horizontally.
--- a/layout/tables/nsTableRowGroupFrame.cpp
+++ b/layout/tables/nsTableRowGroupFrame.cpp
@@ -231,19 +231,93 @@ static void DisplayRows(nsDisplayListBui
 
     kid = kid->GetNextSibling();
   }
   if (cursor) {
     cursor->FinishBuildingCursor();
   }
 }
 
+void nsTableRowGroupFrame::PaintCellBackgroundsForColumns(
+    nsIFrame* aFrame, nsDisplayListBuilder* aBuilder,
+    const nsDisplayListSet& aLists, const nsTArray<uint32_t>& aColIdx,
+    const nsPoint& aOffset) {
+  MOZ_DIAGNOSTIC_ASSERT(!aColIdx.IsEmpty(),
+                        "Must be painting backgrounds for something");
+
+  for (nsTableRowFrame* row = GetFirstRow(); row; row = row->GetNextRow()) {
+    auto rowPos = row->GetNormalPosition() + aOffset;
+    if (!aBuilder->GetDirtyRect().Intersects(nsRect(rowPos, row->GetSize()))) {
+      continue;
+    }
+    for (nsTableCellFrame* cell = row->GetFirstCell(); cell;
+         cell = cell->GetNextCell()) {
+      uint32_t curColIdx = cell->ColIndex();
+      if (!aColIdx.ContainsSorted(curColIdx)) {
+        if (curColIdx > aColIdx.LastElement()) {
+          // We can just stop looking at this row.
+          break;
+        }
+        continue;
+      }
+
+      if (!cell->ShouldPaintBackground(aBuilder)) {
+        continue;
+      }
+
+      auto cellPos = cell->GetNormalPosition() + rowPos;
+      auto cellRect = nsRect(cellPos, cell->GetSize());
+      if (!aBuilder->GetDirtyRect().Intersects(cellRect)) {
+        continue;
+      }
+      nsDisplayBackgroundImage::AppendBackgroundItemsToTop(
+          aBuilder, aFrame, cellRect, aLists.BorderBackground(), false, nullptr,
+          aFrame->GetRectRelativeToSelf(), cell);
+    }
+  }
+}
+
 void nsTableRowGroupFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
                                             const nsDisplayListSet& aLists) {
-  nsTableFrame::DisplayGenericTablePart(aBuilder, this, aLists, DisplayRows);
+  if (IsVisibleForPainting()) {
+    // XXXbz should box-shadow for rows/rowgroups/columns/colgroups get painted
+    // just because we're visible?  Or should it depend on the cell visibility
+    // when we're not the whole table?
+
+    // Paint the outset box-shadows for the table frames
+    if (StyleEffects()->mBoxShadow) {
+      aLists.BorderBackground()->AppendNewToTop<nsDisplayBoxShadowOuter>(
+          aBuilder, this);
+    }
+  }
+
+  for (nsTableRowFrame* row = GetFirstRow(); row; row = row->GetNextRow()) {
+    if (!aBuilder->GetDirtyRect().Intersects(
+            nsRect(row->GetNormalPosition(), row->GetSize()))) {
+      continue;
+    }
+    row->PaintCellBackgroundsForFrame(this, aBuilder, aLists,
+                                      row->GetNormalPosition());
+  }
+
+  if (IsVisibleForPainting()) {
+    // XXXbz should box-shadow for rows/rowgroups/columns/colgroups get painted
+    // just because we're visible?  Or should it depend on the cell visibility
+    // when we're not the whole table?
+
+    // Paint the inset box-shadows for the table frames
+    if (StyleEffects()->mBoxShadow) {
+      aLists.BorderBackground()->AppendNewToTop<nsDisplayBoxShadowInner>(
+          aBuilder, this);
+    }
+  }
+
+  DisplayOutline(aBuilder, aLists);
+
+  DisplayRows(aBuilder, this, aLists);
 }
 
 nsIFrame::LogicalSides nsTableRowGroupFrame::GetLogicalSkipSides(
     const ReflowInput* aReflowInput) const {
   if (MOZ_UNLIKELY(StyleBorder()->mBoxDecorationBreak ==
                    StyleBoxDecorationBreak::Clone)) {
     return LogicalSides();
   }
--- a/layout/tables/nsTableRowGroupFrame.h
+++ b/layout/tables/nsTableRowGroupFrame.h
@@ -71,16 +71,22 @@ class nsTableRowGroupFrame final : publi
 
   virtual nsMargin GetUsedMargin() const override;
   virtual nsMargin GetUsedBorder() const override;
   virtual nsMargin GetUsedPadding() const override;
 
   virtual void BuildDisplayList(nsDisplayListBuilder* aBuilder,
                                 const nsDisplayListSet& aLists) override;
 
+  void PaintCellBackgroundsForColumns(nsIFrame* aFrame,
+                                      nsDisplayListBuilder* aBuilder,
+                                      const nsDisplayListSet& aLists,
+                                      const nsTArray<uint32_t>& aColIdx,
+                                      const nsPoint& aOffset);
+
   /**
    * Calls Reflow for all of its child rows.
    *
    * Rows are all set to the same isize and stacked in the block direction.
    *
    * Rows are not split unless absolutely necessary.
    *
    * @param aDesiredSize isize set to isize of rows, bsize set to