[mathml] Refine when legacy/NG layout is forced

This is a refinement of [1] [2] [3] [4]:

1. MathML elements should not try to force NG layout when using a
  non-math display.
2. Really make ShouldForceNGLayout() override ShouldForceLegacyLayout()
   ([1] was only doing that for the inherited child context).
3. Make MathML elements class keep handling the case of a forced legacy
  layout, but only on non-math display values. Note that this check is
  not necessary in LayoutObjectFactory::CreateMath which is only called
  for math display values.
4. Allow to override the display of the <math> tag too (it's unclear
   why [3] had a special case and that does not match the spec).

Ideally, 3. can be removed when we completely switch to NG layout for
all displays. New tests are added to verify related crashes found by
clusterfuzz and check overriding of the layout MathML elements with
various display values.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2398702
[2] https://chromium-review.googlesource.com/c/chromium/src/+/2397949
[3] https://chromium-review.googlesource.com/c/chromium/src/+/1985766
[4] https://chromium-review.googlesource.com/c/chromium/src/+/1917207

Bug: 6606, 1127628, 1127407, 1127222
Change-Id: I62b0ef4de623f4eb93184a1cecce598905160dad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2409957
Commit-Queue: Frédéric Wang <fwang@igalia.com>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806771}

third_party/blink/renderer/core/dom/element.h

@@ -804,6 +804,8 @@ class CORE_EXPORT Element : public ContainerNode, public Animatable {
   // legacy layout on the entire subtree, unless this is overridden by
   // ShouldForceNGLayout().
   bool ShouldForceLegacyLayout() const {
+    if (ShouldForceNGLayout())
+      return false;
     if (TypeShouldForceLegacyLayout())
       return true;
     if (!HasRareData())

third_party/blink/renderer/core/mathml/mathml_element.cc

@@ -9,6 +9,7 @@
 #include "third_party/blink/renderer/core/css/parser/css_parser.h"
 #include "third_party/blink/renderer/core/css_value_keywords.h"
 #include "third_party/blink/renderer/core/dom/document.h"
+#include "third_party/blink/renderer/core/dom/node_computed_style.h"
 #include "third_party/blink/renderer/core/html/html_element.h"
 
 namespace blink {
@@ -144,4 +145,8 @@ bool MathMLElement::IsTokenElement() const {
          HasTagName(mathml_names::kMsTag);
 }
 
+bool MathMLElement::ShouldForceNGLayout() const {
+  return ComputedStyleRef().IsDisplayMathType();
+}
+
 }  // namespace blink

third_party/blink/renderer/core/mathml/mathml_element.h

@@ -55,8 +55,7 @@ class CORE_EXPORT MathMLElement : public Element {
 
  private:
   // Force NG layout as MathML elements don't have legacy layout implementation.
-  // TODO(crbug.com/1127197): Check the display of the computed style.
-  bool ShouldForceNGLayout() const final { return true; }
+  bool ShouldForceNGLayout() const final;
 };
 
 template <typename T>

third_party/blink/renderer/core/mathml/mathml_padded_element.cc

@@ -85,7 +85,7 @@ void MathMLPaddedElement::CollectStyleForPresentationAttribute(
 LayoutObject* MathMLPaddedElement::CreateLayoutObject(
     const ComputedStyle& style,
     LegacyLayout legacy) {
-  DCHECK_NE(legacy, LegacyLayout::kForce);
+  DCHECK(!style.IsDisplayMathType() || legacy != LegacyLayout::kForce);
   if (!RuntimeEnabledFeatures::MathMLCoreEnabled() ||
       !style.IsDisplayMathType())
     return MathMLElement::CreateLayoutObject(style, legacy);

third_party/blink/renderer/core/mathml/mathml_radical_element.cc

@@ -19,7 +19,7 @@ bool MathMLRadicalElement::HasIndex() const {
 LayoutObject* MathMLRadicalElement::CreateLayoutObject(
     const ComputedStyle& style,
     LegacyLayout legacy) {
-  DCHECK_NE(legacy, LegacyLayout::kForce);
+  DCHECK(!style.IsDisplayMathType() || legacy != LegacyLayout::kForce);
   if (!RuntimeEnabledFeatures::MathMLCoreEnabled() ||
       !style.IsDisplayMathType())
     return MathMLElement::CreateLayoutObject(style, legacy);

third_party/blink/renderer/core/mathml/mathml_row_element.cc

@@ -15,9 +15,9 @@ MathMLRowElement::MathMLRowElement(const QualifiedName& tagName,
 
 LayoutObject* MathMLRowElement::CreateLayoutObject(const ComputedStyle& style,
                                                    LegacyLayout legacy) {
-  DCHECK_NE(legacy, LegacyLayout::kForce);
+  DCHECK(!style.IsDisplayMathType() || legacy != LegacyLayout::kForce);
   if (!RuntimeEnabledFeatures::MathMLCoreEnabled() ||
-      (!style.IsDisplayMathType() && !HasTagName(mathml_names::kMathTag)))
+      !style.IsDisplayMathType())
     return MathMLElement::CreateLayoutObject(style, legacy);
   return new LayoutNGMathMLBlock(this);
 }

third_party/blink/web_tests/external/wpt/mathml/crashtests/display-and-column-properties.html

+<!DOCTYPE html>
+<html class="test-wait">
+  <head>
+    <title>MathML elements with display and column properties</title>
+    <meta charset="utf-8"/>
+    <style>
+      math {
+          column-width: 100px;
+      }
+    </style>
+  </head>
+  <body>
+    <math></math>
+    <math style="display: list-item"></math>
+    <script>
+      window.addEventListener("load", function() {
+          // Force initial layout.
+          document.documentElement.getBoundingClientRect();
+
+          // Change display and reforce layout.
+          let maths = document.getElementsByTagName("math");
+          maths[0].setAttribute("style", "display: list-item");
+          maths[1].removeAttribute("style");
+          document.documentElement.getBoundingClientRect();
+
+          document.documentElement.classList.remove("test-wait");
+      });
+    </script>
+  </body>
+</html>

third_party/blink/web_tests/external/wpt/mathml/relations/css-styling/display-2.html

+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<title>Verify that one can override the layout of MathML elements with the CSS display property</title>
+<link rel="help" href="https://mathml-refresh.github.io/mathml-core/#layout-algorithms">
+<meta name="assert" content="Verify that one can override the display of a MathML element.">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/mathml/support/layout-comparison.js"></script>
+<script src="/mathml/support/mathml-fragments.js"></script>
+<style>
+  mfrac {
+      padding: 0;
+  }
+</style>
+<script>
+  const Xsize = 25;
+  const templates = {
+      "block display": `<math style="display: block;">XXX</math>`,
+      "block display with contrained width": `<math style="display: block; width: ${2*Xsize}px;">XXX</math>`,
+      "list display inside display block": `<math style="display: block">\
+  <mmultiscripts style="display: list-item;">X</mmultiscripts>\
+  <maction style="display: list-item;">X</maction>\
+  <mpadded style="display: list-item;">X</mpadded>\
+</math>`,
+      "inline display": `<math style="display: inline;">XXX</math>`,
+      "inline-block display": `<math style="display: inline-block">XXX</math>`,
+      "table display (math)": `<math style="display: table">\
+  <mfrac style='display: table-row'>\
+    <msub style='display: table-cell'>X</msub>\
+    <msup style='display: table-cell'>X</msup>\
+    <msubsup style='display: table-cell'>X</msubsup>\
+  </mfrac>\
+  <mtable style='display: table-row'>\
+    <munder style='display: table-cell'>X</munder>\
+    <mover style='display: table-cell'>X</mover>\
+    <munderover style='display: table-cell'>X</munderover>\
+  </mtable>\
+</math>`,
+      "table display (mrow)": `<math display="block">\
+<mrow style="display: table">\
+  <mfrac style='display: table-row'>\
+    <msub style='display: table-cell'>X</msub>\
+    <msup style='display: table-cell'>X</msup>\
+    <msubsup style='display: table-cell'>X</msubsup>\
+  </mfrac>\
+  <mtable style='display: table-row'>\
+    <munder style='display: table-cell'>X</munder>\
+    <mover style='display: table-cell'>X</mover>\
+    <munderover style='display: table-cell'>X</munderover>\
+  </mtable>\
+</mrow></math>`,
+      "inline-table display (math)": `<math style="display: inline-table">\
+  <mfrac style='display: table-row'>\
+    <msub style='display: table-cell'>X</msub>\
+    <msup style='display: table-cell'>X</msup>\
+    <msubsup style='display: table-cell'>X</msubsup>\
+  </mfrac>\
+  <mtable style='display: table-row'>\
+    <munder style='display: table-cell'>X</munder>\
+    <mover style='display: table-cell'>X</mover>\
+    <munderover style='display: table-cell'>X</munderover>\
+  </mtable>\
+</math>`,
+      "inline-table display (mrow)": `<math display="block">\
+<mrow style="display: inline-table">\
+  <mfrac style='display: table-row'>\
+    <msub style='display: table-cell'>X</msub>\
+    <msup style='display: table-cell'>X</msup>\
+    <msubsup style='display: table-cell'>X</msubsup>\
+  </mfrac>\
+  <mtable style='display: table-row'>\
+    <munder style='display: table-cell'>X</munder>\
+    <mover style='display: table-cell'>X</mover>\
+    <munderover style='display: table-cell'>X</munderover>\
+  </mtable>\
+</mrow></math>`,
+      "flexbox display (math)": `<math style="display: flex; flex-direction: column;">XXX</math>`,
+      "flexbox display (mrow)": `<math display="block"><mrow style="display: flex; flex-direction: column;">XXX</mrow></math>`,
+      "grid display (math)": `<math style="display: grid; grid-gap: 2px; grid-template-columns: ${Xsize}px ${Xsize}px ${Xsize}px;>">XXXXXXXXX</math>`,
+      "grid display (mrow)": `<math display="block"><mrow style="display: grid; grid-gap: 2px; grid-template-columns: ${Xsize}px ${Xsize}px ${Xsize}px;>">XXXXXXXXX</mrow></math>`,
+      "ruby display (math)": `<math style="display: ruby;">\
+<mrow style="display: ruby-base;">X</mrow>\
+<mrow style="display: ruby-text">XX</mrow>\
+</math>`,
+      "ruby display (mrow)": `<math display="block"><mrow style="display: ruby;">\
+<mrow style="display: ruby-base;">X</mrow>\
+<mrow style="display: ruby-text">XX</mrow>\
+</mrow></math>`,
+      "block display with column width (math)": `<math style="display: block; column-width: ${2*Xsize}px">\
+    <mrow>XXXX</mrow><mrow>XXXX</mrow><mrow>XXXX</mrow>\
+</math>`,
+      "block display with column width (mrow)": `<math style="display: block"><mrow style="display: block; column-width: ${2*Xsize}px">\
+    <mrow>XXXX</mrow><mrow>XXXX</mrow><mrow>XXXX</mrow>\
+</mrow></math>`,
+  };
+
+  setup({ explicit_done: true });
+  window.addEventListener("load", runTests);
+
+  function runTests() {
+
+      for (let key in templates) {
+          if (!templates.hasOwnProperty(key))
+              continue;
+          let mathtest = templates[key].
+              replace(/X/g, `<mspace style="display: inline-block; width: ${Xsize}px; height: ${Xsize}px; background: black"></mspace>`);
+          let reference = mathtest.
+              replace(/maction|math|mfrac|mmultiscripts|mover|mover|mpadded|mrow|mspace|msubsup|msub|msup|mtable|munderover|munder/g, "div");
+          document.body.insertAdjacentHTML("beforeend", `<div style="position: absolute;">\
+<div><span>${key}:</span>${mathtest}</div>\
+<div><span>${key}:</span>${reference}</div>\
+</div>`);
+          let div = document.body.lastElementChild;
+          let elementDiv = div.firstElementChild;
+          let referenceDiv = div.lastElementChild;
+
+          test(function() {
+              const epsilon = 1;
+              compareLayout(elementDiv, referenceDiv, epsilon);
+          }, `${key}`);
+
+          div.style = "display: none;"; // Hide the div after measurement.
+      }
+
+      done();
+  }
+</script>
+</head>
+<body>
+  <div id="log"></div>
+</body>
+</html>

third_party/blink/web_tests/platform/linux/external/wpt/mathml/relations/css-styling/display-2-expected.txt

+This is a testharness.js-based test.
+PASS block display
+PASS block display with contrained width
+PASS list display inside display block
+PASS inline display
+PASS inline-block display
+FAIL table display (math) assert_approx_equals: block size expected 80 +/- 1 but got 78
+PASS table display (mrow)
+FAIL inline-table display (math) assert_approx_equals: block size expected 60 +/- 1 but got 58
+PASS inline-table display (mrow)
+PASS flexbox display (math)
+PASS flexbox display (mrow)
+PASS grid display (math)
+PASS grid display (mrow)
+FAIL ruby display (math) assert_approx_equals: block size expected 80 +/- 1 but got 30
+FAIL ruby display (mrow) assert_approx_equals: block size expected 80 +/- 1 but got 45
+FAIL block display with column width (math) assert_approx_equals: block size expected 80 +/- 1 but got 45
+FAIL block display with column width (mrow) assert_approx_equals: block size expected 80 +/- 1 but got 45
+Harness: the test ran to completion.
+

third_party/blink/web_tests/platform/mac/external/wpt/mathml/relations/css-styling/display-2-expected.txt

+This is a testharness.js-based test.
+PASS block display
+PASS block display with contrained width
+PASS list display inside display block
+PASS inline display
+PASS inline-block display
+PASS table display (math)
+PASS table display (mrow)
+PASS inline-table display (math)
+PASS inline-table display (mrow)
+PASS flexbox display (math)
+PASS flexbox display (mrow)
+PASS grid display (math)
+PASS grid display (mrow)
+FAIL ruby display (math) assert_approx_equals: block size expected 76 +/- 1 but got 29
+FAIL ruby display (mrow) assert_approx_equals: block size expected 76 +/- 1 but got 43
+FAIL block display with column width (math) assert_approx_equals: block size expected 76 +/- 1 but got 43
+FAIL block display with column width (mrow) assert_approx_equals: block size expected 76 +/- 1 but got 43
+Harness: the test ran to completion.
+

third_party/blink/web_tests/platform/win/external/wpt/mathml/relations/css-styling/display-2-expected.txt

+This is a testharness.js-based test.
+PASS block display
+PASS block display with contrained width
+PASS list display inside display block
+PASS inline display
+PASS inline-block display
+FAIL table display (math) assert_approx_equals: block size expected 80 +/- 1 but got 78
+PASS table display (mrow)
+FAIL inline-table display (math) assert_approx_equals: block size expected 60 +/- 1 but got 58
+PASS inline-table display (mrow)
+PASS flexbox display (math)
+PASS flexbox display (mrow)
+PASS grid display (math)
+PASS grid display (mrow)
+FAIL ruby display (math) assert_approx_equals: block size expected 80 +/- 1 but got 30
+FAIL ruby display (mrow) assert_approx_equals: block size expected 80 +/- 1 but got 45
+FAIL block display with column width (math) assert_approx_equals: block size expected 80 +/- 1 but got 45
+FAIL block display with column width (mrow) assert_approx_equals: block size expected 80 +/- 1 but got 45
+Harness: the test ran to completion.
+