Bug 1308876 - Mark child frames as dirty before starting reflow of the parent, so that if we reflow a child twice, it's only dirty the first time. r=dholbert

This is the primary patch in this bug, and makes the performance
improvement that fixes this bug.

The assertion count increase for layout/generic/crashtests/1015844.html
is accompanied by a layout change in the testcase as well.  However, I'm
not planning to fix it in this sequence; fundamentally columnsets with
specified heights inside a paginated context (like another columnset) do
not work in any reasonable way, and changing the number of times we
reflow them can change the layout.  At least, assuming I didn't lose
something in the process of simplifying the testcase.

ISSUES:
 - may make block inside XUL worse in performance by marking dirty more (see subdoc in Firefox UI, or text control innards?)

MozReview-Commit-ID: GdOvPynqcFP

layout/generic/ReflowInput.cpp

@@ -206,13 +206,6 @@ ReflowInput::ReflowInput(
 
   mParentReflowInput = &aParentReflowInput;
 
-  // If the parent is dirty, then the child is as well.
-  // XXX Are the other cases where the parent reflows a child a second
-  // time, as a resize?
-  if (!mFlags.mSpecialBSizeReflow)
-    mFrame->AddStateBits(mParentReflowInput->mFrame->GetStateBits() &
-                        NS_FRAME_IS_DIRTY);
-
   AvailableISize() = aAvailableSpace.ISize(mWritingMode);
   AvailableBSize() = aAvailableSpace.BSize(mWritingMode);
 
@@ -366,6 +359,25 @@ ReflowInput::Init(nsPresContext*     aPresContext,
                         const nsMargin*    aBorder,
                         const nsMargin*    aPadding)
 {
+  if ((mFrame->GetStateBits() & NS_FRAME_IS_DIRTY) &&
+      !mFrame->IsXULBoxFrame()) {
+    // Mark all child frames as dirty.
+    //
+    // We don't do this for XUL boxes because they handle their child
+    // reflow separately.
+    //
+    // 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.
+    for (nsIFrame::ChildListIterator childLists(mFrame);
+         !childLists.IsDone(); childLists.Next()) {
+      for (nsIFrame* childFrame : childLists.CurrentList()) {
+        childFrame->AddStateBits(NS_FRAME_IS_DIRTY);
+      }
+    }
+  }
+
   if (AvailableISize() == NS_UNCONSTRAINEDSIZE) {
     // Look up the parent chain for an orthogonal inline limit,
     // and reset AvailableISize() if found.

layout/generic/crashtests/crashtests.list

@@ -310,7 +310,7 @@ load 455171-3.html
 load 455643-1.xhtml
 load 457375.html
 load 457380-1.html
-asserts-if(!Android,4) load 459968.html # bug 1067022
+asserts-if(!Android,1) load 459968.html # bug 1067022
 load 460910-1.xml
 load 461294-1.html
 load 462968.xhtml
@@ -502,7 +502,7 @@ load 786740-1.html
 load 790260-1.html
 test-pref(font.size.inflation.emPerLine,15) load 791601.xhtml
 test-pref(font.size.inflation.minTwips,120) load 794693.html
-asserts-if(!Android,4) load 798020-1.html
+asserts-if(!Android,1) load 798020-1.html
 load 798235-1.html
 load 799207-1.html
 load 799207-2.html
@@ -573,7 +573,7 @@ load 1003441.xul
 pref(layout.css.grid.enabled,true) load 1015562.html
 asserts(1-2) load 1015563-1.html
 asserts(1-2) load 1015563-2.html
-asserts(0-300) load 1015844.html # bug 574889
+asserts(543) load 1015844.html # bug 574889, bug 1374479
 pref(font.size.inflation.minTwips,200) load 1032450.html
 load 1032613-1.svg
 load 1032613-2.html

layout/generic/nsColumnSetFrame.cpp

@@ -485,6 +485,14 @@ nsColumnSetFrame::ChooseColumnStrategy(const ReflowInput& aReflowInput,
   return config;
 }
 
+static void
+MarkPrincipalChildrenDirty(nsIFrame* aFrame)
+{
+  for (nsIFrame* childFrame : aFrame->PrincipalChildList()) {
+    childFrame->AddStateBits(NS_FRAME_IS_DIRTY);
+  }
+}
+
 bool
 nsColumnSetFrame::ReflowColumns(ReflowOutput& aDesiredSize,
                                 const ReflowInput& aReflowInput,
@@ -1131,7 +1139,7 @@ nsColumnSetFrame::FindBestBalanceBSize(const ReflowInput& aReflowInput,
     aConfig.mColMaxBSize = nextGuess;
 
     aUnboundedLastColumn = false;
-    AddStateBits(NS_FRAME_IS_DIRTY);
+    MarkPrincipalChildrenDirty(this);
     feasible = ReflowColumns(aDesiredSize, aReflowInput, aStatus, aConfig, false,
                              &aOutMargin, aColData);
 
@@ -1160,7 +1168,7 @@ nsColumnSetFrame::FindBestBalanceBSize(const ReflowInput& aReflowInput,
       // allowed to have arbitrary height here, even though we were balancing.
       // Otherwise we'd have to split, and it's not clear what we'd do with
       // that.
-      AddStateBits(NS_FRAME_IS_DIRTY);
+      MarkPrincipalChildrenDirty(this);
       feasible = ReflowColumns(aDesiredSize, aReflowInput, aStatus, aConfig,
                                availableContentBSize == NS_UNCONSTRAINEDSIZE,
                                &aOutMargin, aColData);

layout/generic/nsRubyFrame.cpp

@@ -120,6 +120,29 @@ 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();