Exploit shared attribute data to avoid parsing identical "style" attr…

…ibutes.

<http://webkit.org/b/101163>

Reviewed by Antti Koivisto.

Track the "inline style dirty" state on ElementAttributeData instead of in a Node flag.
This allows us to avoid duplicate work for ElementAttributeData that are shared between multiple elements,
since the state is no longer per-Element.

* css/StyleResolver.cpp:
(WebCore::isCacheableInMatchedPropertiesCache):

    Disable the matched properties cache for styles with non-standard writing-mode.
    This is necessary because some CSS properties have different meaning depending on context -
    properties handled by CSSProperty::resolveDirectionAwareProperty().

    Now that multiple elements may have identical inlineStyle() pointers, this is necessary to
    avoid mapping StylePropertySets with direction-aware properties to RenderStyles with differing
    writing-modes in the matched properties cache.

* dom/Node.h:
* dom/ElementAttributeData.cpp:
(WebCore::ElementAttributeData::ElementAttributeData):
* dom/ElementAttributeData.h:
(WebCore::ElementAttributeData::ElementAttributeData):
(ElementAttributeData):
* dom/Element.h:
(WebCore::Element::updateInvalidAttributes):
* dom/Element.cpp:
(WebCore::Element::getAttribute):
(WebCore::Element::removeAttribute):
* dom/StyledElement.h:
(WebCore::StyledElement::invalidateStyleAttribute):
* dom/StyledElement.cpp:
(WebCore::StyledElement::updateStyleAttribute):

    Move "style attribute dirty" flag to ElementAttributeData.

(WebCore::Element::cloneAttributesFromElement):

    Remove ugly optimization to avoid reparsing inline style when cloning elements. This now happens
    automagically since cloning nodes just refs the original attribute data.

* dom/StyledElement.cpp:
(WebCore::StyledElement::updateStyleAttribute):
(WebCore::StyledElement::setInlineStyleFromString):
(WebCore::StyledElement::styleAttributeChanged):
(WebCore::StyledElement::inlineStyleChanged):

    Avoid reparsing the inline style if the element's attribute data is immutable and already has
    a parsed inlineStyle(). Split the set-inline-style-from-string code out of styleAttributeChanged()
    to make the code more understandable.


Canonical link: https://commits.webkit.org/120704@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@135021 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,58 @@
+2012-11-16  Andreas Kling  <akling@apple.com>
+
+        Exploit shared attribute data to avoid parsing identical "style" attributes.
+        <http://webkit.org/b/101163>
+
+        Reviewed by Antti Koivisto.
+
+        Track the "inline style dirty" state on ElementAttributeData instead of in a Node flag.
+        This allows us to avoid duplicate work for ElementAttributeData that are shared between multiple elements,
+        since the state is no longer per-Element.
+
+        * css/StyleResolver.cpp:
+        (WebCore::isCacheableInMatchedPropertiesCache):
+
+            Disable the matched properties cache for styles with non-standard writing-mode.
+            This is necessary because some CSS properties have different meaning depending on context -
+            properties handled by CSSProperty::resolveDirectionAwareProperty().
+
+            Now that multiple elements may have identical inlineStyle() pointers, this is necessary to
+            avoid mapping StylePropertySets with direction-aware properties to RenderStyles with differing
+            writing-modes in the matched properties cache.
+
+        * dom/Node.h:
+        * dom/ElementAttributeData.cpp:
+        (WebCore::ElementAttributeData::ElementAttributeData):
+        * dom/ElementAttributeData.h:
+        (WebCore::ElementAttributeData::ElementAttributeData):
+        (ElementAttributeData):
+        * dom/Element.h:
+        (WebCore::Element::updateInvalidAttributes):
+        * dom/Element.cpp:
+        (WebCore::Element::getAttribute):
+        (WebCore::Element::removeAttribute):
+        * dom/StyledElement.h:
+        (WebCore::StyledElement::invalidateStyleAttribute):
+        * dom/StyledElement.cpp:
+        (WebCore::StyledElement::updateStyleAttribute):
+
+            Move "style attribute dirty" flag to ElementAttributeData.
+
+        (WebCore::Element::cloneAttributesFromElement):
+
+            Remove ugly optimization to avoid reparsing inline style when cloning elements. This now happens
+            automagically since cloning nodes just refs the original attribute data.
+
+        * dom/StyledElement.cpp:
+        (WebCore::StyledElement::updateStyleAttribute):
+        (WebCore::StyledElement::setInlineStyleFromString):
+        (WebCore::StyledElement::styleAttributeChanged):
+        (WebCore::StyledElement::inlineStyleChanged):
+
+            Avoid reparsing the inline style if the element's attribute data is immutable and already has
+            a parsed inlineStyle(). Split the set-inline-style-from-string code out of styleAttributeChanged()
+            to make the code more understandable.
+
 2012-11-16  Simon Fraser  <simon.fraser@apple.com>
 
         Don't update layer positions on scrolling if we're in the middle of layout

Source/WebCore/css/StyleResolver.cpp

@@ -2404,6 +2404,8 @@ static bool isCacheableInMatchedPropertiesCache(const Element* element, const Re
         return false;
     if (style->zoom() != RenderStyle::initialZoom())
         return false;
+    if (style->writingMode() != RenderStyle::initialWritingMode())
+        return false;
     // The cache assumes static knowledge about which properties are inherited.
     if (parentStyle->hasExplicitlyInheritedProperties())
         return false;

Source/WebCore/dom/Element.cpp

@@ -304,7 +304,7 @@ bool Element::hasAttribute(const QualifiedName& name) const
 
 const AtomicString& Element::getAttribute(const QualifiedName& name) const
 {
-    if (UNLIKELY(name == styleAttr) && !isStyleAttributeValid())
+    if (UNLIKELY(name == styleAttr) && attributeData() && attributeData()->m_styleAttributeIsDirty)
         updateStyleAttribute();
 
 #if ENABLE(SVG)
@@ -662,7 +662,7 @@ const AtomicString& Element::getAttribute(const AtomicString& name) const
     bool ignoreCase = shouldIgnoreAttributeCase(this);
 
     // Update the 'style' attribute if it's invalid and being requested:
-    if (!isStyleAttributeValid() && equalPossiblyIgnoringCase(name, styleAttr.localName(), ignoreCase))
+    if (attributeData() && attributeData()->m_styleAttributeIsDirty && equalPossiblyIgnoringCase(name, styleAttr.localName(), ignoreCase))
         updateStyleAttribute();
 
 #if ENABLE(SVG)
@@ -1652,7 +1652,7 @@ void Element::removeAttribute(const AtomicString& name)
     AtomicString localName = shouldIgnoreAttributeCase(this) ? name.lower() : name;
     size_t index = attributeData()->getAttributeItemIndex(localName, false);
     if (index == notFound) {
-        if (UNLIKELY(localName == styleAttr) && !isStyleAttributeValid() && isStyledElement())
+        if (UNLIKELY(localName == styleAttr) && attributeData()->m_styleAttributeIsDirty && isStyledElement())
             static_cast<StyledElement*>(this)->removeAllInlineStyleProperties();
         return;
     }
@@ -2444,8 +2444,6 @@ void Element::cloneAttributesFromElement(const Element& other)
     if (hasSyntheticAttrChildNodes())
         detachAllAttrNodesFromElement();
 
-    setIsStyleAttributeValid(other.isStyleAttributeValid());
-
     other.updateInvalidAttributes();
     if (!other.m_attributeData) {
         m_attributeData.clear();
@@ -2476,11 +2474,6 @@ void Element::cloneAttributesFromElement(const Element& other)
 
     for (unsigned i = 0; i < m_attributeData->length(); ++i) {
         const Attribute* attribute = const_cast<const ElementAttributeData*>(m_attributeData.get())->attributeItem(i);
-        // This optimization isn't very nicely factored, but a huge win for cloning elements with inline style.
-        if (isStyledElement() && attribute->name() == HTMLNames::styleAttr) {
-            static_cast<StyledElement*>(this)->styleAttributeChanged(attribute->value(), StyledElement::DoNotReparseStyleAttribute);
-            continue;
-        }
         attributeChanged(attribute->name(), attribute->value());
     }
 }

Source/WebCore/dom/Element.h

@@ -732,7 +732,7 @@ inline Attribute* Element::getAttributeItem(const QualifiedName& name)
 
 inline void Element::updateInvalidAttributes() const
 {
-    if (!isStyleAttributeValid())
+    if (attributeData() && attributeData()->m_styleAttributeIsDirty)
         updateStyleAttribute();
 
 #if ENABLE(SVG)

Source/WebCore/dom/ElementAttributeData.cpp

@@ -83,6 +83,7 @@ ElementAttributeData::ElementAttributeData(const ElementAttributeData& other, bo
     : m_isMutable(isMutable)
     , m_arraySize(isMutable ? 0 : other.length())
     , m_presentationAttributeStyleIsDirty(other.m_presentationAttributeStyleIsDirty)
+    , m_styleAttributeIsDirty(other.m_styleAttributeIsDirty)
     , m_presentationAttributeStyle(other.m_presentationAttributeStyle)
     , m_classNames(other.m_classNames)
     , m_idForStyleResolution(other.m_idForStyleResolution)

Source/WebCore/dom/ElementAttributeData.h

@@ -89,19 +89,22 @@ class ElementAttributeData : public RefCounted<ElementAttributeData> {
         : m_isMutable(true)
         , m_arraySize(0)
         , m_presentationAttributeStyleIsDirty(false)
+        , m_styleAttributeIsDirty(false)
     { }
 
     ElementAttributeData(unsigned arraySize)
         : m_isMutable(false)
         , m_arraySize(arraySize)
         , m_presentationAttributeStyleIsDirty(false)
+        , m_styleAttributeIsDirty(false)
     { }
 
     ElementAttributeData(const ElementAttributeData&, bool isMutable);
 
     unsigned m_isMutable : 1;
-    unsigned m_arraySize : 30;
+    unsigned m_arraySize : 29;
     mutable unsigned m_presentationAttributeStyleIsDirty : 1;
+    mutable unsigned m_styleAttributeIsDirty : 1;
 
     mutable RefPtr<StylePropertySet> m_inlineStyle;
     mutable RefPtr<StylePropertySet> m_presentationAttributeStyle;

Source/WebCore/dom/Node.h

@@ -98,7 +98,7 @@ class PropertyNodeList;
 
 typedef int ExceptionCode;
 
-const int nodeStyleChangeShift = 20;
+const int nodeStyleChangeShift = 19;
 
 // SyntheticStyleChange means that we need to go through the entire style change logic even though
 // no style property has actually changed. It is used to restructure the tree when, for instance,
@@ -710,34 +710,33 @@ class Node : public EventTarget, public ScriptWrappable, public TreeShared<Node,
         // These bits are used by derived classes, pulled up here so they can
         // be stored in the same memory word as the Node bits above.
         IsParsingChildrenFinishedFlag = 1 << 15, // Element
-        IsStyleAttributeValidFlag = 1 << 16, // StyledElement
 #if ENABLE(SVG)
-        AreSVGAttributesValidFlag = 1 << 17, // Element
-        IsSynchronizingSVGAttributesFlag = 1 << 18, // SVGElement
-        HasSVGRareDataFlag = 1 << 19, // SVGElement
+        AreSVGAttributesValidFlag = 1 << 16, // Element
+        IsSynchronizingSVGAttributesFlag = 1 << 17, // SVGElement
+        HasSVGRareDataFlag = 1 << 18, // SVGElement
 #endif
 
         StyleChangeMask = 1 << nodeStyleChangeShift | 1 << (nodeStyleChangeShift + 1),
 
-        SelfOrAncestorHasDirAutoFlag = 1 << 22,
+        SelfOrAncestorHasDirAutoFlag = 1 << 21,
 
-        HasNameFlag = 1 << 23,
+        HasNameFlag = 1 << 22,
 
-        InNamedFlowFlag = 1 << 24,
-        HasSyntheticAttrChildNodesFlag = 1 << 25,
-        HasCustomCallbacksFlag = 1 << 26,
-        HasScopedHTMLStyleChildFlag = 1 << 27,
-        HasEventTargetDataFlag = 1 << 28,
-        InEdenFlag = 1 << 29,
+        InNamedFlowFlag = 1 << 23,
+        HasSyntheticAttrChildNodesFlag = 1 << 24,
+        HasCustomCallbacksFlag = 1 << 25,
+        HasScopedHTMLStyleChildFlag = 1 << 26,
+        HasEventTargetDataFlag = 1 << 27,
+        InEdenFlag = 1 << 28,
 
 #if ENABLE(SVG)
-        DefaultNodeFlags = IsParsingChildrenFinishedFlag | IsStyleAttributeValidFlag | AreSVGAttributesValidFlag,
+        DefaultNodeFlags = IsParsingChildrenFinishedFlag | AreSVGAttributesValidFlag,
 #else
-        DefaultNodeFlags = IsParsingChildrenFinishedFlag | IsStyleAttributeValidFlag,
+        DefaultNodeFlags = IsParsingChildrenFinishedFlag,
 #endif
     };
 
-    // 2 bits remaining
+    // 3 bits remaining
 
     bool getFlag(NodeFlags mask) const { return m_nodeFlags & mask; }
     void setFlag(bool f, NodeFlags mask) const { m_nodeFlags = (m_nodeFlags & ~mask) | (-(int32_t)f & mask); } 
@@ -838,12 +837,6 @@ class Node : public EventTarget, public ScriptWrappable, public TreeShared<Node,
         NodeRareDataBase* m_rareData;
     } m_data;
 
-public:
-    bool isStyleAttributeValid() const { return getFlag(IsStyleAttributeValidFlag); }
-    void setIsStyleAttributeValid(bool f) { setFlag(f, IsStyleAttributeValidFlag); }
-    void setIsStyleAttributeValid() const { setFlag(IsStyleAttributeValidFlag); }
-    void clearIsStyleAttributeValid() { clearFlag(IsStyleAttributeValidFlag); }
-
 protected:
     bool isParsingChildrenFinished() const { return getFlag(IsParsingChildrenFinishedFlag); }
     void setIsParsingChildrenFinished() { setFlag(IsParsingChildrenFinishedFlag); }

Source/WebCore/dom/StyledElement.cpp

@@ -128,8 +128,9 @@ static PresentationAttributeCacheCleaner& presentationAttributeCacheCleaner()
 
 void StyledElement::updateStyleAttribute() const
 {
-    ASSERT(!isStyleAttributeValid());
-    setIsStyleAttributeValid();
+    ASSERT(attributeData());
+    ASSERT(attributeData()->m_styleAttributeIsDirty);
+    attributeData()->m_styleAttributeIsDirty = false;
     if (const StylePropertySet* inlineStyle = this->inlineStyle())
         const_cast<StyledElement*>(this)->setSynchronizedLazyAttribute(styleAttr, inlineStyle->asText());
 }
@@ -181,30 +182,41 @@ PropertySetCSSStyleDeclaration* StyledElement::inlineStyleCSSOMWrapper()
     return cssomWrapper;
 }
 
-void StyledElement::styleAttributeChanged(const AtomicString& newStyleString, ShouldReparseStyleAttribute shouldReparse)
+inline void StyledElement::setInlineStyleFromString(const AtomicString& newStyleString)
 {
-    if (shouldReparse) {
-        WTF::OrdinalNumber startLineNumber = WTF::OrdinalNumber::beforeFirst();
-        if (document() && document()->scriptableDocumentParser() && !document()->isInDocumentWrite())
-            startLineNumber = document()->scriptableDocumentParser()->lineNumber();
-
-        if (newStyleString.isNull()) {
-            if (PropertySetCSSStyleDeclaration* cssomWrapper = inlineStyleCSSOMWrapper())
-                cssomWrapper->clearParentElement();
-            mutableAttributeData()->m_inlineStyle.clear();
-        } else if (document()->contentSecurityPolicy()->allowInlineStyle(document()->url(), startLineNumber)) {
-            // We reconstruct the property set instead of mutating if there is no CSSOM wrapper.
-            // This makes wrapperless property sets immutable and so cacheable.
-            RefPtr<StylePropertySet>& inlineStyle = attributeData()->m_inlineStyle;
-            if (inlineStyle && !inlineStyle->isMutable())
-                inlineStyle.clear();
-            if (!inlineStyle)
-                inlineStyle = CSSParser::parseInlineStyleDeclaration(newStyleString, this);
-            else
-                inlineStyle->parseDeclaration(newStyleString, document()->elementSheet()->contents());
-        }
-        setIsStyleAttributeValid();
-    }
+    RefPtr<StylePropertySet>& inlineStyle = attributeData()->m_inlineStyle;
+
+    // Avoid redundant work if we're using shared attribute data with already parsed inline style.
+    if (inlineStyle && !attributeData()->isMutable())
+        return;
+
+    // We reconstruct the property set instead of mutating if there is no CSSOM wrapper.
+    // This makes wrapperless property sets immutable and so cacheable.
+    if (inlineStyle && !inlineStyle->isMutable())
+        inlineStyle.clear();
+
+    if (!inlineStyle)
+        inlineStyle = CSSParser::parseInlineStyleDeclaration(newStyleString, this);
+    else
+        inlineStyle->parseDeclaration(newStyleString, document()->elementSheet()->contents());
+}
+
+void StyledElement::styleAttributeChanged(const AtomicString& newStyleString)
+{
+    WTF::OrdinalNumber startLineNumber = WTF::OrdinalNumber::beforeFirst();
+    if (document() && document()->scriptableDocumentParser() && !document()->isInDocumentWrite())
+        startLineNumber = document()->scriptableDocumentParser()->lineNumber();
+
+    if (newStyleString.isNull()) {
+        if (PropertySetCSSStyleDeclaration* cssomWrapper = inlineStyleCSSOMWrapper())
+            cssomWrapper->clearParentElement();
+        mutableAttributeData()->m_inlineStyle.clear();
+    } else if (document()->contentSecurityPolicy()->allowInlineStyle(document()->url(), startLineNumber))
+        setInlineStyleFromString(newStyleString);
+
+    attributeData()->m_styleAttributeIsDirty = false;
+
+    // FIXME: Why aren't we using setNeedsStyleRecalc(InlineStyleChange) here?
     setNeedsStyleRecalc();
     InspectorInstrumentation::didInvalidateStyleAttr(document(), this);
 }
@@ -220,7 +232,8 @@ void StyledElement::parseAttribute(const Attribute& attribute)
 void StyledElement::inlineStyleChanged()
 {
     setNeedsStyleRecalc(InlineStyleChange);
-    setIsStyleAttributeValid(false);
+    ASSERT(attributeData());
+    attributeData()->m_styleAttributeIsDirty = true;
     InspectorInstrumentation::didInvalidateStyleAttr(document(), this);
 }
 

Source/WebCore/dom/StyledElement.h

@@ -56,10 +56,6 @@ class StyledElement : public Element {
 
     virtual void collectStyleForPresentationAttribute(const Attribute&, StylePropertySet*) { }
 
-    // May be called by ElementAttributeData::cloneDataFrom().
-    enum ShouldReparseStyleAttribute { DoNotReparseStyleAttribute = 0, ReparseStyleAttribute = 1 };
-    void styleAttributeChanged(const AtomicString& newStyleString, ShouldReparseStyleAttribute = ReparseStyleAttribute);
-
 protected:
     StyledElement(const QualifiedName&, Document*, ConstructionType);
 
@@ -75,17 +71,21 @@ class StyledElement : public Element {
     virtual void addSubresourceAttributeURLs(ListHashSet<KURL>&) const;
 
 private:
+    void styleAttributeChanged(const AtomicString& newStyleString);
+
     virtual void updateStyleAttribute() const;
     void inlineStyleChanged();
     PropertySetCSSStyleDeclaration* inlineStyleCSSOMWrapper();
+    void setInlineStyleFromString(const AtomicString&);
 
     void makePresentationAttributeCacheKey(PresentationAttributeCacheKey&) const;
     void rebuildPresentationAttributeStyle();
 };
 
 inline void StyledElement::invalidateStyleAttribute()
 {
-    clearIsStyleAttributeValid();
+    ASSERT(attributeData());
+    attributeData()->m_styleAttributeIsDirty = true;
 }
 
 inline const StylePropertySet* StyledElement::presentationAttributeStyle()