Commit 0b0e2c2a authored by Ian Kilpatrick's avatar Ian Kilpatrick Committed by Commit Bot

[LayoutNGFragmentPaint] Fix containing-block of OOF-positioned objects.

We had an issue in the existing invalidation code when an object could
contain an OOF-positioned node, but wasn't a LayoutBlock.

This already happens when we have a LayoutInline being a
containing-block, but there are other cases where this is true.

Clusterfuzz found that LayoutTableSection falls into this category.
E.g.
The OOF-positioned node would be inserted into the nearest
containing-block (the anonymous LayoutTable in this case).

When it stopped being a containing-block, the OOF-positioned node was
never removed from the LayoutTable.

This caused a crash when the OOF's layout was invalidated.
The OOF marked itself, and the LayoutView (its new containing block)
for layout.

But the LayoutView didn't know it had this as an OOF-positioned child.

This patch moves the current logic within LayoutBlock into
LayoutBoxModelObject.

Bug: 1021491, 1021676, 1022545
Change-Id: I0f0b4c8aa655fc7edca5d79379205a8d445713d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1906708Reviewed-by: default avatarAleks Totic <atotic@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714487}
parent d9548dd1
......@@ -159,46 +159,7 @@ void LayoutBlock::WillBeDestroyed() {
void LayoutBlock::StyleWillChange(StyleDifference diff,
const ComputedStyle& new_style) {
const ComputedStyle* old_style = Style();
SetIsAtomicInlineLevel(new_style.IsDisplayInlineType());
if (old_style && Parent()) {
bool old_style_contains_fixed_position = ComputeIsFixedContainer(old_style);
bool old_style_contains_absolute_position =
ComputeIsAbsoluteContainer(old_style);
bool new_style_contains_fixed_position =
ComputeIsFixedContainer(&new_style);
bool new_style_contains_absolute_position =
ComputeIsAbsoluteContainer(&new_style);
if ((old_style_contains_fixed_position &&
!new_style_contains_fixed_position) ||
(old_style_contains_absolute_position &&
!new_style_contains_absolute_position)) {
// Clear our positioned objects list. Our absolute and fixed positioned
// descendants will be inserted into our containing block's positioned
// objects list during layout.
RemovePositionedObjects(nullptr, kNewContainingBlock);
}
if (!old_style_contains_absolute_position &&
new_style_contains_absolute_position) {
// Remove our absolutely positioned descendants from their current
// containing block.
// They will be inserted into our positioned objects list during layout.
if (LayoutBlock* cb = ContainingBlockForAbsolutePosition())
cb->RemovePositionedObjects(this, kNewContainingBlock);
}
if (!old_style_contains_fixed_position &&
new_style_contains_fixed_position) {
// Remove our fixed positioned descendants from their current containing
// block.
// They will be inserted into our positioned objects list during layout.
if (LayoutBlock* cb = ContainingBlockForFixedPosition())
cb->RemovePositionedObjects(this, kNewContainingBlock);
}
}
LayoutBox::StyleWillChange(diff, new_style);
}
......
......@@ -329,9 +329,11 @@ void LayoutBoxModelObject::StyleDidChange(StyleDifference diff,
}
}
if (old_style &&
(could_contain_fixed != CanContainFixedPositionObjects() ||
could_contain_absolute != CanContainAbsolutePositionObjects())) {
bool can_contain_fixed = CanContainFixedPositionObjects();
bool can_contain_absolute = CanContainAbsolutePositionObjects();
if (old_style && (could_contain_fixed != can_contain_fixed ||
could_contain_absolute != can_contain_absolute)) {
// If out of flow element containment changed, then we need to force a
// subtree paint property update, since the children elements may now be
// referencing a different container.
......@@ -347,6 +349,32 @@ void LayoutBoxModelObject::StyleDidChange(StyleDifference diff,
Layer()->SetNeedsCompositingInputsUpdate();
}
if (old_style && Parent()) {
LayoutBlock* block = FindNonAnonymousContainingBlock(this);
if ((could_contain_fixed && !can_contain_fixed) ||
(could_contain_absolute && !can_contain_absolute)) {
// Clear our positioned objects list. Our absolute and fixed positioned
// descendants will be inserted into our containing block's positioned
// objects list during layout.
block->RemovePositionedObjects(nullptr, kNewContainingBlock);
}
if (!could_contain_absolute && can_contain_absolute) {
// Remove our absolute positioned descendants from their current
// containing block.
// They will be inserted into our positioned objects list during layout.
if (LayoutBlock* cb = block->ContainingBlockForAbsolutePosition())
cb->RemovePositionedObjects(this, kNewContainingBlock);
}
if (!could_contain_fixed && can_contain_fixed) {
// Remove our fixed positioned descendants from their current containing
// block.
// They will be inserted into our positioned objects list during layout.
if (LayoutBlock* cb = block->ContainingBlockForFixedPosition())
cb->RemovePositionedObjects(this, kNewContainingBlock);
}
}
if (Layer()) {
Layer()->StyleDidChange(diff, old_style);
if (had_layer && Layer()->IsSelfPaintingLayer() != layer_was_self_painting)
......
......@@ -311,23 +311,6 @@ void LayoutInline::StyleDidChange(StyleDifference diff,
}
}
bool old_style_is_containing_block = ComputeIsAbsoluteContainer(old_style);
bool new_style_is_containing_block = CanContainAbsolutePositionObjects();
// If we are changing to/from static, we need to reposition
// out-of-flow positioned descendants.
if (old_style_is_containing_block != new_style_is_containing_block) {
LayoutBlock* abs_containing_block = nullptr;
if (!old_style_is_containing_block) {
abs_containing_block = ContainingBlockForAbsolutePosition();
} else {
// When position was not static, containingBlockForAbsolutePosition
// for our children is our existing containingBlock.
abs_containing_block = FindNonAnonymousContainingBlock(this);
}
if (abs_containing_block)
abs_containing_block->RemovePositionedObjects(this, kNewContainingBlock);
}
PropagateStyleToAnonymousChildren();
}
......
<!DOCTYPE html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<link rel="help" href="https://crbug.com/1021676">
<div id="tfoot" style="display: table-footer-group; transform: scale(2);">
<div id="oof" style="position: absolute;">text</div>
</div>
<script>
test(() => {
document.body.offsetTop;
// Make the ICB the containing-block.
document.getElementById('tfoot').style.transform = '';
document.body.offsetTop;
document.getElementById('oof').innerText = '';
document.body.offsetTop;
}, 'test passes if it does not crash');
</script>
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