Title: [224894] trunk
Revision
224894
Author
fred.w...@free.fr
Date
2017-11-15 12:07:51 -0800 (Wed, 15 Nov 2017)

Log Message

ASSERTION FAILED: !renderer->needsLayout() in WebCore::RenderBlock::checkPositionedObjectsNeedLayout with MathML
https://bugs.webkit.org/show_bug.cgi?id=178865

Patch by Frederic Wang <fw...@igalia.com> on 2017-11-15
Reviewed by Manuel Rego Casasnovas.

Source/WebCore:

MathML token elements can contain HTML elements and hence MathML elements can contain
out-of-flow positioned descendants. Also all MathML elements can be containing block and hence
should position their out-of-flow positioned descendants before calling clearNeedsLayout().
This patch does that in all places in the MathML renderer classes, except a few of them:
- RenderMathMLSpace, which can not have descendants.
- RenderMathMLToken and RenderMathMLOperator, since they will use the layout implementation
  of RenderMathMLBlock when they contain non-text children.
The patch also fixes an ASSERTION failure in WebCore::RenderBlock::checkPositionedObjectsNeedLayout
due to some descendants that are not laid out.

Test: mathml/out-of-flow-in-token-crash.html

* rendering/mathml/RenderMathMLBlock.cpp:
(WebCore::RenderMathMLBlock::layoutBlock): Call layoutPositionedObjects.
(WebCore::RenderMathMLBlock::layoutInvalidMarkup): Ditto and pass the relayoutChildren boolean.
* rendering/mathml/RenderMathMLBlock.h: Add a relayoutChildren boolean to layoutInvalidMarkup.
* rendering/mathml/RenderMathMLFraction.cpp:
(WebCore::RenderMathMLFraction::layoutBlock): Pass the relayoutChildren boolean to layoutInvalidMarkup
and call layoutPositionedObjects.
* rendering/mathml/RenderMathMLRoot.cpp:
(WebCore::RenderMathMLRoot::layoutBlock): Ditto.
* rendering/mathml/RenderMathMLScripts.cpp:
(WebCore::RenderMathMLScripts::layoutBlock): Ditto.
* rendering/mathml/RenderMathMLUnderOver.cpp:
(WebCore::RenderMathMLUnderOver::layoutBlock): Ditto.
* rendering/mathml/RenderMathMLMenclose.cpp:
(WebCore::RenderMathMLMenclose::layoutBlock): Call layoutPositionedObjects.
* rendering/mathml/RenderMathMLPadded.cpp:
(WebCore::RenderMathMLPadded::layoutBlock): Ditto.
* rendering/mathml/RenderMathMLRow.cpp:
(WebCore::RenderMathMLRow::layoutBlock): Ditto.

LayoutTests:

Add a test to trigger various clearNeedsLayout() in a MathML containing block with
out-of-flow positioned descendants.

* mathml/out-of-flow-in-token-crash-expected.txt: Added.
* mathml/out-of-flow-in-token-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (224893 => 224894)


--- trunk/LayoutTests/ChangeLog	2017-11-15 20:02:04 UTC (rev 224893)
+++ trunk/LayoutTests/ChangeLog	2017-11-15 20:07:51 UTC (rev 224894)
@@ -1,3 +1,16 @@
+2017-11-15  Frederic Wang  <fw...@igalia.com>
+
+        ASSERTION FAILED: !renderer->needsLayout() in WebCore::RenderBlock::checkPositionedObjectsNeedLayout with MathML
+        https://bugs.webkit.org/show_bug.cgi?id=178865
+
+        Reviewed by Manuel Rego Casasnovas.
+
+        Add a test to trigger various clearNeedsLayout() in a MathML containing block with
+        out-of-flow positioned descendants.
+
+        * mathml/out-of-flow-in-token-crash-expected.txt: Added.
+        * mathml/out-of-flow-in-token-crash.html: Added.
+
 2017-11-15  Nan Wang  <n_w...@apple.com>
 
         [GTK] accessibility/accessibility-object-model.html fails

Added: trunk/LayoutTests/mathml/out-of-flow-in-token-crash-expected.txt (0 => 224894)


--- trunk/LayoutTests/mathml/out-of-flow-in-token-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/mathml/out-of-flow-in-token-crash-expected.txt	2017-11-15 20:07:51 UTC (rev 224894)
@@ -0,0 +1,25 @@
+This test passes if it does not crash
+
+RenderMathMLFenced
+
+RenderMathMLFraction
+
+RenderMathMLMath
+
+RenderMathMLMenclose
+
+RenderMathMLOperator
+
+RenderMathMLPadded
+
+RenderMathMLRoot
+
+RenderMathMLRow
+RenderMathMLScripts
+
+RenderMathMLToken
+
+RenderMathMLUnderOver
+
+RenderMathMLBlock (invalid markup)
+
Property changes on: trunk/LayoutTests/mathml/out-of-flow-in-token-crash-expected.txt
___________________________________________________________________

Added: svn:eol-style

+LF \ No newline at end of property

Added: trunk/LayoutTests/mathml/out-of-flow-in-token-crash.html (0 => 224894)


--- trunk/LayoutTests/mathml/out-of-flow-in-token-crash.html	                        (rev 0)
+++ trunk/LayoutTests/mathml/out-of-flow-in-token-crash.html	2017-11-15 20:07:51 UTC (rev 224894)
@@ -0,0 +1,156 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <title>ASSERTION with out-of-flow elements inside MathML</title>
+    <meta charset="utf-8"/>
+    <script>
+      if (window.testRunner)
+          testRunner.dumpAsText();
+    </script>
+  </head>
+  <body>
+
+    <p>This test passes if it does not crash</p>
+
+    <p>
+      <math>
+        <mfenced style="position: relative">
+          <mtext>
+            <span>
+              <span style="position: absolute">RenderMathMLFenced</span>
+            </span>
+          </mtext>
+        </mfenced>
+      </math>
+    </p>
+
+    <p>
+      <math>
+        <mfrac style="position: relative">
+          <mtext>
+            <span>
+              <span style="position: absolute">RenderMathMLFraction</span>
+            </span>
+          </mtext>
+          <mspace></mspace>
+        </mfrac>
+      </math>
+    </p>
+
+    <p>
+      <math style="position: relative">
+        <mtext>
+          <span>
+            <span style="position: absolute">RenderMathMLMath</span>
+          </span>
+        </mtext>
+      </math>
+    </p>
+
+    <p>
+      <math>
+        <menclose style="position: relative">
+          <mtext>
+            <span>
+              <span style="position: absolute">RenderMathMLMenclose</span>
+            </span>
+          </mtext>
+        </menclose>
+      </math>
+    </p>
+
+    <p>
+      <math>
+        <mo style="position: relative">
+          <span>
+            <span style="position: absolute">RenderMathMLOperator</span>
+            </span>
+        </mo>
+      </math>
+    </p>
+
+    <p>
+      <math>
+        <mpadded style="position: relative">
+          <mtext>
+            <span>
+              <span style="position: absolute">RenderMathMLPadded</span>
+            </span>
+          </mtext>
+        </mpadded>
+      </math>
+    </p>
+
+    <p>
+      <math>
+        <mroot style="position: relative">
+          <mtext>
+            <span>
+              <span style="position: absolute">RenderMathMLRoot</span>
+            </span>
+          </mtext>
+          <mspace></mspace>
+        </mroot>
+      </math>
+    </p>
+
+    <p>
+      <math>
+        <mrow style="position: relative">
+          <mtext><span><span style="position: absolute">RenderMathMLRow</span><span></mtext>
+        </mrow>
+      </math>
+    </p>
+
+    <p>
+      <math>
+        <msubsup style="position: relative">
+          <mtext>
+            <span>
+              <span style="position: absolute">RenderMathMLScripts</span>
+            </span>
+          </mtext>
+          <mspace></mspace>
+          <mspace></mspace>
+        </msubsup>
+      </math>
+    </p>
+
+    <p>
+      <math>
+        <mtext style="position: relative">
+          <span>
+            <span style="position: absolute">RenderMathMLToken</span>
+          </span>
+        </mtext>
+      </math>
+    </p>
+
+    <p>
+      <math>
+        <munderover style="position: relative">
+          <mtext>
+            <span>
+              <span style="position: absolute">RenderMathMLUnderOver</span>
+            </span>
+          </mtext>
+          <mspace></mspace>
+          <mspace></mspace>
+        </munderover>
+      </math>
+    </p>
+
+    <p>
+      <math>
+        <mfrac style="position: relative">
+          <mtext>
+            <span>
+              <span style="position: absolute">RenderMathMLBlock (invalid markup)</span>
+            </span>
+          </mtext>
+        </mfrac>
+      </math>
+    </p>
+
+  </body>
+</html>
Property changes on: trunk/LayoutTests/mathml/out-of-flow-in-token-crash.html
___________________________________________________________________

Added: svn:eol-style

+LF \ No newline at end of property

Added: svn:mime-type

+text/html \ No newline at end of property

Modified: trunk/Source/WebCore/ChangeLog (224893 => 224894)


--- trunk/Source/WebCore/ChangeLog	2017-11-15 20:02:04 UTC (rev 224893)
+++ trunk/Source/WebCore/ChangeLog	2017-11-15 20:07:51 UTC (rev 224894)
@@ -1,3 +1,42 @@
+2017-11-15  Frederic Wang  <fw...@igalia.com>
+
+        ASSERTION FAILED: !renderer->needsLayout() in WebCore::RenderBlock::checkPositionedObjectsNeedLayout with MathML
+        https://bugs.webkit.org/show_bug.cgi?id=178865
+
+        Reviewed by Manuel Rego Casasnovas.
+
+        MathML token elements can contain HTML elements and hence MathML elements can contain
+        out-of-flow positioned descendants. Also all MathML elements can be containing block and hence
+        should position their out-of-flow positioned descendants before calling clearNeedsLayout().
+        This patch does that in all places in the MathML renderer classes, except a few of them:
+        - RenderMathMLSpace, which can not have descendants.
+        - RenderMathMLToken and RenderMathMLOperator, since they will use the layout implementation
+          of RenderMathMLBlock when they contain non-text children.
+        The patch also fixes an ASSERTION failure in WebCore::RenderBlock::checkPositionedObjectsNeedLayout
+        due to some descendants that are not laid out.
+
+        Test: mathml/out-of-flow-in-token-crash.html
+
+        * rendering/mathml/RenderMathMLBlock.cpp:
+        (WebCore::RenderMathMLBlock::layoutBlock): Call layoutPositionedObjects.
+        (WebCore::RenderMathMLBlock::layoutInvalidMarkup): Ditto and pass the relayoutChildren boolean.
+        * rendering/mathml/RenderMathMLBlock.h: Add a relayoutChildren boolean to layoutInvalidMarkup.
+        * rendering/mathml/RenderMathMLFraction.cpp:
+        (WebCore::RenderMathMLFraction::layoutBlock): Pass the relayoutChildren boolean to layoutInvalidMarkup
+        and call layoutPositionedObjects.
+        * rendering/mathml/RenderMathMLRoot.cpp:
+        (WebCore::RenderMathMLRoot::layoutBlock): Ditto.
+        * rendering/mathml/RenderMathMLScripts.cpp:
+        (WebCore::RenderMathMLScripts::layoutBlock): Ditto.
+        * rendering/mathml/RenderMathMLUnderOver.cpp:
+        (WebCore::RenderMathMLUnderOver::layoutBlock): Ditto.
+        * rendering/mathml/RenderMathMLMenclose.cpp:
+        (WebCore::RenderMathMLMenclose::layoutBlock): Call layoutPositionedObjects.
+        * rendering/mathml/RenderMathMLPadded.cpp:
+        (WebCore::RenderMathMLPadded::layoutBlock): Ditto.
+        * rendering/mathml/RenderMathMLRow.cpp:
+        (WebCore::RenderMathMLRow::layoutBlock): Ditto.
+
 2017-11-15  Youenn Fablet  <you...@apple.com>
 
         Add ServiceWorker to WebProcess plumbery for FormData fetch responses

Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp (224893 => 224894)


--- trunk/Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp	2017-11-15 20:02:04 UTC (rev 224893)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp	2017-11-15 20:07:51 UTC (rev 224894)
@@ -245,12 +245,14 @@
 
     updateLogicalHeight();
 
+    layoutPositionedObjects(relayoutChildren);
+
     repainter.repaintAfterLayout();
 
     clearNeedsLayout();
 }
 
-void RenderMathMLBlock::layoutInvalidMarkup()
+void RenderMathMLBlock::layoutInvalidMarkup(bool relayoutChildren)
 {
     // Invalid MathML subtrees are just renderered as empty boxes.
     // FIXME: https://webkit.org/b/135460 - Should we display some "invalid" markup message instead?
@@ -259,6 +261,7 @@
         child->layoutIfNeeded();
     setLogicalWidth(0);
     setLogicalHeight(0);
+    layoutPositionedObjects(relayoutChildren);
     clearNeedsLayout();
 }
 

Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLBlock.h (224893 => 224894)


--- trunk/Source/WebCore/rendering/mathml/RenderMathMLBlock.h	2017-11-15 20:02:04 UTC (rev 224893)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLBlock.h	2017-11-15 20:07:51 UTC (rev 224894)
@@ -84,7 +84,7 @@
     }
 
     void layoutBlock(bool relayoutChildren, LayoutUnit pageLogicalHeight = 0) override;
-    void layoutInvalidMarkup();
+    void layoutInvalidMarkup(bool relayoutChildren);
 
 private:
     bool isRenderMathMLBlock() const final { return true; }

Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLFraction.cpp (224893 => 224894)


--- trunk/Source/WebCore/rendering/mathml/RenderMathMLFraction.cpp	2017-11-15 20:02:04 UTC (rev 224893)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLFraction.cpp	2017-11-15 20:07:51 UTC (rev 224894)
@@ -183,7 +183,7 @@
         return;
 
     if (!isValid()) {
-        layoutInvalidMarkup();
+        layoutInvalidMarkup(relayoutChildren);
         return;
     }
 
@@ -226,6 +226,8 @@
     verticalOffset = std::max(verticalOffset + denominator().logicalHeight(), m_ascent + denominatorDescent); // This is the bottom of our renderer.
     setLogicalHeight(verticalOffset);
 
+    layoutPositionedObjects(relayoutChildren);
+
     clearNeedsLayout();
 }
 

Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLMenclose.cpp (224893 => 224894)


--- trunk/Source/WebCore/rendering/mathml/RenderMathMLMenclose.cpp	2017-11-15 20:02:04 UTC (rev 224893)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLMenclose.cpp	2017-11-15 20:07:51 UTC (rev 224894)
@@ -189,6 +189,8 @@
 
     m_contentRect = LayoutRect(space.left, space.top, contentWidth, contentAscent + contentDescent);
 
+    layoutPositionedObjects(relayoutChildren);
+
     clearNeedsLayout();
 }
 

Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLPadded.cpp (224893 => 224894)


--- trunk/Source/WebCore/rendering/mathml/RenderMathMLPadded.cpp	2017-11-15 20:02:04 UTC (rev 224893)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLPadded.cpp	2017-11-15 20:07:51 UTC (rev 224894)
@@ -110,6 +110,8 @@
     setLogicalWidth(width);
     setLogicalHeight(ascent + descent);
 
+    layoutPositionedObjects(relayoutChildren);
+
     clearNeedsLayout();
 }
 

Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLRoot.cpp (224893 => 224894)


--- trunk/Source/WebCore/rendering/mathml/RenderMathMLRoot.cpp	2017-11-15 20:02:04 UTC (rev 224893)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLRoot.cpp	2017-11-15 20:07:51 UTC (rev 224894)
@@ -188,7 +188,7 @@
     m_baseWidth = 0;
 
     if (!isValid()) {
-        layoutInvalidMarkup();
+        layoutInvalidMarkup(relayoutChildren);
         return;
     }
 
@@ -255,6 +255,9 @@
     }
 
     setLogicalHeight(ascent + descent);
+
+    layoutPositionedObjects(relayoutChildren);
+
     clearNeedsLayout();
 }
 

Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLRow.cpp (224893 => 224894)


--- trunk/Source/WebCore/rendering/mathml/RenderMathMLRow.cpp	2017-11-15 20:02:04 UTC (rev 224893)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLRow.cpp	2017-11-15 20:07:51 UTC (rev 224894)
@@ -187,6 +187,8 @@
 
     updateLogicalHeight();
 
+    layoutPositionedObjects(relayoutChildren);
+
     clearNeedsLayout();
 }
 

Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLScripts.cpp (224893 => 224894)


--- trunk/Source/WebCore/rendering/mathml/RenderMathMLScripts.cpp	2017-11-15 20:02:04 UTC (rev 224893)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLScripts.cpp	2017-11-15 20:07:51 UTC (rev 224894)
@@ -366,7 +366,7 @@
 
     auto possibleReference = validateAndGetReferenceChildren();
     if (!possibleReference) {
-        layoutInvalidMarkup();
+        layoutInvalidMarkup(relayoutChildren);
         return;
     }
     auto& reference = possibleReference.value();
@@ -471,6 +471,8 @@
     }
     }
 
+    layoutPositionedObjects(relayoutChildren);
+
     clearNeedsLayout();
 }
 

Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp (224893 => 224894)


--- trunk/Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp	2017-11-15 20:02:04 UTC (rev 224893)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp	2017-11-15 20:07:51 UTC (rev 224894)
@@ -250,7 +250,7 @@
         return;
 
     if (!isValid()) {
-        layoutInvalidMarkup();
+        layoutInvalidMarkup(relayoutChildren);
         return;
     }
 
@@ -311,6 +311,8 @@
 
     setLogicalHeight(verticalOffset);
 
+    layoutPositionedObjects(relayoutChildren);
+
     clearNeedsLayout();
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to