From 39402a8e661deb922d47e8b9632212e760e40494 Mon Sep 17 00:00:00 2001 From: Morten Stenshorne Date: Thu, 23 May 2019 21:17:02 +0000 Subject: [PATCH] Don't add %-height descendants outside the containing block chain. A video element creates some children (flexboxes, etc.), and the video ought to be their containing "block" for all purposes, but LayoutVideo is not a LayoutBlock (it's LayoutReplaced), so it doesn't really work. Make sure that we don't skip upwards and escape from the innards of the video element and start adding percentage descendants to blocks that are not in the containing block chain of the video (the video may be out-of-flow positioned). Bug: 965032 Change-Id: Ic9fdb7b9dcff5724b9488d85dfafc9c87f4bee62 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1622806 Reviewed-by: Emil A Eklund Commit-Queue: Christian Biesinger Cr-Commit-Position: refs/heads/master@{#662821} --- .../blink/renderer/core/layout/layout_block.cc | 9 +++++++++ .../blink/renderer/core/layout/layout_box.cc | 8 +++++++- ...-percentage-height-video-inline-fc-crash.html | 16 ++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 third_party/blink/web_tests/fast/replaced/abspos-percentage-height-video-inline-fc-crash.html diff --git a/third_party/blink/renderer/core/layout/layout_block.cc b/third_party/blink/renderer/core/layout/layout_block.cc index 84ce09c90b2c6f..54b0485d5ce3ed 100644 --- a/third_party/blink/renderer/core/layout/layout_block.cc +++ b/third_party/blink/renderer/core/layout/layout_block.cc @@ -1114,6 +1114,15 @@ void LayoutBlock::RemovePositionedObjects( } void LayoutBlock::AddPercentHeightDescendant(LayoutBox* descendant) { + // 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 + // inserts some percentage height flexbox children). Assert that the + // descendant hasn't escaped from within a replaced object. Registering the + // percentage height descendant further up in the tree is only going to cause + // trouble, especially if the replaced object is out-of-flow positioned (and + // we failed to notice). + DCHECK(!descendant->Container()->IsLayoutReplaced()); + if (descendant->PercentHeightContainer()) { if (descendant->PercentHeightContainer() == this) { DCHECK(HasPercentHeightDescendant(descendant)); diff --git a/third_party/blink/renderer/core/layout/layout_box.cc b/third_party/blink/renderer/core/layout/layout_box.cc index a9ccb49753e8bb..184298b76cec09 100644 --- a/third_party/blink/renderer/core/layout/layout_box.cc +++ b/third_party/blink/renderer/core/layout/layout_box.cc @@ -3796,7 +3796,13 @@ LayoutUnit LayoutBox::ComputePercentageLogicalHeight( &cb, &skipped_auto_height_containing_block); DCHECK(cb); - cb->AddPercentHeightDescendant(const_cast(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(this)); if (available_height == -1) return available_height; diff --git a/third_party/blink/web_tests/fast/replaced/abspos-percentage-height-video-inline-fc-crash.html b/third_party/blink/web_tests/fast/replaced/abspos-percentage-height-video-inline-fc-crash.html new file mode 100644 index 00000000000000..0adb27a4a08525 --- /dev/null +++ b/third_party/blink/web_tests/fast/replaced/abspos-percentage-height-video-inline-fc-crash.html @@ -0,0 +1,16 @@ + +
+
+
+
+ +
+
+ + +