Commit f55cd2cd authored by Scott Violet's avatar Scott Violet Committed by Commit Bot

css-flexbox: fixes for -webkit-box using LayoutFlexibleBox

This addresses two problems that triggered CHECKs/use-after-free:

LayoutBlock, for -webkit-box, needs to add children to anonymous
blocks in some cases.

LayoutFlexibleBox should not merge anonymous flex items for
-webkit-box.

BUG=1010706
TEST=none

Change-Id: Ic8cb22f4f031813c1e1447bc23f4bfb930b3caec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1838754
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703115}
parent d611deb6
......@@ -308,7 +308,8 @@ void LayoutBlock::AddChildBeforeDescendant(LayoutObject* new_child,
// Insert the child into the anonymous block box instead of here.
if (new_child->IsInline() ||
(new_child->IsFloatingOrOutOfFlowPositioned() &&
!IsFlexibleBoxIncludingNG() && !IsLayoutGrid()) ||
(StyleRef().IsDeprecatedFlexboxUsingFlexLayout() ||
(!IsFlexibleBoxIncludingNG() && !IsLayoutGrid()))) ||
before_descendant->Parent()->SlowFirstChild() != before_descendant) {
before_descendant_container->AddChild(new_child, before_descendant);
} else {
......@@ -350,7 +351,8 @@ void LayoutBlock::AddChild(LayoutObject* new_child,
if (new_child->IsInline() ||
(new_child->IsFloatingOrOutOfFlowPositioned() &&
!IsFlexibleBoxIncludingNG() && !IsLayoutGrid())) {
(StyleRef().IsDeprecatedFlexboxUsingFlexLayout() ||
(!IsFlexibleBoxIncludingNG() && !IsLayoutGrid())))) {
// If we're inserting an inline child but all of our children are blocks,
// then we have to make sure it is put into an anomyous block box. We try to
// use an existing anonymous box if possible, otherwise a new one is created
......
......@@ -46,6 +46,7 @@
#include "third_party/blink/renderer/core/style/computed_style.h"
#include "third_party/blink/renderer/platform/geometry/length_functions.h"
#include "third_party/blink/renderer/platform/instrumentation/use_counter.h"
#include "third_party/blink/renderer/platform/runtime_enabled_features.h"
#include "third_party/blink/renderer/platform/wtf/math_extras.h"
namespace blink {
......@@ -273,8 +274,10 @@ void LayoutFlexibleBox::MergeAnonymousFlexItems(LayoutObject* remove_child) {
}
void LayoutFlexibleBox::RemoveChild(LayoutObject* child) {
if (!DocumentBeingDestroyed())
if (!DocumentBeingDestroyed() &&
!StyleRef().IsDeprecatedFlexboxUsingFlexLayout()) {
MergeAnonymousFlexItems(child);
}
LayoutBlock::RemoveChild(child);
intrinsic_size_along_main_axis_.erase(child);
......
......@@ -255,11 +255,8 @@ LayoutObject* LayoutObject::CreateObject(Element* element,
return LayoutObjectFactory::CreateTableCaption(*element, style, legacy);
case EDisplay::kWebkitBox:
case EDisplay::kWebkitInlineBox:
if (RuntimeEnabledFeatures::WebkitBoxLayoutUsesFlexLayoutEnabled() &&
(!style.HasLineClamp() ||
style.BoxOrient() == EBoxOrient::kHorizontal)) {
if (style.IsDeprecatedFlexboxUsingFlexLayout())
return new LayoutFlexibleBox(element);
}
return new LayoutDeprecatedFlexibleBox(*element);
case EDisplay::kFlex:
case EDisplay::kInlineFlex:
......
......@@ -1203,6 +1203,11 @@ class ComputedStyle : public ComputedStyleBase,
return Display() == EDisplay::kWebkitBox ||
Display() == EDisplay::kWebkitInlineBox;
}
bool IsDeprecatedFlexboxUsingFlexLayout() const {
return IsDeprecatedWebkitBox() &&
RuntimeEnabledFeatures::WebkitBoxLayoutUsesFlexLayoutEnabled() &&
(!HasLineClamp() || BoxOrient() == EBoxOrient::kHorizontal);
}
// Variables.
bool HasVariables() const;
......@@ -2091,7 +2096,6 @@ class ComputedStyle : public ComputedStyleBase,
bool IsDisplayFlexibleOrGridBox() const {
return IsDisplayFlexibleBox(Display()) || IsDisplayGridBox(Display());
}
bool IsDisplayFlexibleBox() const { return IsDisplayFlexibleBox(Display()); }
bool IsDisplayLayoutCustomBox() const {
return IsDisplayLayoutCustomBox(Display());
}
......
<!DOCTYPE html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<div style="display: -webkit-box">
<span><div></div></span>
<div id="target"></div>
text
</div>
<script>
// Force a layout before removing.
document.body.offsetTop;
document.getElementById('target').remove();
done();
</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