Separate mutating CSSStyleDeclaration operations.
authorkling@webkit.org <kling@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Jun 2012 15:21:02 +0000 (15:21 +0000)
committerkling@webkit.org <kling@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Jun 2012 15:21:02 +0000 (15:21 +0000)
<http://webkit.org/b/89945>

Reviewed by Antti Koivisto.

Use separate paths for mutating the StylePropertySet wrapped by a CSSStyleDeclaration.
PropertySetCSSStyleDeclaration now has:

    - propertySet() const
    - ensureMutablePropertySet()

This is prep work for supporting immutable ElementAttributeData objects, the idea being
that calling ensureMutablePropertySet() may cause the element to convert its internal
attribute storage (which also holds the inline StylePropertySet.)

To that end, also removed the weird logic that allowed you to kill the inline style object
by removing the 'style' attribute. We now simply clear out all the properties in that case
which saves us a bunch of hassle (no need for a ~StyledElement anymore.)
Note that InlineCSSStyleDeclaration now refs the element rather than the inline style.

There should be no web-facing behavior change from any of this.

* css/PropertySetCSSStyleDeclaration.cpp:
(WebCore::PropertySetCSSStyleDeclaration::length):
(WebCore::PropertySetCSSStyleDeclaration::item):
(WebCore::PropertySetCSSStyleDeclaration::cssText):
(WebCore::PropertySetCSSStyleDeclaration::setCssText):
(WebCore::PropertySetCSSStyleDeclaration::getPropertyCSSValue):
(WebCore::PropertySetCSSStyleDeclaration::getPropertyValue):
(WebCore::PropertySetCSSStyleDeclaration::getPropertyPriority):
(WebCore::PropertySetCSSStyleDeclaration::getPropertyShorthand):
(WebCore::PropertySetCSSStyleDeclaration::isPropertyImplicit):
(WebCore::PropertySetCSSStyleDeclaration::setProperty):
(WebCore::PropertySetCSSStyleDeclaration::removeProperty):
(WebCore::PropertySetCSSStyleDeclaration::getPropertyCSSValueInternal):
(WebCore::PropertySetCSSStyleDeclaration::getPropertyValueInternal):
(WebCore::PropertySetCSSStyleDeclaration::setPropertyInternal):
(WebCore::PropertySetCSSStyleDeclaration::copy):
(WebCore::PropertySetCSSStyleDeclaration::makeMutable):
(WebCore::PropertySetCSSStyleDeclaration::cssPropertyMatches):
(WebCore::InlineCSSStyleDeclaration::InlineCSSStyleDeclaration):
(WebCore::InlineCSSStyleDeclaration::ref):
(WebCore::InlineCSSStyleDeclaration::deref):
(WebCore::InlineCSSStyleDeclaration::didMutate):
(WebCore::InlineCSSStyleDeclaration::parentStyleSheet):
(WebCore::InlineCSSStyleDeclaration::ensureMutablePropertySet):
* css/PropertySetCSSStyleDeclaration.h:
(PropertySetCSSStyleDeclaration):
(WebCore::PropertySetCSSStyleDeclaration::propertySet):
(WebCore::PropertySetCSSStyleDeclaration::ensureMutablePropertySet):
(InlineCSSStyleDeclaration):
* css/StylePropertySet.cpp:
(WebCore::StylePropertySet::ensureInlineCSSStyleDeclaration):
* css/StylePropertySet.h:
(StylePropertySet):
* dom/ElementAttributeData.cpp:
* dom/ElementAttributeData.h:
(ElementAttributeData):
* dom/StyledElement.cpp:
(WebCore::StyledElement::styleAttributeChanged):
* dom/StyledElement.h:
(WebCore::StyledElement::~StyledElement):
(StyledElement):

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

Source/WebCore/ChangeLog
Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp
Source/WebCore/css/PropertySetCSSStyleDeclaration.h
Source/WebCore/css/StylePropertySet.cpp
Source/WebCore/css/StylePropertySet.h
Source/WebCore/dom/ElementAttributeData.cpp
Source/WebCore/dom/ElementAttributeData.h
Source/WebCore/dom/StyledElement.cpp
Source/WebCore/dom/StyledElement.h

index ca5c6c6..1215c2c 100644 (file)
@@ -1,3 +1,69 @@
+2012-06-29  Andreas Kling  <kling@webkit.org>
+
+        Separate mutating CSSStyleDeclaration operations.
+        <http://webkit.org/b/89945>
+
+        Reviewed by Antti Koivisto.
+
+        Use separate paths for mutating the StylePropertySet wrapped by a CSSStyleDeclaration.
+        PropertySetCSSStyleDeclaration now has:
+
+            - propertySet() const
+            - ensureMutablePropertySet()
+
+        This is prep work for supporting immutable ElementAttributeData objects, the idea being
+        that calling ensureMutablePropertySet() may cause the element to convert its internal
+        attribute storage (which also holds the inline StylePropertySet.)
+
+        To that end, also removed the weird logic that allowed you to kill the inline style object
+        by removing the 'style' attribute. We now simply clear out all the properties in that case
+        which saves us a bunch of hassle (no need for a ~StyledElement anymore.)
+        Note that InlineCSSStyleDeclaration now refs the element rather than the inline style.
+
+        There should be no web-facing behavior change from any of this.
+
+        * css/PropertySetCSSStyleDeclaration.cpp:
+        (WebCore::PropertySetCSSStyleDeclaration::length):
+        (WebCore::PropertySetCSSStyleDeclaration::item):
+        (WebCore::PropertySetCSSStyleDeclaration::cssText):
+        (WebCore::PropertySetCSSStyleDeclaration::setCssText):
+        (WebCore::PropertySetCSSStyleDeclaration::getPropertyCSSValue):
+        (WebCore::PropertySetCSSStyleDeclaration::getPropertyValue):
+        (WebCore::PropertySetCSSStyleDeclaration::getPropertyPriority):
+        (WebCore::PropertySetCSSStyleDeclaration::getPropertyShorthand):
+        (WebCore::PropertySetCSSStyleDeclaration::isPropertyImplicit):
+        (WebCore::PropertySetCSSStyleDeclaration::setProperty):
+        (WebCore::PropertySetCSSStyleDeclaration::removeProperty):
+        (WebCore::PropertySetCSSStyleDeclaration::getPropertyCSSValueInternal):
+        (WebCore::PropertySetCSSStyleDeclaration::getPropertyValueInternal):
+        (WebCore::PropertySetCSSStyleDeclaration::setPropertyInternal):
+        (WebCore::PropertySetCSSStyleDeclaration::copy):
+        (WebCore::PropertySetCSSStyleDeclaration::makeMutable):
+        (WebCore::PropertySetCSSStyleDeclaration::cssPropertyMatches):
+        (WebCore::InlineCSSStyleDeclaration::InlineCSSStyleDeclaration):
+        (WebCore::InlineCSSStyleDeclaration::ref):
+        (WebCore::InlineCSSStyleDeclaration::deref):
+        (WebCore::InlineCSSStyleDeclaration::didMutate):
+        (WebCore::InlineCSSStyleDeclaration::parentStyleSheet):
+        (WebCore::InlineCSSStyleDeclaration::ensureMutablePropertySet):
+        * css/PropertySetCSSStyleDeclaration.h:
+        (PropertySetCSSStyleDeclaration):
+        (WebCore::PropertySetCSSStyleDeclaration::propertySet):
+        (WebCore::PropertySetCSSStyleDeclaration::ensureMutablePropertySet):
+        (InlineCSSStyleDeclaration):
+        * css/StylePropertySet.cpp:
+        (WebCore::StylePropertySet::ensureInlineCSSStyleDeclaration):
+        * css/StylePropertySet.h:
+        (StylePropertySet):
+        * dom/ElementAttributeData.cpp:
+        * dom/ElementAttributeData.h:
+        (ElementAttributeData):
+        * dom/StyledElement.cpp:
+        (WebCore::StyledElement::styleAttributeChanged):
+        * dom/StyledElement.h:
+        (WebCore::StyledElement::~StyledElement):
+        (StyledElement):
+
 2012-06-29  Kwang Yul Seo  <skyul@company100.net>
 
         Don't call SegmentedString::toString() twice in XMLDocumentParser::append(const SegmentedString&)
index 2ad4d20..f7695ab 100644 (file)
@@ -132,19 +132,19 @@ void PropertySetCSSStyleDeclaration::deref()
 
 unsigned PropertySetCSSStyleDeclaration::length() const
 {
-    return m_propertySet->propertyCount();
+    return propertySet()->propertyCount();
 }
 
 String PropertySetCSSStyleDeclaration::item(unsigned i) const
 {
-    if (i >= m_propertySet->propertyCount())
+    if (i >= propertySet()->propertyCount())
         return "";
-    return getPropertyName(m_propertySet->propertyAt(i).id());
+    return getPropertyName(propertySet()->propertyAt(i).id());
 }
 
 String PropertySetCSSStyleDeclaration::cssText() const
 {
-    return m_propertySet->asText();
+    return propertySet()->asText();
 }
     
 void PropertySetCSSStyleDeclaration::setCssText(const String& text, ExceptionCode& ec)
@@ -156,7 +156,7 @@ void PropertySetCSSStyleDeclaration::setCssText(const String& text, ExceptionCod
 
     ec = 0;
     // FIXME: Detect syntax errors and set ec.
-    m_propertySet->parseDeclaration(text, contextStyleSheet());
+    ensureMutablePropertySet()->parseDeclaration(text, contextStyleSheet());
 
     didMutate(PropertyChanged);
 
@@ -170,7 +170,7 @@ PassRefPtr<CSSValue> PropertySetCSSStyleDeclaration::getPropertyCSSValue(const S
     CSSPropertyID propertyID = cssPropertyID(propertyName);
     if (!propertyID)
         return 0;
-    return cloneAndCacheForCSSOM(m_propertySet->getPropertyCSSValue(propertyID).get());
+    return cloneAndCacheForCSSOM(propertySet()->getPropertyCSSValue(propertyID).get());
 }
 
 String PropertySetCSSStyleDeclaration::getPropertyValue(const String &propertyName)
@@ -178,7 +178,7 @@ String PropertySetCSSStyleDeclaration::getPropertyValue(const String &propertyNa
     CSSPropertyID propertyID = cssPropertyID(propertyName);
     if (!propertyID)
         return String();
-    return m_propertySet->getPropertyValue(propertyID);
+    return propertySet()->getPropertyValue(propertyID);
 }
 
 String PropertySetCSSStyleDeclaration::getPropertyPriority(const String& propertyName)
@@ -186,7 +186,7 @@ String PropertySetCSSStyleDeclaration::getPropertyPriority(const String& propert
     CSSPropertyID propertyID = cssPropertyID(propertyName);
     if (!propertyID)
         return String();
-    return m_propertySet->propertyIsImportant(propertyID) ? "important" : "";
+    return propertySet()->propertyIsImportant(propertyID) ? "important" : "";
 }
 
 String PropertySetCSSStyleDeclaration::getPropertyShorthand(const String& propertyName)
@@ -194,7 +194,7 @@ String PropertySetCSSStyleDeclaration::getPropertyShorthand(const String& proper
     CSSPropertyID propertyID = cssPropertyID(propertyName);
     if (!propertyID)
         return String();
-    CSSPropertyID shorthandID = m_propertySet->getPropertyShorthand(propertyID);
+    CSSPropertyID shorthandID = propertySet()->getPropertyShorthand(propertyID);
     if (!shorthandID)
         return String();
     return getPropertyName(shorthandID);
@@ -205,7 +205,7 @@ bool PropertySetCSSStyleDeclaration::isPropertyImplicit(const String& propertyNa
     CSSPropertyID propertyID = cssPropertyID(propertyName);
     if (!propertyID)
         return false;
-    return m_propertySet->isPropertyImplicit(propertyID);
+    return propertySet()->isPropertyImplicit(propertyID);
 }
 
 void PropertySetCSSStyleDeclaration::setProperty(const String& propertyName, const String& value, const String& priority, ExceptionCode& ec)
@@ -222,7 +222,7 @@ void PropertySetCSSStyleDeclaration::setProperty(const String& propertyName, con
     willMutate();
 
     ec = 0;
-    bool changed = m_propertySet->setProperty(propertyID, value, important, contextStyleSheet());
+    bool changed = ensureMutablePropertySet()->setProperty(propertyID, value, important, contextStyleSheet());
 
     didMutate(changed ? PropertyChanged : NoChanges);
 
@@ -248,7 +248,7 @@ String PropertySetCSSStyleDeclaration::removeProperty(const String& propertyName
 
     ec = 0;
     String result;
-    bool changed = m_propertySet->removeProperty(propertyID, &result);
+    bool changed = ensureMutablePropertySet()->removeProperty(propertyID, &result);
 
     didMutate(changed ? PropertyChanged : NoChanges);
 
@@ -262,12 +262,12 @@ String PropertySetCSSStyleDeclaration::removeProperty(const String& propertyName
 
 PassRefPtr<CSSValue> PropertySetCSSStyleDeclaration::getPropertyCSSValueInternal(CSSPropertyID propertyID)
 {
-    return m_propertySet->getPropertyCSSValue(propertyID);
+    return propertySet()->getPropertyCSSValue(propertyID);
 }
 
 String PropertySetCSSStyleDeclaration::getPropertyValueInternal(CSSPropertyID propertyID)
 { 
-    return m_propertySet->getPropertyValue(propertyID); 
+    return propertySet()->getPropertyValue(propertyID);
 }
 
 void PropertySetCSSStyleDeclaration::setPropertyInternal(CSSPropertyID propertyID, const String& value, bool important, ExceptionCode& ec)
@@ -278,7 +278,7 @@ void PropertySetCSSStyleDeclaration::setPropertyInternal(CSSPropertyID propertyI
     willMutate();
 
     ec = 0;
-    bool changed = m_propertySet->setProperty(propertyID, value, important, contextStyleSheet());
+    bool changed = ensureMutablePropertySet()->setProperty(propertyID, value, important, contextStyleSheet());
 
     didMutate(changed ? PropertyChanged : NoChanges);
 
@@ -313,17 +313,17 @@ StyleSheetContents* PropertySetCSSStyleDeclaration::contextStyleSheet() const
 
 PassRefPtr<StylePropertySet> PropertySetCSSStyleDeclaration::copy() const
 {
-    return m_propertySet->copy();
+    return propertySet()->copy();
 }
 
 PassRefPtr<StylePropertySet> PropertySetCSSStyleDeclaration::makeMutable()
 {
-    return m_propertySet;
+    return ensureMutablePropertySet();
 }
 
 bool PropertySetCSSStyleDeclaration::cssPropertyMatches(const CSSProperty* property) const
 {
-    return m_propertySet->propertyMatches(property);
+    return propertySet()->propertyMatches(property);
 }
     
 StyleRuleCSSStyleDeclaration::StyleRuleCSSStyleDeclaration(StylePropertySet* propertySet, CSSRule* parentRule)
@@ -380,6 +380,22 @@ void StyleRuleCSSStyleDeclaration::reattach(StylePropertySet* propertySet)
     m_propertySet->ref();
 }
 
+InlineCSSStyleDeclaration::InlineCSSStyleDeclaration(StyledElement* parentElement)
+    : PropertySetCSSStyleDeclaration(const_cast<StylePropertySet*>(parentElement->inlineStyle()))
+    , m_parentElement(parentElement)
+{
+}
+
+void InlineCSSStyleDeclaration::ref()
+{
+    m_parentElement->ref();
+}
+
+void InlineCSSStyleDeclaration::deref()
+{
+    m_parentElement->deref();
+}
+
 void InlineCSSStyleDeclaration::didMutate(MutationType type)
 {
     if (type == NoChanges)
@@ -387,8 +403,6 @@ void InlineCSSStyleDeclaration::didMutate(MutationType type)
 
     m_cssomCSSValueClones.clear();
 
-    if (!m_parentElement)
-        return;
     m_parentElement->setNeedsStyleRecalc(InlineStyleChange);
     m_parentElement->invalidateStyleAttribute();
     StyleAttributeMutationScope(this).didInvalidateStyleAttr();
@@ -396,7 +410,14 @@ void InlineCSSStyleDeclaration::didMutate(MutationType type)
 
 CSSStyleSheet* InlineCSSStyleDeclaration::parentStyleSheet() const
 {
-    return m_parentElement ? m_parentElement->document()->elementSheet() : 0;
+    return m_parentElement->document()->elementSheet();
+}
+
+StylePropertySet* InlineCSSStyleDeclaration::ensureMutablePropertySet()
+{
+    ASSERT(m_propertySet);
+    ASSERT(m_propertySet == m_parentElement->inlineStyle());
+    return m_propertySet;
 }
 
 } // namespace WebCore
index caf63bd..fb9e09d 100644 (file)
@@ -42,12 +42,15 @@ public:
     PropertySetCSSStyleDeclaration(StylePropertySet* propertySet) : m_propertySet(propertySet) { }
     
     virtual StyledElement* parentElement() const { return 0; }
-    virtual void clearParentElement() { ASSERT_NOT_REACHED(); }
     StyleSheetContents* contextStyleSheet() const;
     
     virtual void ref() OVERRIDE;
     virtual void deref() OVERRIDE;
 
+protected:
+    const StylePropertySet* propertySet() const { return m_propertySet; }
+    virtual StylePropertySet* ensureMutablePropertySet() { return m_propertySet; }
+
 private:
     virtual CSSRule* parentRule() const OVERRIDE { return 0; };
     virtual unsigned length() const OVERRIDE;
@@ -106,6 +109,8 @@ private:
     virtual void willMutate() OVERRIDE;
     virtual void didMutate(MutationType) OVERRIDE;
 
+    virtual StylePropertySet* ensureMutablePropertySet() OVERRIDE { return m_propertySet; }
+
     unsigned m_refCount;
     CSSRule* m_parentRule;
 };
@@ -113,18 +118,18 @@ private:
 class InlineCSSStyleDeclaration : public PropertySetCSSStyleDeclaration
 {
 public:
-    InlineCSSStyleDeclaration(StylePropertySet* propertySet, StyledElement* parentElement)
-        : PropertySetCSSStyleDeclaration(propertySet)
-        , m_parentElement(parentElement) 
-    {
-    }
+    InlineCSSStyleDeclaration(StyledElement*);
+
+    virtual void ref() OVERRIDE;
+    virtual void deref() OVERRIDE;
     
 private:
     virtual CSSStyleSheet* parentStyleSheet() const OVERRIDE;
     virtual StyledElement* parentElement() const OVERRIDE { return m_parentElement; }
-    virtual void clearParentElement() OVERRIDE { m_parentElement = 0; }
 
     virtual void didMutate(MutationType) OVERRIDE;
+
+    virtual StylePropertySet* ensureMutablePropertySet() OVERRIDE;
     
     StyledElement* m_parentElement;
 };
index 7fc836f..2cbeccb 100644 (file)
@@ -1051,19 +1051,11 @@ CSSStyleDeclaration* StylePropertySet::ensureInlineCSSStyleDeclaration(const Sty
         return propertySetCSSOMWrapperMap().get(this);
     }
     m_ownsCSSOMWrapper = true;
-    PropertySetCSSStyleDeclaration* cssomWrapper = new InlineCSSStyleDeclaration(const_cast<StylePropertySet*>(this), const_cast<StyledElement*>(parentElement));
+    PropertySetCSSStyleDeclaration* cssomWrapper = new InlineCSSStyleDeclaration(const_cast<StyledElement*>(parentElement));
     propertySetCSSOMWrapperMap().add(this, adoptPtr(cssomWrapper));
     return cssomWrapper;
 }
 
-void StylePropertySet::clearParentElement(StyledElement* element)
-{
-    if (!m_ownsCSSOMWrapper)
-        return;
-    ASSERT_UNUSED(element, propertySetCSSOMWrapperMap().get(this)->parentElement() == element);
-    propertySetCSSOMWrapperMap().get(this)->clearParentElement();
-}
-
 unsigned StylePropertySet::averageSizeInBytes()
 {
     // Please update this if the storage scheme changes so that this longer reflects the actual size.
index 559cfe6..4d69c18 100644 (file)
@@ -103,8 +103,6 @@ public:
     
     String asText() const;
     
-    void clearParentElement(StyledElement*);
-
     CSSStyleDeclaration* ensureCSSStyleDeclaration() const;
     CSSStyleDeclaration* ensureInlineCSSStyleDeclaration(const StyledElement* parentElement) const;
 
index ab2768e..961dcdb 100644 (file)
@@ -161,14 +161,6 @@ void ElementAttributeData::updateInlineStyleAvoidingMutation(StyledElement* elem
     m_inlineStyleDecl->parseDeclaration(text, element->document()->elementSheet()->contents());
 }
 
-void ElementAttributeData::destroyInlineStyle(StyledElement* element)
-{
-    if (!m_inlineStyleDecl)
-        return;
-    m_inlineStyleDecl->clearParentElement(element);
-    m_inlineStyleDecl = 0;
-}
-
 void ElementAttributeData::addAttribute(const Attribute& attribute, Element* element, EInUpdateStyleAttribute inUpdateStyleAttribute)
 {
     if (element && inUpdateStyleAttribute == NotInUpdateStyleAttribute)
index a13d4d2..ea7915c 100644 (file)
@@ -67,7 +67,6 @@ public:
     StylePropertySet* ensureInlineStyle(StyledElement*);
     StylePropertySet* ensureMutableInlineStyle(StyledElement*);
     void updateInlineStyleAvoidingMutation(StyledElement*, const String& text);
-    void destroyInlineStyle(StyledElement*);
 
     StylePropertySet* attributeStyle() const { return m_attributeStyle.get(); }
     void setAttributeStyle(PassRefPtr<StylePropertySet> style) { m_attributeStyle = style; }
index a673351..f98348a 100644 (file)
@@ -135,11 +135,6 @@ StyledElement::StyledElement(const QualifiedName& name, Document* document, Cons
 {
 }
 
-StyledElement::~StyledElement()
-{
-    destroyInlineStyle();
-}
-
 CSSStyleDeclaration* StyledElement::style()
 {
     return ensureAttributeData()->ensureMutableInlineStyle(this)->ensureInlineCSSStyleDeclaration(this); 
@@ -183,9 +178,7 @@ void StyledElement::styleAttributeChanged(const AtomicString& newStyleString, Sh
         WTF::OrdinalNumber startLineNumber = WTF::OrdinalNumber::beforeFirst();
         if (document() && document()->scriptableDocumentParser() && !document()->isInDocumentWrite())
             startLineNumber = document()->scriptableDocumentParser()->lineNumber();
-        if (newStyleString.isNull())
-            destroyInlineStyle();
-        else if (document()->contentSecurityPolicy()->allowInlineStyle(document()->url(), startLineNumber))
+        if (document()->contentSecurityPolicy()->allowInlineStyle(document()->url(), startLineNumber))
             ensureAttributeData()->updateInlineStyleAvoidingMutation(this, newStyleString);
         setIsStyleAttributeValid();
     }
index abd49ad..21d65eb 100644 (file)
@@ -35,7 +35,7 @@ struct PresentationAttributeCacheKey;
 
 class StyledElement : public Element {
 public:
-    virtual ~StyledElement();
+    virtual ~StyledElement() { }
 
     virtual StylePropertySet* additionalAttributeStyle() { return 0; }
     void invalidateStyleAttribute();
@@ -86,12 +86,6 @@ private:
 
     void makePresentationAttributeCacheKey(PresentationAttributeCacheKey&) const;
     void updateAttributeStyle();
-
-    void destroyInlineStyle()
-    {
-        if (attributeData())
-            attributeData()->destroyInlineStyle(this);
-    }
 };
 
 inline const SpaceSplitString& StyledElement::classNames() const