Changeset 227570 in webkit

Timestamp:

Jan 24, 2018 2:20:55 PM (6 years ago)

Author:

Antti Koivisto

Message:

Assertion failure in RenderMultiColumnSet::requiresBalancing() on fast/multicol/spanner-crash-when-adding-summary.html
​https://bugs.webkit.org/show_bug.cgi?id=179308
<rdar://problem/34592771>

Reviewed by Zalan Bujtas.

Source/WebCore:

The issue here is that we fail to tear down render tree for a summary element because adding another summary element
takes it out of the composed tree. This leaves behind renderers that break some multicolumn assumptions.

• rendering/updating/RenderTreeUpdater.cpp:

(WebCore::RenderTreeUpdater::tearDownRenderers):
(WebCore::RenderTreeUpdater::tearDownLeftoverShadowHostChildren):

When tearing down renderers go through the real children of the shadow hosts at the end and see if we left any renderers behind.
If so, tear them down too.

• rendering/updating/RenderTreeUpdater.h:

LayoutTests:

Unskip fast/multicol/spanner-crash-when-adding-summary.html

• platform/ios/TestExpectations:
• platform/mac/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/ios/TestExpectations (modified) (1 diff)
• LayoutTests/platform/mac/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/updating/RenderTreeUpdater.cpp (modified) (2 diffs)
• Source/WebCore/rendering/updating/RenderTreeUpdater.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2018-01-24  Antti Koivisto  <antti@apple.com>
+
+        Assertion failure in RenderMultiColumnSet::requiresBalancing() on fast/multicol/spanner-crash-when-adding-summary.html
+        https://bugs.webkit.org/show_bug.cgi?id=179308
+        <rdar://problem/34592771>
+
+        Reviewed by Zalan Bujtas.
+
+        Unskip fast/multicol/spanner-crash-when-adding-summary.html
+
+        * platform/ios/TestExpectations:
+        * platform/mac/TestExpectations:
+
 2018-01-24  Daniel Bates  <dabates@apple.com>
 

trunk/LayoutTests/platform/ios/TestExpectations

@@ -3060 +3060 @@
 webkit.org/b/172052 [ Release ] imported/w3c/web-platform-tests/html/webappapis/timers/type-long-setinterval.html [ Pass Failure ]
 
-webkit.org/b/176878 [ Debug ] fast/multicol/spanner-crash-when-adding-summary.html [ Skip ]
-
 # <rdar://problem/32542437> REGRESSION (iOS 11): LayoutTest fast/events/ios/before-input-events-prevent-candidate-insertion.html is timing out
 fast/events/ios/before-input-events-prevent-candidate-insertion.html [ Timeout ]

trunk/LayoutTests/platform/mac/TestExpectations

@@ -1587 +1587 @@
 webkit.org/b/172052 [ Debug ] imported/w3c/web-platform-tests/html/webappapis/timers/type-long-setinterval.html [ Pass Failure ]
 
-webkit.org/b/176878 [ Debug ] fast/multicol/spanner-crash-when-adding-summary.html [ Skip ]
-
 # <rdar://problem/29031509> REGRESSION? (FontParser-195): svg/W3C-SVG-1.1/fonts-elem-* and svg/W3C-SVG-1.1/text-intro-* tests failing
 [ HighSierra+ ] svg/W3C-SVG-1.1/fonts-elem-01-t.svg [ Failure ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-01-24  Antti Koivisto  <antti@apple.com>
+
+        Assertion failure in RenderMultiColumnSet::requiresBalancing() on fast/multicol/spanner-crash-when-adding-summary.html
+        https://bugs.webkit.org/show_bug.cgi?id=179308
+        <rdar://problem/34592771>
+
+        Reviewed by Zalan Bujtas.
+
+        The issue here is that we fail to tear down render tree for a summary element because adding another summary element
+        takes it out of the composed tree. This leaves behind renderers that break some multicolumn assumptions.
+
+        * rendering/updating/RenderTreeUpdater.cpp:
+        (WebCore::RenderTreeUpdater::tearDownRenderers):
+        (WebCore::RenderTreeUpdater::tearDownLeftoverShadowHostChildren):
+
+        When tearing down renderers go through the real children of the shadow hosts at the end and see if we left any renderers behind.
+        If so, tear them down too.
+
+        * rendering/updating/RenderTreeUpdater.h:
+
 2018-01-24  Daniel Bates  <dabates@apple.com>
 

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

@@ -555 +555 @@
                 element.setRenderer(nullptr);
             }
+
+            // Make sure we don't leave any renderers behind in nodes outside the composed tree.
+            if (element.shadowRoot())
+                tearDownLeftoverShadowHostChildren(element);
+
             if (element.hasCustomStyleResolveCallbacks())
                 element.didDetachRenderers();
@@ -584 +589 @@
     renderer->removeFromParentAndDestroyCleaningUpAnonymousWrappers();
     text.setRenderer(nullptr);
+}
+
+void RenderTreeUpdater::tearDownLeftoverShadowHostChildren(Element& host)
+{
+    for (auto* hostChild = host.firstChild(); hostChild; hostChild = hostChild->nextSibling()) {
+        if (!hostChild->renderer())
+            continue;
+        if (is<Text>(*hostChild)) {
+            tearDownTextRenderer(downcast<Text>(*hostChild));
+            continue;
+        }
+        if (is<Element>(*hostChild))
+            tearDownRenderers(downcast<Element>(*hostChild), TeardownType::Full);
+    }
 }
 

trunk/Source/WebCore/rendering/updating/RenderTreeUpdater.h

@@ -89 +89 @@
     static void tearDownRenderers(Element&, TeardownType);
     static void tearDownTextRenderer(Text&);
+    static void tearDownLeftoverShadowHostChildren(Element&);
 
     RenderView& renderView();