Changeset 245158 in webkit

Timestamp:

May 9, 2019, 2:37:56 PM (7 years ago)

Author:

Alan Bujtas

Message:

Do not mix inline and block level boxes.
​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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/ruby/continuation-and-column-spanner-crash-expected.txt (added)
• LayoutTests/fast/ruby/continuation-and-column-spanner-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/updating/RenderTreeBuilder.cpp (modified) (1 diff)
• Source/WebCore/rendering/updating/RenderTreeBuilderContinuation.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp

@@ -685 +685 @@
     // 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;
-
-    RenderObject* next;
-    for (current = blockParent.firstChild(); current; current = next) {
+    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 = nullptr;
+    for (auto* current = blockParent.firstChild(); current; current = next) {
         next = current->nextSibling();
         if (current->isAnonymousBlock())

trunk/Source/WebCore/rendering/updating/RenderTreeBuilderContinuation.cpp

@@ -38 +38 @@
 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());