Skip to content

GitLab

T
tangled
Commit 6e1680b7 authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot
Browse files
Options

[LayoutNG] Fix |ComputeMinMaxSize| not to clear |NeedsLayout| 

This patch fixes |ComputeMinMaxSize| not to clear |NeedsLayout|
when all of following conditions are met:
1. Needs |MinMaxSize|
2. Is an orthogonal flow root.
3. Is legacy layout.
4. Change is applied that dirties the layout.

In this case, |NGBlockNode::ComputeMinMaxSize()| calls
|Layout()|, which goes to |RunLegacyLayout()|, which calls
|LayoutObject::LayoutIfNeeded()|. However, its constraint
space is intermediate that it does not update
|CachedLayoutResult|.

Then when laying it out, because |NeedsLayout()| is cleared,
existing out-of-date |CachedLayoutResult| is used.

This patch fixes:
1. Not to pre-layout orthogonal roots if it has
   |CachedLayoutResult|.
2. Set |NeedsLayout()| if |RunLegacyLayout()| clears it, but
   |CachedLayoutResult| was not updated.

Bug: 976859
Change-Id: Ic2d979a3c4f9479040bfd0e54cf4283b60ed52b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1670146Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671460}
parent 08f005a8
Hide whitespace changes
Inline Side-by-side
Showing with 54 additions and 6 deletions
third_party/blink/renderer/core/frame/local_frame_view.cc
View file @ 6e1680b7
......@@ -1520,12 +1520,23 @@ static bool PrepareOrthogonalWritingModeRootForLayout(LayoutObject& root) {
ToLayoutBox(root).IsGridItem() || root.IsTablePart())
return false;
// Do not pre-layout objects that are fully managed by LayoutNG; it is not
// necessary and may lead to double layouts. We do need to pre-layout
// objects whose containing block is a legacy object so that it can
// properly compute its intrinsic size.
if (RuntimeEnabledFeatures::LayoutNGEnabled() && IsManagedByLayoutNG(root))
return false;
if (RuntimeEnabledFeatures::LayoutNGEnabled()) {
// Do not pre-layout objects that are fully managed by LayoutNG; it is not
// necessary and may lead to double layouts. We do need to pre-layout
// objects whose containing block is a legacy object so that it can
// properly compute its intrinsic size.
if (IsManagedByLayoutNG(root))
return false;
// If the root is legacy but has |CachedLayoutResult|, its parent is NG,
// which called |RunLegacyLayout()|. This parent not only needs to run
// pre-layout, but also clearing |NeedsLayout()| without updating
// |CachedLayoutResult| is harmful.
if (const auto* box = ToLayoutBoxOrNull(&root)) {
if (box->GetCachedLayoutResult())
return false;
}
}
RemoveFloatingObjectsForSubtreeRoot(root);
return true;
......
third_party/blink/renderer/core/layout/ng/ng_block_node.cc
View file @ 6e1680b7
......@@ -1007,6 +1007,7 @@ scoped_refptr<const NGLayoutResult> NGBlockNode::RunLegacyLayout(
// Using |LayoutObject::LayoutIfNeeded| save us a little bit of overhead,
// compared to |LayoutObject::ForceLayout|.
DCHECK(!box_->IsLayoutNGMixin());
bool needed_layout = box_->NeedsLayout();
if (box_->NeedsLayout() && !needs_force_relayout)
box_->LayoutIfNeeded();
else
......@@ -1034,6 +1035,12 @@ scoped_refptr<const NGLayoutResult> NGBlockNode::RunLegacyLayout(
layout_result = builder.ToBoxFragment();
box_->SetCachedLayoutResult(*layout_result, /* break_token */ nullptr);
// If |SetCachedLayoutResult| did not update cached |LayoutResult|,
// |NeedsLayout()| flag should not be cleared.
if (needed_layout && layout_result != box_->GetCachedLayoutResult()) {
box_->SetNeedsLayout(layout_invalidation_reason::kUnknown);
}
} else if (layout_result) {
// OOF-positioned nodes have a two-tier cache, and their layout results
// must always contain the correct percentage resolution size.
......
third_party/blink/web_tests/external/wpt/css/css-sizing/min-max-content-orthogonal-flow-crash-001.html 0 → 100644
View file @ 6e1680b7
<!DOCTYPE html>
<title>CSS Test: Check computing min-/max-content does not cause crash</title>
<link rel="help" href="https://crbug.com/976859">
<link rel="help" href="https://drafts.csswg.org/css-sizing-3/#sizing-values">
<link rel="author" title="Koji Ishii" href="mailto:kojii@chromium.org">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style>
body {
width: fit-content;
}
#target {
display: block;
writing-mode: vertical-lr;
columns: 2;
}
.after #target {
float: left;
}
</style>
<body>
<div id="target"></div>
<script>
test(() => {
const body = document.body;
body.offsetTop;
body.classList.add("after");
});
</script>
</body>
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