Changeset 117323 in webkit


Ignore:
Timestamp:
May 16, 2012 12:31:40 PM (12 years ago)
Author:
kling@webkit.org
Message:

Avoid reparsing the style attribute when cloning elements.
<​http://webkit.org/b/86574>

Reviewed by Antti Koivisto.

Refactor cloning of attributes a bit to dodge the styleAttr reparse previously
caused by ElementAttributeData::setAttributes().

Introduced Element::cloneDataFromElement() which takes care of cloning the
ElementAttributeData as well as "non-attribute properties" (which is currently
specific to HTMLInputElement.)

Also includes some additional dodging of attribute vector traversal to find
old/new 'id' and 'name' attributes.

I'm seeing a ~10% improvement on PerformanceTests/DOM/CloneNodes locally.

  • dom/Document.cpp:

(WebCore::Document::importNode):

  • dom/Element.cpp:

(WebCore::Element::cloneElementWithoutChildren):
(WebCore::Element::cloneAttributesFromElement):
(WebCore::Element::cloneDataFromElement):

  • dom/Element.h:

(WebCore::Element::copyNonAttributePropertiesFromElement):

  • dom/ElementAttributeData.cpp:

(WebCore::ElementAttributeData::cloneDataFrom):

  • dom/ElementAttributeData.h:

(ElementAttributeData):

  • dom/Node.h:
  • dom/StyledElement.cpp:

(WebCore::StyledElement::styleAttributeChanged):
(WebCore::StyledElement::parseAttribute):

  • dom/StyledElement.h:
  • editing/ReplaceNodeWithSpanCommand.cpp:

(WebCore::swapInNodePreservingAttributesAndChildren):

  • html/HTMLElement.cpp:
  • html/HTMLInputElement.cpp:

(WebCore::HTMLInputElement::copyNonAttributePropertiesFromElement):

  • html/HTMLInputElement.h:
  • inspector/DOMPatchSupport.cpp:

(WebCore::DOMPatchSupport::innerPatchNode):

  • inspector/InspectorDOMAgent.cpp:

(WebCore::InspectorDOMAgent::setNodeName):

  • svg/SVGUseElement.cpp:

(WebCore::SVGUseElement::expandSymbolElementsInShadowTree):
(WebCore::SVGUseElement::transferUseAttributesToReplacedElement):

Location:
trunk/Source/WebCore
Files:
16 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r117322 r117323  
     12012-05-16  Andreas Kling  <kling@webkit.org>
     2
     3        Avoid reparsing the style attribute when cloning elements.
     4        <http://webkit.org/b/86574>
     5
     6        Reviewed by Antti Koivisto.
     7
     8        Refactor cloning of attributes a bit to dodge the styleAttr reparse previously
     9        caused by ElementAttributeData::setAttributes().
     10
     11        Introduced Element::cloneDataFromElement() which takes care of cloning the
     12        ElementAttributeData as well as "non-attribute properties" (which is currently
     13        specific to HTMLInputElement.)
     14
     15        Also includes some additional dodging of attribute vector traversal to find
     16        old/new 'id' and 'name' attributes.
     17
     18        I'm seeing a ~10% improvement on PerformanceTests/DOM/CloneNodes locally.
     19
     20        * dom/Document.cpp:
     21        (WebCore::Document::importNode):
     22        * dom/Element.cpp:
     23        (WebCore::Element::cloneElementWithoutChildren):
     24        (WebCore::Element::cloneAttributesFromElement):
     25        (WebCore::Element::cloneDataFromElement):
     26        * dom/Element.h:
     27        (WebCore::Element::copyNonAttributePropertiesFromElement):
     28        * dom/ElementAttributeData.cpp:
     29        (WebCore::ElementAttributeData::cloneDataFrom):
     30        * dom/ElementAttributeData.h:
     31        (ElementAttributeData):
     32        * dom/Node.h:
     33        * dom/StyledElement.cpp:
     34        (WebCore::StyledElement::styleAttributeChanged):
     35        (WebCore::StyledElement::parseAttribute):
     36        * dom/StyledElement.h:
     37        * editing/ReplaceNodeWithSpanCommand.cpp:
     38        (WebCore::swapInNodePreservingAttributesAndChildren):
     39        * html/HTMLElement.cpp:
     40        * html/HTMLInputElement.cpp:
     41        (WebCore::HTMLInputElement::copyNonAttributePropertiesFromElement):
     42        * html/HTMLInputElement.h:
     43        * inspector/DOMPatchSupport.cpp:
     44        (WebCore::DOMPatchSupport::innerPatchNode):
     45        * inspector/InspectorDOMAgent.cpp:
     46        (WebCore::InspectorDOMAgent::setNodeName):
     47        * svg/SVGUseElement.cpp:
     48        (WebCore::SVGUseElement::expandSymbolElementsInShadowTree):
     49        (WebCore::SVGUseElement::transferUseAttributesToReplacedElement):
     50
    1512012-05-16  Brent Fulgham  <bfulgham@webkit.org>
    252

  • trunk/Source/WebCore/dom/Document.cpp

    r117029 r117323  
    915915            return 0;
    916916
    917         newElement->setAttributesFromElement(*oldElement);
    918         newElement->copyNonAttributeProperties(oldElement);
     917        newElement->cloneDataFromElement(*oldElement);
    919918
    920919        if (deep) {

  • trunk/Source/WebCore/dom/Element.cpp

    r117242 r117323  
    179179    ASSERT(isHTMLElement() == clone->isHTMLElement());
    180180
    181     clone->setAttributesFromElement(*this);
    182     clone->copyNonAttributeProperties(this);
    183 
     181    clone->cloneDataFromElement(*this);
    184182    return clone.release();
    185183}
    … …  
    188186{
    189187    return document()->createElement(tagQName(), false);
    190 }
    191 
    192 void Element::copyNonAttributeProperties(const Element*)
    193 {
    194188}
    195189
    … …  
    21182112
    21192113
     2114void Element::cloneAttributesFromElement(const Element& other)
     2115{
     2116    if (ElementAttributeData* attributeData = other.updatedAttributeData())
     2117        ensureUpdatedAttributeData()->cloneDataFrom(*attributeData, other, *this);
     2118    else if (m_attributeData) {
     2119        m_attributeData->clearAttributes(this);
     2120        m_attributeData.clear();
     2121    }
     2122}
     2123
     2124void Element::cloneDataFromElement(const Element& other)
     2125{
     2126    cloneAttributesFromElement(other);
     2127    copyNonAttributePropertiesFromElement(other);
     2128}
     2129
    21202130} // namespace WebCore

  • trunk/Source/WebCore/dom/Element.h

    r117242 r117323  
    252252    ElementAttributeData* ensureUpdatedAttributeData() const;
    253253
    254     void setAttributesFromElement(const Element&);
     254    // Clones attributes only.
     255    void cloneAttributesFromElement(const Element&);
     256
     257    // Clones all attribute-derived data, including subclass specifics (through copyNonAttributeProperties.)
     258    void cloneDataFromElement(const Element&);
     259
    255260    bool hasEquivalentAttributes(const Element* other) const;
    256261
    257     virtual void copyNonAttributeProperties(const Element* source);
     262    virtual void copyNonAttributePropertiesFromElement(const Element&) { }
    258263
    259264    virtual void attach();
    … …  
    563568}
    564569
    565 inline void Element::setAttributesFromElement(const Element& other)
    566 {
    567     if (ElementAttributeData* attributeData = other.updatedAttributeData())
    568         ensureUpdatedAttributeData()->setAttributes(*attributeData, this);
    569 }
    570 
    571570inline void Element::updateName(const AtomicString& oldName, const AtomicString& newName)
    572571{

  • trunk/Source/WebCore/dom/ElementAttributeData.cpp

    r117195 r117323  
    253253}
    254254
    255 void ElementAttributeData::setAttributes(const ElementAttributeData& other, Element* element)
    256 {
    257     ASSERT(element);
    258 
    259     // If assigning the map changes the id attribute, we need to call
    260     // updateId.
    261     Attribute* oldId = getAttributeItem(element->document()->idAttributeName());
    262     Attribute* newId = other.getAttributeItem(element->document()->idAttributeName());
    263 
    264     if (oldId || newId)
    265         element->updateId(oldId ? oldId->value() : nullAtom, newId ? newId->value() : nullAtom);
    266 
    267     Attribute* oldName = getAttributeItem(HTMLNames::nameAttr);
    268     Attribute* newName = other.getAttributeItem(HTMLNames::nameAttr);
    269 
    270     if (oldName || newName)
    271         element->updateName(oldName ? oldName->value() : nullAtom, newName ? newName->value() : nullAtom);
    272 
    273     clearAttributes(element);
    274     m_attributes = other.m_attributes;
    275     for (unsigned i = 0; i < m_attributes.size(); ++i)
    276         element->attributeChanged(m_attributes[i]);
     255void ElementAttributeData::cloneDataFrom(const ElementAttributeData& sourceData, const Element& sourceElement, Element& targetElement)
     256{
     257    const AtomicString& oldID = targetElement.getIdAttribute();
     258    const AtomicString& newID = sourceElement.getIdAttribute();
     259
     260    if (!oldID.isNull() || !newID.isNull())
     261        targetElement.updateId(oldID, newID);
     262
     263    const AtomicString& oldName = targetElement.getNameAttribute();
     264    const AtomicString& newName = sourceElement.getNameAttribute();
     265
     266    if (!oldName.isNull() || !newName.isNull())
     267        targetElement.updateName(oldName, newName);
     268
     269    clearAttributes(&targetElement);
     270    m_attributes = sourceData.m_attributes;
     271    for (unsigned i = 0; i < m_attributes.size(); ++i) {
     272        if (targetElement.isStyledElement() && m_attributes[i].name() == HTMLNames::styleAttr) {
     273            static_cast<StyledElement&>(targetElement).styleAttributeChanged(m_attributes[i].value(), StyledElement::DoNotReparseStyleAttribute);
     274            continue;
     275        }
     276        targetElement.attributeChanged(m_attributes[i]);
     277    }
     278
     279    if (targetElement.isStyledElement() && sourceData.m_inlineStyleDecl) {
     280        StylePropertySet* inlineStyle = ensureMutableInlineStyle(static_cast<StyledElement*>(&targetElement));
     281        inlineStyle->copyPropertiesFrom(*sourceData.m_inlineStyleDecl);
     282        inlineStyle->setCSSParserMode(sourceData.m_inlineStyleDecl->cssParserMode());
     283        targetElement.setIsStyleAttributeValid(sourceElement.isStyleAttributeValid());
     284    }
    277285}
    278286

  • trunk/Source/WebCore/dom/ElementAttributeData.h

    r116682 r117323  
    114114    Attribute* getAttributeItem(const String& name, bool shouldIgnoreAttributeCase) const;
    115115    size_t getAttributeItemIndexSlowCase(const String& name, bool shouldIgnoreAttributeCase) const;
    116     void setAttributes(const ElementAttributeData& other, Element*);
     116    void cloneDataFrom(const ElementAttributeData& sourceData, const Element& sourceElement, Element& targetElement);
    117117    void clearAttributes(Element*);
    118118    void replaceAttribute(size_t index, const Attribute&, Element*);

  • trunk/Source/WebCore/dom/Node.h

    r117242 r117323  
    791791    RenderObject* m_renderer;
    792792
     793public:
     794    bool isStyleAttributeValid() const { return getFlag(IsStyleAttributeValidFlag); }
     795    void setIsStyleAttributeValid(bool f) { setFlag(f, IsStyleAttributeValidFlag); }
     796    void setIsStyleAttributeValid() const { setFlag(IsStyleAttributeValidFlag); }
     797    void clearIsStyleAttributeValid() { clearFlag(IsStyleAttributeValidFlag); }
     798
    793799protected:
    794800    bool isParsingChildrenFinished() const { return getFlag(IsParsingChildrenFinishedFlag); }
    795801    void setIsParsingChildrenFinished() { setFlag(IsParsingChildrenFinishedFlag); }
    796802    void clearIsParsingChildrenFinished() { clearFlag(IsParsingChildrenFinishedFlag); }
    797     bool isStyleAttributeValid() const { return getFlag(IsStyleAttributeValidFlag); }
    798     void setIsStyleAttributeValid(bool f) { setFlag(f, IsStyleAttributeValidFlag); }
    799     void setIsStyleAttributeValid() const { setFlag(IsStyleAttributeValidFlag); }
    800     void clearIsStyleAttributeValid() { clearFlag(IsStyleAttributeValidFlag); }
    801803
    802804#if ENABLE(SVG)

  • trunk/Source/WebCore/dom/StyledElement.cpp

    r117195 r117323  
    169169}
    170170
     171void StyledElement::styleAttributeChanged(const AtomicString& newStyleString, ShouldReparseStyleAttribute shouldReparse)
     172{
     173    if (shouldReparse) {
     174        if (newStyleString.isNull())
     175            destroyInlineStyle();
     176        else if (document()->contentSecurityPolicy()->allowInlineStyle())
     177            ensureAttributeData()->updateInlineStyleAvoidingMutation(this, newStyleString);
     178        setIsStyleAttributeValid();
     179    }
     180    setNeedsStyleRecalc();
     181    InspectorInstrumentation::didInvalidateStyleAttr(document(), this);
     182}
     183
    171184void StyledElement::parseAttribute(const Attribute& attribute)
    172185{
    173186    if (attribute.name() == classAttr)
    174187        classAttributeChanged(attribute.value());
    175     else if (attribute.name() == styleAttr) {
    176         if (attribute.isNull())
    177             destroyInlineStyle();
    178         else if (document()->contentSecurityPolicy()->allowInlineStyle())
    179             ensureAttributeData()->updateInlineStyleAvoidingMutation(this, attribute.value());
    180         setIsStyleAttributeValid();
    181         setNeedsStyleRecalc();
    182         InspectorInstrumentation::didInvalidateStyleAttr(document(), this);
    183     }
     188    else if (attribute.name() == styleAttr)
     189        styleAttributeChanged(attribute.value());
    184190}
    185191

  • trunk/Source/WebCore/dom/StyledElement.h

    r117195 r117323  
    5858    virtual void collectStyleForAttribute(const Attribute&, StylePropertySet*) { }
    5959
     60    // May be called by ElementAttributeData::cloneDataFrom().
     61    enum ShouldReparseStyleAttribute { DoNotReparseStyleAttribute = 0, ReparseStyleAttribute = 1 };
     62    void styleAttributeChanged(const AtomicString& newStyleString, ShouldReparseStyleAttribute = ReparseStyleAttribute);
     63
    6064protected:
    6165    StyledElement(const QualifiedName& name, Document* document, ConstructionType type)
    … …  
    6670    virtual void attributeChanged(const Attribute&) OVERRIDE;
    6771    virtual void parseAttribute(const Attribute&);
    68     virtual void copyNonAttributeProperties(const Element*);
    6972
    7073    virtual bool isPresentationAttribute(const QualifiedName&) const { return false; }

  • trunk/Source/WebCore/editing/ReplaceNodeWithSpanCommand.cpp

    r116717 r117323  
    6565
    6666    // FIXME: Fix this to send the proper MutationRecords when MutationObservers are present.
    67     newNode->setAttributesFromElement(*nodeToReplace);
     67    newNode->cloneDataFromElement(*nodeToReplace);
    6868
    6969    parentNode->removeChild(nodeToReplace, ec);

  • trunk/Source/WebCore/html/HTMLElement.cpp

    r117195 r117323  
    11091109}
    11101110
    1111 void StyledElement::copyNonAttributeProperties(const Element* sourceElement)
    1112 {
    1113     ASSERT(sourceElement);
    1114     ASSERT(sourceElement->isStyledElement());
    1115 
    1116     const StyledElement* source = static_cast<const StyledElement*>(sourceElement);
    1117     if (!source->inlineStyle())
    1118         return;
    1119 
    1120     StylePropertySet* inlineStyle = ensureAttributeData()->ensureMutableInlineStyle(this);
    1121     inlineStyle->copyPropertiesFrom(*source->inlineStyle());
    1122     inlineStyle->setCSSParserMode(source->inlineStyle()->cssParserMode());
    1123 
    1124     setIsStyleAttributeValid(source->isStyleAttributeValid());
    1125 
    1126     Element::copyNonAttributeProperties(sourceElement);
    1127 }
    1128 
    1129 
    11301111} // namespace WebCore
    11311112

  • trunk/Source/WebCore/html/HTMLInputElement.cpp

    r117263 r117323  
    826826}
    827827
    828 void HTMLInputElement::copyNonAttributeProperties(const Element* source)
    829 {
    830     const HTMLInputElement* sourceElement = static_cast<const HTMLInputElement*>(source);
    831 
    832     m_valueIfDirty = sourceElement->m_valueIfDirty;
     828void HTMLInputElement::copyNonAttributePropertiesFromElement(const Element& source)
     829{
     830    const HTMLInputElement& sourceElement = static_cast<const HTMLInputElement&>(source);
     831
     832    m_valueIfDirty = sourceElement.m_valueIfDirty;
    833833    m_wasModifiedByUser = false;
    834     setChecked(sourceElement->m_isChecked);
    835     m_reflectsCheckedAttribute = sourceElement->m_reflectsCheckedAttribute;
    836     m_isIndeterminate = sourceElement->m_isIndeterminate;
    837 
    838     HTMLTextFormControlElement::copyNonAttributeProperties(source);
     834    setChecked(sourceElement.m_isChecked);
     835    m_reflectsCheckedAttribute = sourceElement.m_reflectsCheckedAttribute;
     836    m_isIndeterminate = sourceElement.m_isIndeterminate;
     837
     838    HTMLTextFormControlElement::copyNonAttributePropertiesFromElement(source);
    839839
    840840    setFormControlValueMatchesRenderer(false);

  • trunk/Source/WebCore/html/HTMLInputElement.h

    r117263 r117323  
    291291    virtual void finishParsingChildren();
    292292
    293     virtual void copyNonAttributeProperties(const Element* source);
     293    virtual void copyNonAttributePropertiesFromElement(const Element&);
    294294
    295295    virtual void attach();

  • trunk/Source/WebCore/inspector/DOMPatchSupport.cpp

    r112555 r117323  
    181181        }
    182182
    183         // FIXME: Create a function in Element for copying properties. setAttributesFromElement() is close but not enough for this case.
     183        // FIXME: Create a function in Element for copying properties. cloneDataFromElement() is close but not enough for this case.
    184184        if (newElement->hasAttributesWithoutUpdate()) {
    185185            size_t numAttrs = newElement->attributeCount();

  • trunk/Source/WebCore/inspector/InspectorDOMAgent.cpp

    r116904 r117323  
    654654
    655655    // Copy over the original node's attributes.
    656     newElem->setAttributesFromElement(*toElement(oldNode));
     656    newElem->cloneAttributesFromElement(*toElement(oldNode));
    657657
    658658    // Copy over the original node's children.

  • trunk/Source/WebCore/svg/SVGUseElement.cpp

    r117242 r117323  
    767767        RefPtr<SVGSVGElement> svgElement = SVGSVGElement::create(SVGNames::svgTag, referencedDocument());
    768768
    769         // Transfer all attributes from <symbol> to the new <svg> element
    770         svgElement->setAttributesFromElement(*toElement(element));
     769        // Transfer all data (attributes, etc.) from <symbol> to the new <svg> element.
     770        svgElement->cloneDataFromElement(*toElement(element));
    771771
    772772        // Only clone symbol children, and add them to the new <svg> element
    … …  
    899899    ASSERT(to);
    900900
    901     to->setAttributesFromElement(*from);
     901    to->cloneDataFromElement(*from);
    902902
    903903    to->removeAttribute(SVGNames::xAttr);
Note: See TracChangeset for help on using the changeset viewer.