Skip to content

Commit

Permalink
[@container] Handle containers in the top layer with ::backdrop
Browse files Browse the repository at this point in the history
Don't skip recalc for descendants of elements in the top layer which are
containers. Otherwise the ::backdrop pseudo element box will be inserted
before the top layer element currently being laid out and stay dirty.

Also, avoid retrieving the next layout object pointer before laying out
the previous one. For container queries and top layer, we may detach
dialogs and their ::backdrops from the top layer during layout of the
document tree which precedes them in the LayoutView.

Removed the speculative CHECK introduced for issue 632848 as that was
marked as fixed ages ago.

Bug: 1259707
Change-Id: I126d012e5b31c9c599ddb0884c28a40347dbc57d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3237247
Reviewed-by: Morten Stenshorne <[email protected]>
Commit-Queue: Rune Lillesveen <[email protected]>
Cr-Commit-Position: refs/heads/main@{#935098}
  • Loading branch information
lilles authored and Chromium LUCI CQ committed Oct 26, 2021
1 parent 9609b62 commit cafdd19
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 24 deletions.
9 changes: 9 additions & 0 deletions third_party/blink/renderer/core/dom/element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2908,6 +2908,15 @@ bool Element::SkipStyleRecalcForContainer(
return false;
}
}

// If we are moving the ::backdrop element to the top layer while laying out
// its originating element, it means we will add a layout-dirty box as a
// preceding sibling of the originating element's box which means we will not
// reach the box for ::backdrop during layout. Don't skip style recalc for
// children of containers in the top layer for this reason.
if (IsInTopLayer())
return false;

// Store the child_change so that we can continue interleaved style layout
// from where we left off.
EnsureElementRareData().EnsureContainerQueryData().SkipStyleRecalc(
Expand Down
37 changes: 13 additions & 24 deletions third_party/blink/renderer/core/layout/layout_block_flow.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1608,40 +1608,29 @@ void LayoutBlockFlow::LayoutBlockChildren(bool relayout_children,
// It doesn't get included in the normal layout process but is instead skipped
LayoutObject* child_to_exclude =
LayoutSpecialExcludedChild(relayout_children, layout_scope);

// TODO(foolip): Speculative CHECKs to crash if any non-LayoutBox
// children ever appear, the childrenInline() check at the call site
// should make this impossible. crbug.com/632848
LayoutObject* first_child = FirstChild();
CHECK(!first_child || first_child->IsBox());
auto* next = To<LayoutBox>(first_child);
LayoutBox* last_normal_flow_child = nullptr;

while (next) {
LayoutBox* child = next;
LayoutObject* next_sibling = child->NextSibling();
CHECK(!next_sibling || next_sibling->IsBox());
next = To<LayoutBox>(next_sibling);

for (auto* child = FirstChild(); child; child = child->NextSibling()) {
child->SetShouldCheckForPaintInvalidation();

if (child_to_exclude == child)
continue; // Skip this child, since it will be positioned by the
// specialized subclass (fieldsets and ruby runs).

UpdateBlockChildDirtyBitsBeforeLayout(relayout_children, *child);
LayoutBox* box = To<LayoutBox>(child);
UpdateBlockChildDirtyBitsBeforeLayout(relayout_children, *box);

if (child->IsOutOfFlowPositioned()) {
child->ContainingBlock()->InsertPositionedObject(child);
AdjustPositionedBlock(*child, layout_info);
if (box->IsOutOfFlowPositioned()) {
box->ContainingBlock()->InsertPositionedObject(box);
AdjustPositionedBlock(*box, layout_info);
continue;
}
if (child->IsFloating()) {
InsertFloatingObject(*child);
if (box->IsFloating()) {
InsertFloatingObject(*box);
AdjustFloatingBlock(margin_info);
continue;
}
if (child->IsColumnSpanAll()) {
if (box->IsColumnSpanAll()) {
// This is not the containing block of the spanner. The spanner's
// placeholder will lay it out in due course. For now we just need to
// consult our flow thread, so that the columns (if any) preceding and
Expand All @@ -1651,15 +1640,15 @@ void LayoutBlockFlow::LayoutBlockChildren(bool relayout_children,
SetLogicalHeight(LogicalHeight() + margin_info.Margin());
margin_info.ClearMargin();

child->SpannerPlaceholder()->FlowThread()->SkipColumnSpanner(
child, OffsetFromLogicalTopOfFirstPage() + LogicalHeight());
box->SpannerPlaceholder()->FlowThread()->SkipColumnSpanner(
box, OffsetFromLogicalTopOfFirstPage() + LogicalHeight());
continue;
}

// Lay out the child.
LayoutBlockChild(*child, layout_info);
LayoutBlockChild(*box, layout_info);
layout_info.ClearIsAtFirstInFlowChild();
last_normal_flow_child = child;
last_normal_flow_child = box;
}

// Now do the handling of the bottom of the block, adding in our bottom
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<!doctype html>
<title>@container with modal dialog child</title>
<link rel="help" href="https://drafts.csswg.org/css-contain-3/#container-queries">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style>
#container {
container-type: inline-size;
}
dialog {
color: red;
}
@container (max-width: 200px) {
dialog { color: green; }
}
@container (max-width: 100px) {
dialog { color: lime; }
}
</style>
<div id="container">
<dialog id="dialog"></dialog>
</div>
<script>
test(() => {
assert_equals(getComputedStyle(dialog).color, "rgb(255, 0, 0)");
}, "#container initially wider than 200px");

test(() => {
container.style.width = "200px";
assert_equals(getComputedStyle(dialog).color, "rgb(0, 128, 0)");
}, "#container changed to 200px");

test(() => {
dialog.showModal();
assert_equals(getComputedStyle(dialog).color, "rgb(0, 128, 0)");
}, "Modal dialog still has parent as query container while in top layer");

test(() => {
container.style.width = "100px";
assert_equals(getComputedStyle(dialog).color, "rgb(0, 255, 0)");
}, "Container changes width while dialog is in top layer");
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<!doctype html>
<title>Top layer element as a @container</title>
<link rel="help" href="https://drafts.csswg.org/css-contain-3/#container-queries">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style>
#parent { width: 100px; }
#dialog {
container-type: inline-size;
width: auto;
border: none;
}
#child { color: red; }
@container (min-width: 200px) {
#child { color: green; }
}
</style>
<div id="parent">
<dialog id="dialog"><span id="child"></span></dialog>
</div>
<script>
test(() => {
assert_equals(getComputedStyle(child).color, "rgb(255, 0, 0)");
}, "#dialog initially sized by #containing-block");

test(() => {
dialog.showModal();
assert_equals(getComputedStyle(child).color, "rgb(0, 128, 0)");
}, "#dialog sized by viewport");
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<!doctype html>
<html style="background:green">
<title>CSS Test Reference</title>
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<!doctype html>
<title>::backdrop depending on @container</title>
<link rel="help" href="https://drafts.csswg.org/css-contain-3/#container-queries">
<link rel="match" href="top-layer-003-ref.html">
<style>
html { background: green; }
#container { container-type: inline-size; }
@container (max-width: 200px) {
::backdrop { display: none; }
#dialog { visibility: hidden; }
}
</style>
<div id="container">
<dialog id="dialog"></dialog>
</div>
<script>
dialog.showModal();
dialog.offsetTop;
container.style.width = "100px";
</script>

0 comments on commit cafdd19

Please sign in to comment.