[LayoutNG] Don't break fieldset border/padding

The fieldset layout algorithm was able to handle breaks inside borders
and padding, but since this is no longer supported, some code had to be
removed. This change addresses the TODO that was proposed in
https://chromium-review.googlesource.com/c/chromium/src/+/2263353

In a previous change where we had made the legends monolithic
https://chromium-review.googlesource.com/c/chromium/src/+/2353354
additional TODOs were added as a result of the following open issue
crbug.com/1097012. In this change, a missing TODO was added and comments
have been updated.

Bug: 1097012
Change-Id: I2087957b98681904606a4d6d780094c24a8e7ece
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2367698
Commit-Queue: Ana Sollano Kim <ansollan@microsoft.com>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Alison Maher <almaher@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#800756}

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

@@ -83,17 +83,8 @@ scoped_refptr<const NGLayoutResult> NGFieldsetLayoutAlgorithm::Layout() {
   // scrollbars are handled by the anonymous child box, and since padding is
   // inside the scrollport, padding also needs to be handled by the anonymous
   // child.
-
-  // Calculate the amount of the border block-start that was consumed in
-  // previous fragments.
-  //
-  // TODO(layout-dev): The fieldset algorithm tries to handle fragmentation
-  // inside borders and padding, but this cannot happen anymore. Unless we want
-  // to re-introduce breaks inside borders and padding, we should remove the
-  // code that's only needed to support this.
-  consumed_border_block_start_ =
-      std::min(consumed_block_size_, borders_.block_start);
-  intrinsic_block_size_ = borders_.block_start - consumed_border_block_start_;
+  intrinsic_block_size_ =
+      IsResumingLayout(BreakToken()) ? LayoutUnit() : borders_.block_start;
 
   NGBreakStatus break_status = LayoutChildren();
   if (break_status == NGBreakStatus::kNeedsEarlierBreak) {
@@ -177,43 +168,38 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutChildren() {
     }
   }
 
-  NGBoxStrut borders_with_legend = borders_;
-  borders_with_legend.block_start = intrinsic_block_size_;
   LogicalSize adjusted_padding_box_size =
-      ShrinkLogicalSize(border_box_size_, borders_with_legend);
-
-  if (adjusted_padding_box_size.block_size != kIndefiniteSize) {
-    // If intrinsic_block_size_ does not include the border block-start that was
-    // consumed in previous fragments, exclude consumed_border_block_start_ from
-    // adjusted_padding_box_size, as well.
-    if (consumed_border_block_start_ > LayoutUnit())
-      adjusted_padding_box_size.block_size -= consumed_border_block_start_;
-
-    // If the legend has been laid out in previous fragments,
-    // adjusted_padding_box_size will need to be adjusted further to account for
-    // block size taken up by the legend.
-    if (legend) {
-      LayoutUnit content_consumed_block_size =
-          content_break_token ? content_break_token->ConsumedBlockSize()
-                              : LayoutUnit();
-
-      // Calculate the amount of the border block-end that was consumed in
-      // previous fragments.
-      DCHECK_NE(border_box_size_.block_size, kIndefiniteSize);
-      LayoutUnit consumed_border_block_end =
-          std::max(consumed_block_size_ -
-                       (border_box_size_.block_size - borders_.block_end),
-                   LayoutUnit());
-
-      LayoutUnit legend_block_size =
-          consumed_block_size_ - content_consumed_block_size -
-          consumed_border_block_start_ - consumed_border_block_end;
-      DCHECK_GE(legend_block_size, LayoutUnit());
-
-      adjusted_padding_box_size.block_size =
-          std::max(padding_.BlockSum(),
-                   adjusted_padding_box_size.block_size - legend_block_size);
-    }
+      ShrinkLogicalSize(border_box_size_, borders_);
+
+  // If the legend has been laid out in previous fragments,
+  // adjusted_padding_box_size will need to be adjusted further to account for
+  // block size taken up by the legend.
+  if (adjusted_padding_box_size.block_size != kIndefiniteSize && legend) {
+    LayoutUnit content_consumed_block_size =
+        content_break_token ? content_break_token->ConsumedBlockSize()
+                            : LayoutUnit();
+
+    // Calculate the amount of the border block-start that was consumed in
+    // previous fragments.
+    LayoutUnit consumed_border_block_start =
+        borders_.block_start - intrinsic_block_size_;
+
+    // Calculate the amount of the border block-end that was consumed in
+    // previous fragments.
+    DCHECK_NE(border_box_size_.block_size, kIndefiniteSize);
+    LayoutUnit consumed_border_block_end =
+        std::max(consumed_block_size_ -
+                     (border_box_size_.block_size - borders_.block_end),
+                 LayoutUnit());
+
+    LayoutUnit legend_block_size =
+        consumed_block_size_ - content_consumed_block_size -
+        consumed_border_block_start - consumed_border_block_end;
+    DCHECK_GE(legend_block_size, LayoutUnit());
+
+    adjusted_padding_box_size.block_size =
+        std::max(padding_.BlockSum(),
+                 adjusted_padding_box_size.block_size - legend_block_size);
   }
 
   // Proceed with normal fieldset children (excluding the rendered legend). They
@@ -247,7 +233,6 @@ void NGFieldsetLayoutAlgorithm::LayoutLegend(NGBlockNode& legend) {
       legend.Style(), percentage_size.inline_size,
       ConstraintSpace().GetWritingMode(), ConstraintSpace().Direction());
 
-  LayoutUnit block_offset;
   auto legend_space = CreateConstraintSpaceForLegend(
       legend, ChildAvailableSize(), percentage_size);
   scoped_refptr<const NGLayoutResult> result =
@@ -265,6 +250,7 @@ void NGFieldsetLayoutAlgorithm::LayoutLegend(NGBlockNode& legend) {
                                             legend_margins.block_end;
 
   LayoutUnit space_left = borders_.block_start - legend_border_box_block_size;
+  LayoutUnit block_offset;
   if (space_left > LayoutUnit()) {
     // https://html.spec.whatwg.org/C/#the-fieldset-and-legend-elements
     // * The element is expected to be positioned in the block-flow direction

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

@@ -68,10 +68,6 @@ class CORE_EXPORT NGFieldsetLayoutAlgorithm
   // the legend.
   LayoutUnit minimum_border_box_block_size_;
 
-  // The amount of the border block-start that was consumed in previous
-  // fragments.
-  LayoutUnit consumed_border_block_start_;
-
   // If true, the legend is taller than the block-start border, so that it
   // sticks below it, allowing for a class C breakpoint [1] before any fieldset
   // content.

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

@@ -1005,7 +1005,11 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendAndContentFragmentation) {
   fragment = NGBaseLayoutAlgorithmTest::RunFieldsetLayoutAlgorithm(
       node, space, fragment->BreakToken());
   ASSERT_TRUE(fragment->BreakToken());
+
   dump = DumpFragmentTree(fragment.get());
+  // TODO(crbug.com/1097012): The height of the outermost fragment here should
+  // be 23, not 0, but the fragmentation machinery gets confused by the
+  // fieldset padding.
   expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
   offset:unplaced size:176x0
     offset:3,0 size:170x20
@@ -1684,12 +1688,8 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, MarginBottomPastEndOfFragmentainer) {
 }
 
 // Tests that a fieldset with a large border and a small legend fragment
-// correctly. In this case, the legend block offset is not adjusted because the
-// legend is broken across multiple fragments. Since we don't allow breaking
-// inside borders, they will overflow fragmentainers.
-// TODO(layout-dev): Allowing breaks inside a legend when not allowing breaks
-// inside borders seems inconsistent. Consider making legends monolithic, unless
-// they establish a parallel flow.
+// correctly. Since we don't allow breaking inside borders, they will overflow
+// fragmentainers.
 TEST_F(NGFieldsetLayoutAlgorithmTest, SmallLegendLargeBorderFragmentation) {
   SetBodyInnerHTML(R"HTML(
     <style>
@@ -1854,8 +1854,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, SmallerLegendLargeBorderFragmentation2) {
 }
 
 // Tests that a fieldset with a large border and a small legend fragment
-// correctly. In this case, the legend block offset is not adjusted because the
-// legend breaks after attempting to adjust the offset.
+// correctly.
 TEST_F(NGFieldsetLayoutAlgorithmTest, SmallerLegendLargeBorderWithBreak) {
   SetBodyInnerHTML(R"HTML(
     <style>