[TablesNG] Make LayoutTableTest* unit tests pass

Small change to ComputeGridMinMax was necessary.
0% width cells are treated as "no percentage".
This is a common pattern in table algorithms:
"Zeroes are special!"

The fix for LayoutNGTable::AddVisualEffectOverflow was
good. It made 11 more invalidation tests pass.
The old visualoverflow was too large.

Bug: 958381
Change-Id: Ie6eec7f749ebd4400674e581f1dc128e4c3f81ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2551540Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Aleks Totic <atotic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829888}

third_party/blink/renderer/core/layout/layout_table_test.cc

@@ -80,9 +80,16 @@ TEST_F(LayoutTableTest, OverflowWithCollapsedBorders) {
                                             LayoutUnit(0), LayoutUnit(10));
   EXPECT_EQ(expected_self_visual_overflow,
             table->PhysicalSelfVisualOverflowRect());
-  // For this table, its layout overflow equals self visual overflow.
-  EXPECT_EQ(expected_self_visual_overflow, table->PhysicalLayoutOverflowRect());
-
+  if (RuntimeEnabledFeatures::LayoutNGTableEnabled()) {
+    EXPECT_EQ(table->PhysicalContentBoxRect(),
+              table->PhysicalLayoutOverflowRect());
+  } else {
+    // In Legacy, visual overflow incorrectly does not include borders
+    // that extend beyond table boundaries.
+    // For this table, its layout overflow equals self visual overflow.
+    EXPECT_EQ(expected_self_visual_overflow,
+              table->PhysicalLayoutOverflowRect());
+  }
   // The table's visual overflow covers self visual overflow and content visual
   // overflows.
   auto expected_visual_overflow = table->PhysicalContentBoxRect();
@@ -133,14 +140,25 @@ TEST_F(LayoutTableTest, CollapsedBorders) {
 
   // Cells have wider borders.
   auto* table3 = GetTableByElementId("table3");
-  // Cell E's border-top won.
-  EXPECT_EQ(7, table3->BorderBefore());
-  // Cell H's border-bottom won.
-  EXPECT_EQ(20, table3->BorderAfter());
-  // Cell E's border-left won.
-  EXPECT_EQ(10, table3->BorderStart());
-  // Cell F's border-bottom won.
-  EXPECT_EQ(13, table3->BorderEnd());
+  if (RuntimeEnabledFeatures::LayoutNGTableEnabled()) {
+    // Cell E's border-top won.
+    EXPECT_EQ(LayoutUnit(7.5), table3->BorderBefore());
+    // Cell H's border-bottom won.
+    EXPECT_EQ(20, table3->BorderAfter());
+    // Cell E's border-left won.
+    EXPECT_EQ(LayoutUnit(10.5), table3->BorderStart());
+    // Cell F's border-bottom won.
+    EXPECT_EQ(LayoutUnit(12.5), table3->BorderEnd());
+  } else {
+    // Cell E's border-top won.
+    EXPECT_EQ(7, table3->BorderBefore());
+    // Cell H's border-bottom won.
+    EXPECT_EQ(20, table3->BorderAfter());
+    // Cell E's border-left won.
+    EXPECT_EQ(10, table3->BorderStart());
+    // Cell F's border-bottom won.
+    EXPECT_EQ(13, table3->BorderEnd());
+  }
 }
 
 TEST_F(LayoutTableTest, CollapsedBordersWithCol) {
@@ -224,7 +242,11 @@ TEST_F(LayoutTableTest, WidthPercentagesExceedHundred) {
 
   // Table width should be TableLayoutAlgorithm::kMaxTableWidth
   auto* table = GetTableByElementId("onlyTable");
-  EXPECT_EQ(1000000, table->OffsetWidth());
+  // TablesNG will grow up to LayoutUnit::Max()
+  if (RuntimeEnabledFeatures::LayoutNGTableEnabled())
+    EXPECT_EQ(2000000, table->OffsetWidth());
+  else
+    EXPECT_EQ(1000000, table->OffsetWidth());
 }
 
 TEST_F(LayoutTableTest, CloseToMaxWidth) {

third_party/blink/renderer/core/layout/ng/table/layout_ng_table.cc

@@ -225,17 +225,23 @@ void LayoutNGTable::AddVisualEffectOverflow() {
   if (const NGPhysicalBoxFragment* fragment = GetPhysicalFragment(0)) {
     DCHECK_EQ(PhysicalFragmentCount(), 1u);
     // Table's collapsed borders contribute to visual overflow.
-    // Table border side can be composed of multiple border segments.
-    // Inline visual overflow uses size of the largest border segment.
-    // Block visual overflow uses size of first border segment.
+    // In the inline direction, table's border box does not include
+    // visual border width (largest border), but does include
+    // layout border width (border of first cell).
+    // Expands border box to include visual border width.
     if (const NGTableBorders* collapsed_borders =
             fragment->TableCollapsedBorders()) {
       PhysicalRect borders_overflow = PhysicalBorderBoxRect();
       NGBoxStrut table_borders = collapsed_borders->TableBorder();
       auto visual_inline_strut =
           collapsed_borders->GetCollapsedBorderVisualInlineStrut();
-      table_borders.inline_start = visual_inline_strut.first;
-      table_borders.inline_end = visual_inline_strut.second;
+      // Expand by difference between visual and layout border width.
+      table_borders.inline_start =
+          visual_inline_strut.first - table_borders.inline_start;
+      table_borders.inline_end =
+          visual_inline_strut.second - table_borders.inline_end;
+      table_borders.block_start = LayoutUnit();
+      table_borders.block_end = LayoutUnit();
       borders_overflow.Expand(
           table_borders.ConvertToPhysical(StyleRef().GetWritingDirection()));
       AddSelfVisualOverflow(borders_overflow);

third_party/blink/renderer/core/layout/ng/table/layout_ng_table.h

@@ -108,6 +108,10 @@ class CORE_EXPORT LayoutNGTable : public LayoutNGMixin<LayoutBlock>,
 
   LayoutRectOutsets BorderBoxOutsets() const override;
 
+  // TODO(1151101)
+  // ClientLeft/Top are incorrect for tables, but cannot be fixed
+  // by subclassing ClientLeft/Top.
+
   PhysicalRect OverflowClipRect(const PhysicalOffset&,
                                 OverlayScrollbarClipBehavior) const override;
 
@@ -177,8 +181,8 @@ class CORE_EXPORT LayoutNGTable : public LayoutNGMixin<LayoutBlock>,
     return absolute_column_index;
   }
 
-  // Legacy caches sections. Might not be needed by NG.
-  void RecalcSectionsIfNeeded() const final { NOTIMPLEMENTED(); }
+  // NG does not need this method. Sections are not cached.
+  void RecalcSectionsIfNeeded() const final {}
 
   // Legacy caches sections. Might not be needed by NG.
   void ForceSectionsRecalc() final { NOTIMPLEMENTED(); }

third_party/blink/renderer/core/layout/ng/table/layout_ng_table_cell.cc

@@ -37,8 +37,13 @@ void LayoutNGTableCell::InvalidateLayoutResultCacheAfterMeasure() const {
 
 LayoutRectOutsets LayoutNGTableCell::BorderBoxOutsets() const {
   NOT_DESTROYED();
-  DCHECK_GE(PhysicalFragmentCount(), 0u);
-  return GetPhysicalFragment(0)->Borders().ToLayoutRectOutsets();
+  // TODO(1061423) This function should not be called before layout.
+  // ScrollAnchor::Examine does. Example trigger:
+  // ScrollTimelineTest.TimelineInvalidationWhenScrollerDisplayPropertyChanges
+  // DCHECK_GE(PhysicalFragmentCount(), 0u);
+  if (PhysicalFragmentCount() > 0)
+    return GetPhysicalFragment(0)->Borders().ToLayoutRectOutsets();
+  return LayoutNGBlockFlowMixin<LayoutBlockFlow>::BorderBoxOutsets();
 }
 
 LayoutUnit LayoutNGTableCell::BorderTop() const {

third_party/blink/renderer/core/layout/ng/table/ng_table_layout_algorithm_helpers.cc

@@ -855,8 +855,8 @@ MinMaxSizes NGTableAlgorithmHelpers::ComputeGridInlineMinMax(
       } else {
         minmax.min_size += *column.min_inline_size;
       }
-      if (column.percent) {
-        if (*column.max_inline_size > LayoutUnit() && *column.percent > 0) {
+      if (column.percent && *column.percent > 0) {
+        if (*column.max_inline_size > LayoutUnit()) {
           LayoutUnit estimate = LayoutUnit(
               100 / *column.percent *
               (*column.max_inline_size - column.percent_border_padding));