Commit a1c9e2c4 authored by Ian Kilpatrick's avatar Ian Kilpatrick Committed by Commit Bot

[LayoutNG] Make table-cells always calculate thier block-size to the intrinsic size.

Prior to this patch, during simplified layout we'd calculate the block-size
of a table-cell the same as any node. This missed the specific table-cell
logic within the NGBlockLayoutAlgorithm.

This moves this logic down into the ComputeBlockSizeForFragment method.

Bug: 968016
Change-Id: Idb4d76bb199d1690705e1f24d9de3ea53e2c8af4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1637219Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664843}
parent e3acfb09
...@@ -610,12 +610,8 @@ scoped_refptr<const NGLayoutResult> NGBlockLayoutAlgorithm::FinishLayout( ...@@ -610,12 +610,8 @@ scoped_refptr<const NGLayoutResult> NGBlockLayoutAlgorithm::FinishLayout(
Node(), border_scrollbar_padding_, intrinsic_block_size_); Node(), border_scrollbar_padding_, intrinsic_block_size_);
// Recompute the block-axis size now that we know our content size. // Recompute the block-axis size now that we know our content size.
// NOTE: For table cells, the block-size is just the intrinsic block-size. border_box_size.block_size = ComputeBlockSizeForFragment(
border_box_size.block_size = ConstraintSpace(), Node(), border_padding_, intrinsic_block_size_);
Node().IsTableCell()
? intrinsic_block_size_
: ComputeBlockSizeForFragment(ConstraintSpace(), Style(),
border_padding_, intrinsic_block_size_);
container_builder_.SetBlockSize(border_box_size.block_size); container_builder_.SetBlockSize(border_box_size.block_size);
// If our BFC block-offset is still unknown, we check: // If our BFC block-offset is still unknown, we check:
...@@ -1732,7 +1728,7 @@ void NGBlockLayoutAlgorithm::FinalizeForFragmentation() { ...@@ -1732,7 +1728,7 @@ void NGBlockLayoutAlgorithm::FinalizeForFragmentation() {
LayoutUnit used_block_size = LayoutUnit used_block_size =
BreakToken() ? BreakToken()->UsedBlockSize() : LayoutUnit(); BreakToken() ? BreakToken()->UsedBlockSize() : LayoutUnit();
LayoutUnit block_size = LayoutUnit block_size =
ComputeBlockSizeForFragment(ConstraintSpace(), Style(), border_padding_, ComputeBlockSizeForFragment(ConstraintSpace(), Node(), border_padding_,
used_block_size + intrinsic_block_size_); used_block_size + intrinsic_block_size_);
block_size -= used_block_size; block_size -= used_block_size;
......
...@@ -122,7 +122,7 @@ scoped_refptr<const NGLayoutResult> NGFieldsetLayoutAlgorithm::Layout() { ...@@ -122,7 +122,7 @@ scoped_refptr<const NGLayoutResult> NGFieldsetLayoutAlgorithm::Layout() {
// Recompute the block-axis size now that we know our content size. // Recompute the block-axis size now that we know our content size.
border_box_size.block_size = ComputeBlockSizeForFragment( border_box_size.block_size = ComputeBlockSizeForFragment(
ConstraintSpace(), Style(), border_padding_, intrinsic_block_size); ConstraintSpace(), Node(), border_padding_, intrinsic_block_size);
// The above computation utility knows nothing about fieldset weirdness. The // The above computation utility knows nothing about fieldset weirdness. The
// legend may eat from the available content box block size. Make room for // legend may eat from the available content box block size. Make room for
......
...@@ -41,7 +41,7 @@ LayoutUnit NGFlexLayoutAlgorithm::MainAxisContentExtent( ...@@ -41,7 +41,7 @@ LayoutUnit NGFlexLayoutAlgorithm::MainAxisContentExtent(
LayoutUnit sum_hypothetical_main_size) { LayoutUnit sum_hypothetical_main_size) {
if (Style().IsColumnFlexDirection()) { if (Style().IsColumnFlexDirection()) {
return ComputeBlockSizeForFragment( return ComputeBlockSizeForFragment(
ConstraintSpace(), Style(), border_padding_, ConstraintSpace(), Node(), border_padding_,
sum_hypothetical_main_size + (border_padding_).BlockSum()) - sum_hypothetical_main_size + (border_padding_).BlockSum()) -
border_scrollbar_padding_.BlockSum(); border_scrollbar_padding_.BlockSum();
} }
...@@ -324,7 +324,7 @@ scoped_refptr<const NGLayoutResult> NGFlexLayoutAlgorithm::Layout() { ...@@ -324,7 +324,7 @@ scoped_refptr<const NGLayoutResult> NGFlexLayoutAlgorithm::Layout() {
LayoutUnit intrinsic_block_size = LayoutUnit intrinsic_block_size =
intrinsic_block_content_size + border_scrollbar_padding_.BlockSum(); intrinsic_block_content_size + border_scrollbar_padding_.BlockSum();
LayoutUnit block_size = ComputeBlockSizeForFragment( LayoutUnit block_size = ComputeBlockSizeForFragment(
ConstraintSpace(), Style(), border_padding_, intrinsic_block_size); ConstraintSpace(), Node(), border_padding_, intrinsic_block_size);
container_builder_.SetBlockSize(block_size); container_builder_.SetBlockSize(block_size);
container_builder_.SetIntrinsicBlockSize( container_builder_.SetIntrinsicBlockSize(
......
...@@ -262,7 +262,7 @@ NGLayoutCacheStatus CalculateSizeBasedLayoutCacheStatusWithGeometry( ...@@ -262,7 +262,7 @@ NGLayoutCacheStatus CalculateSizeBasedLayoutCacheStatusWithGeometry(
} }
block_size = ComputeBlockSizeForFragment( block_size = ComputeBlockSizeForFragment(
new_space, style, fragment_geometry.border + fragment_geometry.padding, new_space, node, fragment_geometry.border + fragment_geometry.padding,
layout_result.IntrinsicBlockSize()); layout_result.IntrinsicBlockSize());
} }
......
...@@ -517,16 +517,20 @@ LayoutUnit ComputeBlockSizeForFragmentInternal( ...@@ -517,16 +517,20 @@ LayoutUnit ComputeBlockSizeForFragmentInternal(
LayoutUnit ComputeBlockSizeForFragment( LayoutUnit ComputeBlockSizeForFragment(
const NGConstraintSpace& constraint_space, const NGConstraintSpace& constraint_space,
const ComputedStyle& style, const NGBlockNode& node,
const NGBoxStrut& border_padding, const NGBoxStrut& border_padding,
LayoutUnit content_size) { LayoutUnit content_size) {
// The final block-size of a table-cell is always its intrinsic size.
if (node.IsTableCell() && content_size != kIndefiniteSize)
return content_size;
if (constraint_space.IsFixedSizeBlock()) if (constraint_space.IsFixedSizeBlock())
return constraint_space.AvailableSize().block_size; return constraint_space.AvailableSize().block_size;
if (constraint_space.IsAnonymous()) if (constraint_space.IsAnonymous())
return content_size; return content_size;
return ComputeBlockSizeForFragmentInternal(constraint_space, style, return ComputeBlockSizeForFragmentInternal(constraint_space, node.Style(),
border_padding, content_size); border_padding, content_size);
} }
...@@ -1042,8 +1046,8 @@ NGFragmentGeometry CalculateInitialFragmentGeometry( ...@@ -1042,8 +1046,8 @@ NGFragmentGeometry CalculateInitialFragmentGeometry(
constraint_space, node, border_scrollbar_padding); constraint_space, node, border_scrollbar_padding);
LogicalSize border_box_size( LogicalSize border_box_size(
ComputeInlineSizeForFragment(constraint_space, node, border_padding), ComputeInlineSizeForFragment(constraint_space, node, border_padding),
ComputeBlockSizeForFragment(constraint_space, node.Style(), ComputeBlockSizeForFragment(constraint_space, node, border_padding,
border_padding, default_block_size)); default_block_size));
if (UNLIKELY(border_box_size.inline_size < if (UNLIKELY(border_box_size.inline_size <
border_scrollbar_padding.InlineSum() && border_scrollbar_padding.InlineSum() &&
......
...@@ -253,7 +253,7 @@ CORE_EXPORT LayoutUnit ComputeInlineSizeForFragment( ...@@ -253,7 +253,7 @@ CORE_EXPORT LayoutUnit ComputeInlineSizeForFragment(
// Same as ComputeInlineSizeForFragment, but uses height instead of width. // Same as ComputeInlineSizeForFragment, but uses height instead of width.
CORE_EXPORT LayoutUnit CORE_EXPORT LayoutUnit
ComputeBlockSizeForFragment(const NGConstraintSpace&, ComputeBlockSizeForFragment(const NGConstraintSpace&,
const ComputedStyle&, const NGBlockNode&,
const NGBoxStrut& border_padding, const NGBoxStrut& border_padding,
LayoutUnit content_size); LayoutUnit content_size);
......
...@@ -86,16 +86,6 @@ class NGLengthUtilsTest : public testing::Test { ...@@ -86,16 +86,6 @@ class NGLengthUtilsTest : public testing::Test {
LengthResolvePhase::kLayout); LengthResolvePhase::kLayout);
} }
LayoutUnit ComputeBlockSizeForFragment(
NGConstraintSpace constraint_space = ConstructConstraintSpace(200, 300),
LayoutUnit content_size = LayoutUnit()) {
NGBoxStrut border_padding = ComputeBordersForTest(*style_) +
ComputePadding(constraint_space, *style_);
return ::blink::ComputeBlockSizeForFragment(constraint_space, *style_,
border_padding, content_size);
}
scoped_refptr<ComputedStyle> style_; scoped_refptr<ComputedStyle> style_;
}; };
...@@ -120,6 +110,20 @@ class NGLengthUtilsTestWithNode : public NGLayoutTest { ...@@ -120,6 +110,20 @@ class NGLengthUtilsTestWithNode : public NGLayoutTest {
border_padding, &sizes); border_padding, &sizes);
} }
LayoutUnit ComputeBlockSizeForFragment(
NGConstraintSpace constraint_space = ConstructConstraintSpace(200, 300),
LayoutUnit content_size = LayoutUnit()) {
LayoutBox* body = ToLayoutBox(GetDocument().body()->GetLayoutObject());
body->SetStyle(style_);
body->SetPreferredLogicalWidthsDirty();
NGBlockNode node(body);
NGBoxStrut border_padding = ComputeBordersForTest(*style_) +
ComputePadding(constraint_space, *style_);
return ::blink::ComputeBlockSizeForFragment(constraint_space, node,
border_padding, content_size);
}
scoped_refptr<ComputedStyle> style_; scoped_refptr<ComputedStyle> style_;
}; };
...@@ -344,7 +348,7 @@ TEST_F(NGLengthUtilsTestWithNode, testComputeInlineSizeForFragment) { ...@@ -344,7 +348,7 @@ TEST_F(NGLengthUtilsTestWithNode, testComputeInlineSizeForFragment) {
ComputeInlineSizeForFragment(constraint_space, sizes)); ComputeInlineSizeForFragment(constraint_space, sizes));
} }
TEST_F(NGLengthUtilsTest, testComputeBlockSizeForFragment) { TEST_F(NGLengthUtilsTestWithNode, testComputeBlockSizeForFragment) {
style_->SetLogicalHeight(Length::Percent(30)); style_->SetLogicalHeight(Length::Percent(30));
EXPECT_EQ(LayoutUnit(90), ComputeBlockSizeForFragment()); EXPECT_EQ(LayoutUnit(90), ComputeBlockSizeForFragment());
...@@ -402,7 +406,7 @@ TEST_F(NGLengthUtilsTest, testComputeBlockSizeForFragment) { ...@@ -402,7 +406,7 @@ TEST_F(NGLengthUtilsTest, testComputeBlockSizeForFragment) {
// TODO(layout-ng): test {min,max}-content on max-height. // TODO(layout-ng): test {min,max}-content on max-height.
} }
TEST_F(NGLengthUtilsTest, testIndefinitePercentages) { TEST_F(NGLengthUtilsTestWithNode, testIndefinitePercentages) {
style_->SetMinHeight(Length::Fixed(20)); style_->SetMinHeight(Length::Fixed(20));
style_->SetHeight(Length::Percent(20)); style_->SetHeight(Length::Percent(20));
......
...@@ -65,7 +65,7 @@ scoped_refptr<const NGLayoutResult> NGPageLayoutAlgorithm::Layout() { ...@@ -65,7 +65,7 @@ scoped_refptr<const NGLayoutResult> NGPageLayoutAlgorithm::Layout() {
// Recompute the block-axis size now that we know our content size. // Recompute the block-axis size now that we know our content size.
border_box_size.block_size = ComputeBlockSizeForFragment( border_box_size.block_size = ComputeBlockSizeForFragment(
ConstraintSpace(), Style(), border_padding_, intrinsic_block_size); ConstraintSpace(), Node(), border_padding_, intrinsic_block_size);
container_builder_.SetBlockSize(border_box_size.block_size); container_builder_.SetBlockSize(border_box_size.block_size);
NGOutOfFlowLayoutPart( NGOutOfFlowLayoutPart(
......
...@@ -69,7 +69,7 @@ NGSimplifiedLayoutAlgorithm::NGSimplifiedLayoutAlgorithm( ...@@ -69,7 +69,7 @@ NGSimplifiedLayoutAlgorithm::NGSimplifiedLayoutAlgorithm(
} }
container_builder_.SetBlockSize(ComputeBlockSizeForFragment( container_builder_.SetBlockSize(ComputeBlockSizeForFragment(
ConstraintSpace(), Style(), ConstraintSpace(), Node(),
container_builder_.Borders() + container_builder_.Padding(), container_builder_.Borders() + container_builder_.Padding(),
result.IntrinsicBlockSize())); result.IntrinsicBlockSize()));
......
<!DOCTYPE html>
<html class="reftest-wait">
<link rel="match" href="/css/reference/ref-filled-green-100px-square.xht">
<link rel="help" href="https://crbug.com/968016">
<script src="/common/reftest-wait.js"></script>
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="display: table-cell; background: red; height: 50px;">
<div style="width: 100px; height: 100px; background: green; position: relative;">
<div id="target" style="position: absolute; width: 10px; height: 10px;"></div>
</div>
</div>
<script>
document.body.offsetTop;
document.getElementById('target').style.top = '10px';
document.body.offsetTop;
takeScreenshot();
</script>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment