Bug 1474771 - Revert all changes from bug 1459937 since they are no longer needed. r=dholbert
authorL. David Baron <dbaron@dbaron.org>
Mon, 01 Jul 2019 21:58:35 +0000
changeset 544011 c70683bb3522b4ae05a185242a72cc20504bc93f
parent 544010 ae0a517daf9700bcb8c7ac0b035870fb508f2929
child 544012 89ef9660735bc8a85a5302910dede379b1d0bac8
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1474771, 1459937
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 1474771 - Revert all changes from bug 1459937 since they are no longer needed. r=dholbert (The single line that made them active was commented out in the previous patch.) Differential Revision: https://phabricator.services.mozilla.com/D36425
layout/base/nsLayoutUtils.h
layout/generic/nsBlockFrame.cpp
layout/generic/nsBlockFrame.h
layout/generic/nsContainerFrame.cpp
layout/generic/nsContainerFrame.h
layout/generic/nsInlineFrame.cpp
layout/generic/nsRubyBaseContainerFrame.cpp
layout/generic/nsRubyFrame.cpp
--- a/layout/base/nsLayoutUtils.h
+++ b/layout/base/nsLayoutUtils.h
@@ -128,22 +128,16 @@ enum class RelativeTo { ScrollPort, Scro
 
 // Flags to customize the behavior of nsLayoutUtils::DrawString.
 enum class DrawStringFlags {
   Default = 0x0,
   ForceHorizontal = 0x1  // Forces the text to be drawn horizontally.
 };
 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
  * is not to define multiple copies of the same static helper.
  */
 class nsLayoutUtils {
   typedef mozilla::AspectRatio AspectRatio;
   typedef mozilla::ComputedStyle ComputedStyle;
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -575,89 +575,33 @@ void nsBlockFrame::GetChildLists(nsTArra
   if (list) {
     list->AppendIfNonempty(aLists, kPushedFloatsList);
   }
 }
 
 /* virtual */
 bool nsBlockFrame::IsFloatContainingBlock() const { return true; }
 
-static void ReparentFrameInternal(nsIFrame* aFrame,
-                                  nsContainerFrame* aOldParent,
-                                  nsContainerFrame* aNewParent,
-                                  bool aMarkDirty) {
+static void ReparentFrame(nsIFrame* aFrame, nsContainerFrame* aOldParent,
+                          nsContainerFrame* aNewParent) {
   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(
-#ifdef DEBUG
-    nsContainerFrame* aOldParent,
-#endif
-    nsIFrame* aNewParent, ReparentingDirection aDirection) {
-#ifdef DEBUG
-  MOZ_ASSERT(aOldParent->FirstInFlow() == aNewParent->FirstInFlow(),
-             "Reparenting should be between continuations of the same frame");
-  if (aDirection != ReparentingDirection::Variable &&
-      aOldParent->FirstInFlow() == aNewParent->FirstInFlow()) {
-    auto IndexInFlow = [](const nsIFrame* f) {
-      int i = 0;
-      while ((f = f->GetPrevInFlow())) {
-        ++i;
-      }
-      return i;
-    };
-    MOZ_ASSERT((IndexInFlow(aOldParent) < IndexInFlow(aNewParent)) ==
-                   (aDirection == ReparentingDirection::Forwards),
-               "Parents not in expected order");
-  }
-#endif
-  // 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(
-#ifdef DEBUG
-      aOldParent,
-#endif
-      aNewParent, aDirection);
-  ReparentFrameInternal(aFrame, aOldParent, aNewParent, markDirty);
-}
-
 static void ReparentFrames(nsFrameList& aFrameList,
                            nsContainerFrame* aOldParent,
-                           nsContainerFrame* aNewParent,
-                           ReparentingDirection aDirection) {
-  const bool markDirty = ShouldMarkReparentedFramesDirty(
-#ifdef DEBUG
-      aOldParent,
-#endif
-      aNewParent, aDirection);
+                           nsContainerFrame* aNewParent) {
   for (nsFrameList::Enumerator e(aFrameList); !e.AtEnd(); e.Next()) {
-    ReparentFrameInternal(e.get(), aOldParent, aNewParent, markDirty);
+    ReparentFrame(e.get(), aOldParent, aNewParent);
   }
 }
 
 /**
  * Remove the first line from aFromLines and adjust the associated frame list
  * aFromFrames accordingly.  The removed line is assigned to *aOutLine and
  * a frame list with its frames is assigned to *aOutFrames, i.e. the frames
  * that were extracted from the head of aFromFrames.
@@ -2334,25 +2278,24 @@ static bool LineHasClear(nsLineBox* aLin
 
 /**
  * Reparent a whole list of floats from aOldParent to this block.  The
  * floats might be taken from aOldParent's overflow list. They will be
  * removed from the list. They end up appended to our mFloats list.
  */
 void nsBlockFrame::ReparentFloats(nsIFrame* aFirstFrame,
                                   nsBlockFrame* aOldParent,
-                                  bool aReparentSiblings,
-                                  ReparentingDirection aDirection) {
+                                  bool aReparentSiblings) {
   nsFrameList list;
   aOldParent->CollectFloats(aFirstFrame, list, aReparentSiblings);
   if (list.NotEmpty()) {
     for (nsIFrame* f : list) {
       MOZ_ASSERT(!(f->GetStateBits() & NS_FRAME_IS_PUSHED_FLOAT),
                  "CollectFloats should've removed that bit");
-      ReparentFrame(f, aOldParent, this, aDirection);
+      ReparentFrame(f, aOldParent, this);
     }
     mFloats.AppendFrames(nullptr, list);
   }
 }
 
 static void DumpLine(const BlockReflowInput& aState, nsLineBox* aLine,
                      nscoord aDeltaBCoord, int32_t aDeltaIndent) {
 #ifdef DEBUG
@@ -2837,36 +2780,34 @@ void nsBlockFrame::ReflowDirtyLines(Bloc
             "bad empty line");
         nextInFlow->FreeLineBox(pulledLine);
         continue;
       }
 
       if (pulledLine == nextInFlow->GetLineCursor()) {
         nextInFlow->ClearLineCursor();
       }
-      ReparentFrames(pulledFrames, nextInFlow, this,
-                     ReparentingDirection::Backwards);
+      ReparentFrames(pulledFrames, nextInFlow, this);
 
       NS_ASSERTION(pulledFrames.LastChild() == pulledLine->LastChild(),
                    "Unexpected last frame");
       NS_ASSERTION(aState.mPrevChild || mLines.empty(),
                    "should have a prevchild here");
       NS_ASSERTION(aState.mPrevChild == mFrames.LastChild(),
                    "Incorrect aState.mPrevChild before inserting line at end");
 
       // Shift pulledLine's frames into our mFrames list.
       mFrames.AppendFrames(nullptr, pulledFrames);
 
       // Add line to our line list, and set its last child as our new prev-child
       line = mLines.before_insert(LinesEnd(), pulledLine);
       aState.mPrevChild = mFrames.LastChild();
 
       // Reparent floats whose placeholders are in the line.
-      ReparentFloats(pulledLine->mFirstChild, nextInFlow, true,
-                     ReparentingDirection::Backwards);
+      ReparentFloats(pulledLine->mFirstChild, nextInFlow, true);
 
       DumpLine(aState, pulledLine, deltaBCoord, 0);
 #ifdef DEBUG
       AutoNoisyIndenter indent2(gNoisyReflow);
 #endif
 
       if (aState.mPresContext->HasPendingInterrupt()) {
         MarkLineDirtyForInterrupt(line);
@@ -3126,23 +3067,22 @@ nsIFrame* nsBlockFrame::PullFrameFrom(ns
     // need to add it to our sibling list.
     MOZ_ASSERT(aLine == mLines.back());
     MOZ_ASSERT(aFromLine == aFromContainer->mLines.begin(),
                "should only pull from first line");
     aFromContainer->mFrames.RemoveFrame(frame);
 
     // When pushing and pulling frames we need to check for whether any
     // views need to be reparented.
-    ReparentFrame(frame, aFromContainer, this, ReparentingDirection::Backwards);
+    ReparentFrame(frame, aFromContainer, this);
     mFrames.AppendFrame(nullptr, frame);
 
     // The frame might have (or contain) floats that need to be brought
     // over too. (pass 'false' since there are no siblings to check)
-    ReparentFloats(frame, aFromContainer, false,
-                   ReparentingDirection::Backwards);
+    ReparentFloats(frame, aFromContainer, false);
   } else {
     MOZ_ASSERT(aLine == aFromLine.prev());
   }
 
   aLine->NoteFrameAdded(frame);
   fromLine->NoteFrameRemoved(frame);
 
   if (fromLine->GetChildCount() > 0) {
@@ -3864,18 +3804,17 @@ void nsBlockFrame::ReflowBlockFrame(Bloc
               nsOverflowContinuationTracker::AutoFinish fini(
                   aState.mOverflowTracker, frame);
               nsContainerFrame* parent = nextFrame->GetParent();
               nsresult rv = parent->StealFrame(nextFrame);
               if (NS_FAILED(rv)) {
                 return;
               }
               if (parent != this) {
-                ReparentFrame(nextFrame, parent, this,
-                              ReparentingDirection::Variable);
+                ReparentFrame(nextFrame, parent, this);
               }
               mFrames.InsertFrame(nullptr, frame, nextFrame);
               madeContinuation = true;  // needs to be added to mLines
               nextFrame->RemoveStateBits(NS_FRAME_IS_OVERFLOW_CONTAINER);
               frameReflowStatus.SetNextInFlowNeedsReflow();
             }
 
             // Push continuation to a new line, but only if we actually made
@@ -4504,18 +4443,17 @@ void nsBlockFrame::SplitFloat(BlockReflo
   MOZ_ASSERT(aState.mBlock == this);
 
   nsIFrame* nextInFlow = aFloat->GetNextInFlow();
   if (nextInFlow) {
     nsContainerFrame* oldParent = nextInFlow->GetParent();
     DebugOnly<nsresult> rv = oldParent->StealFrame(nextInFlow);
     NS_ASSERTION(NS_SUCCEEDED(rv), "StealFrame failed");
     if (oldParent != this) {
-      ReparentFrame(nextInFlow, oldParent, this,
-                    ReparentingDirection::Backwards);
+      ReparentFrame(nextInFlow, oldParent, this);
     }
     if (!aFloatStatus.IsOverflowIncomplete()) {
       nextInFlow->RemoveStateBits(NS_FRAME_IS_OVERFLOW_CONTAINER);
     }
   } else {
     nextInFlow = aState.mPresContext->PresShell()
                      ->FrameConstructor()
                      ->CreateContinuingFrame(aState.mPresContext, aFloat, this);
@@ -5002,18 +4940,17 @@ bool nsBlockFrame::DrainOverflowLines() 
   // Steal the prev-in-flow's overflow lines and prepend them.
   bool didFindOverflow = false;
   nsBlockFrame* prevBlock = static_cast<nsBlockFrame*>(GetPrevInFlow());
   if (prevBlock) {
     prevBlock->ClearLineCursor();
     FrameLines* overflowLines = prevBlock->RemoveOverflowLines();
     if (overflowLines) {
       // Make all the frames on the overflow line list mine.
-      ReparentFrames(overflowLines->mFrames, prevBlock, this,
-                     ReparentingDirection::Forwards);
+      ReparentFrames(overflowLines->mFrames, prevBlock, this);
 
       // Collect overflow containers from our [Excess]OverflowContainers lists
       // that are continuations from the frames we picked up from our
       // prev-in-flow. We'll append these to mFrames to ensure the continuations
       // are ordered.
       auto HasOverflowContainers = [this]() -> bool {
         return GetPropTableFrames(OverflowContainersProperty()) ||
                GetPropTableFrames(ExcessOverflowContainersProperty());
@@ -5049,18 +4986,17 @@ bool nsBlockFrame::DrainOverflowLines() 
         for (nsFrameList::Enumerator e(oofs.mList); !e.AtEnd(); e.Next()) {
           nsIFrame* nif = e.get()->GetNextInFlow();
           for (; nif && nif->GetParent() == this; nif = nif->GetNextInFlow()) {
             MOZ_ASSERT(nif->HasAnyStateBits(NS_FRAME_IS_PUSHED_FLOAT));
             RemoveFloat(nif);
             pushedFloats.AppendFrame(nullptr, nif);
           }
         }
-        ReparentFrames(oofs.mList, prevBlock, this,
-                       ReparentingDirection::Forwards);
+        ReparentFrames(oofs.mList, prevBlock, this);
         mFloats.InsertFrames(nullptr, nullptr, oofs.mList);
         if (!pushedFloats.IsEmpty()) {
           nsFrameList* pf = EnsurePushedFloats();
           pf->InsertFrames(nullptr, nullptr, pushedFloats);
         }
       }
 
       if (!mLines.empty()) {
--- a/layout/generic/nsBlockFrame.h
+++ b/layout/generic/nsBlockFrame.h
@@ -517,17 +517,17 @@ class nsBlockFrame : public nsContainerF
    */
   enum { REMOVE_FIXED_CONTINUATIONS = 0x02, FRAMES_ARE_EMPTY = 0x04 };
   void DoRemoveFrame(nsIFrame* aDeletedFrame, uint32_t aFlags) {
     AutoPostDestroyData data(PresContext());
     DoRemoveFrameInternal(aDeletedFrame, aFlags, data.mData);
   }
 
   void ReparentFloats(nsIFrame* aFirstFrame, nsBlockFrame* aOldParent,
-                      bool aReparentSiblings, ReparentingDirection aDirection);
+                      bool aReparentSiblings);
 
   virtual bool ComputeCustomOverflow(nsOverflowAreas& aOverflowAreas) override;
 
   virtual void UnionChildOverflow(nsOverflowAreas& aOverflowAreas) override;
 
   /**
    * Load all of aFrame's floats into the float manager iff aFrame is not a
    * block formatting context. Handles all necessary float manager translations;
--- a/layout/generic/nsContainerFrame.cpp
+++ b/layout/generic/nsContainerFrame.cpp
@@ -1485,18 +1485,17 @@ bool nsContainerFrame::MoveInlineOverflo
   if (auto prevInFlow = static_cast<nsContainerFrame*>(GetPrevInFlow())) {
     AutoFrameListPtr prevOverflowFrames(PresContext(),
                                         prevInFlow->StealOverflowFrames());
     if (prevOverflowFrames) {
       // We may need to reparent floats from prev-in-flow to our line
       // container if the container has prev continuation.
       if (aLineContainer->GetPrevContinuation()) {
         ReparentFloatsForInlineChild(aLineContainer,
-                                     prevOverflowFrames->FirstChild(), true,
-                                     ReparentingDirection::Forwards);
+                                     prevOverflowFrames->FirstChild(), true);
       }
       // When pushing and pulling frames we need to check for whether
       // any views need to be reparented.
       nsContainerFrame::ReparentFrameViewList(*prevOverflowFrames, prevInFlow,
                                               this);
       // Prepend overflow frames to the list.
       mFrames.InsertFrames(this, nullptr, *prevOverflowFrames);
       result = true;
@@ -1634,19 +1633,19 @@ nsIFrame* nsContainerFrame::PullNextInFl
     // AppendFrame has reparented the frame, we need
     // to reparent the frame view then.
     nsContainerFrame::ReparentFrameView(frame, nextInFlow, this);
   }
   return frame;
 }
 
 /* static */
-void nsContainerFrame::ReparentFloatsForInlineChild(
-    nsIFrame* aOurLineContainer, nsIFrame* aFrame, bool aReparentSiblings,
-    ReparentingDirection aDirection) {
+void nsContainerFrame::ReparentFloatsForInlineChild(nsIFrame* aOurLineContainer,
+                                                    nsIFrame* aFrame,
+                                                    bool aReparentSiblings) {
   // XXXbz this would be better if it took a nsFrameList or a frame
   // list slice....
   NS_ASSERTION(aOurLineContainer->GetNextContinuation() ||
                    aOurLineContainer->GetPrevContinuation(),
                "Don't call this when we have no continuation, it's a waste");
   if (!aFrame) {
     NS_ASSERTION(aReparentSiblings, "Why did we get called?");
     return;
@@ -1656,31 +1655,30 @@ void nsContainerFrame::ReparentFloatsFor
   if (!frameBlock || frameBlock == aOurLineContainer) {
     return;
   }
 
   nsBlockFrame* ourBlock = do_QueryFrame(aOurLineContainer);
   NS_ASSERTION(ourBlock, "Not a block, but broke vertically?");
 
   while (true) {
-    ourBlock->ReparentFloats(aFrame, frameBlock, false, aDirection);
+    ourBlock->ReparentFloats(aFrame, frameBlock, false);
 
     if (!aReparentSiblings) return;
     nsIFrame* next = aFrame->GetNextSibling();
     if (!next) return;
     if (next->GetParent() == aFrame->GetParent()) {
       aFrame = next;
       continue;
     }
     // This is paranoid and will hardly ever get hit ... but we can't actually
     // trust that the frames in the sibling chain all have the same parent,
     // because lazy reparenting may be going on. If we find a different
     // parent we need to redo our analysis.
-    ReparentFloatsForInlineChild(aOurLineContainer, next, aReparentSiblings,
-                                 aDirection);
+    ReparentFloatsForInlineChild(aOurLineContainer, next, aReparentSiblings);
     return;
   }
 }
 
 bool nsContainerFrame::ResolvedOrientationIsVertical() {
   StyleOrient orient = StyleDisplay()->mOrient;
   switch (orient) {
     case StyleOrient::Horizontal:
--- a/layout/generic/nsContainerFrame.h
+++ b/layout/generic/nsContainerFrame.h
@@ -591,18 +591,17 @@ class nsContainerFrame : public nsSplitt
   /**
    * Reparent floats whose placeholders are inline descendants of aFrame from
    * whatever block they're currently parented by to aOurBlock.
    * @param aReparentSiblings if this is true, we follow aFrame's
    * GetNextSibling chain reparenting them all
    */
   static void ReparentFloatsForInlineChild(nsIFrame* aOurBlock,
                                            nsIFrame* aFrame,
-                                           bool aReparentSiblings,
-                                           ReparentingDirection aDirection);
+                                           bool aReparentSiblings);
 
   // ==========================================================================
   /*
    * Convenience methods for traversing continuations
    */
 
   struct ContinuationTraversingState {
     nsContainerFrame* mNextInFlow;
--- a/layout/generic/nsInlineFrame.cpp
+++ b/layout/generic/nsInlineFrame.cpp
@@ -751,18 +751,17 @@ nsIFrame* nsInlineFrame::PullOneFrame(ns
     if (frame) {
       // If our block has no next continuation, then any floats belonging to
       // the pulled frame must belong to our block already. This check ensures
       // we do no extra work in the common non-vertical-breaking case.
       if (irs.mLineContainer && irs.mLineContainer->GetNextContinuation()) {
         // The blockChildren.ContainsFrame check performed by
         // ReparentFloatsForInlineChild will be fast because frame's ancestor
         // will be the first child of its containing block.
-        ReparentFloatsForInlineChild(irs.mLineContainer, frame, false,
-                                     ReparentingDirection::Backwards);
+        ReparentFloatsForInlineChild(irs.mLineContainer, frame, false);
       }
       nextInFlow->mFrames.RemoveFirstChild();
       // nsFirstLineFrame::PullOneFrame calls ReparentComputedStyle.
 
       mFrames.InsertFrame(this, irs.mPrevFrame, frame);
       if (irs.mLineLayout) {
         irs.mLineLayout->SetDirtyNextLine();
       }
--- a/layout/generic/nsRubyBaseContainerFrame.cpp
+++ b/layout/generic/nsRubyBaseContainerFrame.cpp
@@ -741,18 +741,17 @@ void nsRubyBaseContainerFrame::PullOneCo
                  "All frames in the same ruby column should share "
                  "the same old float containing block");
     }
 #endif
     nsBlockFrame* newFloatCB = do_QueryFrame(aLineLayout->LineContainerFrame());
     MOZ_ASSERT(newFloatCB, "Must have a float containing block");
     if (oldFloatCB != newFloatCB) {
       for (nsIFrame* frame : aColumn) {
-        newFloatCB->ReparentFloats(frame, oldFloatCB, false,
-                                   ReparentingDirection::Backwards);
+        newFloatCB->ReparentFloats(frame, oldFloatCB, false);
       }
     }
   }
 
   // Pull the frames of this column.
   if (aColumn.mBaseFrame) {
     DebugOnly<nsIFrame*> pulled = PullNextInFlowChild(aPullFrameState.mBase);
     MOZ_ASSERT(pulled == aColumn.mBaseFrame, "pulled a wrong frame?");
--- a/layout/generic/nsRubyFrame.cpp
+++ b/layout/generic/nsRubyFrame.cpp
@@ -383,15 +383,14 @@ nsRubyBaseContainerFrame* nsRubyFrame::P
   while ((nextFrame = GetNextInFlowChild(aState)) != nullptr &&
          nextFrame->IsRubyTextContainerFrame()) {
     PullNextInFlowChild(aState);
   }
 
   if (nsBlockFrame* newFloatCB =
           do_QueryFrame(aLineLayout->LineContainerFrame())) {
     if (oldFloatCB && oldFloatCB != newFloatCB) {
-      newFloatCB->ReparentFloats(baseFrame, oldFloatCB, true,
-                                 ReparentingDirection::Backwards);
+      newFloatCB->ReparentFloats(baseFrame, oldFloatCB, true);
     }
   }
 
   return static_cast<nsRubyBaseContainerFrame*>(baseFrame);
 }