Issue #1355 - Better way to create display items for column backgrounds

Part 1: Remove current table item, as it's never set.

Part 2: Get rid of generic table painting code, and handle each class separately.

Part 4: Hoist outline skipping into col(group) frame code.

Part 5: Skip box-shadow for table column and column groups.

Part 6: Store column and column group backgrounds separately, and then append them before the rest of the table contents.

Part 7: Pass rects in display list coordinates to AppendBackgroundItemsToTop.

Part 8: Create column and column group background display items as part of the cell's BuildDisplayList.

Part 9: Used cached values instead of calling nsDisplayListBuilder::ToReferenceFrame when possible, since it can be expensive when the requested frame isn't the builder's current frame.

Part 10: Make sure we build display items for table parts where only the normal position is visible, since we may need to create background items for ancestors at that position.

Part 11: Create an AutoBuildingDisplayList when we create background items for table columns and column groups, so that we initialize the invalidation state correctly.

14 files changed, 337 insertions, 293 deletions

diff --git a/layout/tables/nsTableCellFrame.cpp b/layout/tables/nsTableCellFrame.cpp
index 8b811df1e..9c715d999 100644
--- a/layout/tables/nsTableCellFrame.cpp
+++ b/layout/tables/nsTableCellFrame.cpp
@@ -450,7 +450,6 @@ nsTableCellFrame::ShouldPaintBackground(nsDisplayListBuilder* aBuilder)
 
 void
 nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
-                                   const nsRect&           aDirtyRect,
                                    const nsDisplayListSet& aLists)
 {
   DO_GLOBAL_REFLOW_COUNT_DSP("nsTableCellFrame");
@@ -462,14 +461,14 @@ nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
         new (aBuilder) nsDisplayBoxShadowOuter(aBuilder, this));
     }
 
+    nsRect bgRect = GetRectRelativeToSelf() + aBuilder->ToReferenceFrame(this);
+
     // display background if we need to.
     if (aBuilder->IsForEventDelivery() ||
         !StyleBackground()->IsTransparent() ||
         StyleDisplay()->mAppearance) {
-      nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder,
-          this,
-          GetRectRelativeToSelf(),
-          aLists.BorderBackground());
+      nsDisplayBackgroundImage::AppendBackgroundItemsToTop(
+          aBuilder, this, bgRect, aLists.BorderBackground());
     }
 
     // display inset box-shadows if we need to.
@@ -488,16 +487,49 @@ nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
                          "TableCellSelection",
                          nsDisplayItem::TYPE_TABLE_CELL_SELECTION));
     }
+
+  // This can be null if display list building initiated in the middle
+    // of the table, which can happen with background-clip:text and
+    // -moz-element.
+    nsDisplayTableBackgroundSet* backgrounds =
+        aBuilder->GetTableBackgroundSet();
+    if (backgrounds) {
+      // Compute bgRect relative to reference frame, but using the
+      // normal (without position:relative offsets) positions for the
+      // cell, row and row group.
+      bgRect = GetRectRelativeToSelf() + GetNormalPosition();
+
+      nsTableRowFrame* row = GetTableRowFrame();
+      bgRect += row->GetNormalPosition();
+
+      nsTableRowGroupFrame* rowGroup = row->GetTableRowGroupFrame();
+      bgRect += rowGroup->GetNormalPosition();
+
+      bgRect += backgrounds->TableToReferenceFrame();
+
+      // Create backgrounds items as needed for the column and column
+      // group that this cell occupies.
+      nsTableColFrame* col = backgrounds->GetColForIndex(ColIndex());
+      nsTableColGroupFrame* colGroup = col->GetTableColGroupFrame();
+
+      Maybe<nsDisplayListBuilder::AutoBuildingDisplayList> buildingForColGroup;
+      nsDisplayBackgroundImage::AppendBackgroundItemsToTop(
+          aBuilder, colGroup, bgRect, backgrounds->ColGroupBackgrounds(), false,
+          nullptr, colGroup->GetRect() + backgrounds->TableToReferenceFrame(),
+          this, &buildingForColGroup);
+
+      Maybe<nsDisplayListBuilder::AutoBuildingDisplayList> buildingForCol;
+      nsDisplayBackgroundImage::AppendBackgroundItemsToTop(
+          aBuilder, col, bgRect, backgrounds->ColBackgrounds(), false, nullptr,
+          col->GetRect() + colGroup->GetPosition() +
+          backgrounds->TableToReferenceFrame(),
+          this, &buildingForCol);
+    }
   }
 
   // the 'empty-cells' property has no effect on 'outline'
   DisplayOutline(aBuilder, aLists);
 
-  // Push a null 'current table item' so that descendant tables can't
-  // accidentally mess with our table
-  nsAutoPushCurrentTableItem pushTableItem;
-  pushTableItem.Push(aBuilder, nullptr);
-
   nsIFrame* kid = mFrames.FirstChild();
   NS_ASSERTION(kid && !kid->GetNextSibling(), "Table cells should have just one child");
   // The child's background will go in our BorderBackground() list.
@@ -506,7 +538,7 @@ nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
   // because that/ would put the child's background in the Content() list
   // which isn't right (e.g., would end up on top of our child floats for
   // event handling).
-  BuildDisplayListForChild(aBuilder, kid, aDirtyRect, aLists);
+  BuildDisplayListForChild(aBuilder, kid, aLists);
 }
 
 nsIFrame::LogicalSides
diff --git a/layout/tables/nsTableCellFrame.h b/layout/tables/nsTableCellFrame.h
index f626a45b0..a822e309d 100644
--- a/layout/tables/nsTableCellFrame.h
+++ b/layout/tables/nsTableCellFrame.h
@@ -104,7 +104,6 @@ public:
   virtual bool NeedsToObserve(const ReflowInput& aReflowInput) override;
 
   virtual void BuildDisplayList(nsDisplayListBuilder*   aBuilder,
-                                const nsRect&           aDirtyRect,
                                 const nsDisplayListSet& aLists) override;
 
   virtual nsresult ProcessBorders(nsTableFrame* aFrame,
diff --git a/layout/tables/nsTableColFrame.cpp b/layout/tables/nsTableColFrame.cpp
index 54b03522b..6723f4701 100644
--- a/layout/tables/nsTableColFrame.cpp
+++ b/layout/tables/nsTableColFrame.cpp
@@ -110,10 +110,15 @@ nsTableColFrame::Reflow(nsPresContext*          aPresContext,
 
 void
 nsTableColFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
-                                  const nsRect&           aDirtyRect,
                                   const nsDisplayListSet& aLists)
 {
-  nsTableFrame::DisplayGenericTablePart(aBuilder, this, aDirtyRect, aLists);
+  // Per https://drafts.csswg.org/css-tables-3/#global-style-overrides:
+  // "All css properties of table-column and table-column-group boxes are
+  // ignored, except when explicitly specified by this specification."
+  // CSS outlines and box-shadows fall into this category, so we skip them
+  // on these boxes.
+
+  MOZ_ASSERT_UNREACHABLE("Cols don't paint themselves");
 }
 
 int32_t nsTableColFrame::GetSpan()
diff --git a/layout/tables/nsTableColFrame.h b/layout/tables/nsTableColFrame.h
index fb989061f..ed6a843b6 100644
--- a/layout/tables/nsTableColFrame.h
+++ b/layout/tables/nsTableColFrame.h
@@ -60,7 +60,6 @@ public:
                       nsReflowStatus&          aStatus) override;
 
   virtual void BuildDisplayList(nsDisplayListBuilder*   aBuilder,
-                                const nsRect&           aDirtyRect,
                                 const nsDisplayListSet& aLists) override;
 
   /**
diff --git a/layout/tables/nsTableColGroupFrame.cpp b/layout/tables/nsTableColGroupFrame.cpp
index 6ee7f0b24..be5b71bb2 100644
--- a/layout/tables/nsTableColGroupFrame.cpp
+++ b/layout/tables/nsTableColGroupFrame.cpp
@@ -385,10 +385,15 @@ nsTableColGroupFrame::Reflow(nsPresContext*          aPresContext,
 
 void
 nsTableColGroupFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
-                                       const nsRect&           aDirtyRect,
                                        const nsDisplayListSet& aLists)
 {
-  nsTableFrame::DisplayGenericTablePart(aBuilder, this, aDirtyRect, aLists);
+  // Per https://drafts.csswg.org/css-tables-3/#global-style-overrides:
+  // "All css properties of table-column and table-column-group boxes are
+  // ignored, except when explicitly specified by this specification."
+  // CSS outlines and box-shadows fall into this category, so we skip them
+  // on these boxes.
+
+  MOZ_ASSERT_UNREACHABLE("Colgroups don't paint themselves");
 }
 
 nsTableColFrame * nsTableColGroupFrame::GetFirstColumn()
diff --git a/layout/tables/nsTableColGroupFrame.h b/layout/tables/nsTableColGroupFrame.h
index b3dfb94e7..88ed67c2a 100644
--- a/layout/tables/nsTableColGroupFrame.h
+++ b/layout/tables/nsTableColGroupFrame.h
@@ -44,7 +44,6 @@ public:
   }
 
   virtual void BuildDisplayList(nsDisplayListBuilder*   aBuilder,
-                                const nsRect&           aDirtyRect,
                                 const nsDisplayListSet& aLists) override;
 
   /** A colgroup can be caused by three things:
diff --git a/layout/tables/nsTableFrame.cpp b/layout/tables/nsTableFrame.cpp
index 4257c9c57..32fe38b05 100644
--- a/layout/tables/nsTableFrame.cpp
+++ b/layout/tables/nsTableFrame.cpp
@@ -1161,242 +1161,127 @@ nsDisplayTableBorderCollapse::Paint(nsDisplayListBuilder* aBuilder,
   static_cast<nsTableFrame*>(mFrame)->PaintBCBorders(*drawTarget, mVisibleRect - pt);
 }
 
-/* static */ void
-nsTableFrame::GenericTraversal(nsDisplayListBuilder* aBuilder, nsFrame* aFrame,
-                               const nsRect& aDirtyRect, 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, aDirtyRect, aLists);
+static inline bool FrameHasBorder(nsIFrame* f) {
+  if (!f->StyleVisibility()->IsVisible()) {
+    return false;
   }
 
-  for (nsIFrame* kid : aFrame->PrincipalChildList()) {
-    aFrame->BuildDisplayListForChild(aBuilder, kid, aDirtyRect, aLists);
+  if (f->StyleBorder()->HasBorder()) {
+    return true;
   }
-}
-
-static void
-PaintRowBackground(nsTableRowFrame* aRow,
-                   nsIFrame* aFrame,
-                   nsDisplayListBuilder* aBuilder,
-                   const nsDisplayListSet& aLists,
-                   const nsRect& aDirtyRect,
-                   const nsPoint& aOffset = nsPoint())
-{
-  // Compute background rect by iterating over all cell frames.
-  for (nsTableCellFrame* cell = aRow->GetFirstCell(); cell; cell = cell->GetNextCell()) {
-    if (!cell->ShouldPaintBackground(aBuilder)) {
-      continue;
-    }
 
-    auto cellRect = cell->GetRectRelativeToSelf() + cell->GetNormalPosition() + aOffset;
-    if (!aDirtyRect.Intersects(cellRect)) {
-      continue;
-    }
-    nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, cellRect,
-                                                         aLists.BorderBackground(),
-                                                         true, nullptr,
-                                                         aFrame->GetRectRelativeToSelf(),
-                                                         cell);
-  }
+  return false;
 }
 
-static void
-PaintRowGroupBackground(nsTableRowGroupFrame* aRowGroup,
-                        nsIFrame* aFrame,
-                        nsDisplayListBuilder* aBuilder,
-                        const nsDisplayListSet& aLists,
-                        const nsRect& aDirtyRect)
-{
-  for (nsTableRowFrame* row = aRowGroup->GetFirstRow(); row; row = row->GetNextRow()) {
-    if (!aDirtyRect.Intersects(nsRect(row->GetNormalPosition(), row->GetSize()))) {
-      continue;
-    }
-    PaintRowBackground(row, aFrame, aBuilder, aLists, aDirtyRect, row->GetNormalPosition());
+void nsTableFrame::CalcHasBCBorders() {
+  if (!IsBorderCollapse()) {
+    SetHasBCBorders(false);
+    return;
   }
-}
 
-static void
-PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup,
-                                nsIFrame* aFrame,
-                                nsDisplayListBuilder* aBuilder,
-                                const nsDisplayListSet& aLists,
-                                const nsRect& aDirtyRect,
-                                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 (!aDirtyRect.Intersects(nsRect(rowPos, row->GetSize()))) {
-      continue;
-    }
-    for (nsTableCellFrame* cell = row->GetFirstCell(); cell; cell = cell->GetNextCell()) {
-
-      uint32_t curColIdx = cell->ColIndex();
-      if (!aColIdx.Contains(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 (!aDirtyRect.Intersects(cellRect)) {
-          continue;
-        }
-        nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, cellRect,
-                                                             aLists.BorderBackground(),
-                                                             true, nullptr,
-                                                             aFrame->GetRectRelativeToSelf(),
-                                                             cell);
-    }
+  if (FrameHasBorder(this)) {
+    SetHasBCBorders(true);
+    return;
   }
-}
 
-/* static */ void
-nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder,
-                                      nsFrame* aFrame,
-                                      const nsRect& aDirtyRect,
-                                      const nsDisplayListSet& aLists,
-                                      DisplayGenericTablePartTraversal aTraversal)
-{
-  bool isVisible = aFrame->IsVisibleForPainting(aBuilder);
-  bool isTable = (aFrame->GetType() == nsGkAtoms::tableFrame);
-  
-  if (isVisible || !isTable) {
-    nsDisplayTableItem* currentItem = aBuilder->GetCurrentTableItem();
-    // currentItem may be null, when none of the table parts have a
-    // background or border
-    if (currentItem) {
-      currentItem->UpdateForFrameBackground(aFrame);
-    }
-  }
-  
-  if (isVisible) {
-    // XXX: 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) {
-      aLists.BorderBackground()->AppendNewToTop(
-        new (aBuilder) nsDisplayBoxShadowOuter(aBuilder, aFrame));
-    }
-  }
-
-  // Background visibility for rows, rowgroups, columns, colgroups depends on
-  // the visibility of the _cell_, not of the row/col(group).
-  // See spec at https://drafts.csswg.org/css-tables-3/#drawing-cell-backgrounds
-  if (aFrame->GetType() == nsGkAtoms::tableRowGroupFrame) {
-    nsTableRowGroupFrame* rowGroup = static_cast<nsTableRowGroupFrame*>(aFrame);
-    PaintRowGroupBackground(rowGroup, aFrame, aBuilder, aLists, aDirtyRect);
-  } else if (aFrame->GetType() == nsGkAtoms::tableRowFrame) {
-    nsTableRowFrame* row = static_cast<nsTableRowFrame*>(aFrame);
-    PaintRowBackground(row, aFrame, aBuilder, aLists, aDirtyRect);
-  } else if (aFrame->GetType() == nsGkAtoms::tableColGroupFrame) {
-    // 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 (!aDirtyRect.Intersects(nsRect(offset, rowGroup->GetSize()))) {
-          continue;
-        }
-      PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, aDirtyRect, colIdx, offset);
-      }
+  // Check col and col group has borders.
+  for (nsIFrame* f : this->GetChildList(kColGroupList)) {
+    if (FrameHasBorder(f)) {
+      SetHasBCBorders(true);
+      return;
     }
-  } else if (aFrame->GetType() == nsGkAtoms::tableColFrame) {
-    // 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 (!aDirtyRect.Intersects(nsRect(offset, rowGroup->GetSize()))) {
-        continue;
+    nsTableColGroupFrame* colGroup = static_cast<nsTableColGroupFrame*>(f);
+    for (nsTableColFrame* col = colGroup->GetFirstColumn(); col;
+         col = col->GetNextCol()) {
+      if (FrameHasBorder(col)) {
+        SetHasBCBorders(true);
+        return;
       }
-      PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, aDirtyRect, colIdx, offset);
     }
-  } else if (isVisible) {
-    nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame,
-                                                         aFrame->GetRectRelativeToSelf(),
-                                                         aLists.BorderBackground());
   }
 
-  if (isVisible) {
-    // XXX: 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) {
-      aLists.BorderBackground()->AppendNewToTop(
-        new (aBuilder) nsDisplayBoxShadowInner(aBuilder, aFrame));
+  // check row group, row and cell has borders.
+  RowGroupArray rowGroups;
+  OrderRowGroups(rowGroups);
+  for (nsTableRowGroupFrame* rowGroup : rowGroups) {
+    if (FrameHasBorder(rowGroup)) {
+      SetHasBCBorders(true);
+      return;
     }
-  }
 
-  aTraversal(aBuilder, aFrame, aDirtyRect, aLists);
+    for (nsTableRowFrame* row = rowGroup->GetFirstRow(); row;
+         row = row->GetNextRow()) {
+      if (FrameHasBorder(row)) {
+        SetHasBCBorders(true);
+        return;
+      }
 
-  if (isVisible) {
-    if (isTable) {
-      nsTableFrame* table = static_cast<nsTableFrame*>(aFrame);
-      // In the collapsed border model, overlay all collapsed borders.
-      if (table->IsBorderCollapse()) {
-        aLists.BorderBackground()->AppendNewToTop(
-          new (aBuilder) nsDisplayTableBorderCollapse(aBuilder, table));
-      } else {
-        aLists.BorderBackground()->AppendNewToTop(
-          new (aBuilder) nsDisplayBorder(aBuilder, table));
+      for (nsTableCellFrame* cell = row->GetFirstCell(); cell;
+           cell = cell->GetNextCell()) {
+        if (FrameHasBorder(cell)) {
+          SetHasBCBorders(true);
+          return;
+        }
       }
     }
   }
 
-  aFrame->DisplayOutline(aBuilder, aLists);
+  SetHasBCBorders(false);
 }
 
 // table paint code is concerned primarily with borders and bg color
 // SEC: TODO: adjust the rect for captions
 void
 nsTableFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
-                               const nsRect&           aDirtyRect,
                                const nsDisplayListSet& aLists)
 {
   DO_GLOBAL_REFLOW_COUNT_DSP_COLOR("nsTableFrame", NS_RGB(255,128,255));
 
-  DisplayGenericTablePart(aBuilder, this, aDirtyRect, aLists);
+  DisplayBorderBackgroundOutline(aBuilder, aLists);
+
+  nsDisplayTableBackgroundSet tableBGs(aBuilder, this);
+  nsDisplayListCollection lists(aBuilder);
+
+// 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* colGroup : FirstContinuation()->GetChildList(kColGroupList)) {
+    for (nsIFrame* col : colGroup->PrincipalChildList()) {
+      tableBGs.AddColumn((nsTableColFrame*)col);
+    }
+  }
+
+  for (nsIFrame* kid : PrincipalChildList()) {
+    BuildDisplayListForChild(aBuilder, kid, lists);
+  }
+
+  tableBGs.MoveTo(aLists);
+  lists.MoveTo(aLists);
+
+  if (IsVisibleForPainting(aBuilder)) {
+    // In the collapsed border model, overlay all collapsed borders.
+    if (IsBorderCollapse()) {
+      if (HasBCBorders()) {
+        aLists.BorderBackground()->AppendNewToTop(
+          new (aBuilder) nsDisplayTableBorderCollapse
+            (aBuilder, this));
+      }
+    } else {
+      const nsStyleBorder* borderStyle = StyleBorder();
+      if (borderStyle->HasBorder()) {
+        aLists.BorderBackground()->AppendNewToTop(
+          new (aBuilder) nsDisplayBorder
+            (aBuilder, this));
+      }
+    }
+  }
 }
 
 nsMargin
@@ -4101,6 +3986,7 @@ nsTableFrame::AddBCDamageArea(const TableArea& aValue)
 #endif
 
   SetNeedToCalcBCBorders(true);
+  SetNeedToCalcHasBCBorders(true);
   // Get the property
   BCPropertyData* value = GetOrCreateBCProperty();
   if (value) {
@@ -4141,6 +4027,7 @@ nsTableFrame::SetFullBCDamageArea()
   NS_ASSERTION(IsBorderCollapse(), "invalid SetFullBCDamageArea call");
 
   SetNeedToCalcBCBorders(true);
+  SetNeedToCalcHasBCBorders(true);
 
   BCPropertyData* value = GetOrCreateBCProperty();
   if (value) {
diff --git a/layout/tables/nsTableFrame.h b/layout/tables/nsTableFrame.h
index a6b786402..7fcdcb9d8 100644
--- a/layout/tables/nsTableFrame.h
+++ b/layout/tables/nsTableFrame.h
@@ -72,36 +72,59 @@ private:
   bool mDrawsBackground;
 };
 
-class nsAutoPushCurrentTableItem
-{
-public:
-  nsAutoPushCurrentTableItem() : mBuilder(nullptr) {}
+class nsDisplayTableBackgroundSet {
+ public:
+  nsDisplayList* ColGroupBackgrounds() { return &mColGroupBackgrounds; }
+
+  nsDisplayList* ColBackgrounds() { return &mColBackgrounds; }
+
+  nsDisplayTableBackgroundSet(nsDisplayListBuilder* aBuilder, nsIFrame* aTable)
+      : mBuilder(aBuilder) {
+    mPrevTableBackgroundSet = mBuilder->SetTableBackgroundSet(this);
+    mozilla::DebugOnly<const nsIFrame*> reference =
+        mBuilder->FindReferenceFrameFor(aTable, &mToReferenceFrame);
+    MOZ_ASSERT(nsLayoutUtils::IsAncestorFrameCrossDoc(reference, aTable));
+    mDirtyRect = mBuilder->GetDirtyRect();
+  }
 
-  void Push(nsDisplayListBuilder* aBuilder, nsDisplayTableItem* aPushItem)
-  {
-    mBuilder = aBuilder;
-    mOldCurrentItem = aBuilder->GetCurrentTableItem();
-    aBuilder->SetCurrentTableItem(aPushItem);
-#ifdef DEBUG
-    mPushedItem = aPushItem;
-#endif
+  ~nsDisplayTableBackgroundSet() {
+    mozilla::DebugOnly<nsDisplayTableBackgroundSet*> result =
+        mBuilder->SetTableBackgroundSet(mPrevTableBackgroundSet);
+    MOZ_ASSERT(result == this);
   }
-  ~nsAutoPushCurrentTableItem() {
-    if (!mBuilder)
-      return;
-#ifdef DEBUG
-    NS_ASSERTION(mBuilder->GetCurrentTableItem() == mPushedItem,
-                 "Someone messed with the current table item behind our back!");
-#endif
-    mBuilder->SetCurrentTableItem(mOldCurrentItem);
+
+  /**
+   * Move all display items in our lists to top of the corresponding lists in
+   * the destination.
+   */
+  void MoveTo(const nsDisplayListSet& aDestination) {
+    aDestination.BorderBackground()->AppendToTop(ColGroupBackgrounds());
+    aDestination.BorderBackground()->AppendToTop(ColBackgrounds());
   }
 
-private:
+  void AddColumn(nsTableColFrame* aFrame) { mColumns.AppendElement(aFrame); }
+
+  nsTableColFrame* GetColForIndex(int32_t aIndex) { return mColumns[aIndex]; }
+
+  const nsPoint& TableToReferenceFrame() { return mToReferenceFrame; }
+
+  const nsRect& GetDirtyRect() { return mDirtyRect; }
+
+ private:
+  // This class is only used on stack, so we don't have to worry about leaking
+  // it.  Don't let us be heap-allocated!
+  void* operator new(size_t sz) CPP_THROW_NEW;
+
+ protected:
   nsDisplayListBuilder* mBuilder;
-  nsDisplayTableItem*   mOldCurrentItem;
-#ifdef DEBUG
-  nsDisplayTableItem*   mPushedItem;
-#endif
+  nsDisplayTableBackgroundSet* mPrevTableBackgroundSet;
+
+  nsDisplayList mColGroupBackgrounds;
+  nsDisplayList mColBackgrounds;
+
+  nsTArray<nsTableColFrame*> mColumns;
+  nsPoint mToReferenceFrame;
+  nsRect mDirtyRect;
 };
 
 /* ============================================================================ */
@@ -229,29 +252,6 @@ public:
                                                    nsIFrame* aSourceFrame,
                                                    bool* aDidPassThrough);
 
-  typedef void (* DisplayGenericTablePartTraversal)
-      (nsDisplayListBuilder* aBuilder, nsFrame* aFrame,
-       const nsRect& aDirtyRect, const nsDisplayListSet& aLists);
-  static void GenericTraversal(nsDisplayListBuilder* aBuilder, nsFrame* aFrame,
-                               const nsRect& aDirtyRect, 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 nsRect& aDirtyRect,
-                                      const nsDisplayListSet& aLists,
-                                      DisplayGenericTablePartTraversal aTraversal = GenericTraversal);
-
   // Return the closest sibling of aPriorChildFrame (including aPriroChildFrame)
   // of type aChildType.
   static nsIFrame* GetFrameAtOrBefore(nsIFrame*       aParentFrame,
@@ -268,7 +268,6 @@ public:
   virtual void GetChildLists(nsTArray<ChildList>* aLists) const override;
 
   virtual void BuildDisplayList(nsDisplayListBuilder*   aBuilder,
-                                const nsRect&           aDirtyRect,
                                 const nsDisplayListSet& aLists) override;
 
   /** Get the outer half (i.e., the part outside the height and width of
@@ -761,6 +760,13 @@ public:
   bool NeedToCollapse() const;
   void SetNeedToCollapse(bool aValue);
 
+  bool NeedToCalcHasBCBorders() const;
+  void SetNeedToCalcHasBCBorders(bool aValue);
+
+  void CalcHasBCBorders();
+  bool HasBCBorders();
+  void SetHasBCBorders(bool aValue);
+
   /** The GeometryDirty bit is similar to the NS_FRAME_IS_DIRTY frame
     * state bit, which implies that all descendants are dirty.  The
     * GeometryDirty still implies that all the parts of the table are
@@ -870,6 +876,8 @@ protected:
     uint32_t mIStartContBCBorder:8;
     uint32_t mNeedToCollapse:1;        // rows, cols that have visibility:collapse need to be collapsed
     uint32_t mResizedColumns:1;        // have we resized columns since last reflow?
+    uint32_t mNeedToCalcHasBCBorders:1;
+    uint32_t mHasBCBorders:1;
   } mBits;
 
   nsTableCellMap*         mCellMap;            // maintains the relationships between rows, cols, and cells
@@ -965,6 +973,30 @@ inline void nsTableFrame::SetNeedToCalcBCBorders(bool aValue)
   mBits.mNeedToCalcBCBorders = (unsigned)aValue;
 }
 
+inline bool nsTableFrame::NeedToCalcHasBCBorders() const
+{
+  return (bool)mBits.mNeedToCalcHasBCBorders;
+}
+
+inline void nsTableFrame::SetNeedToCalcHasBCBorders(bool aValue)
+{
+  mBits.mNeedToCalcHasBCBorders = (unsigned)aValue;
+}
+
+inline bool nsTableFrame::HasBCBorders()
+{
+  if (NeedToCalcHasBCBorders()) {
+    CalcHasBCBorders();
+    SetNeedToCalcHasBCBorders(false);
+  }
+  return (bool)mBits.mHasBCBorders;
+}
+
+inline void nsTableFrame::SetHasBCBorders(bool aValue)
+{
+  mBits.mHasBCBorders = (unsigned)aValue;
+}
+
 inline nscoord
 nsTableFrame::GetContinuousIStartBCBorderWidth() const
 {
diff --git a/layout/tables/nsTableRowFrame.cpp b/layout/tables/nsTableRowFrame.cpp
index 02b85a141..18f11f876 100644
--- a/layout/tables/nsTableRowFrame.cpp
+++ b/layout/tables/nsTableRowFrame.cpp
@@ -549,12 +549,66 @@ nsTableRowFrame::CalcBSize(const ReflowInput& aReflowInput)
   return GetInitialBSize();
 }
 
+void nsTableRowFrame::PaintCellBackgroundsForFrame(
+    nsIFrame* aFrame, nsDisplayListBuilder* aBuilder,
+    const nsDisplayListSet& aLists, const nsPoint& aOffset) {
+  // Compute background rect by iterating all cell frame.
+  const nsPoint toReferenceFrame = aBuilder->ToReferenceFrame(aFrame);
+  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;
+    }
+    cellRect += toReferenceFrame;
+    nsDisplayBackgroundImage::AppendBackgroundItemsToTop(
+        aBuilder, aFrame, cellRect, aLists.BorderBackground(), true, nullptr,
+        aFrame->GetRectRelativeToSelf() + toReferenceFrame, cell);
+  }
+}
+
 void
 nsTableRowFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
-                                  const nsRect&           aDirtyRect,
                                   const nsDisplayListSet& aLists)
 {
-  nsTableFrame::DisplayGenericTablePart(aBuilder, this, aDirtyRect, aLists);
+  if (IsVisibleForPainting(aBuilder)) {
+    // 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(
+        new (aBuilder) nsDisplayBoxShadowOuter
+          (aBuilder, this));
+    }
+  }
+
+  PaintCellBackgroundsForFrame(this, aBuilder, aLists);
+
+  if (IsVisibleForPainting(aBuilder)) {
+    // 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(
+        new (aBuilder) nsDisplayBoxShadowInner
+          (aBuilder, this));
+    }
+  }
+
+  DisplayOutline(aBuilder, aLists);
+
+  for (nsIFrame* kid : PrincipalChildList()) {
+    BuildDisplayListForChild(aBuilder, kid, aLists);
+  }
 }
 
 nsIFrame::LogicalSides
diff --git a/layout/tables/nsTableRowFrame.h b/layout/tables/nsTableRowFrame.h
index c53c81ff1..4b6dd2628 100644
--- a/layout/tables/nsTableRowFrame.h
+++ b/layout/tables/nsTableRowFrame.h
@@ -79,9 +79,13 @@ public:
   virtual nsMargin GetUsedPadding() const override;
 
   virtual void BuildDisplayList(nsDisplayListBuilder*   aBuilder,
-                                const nsRect&           aDirtyRect,
                                 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;
diff --git a/layout/tables/nsTableRowGroupFrame.cpp b/layout/tables/nsTableRowGroupFrame.cpp
index 37f577f5c..56ca394de 100644
--- a/layout/tables/nsTableRowGroupFrame.cpp
+++ b/layout/tables/nsTableRowGroupFrame.cpp
@@ -156,7 +156,7 @@ nsTableRowGroupFrame::InitRepeatedFrame(nsTableRowGroupFrame* aHeaderFooterFrame
 // Handle the child-traversal part of DisplayGenericTablePart
 static void
 DisplayRows(nsDisplayListBuilder* aBuilder, nsFrame* aFrame,
-            const nsRect& aDirtyRect, const nsDisplayListSet& aLists)
+            const nsDisplayListSet& aLists)
 {
   nscoord overflowAbove;
   nsTableRowGroupFrame* f = static_cast<nsTableRowGroupFrame*>(aFrame);
@@ -168,16 +168,16 @@ DisplayRows(nsDisplayListBuilder* aBuilder, nsFrame* aFrame,
   // the rows in |f|, but that's exactly what we're trying to avoid, so we
   // approximate it by checking it for |f|: if it's true for any row
   // in |f| then it's true for |f| itself.
-  nsIFrame* kid = aBuilder->ShouldDescendIntoFrame(f) ?
-    nullptr : f->GetFirstRowContaining(aDirtyRect.y, &overflowAbove);
+  nsIFrame* kid = aBuilder->ShouldDescendIntoFrame(f, true) ?
+    nullptr : f->GetFirstRowContaining(aBuilder->GetVisibleRect().y, &overflowAbove);
 
   if (kid) {
     // If we have a cursor, use it
     while (kid) {
-      if (kid->GetRect().y - overflowAbove >= aDirtyRect.YMost()) {
+      if (kid->GetRect().y - overflowAbove >= aBuilder->GetDirtyRect().YMost()) {
         break;
       }
-      f->BuildDisplayListForChild(aBuilder, kid, aDirtyRect, aLists);
+      f->BuildDisplayListForChild(aBuilder, kid, aLists);
       kid = kid->GetNextSibling();
     }
     return;
@@ -187,7 +187,7 @@ DisplayRows(nsDisplayListBuilder* aBuilder, nsFrame* aFrame,
   nsTableRowGroupFrame::FrameCursorData* cursor = f->SetupRowCursor();
   kid = f->PrincipalChildList().FirstChild();
   while (kid) {
-    f->BuildDisplayListForChild(aBuilder, kid, aDirtyRect, aLists);
+    f->BuildDisplayListForChild(aBuilder, kid, aLists);
 
     if (cursor) {
       if (!cursor->AppendFrame(kid)) {
@@ -205,11 +205,45 @@ DisplayRows(nsDisplayListBuilder* aBuilder, nsFrame* aFrame,
 
 void
 nsTableRowGroupFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
-                                       const nsRect&           aDirtyRect,
                                        const nsDisplayListSet& aLists)
 {
-  nsTableFrame::DisplayGenericTablePart(aBuilder, this, aDirtyRect,
-                                        aLists, DisplayRows);
+  if (IsVisibleForPainting(aBuilder)) {
+    // 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(
+        new (aBuilder) nsDisplayBoxShadowOuter
+          (aBuilder, this));
+    }
+  }
+
+  for (nsTableRowFrame* row = GetFirstRow(); row; row = row->GetNextRow()) {
+    if (!aBuilder->GetDirtyRect().Intersects(row->GetVisualOverflowRect() + row->GetNormalPosition())) {
+      continue;
+    }
+    row->PaintCellBackgroundsForFrame(this, aBuilder, aLists,
+                                      row->GetNormalPosition());
+  }
+
+  if (IsVisibleForPainting(aBuilder)) {
+    // 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(
+        new (aBuilder) nsDisplayBoxShadowInner
+          (aBuilder, this));
+    }
+  }
+
+  DisplayOutline(aBuilder, aLists);
+
+  DisplayRows(aBuilder, this, aLists);
 }
 
 nsIFrame::LogicalSides
diff --git a/layout/tables/nsTableRowGroupFrame.h b/layout/tables/nsTableRowGroupFrame.h
index 721d91046..eb9f6205e 100644
--- a/layout/tables/nsTableRowGroupFrame.h
+++ b/layout/tables/nsTableRowGroupFrame.h
@@ -75,7 +75,6 @@ public:
   virtual nsMargin GetUsedPadding() const override;
 
   virtual void BuildDisplayList(nsDisplayListBuilder*   aBuilder,
-                                const nsRect&           aDirtyRect,
                                 const nsDisplayListSet& aLists) override;
 
    /** calls Reflow for all of its child rows.
diff --git a/layout/tables/nsTableWrapperFrame.cpp b/layout/tables/nsTableWrapperFrame.cpp
index da71375d5..476024e96 100644
--- a/layout/tables/nsTableWrapperFrame.cpp
+++ b/layout/tables/nsTableWrapperFrame.cpp
@@ -168,7 +168,6 @@ nsTableWrapperFrame::RemoveFrame(ChildListID  aListID,
 
 void
 nsTableWrapperFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
-                                      const nsRect&           aDirtyRect,
                                       const nsDisplayListSet& aLists)
 {
   // No border, background or outline are painted because they all belong
@@ -177,16 +176,15 @@ nsTableWrapperFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
   // If there's no caption, take a short cut to avoid having to create
   // the special display list set and then sort it.
   if (mCaptionFrames.IsEmpty()) {
-    BuildDisplayListForInnerTable(aBuilder, aDirtyRect, aLists);
+    BuildDisplayListForInnerTable(aBuilder, aLists);
     return;
   }
 
-  nsDisplayListCollection set;
-  BuildDisplayListForInnerTable(aBuilder, aDirtyRect, set);
+  nsDisplayListCollection set(aBuilder);
+  BuildDisplayListForInnerTable(aBuilder, set);
   
   nsDisplayListSet captionSet(set, set.BlockBorderBackgrounds());
-  BuildDisplayListForChild(aBuilder, mCaptionFrames.FirstChild(),
-                           aDirtyRect, captionSet);
+  BuildDisplayListForChild(aBuilder, mCaptionFrames.FirstChild(), captionSet);
 
   // Now we have to sort everything by content order, since the caption
   // may be somewhere inside the table
@@ -200,7 +198,6 @@ nsTableWrapperFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
 
 void
 nsTableWrapperFrame::BuildDisplayListForInnerTable(nsDisplayListBuilder*   aBuilder,
-                                                   const nsRect&           aDirtyRect,
                                                    const nsDisplayListSet& aLists)
 {
   // Just paint the regular children, but the children's background is our
@@ -208,7 +205,7 @@ nsTableWrapperFrame::BuildDisplayListForInnerTable(nsDisplayListBuilder*   aBuil
   nsIFrame* kid = mFrames.FirstChild();
   // The children should be in content order
   while (kid) {
-    BuildDisplayListForChild(aBuilder, kid, aDirtyRect, aLists);
+    BuildDisplayListForChild(aBuilder, kid, aLists);
     kid = kid->GetNextSibling();
   }
 }
diff --git a/layout/tables/nsTableWrapperFrame.h b/layout/tables/nsTableWrapperFrame.h
index 45d7c33e4..8095c7fc4 100644
--- a/layout/tables/nsTableWrapperFrame.h
+++ b/layout/tables/nsTableWrapperFrame.h
@@ -59,11 +59,9 @@ public:
 #endif
 
   virtual void BuildDisplayList(nsDisplayListBuilder*   aBuilder,
-                                const nsRect&           aDirtyRect,
                                 const nsDisplayListSet& aLists) override;
 
   void BuildDisplayListForInnerTable(nsDisplayListBuilder*   aBuilder,
-                                     const nsRect&           aDirtyRect,
                                      const nsDisplayListSet& aLists);
 
   virtual nscoord GetLogicalBaseline(mozilla::WritingMode aWritingMode) const override;