[layoutng] Use the correct coordinate system for intrinsic sizes

When we get a child's intrinsic size we need to make sure to use the correct
writing mode. This patch only fixes the case where the child has a size
specified.

Makes a few more tests pass (see the test expectations change). Makes one test
fail, but that one passed more by coincidence than anything else.

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: Ia14edd6c85970ccb17c8735dbb3a307e657015a6
Reviewed-on: https://chromium-review.googlesource.com/1053885
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Aleks Totic <atotic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557715}

third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG

@@ -160,9 +160,7 @@ crbug.com/714962 css3/filters/effect-reference-zoom-hw.html [ Failure ]
 crbug.com/591099 css3/filters/multiple-references-id-mutate-crash-2.html [ Crash ]
 crbug.com/591099 css3/flexbox/bug646288.html [ Failure ]
 crbug.com/591099 css3/flexbox/bug669714.html [ Failure ]
-crbug.com/591099 css3/flexbox/flex-flow-border.html [ Failure ]
 crbug.com/591099 css3/flexbox/flex-flow-margins-auto-size.html [ Failure ]
-crbug.com/591099 css3/flexbox/flex-flow-padding.html [ Failure ]
 crbug.com/591099 css3/flexbox/flex-item-contains-strict.html [ Failure ]
 crbug.com/591099 css3/flexbox/flexbox-baseline.html [ Failure ]
 crbug.com/591099 css3/flexbox/flexbox-with-multi-column-property.html [ Failure ]
@@ -545,6 +543,7 @@ crbug.com/591099 external/wpt/css/css-writing-modes/inline-block-alignment-006.x
 crbug.com/591099 external/wpt/css/css-writing-modes/line-box-direction-vrl-009.xht [ Pass ]
 crbug.com/591099 external/wpt/css/css-writing-modes/margin-collapse-vrl-010.xht [ Failure ]
 crbug.com/591099 external/wpt/css/css-writing-modes/mongolian-orientation-002.html [ Pass ]
+crbug.com/591099 external/wpt/css/css-writing-modes/nested-orthogonal-001.html [ Failure ]
 crbug.com/591099 external/wpt/css/css-writing-modes/ortho-htb-alongside-vrl-floats-002.xht [ Failure ]
 crbug.com/591099 external/wpt/css/css-writing-modes/ortho-htb-alongside-vrl-floats-010.xht [ Failure ]
 crbug.com/591099 external/wpt/css/css-writing-modes/orthogonal-parent-shrink-to-fit-001a.html [ Failure ]
@@ -914,7 +913,6 @@ crbug.com/591099 fast/css-grid-layout/grid-self-baseline-02.html [ Failure ]
 crbug.com/591099 fast/css-grid-layout/grid-self-baseline-03.html [ Failure ]
 crbug.com/591099 fast/css-grid-layout/grid-self-baseline-04.html [ Failure ]
 crbug.com/591099 fast/css-grid-layout/grid-self-baseline-05.html [ Failure ]
-crbug.com/591099 fast/css-grid-layout/grid-self-baseline-06.html [ Failure ]
 crbug.com/591099 fast/css-grid-layout/grid-self-baseline-07.html [ Failure ]
 crbug.com/591099 fast/css-grid-layout/grid-self-baseline-horiz-04.html [ Failure ]
 crbug.com/591099 fast/css-grid-layout/grid-self-baseline-horiz-05.html [ Failure ]
@@ -1430,7 +1428,6 @@ crbug.com/714962 fast/writing-mode/border-image-vertical-rl.html [ Failure ]
 crbug.com/591099 fast/writing-mode/border-radius-clipping-vertical-lr.html [ Failure ]
 crbug.com/714962 fast/writing-mode/border-styles-vertical-lr.html [ Failure ]
 crbug.com/714962 fast/writing-mode/border-styles-vertical-rl.html [ Failure ]
-crbug.com/591099 fast/writing-mode/borders.html [ Failure ]
 crbug.com/591099 fast/writing-mode/english-rl-text.html [ Failure ]
 crbug.com/591099 fast/writing-mode/fieldsets.html [ Failure ]
 crbug.com/714962 fast/writing-mode/flipped-blocks-hit-test-line-edges.html [ Failure ]

third_party/blink/renderer/core/layout/ng/inline/ng_inline_node.cc

@@ -633,13 +633,14 @@ static LayoutUnit ComputeContentSize(NGInlineNode node,
       const ComputedStyle& float_style = float_node.Style();
 
       base::Optional<MinMaxSize> child_minmax;
-      if (NeedMinMaxSizeForContentContribution(float_style)) {
+      if (NeedMinMaxSizeForContentContribution(writing_mode, float_style)) {
         MinMaxSizeInput zero_input;  // Floats don't intrude into floats.
+        // TODO(layoutng): This is wrong for orthogonal writing modes.
         child_minmax = float_node.ComputeMinMaxSize(zero_input);
       }
 
-      MinMaxSize child_sizes =
-          ComputeMinAndMaxContentContribution(float_style, child_minmax);
+      MinMaxSize child_sizes = ComputeMinAndMaxContentContribution(
+          writing_mode, float_style, child_minmax);
       LayoutUnit child_inline_margins =
           ComputeMinMaxMargins(style, float_node).InlineSum();
 

third_party/blink/renderer/core/layout/ng/ng_block_layout_algorithm.cc

@@ -149,11 +149,14 @@ base::Optional<MinMaxSize> NGBlockLayoutAlgorithm::ComputeMinMaxSize(
       child_sizes = child.ComputeMinMaxSize(child_input);
     } else {
       base::Optional<MinMaxSize> child_minmax;
-      if (NeedMinMaxSizeForContentContribution(child_style))
+      if (NeedMinMaxSizeForContentContribution(Style().GetWritingMode(),
+                                               child_style)) {
+        // TODO(layoutng): This is wrong for orthogonal writing modes.
         child_minmax = child.ComputeMinMaxSize(child_input);
+      }
 
-      child_sizes =
-          ComputeMinAndMaxContentContribution(child_style, child_minmax);
+      child_sizes = ComputeMinAndMaxContentContribution(
+          Style().GetWritingMode(), child_style, child_minmax);
     }
 
     // Determine the max inline contribution of the child.

third_party/blink/renderer/core/layout/ng/ng_length_utils.cc

@@ -33,10 +33,14 @@ bool NeedMinMaxSize(const ComputedStyle& style) {
          style.LogicalMaxWidth().IsIntrinsic();
 }
 
-bool NeedMinMaxSizeForContentContribution(const ComputedStyle& style) {
-  return style.LogicalWidth().IsIntrinsicOrAuto() ||
-         style.LogicalMinWidth().IsIntrinsic() ||
-         style.LogicalMaxWidth().IsIntrinsic();
+bool NeedMinMaxSizeForContentContribution(WritingMode mode,
+                                          const ComputedStyle& style) {
+  if (mode == WritingMode::kHorizontalTb) {
+    return style.Width().IsIntrinsicOrAuto() ||
+           style.MinWidth().IsIntrinsic() || style.MaxWidth().IsIntrinsic();
+  }
+  return style.Height().IsIntrinsicOrAuto() ||
+         style.MinHeight().IsIntrinsic() || style.MaxHeight().IsIntrinsic();
 }
 
 LayoutUnit ResolveInlineLength(const NGConstraintSpace& constraint_space,
@@ -224,39 +228,68 @@ LayoutUnit ResolveMarginPaddingLength(const NGConstraintSpace& constraint_space,
 }
 
 MinMaxSize ComputeMinAndMaxContentContribution(
+    WritingMode writing_mode,
     const ComputedStyle& style,
     const base::Optional<MinMaxSize>& min_and_max) {
   // Synthesize a zero-sized constraint space for passing to
   // ResolveInlineLength.
-  WritingMode writing_mode = style.GetWritingMode();
+  // The constraint space's writing mode has to match the style, so we can't
+  // use the passed-in mode here.
   NGConstraintSpaceBuilder builder(
-      writing_mode,
+      style.GetWritingMode(),
       /* icb_size */ {NGSizeIndefinite, NGSizeIndefinite});
   scoped_refptr<NGConstraintSpace> space =
-      builder.ToConstraintSpace(writing_mode);
+      builder.ToConstraintSpace(style.GetWritingMode());
+
+  LayoutUnit content_size =
+      min_and_max ? min_and_max->max_size : NGSizeIndefinite;
 
   MinMaxSize computed_sizes;
-  Length inline_size = style.LogicalWidth();
+  Length inline_size = writing_mode == WritingMode::kHorizontalTb
+                           ? style.Width()
+                           : style.Height();
   if (inline_size.IsAuto()) {
     CHECK(min_and_max.has_value());
     computed_sizes = *min_and_max;
   } else {
-    computed_sizes.min_size = computed_sizes.max_size =
-        ResolveInlineLength(*space, style, min_and_max, inline_size,
-                            LengthResolveType::kContentSize);
+    if (IsParallelWritingMode(writing_mode, style.GetWritingMode())) {
+      computed_sizes.min_size = computed_sizes.max_size =
+          ResolveInlineLength(*space, style, min_and_max, inline_size,
+                              LengthResolveType::kContentSize);
+    } else {
+      computed_sizes.min_size = computed_sizes.max_size =
+          ResolveBlockLength(*space, style, inline_size, content_size,
+                             LengthResolveType::kContentSize);
+    }
   }
 
-  Length max_length = style.LogicalMaxWidth();
+  Length max_length = writing_mode == WritingMode::kHorizontalTb
+                          ? style.MaxWidth()
+                          : style.MaxHeight();
   if (!max_length.IsMaxSizeNone()) {
-    LayoutUnit max = ResolveInlineLength(*space, style, min_and_max, max_length,
-                                         LengthResolveType::kMaxSize);
+    LayoutUnit max;
+    if (IsParallelWritingMode(writing_mode, style.GetWritingMode())) {
+      max = ResolveInlineLength(*space, style, min_and_max, max_length,
+                                LengthResolveType::kMaxSize);
+    } else {
+      max = ResolveBlockLength(*space, style, max_length, content_size,
+                               LengthResolveType::kMaxSize);
+    }
     computed_sizes.min_size = std::min(computed_sizes.min_size, max);
     computed_sizes.max_size = std::min(computed_sizes.max_size, max);
   }
 
-  LayoutUnit min =
-      ResolveInlineLength(*space, style, min_and_max, style.LogicalMinWidth(),
-                          LengthResolveType::kMinSize);
+  Length min_length = writing_mode == WritingMode::kHorizontalTb
+                          ? style.MinWidth()
+                          : style.MinHeight();
+  LayoutUnit min;
+  if (IsParallelWritingMode(writing_mode, style.GetWritingMode())) {
+    min = ResolveInlineLength(*space, style, min_and_max, min_length,
+                              LengthResolveType::kMinSize);
+  } else {
+    min = ResolveBlockLength(*space, style, min_length, content_size,
+                             LengthResolveType::kMinSize);
+  }
   computed_sizes.min_size = std::max(computed_sizes.min_size, min);
   computed_sizes.max_size = std::max(computed_sizes.max_size, min);
 

third_party/blink/renderer/core/layout/ng/ng_length_utils.h

@@ -33,7 +33,11 @@ CORE_EXPORT bool NeedMinMaxSize(const ComputedStyle&);
 
 // Like NeedMinMaxSize, but for use when calling
 // ComputeMinAndMaxContentContribution.
-CORE_EXPORT bool NeedMinMaxSizeForContentContribution(const ComputedStyle&);
+// Because content contributions are commonly needed by a block's parent,
+// we also take a writing mode here so we can check this in the parent's
+// coordinate system.
+CORE_EXPORT bool NeedMinMaxSizeForContentContribution(WritingMode mode,
+                                                      const ComputedStyle&);
 
 // Convert an inline-axis length to a layout unit using the given constraint
 // space.
@@ -63,8 +67,12 @@ CORE_EXPORT LayoutUnit ResolveMarginPaddingLength(const NGConstraintSpace&,
 // to zero) and that an auto inline size resolves to the respective min/max
 // content size.
 // Also, the min/max contribution does include the inline margins as well.
+// Because content contributions are commonly needed by a block's parent,
+// we also take a writing mode here so we can compute this in the parent's
+// coordinate system.
 CORE_EXPORT MinMaxSize
-ComputeMinAndMaxContentContribution(const ComputedStyle&,
+ComputeMinAndMaxContentContribution(WritingMode writing_mode,
+                                    const ComputedStyle&,
                                     const base::Optional<MinMaxSize>&);
 
 // Resolves the given length to a layout unit, constraining it by the min

third_party/blink/renderer/core/layout/ng/ng_length_utils_test.cc

@@ -132,41 +132,48 @@ TEST_F(NGLengthUtilsTest, testComputeContentContribution) {
 
   MinMaxSize expected{LayoutUnit(), LayoutUnit()};
   style_->SetLogicalWidth(Length(30, kPercent));
-  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(*style_, sizes));
+  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(
+                          style_->GetWritingMode(), *style_, sizes));
 
   style_->SetLogicalWidth(Length(kFillAvailable));
-  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(*style_, sizes));
+  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(
+                          style_->GetWritingMode(), *style_, sizes));
 
   expected = MinMaxSize{LayoutUnit(150), LayoutUnit(150)};
   style_->SetLogicalWidth(Length(150, kFixed));
-  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(*style_, sizes));
+  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(
+                          style_->GetWritingMode(), *style_, sizes));
 
   expected = sizes;
   style_->SetLogicalWidth(Length(kAuto));
-  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(*style_, sizes));
+  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(
+                          style_->GetWritingMode(), *style_, sizes));
 
   expected = MinMaxSize{LayoutUnit(430), LayoutUnit(440)};
   style_->SetPaddingLeft(Length(400, kFixed));
   auto sizes_padding400 = sizes;
   sizes_padding400 += LayoutUnit(400);
-  EXPECT_EQ(expected,
-            ComputeMinAndMaxContentContribution(*style_, sizes_padding400));
+  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(
+                          style_->GetWritingMode(), *style_, sizes_padding400));
 
   expected = MinMaxSize{LayoutUnit(100), LayoutUnit(100)};
   style_->SetPaddingLeft(Length(0, kFixed));
   style_->SetLogicalWidth(Length(CalculationValue::Create(
       PixelsAndPercent(100, -10), kValueRangeNonNegative)));
-  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(*style_, sizes));
+  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(
+                          style_->GetWritingMode(), *style_, sizes));
 
   expected = MinMaxSize{LayoutUnit(30), LayoutUnit(35)};
   style_->SetLogicalWidth(Length(kAuto));
   style_->SetMaxWidth(Length(35, kFixed));
-  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(*style_, sizes));
+  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(
+                          style_->GetWritingMode(), *style_, sizes));
 
   expected = MinMaxSize{LayoutUnit(80), LayoutUnit(80)};
   style_->SetLogicalWidth(Length(50, kFixed));
   style_->SetMinWidth(Length(80, kFixed));
-  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(*style_, sizes));
+  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(
+                          style_->GetWritingMode(), *style_, sizes));
 
   expected = MinMaxSize{LayoutUnit(150), LayoutUnit(150)};
   style_ = ComputedStyle::Create();
@@ -174,35 +181,36 @@ TEST_F(NGLengthUtilsTest, testComputeContentContribution) {
   style_->SetPaddingLeft(Length(50, kFixed));
   auto sizes_padding50 = sizes;
   sizes_padding50 += LayoutUnit(50);
-  EXPECT_EQ(expected,
-            ComputeMinAndMaxContentContribution(*style_, sizes_padding50));
+  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(
+                          style_->GetWritingMode(), *style_, sizes_padding50));
 
   expected = MinMaxSize{LayoutUnit(100), LayoutUnit(100)};
   style_->SetBoxSizing(EBoxSizing::kBorderBox);
-  EXPECT_EQ(expected,
-            ComputeMinAndMaxContentContribution(*style_, sizes_padding50));
+  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(
+                          style_->GetWritingMode(), *style_, sizes_padding50));
 
   // Content size should never be below zero, even with box-sizing: border-box
   // and a large padding...
   expected = MinMaxSize{LayoutUnit(400), LayoutUnit(400)};
   style_->SetPaddingLeft(Length(400, kFixed));
-  EXPECT_EQ(expected,
-            ComputeMinAndMaxContentContribution(*style_, sizes_padding400));
+  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(
+                          style_->GetWritingMode(), *style_, sizes_padding400));
 
   expected.min_size = expected.max_size = sizes.min_size + LayoutUnit(400);
   style_->SetLogicalWidth(Length(kMinContent));
-  EXPECT_EQ(expected,
-            ComputeMinAndMaxContentContribution(*style_, sizes_padding400));
+  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(
+                          style_->GetWritingMode(), *style_, sizes_padding400));
   style_->SetLogicalWidth(Length(100, kFixed));
   style_->SetMaxWidth(Length(kMaxContent));
   // Due to padding and box-sizing, width computes to 400px and max-width to
   // 440px, so the result is 400.
   expected = MinMaxSize{LayoutUnit(400), LayoutUnit(400)};
-  EXPECT_EQ(expected,
-            ComputeMinAndMaxContentContribution(*style_, sizes_padding400));
+  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(
+                          style_->GetWritingMode(), *style_, sizes_padding400));
   expected = MinMaxSize{LayoutUnit(40), LayoutUnit(40)};
   style_->SetPaddingLeft(Length(0, kFixed));
-  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(*style_, sizes));
+  EXPECT_EQ(expected, ComputeMinAndMaxContentContribution(
+                          style_->GetWritingMode(), *style_, sizes));
 }
 
 TEST_F(NGLengthUtilsTest, testComputeInlineSizeForFragment) {