Bug 1620749 - Adjust FinishReflowChild()'s relative positioning conversion, to make it a writing-mode-agnostic API. r=dholbert

Currently, when using ReflowChildFlags::ApplyRelativePositioning flag in
FinishReflowChild(), we (implicitly) force the callers to pass aWM and
aPos in the same writing-mode as the reflow input, i.e. the child's
writing-mode. This results in an unintuitive LogicalPoint conversion
`mContainerSize - mMetrics.PhysicalSize()` in
nsBlockReflowContext::PlaceBlock().

We should allow the callers to use their preferred aWM and aPos (either
in parent's wm or child's wm), and do the necessary conversion in
FinishReflowChild() (like we did to convert aDesiredSize from
ReflowOutput's wm to aWM).

Also, correct the documentation for aWM in ReflowChild() and
FinishReflowChild(). aWM doesn't need to be the containing frame's
writing-mode. It can be any as long as it is the writing-mode that aPos
is used.

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

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

layout/generic/nsBlockReflowContext.cpp

@@ -429,16 +429,11 @@ bool nsBlockReflowContext::PlaceBlock(const ReflowInput& aReflowInput,
                    mMetrics.ISize(mWritingMode), mMetrics.BSize(mWritingMode),
                    mContainerSize);
 
-  WritingMode frameWM = mFrame->GetWritingMode();
-  LogicalPoint logPos =
-      LogicalPoint(mWritingMode, mICoord, mBCoord)
-          .ConvertTo(frameWM, mWritingMode,
-                     mContainerSize - mMetrics.PhysicalSize());
-
   // Now place the frame and complete the reflow process
   nsContainerFrame::FinishReflowChild(
-      mFrame, mPresContext, mMetrics, &aReflowInput, frameWM, logPos,
-      mContainerSize, nsIFrame::ReflowChildFlags::ApplyRelativePositioning);
+      mFrame, mPresContext, mMetrics, &aReflowInput, mWritingMode,
+      LogicalPoint(mWritingMode, mICoord, mBCoord), mContainerSize,
+      nsIFrame::ReflowChildFlags::ApplyRelativePositioning);
 
   aOverflowAreas = mMetrics.mOverflowAreas + mFrame->GetPosition();
 

layout/generic/nsContainerFrame.cpp

@@ -1026,7 +1026,12 @@ void nsContainerFrame::FinishReflowChild(
     // ApplyRelativePositioning in right-to-left writing modes needs to know
     // the updated frame width to set the normal position correctly.
     aKidFrame->SetSize(aWM, convertedSize);
-    aReflowInput->ApplyRelativePositioning(&pos, aContainerSize);
+
+    const LogicalMargin offsets =
+        aReflowInput->ComputedLogicalOffsets().ConvertTo(
+            aWM, aReflowInput->GetWritingMode());
+    ReflowInput::ApplyRelativePositioning(aKidFrame, aWM, offsets, &pos,
+                                          aContainerSize);
   }
 
   if (ReflowChildFlags::NoMoveFrame !=

layout/generic/nsContainerFrame.h

@@ -209,8 +209,9 @@ class nsContainerFrame : public nsSplittableFrame {
    * If the reflow status after reflowing the child is FullyComplete then any
    * next-in-flows are deleted using DeleteNextInFlowChild().
    *
-   * @param aWM Containing frame's writing-mode.
-   * @param aPos Position of the aKidFrame to be moved.
+   * @param aReflowInput the reflow input for aKidFrame.
+   * @param aWM aPos's writing-mode (any writing mode will do).
+   * @param aPos Position of the aKidFrame to be moved, in terms of aWM.
    * @param aContainerSize Size of the border-box of the containing frame.
    *
    * Note: If ReflowChildFlags::NoMoveFrame is requested, both aPos and
@@ -234,8 +235,9 @@ class nsContainerFrame : public nsSplittableFrame {
    * - sets the view's visibility, opacity, content transparency, and clip
    * - invoked the DidReflow() function
    *
-   * @param aWM Containing frame's writing-mode.
-   * @param aPos Position of the aKidFrame to be positioned.
+   * @param aReflowInput the reflow input for aKidFrame.
+   * @param aWM aPos's writing-mode (any writing mode will do).
+   * @param aPos Position of the aKidFrame to be moved, in terms of aWM.
    * @param aContainerSize Size of the border-box of the containing frame.
    *
    * Note: If ReflowChildFlags::NoMoveFrame is requested, both aPos and

layout/generic/nsFlexContainerFrame.cpp

@@ -5108,29 +5108,9 @@ void nsFlexContainerFrame::ReflowFlexItem(
              "We gave flex item unconstrained available height, so it "
              "should be complete");
 
-  // ApplyRelativePositioning in right-to-left writing modes needs to know the
-  // updated frame width to set the normal position correctly.
-  //
-  // It may look like we could handle this instead by passing the
-  // ApplyRelativePositioning flag to FinishReflowChild.  However, we're
-  // unlike other callers of FinishReflowChild in that we're keeping its aPos
-  // (our aFramePos) in the parent's writing mode rather than the child's, and
-  // thus passing its aWM (our outerWM) as the parent's writing mode as well.
-  //
-  // Thus this could be converted, but requires a little bit of care to do so
-  // (and would probably require a point conversion like the one in
-  // nsBlockReflowContext::PlaceBlock).  Alternatively, maybe things should be
-  // restructured a bit so that sort of conversion isn't needed.
-  aItem.Frame()->SetSize(outerWM,
-                         childDesiredSize.Size(wm).ConvertTo(outerWM, wm));
-  LogicalMargin offsets =
-      childReflowInput.ComputedLogicalOffsets().ConvertTo(outerWM, wm);
-  ReflowInput::ApplyRelativePositioning(aItem.Frame(), outerWM, offsets,
-                                        &aFramePos, aContainerSize);
-
   FinishReflowChild(aItem.Frame(), PresContext(), childDesiredSize,
                     &childReflowInput, outerWM, aFramePos, aContainerSize,
-                    ReflowChildFlags::Default);
+                    ReflowChildFlags::ApplyRelativePositioning);
 
   aItem.SetAscent(childDesiredSize.BlockStartAscent());
 }