Commit 623e063e authored by Ian Kilpatrick's avatar Ian Kilpatrick Committed by Commit Bot

[FlexNG] Don't consider replaced elements as having children.

Previously we considered any legacy layout root as potentially having
%-dependent descendants.

This typically wasn't an issue for when we first rolled out LayoutNG as
it was relatively common for *something* to stop the propagation of this
bit up the tree.

With FlexNG however, we run into far more cases due to the
"definiteness" potentially changing.

This patch treats images, svgs, etc has having no children for the
purposes of this calculation.

Bug: 845235
Change-Id: I98648b8182fd549d6e6bee1167cd8ae89d55687b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2148216
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Reviewed-by: default avatarDavid Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759271}
parent f6f4118f
...@@ -1553,5 +1553,37 @@ TEST_F(NGLayoutResultCachingTest, HitFlexBoxMeasureAndLayout) { ...@@ -1553,5 +1553,37 @@ TEST_F(NGLayoutResultCachingTest, HitFlexBoxMeasureAndLayout) {
EXPECT_NE(result.get(), nullptr); EXPECT_NE(result.get(), nullptr);
} }
TEST_F(NGLayoutResultCachingTest, HitFlexLegacyImg) {
SetBodyInnerHTML(R"HTML(
<style>
.bfc { display: flex; flex-direction: column; width: 300px; }
.bfc > * { display: flex; }
</style>
<div class="bfc">
<div id="test">
<img />
</div>
</div>
<div class="bfc" style="height: 200px;">
<div id="src">
<img />
</div>
</div>
)HTML");
auto* test = To<LayoutBlock>(GetLayoutObjectByElementId("test"));
auto* src = To<LayoutBlock>(GetLayoutObjectByElementId("src"));
NGLayoutCacheStatus cache_status;
base::Optional<NGFragmentGeometry> fragment_geometry;
const NGConstraintSpace& space =
src->GetCachedLayoutResult()->GetConstraintSpaceForCaching();
scoped_refptr<const NGLayoutResult> result = test->CachedLayoutResult(
space, nullptr, nullptr, &fragment_geometry, &cache_status);
EXPECT_EQ(cache_status, NGLayoutCacheStatus::kHit);
EXPECT_NE(result.get(), nullptr);
}
} // namespace } // namespace
} // namespace blink } // namespace blink
...@@ -322,6 +322,12 @@ bool NGPhysicalContainerFragment::DependsOnPercentageBlockSize( ...@@ -322,6 +322,12 @@ bool NGPhysicalContainerFragment::DependsOnPercentageBlockSize(
if (!node || node.IsInline()) if (!node || node.IsInline())
return builder.has_descendant_that_depends_on_percentage_block_size_; return builder.has_descendant_that_depends_on_percentage_block_size_;
// For the below if-stmt we only want to consider legacy *containers* as
// potentially having %-dependent children - i.e. an image doesn't have any
// children.
bool is_legacy_container =
builder.is_legacy_layout_root_ && !node.IsReplaced();
// NOTE: If an element is OOF positioned, and has top/bottom constraints // NOTE: If an element is OOF positioned, and has top/bottom constraints
// which are percentage based, this function will return false. // which are percentage based, this function will return false.
// //
...@@ -346,7 +352,7 @@ bool NGPhysicalContainerFragment::DependsOnPercentageBlockSize( ...@@ -346,7 +352,7 @@ bool NGPhysicalContainerFragment::DependsOnPercentageBlockSize(
// We only need to know about if this flex-item has a %-block-size child if // We only need to know about if this flex-item has a %-block-size child if
// the "definiteness" changes, not if the percentage resolution size changes. // the "definiteness" changes, not if the percentage resolution size changes.
if ((builder.has_descendant_that_depends_on_percentage_block_size_ || if ((builder.has_descendant_that_depends_on_percentage_block_size_ ||
builder.is_legacy_layout_root_) && is_legacy_container) &&
(node.UseParentPercentageResolutionBlockSizeForChildren() || (node.UseParentPercentageResolutionBlockSizeForChildren() ||
node.IsFlexItem())) node.IsFlexItem()))
return true; return true;
......
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