Commit 77b8af8e authored by Ian Kilpatrick's avatar Ian Kilpatrick Committed by Commit Bot

[FlexNG] Use measure cache slot for initial flex-row layout.

During flex-row layout we typically perform three passes:
1) Determine the min/max sizes.
2) Layout with a non-stretched size.
3) Layout with a stretched size.

Previously we were using the "layout" cache slot for both steps, (2, 3).
This patch changes step (2) to use the "measure" cache slot.

This also updates the simplified layout pass to abort if it is trying to
use a fragment from the "measure" pass, which was triggering a DCHECK.

Bug: 845235, 1072429
Change-Id: Ib98187e09b23e86d67b39306a24318e9e932ca60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2154489
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Reviewed-by: default avatarDavid Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#761097}
parent c09c8b7f
......@@ -516,11 +516,17 @@ scoped_refptr<const NGLayoutResult> NGBlockNode::Layout(
return layout_result;
}
scoped_refptr<const NGLayoutResult> NGBlockNode::SimplifiedLayout() {
scoped_refptr<const NGLayoutResult> NGBlockNode::SimplifiedLayout(
const NGPhysicalFragment& previous_fragment) {
scoped_refptr<const NGLayoutResult> previous_result =
box_->GetCachedLayoutResult();
DCHECK(previous_result);
// We might be be trying to perform simplfied layout on a fragment in the
// "measure" cache slot, abort if this is the case.
if (&previous_result->PhysicalFragment() != &previous_fragment)
return nullptr;
if (!box_->NeedsLayout())
return previous_result;
......
......@@ -43,7 +43,8 @@ class CORE_EXPORT NGBlockNode final : public NGLayoutInputNode {
// If layout is dirty, it will perform layout using the previous constraint
// space used to generate the |NGLayoutResult|.
// Otherwise it will simply return the previous layout result generated.
scoped_refptr<const NGLayoutResult> SimplifiedLayout();
scoped_refptr<const NGLayoutResult> SimplifiedLayout(
const NGPhysicalFragment& previous_fragment);
// This method is just for use within the |NGOutOfFlowLayoutPart|.
//
......
......@@ -852,6 +852,16 @@ scoped_refptr<const NGLayoutResult> NGFlexLayoutAlgorithm::Layout() {
available_size.block_size = CalculateFixedCrossSize(
available_size.block_size, flex_item.min_max_cross_sizes.value(),
margins.BlockSum());
} else {
// If we are in a row flexbox, and we don't have a fixed block-size
// (yet), use the "measure" cache slot. Typically this will be the
// first layout, and we will use the "layout" cache slot if this gets
// stretched later.
//
// Setting the "measure" cache slot on the space writes the result
// into both the "measure", and "layout" cache slots. If a subsequent
// "layout" occurs, it'll just get written into the "layout" slot.
space_builder.SetCacheSlot(NGCacheSlot::kMeasure);
}
}
......
......@@ -1502,22 +1502,25 @@ TEST_F(NGLayoutResultCachingTest, MissIsFixedBlockSizeIndefinite) {
EXPECT_EQ(result.get(), nullptr);
}
TEST_F(NGLayoutResultCachingTest, HitFlexBoxMeasureAndLayout) {
TEST_F(NGLayoutResultCachingTest, HitColumnFlexBoxMeasureAndLayout) {
ScopedLayoutNGFlexBoxForTest layout_ng_flex_box(true);
SetBodyInnerHTML(R"HTML(
<!DOCTYPE html>
<div style="display: flex; flex-direction: column; width: 100px; height: 100px;">
<style>
.bfc { display: flex; flex-direction: column; width: 100px; height: 100px; }
</style>
<div class="bfc">
<div id="src1" style="flex-grow: 0;">
<div style="height: 50px;"></div>
</div>
</div>
<div style="display: flex; flex-direction: column; width: 100px; height: 100px;">
<div class="bfc">
<div id="src2" style="flex-grow: 1;">
<div style="height: 50px;"></div>
</div>
</div>
<div style="display: flex; flex-direction: column; width: 100px; height: 100px;">
<div class="bfc">
<div id="test1" style="flex-grow: 2;">
<div style="height: 50px;"></div>
</div>
......@@ -1553,6 +1556,62 @@ TEST_F(NGLayoutResultCachingTest, HitFlexBoxMeasureAndLayout) {
EXPECT_NE(result.get(), nullptr);
}
TEST_F(NGLayoutResultCachingTest, HitRowFlexBoxMeasureAndLayout) {
ScopedLayoutNGFlexBoxForTest layout_ng_flex_box(true);
SetBodyInnerHTML(R"HTML(
<!DOCTYPE html>
<style>
.bfc { display: flex; width: 100px; }
</style>
<div class="bfc">
<div id="src1">
<div style="height: 50px;"></div>
</div>
</div>
<div class="bfc">
<div id="src2">
<div style="height: 70px;"></div>
</div>
<div style="width: 0px; height: 100px;"></div>
</div>
<div class="bfc">
<div id="test1">
<div style="height: 50px;"></div>
</div>
<div style="width: 0px; height: 100px;"></div>
</div>
)HTML");
auto* test1 = To<LayoutBlockFlow>(GetLayoutObjectByElementId("test1"));
auto* src1 = To<LayoutBlockFlow>(GetLayoutObjectByElementId("src1"));
auto* src2 = To<LayoutBlockFlow>(GetLayoutObjectByElementId("src2"));
NGLayoutCacheStatus cache_status;
base::Optional<NGFragmentGeometry> fragment_geometry;
// "src1" only had one "measure" pass performed, and should hit the "measure"
// cache-slot for "test1".
NGConstraintSpace space =
src1->GetCachedLayoutResult()->GetConstraintSpaceForCaching();
scoped_refptr<const NGLayoutResult> result = test1->CachedLayoutResult(
space, nullptr, nullptr, &fragment_geometry, &cache_status);
EXPECT_EQ(space.CacheSlot(), NGCacheSlot::kMeasure);
EXPECT_EQ(cache_status, NGLayoutCacheStatus::kHit);
EXPECT_NE(result.get(), nullptr);
// "src2" had both a "measure" and "layout" pass performed, and should hit
// the "layout" cache-slot for "test1".
space = src2->GetCachedLayoutResult()->GetConstraintSpaceForCaching();
result = test1->CachedLayoutResult(space, nullptr, nullptr,
&fragment_geometry, &cache_status);
EXPECT_EQ(space.CacheSlot(), NGCacheSlot::kLayout);
EXPECT_EQ(cache_status, NGLayoutCacheStatus::kHit);
EXPECT_NE(result.get(), nullptr);
}
TEST_F(NGLayoutResultCachingTest, HitFlexLegacyImg) {
SetBodyInnerHTML(R"HTML(
<style>
......
......@@ -161,7 +161,7 @@ scoped_refptr<const NGLayoutResult> NGSimplifiedLayoutAlgorithm::Layout() {
// Add the (potentially updated) layout result.
scoped_refptr<const NGLayoutResult> result =
To<NGBlockNode>(child).SimplifiedLayout();
To<NGBlockNode>(child).SimplifiedLayout(**it);
// The child may have failed "simplified" layout! (Due to adding/removing
// scrollbars). In this case we also return a nullptr, indicating a full
......
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