Skip to content
Permalink
Browse files
Fix broken preferred widths optimization involving subtree layout roots
Fix broken preferred widths optimization involving subtree layout roots
https://bugs.webkit.org/show_bug.cgi?id=247742

Reviewed by Alan Baradlay.

Merge - https://chromium.googlesource.com/chromium/blink/+/ed902d339fbff3a62e2f6124d8d40bbad45e97c0

This patch is to remove optimization or fix introduced in 2006, which is about RenderBox::computePreferredLogicalWidths
to bail when in a subtree layout.
This can result in a box not updating its preferred widths even when it
should in the case that its a subtree layout root that was marked for layout
of its children, then subsequently marked for layout itself.

* Source/WebCore/rendering/RenderBox.cpp:
(RenderBox:computeLogicalWidthInFragment): Remove "subtree' layout optimization
* LayoutTests/fast/layout/nested-subtree-layout-preferred-widths.html: Added Test Case
* LayoutTests/fast/layout/nested-subtree-layout-preferred-widths-expected.html: Added Test Case Expectation
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/auto-006-expected.txt: Updated Test Expectations
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/auto-010-expected.txt: Ditto

Canonical link: https://commits.webkit.org/256623@main
  • Loading branch information
Ahmad-S792 authored and Ahmad Saleem committed Nov 14, 2022
1 parent f2e73e7 commit 06fa3e82eb2e0267284e9c57342078a070e8016c
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 20 deletions.
@@ -0,0 +1,2 @@
crbug.com/497178: This test ensures we properly recompute preferred widths for nested subtree roots. If this test is ever flaky, it should be considered failing due the the non-deterministic way we iterate over subtree layout roots.
PASS
@@ -0,0 +1,40 @@
<!DOCTYPE html>
<style>
div {
overflow: hidden;
}
#root {
width: 200px;
height: 400px;
background: red;
}
#content {
background: green;
width: 100%;
height: 400px;
display: block;
}
#container {
width: 400px;
height: 400px;
}
</style>
<script src="../../resources/check-layout.js"></script>
<div>
crbug.com/497178: This test ensures we properly recompute preferred widths for
nested subtree roots. If this test is ever flaky, it should be considered
failing due the the non-deterministic way we iterate over subtree layout roots.
</div>
<div id="container">
<div id="root">
<div data-expected-width="400" id="content">OriginalText</div>
</div>
</div>
<script>
document.body.offsetTop;
var rootElement = document.getElementById("root");
var content = document.getElementById("content");
content.innerText = "";
rootElement.style.width = "400px";
checkLayout("#content");
</script>
@@ -1,19 +1,19 @@

FAIL Basic usage assert_equals: Sizing normally - clientWidth expected 100 but got 1
FAIL Last remembered size can be 0 assert_equals: Using last remembered size - clientHeight expected 0 but got 2
FAIL Last remembered size can be set to 0 after losing display:none assert_equals: Sizing normally - clientWidth expected 100 but got 0
FAIL Last remembered size is logical assert_equals: Sizing normally - clientWidth expected 100 but got 0
FAIL Last remembered size survives box destruction assert_equals: Sizing normally - clientWidth expected 100 but got 0
FAIL Last remembered size survives display type changes assert_equals: Sizing normally - clientWidth expected 100 but got 0
FAIL Losing cis:auto removes last remembered size assert_equals: Sizing normally - clientWidth expected 100 but got 0
FAIL Losing cis:auto removes last remembered size even if size doesn't change assert_equals: Sizing normally - clientWidth expected 100 but got 0
FAIL Losing cis:auto removes last remembered size immediately assert_equals: Sizing normally - clientWidth expected 100 but got 0
FAIL Losing cis:auto during display:none doesn't remove last remembered size assert_equals: Sizing normally - clientWidth expected 100 but got 0
FAIL Lack of cis:auto during box creation removes last remembered size assert_equals: Sizing normally - clientWidth expected 100 but got 0
FAIL Last remembered size can be removed synchronously assert_equals: Sizing normally - clientWidth expected 100 but got 0
FAIL Disconnected element can briefly keep last remembered size assert_equals: Sizing normally - clientWidth expected 100 but got 0
FAIL Disconnected element ends up losing last remembered size assert_equals: Sizing normally - clientWidth expected 100 but got 0
FAIL Last remembered size can be 0 assert_equals: Using last remembered size - clientWidth expected 0 but got 1
FAIL Last remembered size can be set to 0 after losing display:none assert_equals: Sizing normally - clientWidth expected 100 but got 1
FAIL Last remembered size is logical assert_equals: Sizing normally - clientWidth expected 100 but got 1
FAIL Last remembered size survives box destruction assert_equals: Sizing normally - clientWidth expected 100 but got 1
FAIL Last remembered size survives display type changes assert_equals: Sizing normally - clientWidth expected 100 but got 1
FAIL Losing cis:auto removes last remembered size assert_equals: Sizing normally - clientWidth expected 100 but got 1
FAIL Losing cis:auto removes last remembered size even if size doesn't change assert_equals: Sizing normally - clientWidth expected 100 but got 1
FAIL Losing cis:auto removes last remembered size immediately assert_equals: Sizing normally - clientWidth expected 100 but got 1
FAIL Losing cis:auto during display:none doesn't remove last remembered size assert_equals: Sizing normally - clientWidth expected 100 but got 1
FAIL Lack of cis:auto during box creation removes last remembered size assert_equals: Sizing normally - clientWidth expected 100 but got 1
FAIL Last remembered size can be removed synchronously assert_equals: Sizing normally - clientWidth expected 100 but got 1
FAIL Disconnected element can briefly keep last remembered size assert_equals: Sizing normally - clientWidth expected 100 but got 1
FAIL Disconnected element ends up losing last remembered size assert_equals: Sizing normally - clientWidth expected 100 but got 1
PASS Disconnected element ends up losing last remembered size even if size was 0x0
FAIL Last remembered size survives becoming inline assert_equals: Still using previous last remembered size - clientWidth expected 100 but got 1
FAIL Last remembered size can be set to 0x0 after losing display:inline assert_equals: Last remembered size is now 0x0 - clientHeight expected 0 but got 2
FAIL Last remembered size can be set to 0x0 after losing display:inline assert_equals: Last remembered size is now 0x0 - clientWidth expected 0 but got 1

@@ -1,4 +1,4 @@

FAIL Last remembered size supports multiple fragments assert_equals: Using last remembered size - fragment #1 height expected 150 but got 1
FAIL Last remembered size is updated when 2nd fragment changes size assert_equals: Using updated last remembered size - fragment #1 height expected 175 but got 1
FAIL Last remembered size supports multiple fragments assert_equals: Using last remembered size - fragment #1 width expected 50 but got 1
FAIL Last remembered size is updated when 2nd fragment changes size assert_equals: Using updated last remembered size - fragment #1 width expected 50 but got 1

@@ -2607,10 +2607,6 @@ void RenderBox::computeLogicalWidthInFragment(LogicalExtentComputedValues& compu
return;
}

// If layout is limited to a subtree, the subtree root's logical width does not change.
if (element() && !view().frameView().layoutContext().isLayoutPending() && view().frameView().layoutContext().subtreeLayoutRoot() == this)
return;

// The parent box is flexing us, so it has increased or decreased our
// width. Use the width from the style context.
// FIXME: Account for block-flow in flexible boxes.

0 comments on commit 06fa3e8

Please sign in to comment.