Bug 1677917 - Change BlockReflowInput's skipsides setup to be sane. r=mats,TYLin

Precomputing the skipBEnd bit is odd / wrong. Using the PreReflow
version causes no regression, and allows us to simplify the code.

It also reverts the test annotations added to bug 1675376 which were
caused by the extra argument to GetLogicalSkipSides() somehow.

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

layout/generic/BlockReflowInput.cpp

@@ -42,9 +42,7 @@ BlockReflowInput::BlockReflowInput(const ReflowInput& aReflowInput,
       mBorderPadding(
           mReflowInput
               .ComputedLogicalBorderPadding(mReflowInput.GetWritingMode())
-              .ApplySkipSides(aFrame->GetLogicalSkipSides(
-                  Some(nsIFrame::SkipSidesDuringReflow{aReflowInput,
-                                                       aConsumedBSize})))),
+              .ApplySkipSides(aFrame->PreReflowBlockLevelLogicalSkipSides())),
       mPrevBEndMargin(),
       mLineNumber(0),
       mFloatBreakType(StyleClear::None),
@@ -120,8 +118,11 @@ BlockReflowInput::BlockReflowInput(const ReflowInput& aReflowInput,
     // We are in a paginated situation. The block-end edge is just inside the
     // block-end border and padding. The content area block-size doesn't include
     // either border or padding edge.
-    mContentArea.BSize(wm) = std::max(
-        0, aReflowInput.AvailableBSize() - mBorderPadding.BStartEnd(wm));
+    auto bp = aFrame->StyleBorder()->mBoxDecorationBreak ==
+                      StyleBoxDecorationBreak::Clone
+                  ? mBorderPadding.BStartEnd(wm)
+                  : mBorderPadding.BStart(wm);
+    mContentArea.BSize(wm) = std::max(0, aReflowInput.AvailableBSize() - bp);
   } else {
     // When we are not in a paginated situation, then we always use a
     // unconstrained block-size.

layout/generic/nsBlockFrame.cpp

@@ -1879,8 +1879,8 @@ void nsBlockFrame::ComputeFinalSize(const ReflowInput& aReflowInput,
   }
 
   if (NS_UNCONSTRAINEDSIZE != aReflowInput.ComputedBSize()) {
-    // Note: We don't use blockEndEdgeOfChildren because it inclues the previous
-    // margin.
+    // Note: We don't use blockEndEdgeOfChildren because it includes the
+    // previous margin.
     nscoord contentBSize = aState.mBCoord + nonCarriedOutBDirMargin;
     finalSize.BSize(wm) =
         ComputeFinalBSize(aReflowInput, aState.mReflowStatus, contentBSize,
@@ -3769,9 +3769,12 @@ void nsBlockFrame::ReflowBlockFrame(BlockReflowInput& aState,
       // input for ColumnSet so that ColumnSet can use it to compute its max
       // column block size.
       if (frame->IsColumnSetFrame()) {
-        if (availSize.BSize(wm) != NS_UNCONSTRAINEDSIZE) {
+        if (availSize.BSize(wm) != NS_UNCONSTRAINEDSIZE &&
+            StyleBorder()->mBoxDecorationBreak ==
+                StyleBoxDecorationBreak::Clone) {
           // If the available size is constrained, we need to subtract
-          // ColumnSetWrapper's block-end border and padding.
+          // ColumnSetWrapper's block-end border and padding, if we know we're
+          // going to use it.
           availSize.BSize(wm) -= aState.BorderPadding().BEnd(wm);
         }
 
@@ -7665,13 +7668,12 @@ nscoord nsBlockFrame::ComputeFinalBSize(const ReflowInput& aReflowInput,
   NS_ASSERTION(!(IsTrueOverflowContainer() && computedBSizeLeftOver),
                "overflow container must not have computedBSizeLeftOver");
 
-  const nsReflowStatus statusFromChildren = aStatus;
   const nscoord availBSize = aReflowInput.AvailableBSize();
   nscoord finalBSize = NSCoordSaturatingAdd(
       NSCoordSaturatingAdd(aBorderPadding.BStart(wm), computedBSizeLeftOver),
       aBorderPadding.BEnd(wm));
 
-  if (statusFromChildren.IsIncomplete() && finalBSize <= availBSize) {
+  if (aStatus.IsIncomplete() && finalBSize <= availBSize) {
     // We used up all of our element's remaining computed block-size on this
     // page/column, but our children are incomplete. Set aStatus to
     // overflow-incomplete.
@@ -7694,7 +7696,7 @@ nscoord nsBlockFrame::ComputeFinalBSize(const ReflowInput& aReflowInput,
     return std::min(finalBSize, aBEndEdgeOfChildren);
   }
 
-  if (statusFromChildren.IsComplete()) {
+  if (aStatus.IsComplete()) {
     if (computedBSizeLeftOver > 0 && NS_UNCONSTRAINEDSIZE != availBSize &&
         finalBSize > availBSize) {
       if (ShouldAvoidBreakInside(aReflowInput)) {

layout/generic/nsFirstLetterFrame.cpp

@@ -379,8 +379,7 @@ nscoord nsFirstLetterFrame::GetLogicalBaseline(WritingMode aWritingMode) const {
   return mBaseline;
 }
 
-nsIFrame::LogicalSides nsFirstLetterFrame::GetLogicalSkipSides(
-    const Maybe<SkipSidesDuringReflow>&) const {
+LogicalSides nsFirstLetterFrame::GetLogicalSkipSides() const {
   if (GetPrevContinuation()) {
     // We shouldn't get calls to GetSkipSides for later continuations since
     // they have separate ComputedStyles with initial values for all the

layout/generic/nsFirstLetterFrame.h

@@ -62,8 +62,7 @@ class nsFirstLetterFrame final : public nsContainerFrame {
   virtual bool CanContinueTextRun() const override;
   virtual nscoord GetLogicalBaseline(
       mozilla::WritingMode aWritingMode) const override;
-  virtual LogicalSides GetLogicalSkipSides(
-      const Maybe<SkipSidesDuringReflow>&) const override;
+  virtual LogicalSides GetLogicalSkipSides() const override;
 
   // override of nsFrame method
   virtual nsresult GetChildFrameContainingOffset(

layout/generic/nsIFrame.h

@@ -3664,23 +3664,12 @@ class nsIFrame : public nsQueryFrame {
    * @note (See also bug 743402, comment 11) GetSkipSides() checks to see
    *       if this frame has a previous or next continuation to determine
    *       if a side should be skipped.
-   *       Unfortunately, this only works after reflow has been completed. In
-   *       lieu of this, during reflow, a SkipSidesDuringReflow parameter can
-   *       be passed in, indicating that it should be used to determine if sides
-   *       should be skipped during reflow.
-   *
-   * FIXME(emilio, bug 1677917): That's wrong, fix BlockReflowInput and remove
-   * SkipSidesDuringReflow and related code.
+   *       So this only works after the entire frame tree has been reflowed.
+   *       During reflow, if this frame can be split in the block axis, you
+   *       should use nsSplittableFrame::PreReflowBlockLevelLogicalSkipSides().
    */
   Sides GetSkipSides() const;
-
-  struct SkipSidesDuringReflow {
-    const ReflowInput& mReflowInput;
-    const nscoord mConsumedBSize = NS_UNCONSTRAINEDSIZE;
-  };
-
-  virtual LogicalSides GetLogicalSkipSides(
-      const Maybe<SkipSidesDuringReflow>& = Nothing()) const {
+  virtual LogicalSides GetLogicalSkipSides() const {
     return LogicalSides(mWritingMode);
   }
 

layout/generic/nsImageFrame.cpp

@@ -2457,8 +2457,7 @@ void nsImageFrame::List(FILE* out, const char* aPrefix,
 }
 #endif
 
-nsIFrame::LogicalSides nsImageFrame::GetLogicalSkipSides(
-    const Maybe<SkipSidesDuringReflow>&) const {
+LogicalSides nsImageFrame::GetLogicalSkipSides() const {
   LogicalSides skip(mWritingMode);
   if (MOZ_UNLIKELY(StyleBorder()->mBoxDecorationBreak ==
                    StyleBoxDecorationBreak::Clone)) {

layout/generic/nsImageFrame.h

@@ -118,8 +118,7 @@ class nsImageFrame : public nsAtomicContainerFrame, public nsIReflowCallback {
             ListFlags aFlags = ListFlags()) const final;
 #endif
 
-  LogicalSides GetLogicalSkipSides(
-      const Maybe<SkipSidesDuringReflow>&) const final;
+  LogicalSides GetLogicalSkipSides() const final;
 
   static void ReleaseGlobals() {
     if (gIconLoad) {

layout/generic/nsInlineFrame.cpp

@@ -791,8 +791,7 @@ void nsInlineFrame::PushFrames(nsPresContext* aPresContext,
 
 //////////////////////////////////////////////////////////////////////
 
-nsIFrame::LogicalSides nsInlineFrame::GetLogicalSkipSides(
-    const Maybe<SkipSidesDuringReflow>&) const {
+LogicalSides nsInlineFrame::GetLogicalSkipSides() const {
   LogicalSides skip(mWritingMode);
   if (MOZ_UNLIKELY(StyleBorder()->mBoxDecorationBreak ==
                    StyleBoxDecorationBreak::Clone)) {

layout/generic/nsInlineFrame.h

@@ -148,8 +148,7 @@ class nsInlineFrame : public nsContainerFrame {
       : nsContainerFrame(aStyle, aPresContext, aID),
         mBaseline(NS_INTRINSIC_ISIZE_UNKNOWN) {}
 
-  LogicalSides GetLogicalSkipSides(
-      const Maybe<SkipSidesDuringReflow>&) const override;
+  LogicalSides GetLogicalSkipSides() const override;
 
   void ReflowFrames(nsPresContext* aPresContext,
                     const ReflowInput& aReflowInput, InlineReflowInput& rs,

layout/generic/nsSplittableFrame.cpp

@@ -234,10 +234,10 @@ nscoord nsSplittableFrame::GetEffectiveComputedBSize(
   return std::max(0, bSize);
 }
 
-nsIFrame::LogicalSides nsSplittableFrame::GetLogicalSkipSides(
-    const Maybe<SkipSidesDuringReflow>& aDuringReflow) const {
+LogicalSides nsSplittableFrame::GetBlockLevelLogicalSkipSides(
+    bool aAfterReflow) const {
   LogicalSides skip(mWritingMode);
-  if (IsTrueOverflowContainer()) {
+  if (MOZ_UNLIKELY(IsTrueOverflowContainer())) {
     skip |= eLogicalSideBitsBBoth;
     return skip;
   }
@@ -251,49 +251,18 @@ nsIFrame::LogicalSides nsSplittableFrame::GetLogicalSkipSides(
     skip |= eLogicalSideBitsBStart;
   }
 
-  if (aDuringReflow) {
-    nscoord availBSize = aDuringReflow->mReflowInput.AvailableBSize();
-    // We're in the midst of reflow right now, so it's possible that we haven't
-    // created a next-in-flow yet. If our content block-size is going to exceed
-    // our available block-size, though, then we're going to need a
-    // next-in-flow, it just hasn't been created yet.
-    if (NS_UNCONSTRAINEDSIZE != availBSize) {
-      nscoord effectiveBSize = GetEffectiveComputedBSize(
-          aDuringReflow->mReflowInput, aDuringReflow->mConsumedBSize);
-      if (effectiveBSize != NS_UNCONSTRAINEDSIZE &&
-          effectiveBSize > availBSize) {
-        // Our computed block-size is going to exceed our available block-size,
-        // so we're going to need a next-in-flow.
-        skip |= eLogicalSideBitsBEnd;
-      }
-    }
-  } else {
-    nsIFrame* nif = GetNextContinuation();
-    if (nif && !nif->IsTrueOverflowContainer()) {
-      skip |= eLogicalSideBitsBEnd;
-    }
-  }
-
   // Always skip block-end side if we have a *later* sibling across column-span
   // split.
   if (HasColumnSpanSiblings()) {
     skip |= eLogicalSideBitsBEnd;
   }
 
-  return skip;
-}
+  if (aAfterReflow) {
+    nsIFrame* nif = GetNextContinuation();
+    if (nif && !nif->IsTrueOverflowContainer()) {
+      skip |= eLogicalSideBitsBEnd;
+    }
+  }
 
-LogicalSides nsSplittableFrame::PreReflowBlockLevelLogicalSkipSides() const {
-  LogicalSides skip(mWritingMode);
-  if (MOZ_UNLIKELY(IsTrueOverflowContainer())) {
-    skip |= mozilla::eLogicalSideBitsBBoth;
-    return skip;
-  }
-  if (MOZ_LIKELY(StyleBorder()->mBoxDecorationBreak !=
-                 StyleBoxDecorationBreak::Clone) &&
-      GetPrevInFlow()) {
-    skip |= mozilla::eLogicalSideBitsBStart;
-    return skip;
-  }
   return skip;
 }

layout/generic/nsSplittableFrame.h

@@ -107,20 +107,25 @@ class nsSplittableFrame : public nsIFrame {
   /**
    * @see nsIFrame::GetLogicalSkipSides()
    */
-  LogicalSides GetLogicalSkipSides(
-      const Maybe<SkipSidesDuringReflow>& = Nothing()) const override;
+  LogicalSides GetLogicalSkipSides() const override {
+    return GetBlockLevelLogicalSkipSides(true);
+  }
+
+  LogicalSides GetBlockLevelLogicalSkipSides(bool aAfterReflow) const;
 
   /**
-   * A faster version of GetLogicalSkipSides() that is intended to be used
-   * inside Reflow before it's known if |this| frame will be COMPLETE or not.
+   * A version of GetLogicalSkipSides() that is intended to be used inside
+   * Reflow before it's known if |this| frame will be COMPLETE or not.
    * It returns a result that assumes this fragment is the last and thus
    * should apply the block-end border/padding etc (except for "true" overflow
    * containers which always skip block sides).  You're then expected to
    * recalculate the block-end side (as needed) when you know |this| frame's
    * reflow status is INCOMPLETE.
-   * This method is intended for frames that breaks in the block axis.
+   * This method is intended for frames that break in the block axis.
    */
-  LogicalSides PreReflowBlockLevelLogicalSkipSides() const;
+  LogicalSides PreReflowBlockLevelLogicalSkipSides() const {
+    return GetBlockLevelLogicalSkipSides(false);
+  };
 
   nsIFrame* mPrevContinuation;
   nsIFrame* mNextContinuation;

layout/reftests/pagination/fieldset-00D-ref.html

@@ -27,6 +27,5 @@ legend {
 </head>
 <body>
   <fieldset><legend></legend></fieldset>
-  <br style="page-break-before: always">
 </body>
 </html>

layout/style/crashtests/crashtests.list

@@ -209,9 +209,9 @@ load large_border_image_width.html
 load link-transition-before.html
 skip-if(winWidget&&isDebugBuild&&/^Windows\x20NT\x206\.1/.test(http.oscpu)) load long-url-list-stack-overflow.html #Bug 1525117
 load scale-on-block-continuation.html
-skip-if((/^Windows\x20NT\x2010\.0/.test(http.oscpu))&&(/^aarch64-msvc/.test(xulRuntime.XPCOMABI))) skip-if(AddressSanitizer) load 1383981.html # Very sensitive to stack size.
-skip-if((/^Windows\x20NT\x2010\.0/.test(http.oscpu))&&(/^aarch64-msvc/.test(xulRuntime.XPCOMABI))) skip-if(AddressSanitizer) load 1383981-2.html
-skip-if((/^Windows\x20NT\x2010\.0/.test(http.oscpu))&&(/^aarch64-msvc/.test(xulRuntime.XPCOMABI))) skip-if(AddressSanitizer) load 1383981-3.html
+skip-if((/^Windows\x20NT\x2010\.0/.test(http.oscpu))&&(/^aarch64-msvc/.test(xulRuntime.XPCOMABI))) load 1383981.html # Very sensitive to stack size.
+skip-if((/^Windows\x20NT\x2010\.0/.test(http.oscpu))&&(/^aarch64-msvc/.test(xulRuntime.XPCOMABI))) load 1383981-2.html
+skip-if((/^Windows\x20NT\x2010\.0/.test(http.oscpu))&&(/^aarch64-msvc/.test(xulRuntime.XPCOMABI))) load 1383981-3.html
 load 1384824-1.html
 load 1384824-2.html
 load 1386773.html

layout/tables/nsTableCellFrame.cpp

@@ -522,8 +522,7 @@ void nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
   BuildDisplayListForChild(aBuilder, kid, aLists);
 }
 
-nsIFrame::LogicalSides nsTableCellFrame::GetLogicalSkipSides(
-    const Maybe<SkipSidesDuringReflow>&) const {
+LogicalSides nsTableCellFrame::GetLogicalSkipSides() const {
   LogicalSides skip(mWritingMode);
   if (MOZ_UNLIKELY(StyleBorder()->mBoxDecorationBreak ==
                    StyleBoxDecorationBreak::Clone)) {

layout/tables/nsTableCellFrame.h

@@ -252,8 +252,7 @@ class nsTableCellFrame : public nsContainerFrame,
                    ClassID aID);
   ~nsTableCellFrame();
 
-  LogicalSides GetLogicalSkipSides(
-      const Maybe<SkipSidesDuringReflow>& aConsumed = Nothing()) const override;
+  LogicalSides GetLogicalSkipSides() const override;
 
   /**
    * GetBorderOverflow says how far the cell's own borders extend

layout/tables/nsTableColGroupFrame.cpp

@@ -297,8 +297,7 @@ void nsTableColGroupFrame::RemoveFrame(ChildListID aListID,
   }
 }
 
-nsIFrame::LogicalSides nsTableColGroupFrame::GetLogicalSkipSides(
-    const Maybe<SkipSidesDuringReflow>&) const {
+nsIFrame::LogicalSides nsTableColGroupFrame::GetLogicalSkipSides() const {
   LogicalSides skip(mWritingMode);
   if (MOZ_UNLIKELY(StyleBorder()->mBoxDecorationBreak ==
                    StyleBoxDecorationBreak::Clone)) {

layout/tables/nsTableColGroupFrame.h

@@ -214,8 +214,7 @@ class nsTableColGroupFrame final : public nsContainerFrame {
 
   void InsertColsReflow(int32_t aColIndex, const nsFrameList::Slice& aCols);
 
-  LogicalSides GetLogicalSkipSides(
-      const Maybe<SkipSidesDuringReflow>& = Nothing()) const override;
+  LogicalSides GetLogicalSkipSides() const override;
 
   // data members
   int32_t mColCount;

layout/tables/nsTableFrame.cpp

@@ -1359,8 +1359,7 @@ nsMargin nsTableFrame::GetDeflationForBackground(
   return GetOuterBCBorder(wm).GetPhysicalMargin(wm);
 }
 
-nsIFrame::LogicalSides nsTableFrame::GetLogicalSkipSides(
-    const Maybe<SkipSidesDuringReflow>&) const {
+LogicalSides nsTableFrame::GetLogicalSkipSides() const {
   LogicalSides skip(mWritingMode);
   if (MOZ_UNLIKELY(StyleBorder()->mBoxDecorationBreak ==
                    StyleBoxDecorationBreak::Clone)) {

layout/tables/nsTableFrame.h

@@ -570,8 +570,7 @@ class nsTableFrame : public nsContainerFrame {
 
   void InitChildReflowInput(ReflowInput& aReflowInput);
 
-  LogicalSides GetLogicalSkipSides(
-      const Maybe<SkipSidesDuringReflow>& = Nothing()) const override;
+  LogicalSides GetLogicalSkipSides() const override;
 
   void IterateBCBorders(BCPaintBorderAction& aAction, const nsRect& aDirtyRect);
 

layout/tables/nsTableRowFrame.cpp

@@ -549,8 +549,7 @@ void nsTableRowFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
   }
 }
 
-nsIFrame::LogicalSides nsTableRowFrame::GetLogicalSkipSides(
-    const Maybe<SkipSidesDuringReflow>&) const {
+LogicalSides nsTableRowFrame::GetLogicalSkipSides() const {
   LogicalSides skip(mWritingMode);
   if (MOZ_UNLIKELY(StyleBorder()->mBoxDecorationBreak ==
                    StyleBoxDecorationBreak::Clone)) {

layout/tables/nsTableRowFrame.h

@@ -269,8 +269,7 @@ class nsTableRowFrame : public nsContainerFrame {
                             bool aBorderCollapse,
                             TableCellReflowInput& aReflowInput);
 
-  LogicalSides GetLogicalSkipSides(
-      const Maybe<SkipSidesDuringReflow>&) const override;
+  LogicalSides GetLogicalSkipSides() const override;
 
   // row-specific methods
 

layout/tables/nsTableRowGroupFrame.cpp

@@ -257,8 +257,7 @@ void nsTableRowGroupFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
   DisplayRows(aBuilder, this, aLists);
 }
 
-nsIFrame::LogicalSides nsTableRowGroupFrame::GetLogicalSkipSides(
-    const Maybe<SkipSidesDuringReflow>&) const {
+LogicalSides nsTableRowGroupFrame::GetLogicalSkipSides() const {
   LogicalSides skip(mWritingMode);
   if (MOZ_UNLIKELY(StyleBorder()->mBoxDecorationBreak ==
                    StyleBoxDecorationBreak::Clone)) {

layout/tables/nsTableRowGroupFrame.h

@@ -328,8 +328,7 @@ class nsTableRowGroupFrame final : public nsContainerFrame,
   void InitChildReflowInput(nsPresContext& aPresContext, bool aBorderCollapse,
                             ReflowInput& aReflowInput);
 
-  LogicalSides GetLogicalSkipSides(
-      const Maybe<SkipSidesDuringReflow>&) const override;
+  LogicalSides GetLogicalSkipSides() const override;
 
   void PlaceChild(nsPresContext* aPresContext,
                   TableRowGroupReflowInput& aReflowInput, nsIFrame* aKidFrame,