Bug 1474771 - Propagate NS_FRAME_IS_DIRTY to descendants when marking as dirty rather than during reflow. r=dholbert

This simplifies dealing with frames that are pushed/pulled between
continuations during reflow, allows us to avoid the complexity of the
fix to 1459937, and hopefully fixes some of the regressions from bug
1308876.

This disables the changes from bug 1459937 by commenting out a single
line in ReparentFrameInternal in nsBlockFrame.cpp, but all the added
code will be removed in the following patch.

Co-authored-by: Gerald Squelart <gsquelart@mozilla.com>
Co-authored-by: L. David Baron <dbaron@dbaron.org>

Depends on D36423

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

--HG--
extra : moz-landing-system : lando

layout/base/PresShell.cpp

@@ -2678,7 +2678,9 @@ void PresShell::FrameNeedsReflow(nsIFrame* aFrame,
       }
     }
 
-    if (aIntrinsicDirty == IntrinsicDirty::StyleChange) {
+    const bool styleChange = (aIntrinsicDirty == IntrinsicDirty::StyleChange);
+    const bool dirty = (aBitToAdd == NS_FRAME_IS_DIRTY);
+    if (styleChange || dirty) {
       // Mark all descendants dirty (using an nsTArray stack rather than
       // recursion).
       // Note that ReflowInput::InitResizeFlags has some similar
@@ -2689,18 +2691,25 @@ void PresShell::FrameNeedsReflow(nsIFrame* aFrame,
       do {
         nsIFrame* f = stack.PopLastElement();
 
-        if (f->IsPlaceholderFrame()) {
-          nsIFrame* oof = nsPlaceholderFrame::GetRealFrameForPlaceholder(f);
-          if (!nsLayoutUtils::IsProperAncestorFrame(subtreeRoot, oof)) {
-            // We have another distinct subtree we need to mark.
-            subtrees.AppendElement(oof);
+        if (styleChange) {
+          if (f->IsPlaceholderFrame()) {
+            nsIFrame* oof = nsPlaceholderFrame::GetRealFrameForPlaceholder(f);
+            if (!nsLayoutUtils::IsProperAncestorFrame(subtreeRoot, oof)) {
+              // We have another distinct subtree we need to mark.
+              subtrees.AppendElement(oof);
+            }
           }
         }
 
         nsIFrame::ChildListIterator lists(f);
         for (; !lists.IsDone(); lists.Next()) {
           for (nsIFrame* kid : lists.CurrentList()) {
-            kid->MarkIntrinsicISizesDirty();
+            if (styleChange) {
+              kid->MarkIntrinsicISizesDirty();
+            }
+            if (dirty) {
+              kid->AddStateBits(NS_FRAME_IS_DIRTY);
+            }
             stack.AppendElement(kid);
           }
         }

layout/forms/nsFileControlFrame.cpp

@@ -173,8 +173,8 @@ void nsFileControlFrame::Reflow(nsPresContext* aPresContext,
                           availableISizeForLabel - labelBP, filename)) {
         nsBlockFrame::DidReflow(aPresContext, &aReflowInput);
         aStatus.Reset();
-        labelFrame->AddStateBits(NS_FRAME_IS_DIRTY |
-                                 NS_BLOCK_NEEDS_BIDI_RESOLUTION);
+        labelFrame->MarkSubtreeDirty();
+        labelFrame->AddStateBits(NS_BLOCK_NEEDS_BIDI_RESOLUTION);
         mCachedMinISize = NS_INTRINSIC_ISIZE_UNKNOWN;
         mCachedPrefISize = NS_INTRINSIC_ISIZE_UNKNOWN;
         done = true;

layout/generic/ReflowInput.cpp

@@ -310,36 +310,9 @@ void ReflowInput::SetComputedHeight(nscoord aComputedHeight) {
   }
 }
 
-/* static */
-void ReflowInput::MarkFrameChildrenDirty(nsIFrame* aFrame) {
-  if (aFrame->IsXULBoxFrame()) {
-    return;
-  }
-  // Mark all child frames as dirty.
-  //
-  // We don't do this for XUL boxes because they handle their child
-  // reflow separately.
-  for (nsIFrame::ChildListIterator childLists(aFrame); !childLists.IsDone();
-       childLists.Next()) {
-    for (nsIFrame* childFrame : childLists.CurrentList()) {
-      if (!childFrame->IsTableColGroupFrame()) {
-        childFrame->AddStateBits(NS_FRAME_IS_DIRTY);
-      }
-    }
-  }
-}
-
 void ReflowInput::Init(nsPresContext* aPresContext,
                        const Maybe<LogicalSize>& aContainingBlockSize,
                        const nsMargin* aBorder, const nsMargin* aPadding) {
-  if ((mFrame->GetStateBits() & NS_FRAME_IS_DIRTY)) {
-    // FIXME (bug 1376530): It would be better for memory locality if we
-    // did this as we went.  However, we need to be careful not to do
-    // this twice for any particular child if we reflow it twice.  The
-    // easiest way to accomplish that is to do it at the start.
-    MarkFrameChildrenDirty(mFrame);
-  }
-
   if (AvailableISize() == NS_UNCONSTRAINEDSIZE) {
     // Look up the parent chain for an orthogonal inline limit,
     // and reset AvailableISize() if found.
@@ -586,12 +559,10 @@ void ReflowInput::InitResizeFlags(nsPresContext* aPresContext,
         mFrame->AddStateBits(NS_FRAME_HAS_DIRTY_CHILDREN);
         nsIFrame* kid = mFrame->PrincipalChildList().FirstChild();
         if (kid) {
-          kid->AddStateBits(NS_FRAME_IS_DIRTY);
-          MarkFrameChildrenDirty(kid);
+          kid->MarkSubtreeDirty();
         }
       } else {
-        mFrame->AddStateBits(NS_FRAME_IS_DIRTY);
-        MarkFrameChildrenDirty(mFrame);
+        mFrame->MarkSubtreeDirty();
       }
 
       // Mark intrinsic widths on all descendants dirty.  We need to do

layout/generic/ReflowInput.h

@@ -1040,12 +1040,6 @@ struct ReflowInput : public SizeComputationInput {
                                     nscoord* aOutsideBoxSizing) const;
 
   void CalculateBlockSideMargins(LayoutFrameType aFrameType);
-
-  /**
-   * Make all descendants of this frame dirty.
-   * Exceptions: XULBoxFrame and TabeColGroupFrame children.
-   */
-  static void MarkFrameChildrenDirty(nsIFrame* aFrame);
 };
 
 }  // namespace mozilla

layout/generic/nsAbsoluteContainingBlock.cpp

@@ -207,7 +207,7 @@ void nsAbsoluteContainingBlock::Reflow(nsContainerFrame* aDelegatingFrame,
     // the case enough of an edge case, that this is probably better.
     if (kidNeedsReflow && aPresContext->CheckForInterrupt(aDelegatingFrame)) {
       if (aDelegatingFrame->GetStateBits() & NS_FRAME_IS_DIRTY) {
-        kidFrame->AddStateBits(NS_FRAME_IS_DIRTY);
+        kidFrame->MarkSubtreeDirty();
       } else {
         kidFrame->AddStateBits(NS_FRAME_HAS_DIRTY_CHILDREN);
       }
@@ -351,7 +351,7 @@ void nsAbsoluteContainingBlock::MarkAllFramesDirty() {
 void nsAbsoluteContainingBlock::DoMarkFramesDirty(bool aMarkAllDirty) {
   for (nsIFrame* kidFrame : mAbsoluteFrames) {
     if (aMarkAllDirty) {
-      kidFrame->AddStateBits(NS_FRAME_IS_DIRTY);
+      kidFrame->MarkSubtreeDirty();
     } else if (FrameDependsOnContainer(kidFrame, true, true)) {
       // Add the weakest flags that will make sure we reflow this frame later
       kidFrame->AddStateBits(NS_FRAME_HAS_DIRTY_CHILDREN);

layout/generic/nsBlockFrame.cpp

@@ -589,7 +589,7 @@ static void ReparentFrameInternal(nsIFrame* aFrame,
 
   aFrame->SetParent(aNewParent);
   if (aMarkDirty) {
-    aFrame->AddStateBits(NS_FRAME_IS_DIRTY);
+    // aFrame->AddStateBits(NS_FRAME_IS_DIRTY);
   }
 
   // When pushing and pulling frames we need to check for whether any
@@ -2991,12 +2991,12 @@ void nsBlockFrame::MarkLineDirtyForInterrupt(nsLineBox* aLine) {
     int32_t n = aLine->GetChildCount();
     for (nsIFrame* f = aLine->mFirstChild; n > 0;
          f = f->GetNextSibling(), --n) {
-      f->AddStateBits(NS_FRAME_IS_DIRTY);
+      f->MarkSubtreeDirty();
     }
     // And mark all the floats whose reflows we might be skipping dirty too.
     if (aLine->HasFloats()) {
       for (nsFloatCache* fc = aLine->GetFirstFloat(); fc; fc = fc->Next()) {
-        fc->mFloat->AddStateBits(NS_FRAME_IS_DIRTY);
+        fc->mFloat->MarkSubtreeDirty();
       }
     }
   } else {

layout/generic/nsColumnSetFrame.cpp

@@ -437,7 +437,7 @@ nsColumnSetFrame::ReflowConfig nsColumnSetFrame::ChooseColumnStrategy(
 
 static void MarkPrincipalChildrenDirty(nsIFrame* aFrame) {
   for (nsIFrame* childFrame : aFrame->PrincipalChildList()) {
-    childFrame->AddStateBits(NS_FRAME_IS_DIRTY);
+    childFrame->MarkSubtreeDirty();
   }
 }
 
@@ -713,7 +713,9 @@ nsColumnSetFrame::ColumnBalanceData nsColumnSetFrame::ReflowChildren(
               ? aReflowInput.mCBReflowInput->ComputedSize(wm)
               : aReflowInput.ComputedSize(wm);
 
-      if (reflowNext) child->AddStateBits(NS_FRAME_IS_DIRTY);
+      if (reflowNext) {
+        child->MarkSubtreeDirty();
+      }
 
       LogicalSize kidCBSize(wm, availSize.ISize(wm), computedSize.BSize(wm));
       ReflowInput kidReflowInput(PresContext(), aReflowInput, child, availSize,
@@ -855,7 +857,7 @@ nsColumnSetFrame::ColumnBalanceData nsColumnSetFrame::ReflowChildren(
       if (columnCount >= aConfig.mBalanceColCount - 1) {
         // No more columns allowed here. Stop.
         aStatus.SetNextInFlowNeedsReflow();
-        kidNextInFlow->AddStateBits(NS_FRAME_IS_DIRTY);
+        kidNextInFlow->MarkSubtreeDirty();
         // Move any of our leftover columns to our overflow list. Our
         // next-in-flow will eventually pick them up.
         const nsFrameList& continuationColumns =
@@ -901,7 +903,7 @@ nsColumnSetFrame::ColumnBalanceData nsColumnSetFrame::ReflowChildren(
     // Otherwise, when we go to mark dirty bits on |child|'s ancestors we'll
     // bail out immediately, since it'll already have a dirty bit.
     for (; child; child = child->GetNextSibling()) {
-      child->AddStateBits(NS_FRAME_IS_DIRTY);
+      child->MarkSubtreeDirty();
     }
   }
 

layout/generic/nsContainerFrame.cpp

@@ -1878,8 +1878,9 @@ nsresult nsOverflowContinuationTracker::Insert(nsIFrame* aOverflowCont,
   }
 
   // If we need to reflow it, mark it dirty
-  if (aReflowStatus.NextInFlowNeedsReflow())
-    aOverflowCont->AddStateBits(NS_FRAME_IS_DIRTY);
+  if (aReflowStatus.NextInFlowNeedsReflow()) {
+    aOverflowCont->MarkSubtreeDirty();
+  }
 
   // It's in our list, just step forward
   StepForward();

layout/generic/nsFrame.cpp

@@ -5360,6 +5360,40 @@ void nsFrame::MarkIntrinsicISizesDirty() {
   }
 }
 
+void nsIFrame::MarkSubtreeDirty() {
+  if (HasAnyStateBits(NS_FRAME_IS_DIRTY)) {
+    return;
+  }
+  // Unconditionally mark given frame dirty.
+  AddStateBits(NS_FRAME_IS_DIRTY);
+
+  // Mark all descendants dirty, unless:
+  // - Already dirty.
+  // - TableColGroup
+  // - XULBox
+  AutoTArray<nsIFrame*, 32> stack;
+  for (nsIFrame::ChildListIterator lists(this); !lists.IsDone(); lists.Next()) {
+    for (nsIFrame* kid : lists.CurrentList()) {
+      stack.AppendElement(kid);
+    }
+  }
+  while (!stack.IsEmpty()) {
+    nsIFrame* f = stack.PopLastElement();
+    if (f->HasAnyStateBits(NS_FRAME_IS_DIRTY) || f->IsTableColGroupFrame() ||
+        f->IsXULBoxFrame()) {
+      continue;
+    }
+
+    f->AddStateBits(NS_FRAME_IS_DIRTY);
+
+    for (nsIFrame::ChildListIterator lists(f); !lists.IsDone(); lists.Next()) {
+      for (nsIFrame* kid : lists.CurrentList()) {
+        stack.AppendElement(kid);
+      }
+    }
+  }
+}
+
 /* virtual */
 nscoord nsFrame::GetMinISize(gfxContext* aRenderingContext) {
   nscoord result = 0;
@@ -10386,7 +10420,7 @@ void nsFrame::BoxReflow(nsBoxLayoutState& aState, nsPresContext* aPresContext,
       // requires a full reflow.  See ReflowInput::InitResizeFlags
       // for more details.
       if (nsLayoutUtils::FontSizeInflationEnabled(aPresContext)) {
-        AddStateBits(NS_FRAME_IS_DIRTY);
+        this->MarkSubtreeDirty();
       }
     }
     if (metrics->mLastSize.height != aHeight) {

layout/generic/nsIFrame.h

@@ -2158,6 +2158,12 @@ class nsIFrame : public nsQueryFrame {
    */
   virtual void MarkIntrinsicISizesDirty() = 0;
 
+  /**
+   * Make this frame and all descendants dirty (if not already).
+   * Exceptions: XULBoxFrame and TableColGroupFrame children.
+   */
+  void MarkSubtreeDirty();
+
   /**
    * Get the min-content intrinsic inline size of the frame.  This must be
    * less than or equal to the max-content intrinsic inline size.

layout/generic/nsRubyFrame.cpp

@@ -112,29 +112,6 @@ void nsRubyFrame::Reflow(nsPresContext* aPresContext,
   // Clear leadings
   mLeadings.Reset();
 
-  // Since the ruby base container is going to reflow not only the ruby
-  // base frames, but also the ruby text frames, and then *afterwards*
-  // we're going to reflow the ruby text containers (which do not reflow
-  // their children), we need to transfer NS_FRAME_IS_DIRTY status from
-  // the ruby text containers to their child ruby texts now, both so
-  // that the ruby texts are marked dirty if needed, and so that the
-  // ruby text container doesn't mark the ruby text frames dirty *after*
-  // they're reflowed and leave dirty bits in a clean tree (suppressing
-  // future reflows, due to lack of a queued reflow to clean them).
-  for (nsIFrame* child : PrincipalChildList()) {
-    if (child->HasAnyStateBits(NS_FRAME_IS_DIRTY) &&
-        child->IsRubyTextContainerFrame()) {
-      for (nsIFrame* grandchild : child->PrincipalChildList()) {
-        grandchild->AddStateBits(NS_FRAME_IS_DIRTY);
-      }
-      // Replace NS_FRAME_IS_DIRTY with NS_FRAME_HAS_DIRTY_CHILDREN so
-      // we still have a dirty marking, but one that we won't transfer
-      // to children again.
-      child->RemoveStateBits(NS_FRAME_IS_DIRTY);
-      child->AddStateBits(NS_FRAME_HAS_DIRTY_CHILDREN);
-    }
-  }
-
   // Begin the span for the ruby frame
   WritingMode frameWM = aReflowInput.GetWritingMode();
   WritingMode lineWM = aReflowInput.mLineLayout->GetWritingMode();

layout/generic/nsTextFrame.cpp

@@ -4749,7 +4749,7 @@ void nsTextFrame::NotifyNativeAnonymousTextnodeChange(uint32_t aOldLength) {
 
   // This is to avoid making a new Reflow request in CharacterDataChanged:
   for (nsTextFrame* f = this; f; f = f->GetNextContinuation()) {
-    f->AddStateBits(NS_FRAME_IS_DIRTY);
+    f->MarkSubtreeDirty();
     f->mReflowRequestedForCharDataChange = true;
   }
 
@@ -4828,7 +4828,7 @@ nsresult nsTextFrame::CharacterDataChanged(
         // Note: if the parent is a block, we're cheating here because we should
         // be marking our line dirty, but we're not. nsTextFrame::SetLength will
         // do that when it gets called during reflow.
-        textFrame->AddStateBits(NS_FRAME_IS_DIRTY);
+        textFrame->MarkSubtreeDirty();
       }
     }
     textFrame->InvalidateFrame();

layout/svg/SVGTextFrame.cpp

@@ -2967,7 +2967,7 @@ void SVGTextFrame::ReflowSVGNonDisplayText() {
 
   // We had a style change, so we mark this frame as dirty so that the next
   // time it is painted, we reflow the anonymous block frame.
-  AddStateBits(NS_FRAME_IS_DIRTY);
+  this->MarkSubtreeDirty();
 
   // We also need to call InvalidateRenderingObservers, so that if the <text>
   // element is within a <mask>, say, the element referencing the <mask> will
@@ -5030,7 +5030,7 @@ void SVGTextFrame::MaybeReflowAnonymousBlockChild() {
       // (Note that our anonymous nsBlockFrame is not an nsSVGDisplayableFrame,
       // so even when we are called via our ReflowSVG this will not be done for
       // us by nsSVGDisplayContainerFrame::ReflowSVG.)
-      kid->AddStateBits(NS_FRAME_IS_DIRTY);
+      kid->MarkSubtreeDirty();
     }
 
     // The RecordCorrespondence and DoReflow calls can result in new text frames

layout/svg/nsSVGContainerFrame.cpp

@@ -314,7 +314,6 @@ void nsSVGDisplayContainerFrame::ReflowSVG() {
     if (SVGFrame) {
       MOZ_ASSERT(!(kid->GetStateBits() & NS_FRAME_IS_NONDISPLAY),
                  "Check for this explicitly in the |if|, then");
-      kid->AddStateBits(mState & NS_FRAME_IS_DIRTY);
       SVGFrame->ReflowSVG();
 
       // We build up our child frame overflows here instead of using

layout/svg/nsSVGForeignObjectFrame.cpp

@@ -337,7 +337,7 @@ void nsSVGForeignObjectFrame::ReflowSVG() {
   // Fully mark our kid dirty so that it gets resized if necessary
   // (NS_FRAME_HAS_DIRTY_CHILDREN isn't enough in that case):
   nsIFrame* kid = PrincipalChildList().FirstChild();
-  kid->AddStateBits(NS_FRAME_IS_DIRTY);
+  kid->MarkSubtreeDirty();
 
   // Make sure to not allow interrupts if we're not being reflown as a root:
   nsPresContext::InterruptPreventer noInterrupts(PresContext());

layout/svg/nsSVGOuterSVGFrame.cpp

@@ -423,9 +423,9 @@ void nsSVGOuterSVGFrame::Reflow(nsPresContext* aPresContext,
     //
     if (svgElem->HasViewBoxOrSyntheticViewBox()) {
       nsIFrame* anonChild = PrincipalChildList().FirstChild();
-      anonChild->AddStateBits(NS_FRAME_IS_DIRTY);
+      anonChild->MarkSubtreeDirty();
       for (nsIFrame* child : anonChild->PrincipalChildList()) {
-        child->AddStateBits(NS_FRAME_IS_DIRTY);
+        child->MarkSubtreeDirty();
       }
     }
     changeBits |= COORD_CONTEXT_CHANGED;
@@ -450,7 +450,6 @@ void nsSVGOuterSVGFrame::Reflow(nsPresContext* aPresContext,
   } else {
     // Update the mRects and visual overflow rects of all our descendants,
     // including our anonymous wrapper kid:
-    anonKid->AddStateBits(mState & NS_FRAME_IS_DIRTY);
     anonKid->ReflowSVG();
     MOZ_ASSERT(!anonKid->GetNextSibling(),
                "We should have one anonymous child frame wrapping our real "

layout/svg/nsSVGUtils.cpp

@@ -191,7 +191,7 @@ void nsSVGUtils::ScheduleReflowSVG(nsIFrame* aFrame) {
   if (aFrame->IsSVGOuterSVGFrame()) {
     outerSVGFrame = static_cast<nsSVGOuterSVGFrame*>(aFrame);
   } else {
-    aFrame->AddStateBits(NS_FRAME_IS_DIRTY);
+    aFrame->MarkSubtreeDirty();
 
     nsIFrame* f = aFrame->GetParent();
     while (f && !f->IsSVGOuterSVGFrame()) {

layout/tables/nsTableWrapperFrame.cpp

@@ -153,7 +153,7 @@ void nsTableWrapperFrame::RemoveFrame(ChildListID aListID,
   if (HasSideCaption()) {
     // The old caption isize had an effect on the inner table isize, so
     // we're going to need to reflow it. Mark it dirty
-    InnerTableFrame()->AddStateBits(NS_FRAME_IS_DIRTY);
+    InnerTableFrame()->MarkSubtreeDirty();
   }
 
   // Remove the frame and destroy it

layout/xul/nsBox.cpp

@@ -35,7 +35,7 @@ nsresult nsBox::BeginXULLayout(nsBoxLayoutState& aState) {
     // does this too).
     nsIFrame* box;
     for (box = GetChildXULBox(this); box; box = GetNextXULBox(box))
-      box->AddStateBits(NS_FRAME_IS_DIRTY);
+      box->MarkSubtreeDirty();
   }
 
   // Another copy-over from ReflowInput.

layout/xul/nsBoxFrame.cpp

@@ -493,6 +493,9 @@ void nsBoxFrame::DidReflow(nsPresContext* aPresContext,
       mState & (NS_FRAME_IS_DIRTY | NS_FRAME_HAS_DIRTY_CHILDREN);
   nsFrame::DidReflow(aPresContext, aReflowInput);
   AddStateBits(preserveBits);
+  if (preserveBits & NS_FRAME_IS_DIRTY) {
+    this->MarkSubtreeDirty();
+  }
 }
 
 bool nsBoxFrame::HonorPrintBackgroundSettings() {