[LayoutNG] Legend break inside avoid

If the legend sets break-inside to avoid, and we can break before the
fieldset, we should do so if it is a more appealing break point.

This, however, was not the case because the fieldset content, if empty,
would get laid out and override the break appeal of the builder,
which is why the fieldset did not know to break before.

This change fixes this issue by avoiding laying out the fieldset content
if we've run out of space from laying out the legend.

Bug: 875235
Change-Id: I2db2554fb1032d8297c6df954d9d8f6194b041d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2116834
Commit-Queue: Alison Maher <almaher@microsoft.com>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753279}

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

@@ -199,9 +199,17 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutChildren() {
   // all live inside an anonymous child box of the fieldset container.
   auto fieldset_content = Node().GetFieldsetContent();
   if (fieldset_content && (content_break_token || !has_seen_all_children)) {
-    NGBreakStatus break_status =
-        LayoutFieldsetContent(fieldset_content, content_break_token,
-                              adjusted_padding_box_size, !!legend);
+    LayoutUnit fragmentainer_block_offset;
+    if (ConstraintSpace().HasBlockFragmentation()) {
+      fragmentainer_block_offset =
+          ConstraintSpace().FragmentainerOffsetAtBfc() + intrinsic_block_size_;
+      if (legend_broke_ &&
+          IsFragmentainerOutOfSpace(fragmentainer_block_offset))
+        return NGBreakStatus::kContinue;
+    }
+    NGBreakStatus break_status = LayoutFieldsetContent(
+        fieldset_content, content_break_token, adjusted_padding_box_size,
+        fragmentainer_block_offset, !!legend);
     if (break_status == NGBreakStatus::kNeedsEarlierBreak)
       return break_status;
   }
@@ -257,6 +265,7 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutLegend(
     }
 
     const auto& physical_fragment = result->PhysicalFragment();
+    legend_broke_ = physical_fragment.BreakToken();
 
     // We have already adjusted the legend block offset, no need to adjust
     // again.
@@ -305,6 +314,7 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutFieldsetContent(
     NGBlockNode& fieldset_content,
     scoped_refptr<const NGBlockBreakToken> content_break_token,
     LogicalSize adjusted_padding_box_size,
+    LayoutUnit fragmentainer_block_offset,
     bool has_legend) {
   auto child_space = CreateConstraintSpaceForFieldsetContent(
       fieldset_content, adjusted_padding_box_size,
@@ -316,11 +326,10 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutFieldsetContent(
 
   NGBreakStatus break_status = NGBreakStatus::kContinue;
   if (ConstraintSpace().HasBlockFragmentation()) {
-    LayoutUnit block_offset =
-        ConstraintSpace().FragmentainerOffsetAtBfc() + intrinsic_block_size_;
     // TODO(almaher): The legend should be treated as out-of-flow.
     break_status = BreakBeforeChildIfNeeded(
-        ConstraintSpace(), fieldset_content, *result.get(), block_offset,
+        ConstraintSpace(), fieldset_content, *result.get(),
+        fragmentainer_block_offset,
         /*has_container_separation*/ has_legend, &container_builder_);
     EBreakBetween break_after = JoinFragmentainerBreakValues(
         result->FinalBreakAfter(), fieldset_content.Style().BreakAfter());
@@ -337,6 +346,13 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutFieldsetContent(
   return break_status;
 }
 
+bool NGFieldsetLayoutAlgorithm::IsFragmentainerOutOfSpace(
+    LayoutUnit block_offset) const {
+  if (!ConstraintSpace().HasKnownFragmentainerBlockSize())
+    return false;
+  return block_offset >= FragmentainerSpaceAtBfcStart(ConstraintSpace());
+}
+
 base::Optional<MinMaxSizes> NGFieldsetLayoutAlgorithm::ComputeMinMaxSizes(
     const MinMaxSizesInput& input) const {
   MinMaxSizes sizes;

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

@@ -37,6 +37,7 @@ class CORE_EXPORT NGFieldsetLayoutAlgorithm
       NGBlockNode& fieldset_content,
       scoped_refptr<const NGBlockBreakToken> content_break_token,
       LogicalSize adjusted_padding_box_size,
+      LayoutUnit fragmentainer_block_offset,
       bool has_legend);
 
   const NGConstraintSpace CreateConstraintSpaceForLegend(
@@ -49,6 +50,8 @@ class CORE_EXPORT NGFieldsetLayoutAlgorithm
       LogicalSize padding_box_size,
       LayoutUnit block_offset);
 
+  bool IsFragmentainerOutOfSpace(LayoutUnit block_offset) const;
+
   const WritingMode writing_mode_;
 
   const NGBoxStrut border_padding_;
@@ -75,6 +78,10 @@ class CORE_EXPORT NGFieldsetLayoutAlgorithm
   // If true, this indicates the block_start_padding_edge_ had changed from its
   // initial value during the current layout pass.
   bool block_start_padding_edge_adjusted_ = false;
+
+  // If true, this indicates that the legend broke during the current layout
+  // pass.
+  bool legend_broke_ = false;
 };
 
 }  // namespace blink

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

@@ -1302,7 +1302,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, BreakInsideAvoid) {
      <fieldset id="fieldset">
       <legend id="legend" style="width:10px; height:50px;"></legend>
       <div style="break-inside:avoid; width:20px; height:70px;"></div>
-    </fieldsest>
+    </fieldset>
   )HTML");
 
   LayoutUnit kFragmentainerSpaceAvailable(100);
@@ -1354,7 +1354,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, BreakInsideAvoidTallBlock) {
      <fieldset id="fieldset">
       <legend id="legend" style="width:10px; height:50px;"></legend>
       <div style="break-inside:avoid; width:20px; height:170px;"></div>
-    </fieldsest>
+    </fieldset>
   )HTML");
 
   LayoutUnit kFragmentainerSpaceAvailable(100);
@@ -1416,7 +1416,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendBreakInsideAvoid) {
       <fieldset id="fieldset">
         <legend id="legend" style="break-inside:avoid; width:10px; height:60px;">
         </legend>
-      </fieldsest>
+      </fieldset>
     </div>
   )HTML");
 
@@ -1432,15 +1432,10 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendBreakInsideAvoid) {
       NGBaseLayoutAlgorithmTest::RunBlockLayoutAlgorithm(node, space);
   ASSERT_FALSE(fragment->BreakToken()->IsFinished());
 
-  // TODO(almaher): Breaking before the fieldset has higher appeal than
-  // breaking inside the legend
   String dump = DumpFragmentTree(fragment.get());
   String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
   offset:unplaced size:1000x100
     offset:0,0 size:20x50
-    offset:0,50 size:100x50
-      offset:0,0 size:10x50
-      offset:0,50 size:100x0
 )DUMP";
   EXPECT_EQ(expectation, dump);
 
@@ -1450,9 +1445,10 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendBreakInsideAvoid) {
 
   dump = DumpFragmentTree(fragment.get());
   expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
-  offset:unplaced size:1000x10
-    offset:0,0 size:100x10
-      offset:0,0 size:10x10
+  offset:unplaced size:1000x60
+    offset:0,0 size:100x60
+      offset:0,0 size:10x60
+      offset:0,60 size:100x0
 )DUMP";
   EXPECT_EQ(expectation, dump);
 }
@@ -1473,7 +1469,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, BreakBeforeAvoid) {
         <legend id="legend" style="width:10px; height:25px;"></legend>
         <div style="width:30px; height:25px;"></div>
         <div style="break-before:avoid; width:15px; height:25px;"></div>
-      </fieldsest>
+      </fieldset>
     </div>
   )HTML");
 
@@ -1527,7 +1523,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendBreakBeforeAvoid) {
       <div style="width:20px; height:90px;"></div>
       <fieldset id="fieldset">
         <legend id="legend" style="break-before:avoid;"></legend>
-      </fieldsest>
+      </fieldset>
     </div>
   )HTML");
 
@@ -1580,7 +1576,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, BreakAfterAvoid) {
         <legend id="legend" style="width:10px; height:25px;"></legend>
         <div style="break-after:avoid; width:30px; height:25px;"></div>
         <div style="width:15px; height:25px;"></div>
-      </fieldsest>
+      </fieldset>
     </div>
   )HTML");
 
@@ -1635,7 +1631,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendBreakAfterAvoid) {
       <fieldset id="fieldset">
         <legend id="legend" style="break-after:avoid;"></legend>
         <div style="width:15px; height:25px;"></div>
-      </fieldsest>
+      </fieldset>
     </div>
   )HTML");
 
@@ -1690,7 +1686,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, MarginTopPastEndOfFragmentainer) {
      <fieldset id="fieldset">
       <legend id="legend" style="margin-top:60px; width:10px; height:20px;"></legend>
       <div style="width:20px; height:20px;"></div>
-    </fieldsest>
+    </fieldset>
   )HTML");
 
   LayoutUnit kFragmentainerSpaceAvailable(50);
@@ -1742,7 +1738,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, MarginBottomPastEndOfFragmentainer) {
      <fieldset id="fieldset">
       <legend id="legend" style="margin-bottom:20px; height:90px;"></legend>
       <div style="width:20px; height:20px;"></div>
-    </fieldsest>
+    </fieldset>
   )HTML");
 
   LayoutUnit kFragmentainerSpaceAvailable(100);