Make CSSMappedAttributeDeclaration have CSSMutableStyleDeclaration instead of being one
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Feb 2012 17:41:27 +0000 (17:41 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Feb 2012 17:41:27 +0000 (17:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=77545

Reviewed by Andreas Kling.

This is the easiest path for eliminating the last remaining subclass of CSSMutableStyleDeclaration.

On negative side this increases memory use of CSSMappedAttributeDeclaration by one ptr and refcount
(it loses the vptr) in total.

This is not meant to be the end state, just an intermediate refactoring step. CSSMappedAttributeDeclaration
should clearly be renamed too but this patch doesn't do that. It might not exist in its current form
much longer.

* css/CSSMutableStyleDeclaration.cpp:
(WebCore::CSSMutableStyleDeclaration::setProperty):
(WebCore::CSSMutableStyleDeclaration::merge):
* css/CSSMutableStyleDeclaration.h:

    Remove protected section. No subclasses remain.
    Rename setPropertyInternal() to setProperty(). All public methods here are internal.

(CSSMutableStyleDeclaration):
* css/CSSParser.cpp:
(WebCore::CSSParser::parseMappedAttributeValue):
* dom/Attribute.cpp:
(WebCore::Attribute::clone):
* dom/Attribute.h:
(Attribute):
(WebCore::Attribute::decl):
(WebCore::Attribute::mappedAttributeDeclaration):
(WebCore::Attribute::setMappedAttributeDeclaration):
(WebCore::Attribute::Attribute):
* dom/CSSMappedAttributeDeclaration.cpp:
(WebCore::CSSMappedAttributeDeclaration::setMappedImageProperty):
(WebCore::CSSMappedAttributeDeclaration::setMappedProperty):
(WebCore::CSSMappedAttributeDeclaration::removeMappedProperty):
* dom/CSSMappedAttributeDeclaration.h:
(CSSMappedAttributeDeclaration):
(WebCore::CSSMappedAttributeDeclaration::declaration):
(WebCore::CSSMappedAttributeDeclaration::CSSMappedAttributeDeclaration):

    Make CSSMutableStyleDeclaration a member instead of the base class.

* dom/StyledElement.cpp:
(WebCore::StyledElement::attributeChanged):
(WebCore::StyledElement::removeCSSProperty):
(WebCore::StyledElement::addCSSProperty):
(WebCore::StyledElement::addCSSImageProperty):
(WebCore::StyledElement::addCSSLength):
(WebCore::StyledElement::addCSSColor):
(WebCore::StyledElement::createMappedDecl):
* svg/SVGStyledElement.cpp:
(WebCore::SVGStyledElement::getPresentationAttribute):

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

Source/WebCore/ChangeLog
Source/WebCore/css/CSSMutableStyleDeclaration.cpp
Source/WebCore/css/CSSMutableStyleDeclaration.h
Source/WebCore/css/CSSParser.cpp
Source/WebCore/dom/Attribute.cpp
Source/WebCore/dom/Attribute.h
Source/WebCore/dom/CSSMappedAttributeDeclaration.cpp
Source/WebCore/dom/CSSMappedAttributeDeclaration.h
Source/WebCore/dom/StyledElement.cpp
Source/WebCore/svg/SVGStyledElement.cpp

index 9c96945..413b663 100644 (file)
@@ -1,3 +1,60 @@
+2012-02-01  Antti Koivisto  <antti@apple.com>
+
+        Make CSSMappedAttributeDeclaration have CSSMutableStyleDeclaration instead of being one
+        https://bugs.webkit.org/show_bug.cgi?id=77545
+
+        Reviewed by Andreas Kling.
+
+        This is the easiest path for eliminating the last remaining subclass of CSSMutableStyleDeclaration.
+        
+        On negative side this increases memory use of CSSMappedAttributeDeclaration by one ptr and refcount
+        (it loses the vptr) in total.
+        
+        This is not meant to be the end state, just an intermediate refactoring step. CSSMappedAttributeDeclaration
+        should clearly be renamed too but this patch doesn't do that. It might not exist in its current form
+        much longer.
+
+        * css/CSSMutableStyleDeclaration.cpp:
+        (WebCore::CSSMutableStyleDeclaration::setProperty):
+        (WebCore::CSSMutableStyleDeclaration::merge):
+        * css/CSSMutableStyleDeclaration.h:
+
+            Remove protected section. No subclasses remain.        
+            Rename setPropertyInternal() to setProperty(). All public methods here are internal.
+        
+        (CSSMutableStyleDeclaration):
+        * css/CSSParser.cpp:
+        (WebCore::CSSParser::parseMappedAttributeValue):
+        * dom/Attribute.cpp:
+        (WebCore::Attribute::clone):
+        * dom/Attribute.h:
+        (Attribute):
+        (WebCore::Attribute::decl):
+        (WebCore::Attribute::mappedAttributeDeclaration):
+        (WebCore::Attribute::setMappedAttributeDeclaration):
+        (WebCore::Attribute::Attribute):
+        * dom/CSSMappedAttributeDeclaration.cpp:
+        (WebCore::CSSMappedAttributeDeclaration::setMappedImageProperty):
+        (WebCore::CSSMappedAttributeDeclaration::setMappedProperty):
+        (WebCore::CSSMappedAttributeDeclaration::removeMappedProperty):
+        * dom/CSSMappedAttributeDeclaration.h:
+        (CSSMappedAttributeDeclaration):
+        (WebCore::CSSMappedAttributeDeclaration::declaration):
+        (WebCore::CSSMappedAttributeDeclaration::CSSMappedAttributeDeclaration):
+        
+            Make CSSMutableStyleDeclaration a member instead of the base class.
+        
+        * dom/StyledElement.cpp:
+        (WebCore::StyledElement::attributeChanged):
+        (WebCore::StyledElement::removeCSSProperty):
+        (WebCore::StyledElement::addCSSProperty):
+        (WebCore::StyledElement::addCSSImageProperty):
+        (WebCore::StyledElement::addCSSLength):
+        (WebCore::StyledElement::addCSSColor):
+        (WebCore::StyledElement::createMappedDecl):
+        * svg/SVGStyledElement.cpp:
+        (WebCore::SVGStyledElement::getPresentationAttribute):
+
 2012-02-01  Allan Sandfeld Jensen  <allan.jensen@nokia.com>
 
         [Qt] Set all PlatformTouchPoint values possible from a QTouch event.
index c8d8bbb..1511014 100644 (file)
@@ -708,7 +708,7 @@ bool CSSMutableStyleDeclaration::setProperty(int propertyID, const String& value
     return true;
 }
 
-void CSSMutableStyleDeclaration::setPropertyInternal(const CSSProperty& property, CSSProperty* slot)
+void CSSMutableStyleDeclaration::setProperty(const CSSProperty& property, CSSProperty* slot)
 {
 #if ENABLE(MUTATION_OBSERVERS)
     StyleAttributeMutationScope mutationScope(this);
@@ -731,7 +731,7 @@ void CSSMutableStyleDeclaration::setPropertyInternal(const CSSProperty& property
 bool CSSMutableStyleDeclaration::setProperty(int propertyID, int value, bool important, bool notifyChanged)
 {
     CSSProperty property(propertyID, CSSPrimitiveValue::createIdentifier(value), important);
-    setPropertyInternal(property);
+    setProperty(property);
     if (notifyChanged)
         setNeedsStyleRecalc();
     return true;
@@ -740,7 +740,7 @@ bool CSSMutableStyleDeclaration::setProperty(int propertyID, int value, bool imp
 bool CSSMutableStyleDeclaration::setProperty(int propertyID, double value, CSSPrimitiveValue::UnitTypes unit, bool important, bool notifyChanged)
 {
     CSSProperty property(propertyID, CSSPrimitiveValue::create(value, unit), important);
-    setPropertyInternal(property);
+    setProperty(property);
     if (notifyChanged)
         setNeedsStyleRecalc();
     return true;
@@ -892,7 +892,7 @@ void CSSMutableStyleDeclaration::merge(const CSSMutableStyleDeclaration* other,
         if (old) {
             if (!argOverridesOnConflict && old->value())
                 continue;
-            setPropertyInternal(toMerge, old);
+            setProperty(toMerge, old);
         } else
             m_properties.append(toMerge);
     }
index 089de6c..b50d551 100644 (file)
@@ -73,8 +73,10 @@ public:
     bool setProperty(int propertyID, int value, bool important = false) { return setProperty(propertyID, value, important, true); }
     bool setProperty(int propertyId, double value, CSSPrimitiveValue::UnitTypes unit, bool important = false) { return setProperty(propertyId, value, unit, important, true); }
     bool setProperty(int propertyID, const String& value, bool important = false) { return setProperty(propertyID, value, important, true); }
-
+    void setProperty(const CSSProperty&, CSSProperty* slot = 0);
+    
     void removeProperty(int propertyID) { removeProperty(propertyID, true, false); }
+    String removeProperty(int propertyID, bool notifyChanged, bool returnText);
 
     // The following parses an entire new style declaration.
     void parseDeclaration(const String& styleDeclaration);
@@ -105,13 +107,8 @@ public:
 
     String asText() const;
 
-protected:
-    CSSMutableStyleDeclaration();
-
-    void setPropertyInternal(const CSSProperty&, CSSProperty* slot = 0);
-    String removeProperty(int propertyID, bool notifyChanged, bool returnText);
-
 private:
+    CSSMutableStyleDeclaration();
     CSSMutableStyleDeclaration(CSSRule* parentRule);
     CSSMutableStyleDeclaration(CSSRule* parentRule, const Vector<CSSProperty>&);
     CSSMutableStyleDeclaration(CSSRule* parentRule, const CSSProperty* const *, int numProperties);
index ce3be60..ac8d177 100644 (file)
@@ -484,18 +484,18 @@ static bool parseSimpleLengthValue(CSSMutableStyleDeclaration* declaration, int
     return true;
 }
 
-bool CSSParser::parseMappedAttributeValue(CSSMappedAttributeDeclaration* declaration, StyledElement* element, int propertyId, const String& value)
+bool CSSParser::parseMappedAttributeValue(CSSMappedAttributeDeclaration* mappedAttribute, StyledElement* element, int propertyId, const String& value)
 {
-    ASSERT(declaration);
+    ASSERT(mappedAttribute);
     ASSERT(element);
     ASSERT(element->document());
     CSSStyleSheet* elementSheet = element->document()->elementSheet();
-    if (parseSimpleLengthValue(declaration, propertyId, value, false, false, elementSheet))
+    if (parseSimpleLengthValue(mappedAttribute->declaration(), propertyId, value, false, false, elementSheet))
         return true;
-    if (parseColorValue(declaration, propertyId, value, false, false, elementSheet))
+    if (parseColorValue(mappedAttribute->declaration(), propertyId, value, false, false, elementSheet))
         return true;
     CSSParser parser(false);
-    return parser.parseValue(declaration, propertyId, value, false, elementSheet);
+    return parser.parseValue(mappedAttribute->declaration(), propertyId, value, false, elementSheet);
 }
 
 bool CSSParser::parseValue(CSSMutableStyleDeclaration* declaration, int propertyId, const String& string, bool important, bool strict)
index 20c05d8..7ed28d8 100644 (file)
@@ -40,7 +40,7 @@ static AttributeAttrMap& attributeAttrMap()
 
 PassRefPtr<Attribute> Attribute::clone() const
 {
-    return adoptRef(new Attribute(m_name, m_value, m_isMappedAttribute, m_styleDecl.get()));
+    return adoptRef(new Attribute(m_name, m_value, m_isMappedAttribute, m_mappedAttributeDeclaration.get()));
 }
 
 Attr* Attribute::attr() const
index 6c2b468..938e192 100644 (file)
@@ -68,9 +68,11 @@ public:
     bool isEmpty() const { return m_value.isEmpty(); }
     
     PassRefPtr<Attribute> clone() const;
+    
+    CSSMutableStyleDeclaration* decl() const { return m_mappedAttributeDeclaration ? m_mappedAttributeDeclaration->declaration() : 0; }
 
-    CSSMappedAttributeDeclaration* decl() const { return m_styleDecl.get(); }
-    void setDecl(PassRefPtr<CSSMappedAttributeDeclaration> decl) { m_styleDecl = decl; }
+    CSSMappedAttributeDeclaration* mappedAttributeDeclaration() const { return m_mappedAttributeDeclaration.get(); }
+    void setMappedAttributeDeclaration(PassRefPtr<CSSMappedAttributeDeclaration> decl) { m_mappedAttributeDeclaration = decl; }
 
     void setValue(const AtomicString& value) { m_value = value; }
     void setPrefix(const AtomicString& prefix) { m_name.setPrefix(prefix); }
@@ -88,7 +90,7 @@ private:
         , m_hasAttr(false)
         , m_name(name)
         , m_value(value)
-        , m_styleDecl(styleDecl)
+        , m_mappedAttributeDeclaration(styleDecl)
     {
     }
 
@@ -97,7 +99,7 @@ private:
         , m_hasAttr(false)
         , m_name(nullAtom, name, nullAtom)
         , m_value(value)
-        , m_styleDecl(styleDecl)
+        , m_mappedAttributeDeclaration(styleDecl)
     {
     }
 
@@ -110,7 +112,7 @@ private:
     
     QualifiedName m_name;
     AtomicString m_value;
-    RefPtr<CSSMappedAttributeDeclaration> m_styleDecl;
+    RefPtr<CSSMappedAttributeDeclaration> m_mappedAttributeDeclaration;
 };
 
 } // namespace WebCore
index 1585f80..72e256a 100644 (file)
@@ -43,7 +43,7 @@ CSSMappedAttributeDeclaration::~CSSMappedAttributeDeclaration()
 
 void CSSMappedAttributeDeclaration::setMappedImageProperty(StyledElement* element, int propertyId, const String& url)
 {
-    setPropertyInternal(CSSProperty(propertyId, CSSImageValue::create(url)));
+    m_declaration->setProperty(CSSProperty(propertyId, CSSImageValue::create(url)));
     setNeedsStyleRecalc(element);
 }
 
@@ -55,7 +55,7 @@ void CSSMappedAttributeDeclaration::setMappedLengthProperty(StyledElement* eleme
 void CSSMappedAttributeDeclaration::setMappedProperty(StyledElement* element, int propertyId, int value)
 {
     ASSERT(element->document());
-    setPropertyInternal(CSSProperty(propertyId, element->document()->cssValuePool()->createIdentifierValue(value)));
+    m_declaration->setProperty(CSSProperty(propertyId, element->document()->cssValuePool()->createIdentifierValue(value)));
     setNeedsStyleRecalc(element);
 }
 
@@ -74,7 +74,7 @@ void CSSMappedAttributeDeclaration::setMappedProperty(StyledElement* element, in
 
 void CSSMappedAttributeDeclaration::removeMappedProperty(StyledElement* element, int propertyId)
 {
-    removeProperty(propertyId, false, false);
+    m_declaration->removeProperty(propertyId, false, false);
     setNeedsStyleRecalc(element);
 }
 
index a5b7551..d9544ed 100644 (file)
@@ -33,14 +33,14 @@ namespace WebCore {
 
 class StyledElement;
 
-class CSSMappedAttributeDeclaration : public CSSMutableStyleDeclaration {
+class CSSMappedAttributeDeclaration : public RefCounted<CSSMappedAttributeDeclaration> {
 public:
     static PassRefPtr<CSSMappedAttributeDeclaration> create()
     {
         return adoptRef(new CSSMappedAttributeDeclaration);
     }
 
-    virtual ~CSSMappedAttributeDeclaration();
+    ~CSSMappedAttributeDeclaration();
 
     void setMappedState(MappedAttributeEntry type, const QualifiedName& name, const AtomicString& val)
     {
@@ -57,10 +57,12 @@ public:
     void setMappedLengthProperty(StyledElement*, int propertyId, const String& value);
 
     void removeMappedProperty(StyledElement*, int propertyId);
+    
+    CSSMutableStyleDeclaration* declaration() const { return m_declaration.get(); }
 
 private:
     CSSMappedAttributeDeclaration()
-        : CSSMutableStyleDeclaration()
+        : m_declaration(CSSMutableStyleDeclaration::create())
         , m_entryType(eNone)
         , m_attrName(anyQName())
     {
@@ -68,6 +70,7 @@ private:
 
     void setNeedsStyleRecalc(StyledElement*);
 
+    RefPtr<CSSMutableStyleDeclaration> m_declaration;
     MappedAttributeEntry m_entryType;
     QualifiedName m_attrName;
     AtomicString m_attrValue;
index 7e99217..25f2cd8 100644 (file)
@@ -137,8 +137,8 @@ void StyledElement::attributeChanged(Attribute* attr, bool preserveDecls)
         return;
     }
  
-    if (attr->decl() && !preserveDecls) {
-        attr->setDecl(0);
+    if (attr->mappedAttributeDeclaration() && !preserveDecls) {
+        attr->setMappedAttributeDeclaration(0);
         setNeedsStyleRecalc();
     }
 
@@ -146,14 +146,14 @@ void StyledElement::attributeChanged(Attribute* attr, bool preserveDecls)
     MappedAttributeEntry entry;
     bool needToParse = mapToEntry(attr->name(), entry);
     if (preserveDecls) {
-        if (attr->decl()) {
+        if (attr->mappedAttributeDeclaration()) {
             setNeedsStyleRecalc();
             checkDecl = false;
         }
     } else if (!attr->isNull() && entry != eNone) {
         CSSMappedAttributeDeclaration* decl = getMappedAttributeDecl(entry, attr);
         if (decl) {
-            attr->setDecl(decl);
+            attr->setMappedAttributeDeclaration(decl);
             setNeedsStyleRecalc();
             checkDecl = false;
         } else
@@ -171,10 +171,10 @@ void StyledElement::attributeChanged(Attribute* attr, bool preserveDecls)
     if (entry == eNone)
         recalcStyleIfNeededAfterAttributeChanged(attr);
 
-    if (checkDecl && attr->decl()) {
+    if (checkDecl && attr->mappedAttributeDeclaration()) {
         // Add the decl to the table in the appropriate spot.
-        setMappedAttributeDecl(entry, attr, attr->decl());
-        attr->decl()->setMappedState(entry, attr->name(), attr->value());
+        setMappedAttributeDecl(entry, attr, attr->mappedAttributeDeclaration());
+        attr->mappedAttributeDeclaration()->setMappedState(entry, attr->name(), attr->value());
     }
 
     updateAfterAttributeChanged(attr);
@@ -228,37 +228,37 @@ void StyledElement::parseMappedAttribute(Attribute* attr)
 
 void StyledElement::removeCSSProperty(Attribute* attribute, int id)
 {
-    if (!attribute->decl())
+    if (!attribute->mappedAttributeDeclaration())
         createMappedDecl(attribute);
-    attribute->decl()->removeMappedProperty(this, id);
+    attribute->mappedAttributeDeclaration()->removeMappedProperty(this, id);
 }
 
 void StyledElement::addCSSProperty(Attribute* attribute, int id, const String &value)
 {
-    if (!attribute->decl())
+    if (!attribute->mappedAttributeDeclaration())
         createMappedDecl(attribute);
-    attribute->decl()->setMappedProperty(this, id, value);
+    attribute->mappedAttributeDeclaration()->setMappedProperty(this, id, value);
 }
 
 void StyledElement::addCSSProperty(Attribute* attribute, int id, int value)
 {
-    if (!attribute->decl())
+    if (!attribute->mappedAttributeDeclaration())
         createMappedDecl(attribute);
-    attribute->decl()->setMappedProperty(this, id, value);
+    attribute->mappedAttributeDeclaration()->setMappedProperty(this, id, value);
 }
 
 void StyledElement::addCSSImageProperty(Attribute* attribute, int id, const String& url)
 {
-    if (!attribute->decl())
+    if (!attribute->mappedAttributeDeclaration())
         createMappedDecl(attribute);
-    attribute->decl()->setMappedImageProperty(this, id, url);
+    attribute->mappedAttributeDeclaration()->setMappedImageProperty(this, id, url);
 }
 
 void StyledElement::addCSSLength(Attribute* attribute, int id, const String &value)
 {
     // FIXME: This function should not spin up the CSS parser, but should instead just figure out the correct
     // length unit and make the appropriate parsed value.
-    if (!attribute->decl())
+    if (!attribute->mappedAttributeDeclaration())
         createMappedDecl(attribute);
 
     // strip attribute garbage..
@@ -282,12 +282,12 @@ void StyledElement::addCSSLength(Attribute* attribute, int id, const String &val
         }
 
         if (l != v->length()) {
-            attribute->decl()->setMappedLengthProperty(this, id, v->substring(0, l));
+            attribute->mappedAttributeDeclaration()->setMappedLengthProperty(this, id, v->substring(0, l));
             return;
         }
     }
 
-    attribute->decl()->setMappedLengthProperty(this, id, value);
+    attribute->mappedAttributeDeclaration()->setMappedLengthProperty(this, id, value);
 }
 
 static String parseColorStringWithCrazyLegacyRules(const String& colorString)
@@ -355,24 +355,24 @@ void StyledElement::addCSSColor(Attribute* attribute, int id, const String& attr
     if (equalIgnoringCase(colorString, "transparent"))
         return;
 
-    if (!attribute->decl())
+    if (!attribute->mappedAttributeDeclaration())
         createMappedDecl(attribute);
 
     // If the string is a named CSS color or a 3/6-digit hex color, use that.
     Color parsedColor(colorString);
     if (parsedColor.isValid()) {
-        attribute->decl()->setMappedProperty(this, id, colorString);
+        attribute->mappedAttributeDeclaration()->setMappedProperty(this, id, colorString);
         return;
     }
 
-    attribute->decl()->setMappedProperty(this, id, parseColorStringWithCrazyLegacyRules(colorString));
+    attribute->mappedAttributeDeclaration()->setMappedProperty(this, id, parseColorStringWithCrazyLegacyRules(colorString));
 }
 
 void StyledElement::createMappedDecl(Attribute* attr)
 {
     RefPtr<CSSMappedAttributeDeclaration> decl = CSSMappedAttributeDeclaration::create();
-    attr->setDecl(decl);
-    ASSERT(!decl->useStrictParsing());
+    attr->setMappedAttributeDeclaration(decl);
+    ASSERT(!decl->declaration()->useStrictParsing());
 }
 
 unsigned MappedAttributeHash::hash(const MappedAttributeKey& key)
index 2e87d3a..62669c8 100644 (file)
@@ -422,7 +422,7 @@ PassRefPtr<CSSValue> SVGStyledElement::getPresentationAttribute(const String& na
 
     QualifiedName attributeName(nullAtom, name, nullAtom);
     Attribute* attr = attributeMap()->getAttributeItem(attributeName);
-    if (!attr || !attr->isMappedAttribute() || !attr->decl())
+    if (!attr || !attr->isMappedAttribute() || !attr->mappedAttributeDeclaration())
         return 0;
 
     Attribute* cssSVGAttr = attr;
@@ -432,8 +432,8 @@ PassRefPtr<CSSValue> SVGStyledElement::getPresentationAttribute(const String& na
     // before returning so that any modifications to the CSSValue will not affect other attributes.
     MappedAttributeEntry entry;
     mapToEntry(attributeName, entry);
-    if (getMappedAttributeDecl(entry, cssSVGAttr) == cssSVGAttr->decl()) {
-        cssSVGAttr->setDecl(0);
+    if (getMappedAttributeDecl(entry, cssSVGAttr) == cssSVGAttr->mappedAttributeDeclaration()) {
+        cssSVGAttr->setMappedAttributeDeclaration(0);
         int propId = SVGStyledElement::cssPropertyIdForSVGAttributeName(cssSVGAttr->name());
         addCSSProperty(cssSVGAttr, propId, cssSVGAttr->value());
     }