Bug 1409114 - Part 1: Remove current table item, as it's never set. r=miko
authorMatt Woodrow <mwoodrow@mozilla.com>
Mon, 20 May 2019 23:14:40 +0000
changeset 474627 a8bd87bfc775789e75c9c8721ffb681121c4f100
parent 474626 0e7d8f96bf123f5d0f491fe7780223bde509e841
child 474628 c0aa0d405e3c38c2b63b57eac0843ec7e3a0f0b3
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)
reviewersmiko
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 1: Remove current table item, as it's never set. r=miko Differential Revision: https://phabricator.services.mozilla.com/D29272
layout/painting/nsDisplayList.cpp
layout/painting/nsDisplayList.h
layout/tables/nsTableCellFrame.cpp
layout/tables/nsTableFrame.cpp
layout/tables/nsTableFrame.h
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -1131,17 +1131,16 @@ nsRect nsDisplayListBuilder::OutOfFlowDi
 }
 
 nsDisplayListBuilder::nsDisplayListBuilder(nsIFrame* aReferenceFrame,
                                            nsDisplayListBuilderMode aMode,
                                            bool aBuildCaret,
                                            bool aRetainingDisplayList)
     : mReferenceFrame(aReferenceFrame),
       mIgnoreScrollFrame(nullptr),
-      mCurrentTableItem(nullptr),
       mCurrentActiveScrolledRoot(nullptr),
       mCurrentContainerASR(nullptr),
       mCurrentFrame(aReferenceFrame),
       mCurrentReferenceFrame(aReferenceFrame),
       mNeedsDisplayListBuild{},
       mRootAGR(AnimatedGeometryRoot::CreateAGRForFrame(
           aReferenceFrame, nullptr, true, aRetainingDisplayList)),
       mCurrentAGR(mRootAGR),
@@ -1393,17 +1392,16 @@ static void UnmarkFrameForDisplayIfVisib
 
 nsDisplayListBuilder::~nsDisplayListBuilder() {
   NS_ASSERTION(mFramesMarkedForDisplay.Length() == 0,
                "All frames should have been unmarked");
   NS_ASSERTION(mFramesWithOOFData.Length() == 0,
                "All OOF data should have been removed");
   NS_ASSERTION(mPresShellStates.Length() == 0,
                "All presshells should have been exited");
-  NS_ASSERTION(!mCurrentTableItem, "No table item should be active");
 
   DisplayItemClipChain* c = mFirstClipChainToDestroy;
   while (c) {
     DisplayItemClipChain* next = c->mNextClipChainToDestroy;
     c->DisplayItemClipChain::~DisplayItemClipChain();
     c = next;
   }
 
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@@ -1482,23 +1482,16 @@ class nsDisplayListBuilder {
   /**
    * The level is increased by one for items establishing 3D rendering
    * context and starting a new accumulation.
    */
   int GetAccumulatedRectLevels() {
     return mPreserves3DCtx.mAccumulatedRectLevels;
   }
 
-  // Helpers for tables
-  nsDisplayTableItem* GetCurrentTableItem() { return mCurrentTableItem; }
-
-  void SetCurrentTableItem(nsDisplayTableItem* aTableItem) {
-    mCurrentTableItem = aTableItem;
-  }
-
   struct OutOfFlowDisplayData {
     OutOfFlowDisplayData(
         const DisplayItemClipChain* aContainingBlockClipChain,
         const DisplayItemClipChain* aCombinedClipChain,
         const ActiveScrolledRoot* aContainingBlockActiveScrolledRoot,
         const nsRect& aVisibleRect, const nsRect& aDirtyRect)
         : mContainingBlockClipChain(aContainingBlockClipChain),
           mCombinedClipChain(aCombinedClipChain),
@@ -1861,17 +1854,16 @@ class nsDisplayListBuilder {
   nsPresArena<32768> mPool;
 
   AutoTArray<PresShellState, 8> mPresShellStates;
   AutoTArray<nsIFrame*, 400> mFramesMarkedForDisplay;
   AutoTArray<nsIFrame*, 40> mFramesMarkedForDisplayIfVisible;
   AutoTArray<nsIFrame*, 20> mFramesWithOOFData;
   nsClassHashtable<nsPtrHashKey<nsDisplayItem>, nsTArray<ThemeGeometry>>
       mThemeGeometries;
-  nsDisplayTableItem* mCurrentTableItem;
   DisplayListClipState mClipState;
   const ActiveScrolledRoot* mCurrentActiveScrolledRoot;
   const ActiveScrolledRoot* mCurrentContainerASR;
   // mCurrentFrame is the frame that we're currently calling (or about to call)
   // BuildDisplayList on.
   const nsIFrame* mCurrentFrame;
   // The reference frame for mCurrentFrame.
   const nsIFrame* mCurrentReferenceFrame;
--- a/layout/tables/nsTableCellFrame.cpp
+++ b/layout/tables/nsTableCellFrame.cpp
@@ -472,21 +472,16 @@ void nsTableCellFrame::BuildDisplayList(
       aLists.BorderBackground()->AppendNewToTop<nsDisplayTableCellSelection>(
           aBuilder, this);
     }
   }
 
   // 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.
   // This isn't a problem since it won't have a real background except for
   // event handling. We do not call BuildDisplayListForNonBlockChildren
   // 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
--- a/layout/tables/nsTableFrame.cpp
+++ b/layout/tables/nsTableFrame.cpp
@@ -1387,28 +1387,16 @@ void nsTableFrame::CalcHasBCBorders() {
 /* static */
 void nsTableFrame::DisplayGenericTablePart(
     nsDisplayListBuilder* aBuilder, nsFrame* aFrame,
     const nsDisplayListSet& aLists,
     DisplayGenericTablePartTraversal aTraversal) {
   bool isVisible = aFrame->IsVisibleForPainting();
   bool isTable = aFrame->IsTableFrame();
 
-  // Note that we UpdateForFrameBackground() even if we're not visible, unless
-  // we're a table frame, because our backgrounds may paint anyway if the _cell_
-  // is visible.
-  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) {
     // 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>(
--- a/layout/tables/nsTableFrame.h
+++ b/layout/tables/nsTableFrame.h
@@ -65,45 +65,16 @@ class nsDisplayTableItem : public nsPain
 
   void UpdateForFrameBackground(nsIFrame* aFrame);
 
  private:
   bool mPartHasFixedBackground;
   bool mDrawsBackground;
 };
 
-class nsAutoPushCurrentTableItem {
- public:
-  nsAutoPushCurrentTableItem() : mBuilder(nullptr), mOldCurrentItem(nullptr) {}
-
-  void Push(nsDisplayListBuilder* aBuilder, nsDisplayTableItem* aPushItem) {
-    mBuilder = aBuilder;
-    mOldCurrentItem = aBuilder->GetCurrentTableItem();
-    aBuilder->SetCurrentTableItem(aPushItem);
-#ifdef DEBUG
-    mPushedItem = aPushItem;
-#endif
-  }
-  ~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);
-  }
-
- private:
-  nsDisplayListBuilder* mBuilder;
-  nsDisplayTableItem* mOldCurrentItem;
-#ifdef DEBUG
-  nsDisplayTableItem* mPushedItem;
-#endif
-};
-
 /* ========================================================================== */
 
 enum nsTableColType {
   eColContent = 0,            // there is real col content associated
   eColAnonymousCol = 1,       // the result of a span on a col
   eColAnonymousColGroup = 2,  // the result of a span on a col group
   eColAnonymousCell = 3       // the result of a cell alone
 };