[TableNG] Allow column percentages only within a block container chain.

There is some interesting behaviour within tables, which only consider
column percentages for intrinsic min/max sizes for a block container
chain.

If there are column percentages present, the intrinsic max-size can go
infinite, which only really works within block flow layout.

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

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

@@ -31,14 +31,6 @@ namespace blink {
 
 namespace {
 
-// TODO(atotic, ikilpatrick)
-// Copy of ShouldScaleColumnsForParent from table_layout_algorithm_auto.cc
-// The real fix would be for containers to understand that
-// Table max really means: max should be trimmed to available inline size.
-bool ShouldIgnorePercentagesForMinMax(const LayoutBox& table) {
-  return false;
-}
-
 NGTableTypes::Caption ComputeCaptionConstraint(
     const ComputedStyle& table_style,
     const NGTableGroupedChildren& grouped_children) {
@@ -99,7 +91,7 @@ LayoutUnit ComputeEmptyTableInlineSize(
 
 // standard: https://www.w3.org/TR/css-tables-3/#computing-the-table-width
 LayoutUnit ComputeAssignableTableInlineSize(
-    const NGBlockNode& table,
+    const NGTableNode& table,
     const NGConstraintSpace& space,
     const NGTableTypes::Columns& column_constraints,
     const NGTableTypes::Caption& caption_constraint,
@@ -113,8 +105,8 @@ LayoutUnit ComputeAssignableTableInlineSize(
 
   const MinMaxSizes grid_min_max =
       NGTableAlgorithmHelpers::ComputeGridInlineMinMax(
-          column_constraints, undistributable_space, is_fixed_layout,
-          /* containing_block_expects_minmax_without_percentages */ false,
+          table, column_constraints, undistributable_space, is_fixed_layout,
+          /* allow_column_percentages */ true,
           /* skip_collapsed_columns */ false);
 
   // Standard: "used width of the table".
@@ -499,12 +491,11 @@ scoped_refptr<const NGLayoutResult> NGTableLayoutAlgorithm::Layout() {
 
 MinMaxSizesResult NGTableLayoutAlgorithm::ComputeMinMaxSizes(
     const MinMaxSizesInput& input) const {
-  LayoutNGTable* layout_table = To<LayoutNGTable>(Node().GetLayoutBox());
   const bool is_fixed_layout = Style().IsFixedTableLayout();
   // Tables need autosizer.
   base::Optional<TextAutosizer::TableLayoutScope> text_autosizer;
   if (!is_fixed_layout)
-    text_autosizer.emplace(layout_table);
+    text_autosizer.emplace(To<LayoutNGTable>(Node().GetLayoutBox()));
 
   const LogicalSize border_spacing = Style().TableBorderSpacing();
   NGTableGroupedChildren grouped_children(Node());
@@ -523,8 +514,8 @@ MinMaxSizesResult NGTableLayoutAlgorithm::ComputeMinMaxSizes(
 
   const MinMaxSizes grid_min_max =
       NGTableAlgorithmHelpers::ComputeGridInlineMinMax(
-          *column_constraints, undistributable_space, is_fixed_layout,
-          ShouldIgnorePercentagesForMinMax(*layout_table),
+          Node(), *column_constraints, undistributable_space, is_fixed_layout,
+          /* allow_column_percentages */ false,
           /* skip_collapsed_columns */ true);
 
   MinMaxSizes min_max{

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

@@ -5,6 +5,7 @@
 #include "third_party/blink/renderer/core/layout/ng/table/ng_table_layout_algorithm_helpers.h"
 
 #include "third_party/blink/renderer/core/layout/geometry/logical_size.h"
+#include "third_party/blink/renderer/core/layout/ng/table/ng_table_node.h"
 
 namespace blink {
 
@@ -823,12 +824,13 @@ void DistributeExcessBlockSizeToRows(
 }  // namespace
 
 MinMaxSizes NGTableAlgorithmHelpers::ComputeGridInlineMinMax(
+    const NGTableNode& node,
     const NGTableTypes::Columns& column_constraints,
     LayoutUnit undistributable_space,
     bool is_fixed_layout,
-    bool containing_block_expects_minmax_without_percentages,
+    bool allow_column_percentages,
     bool skip_collapsed_columns) {
-  MinMaxSizes minmax;
+  MinMaxSizes min_max;
   // https://www.w3.org/TR/css-tables-3/#computing-the-table-width
   // Compute standard GRID_MIN/GRID_MAX. They are sum of column_constraints.
   //
@@ -847,9 +849,9 @@ MinMaxSizes NGTableAlgorithmHelpers::ComputeGridInlineMinMax(
   // T% * MINSUM + M = MINSUM.
 
   // Minimum total size estimate based on column's min_inline_size and percent.
-  LayoutUnit percent_maxsize_estimate;
+  LayoutUnit percent_max_size_estimate;
   // Sum of max_inline_sizes of non-percentage columns.
-  LayoutUnit non_percent_maxsize_sum;
+  LayoutUnit non_percent_max_size_sum;
   float percent_sum = 0;
   for (const NGTableTypes::Column& column : column_constraints.data) {
     if (skip_collapsed_columns && column.is_collapsed)
@@ -858,50 +860,50 @@ MinMaxSizes NGTableAlgorithmHelpers::ComputeGridInlineMinMax(
       // In fixed layout, constrained cells minimum inline size is their
       // maximum.
       if (is_fixed_layout && column.IsFixed()) {
-        minmax.min_size += *column.max_inline_size;
+        min_max.min_size += *column.max_inline_size;
       } else {
-        minmax.min_size += *column.min_inline_size;
+        min_max.min_size += *column.min_inline_size;
       }
       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));
-          percent_maxsize_estimate =
-              std::max(percent_maxsize_estimate, estimate);
+          percent_max_size_estimate =
+              std::max(percent_max_size_estimate, estimate);
         }
       } else {
-        non_percent_maxsize_sum += *column.max_inline_size;
+        non_percent_max_size_sum += *column.max_inline_size;
       }
     }
     if (column.max_inline_size)
-      minmax.max_size += *column.max_inline_size;
+      min_max.max_size += *column.max_inline_size;
     if (column.percent)
       percent_sum += *column.percent;
   }
   DCHECK_LE(percent_sum, 100.0f);
 
-  // Table max inline size constraint can be computed from:
-  // total column percentage combined with max_inline_size of nonpercent
-  // columns.
-  if (percent_sum > 0 && !containing_block_expects_minmax_without_percentages) {
+  // Table max inline size constraint can be computed from the total column
+  // percentage combined with max_inline_size of non-percent columns.
+  if (percent_sum > 0 &&
+      (allow_column_percentages || node.AllowColumnPercentages())) {
     LayoutUnit size_from_percent_and_fixed;
     DCHECK_GE(percent_sum, 0.0f);
-    if (non_percent_maxsize_sum != LayoutUnit()) {
+    if (non_percent_max_size_sum != LayoutUnit()) {
       if (percent_sum == 100.0f) {
         size_from_percent_and_fixed = NGTableTypes::kTableMaxInlineSize;
       } else {
         size_from_percent_and_fixed =
-            LayoutUnit((100 / (100 - percent_sum)) * non_percent_maxsize_sum);
+            LayoutUnit((100 / (100 - percent_sum)) * non_percent_max_size_sum);
       }
     }
-    minmax.max_size = std::max(minmax.max_size, size_from_percent_and_fixed);
-    minmax.max_size = std::max(minmax.max_size, percent_maxsize_estimate);
+    min_max.max_size = std::max(min_max.max_size, size_from_percent_and_fixed);
+    min_max.max_size = std::max(min_max.max_size, percent_max_size_estimate);
   }
 
-  minmax.max_size = std::max(minmax.min_size, minmax.max_size);
-  minmax += undistributable_space;
-  return minmax;
+  min_max.max_size = std::max(min_max.min_size, min_max.max_size);
+  min_max += undistributable_space;
+  return min_max;
 }
 
 void NGTableAlgorithmHelpers::DistributeColspanCellsToColumns(

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

@@ -10,6 +10,8 @@
 
 namespace blink {
 
+class NGTableNode;
+
 // Table size distribution algorithms.
 class CORE_EXPORT NGTableAlgorithmHelpers {
  public:
@@ -24,17 +26,18 @@ class CORE_EXPORT NGTableAlgorithmHelpers {
     return current_column + 1;
   }
 
-  // Flex/grid containing blocks need Table minmax size to be computed without
-  // using percentages.
-  // |containing_block_expects_minmax_without_percentages| is used to do
-  // this.
+  // Flex/grid/table-cell containing blocks require that the table min/max
+  // sizes be computed without percentages. |allow_column_percentages| is used
+  // to change this behaviour.
+  //
   // |undistributable_space| is size of space not occupied by cells
   // (borders, border spacing).
   static MinMaxSizes ComputeGridInlineMinMax(
+      const NGTableNode& node,
       const NGTableTypes::Columns& column_constraints,
       LayoutUnit undistributable_space,
       bool is_fixed_layout,
-      bool containing_block_expects_minmax_without_percentages,
+      bool allow_column_percentages,
       bool skip_collapsed_columns);
 
   static void DistributeColspanCellsToColumns(

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

@@ -5,12 +5,12 @@
 #include "third_party/blink/renderer/core/layout/ng/table/ng_table_layout_algorithm_helpers.h"
 
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/renderer/core/layout/ng/table/ng_table_node.h"
+#include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
 
 namespace blink {
 
-class NGTableAlgorithmHelpersTest : public testing::Test {
-  void SetUp() override {}
-
+class NGTableAlgorithmHelpersTest : public RenderingTest {
  public:
   NGTableTypes::Column MakeColumn(int min_width,
                                   int max_width,
@@ -179,12 +179,19 @@ TEST_F(NGTableAlgorithmHelpersTest, DistributeColspanAutoExactMaxSize) {
 }
 
 TEST_F(NGTableAlgorithmHelpersTest, ComputeGridInlineMinMax) {
+  SetBodyInnerHTML(R"HTML(
+    <div style="display: flex;">
+      <table id=target></table>
+    <div>
+  )HTML");
+  NGTableNode node(To<LayoutBox>(GetLayoutObjectByElementId("target")));
+
   scoped_refptr<NGTableTypes::Columns> column_constraints =
       base::MakeRefCounted<NGTableTypes::Columns>();
 
   LayoutUnit undistributable_space;
   bool is_fixed_layout = false;
-  bool containing_block_expects_minmax_without_percentages = false;
+  bool allow_column_percentages = true;
   bool skip_collapsed_columns = false;
 
   // No percentages, just sums up min/max.
@@ -193,9 +200,8 @@ TEST_F(NGTableAlgorithmHelpersTest, ComputeGridInlineMinMax) {
   column_constraints->data.push_back(MakeColumn(30, 300));
 
   MinMaxSizes minmax = NGTableAlgorithmHelpers::ComputeGridInlineMinMax(
-      *column_constraints, undistributable_space, is_fixed_layout,
-      containing_block_expects_minmax_without_percentages,
-      skip_collapsed_columns);
+      node, *column_constraints, undistributable_space, is_fixed_layout,
+      allow_column_percentages, skip_collapsed_columns);
   EXPECT_EQ(minmax.min_size, LayoutUnit(60));
   EXPECT_EQ(minmax.max_size, LayoutUnit(600));
 
@@ -206,32 +212,28 @@ TEST_F(NGTableAlgorithmHelpersTest, ComputeGridInlineMinMax) {
   column_constraints->data.push_back(MakeColumn(10, 10));
   column_constraints->data.push_back(MakeColumn(10, 10));
   minmax = NGTableAlgorithmHelpers::ComputeGridInlineMinMax(
-      *column_constraints, undistributable_space, is_fixed_layout,
-      containing_block_expects_minmax_without_percentages,
-      skip_collapsed_columns);
+      node, *column_constraints, undistributable_space, is_fixed_layout,
+      allow_column_percentages, skip_collapsed_columns);
   EXPECT_EQ(minmax.min_size, LayoutUnit(30));
   EXPECT_EQ(minmax.max_size, LayoutUnit(990));
 
-  // Without percent, minmax ignores percent
-  containing_block_expects_minmax_without_percentages = true;
+  allow_column_percentages = false;
   minmax = NGTableAlgorithmHelpers::ComputeGridInlineMinMax(
-      *column_constraints, undistributable_space, is_fixed_layout,
-      containing_block_expects_minmax_without_percentages,
-      skip_collapsed_columns);
+      node, *column_constraints, undistributable_space, is_fixed_layout,
+      allow_column_percentages, skip_collapsed_columns);
   EXPECT_EQ(minmax.min_size, LayoutUnit(30));
   EXPECT_EQ(minmax.max_size, LayoutUnit(119));
 
   // Percentage: total percentage of 20%, and non-percent width of 800 =>
   // table max size of 800 + (20% * 800/80%) = 1000
-  containing_block_expects_minmax_without_percentages = false;
+  allow_column_percentages = true;
   column_constraints->data.Shrink(0);
   column_constraints->data.push_back(MakeColumn(10, 100, 10));
   column_constraints->data.push_back(MakeColumn(10, 10, 10));
   column_constraints->data.push_back(MakeColumn(10, 800));
   minmax = NGTableAlgorithmHelpers::ComputeGridInlineMinMax(
-      *column_constraints, undistributable_space, is_fixed_layout,
-      containing_block_expects_minmax_without_percentages,
-      skip_collapsed_columns);
+      node, *column_constraints, undistributable_space, is_fixed_layout,
+      allow_column_percentages, skip_collapsed_columns);
   EXPECT_EQ(minmax.min_size, LayoutUnit(30));
   EXPECT_EQ(minmax.max_size, LayoutUnit(1000));
 }

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

@@ -46,4 +46,21 @@ LayoutUnit NGTableNode::ComputeTableInlineSize(
                                                         border_padding);
 }
 
+bool NGTableNode::AllowColumnPercentages() const {
+  // TODO(layout-dev): This function breaks the rule of "no tree-walks".
+  // However for this specific case it adds a lot of overhead for little gain.
+  // In the future, we could have a bit on a LayoutObject which indicates if we
+  // should allow column percentages, and maintain this when adding/removing
+  // from the tree.
+  const LayoutBlock* block = box_->ContainingBlock();
+  while (!block->IsLayoutView()) {
+    if (block->IsTableCell() || block->IsFlexibleBoxIncludingNG() ||
+        block->IsLayoutGrid())
+      return false;
+
+    block = block->ContainingBlock();
+  }
+  return true;
+}
+
 }  // namespace blink

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

@@ -27,6 +27,16 @@ class CORE_EXPORT NGTableNode final : public NGBlockNode {
 
   LayoutUnit ComputeTableInlineSize(const NGConstraintSpace&,
                                     const NGBoxStrut& border_padding) const;
+
+  // Tables are special in that their max intrinsic-size can be "infinite"
+  // (they should consume as much space as possible). However a lot of layout
+  // modes (flex/grid) don't deal well with "infinite" max intrinsic-size, so
+  // we disable this behaviour whenever we are an arbitrary descendant of one
+  // of these layout modes.
+  //
+  // TODO(layout-dev): This isn't ideal, as we may have a fixed inline-size
+  // parent where an "infinite" size would be fine.
+  bool AllowColumnPercentages() const;
 };
 
 template <>

third_party/blink/web_tests/TestExpectations

@@ -4954,8 +4954,6 @@ crbug.com/1157626 virtual/layout_ng_table/external/wpt/css/css-tables/visibility
 
 # Works in main table patch, needs percentage sizing fix.
 crbug.com/958381 virtual/layout_ng_table/external/wpt/css/css-tables/height-distribution/percentage-sizing-of-table-cell-children.html [ Failure ]
-crbug.com/958381 virtual/layout_ng_table/external/wpt/css/css-tables/percent-width-ignored-001.tentative.html [ Failure ]
-crbug.com/958381 virtual/layout_ng_table/external/wpt/css/css-tables/percent-width-ignored-003.tentative.html [ Failure ]
 
 # Sheriff 2019-08-14
 crbug.com/993671 [ Win ] http/tests/media/video-frame-size-change.html [ Pass Failure ]