Commit 44945bce authored by Ian Kilpatrick's avatar Ian Kilpatrick Committed by Commit Bot

[LayoutNG] Fix the NGExclusionSpaceInternal vectors going out of sync.

NGExclusion::operator== didn't previous compare shape_data. This turned
out to be a bad optimization. :)

Without this we'd end up in a situation where we would have:
 - An exclusion vector containing an exclusion with shape data.
 - Derived geometry which wasn't tracking shape data.

Bug: 963580
Change-Id: I21746f8fd081ff24658768ee6074122f2e913bd5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1613560
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#661191}
parent ec04e7ff
......@@ -7,7 +7,8 @@
namespace blink {
bool NGExclusion::operator==(const NGExclusion& other) const {
return std::tie(other.rect, other.type) == std::tie(rect, type);
return type == other.type && rect == other.rect &&
shape_data == other.shape_data;
}
} // namespace blink
......@@ -228,8 +228,7 @@ void NGExclusionSpaceInternal::Add(scoped_refptr<const NGExclusion> exclusion) {
bool already_exists = false;
if (num_exclusions_ < exclusions_->data.size()) {
if (*exclusion == *exclusions_->data.at(num_exclusions_) &&
!exclusion->shape_data) {
if (*exclusion == *exclusions_->data.at(num_exclusions_)) {
// We might be adding an exclusion seen in a previous layout pass.
already_exists = true;
} else {
......
......@@ -151,7 +151,11 @@ class CORE_EXPORT NGExclusionSpaceInternal {
const NGExclusionSpaceInternal& other) const {
DCHECK_EQ(num_exclusions_, other.num_exclusions_);
for (wtf_size_t i = 0; i < num_exclusions_; ++i) {
DCHECK(*exclusions_->data.at(i) == *other.exclusions_->data.at(i));
const auto& exclusion = *exclusions_->data.at(i);
const auto& other_exclusion = *other.exclusions_->data.at(i);
DCHECK(exclusion.rect == other_exclusion.rect);
DCHECK_EQ(exclusion.type, other_exclusion.type);
DCHECK_EQ((bool)exclusion.shape_data, (bool)other_exclusion.shape_data);
}
}
#endif
......
<!DOCTYPE html>
<meta name="assert" content="This test passes if the renderer does not crash."/>
<link rel="help" href="https://crbug.com/963580" />
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<div style="float:left; width:200px; height:200px; background: green;"></div>
x
<div style="width:200px; height:200px;">
<div id="target" style="float:left; width:100px; height:100px; shape-outside:polygon(10px 10px, 20px 20px);"></div>
y
</div>
<div style="float:left; width:200px; height:200px;"></div>
<script>
test(() => {
document.body.offsetTop;
target.style.shapeOutside = "none";
document.body.offsetTop;
}, 'Test passes if the renderer 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