Merge anonymous table boxes when appropriate.
When a table part box is removed from the layout tree, we may end up
with two anonymous layout part siblings which needs to be merged. We
move the children of the latter into the former. When children of the
latter are themselves anonymous, skip down the layout tree until we find
non-anonymous boxes. This will make sure we merge multiple levels of
anonymous boxes although it means we will have to recreate the anonymous
ancestors of the moved box when the preceding box we are moving it to
does not have anonymous descendants.
Bug: 181374
Change-Id: I8a14cec2e036fb4e5b937f89c4a35411dcc98785
Reviewed-on: https://chromium-review.googlesource.com/846861
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526969}diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations
index b99984926..3b99651c 100644
--- a/third_party/WebKit/LayoutTests/TestExpectations
+++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -2777,7 +2777,6 @@
# ====== Begin of display: contents tests ======
crbug.com/795217 external/wpt/css/css-display/display-contents-details.html [ Failure ]
-crbug.com/181374 external/wpt/css/css-display/display-contents-dynamic-table-001-inline.html [ Failure ]
# ====== End of display: contents tests ======
diff --git a/third_party/WebKit/LayoutTests/external/wpt/css/css-tables/fixup-dynamic-anonymous-inline-table-001.html b/third_party/WebKit/LayoutTests/external/wpt/css/css-tables/fixup-dynamic-anonymous-inline-table-001.html
new file mode 100644
index 0000000..9bf7b4c
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/external/wpt/css/css-tables/fixup-dynamic-anonymous-inline-table-001.html
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<title>CSS Test: CSS Tables fixup merge anonymous inline table siblings (row-group + row-group)</title>
+<link rel="author" title="Rune Lillesveen" href="mailto:futhark@chromium.org">
+<link rel="match" href="../reference/ref-filled-green-100px-square-only.html">
+<link rel="help" href="https://drafts.csswg.org/css-tables/#fixup-algorithm">
+<style>
+ .group {
+ display: table-row-group;
+ }
+ .cell {
+ display: table-cell;
+ }
+ .filler {
+ width: 100px;
+ height: 50px;
+ background-color: green;
+ }
+</style>
+<p>Test passes if there is a filled green square.</p>
+<span>
+ <span class="group">
+ <span class="cell">
+ <div class="filler"></div>
+ </span>
+ </span>
+ <span id="rm">Remove me</span>
+ <span class="group">
+ <span class="cell">
+ <div class="filler"></div>
+ </span>
+ </span>
+</span>
+<script>
+ rm.offsetTop;
+ rm.remove();
+</script>
diff --git a/third_party/WebKit/LayoutTests/external/wpt/css/css-tables/fixup-dynamic-anonymous-inline-table-002.html b/third_party/WebKit/LayoutTests/external/wpt/css/css-tables/fixup-dynamic-anonymous-inline-table-002.html
new file mode 100644
index 0000000..9f3a2ff
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/external/wpt/css/css-tables/fixup-dynamic-anonymous-inline-table-002.html
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<title>CSS Test: CSS Tables fixup merge anonymous inline table siblings (cell + row)</title>
+<link rel="author" title="Rune Lillesveen" href="mailto:futhark@chromium.org">
+<link rel="match" href="../reference/ref-filled-green-100px-square-only.html">
+<link rel="help" href="https://drafts.csswg.org/css-tables/#fixup-algorithm">
+<style>
+ .row {
+ display: table-row;
+ }
+ .cell {
+ display: table-cell;
+ }
+ .filler {
+ width: 100px;
+ height: 50px;
+ background-color: green;
+ }
+</style>
+<p>Test passes if there is a filled green square.</p>
+<span>
+ <span class="cell">
+ <div class="filler"></div>
+ </span>
+ <span id="rm">Remove me</span>
+ <span class="row">
+ <span class="cell">
+ <div class="filler"></div>
+ </span>
+ </span>
+</span>
+<script>
+ rm.offsetTop;
+ rm.remove();
+</script>
diff --git a/third_party/WebKit/LayoutTests/external/wpt/css/css-tables/fixup-dynamic-anonymous-inline-table-003.html b/third_party/WebKit/LayoutTests/external/wpt/css/css-tables/fixup-dynamic-anonymous-inline-table-003.html
new file mode 100644
index 0000000..42b7f9a0
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/external/wpt/css/css-tables/fixup-dynamic-anonymous-inline-table-003.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<title>CSS Test: CSS Tables fixup merge anonymous inline table siblings (cell + cell)</title>
+<link rel="author" title="Rune Lillesveen" href="mailto:futhark@chromium.org">
+<link rel="match" href="../reference/ref-filled-green-100px-square-only.html">
+<link rel="help" href="https://drafts.csswg.org/css-tables/#fixup-algorithm">
+<style>
+ .cell {
+ display: table-cell;
+ }
+ .filler {
+ width: 50px;
+ height: 100px;
+ background-color: green;
+ }
+</style>
+<p>Test passes if there is a filled green square.</p>
+<div style="width:100px">
+ <span>
+ <span class="cell">
+ <div class="filler"></div>
+ </span>
+ <span id="rm">Remove me</span>
+ <span class="cell">
+ <div class="filler"></div>
+ </span>
+ </span>
+</div>
+<script>
+ rm.offsetTop;
+ rm.remove();
+</script>
diff --git a/third_party/WebKit/LayoutTests/external/wpt/css/css-tables/fixup-dynamic-anonymous-table-001.html b/third_party/WebKit/LayoutTests/external/wpt/css/css-tables/fixup-dynamic-anonymous-table-001.html
new file mode 100644
index 0000000..d9dbc51
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/external/wpt/css/css-tables/fixup-dynamic-anonymous-table-001.html
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<title>CSS Test: CSS Tables fixup merge anonymous table siblings (cell + cell)</title>
+<link rel="author" title="Rune Lillesveen" href="mailto:futhark@chromium.org">
+<link rel="match" href="../reference/ref-filled-green-100px-square-only.html">
+<link rel="help" href="https://drafts.csswg.org/css-tables/#fixup-algorithm">
+<style>
+ .cell {
+ display: table-cell;
+ width: 50px;
+ height: 100px;
+ background-color: green;
+ }
+</style>
+<p>Test passes if there is a filled green square.</p>
+<div>
+ <span class="cell"></span>
+ <span id="rm">Remove me</span>
+ <span class="cell"></span>
+</div>
+<script>
+ rm.offsetTop;
+ rm.remove();
+</script>
diff --git a/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp b/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
index 406b1f6..940405d0 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
@@ -1409,10 +1409,7 @@
if (full_remove_insert && IsLayoutBlock() && child->IsBox())
ToLayoutBox(child)->RemoveFromPercentHeightContainer();
- if (full_remove_insert && (to_box_model_object->IsLayoutBlock() ||
- to_box_model_object->IsLayoutInline())) {
- // Takes care of adding the new child correctly if toBlock and fromBlock
- // have different kind of children (block vs inline).
+ if (full_remove_insert) {
to_box_model_object->AddChild(
VirtualChildren()->RemoveChildNode(this, child), before_child);
} else {
@@ -1473,4 +1470,57 @@
return true;
}
+void LayoutBoxModelObject::MoveNonAnonymousTableDescendantsTo(
+ LayoutBoxModelObject* to_box_model_object) {
+ LayoutObject* child = SlowFirstChild();
+ DCHECK(IsAnonymous());
+ // Append non-anonymous descendants' sub-trees of one anonymous table part to
+ // another. We walk down past anonymous descendants to make sure existing
+ // anonymous table part chains in to_box_model_object are used.
+ while (child) {
+ LayoutObject* next_child = child->NextSibling();
+ if (child->IsAnonymous() && child->IsTablePart()) {
+ ToLayoutBoxModelObject(child)->MoveNonAnonymousTableDescendantsTo(
+ to_box_model_object);
+ child->Destroy();
+ } else {
+ MoveChildTo(to_box_model_object, child, true);
+ // If we move a subtree rooted at a table section or row, we need to mark
+ // the section for cell recalc in case there is a cell descendant.
+ // Normally, boxes in a subtree will be added one by one, and AddChild in
+ // LayoutTableRow will make sure cells are added with AddCell or the table
+ // section marked for cell recalc. Since the cell is just moved along with
+ // its subtree here, we need to mark the section for recalc.
+ if (child->IsTableSection())
+ ToLayoutTableSection(child)->SetNeedsCellRecalc();
+ else if (child->IsTableRow())
+ ToLayoutTableRow(child)->Section()->SetNeedsCellRecalc();
+ }
+ child = next_child;
+ }
+}
+
+void LayoutBoxModelObject::MergeAnonymousTablePartsIfNeeded(
+ LayoutBoxModelObject* prev,
+ LayoutBoxModelObject* next) {
+ if (!prev || !next || !prev->IsAnonymous() || !next->IsAnonymous())
+ return;
+ DCHECK(prev->IsInline() == next->IsInline());
+ next->MoveNonAnonymousTableDescendantsTo(prev);
+ next->Destroy();
+}
+
+void LayoutBoxModelObject::RemoveChild(LayoutObject* old_child) {
+ LayoutObject* prev = old_child->PreviousSibling();
+ LayoutObject* next = old_child->NextSibling();
+
+ LayoutObject::RemoveChild(old_child);
+
+ if (DocumentBeingDestroyed())
+ return;
+
+ if (prev && next && prev->IsTable() && next->IsTable())
+ MergeAnonymousTablePartsIfNeeded(ToLayoutTable(prev), ToLayoutTable(next));
+}
+
} // namespace blink
diff --git a/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.h b/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.h
index f439e1e5..a7992f80 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.h
+++ b/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.h
@@ -435,6 +435,8 @@
void AbsoluteQuads(Vector<FloatQuad>& quads,
MapCoordinatesFlags mode = 0) const override;
+ void RemoveChild(LayoutObject*) override;
+
protected:
// Compute absolute quads for |this|, but not any continuations. May only be
// called for objects which can be or have continuations, i.e. LayoutInline or
@@ -494,6 +496,9 @@
void InvalidateStickyConstraints();
+ void MergeAnonymousTablePartsIfNeeded(LayoutBoxModelObject* prev,
+ LayoutBoxModelObject* next);
+
public:
// These functions are only used internally to manipulate the layout tree
// structure via remove/insert/appendChildNode.
@@ -566,6 +571,8 @@
&LayoutBoxModelObject::BorderTop, &LayoutBoxModelObject::BorderRight,
&LayoutBoxModelObject::BorderBottom, &LayoutBoxModelObject::BorderLeft);
}
+
+ void MoveNonAnonymousTableDescendantsTo(LayoutBoxModelObject*);
};
DEFINE_LAYOUT_OBJECT_TYPE_CASTS(LayoutBoxModelObject, IsBoxModelObject());
diff --git a/third_party/WebKit/Source/core/layout/LayoutTable.cpp b/third_party/WebKit/Source/core/layout/LayoutTable.cpp
index 4e9364fa..0564885 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTable.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutTable.cpp
@@ -233,6 +233,18 @@
section->AddChild(child);
}
+void LayoutTable::RemoveChild(LayoutObject* old_child) {
+ if (!DocumentBeingDestroyed()) {
+ LayoutObject* prev = old_child->PreviousSibling();
+ LayoutObject* next = old_child->NextSibling();
+ if (prev && next && prev->IsTableSection() && next->IsTableSection()) {
+ MergeAnonymousTablePartsIfNeeded(ToLayoutTableSection(prev),
+ ToLayoutTableSection(next));
+ }
+ }
+ LayoutBlock::RemoveChild(old_child);
+}
+
void LayoutTable::AddCaption(const LayoutTableCaption* caption) {
DCHECK_EQ(captions_.Find(caption), kNotFound);
captions_.push_back(const_cast<LayoutTableCaption*>(caption));
diff --git a/third_party/WebKit/Source/core/layout/LayoutTable.h b/third_party/WebKit/Source/core/layout/LayoutTable.h
index e58b798..3825db6 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTable.h
+++ b/third_party/WebKit/Source/core/layout/LayoutTable.h
@@ -500,6 +500,8 @@
void SetIsAnyColumnEverCollapsed() { is_any_column_ever_collapsed_ = true; }
+ void RemoveChild(LayoutObject*) final;
+
// TODO(layout-dev): All mutables in this class are lazily updated by
// recalcSections() which is called by various getter methods (e.g.
// borderBefore(), borderAfter()).
diff --git a/third_party/WebKit/Source/core/layout/LayoutTableRow.cpp b/third_party/WebKit/Source/core/layout/LayoutTableRow.cpp
index a9abe265..bba091c 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTableRow.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutTableRow.cpp
@@ -184,6 +184,16 @@
Section()->SetNeedsCellRecalc();
}
+void LayoutTableRow::RemoveChild(LayoutObject* old_child) {
+ if (!DocumentBeingDestroyed()) {
+ LayoutTableCell* prev_cell =
+ ToLayoutTableCell(old_child->PreviousSibling());
+ LayoutTableCell* next_cell = ToLayoutTableCell(old_child->NextSibling());
+ MergeAnonymousTablePartsIfNeeded(prev_cell, next_cell);
+ }
+ LayoutTableBoxComponent::RemoveChild(old_child);
+}
+
void LayoutTableRow::UpdateLayout() {
DCHECK(NeedsLayout());
LayoutAnalyzer::Scope analyzer(*this);
diff --git a/third_party/WebKit/Source/core/layout/LayoutTableRow.h b/third_party/WebKit/Source/core/layout/LayoutTableRow.h
index 201e457a..254781b 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTableRow.h
+++ b/third_party/WebKit/Source/core/layout/LayoutTableRow.h
@@ -154,6 +154,7 @@
void NextSibling() const = delete;
void PreviousSibling() const = delete;
+ void RemoveChild(LayoutObject*) final;
// This field should never be read directly. It should be read through
// rowIndex() above instead. This is to ensure that we never read this
diff --git a/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp b/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
index 2bab6b5..0749d1e 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
@@ -202,6 +202,15 @@
LayoutTableBoxComponent::AddChild(child, before_child);
}
+void LayoutTableSection::RemoveChild(LayoutObject* old_child) {
+ if (!DocumentBeingDestroyed()) {
+ MergeAnonymousTablePartsIfNeeded(
+ ToLayoutTableRow(old_child->PreviousSibling()),
+ ToLayoutTableRow(old_child->NextSibling()));
+ }
+ LayoutTableBoxComponent::RemoveChild(old_child);
+}
+
static inline void CheckThatVectorIsDOMOrdered(
const Vector<LayoutTableCell*, 1>& cells) {
#ifndef NDEBUG
diff --git a/third_party/WebKit/Source/core/layout/LayoutTableSection.h b/third_party/WebKit/Source/core/layout/LayoutTableSection.h
index 02e68d1e..c02e90bc 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTableSection.h
+++ b/third_party/WebKit/Source/core/layout/LayoutTableSection.h
@@ -303,7 +303,8 @@
return type == kLayoutObjectTableSection || LayoutBox::IsOfType(type);
}
- void WillBeRemovedFromTree() override;
+ void WillBeRemovedFromTree() final;
+ void RemoveChild(LayoutObject*) final;
int BorderSpacingForRow(unsigned row) const {
return grid_[row].row ? Table()->VBorderSpacing() : 0;
