resetFlowThreadContainingBlockAndChildInfoIncludingDescendants should not ignore...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 9 May 2017 22:30:11 +0000 (22:30 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 9 May 2017 22:30:11 +0000 (22:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=171873
<rdar://problem/32004954>

Reviewed by Simon Fraser.

Source/WebCore:

Normally a RenderBlock's parent is another RenderBlock, but In some cases (e.g. tables) a RenderBlock can
have a non-RenderBlock(RenderBox) ancestor.
While updating the flow thread state on a subtree, we should descent into subtrees with RenderElement
roots and not just RenderBlocks so that we clear the state on the entire subtree.

Test: fast/multicol/crash-when-column-inside-table.html

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::resetFlowThreadContainingBlockAndChildInfoIncludingDescendants):
* rendering/RenderBlock.h:
* rendering/RenderElement.cpp:
(WebCore::RenderElement::resetFlowThreadContainingBlockAndChildInfoIncludingDescendants):
* rendering/RenderElement.h:

LayoutTests:

* fast/multicol/crash-when-column-inside-table-expected.txt: Added.
* fast/multicol/crash-when-column-inside-table.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/multicol/crash-when-column-inside-table-expected.txt [new file with mode: 0644]
LayoutTests/fast/multicol/crash-when-column-inside-table.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderBlock.h
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderElement.h

index 4b7e941..f800f20 100644 (file)
@@ -1,3 +1,14 @@
+2017-05-09  Zalan Bujtas  <zalan@apple.com>
+
+        resetFlowThreadContainingBlockAndChildInfoIncludingDescendants should not ignore RenderElement subtrees.
+        https://bugs.webkit.org/show_bug.cgi?id=171873
+        <rdar://problem/32004954>
+
+        Reviewed by Simon Fraser.
+
+        * fast/multicol/crash-when-column-inside-table-expected.txt: Added.
+        * fast/multicol/crash-when-column-inside-table.html: Added.
+
 2017-05-09  Ryan Haddad  <ryanhaddad@apple.com>
 
         [iOS Simulator] Flaky failure LayoutTest/webrtc/libwebrtc/release-while-setting-local-description.html
diff --git a/LayoutTests/fast/multicol/crash-when-column-inside-table-expected.txt b/LayoutTests/fast/multicol/crash-when-column-inside-table-expected.txt
new file mode 100644 (file)
index 0000000..e92fce8
--- /dev/null
@@ -0,0 +1 @@
+PASS if no crash or assert.
diff --git a/LayoutTests/fast/multicol/crash-when-column-inside-table.html b/LayoutTests/fast/multicol/crash-when-column-inside-table.html
new file mode 100644 (file)
index 0000000..52da5f5
--- /dev/null
@@ -0,0 +1,23 @@
+<!DOCTYPE html>\r
+<html>\r
+<head style="display: inline; column-count: 2; position: absolute;" id=head>\r
+<title>This tests we can manage columns inside tables.</title>\r
+<script>\r
+    if (window.testRunner)\r
+        testRunner.dumpAsText();\r
+</script>\r
+<script style="display: table;" id=script></script>\r
+</head>\r
+<body>\r
+PASS if no crash or assert.\r
+<div id=div><div></div></div>\r
+<script>\r
+script.appendChild(div);\r
+document.body.offsetHeight;\r
+\r
+div.style.columnCount="2";\r
+script.style.position="absolute";\r
+document.body.offsetHeight;\r
+</script>\r
+</body>\r
+</html>
\ No newline at end of file
index 9f7cea3..16ac1c0 100644 (file)
@@ -1,3 +1,25 @@
+2017-05-09  Zalan Bujtas  <zalan@apple.com>
+
+        resetFlowThreadContainingBlockAndChildInfoIncludingDescendants should not ignore RenderElement subtrees.
+        https://bugs.webkit.org/show_bug.cgi?id=171873
+        <rdar://problem/32004954>
+
+        Reviewed by Simon Fraser.
+
+        Normally a RenderBlock's parent is another RenderBlock, but In some cases (e.g. tables) a RenderBlock can
+        have a non-RenderBlock(RenderBox) ancestor.
+        While updating the flow thread state on a subtree, we should descent into subtrees with RenderElement
+        roots and not just RenderBlocks so that we clear the state on the entire subtree.
+
+        Test: fast/multicol/crash-when-column-inside-table.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::resetFlowThreadContainingBlockAndChildInfoIncludingDescendants):
+        * rendering/RenderBlock.h:
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::resetFlowThreadContainingBlockAndChildInfoIncludingDescendants):
+        * rendering/RenderElement.h:
+
 2017-05-09  Eric Carlson  <eric.carlson@apple.com>
 
         [MediaStream] deviceId constraint doesn't work with getUserMedia
index 76cddf9..b3b111a 100644 (file)
@@ -3416,7 +3416,7 @@ RenderFlowThread* RenderBlock::locateFlowThreadContainingBlock() const
     return rareData->m_flowThreadContainingBlock.value();
 }
 
-void RenderBlock::resetFlowThreadContainingBlockAndChildInfoIncludingDescendants()
+void RenderBlock::resetFlowThreadContainingBlockAndChildInfoIncludingDescendants(RenderFlowThread*)
 {
     if (flowThreadState() == NotInsideFlowThread)
         return;
@@ -3426,16 +3426,7 @@ void RenderBlock::resetFlowThreadContainingBlockAndChildInfoIncludingDescendants
 
     auto* flowThread = cachedFlowThreadContainingBlock();
     setCachedFlowThreadContainingBlockNeedsUpdate();
-
-    if (flowThread)
-        flowThread->removeFlowChildInfo(*this);
-
-    for (auto& child : childrenOfType<RenderElement>(*this)) {
-        if (flowThread)
-            flowThread->removeFlowChildInfo(child);
-        if (is<RenderBlock>(child))
-            downcast<RenderBlock>(child).resetFlowThreadContainingBlockAndChildInfoIncludingDescendants();
-    }
+    RenderElement::resetFlowThreadContainingBlockAndChildInfoIncludingDescendants(flowThread);
 }
 
 LayoutUnit RenderBlock::paginationStrut() const
index 115cc9a..fb9739b 100644 (file)
@@ -325,7 +325,7 @@ public:
     RenderFlowThread* cachedFlowThreadContainingBlock() const;
     void setCachedFlowThreadContainingBlockNeedsUpdate();
     virtual bool cachedFlowThreadContainingBlockNeedsUpdate() const;
-    void resetFlowThreadContainingBlockAndChildInfoIncludingDescendants();
+    void resetFlowThreadContainingBlockAndChildInfoIncludingDescendants(RenderFlowThread* = nullptr) final;
 
     std::optional<LayoutUnit> availableLogicalHeightForPercentageComputation() const;
     bool hasDefiniteLogicalHeight() const;
index 2ef261f..dbde69e 100644 (file)
@@ -2256,6 +2256,15 @@ void RenderElement::removeFromRenderFlowThreadIncludingDescendants(bool shouldUp
         setFlowThreadState(NotInsideFlowThread);
 }
 
+void RenderElement::resetFlowThreadContainingBlockAndChildInfoIncludingDescendants(RenderFlowThread* flowThread)
+{
+    if (flowThread)
+        flowThread->removeFlowChildInfo(*this);
+
+    for (auto& child : childrenOfType<RenderElement>(*this))
+        child.resetFlowThreadContainingBlockAndChildInfoIncludingDescendants(flowThread);
+}
+
 #if ENABLE(TEXT_AUTOSIZING)
 static RenderObject::BlockContentHeightType includeNonFixedHeight(const RenderObject& renderer)
 {
index 4b12375..f9a736c 100644 (file)
@@ -221,6 +221,7 @@ public:
     RespectImageOrientationEnum shouldRespectImageOrientation() const;
 
     void removeFromRenderFlowThread();
+    virtual void resetFlowThreadContainingBlockAndChildInfoIncludingDescendants(RenderFlowThread*);
 
     // Called before anonymousChild.setStyle(). Override to set custom styles for
     // the child.