Move parsing of boolean operator properties into MathMLOperatorElement
authorfred.wang@free.fr <fred.wang@free.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Aug 2016 18:47:14 +0000 (18:47 +0000)
committerfred.wang@free.fr <fred.wang@free.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Aug 2016 18:47:14 +0000 (18:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=160190

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

No new tests, already covered by existing tests.

* mathml/MathMLOperatorDictionary.h: Add a bit mask with all the property flags set.
* mathml/MathMLOperatorElement.cpp:
(WebCore::attributeNameToPropertyFlag): helper function to map from attribute name to flag.
(WebCore::MathMLOperatorElement::computeOperatorFlag): Helper function to update one
bit of m_properties.flags from the corresponding boolean attribute. The default value is
taken from the operator dictionary data stored in m_dictionaryProperty.flags.
(WebCore::MathMLOperatorElement::hasProperty): Returns whether the operator has a property,
parsing it again if the corresponding attribute is dirty.
(WebCore::MathMLOperatorElement::childrenChanged): Make all properties dirty.
(WebCore::propertyFlagToAttributeName): helper function to map from flag to attribute name.
(WebCore::MathMLOperatorElement::parseAttribute): Make all properties dirty if the form
attribute changed. Make each property dirty when the corresponding attribute changed.
(WebCore::MathMLOperatorElement::flags): Deleted. Replaced with the finer hasProperty function.
* mathml/MathMLOperatorElement.h: Define new structure, member and functions to handle
operator properties.
* rendering/mathml/RenderMathMLFencedOperator.cpp:
(WebCore::RenderMathMLFencedOperator::RenderMathMLFencedOperator): Move m_operatorFlags from
the base class to the derived class.
* rendering/mathml/RenderMathMLFencedOperator.h: Ditto.
* rendering/mathml/RenderMathMLOperator.cpp:
(WebCore::RenderMathMLOperator::RenderMathMLOperator): Ditto.
(WebCore::RenderMathMLOperator::hasOperatorFlag): Just call hasOperatorFlag from the
MathMLOperatorElement class.
(WebCore::RenderMathMLOperator::setOperatorProperties): We do not initialize m_operatorFlags
since it has been removed from the base class. We also do not parse attributes since this
has been moved to the MathMLOperatorElement class.
(WebCore::RenderMathMLOperator::setOperatorFlagFromAttribute): Deleted.
(WebCore::RenderMathMLOperator::setOperatorFlagFromAttributeValue): Deleted.
* rendering/mathml/RenderMathMLOperator.h: Move m_operatorFlags from the base class to the
derived class, remove some parsing helper functions and update hasOperatorFlag to make it
overridable.

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

Source/WebCore/ChangeLog
Source/WebCore/mathml/MathMLOperatorDictionary.h
Source/WebCore/mathml/MathMLOperatorElement.cpp
Source/WebCore/mathml/MathMLOperatorElement.h
Source/WebCore/rendering/mathml/RenderMathMLFencedOperator.cpp
Source/WebCore/rendering/mathml/RenderMathMLFencedOperator.h
Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp
Source/WebCore/rendering/mathml/RenderMathMLOperator.h

index cb4ba55..f79ac01 100644 (file)
@@ -1,3 +1,44 @@
+2016-08-02  Frederic Wang  <fwang@igalia.com>
+
+        Move parsing of boolean operator properties into MathMLOperatorElement
+        https://bugs.webkit.org/show_bug.cgi?id=160190
+
+        Reviewed by Darin Adler.
+
+        No new tests, already covered by existing tests.
+
+        * mathml/MathMLOperatorDictionary.h: Add a bit mask with all the property flags set.
+        * mathml/MathMLOperatorElement.cpp:
+        (WebCore::attributeNameToPropertyFlag): helper function to map from attribute name to flag.
+        (WebCore::MathMLOperatorElement::computeOperatorFlag): Helper function to update one
+        bit of m_properties.flags from the corresponding boolean attribute. The default value is
+        taken from the operator dictionary data stored in m_dictionaryProperty.flags.
+        (WebCore::MathMLOperatorElement::hasProperty): Returns whether the operator has a property,
+        parsing it again if the corresponding attribute is dirty.
+        (WebCore::MathMLOperatorElement::childrenChanged): Make all properties dirty.
+        (WebCore::propertyFlagToAttributeName): helper function to map from flag to attribute name.
+        (WebCore::MathMLOperatorElement::parseAttribute): Make all properties dirty if the form
+        attribute changed. Make each property dirty when the corresponding attribute changed.
+        (WebCore::MathMLOperatorElement::flags): Deleted. Replaced with the finer hasProperty function.
+        * mathml/MathMLOperatorElement.h: Define new structure, member and functions to handle
+        operator properties.
+        * rendering/mathml/RenderMathMLFencedOperator.cpp:
+        (WebCore::RenderMathMLFencedOperator::RenderMathMLFencedOperator): Move m_operatorFlags from
+        the base class to the derived class.
+        * rendering/mathml/RenderMathMLFencedOperator.h: Ditto.
+        * rendering/mathml/RenderMathMLOperator.cpp:
+        (WebCore::RenderMathMLOperator::RenderMathMLOperator): Ditto.
+        (WebCore::RenderMathMLOperator::hasOperatorFlag): Just call hasOperatorFlag from the
+        MathMLOperatorElement class.
+        (WebCore::RenderMathMLOperator::setOperatorProperties): We do not initialize m_operatorFlags
+        since it has been removed from the base class. We also do not parse attributes since this
+        has been moved to the MathMLOperatorElement class.
+        (WebCore::RenderMathMLOperator::setOperatorFlagFromAttribute): Deleted.
+        (WebCore::RenderMathMLOperator::setOperatorFlagFromAttributeValue): Deleted.
+        * rendering/mathml/RenderMathMLOperator.h: Move m_operatorFlags from the base class to the
+        derived class, remove some parsing helper functions and update hasOperatorFlag to make it
+        overridable.
+
 2016-08-01  Anders Carlsson  <andersca@apple.com>
 
         Freeze another bunch of Objective-C binding files
index 93e0082..fe148f8 100644 (file)
@@ -43,6 +43,7 @@ enum Flag {
     Stretchy = 0x20,
     Symmetric = 0x40
 };
+const unsigned allFlags = Accent | Fence | LargeOp | MovableLimits | Separator | Stretchy | Symmetric;
 struct Entry {
     UChar character;
     unsigned form : 2;
index f123dc9..2a059dc 100644 (file)
@@ -108,10 +108,58 @@ const MathMLOperatorElement::DictionaryProperty& MathMLOperatorElement::dictiona
     return m_dictionaryProperty.value();
 }
 
-unsigned short MathMLOperatorElement::flags()
+static const QualifiedName& propertyFlagToAttributeName(MathMLOperatorDictionary::Flag flag)
 {
-    // FIXME: We should also handle boolean attributes here (https://webkit.org/b/160190).
-    return dictionaryProperty().flags;
+    switch (flag) {
+    case Accent:
+        return accentAttr;
+    case Fence:
+        return fenceAttr;
+    case LargeOp:
+        return largeopAttr;
+    case MovableLimits:
+        return movablelimitsAttr;
+    case Separator:
+        return separatorAttr;
+    case Stretchy:
+        return stretchyAttr;
+    case Symmetric:
+        return symmetricAttr;
+    }
+    ASSERT_NOT_REACHED();
+}
+
+void MathMLOperatorElement::computeOperatorFlag(MathMLOperatorDictionary::Flag flag)
+{
+    ASSERT(m_properties.dirtyFlags & flag);
+
+    Optional<BooleanValue> property;
+    const auto& name = propertyFlagToAttributeName(flag);
+    const BooleanValue& value = cachedBooleanAttribute(name, property);
+    switch (value) {
+    case BooleanValue::True:
+        m_properties.flags |= flag;
+        break;
+    case BooleanValue::False:
+        m_properties.flags &= ~flag;
+        break;
+    case BooleanValue::Default:
+        // By default, we use the value specified in the operator dictionary.
+        if (dictionaryProperty().flags & flag)
+            m_properties.flags |= flag;
+        else
+            m_properties.flags &= ~flag;
+        break;
+    }
+}
+
+bool MathMLOperatorElement::hasProperty(MathMLOperatorDictionary::Flag flag)
+{
+    if (m_properties.dirtyFlags & flag) {
+        computeOperatorFlag(flag);
+        m_properties.dirtyFlags &= ~flag;
+    }
+    return m_properties.flags & flag;
 }
 
 MathMLElement::Length MathMLOperatorElement::defaultLeadingSpace()
@@ -134,13 +182,36 @@ void MathMLOperatorElement::childrenChanged(const ChildChange& change)
 {
     m_operatorText = Nullopt;
     m_dictionaryProperty = Nullopt;
+    m_properties.dirtyFlags = MathMLOperatorDictionary::allFlags;
     MathMLTextElement::childrenChanged(change);
 }
 
+static Optional<MathMLOperatorDictionary::Flag> attributeNameToPropertyFlag(const QualifiedName& name)
+{
+    if (name == accentAttr)
+        return Accent;
+    if (name == fenceAttr)
+        return Fence;
+    if (name == largeopAttr)
+        return LargeOp;
+    if (name == movablelimitsAttr)
+        return MovableLimits;
+    if (name == separatorAttr)
+        return Separator;
+    if (name == stretchyAttr)
+        return Stretchy;
+    if (name == symmetricAttr)
+        return Symmetric;
+    return Nullopt;
+}
+
 void MathMLOperatorElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
 {
-    if (name == formAttr)
+    if (name == formAttr) {
         m_dictionaryProperty = Nullopt;
+        m_properties.dirtyFlags = MathMLOperatorDictionary::allFlags;
+    } else if (auto flag = attributeNameToPropertyFlag(name))
+        m_properties.dirtyFlags |= flag.value();
 
     if ((name == stretchyAttr || name == lspaceAttr || name == rspaceAttr || name == movablelimitsAttr) && renderer()) {
         downcast<RenderMathMLOperator>(*renderer()).updateFromElement();
index 055cac3..3e250d7 100644 (file)
@@ -38,7 +38,7 @@ public:
     UChar operatorText();
     void setOperatorFormDirty() { m_dictionaryProperty = Nullopt; }
     MathMLOperatorDictionary::Form form() { return dictionaryProperty().form; }
-    unsigned short flags();
+    bool hasProperty(MathMLOperatorDictionary::Flag);
     Length defaultLeadingSpace();
     Length defaultTrailingSpace();
 
@@ -61,6 +61,13 @@ private:
     Optional<DictionaryProperty> m_dictionaryProperty;
     DictionaryProperty computeDictionaryProperty();
     const DictionaryProperty& dictionaryProperty();
+
+    struct OperatorProperties {
+        unsigned short flags;
+        unsigned short dirtyFlags { MathMLOperatorDictionary::allFlags };
+    };
+    OperatorProperties m_properties;
+    void computeOperatorFlag(MathMLOperatorDictionary::Flag);
 };
 
 }
index 61071d5..fdfabd6 100644 (file)
@@ -37,8 +37,9 @@ namespace WebCore {
 using namespace MathMLOperatorDictionary;
 
 RenderMathMLFencedOperator::RenderMathMLFencedOperator(Document& document, RenderStyle&& style, const String& operatorString, MathMLOperatorDictionary::Form form, unsigned short flags)
-    : RenderMathMLOperator(document, WTFMove(style), flags)
+    : RenderMathMLOperator(document, WTFMove(style))
     , m_operatorForm(form)
+    , m_operatorFlags(flags)
 {
     updateOperatorContent(operatorString);
 
index 04f969a..de5fe67 100644 (file)
@@ -41,9 +41,11 @@ private:
     bool isRenderMathMLFencedOperator() const final { return true; }
     void setOperatorProperties() final;
     UChar textContent() const final { return m_textContent; }
+    bool hasOperatorFlag(MathMLOperatorDictionary::Flag flag) const final { return m_operatorFlags & flag; }
 
     UChar m_textContent { 0 };
     MathMLOperatorDictionary::Form m_operatorForm;
+    unsigned short m_operatorFlags;
 };
 
 }; // namespace WebCore
index 5712df9..0f7a5ee 100644 (file)
@@ -53,9 +53,8 @@ RenderMathMLOperator::RenderMathMLOperator(MathMLOperatorElement& element, Rende
     updateTokenContent();
 }
 
-RenderMathMLOperator::RenderMathMLOperator(Document& document, RenderStyle&& style, unsigned short flags)
+RenderMathMLOperator::RenderMathMLOperator(Document& document, RenderStyle&& style)
     : RenderMathMLToken(document, WTFMove(style))
-    , m_operatorFlags(flags)
 {
 }
 
@@ -70,20 +69,9 @@ UChar RenderMathMLOperator::textContent() const
     return element().operatorText();
 }
 
-void RenderMathMLOperator::setOperatorFlagFromAttribute(MathMLOperatorDictionary::Flag flag, const QualifiedName& name)
+bool RenderMathMLOperator::hasOperatorFlag(MathMLOperatorDictionary::Flag flag) const
 {
-    setOperatorFlagFromAttributeValue(flag, element().attributeWithoutSynchronization(name));
-}
-
-void RenderMathMLOperator::setOperatorFlagFromAttributeValue(MathMLOperatorDictionary::Flag flag, const AtomicString& attributeValue)
-{
-    ASSERT(!isAnonymous());
-
-    if (attributeValue == "true")
-        m_operatorFlags |= flag;
-    else if (attributeValue == "false")
-        m_operatorFlags &= ~flag;
-    // We ignore absent or invalid attributes.
+    return element().hasProperty(flag);
 }
 
 void RenderMathMLOperator::setOperatorProperties()
@@ -92,7 +80,6 @@ void RenderMathMLOperator::setOperatorProperties()
     m_isVertical = MathMLOperatorDictionary::isVertical(textContent());
 
     // Initialize with the default values.
-    m_operatorFlags = element().flags();
     m_leadingSpace = toUserUnits(element().defaultLeadingSpace(), style(), 0);
     m_trailingSpace = toUserUnits(element().defaultTrailingSpace(), style(), 0);
     m_minSize = style().fontCascade().size(); // This sets minsize to "1em".
@@ -100,15 +87,6 @@ void RenderMathMLOperator::setOperatorProperties()
 
     if (!isAnonymous()) {
         // Finally, we make the attribute values override the default.
-
-        setOperatorFlagFromAttribute(MathMLOperatorDictionary::Fence, MathMLNames::fenceAttr);
-        setOperatorFlagFromAttribute(MathMLOperatorDictionary::Separator, MathMLNames::separatorAttr);
-        setOperatorFlagFromAttribute(MathMLOperatorDictionary::Stretchy, MathMLNames::stretchyAttr);
-        setOperatorFlagFromAttribute(MathMLOperatorDictionary::Symmetric, MathMLNames::symmetricAttr);
-        setOperatorFlagFromAttribute(MathMLOperatorDictionary::LargeOp, MathMLNames::largeopAttr);
-        setOperatorFlagFromAttribute(MathMLOperatorDictionary::MovableLimits, MathMLNames::movablelimitsAttr);
-        setOperatorFlagFromAttribute(MathMLOperatorDictionary::Accent, MathMLNames::accentAttr);
-
         parseMathMLLength(element().attributeWithoutSynchronization(MathMLNames::lspaceAttr), m_leadingSpace, &style(), false); // FIXME: Negative leading space must be implemented (https://bugs.webkit.org/show_bug.cgi?id=124830).
         parseMathMLLength(element().attributeWithoutSynchronization(MathMLNames::rspaceAttr), m_trailingSpace, &style(), false); // FIXME: Negative trailing space must be implemented (https://bugs.webkit.org/show_bug.cgi?id=124830).
 
index c9508fb..940149b 100644 (file)
@@ -39,7 +39,7 @@ class MathMLOperatorElement;
 class RenderMathMLOperator : public RenderMathMLToken {
 public:
     RenderMathMLOperator(MathMLOperatorElement&, RenderStyle&&);
-    RenderMathMLOperator(Document&, RenderStyle&&, unsigned short flags = 0);
+    RenderMathMLOperator(Document&, RenderStyle&&);
     MathMLOperatorElement& element() const;
 
     void stretchTo(LayoutUnit heightAboveBaseline, LayoutUnit depthBelowBaseline);
@@ -47,7 +47,7 @@ public:
     LayoutUnit stretchSize() const { return m_isVertical ? m_stretchHeightAboveBaseline + m_stretchDepthBelowBaseline : m_stretchWidth; }
     void resetStretchSize();
 
-    bool hasOperatorFlag(MathMLOperatorDictionary::Flag flag) const { return m_operatorFlags & flag; }
+    virtual bool hasOperatorFlag(MathMLOperatorDictionary::Flag) const;
     bool isLargeOperatorInDisplayStyle() const { return !hasOperatorFlag(MathMLOperatorDictionary::Stretchy) && hasOperatorFlag(MathMLOperatorDictionary::LargeOp) && mathMLStyle()->displayStyle(); }
     bool shouldMoveLimits() const { return hasOperatorFlag(MathMLOperatorDictionary::MovableLimits) && !mathMLStyle()->displayStyle(); }
     bool isVertical() const { return m_isVertical; }
@@ -67,7 +67,6 @@ protected:
     LayoutUnit m_trailingSpace;
     LayoutUnit m_minSize;
     LayoutUnit m_maxSize;
-    unsigned short m_operatorFlags;
 
 private:
     void styleDidChange(StyleDifference, const RenderStyle* oldStyle) final;
@@ -90,9 +89,6 @@ private:
     bool shouldAllowStretching() const;
     bool useMathOperator() const;
 
-    void setOperatorFlagFromAttribute(MathMLOperatorDictionary::Flag, const QualifiedName&);
-    void setOperatorFlagFromAttributeValue(MathMLOperatorDictionary::Flag, const AtomicString& attributeValue);
-
     LayoutUnit verticalStretchedOperatorShift() const;
 
     LayoutUnit m_stretchHeightAboveBaseline { 0 };