RenderMultiColumnFlowThread - Avoid render tree mutation during layout
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Aug 2017 17:28:10 +0000 (17:28 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Aug 2017 17:28:10 +0000 (17:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=176026
<rdar://problem/33402891>

Reviewed by Zalan Bujtas.

Source/WebCore:

Mutations should be done in RenderTreeUpdater.

* rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::willCreateColumns const):

    Don't create columns for RenderSVGBlock. Before this patch this was avoided because it
    has custom layout() function that doesn't call to setComputedColumnCountAndWidth.
    Same for mathml and ruby.

    Don't create columns for pseudo elements (first-letter mostly).

(WebCore::RenderBlockFlow::setComputedColumnCountAndWidth):

    This now assumes that the multicolumn renderer has been initialized correctly already.

* rendering/RenderBlockFlow.h:
* style/RenderTreeUpdater.cpp:
(WebCore::updateMultiColumnFlowThread):

    Create or delte multicolumn renderer after descendants are known.

(WebCore::RenderTreeUpdater::commit):
(WebCore::RenderTreeUpdater::updateAfterDescendants):

LayoutTests:

* imported/blink/fast/pagination/first-letter-inherit-all-crash-expected.txt:

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

LayoutTests/ChangeLog
LayoutTests/imported/blink/fast/pagination/first-letter-inherit-all-crash-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlockFlow.cpp
Source/WebCore/rendering/RenderBlockFlow.h
Source/WebCore/style/RenderTreeUpdater.cpp

index f5fe425..e1ea277 100644 (file)
@@ -1,3 +1,13 @@
+2017-08-30  Antti Koivisto  <antti@apple.com>
+
+        RenderMultiColumnFlowThread - Avoid render tree mutation during layout
+        https://bugs.webkit.org/show_bug.cgi?id=176026
+        <rdar://problem/33402891>
+
+        Reviewed by Zalan Bujtas.
+
+        * imported/blink/fast/pagination/first-letter-inherit-all-crash-expected.txt:
+
 2017-08-30  Matt Lewis  <jlewis3@apple.com>
 
         Creation of missing expectation folders and rebaseline for js/dom/global-constructors-attributes-expected.txt after r221302.
index 489e038..73933af 100644 (file)
@@ -1,3 +1,35 @@
+2017-08-30  Antti Koivisto  <antti@apple.com>
+
+        RenderMultiColumnFlowThread - Avoid render tree mutation during layout
+        https://bugs.webkit.org/show_bug.cgi?id=176026
+        <rdar://problem/33402891>
+
+        Reviewed by Zalan Bujtas.
+
+        Mutations should be done in RenderTreeUpdater.
+
+        * rendering/RenderBlockFlow.cpp:
+        (WebCore::RenderBlockFlow::willCreateColumns const):
+
+            Don't create columns for RenderSVGBlock. Before this patch this was avoided because it
+            has custom layout() function that doesn't call to setComputedColumnCountAndWidth.
+            Same for mathml and ruby.
+
+            Don't create columns for pseudo elements (first-letter mostly).
+
+        (WebCore::RenderBlockFlow::setComputedColumnCountAndWidth):
+
+            This now assumes that the multicolumn renderer has been initialized correctly already.
+
+        * rendering/RenderBlockFlow.h:
+        * style/RenderTreeUpdater.cpp:
+        (WebCore::updateMultiColumnFlowThread):
+
+            Create or delte multicolumn renderer after descendants are known.
+
+        (WebCore::RenderTreeUpdater::commit):
+        (WebCore::RenderTreeUpdater::updateAfterDescendants):
+
 2017-08-30  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         The SVG fragment identifier is not respected if it is a part of an HTTP URL
index a2be462..f547f8d 100644 (file)
@@ -456,10 +456,15 @@ bool RenderBlockFlow::willCreateColumns(std::optional<unsigned> desiredColumnCou
     // The following types are not supposed to create multicol context.
     if (isFileUploadControl() || isTextControl() || isListBox())
         return false;
+    if (isRenderSVGBlock() || isRenderMathMLBlock() || isRubyRun())
+        return false;
 
     if (!firstChild())
         return false;
 
+    if (style().styleType() != NOPSEUDO)
+        return false;
+
     // If overflow-y is set to paged-x or paged-y on the body or html element, we'll handle the paginating in the RenderView instead.
     if ((style().overflowY() == OPAGEDX || style().overflowY() == OPAGEDY) && !(isDocumentElementRenderer() || isBody()))
         return true;
@@ -4012,17 +4017,12 @@ bool RenderBlockFlow::requiresColumns(int desiredColumnCount) const
 
 void RenderBlockFlow::setComputedColumnCountAndWidth(int count, LayoutUnit width)
 {
-    bool destroyColumns = !requiresColumns(count);
-    if (destroyColumns) {
-        if (multiColumnFlowThread())
-            destroyMultiColumnFlowThread();
-    } else {
-        if (!multiColumnFlowThread())
-            createMultiColumnFlowThread();
-        multiColumnFlowThread()->setColumnCountAndWidth(count, width);
-        multiColumnFlowThread()->setProgressionIsInline(style().hasInlineColumnAxis());
-        multiColumnFlowThread()->setProgressionIsReversed(style().columnProgression() == ReverseColumnProgression);
-    }
+    ASSERT(!!multiColumnFlowThread() == requiresColumns(count));
+    if (!multiColumnFlowThread())
+        return;
+    multiColumnFlowThread()->setColumnCountAndWidth(count, width);
+    multiColumnFlowThread()->setProgressionIsInline(style().hasInlineColumnAxis());
+    multiColumnFlowThread()->setProgressionIsReversed(style().columnProgression() == ReverseColumnProgression);
 }
 
 void RenderBlockFlow::updateColumnProgressionFromStyle(RenderStyle& style)
index 925ba31..6fc0eac 100644 (file)
@@ -283,7 +283,8 @@ public:
     RenderMultiColumnFlowThread* multiColumnFlowThread() const { return hasRareBlockFlowData() ? rareBlockFlowData()->m_multiColumnFlowThread : nullptr; }
     void setMultiColumnFlowThread(RenderMultiColumnFlowThread*);
     bool willCreateColumns(std::optional<unsigned> desiredColumnCount = std::nullopt) const;
-    
+    virtual bool requiresColumns(int) const;
+
     bool containsFloats() const override { return m_floatingObjects && !m_floatingObjects->set().isEmpty(); }
     bool containsFloat(RenderBox&) const;
 
@@ -469,8 +470,7 @@ protected:
     void addFloatsToNewParent(RenderBlockFlow& toBlockFlow) const;
     
     virtual void computeColumnCountAndWidth();
-    virtual bool requiresColumns(int) const;
-    
+
     virtual void cachePriorCharactersIfNeeded(const LazyLineBreakIterator&) {};
     
 protected:
index 878ba0a..de5f677 100644 (file)
@@ -117,6 +117,18 @@ static ListHashSet<ContainerNode*> findRenderingRoots(const Style::Update& updat
     return renderingRoots;
 }
 
+static void updateMultiColumnFlowThread(RenderBlockFlow& flow)
+{
+    bool needsFlowThread = flow.requiresColumns(flow.style().columnCount());
+    if (!needsFlowThread) {
+        if (flow.multiColumnFlowThread())
+            flow.destroyMultiColumnFlowThread();
+        return;
+    }
+    if (!flow.multiColumnFlowThread())
+        flow.createMultiColumnFlowThread();
+}
+
 void RenderTreeUpdater::commit(std::unique_ptr<const Style::Update> styleUpdate)
 {
     ASSERT(&m_document == &styleUpdate->document());
@@ -135,6 +147,8 @@ void RenderTreeUpdater::commit(std::unique_ptr<const Style::Update> styleUpdate)
 
     generatedContent().updateRemainingQuotes();
 
+    updateMultiColumnFlowThread(renderView());
+
     m_styleUpdate = nullptr;
 }
 
@@ -263,6 +277,8 @@ void RenderTreeUpdater::updateAfterDescendants(Element& element, Style::Change s
         FirstLetter::update(downcast<RenderBlock>(*renderer));
     if (is<RenderListItem>(*renderer))
         ListItem::updateMarker(downcast<RenderListItem>(*renderer));
+    if (is<RenderBlockFlow>(*renderer))
+        updateMultiColumnFlowThread(downcast<RenderBlockFlow>(*renderer));
 
     if (element.hasCustomStyleResolveCallbacks() && styleChange == Style::Detach)
         element.didAttachRenderers();