Bug 1459937 - Mark pulled lines (from n-i-f or overflow) dirty - r=dbaron

Lines pulled from next-in-flow or overflow frames have probably not been marked
dirty (as ReflowInput hasn't dealt with them when it was constructed), so we
need to mark them dirty for proper reflow.

If we don't do that, and they don't fit in the current column, the next column
will only mark its current children dirty, so when pulling back its first lines
from the previous column they will not be reflowed as needed, which causes this
bug.

MozReview-Commit-ID: 8GFO1ZWuZ1b

--HG--
extra : rebase_source : ee55a9ae7408e1f2603c1b2bc80ddcd8dbc837f0

layout/base/nsLayoutUtils.h

@@ -134,6 +134,12 @@ enum class DrawStringFlags {
 };
 MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(DrawStringFlags)
 
+enum class ReparentingDirection {
+  Backwards,
+  Forwards,
+  Variable // Could be either of the above; take most pessimistic action.
+};
+
 /**
  * nsLayoutUtils is a namespace class used for various helper
  * functions that are useful in multiple places in layout.  The goal

layout/generic/nsBlockFrame.cpp

@@ -606,25 +606,53 @@ nsBlockFrame::IsFloatContainingBlock() const
 }
 
 static void
-ReparentFrame(nsIFrame* aFrame, nsContainerFrame* aOldParent,
-              nsContainerFrame* aNewParent)
+ReparentFrameInternal(nsIFrame* aFrame, nsContainerFrame* aOldParent,
+                      nsContainerFrame* aNewParent, bool aMarkDirty)
 {
   NS_ASSERTION(aOldParent == aFrame->GetParent(),
                "Parent not consistent with expectations");
 
   aFrame->SetParent(aNewParent);
+  if (aMarkDirty) {
+    aFrame->AddStateBits(NS_FRAME_IS_DIRTY);
+  }
 
   // When pushing and pulling frames we need to check for whether any
   // views need to be reparented
   nsContainerFrame::ReparentFrameView(aFrame, aOldParent, aNewParent);
 }
 
+static bool
+ShouldMarkReparentedFramesDirty(nsIFrame* aNewParent,
+                                ReparentingDirection aDirection)
+{
+  // Frames going forward must have already been reflowed, or at least marked
+  // dirty. Otherwise frames going backwards (or direction is unknown) may not
+  // be marked dirty yet.
+  return (aDirection != ReparentingDirection::Forwards) &&
+         (aNewParent->GetStateBits() & NS_FRAME_IS_DIRTY);
+}
+
+// Because a frame with NS_FRAME_IS_DIRTY marks all of its children dirty at
+// the start of its reflow, when we move a frame from a later frame backwards to
+// an earlier frame, and the earlier frame has NS_FRAME_IS_DIRTY (though that
+// should corresponded with the later frame having NS_FRAME_IS_DIRTY), we need
+// to add NS_FRAME_IS_DIRTY to the reparented frame.
+static void
+ReparentFrame(nsIFrame* aFrame, nsContainerFrame* aOldParent,
+              nsContainerFrame* aNewParent, ReparentingDirection aDirection)
+{
+  const bool markDirty = ShouldMarkReparentedFramesDirty(aNewParent, aDirection);
+  ReparentFrameInternal(aFrame, aOldParent, aNewParent, markDirty);
+}
+
 static void
 ReparentFrames(nsFrameList& aFrameList, nsContainerFrame* aOldParent,
-               nsContainerFrame* aNewParent)
+               nsContainerFrame* aNewParent, ReparentingDirection aDirection)
 {
+  const bool markDirty = ShouldMarkReparentedFramesDirty(aNewParent, aDirection);
   for (nsFrameList::Enumerator e(aFrameList); !e.AtEnd(); e.Next()) {
-    ReparentFrame(e.get(), aOldParent, aNewParent);
+    ReparentFrameInternal(e.get(), aOldParent, aNewParent, markDirty);
   }
 }
 
@@ -2130,7 +2158,8 @@ nsBlockFrame::ReparentFloats(nsIFrame* aFirstFrame, nsBlockFrame* aOldParent,
     for (nsIFrame* f : list) {
       MOZ_ASSERT(!(f->GetStateBits() & NS_FRAME_IS_PUSHED_FLOAT),
                  "CollectFloats should've removed that bit");
-      ReparentFrame(f, aOldParent, this);
+      // XXX: "Backwards" to force NS_FRAME_IS_DIRTY for now, see next patch.
+      ReparentFrame(f, aOldParent, this, ReparentingDirection::Backwards);
     }
     mFloats.AppendFrames(nullptr, list);
   }
@@ -2627,7 +2656,8 @@ nsBlockFrame::ReflowDirtyLines(BlockReflowInput& aState)
       if (pulledLine == nextInFlow->GetLineCursor()) {
         nextInFlow->ClearLineCursor();
       }
-      ReparentFrames(pulledFrames, nextInFlow, this);
+      ReparentFrames(pulledFrames, nextInFlow, this,
+                     ReparentingDirection::Backwards);
 
       NS_ASSERTION(pulledFrames.LastChild() == pulledLine->LastChild(),
                    "Unexpected last frame");
@@ -2922,7 +2952,7 @@ nsBlockFrame::PullFrameFrom(nsLineBox*           aLine,
 
     // When pushing and pulling frames we need to check for whether any
     // views need to be reparented.
-    ReparentFrame(frame, aFromContainer, this);
+    ReparentFrame(frame, aFromContainer, this, ReparentingDirection::Backwards);
     mFrames.AppendFrame(nullptr, frame);
 
     // The frame might have (or contain) floats that need to be brought
@@ -3671,8 +3701,10 @@ nsBlockFrame::ReflowBlockFrame(BlockReflowInput& aState,
               if (NS_FAILED(rv)) {
                 return;
               }
-              if (parent != this)
-                ReparentFrame(nextFrame, parent, this);
+              if (parent != this) {
+                ReparentFrame(nextFrame, parent, this,
+                              ReparentingDirection::Variable);
+              }
               mFrames.InsertFrame(nullptr, frame, nextFrame);
               madeContinuation = true; // needs to be added to mLines
               nextFrame->RemoveStateBits(NS_FRAME_IS_OVERFLOW_CONTAINER);
@@ -4331,7 +4363,7 @@ nsBlockFrame::SplitFloat(BlockReflowInput& aState,
     DebugOnly<nsresult> rv = oldParent->StealFrame(nextInFlow);
     NS_ASSERTION(NS_SUCCEEDED(rv), "StealFrame failed");
     if (oldParent != this) {
-      ReparentFrame(nextInFlow, oldParent, this);
+      ReparentFrame(nextInFlow, oldParent, this, ReparentingDirection::Backwards);
     }
     if (!aFloatStatus.IsOverflowIncomplete()) {
       nextInFlow->RemoveStateBits(NS_FRAME_IS_OVERFLOW_CONTAINER);
@@ -4850,7 +4882,8 @@ nsBlockFrame::DrainOverflowLines()
     FrameLines* overflowLines = prevBlock->RemoveOverflowLines();
     if (overflowLines) {
       // Make all the frames on the overflow line list mine.
-      ReparentFrames(overflowLines->mFrames, prevBlock, this);
+      ReparentFrames(overflowLines->mFrames, prevBlock, this,
+                     ReparentingDirection::Forwards);
 
       // Make the overflow out-of-flow frames mine too.
       nsAutoOOFFrameList oofs(prevBlock);
@@ -4864,7 +4897,8 @@ nsBlockFrame::DrainOverflowLines()
             nif->RemoveStateBits(NS_FRAME_IS_PUSHED_FLOAT);
           }
         }
-        ReparentFrames(oofs.mList, prevBlock, this);
+        ReparentFrames(oofs.mList, prevBlock, this,
+                       ReparentingDirection::Forwards);
         mFloats.InsertFrames(nullptr, nullptr, oofs.mList);
       }