Skip to content

Commit

Permalink
[css-flexbox] Don't cache definiteness when there's an override conta…
Browse files Browse the repository at this point in the history 
…iner height

See stack trace below. We set the override container logical height to -1
for the initial layout of a flex item so that we compute the correct size
for min-height. However, that messes with our cache for definite heights
because we would always set it to indefinite in such a case.

Instead, just don't cache these values. That way we will later compute the right
thing for resolving flex-basis, etc.

(FlexNG can't come soon enough...)

 #0  blink::LayoutBox::ContainingBlockLogicalHeightForPercentageResolution (this=0x3dda8d434198,
    out_cb=0x7f6e7d42d8c0, out_skipped_auto_height_containing_block=0x0)
    at ../../third_party/blink/renderer/core/layout/layout_box.cc:3833
 #1  0x00007f6ee84ad0a1 in blink::LayoutFlexibleBox::MainAxisLengthIsDefinite (this=0x3dda8d434010,
    child=..., flex_basis=Length(0%, Percent), add_to_cb=false)
    at ../../third_party/blink/renderer/core/layout/layout_flexible_box.cc:762
 #2  0x00007f6ee84af930 in blink::LayoutFlexibleBox::MainSizeIsDefiniteForPercentageResolution (
    this=0x3dda8d434010, child=...)
    at ../../third_party/blink/renderer/core/layout/layout_flexible_box.cc:1125
 #3  0x00007f6ee84ad7f5 in blink::LayoutFlexibleBox::UseOverrideLogicalHeightForPerentageResolution (
    this=0x3dda8d434010, child=...)
    at ../../third_party/blink/renderer/core/layout/layout_flexible_box.cc:1137
 #4  0x00007f6ee83f2b9d in blink::LayoutBlock::AvailableLogicalHeightForPercentageComputation (
    this=0x3dda8d434198) at ../../third_party/blink/renderer/core/layout/layout_block.cc:2333
 #5  0x00007f6ee845e745 in blink::LayoutBox::ContainingBlockLogicalHeightForPercentageResolution (
    this=0x3dda8d4243d0, out_cb=0x0, out_skipped_auto_height_containing_block=0x0)
    at ../../third_party/blink/renderer/core/layout/layout_box.cc:3830
 #6  0x00007f6ee86dcc5c in blink::LayoutBoxUtils::AvailableLogicalHeight (box=..., cb=0x3dda8d434198)
    at ../../third_party/blink/renderer/core/layout/ng/layout_box_utils.cc:64
 #7  0x00007f6ee86eafea in blink::LayoutNGMixin<blink::LayoutBlockFlow>::ComputeIntrinsicLogicalWidths (
    this=0x3dda8d4243d0, min_logical_width=0px, max_logical_width=0px)
    at ../../third_party/blink/renderer/core/layout/ng/layout_ng_mixin.cc:48
 #8  0x00007f6ee83ef53a in blink::LayoutBlock::ComputePreferredLogicalWidths (this=0x3dda8d4243d0)
    at ../../third_party/blink/renderer/core/layout/layout_block.cc:1509
 #9  0x00007f6ee8451f01 in blink::LayoutBox::MaxPreferredLogicalWidth (this=0x3dda8d4243d0)
    at ../../third_party/blink/renderer/core/layout/layout_box.cc:1395
 #10 0x00007f6ee84adba2 in blink::LayoutFlexibleBox::ComputeInnerFlexBaseSizeForChild (this=0x3dda8d434198,
    child=..., main_axis_border_and_padding=0px, child_layout_type=blink::LayoutFlexibleBox::kForceLayout)
    at ../../third_party/blink/renderer/core/layout/layout_flexible_box.cc:890
 #11 0x00007f6ee84ae5d1 in blink::LayoutFlexibleBox::ConstructAndAppendFlexItem (this=0x3dda8d434198,
    algorithm=0x7f6e7d42ed70, child=..., layout_type=blink::LayoutFlexibleBox::kForceLayout)
    at ../../third_party/blink/renderer/core/layout/layout_flexible_box.cc:1203
 #12 0x00007f6ee84aa27b in blink::LayoutFlexibleBox::LayoutFlexItems (this=0x3dda8d434198,
    relayout_children=true, layout_scope=...)
    at ../../third_party/blink/renderer/core/layout/layout_flexible_box.cc:934
 #13 0x00007f6ee84a9cff in blink::LayoutFlexibleBox::UpdateBlockLayout (this=0x3dda8d434198,
    relayout_children=true) at ../../third_party/blink/renderer/core/layout/layout_flexible_box.cc:369

Bug: 1019138
Change-Id: Ie94e69a5f3fe6accc3623d358315b174088d5597
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1902514
Commit-Queue: David Grogan <dgrogan@chromium.org>
Auto-Submit: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713296}
  • Loading branch information
@cbiesinger @chromium-wpt-export-bot
cbiesinger authored and chromium-wpt-export-bot committed Nov 7, 2019
1 parent ad37572 commit 8642cbb
Showing 1 changed file with 37 additions and 0 deletions.
37 changes: 37 additions & 0 deletions css/css-flexbox/percentage-heights-009.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<!DOCTYPE html>
<title>height: 100% should not be considered indefinite on a second flex item (triggers an obscure bug in Blink)</title>
<link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#definite-sizes" />
<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1019138" />
<link rel="match" href="../reference/ref-filled-green-100px-square.xht" />
<style type="text/css">
.container {
height: 100px;
width: 100px;
}
.flexbox {
background: red;
display: flex;
flex-direction: column;
height: 100%;
}
.first-item {
background: green;
display: flex;
}
.second-item {
/* This should not be considered indefinite */
height: 100%;
background: green;
}
</style>

<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div class="container">
<div class="flexbox">
<div class="first-item">
<div style="width: 100px; height: 20px;"></div>
</div>
<div class="second-item">
</div>
</div>
</div>

0 comments on commit 8642cbb

Please sign in to comment.