Changeset 204021 in webkit

Timestamp:

Aug 2, 2016 2:43:01 AM (8 years ago)

Author:

fred.wang@free.fr

Message:

Use Optional members to store parsed MathML attributes.
​https://bugs.webkit.org/show_bug.cgi?id=160400

Patch by Frederic Wang <fwang.igalia.com> on 2016-08-02
Reviewed by Darin Adler.

Parsed MathML attributes are stored on the element classes using the memoize pattern to
minimize the number of parsing updates. Currently, a dirty flag for each member
indicate when it must be parsed again. We change this to wrap these members into an
Optional<T> container instead, where a null value indicates that the member is dirty.

No new tests, behavior is unchanged.

• mathml/MathMLElement.cpp:

(WebCore::MathMLElement::cachedMathMLLength):
(WebCore::MathMLElement::cachedBooleanAttribute):
(WebCore::MathMLElement::parseMathVariantAttribute):
(WebCore::MathMLElement::specifiedDisplayStyle):
(WebCore::MathMLElement::specifiedMathVariant):

• mathml/MathMLElement.h:
• mathml/MathMLFractionElement.cpp:

(WebCore::MathMLFractionElement::lineThickness):
(WebCore::MathMLFractionElement::cachedFractionAlignment):
(WebCore::MathMLFractionElement::parseAttribute):

• mathml/MathMLFractionElement.h:
• mathml/MathMLInlineContainerElement.cpp:

(WebCore::MathMLInlineContainerElement::parseAttribute):

• mathml/MathMLMathElement.cpp:

(WebCore::MathMLMathElement::specifiedDisplayStyle):
(WebCore::MathMLMathElement::parseAttribute):

• mathml/MathMLOperatorElement.cpp:

(WebCore::MathMLOperatorElement::operatorText):

• mathml/MathMLPaddedElement.cpp:

(WebCore::MathMLPaddedElement::parseAttribute):

• mathml/MathMLPaddedElement.h:
• mathml/MathMLScriptsElement.cpp:

(WebCore::MathMLScriptsElement::parseAttribute):

• mathml/MathMLScriptsElement.h:
• mathml/MathMLSpaceElement.cpp:

(WebCore::MathMLSpaceElement::parseAttribute):

• mathml/MathMLSpaceElement.h:
• mathml/MathMLTextElement.cpp:

(WebCore::MathMLTextElement::parseAttribute):

• mathml/MathMLUnderOverElement.cpp:

(WebCore::MathMLUnderOverElement::parseAttribute):

• mathml/MathMLUnderOverElement.h:
• rendering/mathml/MathMLStyle.cpp:

(WebCore::MathMLStyle::resolveMathMLStyle):

• rendering/mathml/MathMLStyle.h:
• rendering/mathml/RenderMathMLToken.cpp:

(WebCore::mathVariant):
(WebCore::RenderMathMLToken::updateMathVariantGlyph):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• mathml/MathMLElement.cpp (modified) (3 diffs)
• mathml/MathMLElement.h (modified) (3 diffs)
• mathml/MathMLFractionElement.cpp (modified) (2 diffs)
• mathml/MathMLFractionElement.h (modified) (1 diff)
• mathml/MathMLInlineContainerElement.cpp (modified) (1 diff)
• mathml/MathMLMathElement.cpp (modified) (2 diffs)
• mathml/MathMLOperatorElement.cpp (modified) (2 diffs)
• mathml/MathMLPaddedElement.cpp (modified) (1 diff)
• mathml/MathMLPaddedElement.h (modified) (1 diff)
• mathml/MathMLScriptsElement.cpp (modified) (1 diff)
• mathml/MathMLScriptsElement.h (modified) (1 diff)
• mathml/MathMLSpaceElement.cpp (modified) (1 diff)
• mathml/MathMLSpaceElement.h (modified) (1 diff)
• mathml/MathMLTextElement.cpp (modified) (1 diff)
• mathml/MathMLUnderOverElement.cpp (modified) (1 diff)
• mathml/MathMLUnderOverElement.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-08-02  Frederic Wang  <fwang.igalia.com>
+
+        Use Optional members to store parsed MathML attributes.
+        https://bugs.webkit.org/show_bug.cgi?id=160400
+
+        Reviewed by Darin Adler.
+
+        Parsed MathML attributes are stored on the element classes using the memoize pattern to
+        minimize the number of parsing updates. Currently, a dirty flag for each member
+        indicate when it must be parsed again. We change this to wrap these members into an
+        Optional<T> container instead, where a null value indicates that the member is dirty.
+
+        No new tests, behavior is unchanged.
+
+        * mathml/MathMLElement.cpp:
+        (WebCore::MathMLElement::cachedMathMLLength):
+        (WebCore::MathMLElement::cachedBooleanAttribute):
+        (WebCore::MathMLElement::parseMathVariantAttribute):
+        (WebCore::MathMLElement::specifiedDisplayStyle):
+        (WebCore::MathMLElement::specifiedMathVariant):
+        * mathml/MathMLElement.h:
+        * mathml/MathMLFractionElement.cpp:
+        (WebCore::MathMLFractionElement::lineThickness):
+        (WebCore::MathMLFractionElement::cachedFractionAlignment):
+        (WebCore::MathMLFractionElement::parseAttribute):
+        * mathml/MathMLFractionElement.h:
+        * mathml/MathMLInlineContainerElement.cpp:
+        (WebCore::MathMLInlineContainerElement::parseAttribute):
+        * mathml/MathMLMathElement.cpp:
+        (WebCore::MathMLMathElement::specifiedDisplayStyle):
+        (WebCore::MathMLMathElement::parseAttribute):
+        * mathml/MathMLOperatorElement.cpp:
+        (WebCore::MathMLOperatorElement::operatorText):
+        * mathml/MathMLPaddedElement.cpp:
+        (WebCore::MathMLPaddedElement::parseAttribute):
+        * mathml/MathMLPaddedElement.h:
+        * mathml/MathMLScriptsElement.cpp:
+        (WebCore::MathMLScriptsElement::parseAttribute):
+        * mathml/MathMLScriptsElement.h:
+        * mathml/MathMLSpaceElement.cpp:
+        (WebCore::MathMLSpaceElement::parseAttribute):
+        * mathml/MathMLSpaceElement.h:
+        * mathml/MathMLTextElement.cpp:
+        (WebCore::MathMLTextElement::parseAttribute):
+        * mathml/MathMLUnderOverElement.cpp:
+        (WebCore::MathMLUnderOverElement::parseAttribute):
+        * mathml/MathMLUnderOverElement.h:
+        * rendering/mathml/MathMLStyle.cpp:
+        (WebCore::MathMLStyle::resolveMathMLStyle):
+        * rendering/mathml/MathMLStyle.h:
+        * rendering/mathml/RenderMathMLToken.cpp:
+        (WebCore::mathVariant):
+        (WebCore::RenderMathMLToken::updateMathVariantGlyph):
+
 2016-08-02  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/mathml/MathMLElement.cpp

@@ -518 +518 @@
 }
 
-const MathMLElement::Length& MathMLElement::cachedMathMLLength(const QualifiedName& name, Length& length)
-{
-    if (length.dirty) {
-        length = parseMathMLLength(attributeWithoutSynchronization(name));
-        length.dirty = false;
-    }
-    return length;
-}
-
-const MathMLElement::BooleanValue& MathMLElement::cachedBooleanAttribute(const QualifiedName& name, BooleanAttribute& attribute)
-{
-    if (!attribute.dirty)
-        return attribute.value;
+const MathMLElement::Length& MathMLElement::cachedMathMLLength(const QualifiedName& name, Optional<Length>& length)
+{
+    if (length)
+        return length.value();
+    length = parseMathMLLength(attributeWithoutSynchronization(name));
+    return length.value();
+}
+
+const MathMLElement::BooleanValue& MathMLElement::cachedBooleanAttribute(const QualifiedName& name, Optional<BooleanValue>& attribute)
+{
+    if (attribute)
+        return attribute.value();
 
     // In MathML, attribute values are case-sensitive.
     const AtomicString& value = attributeWithoutSynchronization(name);
     if (value == "true")
-        attribute.value = BooleanValue::True;
+        attribute = BooleanValue::True;
     else if (value == "false")
-        attribute.value = BooleanValue::False;
+        attribute = BooleanValue::False;
     else
-        attribute.value = BooleanValue::Default;
-    attribute.dirty = false;
-
-    return attribute.value;
+        attribute = BooleanValue::Default;
+
+    return attribute.value();
 }
 
@@ -590 +588 @@
 {
     if (!acceptsDisplayStyleAttribute())
-        return Optional<bool>();
+        return Nullopt;
     const MathMLElement::BooleanValue& specifiedDisplayStyle = cachedBooleanAttribute(displaystyleAttr, m_displayStyle);
-    return specifiedDisplayStyle == BooleanValue::Default ? Optional<bool>() : Optional<bool>(specifiedDisplayStyle == BooleanValue::True);
+    return toOptionalBool(specifiedDisplayStyle);
 }
 
@@ -598 +596 @@
 {
     if (!acceptsMathVariantAttribute())
-        return Optional<MathVariant>();
-    if (m_mathVariant.dirty) {
-        m_mathVariant.value = parseMathVariantAttribute(attributeWithoutSynchronization(mathvariantAttr));
-        m_mathVariant.dirty = false;
-    }
-    return m_mathVariant.value == MathVariant::None ? Optional<MathVariant>() : Optional<MathVariant>(m_mathVariant.value);
+        return Nullopt;
+    if (!m_mathVariant)
+        m_mathVariant = parseMathVariantAttribute(attributeWithoutSynchronization(mathvariantAttr));
+    return m_mathVariant.value() == MathVariant::None ? Nullopt : m_mathVariant;
 }
 

trunk/Source/WebCore/mathml/MathMLElement.h

@@ -64 +64 @@
         LengthType type { LengthType::ParsingFailed };
         float value { 0 };
-        bool dirty { true };
     };
     static Length parseMathMLLength(const String&);
 
     enum class BooleanValue { True, False, Default };
-    struct BooleanAttribute {
-        BooleanValue value { BooleanValue::Default };
-        bool dirty { true };
-    };
 
     // These are the mathvariant values from the MathML recommendation.
@@ -98 +93 @@
         Stretched = 18
     };
-    struct MathVariantAttribute {
-        MathVariant value { MathVariant::None };
-        bool dirty { true };
-    };
 
     virtual Optional<bool> specifiedDisplayStyle();
-    Optional<MathMLElement::MathVariant> specifiedMathVariant();
+    Optional<MathVariant> specifiedMathVariant();
 
 protected:
@@ -122 +113 @@
     void defaultEventHandler(Event*) override;
 
-    const Length& cachedMathMLLength(const QualifiedName&, Length&);
-    const BooleanValue& cachedBooleanAttribute(const QualifiedName&, BooleanAttribute&);
+    const Length& cachedMathMLLength(const QualifiedName&, Optional<Length>&);
+    const BooleanValue& cachedBooleanAttribute(const QualifiedName&, Optional<BooleanValue>&);
 
     virtual bool acceptsDisplayStyleAttribute() { return false; }
     virtual bool acceptsMathVariantAttribute() { return false; }
 
-    BooleanAttribute m_displayStyle;
-    MathVariantAttribute m_mathVariant;
+    static Optional<bool> toOptionalBool(const BooleanValue& value) { return value == BooleanValue::Default ? Nullopt : Optional<bool>(value == BooleanValue::True); }
+    Optional<BooleanValue> m_displayStyle;
+    Optional<MathVariant> m_mathVariant;
 
 private:

trunk/Source/WebCore/mathml/MathMLFractionElement.cpp

@@ -48 +48 @@
 const MathMLElement::Length& MathMLFractionElement::lineThickness()
 {
-    if (!m_lineThickness.dirty)
-        return m_lineThickness;
+    if (m_lineThickness)
+        return m_lineThickness.value();
 
     // The MathML3 recommendation states that "medium" is the default thickness.
     // However, it only states that "thin" and "thick" are respectively thiner and thicker.
     // The MathML in HTML5 implementation note suggests 50% and 200% and these values are also used in Gecko.
-    String thickness = attributeWithoutSynchronization(linethicknessAttr);
+    auto& thickness = attributeWithoutSynchronization(linethicknessAttr);
+    m_lineThickness = Length();
     if (equalLettersIgnoringASCIICase(thickness, "thin")) {
-        m_lineThickness.type = LengthType::UnitLess;
-        m_lineThickness.value = .5;
+        m_lineThickness.value().type = LengthType::UnitLess;
+        m_lineThickness.value().value = .5;
     } else if (equalLettersIgnoringASCIICase(thickness, "medium")) {
-        m_lineThickness.type = LengthType::UnitLess;
-        m_lineThickness.value = 1;
+        m_lineThickness.value().type = LengthType::UnitLess;
+        m_lineThickness.value().value = 1;
     } else if (equalLettersIgnoringASCIICase(thickness, "thick")) {
-        m_lineThickness.type = LengthType::UnitLess;
-        m_lineThickness.value = 2;
+        m_lineThickness.value().type = LengthType::UnitLess;
+        m_lineThickness.value().value = 2;
     } else
         m_lineThickness = parseMathMLLength(thickness);
-    m_lineThickness.dirty = false;
-    return m_lineThickness;
+    return m_lineThickness.value();
 }
 
-MathMLFractionElement::FractionAlignment MathMLFractionElement::cachedFractionAlignment(const QualifiedName& name, FractionAlignmentAttribute& alignment)
+MathMLFractionElement::FractionAlignment MathMLFractionElement::cachedFractionAlignment(const QualifiedName& name, Optional<FractionAlignment>& alignment)
 {
-    if (!alignment.dirty)
-        return alignment.value;
+    if (alignment)
+        return alignment.value();
 
-    String value = attributeWithoutSynchronization(name);
+    auto& value = attributeWithoutSynchronization(name);
     if (equalLettersIgnoringASCIICase(value, "left"))
-        alignment.value = FractionAlignmentLeft;
+        alignment = FractionAlignmentLeft;
     else if (equalLettersIgnoringASCIICase(value, "right"))
-        alignment.value = FractionAlignmentRight;
+        alignment = FractionAlignmentRight;
     else
-        alignment.value = FractionAlignmentCenter;
-    alignment.dirty = false;
-    return alignment.value;
+        alignment = FractionAlignmentCenter;
+    return alignment.value();
 }
 
@@ -99 +98 @@
 {
     if (name == linethicknessAttr)
-        m_lineThickness.dirty = true;
+        m_lineThickness = Nullopt;
     else if (name == numalignAttr)
-        m_numeratorAlignment.dirty = true;
+        m_numeratorAlignment = Nullopt;
     else if (name == denomalignAttr)
-        m_denominatorAlignment.dirty = true;
+        m_denominatorAlignment = Nullopt;
 
     MathMLElement::parseAttribute(name, value);

trunk/Source/WebCore/mathml/MathMLFractionElement.h

@@ -48 +48 @@
     void parseAttribute(const QualifiedName&, const AtomicString&) final;
 
-    struct FractionAlignmentAttribute {
-        FractionAlignment value { FractionAlignmentCenter };
-        bool dirty { true };
-    };
-    FractionAlignment cachedFractionAlignment(const QualifiedName&, FractionAlignmentAttribute&);
+    FractionAlignment cachedFractionAlignment(const QualifiedName&, Optional<FractionAlignment>&);
 
-    Length m_lineThickness;
-    FractionAlignmentAttribute m_numeratorAlignment;
-    FractionAlignmentAttribute m_denominatorAlignment;
+    Optional<Length> m_lineThickness;
+    Optional<FractionAlignment> m_numeratorAlignment;
+    Optional<FractionAlignment> m_denominatorAlignment;
 };
 

trunk/Source/WebCore/mathml/MathMLInlineContainerElement.cpp

@@ -91 +91 @@
     bool mathVariantAttribute = name == mathvariantAttr && acceptsMathVariantAttribute();
     if (displayStyleAttribute)
-        m_displayStyle.dirty = true;
+        m_displayStyle = Nullopt;
     if (mathVariantAttribute)
-        m_mathVariant.dirty = true;
+        m_mathVariant = Nullopt;
     if ((displayStyleAttribute || mathVariantAttribute) && renderer())
         MathMLStyle::resolveMathMLStyleTree(renderer());

trunk/Source/WebCore/mathml/MathMLMathElement.cpp

@@ -59 +59 @@
     if (cachedBooleanAttribute(displaystyleAttr, m_displayStyle) == BooleanValue::Default) {
         // The default displaystyle value of the <math> depends on the display attribute, so we parse it here.
-        const AtomicString& value = attributeWithoutSynchronization(displayAttr);
+        auto& value = attributeWithoutSynchronization(displayAttr);
         if (value == "block")
-            m_displayStyle.value = BooleanValue::True;
+            m_displayStyle = BooleanValue::True;
         else if (value == "inline")
-            m_displayStyle.value = BooleanValue::False;
+            m_displayStyle = BooleanValue::False;
     }
-    return m_displayStyle.value == BooleanValue::Default ? Optional<bool>() : Optional<bool>(m_displayStyle.value == BooleanValue::True);
+    return toOptionalBool(m_displayStyle.value());
 }
 
@@ -73 +73 @@
     bool mathVariantAttribute = name == mathvariantAttr;
     if (displayStyleAttribute)
-        m_displayStyle.dirty = true;
+        m_displayStyle = Nullopt;
     if (mathVariantAttribute)
-        m_mathVariant.dirty = true;
+        m_mathVariant = Nullopt;
     if ((displayStyleAttribute || mathVariantAttribute) && renderer())
         MathMLStyle::resolveMathMLStyleTree(renderer());

trunk/Source/WebCore/mathml/MathMLOperatorElement.cpp

@@ -64 +64 @@
         return m_operatorText.value();
 
-    m_operatorText = Optional<UChar>(parseOperatorText(textContent()));
+    m_operatorText = parseOperatorText(textContent());
     return m_operatorText.value();
 }
@@ -70 +70 @@
 void MathMLOperatorElement::childrenChanged(const ChildChange& change)
 {
-    m_operatorText = Optional<UChar>();
+    m_operatorText = Nullopt;
     MathMLTextElement::childrenChanged(change);
 }

trunk/Source/WebCore/mathml/MathMLPaddedElement.cpp

@@ -74 +74 @@
 {
     if (name == widthAttr)
-        m_width.dirty = true;
+        m_width = Nullopt;
     else if (name == heightAttr)
-        m_height.dirty = true;
+        m_height = Nullopt;
     else if (name == depthAttr)
-        m_depth.dirty = true;
+        m_depth = Nullopt;
     else if (name == lspaceAttr)
-        m_lspace.dirty = true;
+        m_lspace = Nullopt;
     else if (name == voffsetAttr)
-        m_voffset.dirty = true;
+        m_voffset = Nullopt;
 
     MathMLElement::parseAttribute(name, value);

trunk/Source/WebCore/mathml/MathMLPaddedElement.h

@@ -45 +45 @@
     void parseAttribute(const QualifiedName&, const AtomicString&) final;
 
-    Length m_width;
-    Length m_height;
-    Length m_depth;
-    Length m_lspace;
-    Length m_voffset;
+    Optional<Length> m_width;
+    Optional<Length> m_height;
+    Optional<Length> m_depth;
+    Optional<Length> m_lspace;
+    Optional<Length> m_voffset;
 };
 

trunk/Source/WebCore/mathml/MathMLScriptsElement.cpp

@@ -59 +59 @@
 {
     if (name == subscriptshiftAttr)
-        m_subscriptShift.dirty = true;
+        m_subscriptShift = Nullopt;
     else if (name == superscriptshiftAttr)
-        m_superscriptShift.dirty = true;
+        m_superscriptShift = Nullopt;
 
     MathMLElement::parseAttribute(name, value);

trunk/Source/WebCore/mathml/MathMLScriptsElement.h

@@ -44 +44 @@
     void parseAttribute(const QualifiedName&, const AtomicString&) override;
 
-    Length m_subscriptShift;
-    Length m_superscriptShift;
+    Optional<Length> m_subscriptShift;
+    Optional<Length> m_superscriptShift;
 };
 

trunk/Source/WebCore/mathml/MathMLSpaceElement.cpp

@@ -64 +64 @@
 {
     if (name == widthAttr)
-        m_width.dirty = true;
+        m_width = Nullopt;
     else if (name == heightAttr)
-        m_height.dirty = true;
+        m_height = Nullopt;
     else if (name == depthAttr)
-        m_depth.dirty = true;
+        m_depth = Nullopt;
 
     MathMLElement::parseAttribute(name, value);

trunk/Source/WebCore/mathml/MathMLSpaceElement.h

@@ -43 +43 @@
     void parseAttribute(const QualifiedName&, const AtomicString&) final;
 
-    Length m_width;
-    Length m_height;
-    Length m_depth;
+    Optional<Length> m_width;
+    Optional<Length> m_height;
+    Optional<Length> m_depth;
 };
 

trunk/Source/WebCore/mathml/MathMLTextElement.cpp

@@ -66 +66 @@
 {
     if (name == mathvariantAttr) {
-        m_mathVariant.dirty = true;
+        m_mathVariant = Nullopt;
         if (renderer())
             MathMLStyle::resolveMathMLStyleTree(renderer());

trunk/Source/WebCore/mathml/MathMLUnderOverElement.cpp

@@ -59 +59 @@
 {
     if (name == accentAttr)
-        m_accent.dirty = true;
+        m_accent = Nullopt;
     else if (name == accentunderAttr)
-        m_accentUnder.dirty = true;
+        m_accentUnder = Nullopt;
 
     MathMLElement::parseAttribute(name, value);

trunk/Source/WebCore/mathml/MathMLUnderOverElement.h

@@ -42 +42 @@
     void parseAttribute(const QualifiedName&, const AtomicString&) final;
 
-    BooleanAttribute m_accent;
-    BooleanAttribute m_accentUnder;
+    Optional<BooleanValue> m_accent;
+    Optional<BooleanValue> m_accentUnder;
 };