Commit 7a1491ed authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Commit Bot

Never escape replaced "containing block" when adding %-height descendant.

Turns out that the DCHECK added added with
https://chromium-review.googlesource.com/c/chromium/src/+/1622806
can fail. Use a different approach, where we never allow skipping to
ancestors of an replaced element when adding percentage height
descendants of said replaced element.

Bug: 969429
Change-Id: I3380ed24ab3b2767bf79e975b1c4083a3840c3a5
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1640642Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665600}
parent a7771e73
...@@ -1115,13 +1115,13 @@ void LayoutBlock::RemovePositionedObjects( ...@@ -1115,13 +1115,13 @@ void LayoutBlock::RemovePositionedObjects(
void LayoutBlock::AddPercentHeightDescendant(LayoutBox* descendant) { void LayoutBlock::AddPercentHeightDescendant(LayoutBox* descendant) {
// A replaced object is incapable of properly acting as a containing block for // A replaced object is incapable of properly acting as a containing block for
// its children (this is an issue with VIDEO elements, for instance, which // its children. This is an issue with VIDEO elements, for instance, which
// inserts some percentage height flexbox children). Assert that the // insert some percentage height flexbox children. It is also very easily
// descendant hasn't escaped from within a replaced object. Registering the // achievable with a foreignObject inside an SVG. Detect this situation and
// percentage height descendant further up in the tree is only going to cause // bail. The assumption is that there is no situation where we require quirky
// trouble, especially if the replaced object is out-of-flow positioned (and // percentage height behavior inside replaced content.
// we failed to notice). if (UNLIKELY(descendant->Container()->IsLayoutReplaced()))
DCHECK(!descendant->Container()->IsLayoutReplaced()); return;
if (descendant->PercentHeightContainer()) { if (descendant->PercentHeightContainer()) {
if (descendant->PercentHeightContainer() == this) { if (descendant->PercentHeightContainer() == this) {
......
...@@ -3793,13 +3793,7 @@ LayoutUnit LayoutBox::ComputePercentageLogicalHeight( ...@@ -3793,13 +3793,7 @@ LayoutUnit LayoutBox::ComputePercentageLogicalHeight(
&cb, &skipped_auto_height_containing_block); &cb, &skipped_auto_height_containing_block);
DCHECK(cb); DCHECK(cb);
cb->AddPercentHeightDescendant(const_cast<LayoutBox*>(this));
// If the container of the descendant is a replaced element (a VIDEO, for
// instance), |cb| (which uses ContainingBlock()) may actually not be in the
// containing block chain for the descendant.
const LayoutObject* container = Container();
if (!container->IsLayoutReplaced())
cb->AddPercentHeightDescendant(const_cast<LayoutBox*>(this));
if (available_height == -1) if (available_height == -1)
return available_height; return available_height;
......
<!DOCTYPE html>
<svg>
<foreignObject style="height:100%;">
<object style="max-height:100%;"></object>
</foreignObject>
</svg>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script>
test(()=> { }, "did 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