[TableNG] Add specialized cache logic for table-cell baseline alignment.

This patch allows a NGBlockLayoutAlgorithm to "shift" its content, in
order to perform vertical-alignment within table-cells.

Previously this was done by intrinsic-padding.

And the end of layout, we "finalize" for the table cell, shifting
the content to the desired alignment location.

Within the caching logic for the common case where we don't have an
alignment baseline (for the "measure" pass), then we do (for the
"layout" pass), we hit the cache if the alignment-baseline, and the
actual baseline are the same.

Bug: 958381
Change-Id: I4a9edd3dd8ecc6e8ec90f6b7042f7af644bd0be0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2500981
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Aleks Totic <atotic@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821784}

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

@@ -367,6 +367,19 @@ base::Optional<LogicalOffset> NGFragmentItemsBuilder::LogicalOffsetFor(
   return base::nullopt;
 }
 
+void NGFragmentItemsBuilder::MoveChildrenInBlockDirection(LayoutUnit delta) {
+  DCHECK(!is_converted_to_physical_);
+  for (ItemWithOffset* iter = items_.begin(); iter != items_.end(); ++iter) {
+    if (iter->item->Type() == NGFragmentItem::kLine) {
+      iter->offset.block_offset += delta;
+      std::advance(iter, iter->item->DescendantsCount() - 1);
+      DCHECK_LE(iter, items_.end());
+      continue;
+    }
+    iter->offset.block_offset += delta;
+  }
+}
+
 void NGFragmentItemsBuilder::ToFragmentItems(const PhysicalSize& outer_size,
                                              void* data) {
   DCHECK(text_content_);

third_party/blink/renderer/core/layout/ng/inline/ng_fragment_items_builder.h

@@ -126,6 +126,9 @@ class CORE_EXPORT NGFragmentItemsBuilder {
   // Find |LogicalOffset| of the first |NGFragmentItem| for |LayoutObject|.
   base::Optional<LogicalOffset> LogicalOffsetFor(const LayoutObject&) const;
 
+  // Moves all the |NGFragmentItem|s by |offset| in the block-direction.
+  void MoveChildrenInBlockDirection(LayoutUnit offset);
+
   // Converts the |NGFragmentItem| vector to the physical coordinate space and
   // returns the result. This should only be used for determining the inline
   // containing block geometry for OOF-positioned nodes.

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

@@ -489,7 +489,8 @@ inline scoped_refptr<const NGLayoutResult> NGBlockLayoutAlgorithm::Layout(
       ConstraintSpace(), Node(), ChildAvailableSize(), BorderScrollbarPadding(),
       BorderPadding());
 
-  container_builder_.AdjustBorderScrollbarPaddingForTableCell();
+  if (ConstraintSpace().IsLegacyTableCell())
+    container_builder_.AdjustBorderScrollbarPaddingForTableCell();
 
   DCHECK_EQ(!!inline_child_layout_context,
             Node().IsInlineFormattingContextRoot());
@@ -862,6 +863,7 @@ scoped_refptr<const NGLayoutResult> NGBlockLayoutAlgorithm::FinishLayout(
   }
 
   // Save the unconstrained intrinsic size on the builder before clamping it.
+  LayoutUnit unconstrained_intrinsic_block_size = intrinsic_block_size_;
   container_builder_.SetOverflowBlockSize(intrinsic_block_size_);
 
   intrinsic_block_size_ = ClampIntrinsicBlockSize(
@@ -880,13 +882,6 @@ scoped_refptr<const NGLayoutResult> NGBlockLayoutAlgorithm::FinishLayout(
       border_box_size.inline_size);
   container_builder_.SetFragmentsTotalBlockSize(border_box_size.block_size);
 
-  // At this point, we need to perform any final table-cell adjustments
-  // needed.
-  if (ConstraintSpace().IsTableCell()) {
-    container_builder_.SetHasCollapsedBorders(
-        ConstraintSpace().IsTableCellWithCollapsedBorders());
-  }
-
   // If our BFC block-offset is still unknown, we check:
   //  - If we have a non-zero block-size (margins don't collapse through us).
   //  - If we have a break token. (Even if we are self-collapsing we position
@@ -941,6 +936,10 @@ scoped_refptr<const NGLayoutResult> NGBlockLayoutAlgorithm::FinishLayout(
     }
   }
 
+  // At this point, perform any final table-cell adjustments needed.
+  if (ConstraintSpace().IsTableCell())
+    FinalizeForTableCell(unconstrained_intrinsic_block_size);
+
   // We only finalize for fragmentation if the fragment has a BFC block offset.
   // This may occur with a zero block size fragment. We need to know the BFC
   // block offset to determine where the fragmentation line is relative to us.
@@ -971,13 +970,6 @@ scoped_refptr<const NGLayoutResult> NGBlockLayoutAlgorithm::FinishLayout(
   if (ConstraintSpace().UseFirstLineStyle())
     container_builder_.SetStyleVariant(NGStyleVariant::kFirstLine);
 
-  // Table-cells with separate borders, "empty-cells: hide", and no children
-  // don't paint.
-  if (ConstraintSpace().HideTableCellIfEmpty()) {
-    container_builder_.SetIsHiddenForPaint(
-        container_builder_.Children().IsEmpty());
-  }
-
   return container_builder_.ToBoxFragment();
 }
 
@@ -2151,6 +2143,68 @@ LayoutUnit NGBlockLayoutAlgorithm::PositionSelfCollapsingChildWithParentBfc(
   return child_bfc_block_offset;
 }
 
+void NGBlockLayoutAlgorithm::FinalizeForTableCell(
+    LayoutUnit unconstrained_intrinsic_block_size) {
+  // Hide table-cells if:
+  //  - They are within a collapsed column(s).
+  //  - They have "empty-cells: hide", non-collapsed borders, and no children.
+  container_builder_.SetIsHiddenForPaint(
+      ConstraintSpace().IsTableCellHiddenForPaint() ||
+      (ConstraintSpace().HideTableCellIfEmpty() &&
+       container_builder_.Children().IsEmpty()));
+
+  container_builder_.SetHasCollapsedBorders(
+      ConstraintSpace().IsTableCellWithCollapsedBorders());
+
+  // Everything else within this function only applies to new table-cells.
+  if (ConstraintSpace().IsLegacyTableCell())
+    return;
+
+  container_builder_.SetTableCellColumnIndex(
+      ConstraintSpace().TableCellColumnIndex());
+
+  switch (Style().VerticalAlign()) {
+    case EVerticalAlign::kTop:
+      // Do nothing for 'top' vertical alignment.
+      break;
+    case EVerticalAlign::kBaselineMiddle:
+    case EVerticalAlign::kSub:
+    case EVerticalAlign::kSuper:
+    case EVerticalAlign::kTextTop:
+    case EVerticalAlign::kTextBottom:
+    case EVerticalAlign::kLength:
+      // All of the above are treated as 'baseline' for the purposes of
+      // table-cell vertical alignment.
+    case EVerticalAlign::kBaseline:
+      // Table-cells (with baseline vertical alignment) always produce a
+      // baseline of their end-content edge (even if the content doesn't have
+      // any baselines).
+      if (!container_builder_.Baseline() ||
+          Node().ShouldApplyLayoutContainment()) {
+        container_builder_.SetBaseline(unconstrained_intrinsic_block_size -
+                                       BorderScrollbarPadding().block_end);
+      }
+
+      if (auto alignment_baseline =
+              ConstraintSpace().TableCellAlignmentBaseline()) {
+        container_builder_.MoveChildrenInBlockDirection(
+            *alignment_baseline - *container_builder_.Baseline());
+      }
+      break;
+    case EVerticalAlign::kMiddle:
+      container_builder_.MoveChildrenInBlockDirection(
+          (container_builder_.FragmentBlockSize() -
+           unconstrained_intrinsic_block_size) /
+          2);
+      break;
+    case EVerticalAlign::kBottom:
+      container_builder_.MoveChildrenInBlockDirection(
+          container_builder_.FragmentBlockSize() -
+          unconstrained_intrinsic_block_size);
+      break;
+  };
+}
+
 LayoutUnit NGBlockLayoutAlgorithm::FragmentainerSpaceAvailable() const {
   DCHECK(container_builder_.BfcBlockOffset());
   return FragmentainerSpaceAtBfcStart(ConstraintSpace()) -

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

@@ -215,6 +215,9 @@ class CORE_EXPORT NGBlockLayoutAlgorithm
       NGInlineChildLayoutContext*,
       scoped_refptr<const NGInlineBreakToken>* previous_inline_break_token);
 
+  // Performs any final adjustments for table-cells.
+  void FinalizeForTableCell(LayoutUnit unconstrained_intrinsic_block_size);
+
   // Return the amount of block space available in the current fragmentainer
   // for the node being laid out by this algorithm.
   LayoutUnit FragmentainerSpaceAvailable() const;

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

@@ -339,6 +339,35 @@ EBreakBetween NGBoxFragmentBuilder::JoinedBreakBetweenValue(
   return JoinFragmentainerBreakValues(previous_break_after_, break_before);
 }
 
+void NGBoxFragmentBuilder::MoveChildrenInBlockDirection(LayoutUnit delta) {
+  DCHECK(is_new_fc_);
+  DCHECK(!has_oof_candidate_that_needs_block_offset_adjustment_);
+  DCHECK_NE(FragmentBlockSize(), kIndefiniteSize);
+  DCHECK(oof_positioned_descendants_.IsEmpty());
+
+  if (delta == LayoutUnit())
+    return;
+
+  if (baseline_)
+    *baseline_ += delta;
+  if (last_baseline_)
+    *last_baseline_ += delta;
+
+  if (inflow_bounds_)
+    inflow_bounds_->offset.block_offset += delta;
+
+  for (auto& child : children_)
+    child.offset.block_offset += delta;
+
+  for (auto& candidate : oof_positioned_candidates_)
+    candidate.static_position.offset.block_offset += delta;
+  for (auto& descendant : oof_positioned_fragmentainer_descendants_)
+    descendant.static_position.offset.block_offset += delta;
+
+  if (NGFragmentItemsBuilder* items_builder = ItemsBuilder())
+    items_builder->MoveChildrenInBlockDirection(delta);
+}
+
 void NGBoxFragmentBuilder::PropagateBreak(
     const NGLayoutResult& child_layout_result) {
   if (LIKELY(!has_block_fragmentation_))

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

@@ -546,6 +546,10 @@ class CORE_EXPORT NGBoxFragmentBuilder final
   void CheckNoBlockFragmentation() const;
 #endif
 
+  // Moves all the children by |offset| in the block-direction. (Ensure that
+  // any baselines, OOFs, etc, are also moved by the appropriate amount).
+  void MoveChildrenInBlockDirection(LayoutUnit offset);
+
  private:
   // Update whether we have fragmented in this flow.
   void PropagateBreak(const NGLayoutResult&);

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

@@ -1733,5 +1733,108 @@ TEST_F(NGLayoutResultCachingTest, SimpleTable) {
   EXPECT_EQ(result2.get(), measure2.get());
 }
 
+TEST_F(NGLayoutResultCachingTest, MissTableCellMiddleAlignment) {
+  ScopedLayoutNGTableForTest table_ng(true);
+
+  SetBodyInnerHTML(R"HTML(
+    <table>
+      <td id="target" style="vertical-align: middle;">abc</td>
+      <td>abc<br>abc</td>
+    </table>
+  )HTML");
+
+  auto* target = To<LayoutBlock>(GetLayoutObjectByElementId("target"));
+
+  // "target" should be stretched, and miss the measure cache.
+  scoped_refptr<const NGLayoutResult> result = target->GetCachedLayoutResult();
+  scoped_refptr<const NGLayoutResult> measure =
+      target->GetCachedMeasureResult();
+  EXPECT_NE(measure.get(), nullptr);
+  EXPECT_NE(result.get(), nullptr);
+  EXPECT_EQ(measure->GetConstraintSpaceForCaching().CacheSlot(),
+            NGCacheSlot::kMeasure);
+  EXPECT_EQ(result->GetConstraintSpaceForCaching().CacheSlot(),
+            NGCacheSlot::kLayout);
+  EXPECT_NE(result.get(), measure.get());
+}
+
+TEST_F(NGLayoutResultCachingTest, MissTableCellBottomAlignment) {
+  ScopedLayoutNGTableForTest table_ng(true);
+
+  SetBodyInnerHTML(R"HTML(
+    <table>
+      <td id="target" style="vertical-align: bottom;">abc</td>
+      <td>abc<br>abc</td>
+    </table>
+  )HTML");
+
+  auto* target = To<LayoutBlock>(GetLayoutObjectByElementId("target"));
+
+  // "target" should be stretched, and miss the measure cache.
+  scoped_refptr<const NGLayoutResult> result = target->GetCachedLayoutResult();
+  scoped_refptr<const NGLayoutResult> measure =
+      target->GetCachedMeasureResult();
+  EXPECT_NE(measure.get(), nullptr);
+  EXPECT_NE(result.get(), nullptr);
+  EXPECT_EQ(measure->GetConstraintSpaceForCaching().CacheSlot(),
+            NGCacheSlot::kMeasure);
+  EXPECT_EQ(result->GetConstraintSpaceForCaching().CacheSlot(),
+            NGCacheSlot::kLayout);
+  EXPECT_NE(result.get(), measure.get());
+}
+
+TEST_F(NGLayoutResultCachingTest, HitTableCellBaselineAlignment) {
+  ScopedLayoutNGTableForTest table_ng(true);
+
+  SetBodyInnerHTML(R"HTML(
+    <style>
+      td { vertical-align: baseline; }
+    </style>
+    <table>
+      <td id="target">abc</td>
+      <td>def</td>
+    </table>
+  )HTML");
+
+  auto* target = To<LayoutBlock>(GetLayoutObjectByElementId("target"));
+
+  // "target" should align to the baseline, but hit the cache.
+  scoped_refptr<const NGLayoutResult> result = target->GetCachedLayoutResult();
+  scoped_refptr<const NGLayoutResult> measure =
+      target->GetCachedMeasureResult();
+  EXPECT_EQ(result->GetConstraintSpaceForCaching().CacheSlot(),
+            NGCacheSlot::kMeasure);
+  EXPECT_NE(result.get(), nullptr);
+  EXPECT_EQ(result.get(), measure.get());
+}
+
+TEST_F(NGLayoutResultCachingTest, MissTableCellBaselineAlignment) {
+  ScopedLayoutNGTableForTest table_ng(true);
+
+  SetBodyInnerHTML(R"HTML(
+    <style>
+      td { vertical-align: baseline; }
+    </style>
+    <table>
+      <td id="target">abc</td>
+      <td><span style="font-size: 32px">def</span></td>
+    </table>
+  )HTML");
+
+  auto* target = To<LayoutBlock>(GetLayoutObjectByElementId("target"));
+
+  // "target" should align to the baseline, but miss the cache.
+  scoped_refptr<const NGLayoutResult> result = target->GetCachedLayoutResult();
+  scoped_refptr<const NGLayoutResult> measure =
+      target->GetCachedMeasureResult();
+  EXPECT_NE(measure.get(), nullptr);
+  EXPECT_NE(result.get(), nullptr);
+  EXPECT_EQ(measure->GetConstraintSpaceForCaching().CacheSlot(),
+            NGCacheSlot::kMeasure);
+  EXPECT_EQ(result->GetConstraintSpaceForCaching().CacheSlot(),
+            NGCacheSlot::kLayout);
+  EXPECT_NE(result.get(), measure.get());
+}
+
 }  // namespace
 }  // namespace blink

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

@@ -221,7 +221,7 @@ NGLayoutCacheStatus CalculateSizeBasedLayoutCacheStatusWithGeometry(
       // value, as the following |block_size| calculation would be incorrect.
       // TODO(dgrogan): We can hit the cache here for row flexboxes when they
       // don't have stretchy children.
-      if (layout_result.PhysicalFragment().DependsOnPercentageBlockSize()) {
+      if (physical_fragment.DependsOnPercentageBlockSize()) {
         if (new_space.PercentageResolutionBlockSize() !=
             old_space.PercentageResolutionBlockSize())
           return NGLayoutCacheStatus::kNeedsLayout;
@@ -299,7 +299,7 @@ NGLayoutCacheStatus CalculateSizeBasedLayoutCacheStatusWithGeometry(
     // this false-positive by checking if we have an initial indefinite
     // block-size.
     if (is_new_initial_block_size_indefinite &&
-        layout_result.PhysicalFragment().DependsOnPercentageBlockSize()) {
+        physical_fragment.DependsOnPercentageBlockSize()) {
       DCHECK(is_old_initial_block_size_indefinite);
       if (new_space.PercentageResolutionBlockSize() !=
           old_space.PercentageResolutionBlockSize())
@@ -313,12 +313,57 @@ NGLayoutCacheStatus CalculateSizeBasedLayoutCacheStatusWithGeometry(
   if (style.MayHavePadding() && fragment_geometry.padding != fragment.Padding())
     return NGLayoutCacheStatus::kNeedsLayout;
 
-  // Table-cells with vertical alignment might shift their contents if their
+  // Table-cells with vertical alignment might shift their contents if the
   // block-size changes.
-  if (new_space.IsTableCell() && !is_block_size_equal &&
-      style.VerticalAlign() !=
-          ComputedStyleInitialValues::InitialVerticalAlign())
-    return NGLayoutCacheStatus::kNeedsLayout;
+  if (new_space.IsTableCell()) {
+    DCHECK(old_space.IsTableCell());
+
+    switch (style.VerticalAlign()) {
+      case EVerticalAlign::kTop:
+        // Do nothing special for 'top' vertical alignment.
+        break;
+      case EVerticalAlign::kBaselineMiddle:
+      case EVerticalAlign::kSub:
+      case EVerticalAlign::kSuper:
+      case EVerticalAlign::kTextTop:
+      case EVerticalAlign::kTextBottom:
+      case EVerticalAlign::kLength:
+        // All of the above are treated as 'baseline' for the purposes of
+        // table-cell vertical alignment.
+      case EVerticalAlign::kBaseline: {
+        auto new_alignment_baseline = new_space.TableCellAlignmentBaseline();
+        auto old_alignment_baseline = old_space.TableCellAlignmentBaseline();
+
+        // Do nothing if neither alignment baseline is set.
+        if (!new_alignment_baseline && !old_alignment_baseline)
+          break;
+
+        // If we only have an old alignment baseline set, we need layout, as we
+        // can't determine where the un-adjusted baseline is.
+        if (!new_alignment_baseline && old_alignment_baseline)
+          return NGLayoutCacheStatus::kNeedsLayout;
+
+        // We've been provided a new alignment baseline, just check that it
+        // matches the previously generated baseline.
+        if (!old_alignment_baseline) {
+          if (*new_alignment_baseline != physical_fragment.Baseline())
+            return NGLayoutCacheStatus::kNeedsLayout;
+          break;
+        }
+
+        // If the alignment baselines differ at this stage, we need layout.
+        if (*new_alignment_baseline != *old_alignment_baseline)
+          return NGLayoutCacheStatus::kNeedsLayout;
+        break;
+      }
+      case EVerticalAlign::kMiddle:
+      case EVerticalAlign::kBottom:
+        // 'middle', and 'bottom' vertical alignment depend on the block-size.
+        if (!is_block_size_equal)
+          return NGLayoutCacheStatus::kNeedsLayout;
+        break;
+    }
+  }
 
   // If we've reached here we know that we can potentially "stretch"/"shrink"
   // ourselves without affecting any of our children.

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

@@ -237,6 +237,8 @@ NGPhysicalBoxFragment::NGPhysicalBoxFragment(
   is_math_fraction_ = builder->is_math_fraction_;
   is_math_operator_ = builder->is_math_operator_;
 
+  // TODO(ikilpatrick): Investigate if new table-cells should always produce a
+  // baseline.
   bool has_layout_containment = layout_object_->ShouldApplyLayoutContainment();
   if (builder->baseline_.has_value() && !has_layout_containment) {
     has_baseline_ = true;

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

@@ -69,6 +69,9 @@ NGSimplifiedLayoutAlgorithm::NGSimplifiedLayoutAlgorithm(
 
     if (physical_fragment.LastBaseline())
       container_builder_.SetLastBaseline(*physical_fragment.LastBaseline());
+
+    // TODO(ikilpatrick): Set any fields for table-cells which are also set
+    // within the NGBlockLayoutAlgorithm.
   } else {
     // Only block-flow layout sets the following fields.
     DCHECK(physical_fragment.IsFormattingContextRoot());