Do not mix inline and block level boxes.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 May 2019 21:37:56 +0000 (21:37 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 May 2019 21:37:56 +0000 (21:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197462
<rdar://problem/50369362>

Reviewed by Antti Koivisto.

Source/WebCore:

This patch tightens the remove-anonymous-wrappers logic by checking if the removal would
produce an inline-block sibling mix.
When a block level box is removed from the tree, we check if after the removal the anonymous sibling block
boxes are still needed or whether we can removed them as well (and have only inline level child boxes).
In addition to checking if the container is anonymous and is part of a continuation, we also need to check
if collapsing it (and by that moving its children one level up) would cause a inline-block box mix.

Test: fast/ruby/continuation-and-column-spanner-crash.html

* rendering/updating/RenderTreeBuilder.cpp:
(WebCore::RenderTreeBuilder::removeAnonymousWrappersForInlineChildrenIfNeeded):
* rendering/updating/RenderTreeBuilderContinuation.cpp:
(WebCore::RenderTreeBuilder::Continuation::cleanupOnDestroy):

LayoutTests:

* fast/ruby/continuation-and-column-spanner-crash-expected.txt: Added.
* fast/ruby/continuation-and-column-spanner-crash.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245158 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/ruby/continuation-and-column-spanner-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/ruby/continuation-and-column-spanner-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/updating/RenderTreeBuilder.cpp
Source/WebCore/rendering/updating/RenderTreeBuilderContinuation.cpp

index ae0f33b..93408f4 100644 (file)
@@ -1,3 +1,14 @@
+2019-05-08  Zalan Bujtas  <zalan@apple.com>
+
+        Do not mix inline and block level boxes.
+        https://bugs.webkit.org/show_bug.cgi?id=197462
+        <rdar://problem/50369362>
+
+        Reviewed by Antti Koivisto.
+
+        * fast/ruby/continuation-and-column-spanner-crash-expected.txt: Added.
+        * fast/ruby/continuation-and-column-spanner-crash.html: Added.
+
 2019-05-09  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rebaseline test that is now passing after r245088.
diff --git a/LayoutTests/fast/ruby/continuation-and-column-spanner-crash-expected.txt b/LayoutTests/fast/ruby/continuation-and-column-spanner-crash-expected.txt
new file mode 100644 (file)
index 0000000..db07859
--- /dev/null
@@ -0,0 +1,2 @@
+PASS if no crash or assert. 
+
diff --git a/LayoutTests/fast/ruby/continuation-and-column-spanner-crash.html b/LayoutTests/fast/ruby/continuation-and-column-spanner-crash.html
new file mode 100644 (file)
index 0000000..c7a75c9
--- /dev/null
@@ -0,0 +1,10 @@
+PASS if no crash or assert.
+<ruby><rtc><span><details open="false"><span id=span2></span></details></span><div id=div3></div></rtc><rt id=rt2></rt></ruby><script>
+document.body.offsetHeight;
+rt2.remove();
+span2.remove();
+document.body.offsetHeight;
+div3.style.cssText = "column-span: all";
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
index 92d9cea..7de812b 100644 (file)
@@ -1,3 +1,25 @@
+2019-05-09  Zalan Bujtas  <zalan@apple.com>
+
+        Do not mix inline and block level boxes.
+        https://bugs.webkit.org/show_bug.cgi?id=197462
+        <rdar://problem/50369362>
+
+        Reviewed by Antti Koivisto.
+
+        This patch tightens the remove-anonymous-wrappers logic by checking if the removal would
+        produce an inline-block sibling mix.
+        When a block level box is removed from the tree, we check if after the removal the anonymous sibling block
+        boxes are still needed or whether we can removed them as well (and have only inline level child boxes).
+        In addition to checking if the container is anonymous and is part of a continuation, we also need to check
+        if collapsing it (and by that moving its children one level up) would cause a inline-block box mix.
+
+        Test: fast/ruby/continuation-and-column-spanner-crash.html
+
+        * rendering/updating/RenderTreeBuilder.cpp:
+        (WebCore::RenderTreeBuilder::removeAnonymousWrappersForInlineChildrenIfNeeded):
+        * rendering/updating/RenderTreeBuilderContinuation.cpp:
+        (WebCore::RenderTreeBuilder::Continuation::cleanupOnDestroy):
+
 2019-05-09  Eric Carlson  <eric.carlson@apple.com>
 
         Refine AudioSession route sharing policy
index e2cccfb..d37bee8 100644 (file)
@@ -684,15 +684,28 @@ void RenderTreeBuilder::removeAnonymousWrappersForInlineChildrenIfNeeded(RenderE
     // otherwise we can proceed to stripping solitary anonymous wrappers from the inlines.
     // FIXME: We should also handle split inlines here - we exclude them at the moment by returning
     // if we find a continuation.
-    auto* current = blockParent.firstChild();
-    while (current && ((current->isAnonymousBlock() && !downcast<RenderBlock>(*current).isContinuation()) || current->style().isFloating() || current->style().hasOutOfFlowPosition()))
-        current = current->nextSibling();
-
-    if (current)
-        return;
+    Optional<bool> shouldAllChildrenBeInline;
+    for (auto* current = blockParent.firstChild(); current; current = current->nextSibling()) {
+        if (current->style().isFloating() || current->style().hasOutOfFlowPosition())
+            continue;
+        if (!current->isAnonymousBlock() || downcast<RenderBlock>(*current).isContinuation())
+            return;
+        // Anonymous block not in continuation. Check if it holds a set of inline or block children and try not to mix them.
+        auto* firstChild = current->firstChildSlow();
+        if (!firstChild)
+            continue;
+        auto isInlineLevelBox = firstChild->isInline();
+        if (!shouldAllChildrenBeInline.hasValue()) {
+            shouldAllChildrenBeInline = isInlineLevelBox;
+            continue;
+        }
+        // Mixing inline and block level boxes?
+        if (*shouldAllChildrenBeInline != isInlineLevelBox)
+            return;
+    }
 
-    RenderObject* next;
-    for (current = blockParent.firstChild(); current; current = next) {
+    RenderObject* next = nullptr;
+    for (auto* current = blockParent.firstChild(); current; current = next) {
         next = current->nextSibling();
         if (current->isAnonymousBlock())
             blockBuilder().dropAnonymousBoxChild(blockParent, downcast<RenderBlock>(*current));
index c4ca3a6..0cb1229 100644 (file)
@@ -37,8 +37,11 @@ RenderTreeBuilder::Continuation::Continuation(RenderTreeBuilder& builder)
 
 void RenderTreeBuilder::Continuation::cleanupOnDestroy(RenderBoxModelObject& renderer)
 {
-    if (!renderer.continuation() || renderer.isContinuation())
+    if (!renderer.continuation() || renderer.isContinuation()) {
+        if (renderer.hasContinuationChainNode())
+            renderer.removeFromContinuationChain();
         return;
+    }
 
     ASSERT(renderer.hasContinuationChainNode());
     ASSERT(renderer.continuationChainNode());