Reviewed by Ken.
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Nov 2004 01:04:13 +0000 (01:04 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Nov 2004 01:04:13 +0000 (01:04 +0000)
        - fixed <rdar://problem/3880036> Many leaks from CSSComputedStyleDeclarationImpl::getPropertyCSSValue, seen in Mail and Blot

        * khtml/css/css_computedstyle.cpp:
        (DOM::CSSComputedStyleDeclarationImpl::getPropertyValue): Ref and deref the value returned from getPropertyCSSValue,
        since there's no guarantee it's already ref'd.
        * khtml/css/css_valueimpl.cpp:
        (CSSStyleDeclarationImpl::getPropertyValue): Wrap result in a CSSValue to ref/deref.
        (CSSStyleDeclarationImpl::get4Values): Ref/deref explicitly.
        (CSSStyleDeclarationImpl::getShortHandValue): Ditto.
        (CSSStyleDeclarationImpl::merge): Ditto.
        (CSSStyleDeclarationImpl::diff): Ditto.
        * khtml/editing/htmlediting.cpp:
        (khtml::StyleChange::currentlyHasStyle): Ditto.
        (khtml::ApplyStyleCommand::removeCSSStyle): Ditto.
        * khtml/html/html_baseimpl.cpp: (HTMLBodyElementImpl::parseHTMLAttribute): Ditto.
        * khtml/html/html_tableimpl.cpp: (HTMLTableElementImpl::parseHTMLAttribute): Ditto.

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

WebCore/ChangeLog-2005-08-23
WebCore/khtml/css/css_computedstyle.cpp
WebCore/khtml/css/css_valueimpl.cpp
WebCore/khtml/editing/htmlediting.cpp
WebCore/khtml/html/html_baseimpl.cpp
WebCore/khtml/html/html_tableimpl.cpp

index d9cb092..5d6cce2 100644 (file)
@@ -2,6 +2,27 @@
 
         Reviewed by Ken.
 
+        - fixed <rdar://problem/3880036> Many leaks from CSSComputedStyleDeclarationImpl::getPropertyCSSValue, seen in Mail and Blot
+
+        * khtml/css/css_computedstyle.cpp:
+        (DOM::CSSComputedStyleDeclarationImpl::getPropertyValue): Ref and deref the value returned from getPropertyCSSValue,
+        since there's no guarantee it's already ref'd.
+        * khtml/css/css_valueimpl.cpp:
+        (CSSStyleDeclarationImpl::getPropertyValue): Wrap result in a CSSValue to ref/deref.
+        (CSSStyleDeclarationImpl::get4Values): Ref/deref explicitly.
+        (CSSStyleDeclarationImpl::getShortHandValue): Ditto.
+        (CSSStyleDeclarationImpl::merge): Ditto.
+        (CSSStyleDeclarationImpl::diff): Ditto.
+        * khtml/editing/htmlediting.cpp:
+        (khtml::StyleChange::currentlyHasStyle): Ditto.
+        (khtml::ApplyStyleCommand::removeCSSStyle): Ditto.
+        * khtml/html/html_baseimpl.cpp: (HTMLBodyElementImpl::parseHTMLAttribute): Ditto.
+        * khtml/html/html_tableimpl.cpp: (HTMLTableElementImpl::parseHTMLAttribute): Ditto.
+
+2004-11-15  Darin Adler  <darin@apple.com>
+
+        Reviewed by Ken.
+
         Use separate mutable style and computed style types as appropriate.
         For now this should have no effect, but it prepares us for refactoring later.
         Also remove some unnecessary "DOM::" prefixes and in one case factor out
index acf951f..c43aa5e 100644 (file)
@@ -861,8 +861,12 @@ CSSValueImpl *CSSComputedStyleDeclarationImpl::getPropertyCSSValue(int propertyI
 DOMString CSSComputedStyleDeclarationImpl::getPropertyValue(int propertyID) const
 {
     CSSValueImpl* value = getPropertyCSSValue(propertyID);
-    if (value)
-        return value->cssText();
+    if (value) {
+        value->ref();
+        DOMString result = value->cssText();
+        value->deref();
+        return result;
+    }
     return "";
 }
 
index 1a942bb..5361a66 100644 (file)
@@ -106,7 +106,7 @@ DOMString CSSStyleDeclarationImpl::getPropertyValue( int propertyID ) const
 
     CSSValueImpl* value = getPropertyCSSValue( propertyID );
     if ( value )
-        return value->cssText();
+        return CSSValue(value).cssText();
 
     // Shorthand and 4-values properties
     switch ( propertyID ) {
@@ -209,9 +209,11 @@ DOMString CSSStyleDeclarationImpl::get4Values( const int* properties ) const
         if ( !value ) { // apparently all 4 properties must be specified.
             return DOMString();
         }
+        value->ref();
         if ( i > 0 )
             res += " ";
         res += value->cssText();
+        value->deref();
     }
     return res;
 }
@@ -222,9 +224,11 @@ DOMString CSSStyleDeclarationImpl::getShortHandValue( const int* properties, int
     for ( int i = 0 ; i < number ; ++i ) {
         CSSValueImpl* value = getPropertyCSSValue( properties[i] );
         if ( value ) { // TODO provide default value if !value
+            value->ref();
             if ( !res.isNull() )
                 res += " ";
             res += value->cssText();
+            value->deref();
         }
     }
     return res;
@@ -421,7 +425,10 @@ void CSSStyleDeclarationImpl::merge(CSSStyleDeclarationImpl *other, bool argOver
 {
     for (QPtrListIterator<CSSProperty> it(*(other->values())); it.current(); ++it) {
         CSSProperty *property = it.current();
-        if (getPropertyCSSValue(property->id())) {
+        CSSValueImpl *value = getPropertyCSSValue(property->id());
+        if (value) {
+            value->ref();
+            value->deref();
             if (!argOverridesOnConflict)
                 continue;
             removeProperty(property->id());
@@ -443,8 +450,12 @@ void CSSStyleDeclarationImpl::diff(CSSStyleDeclarationImpl *style) const
     for (QPtrListIterator<CSSProperty> it(*style->values()); it.current(); ++it) {
         CSSProperty *property = it.current();
         CSSValueImpl *value = getPropertyCSSValue(property->id());
-        if (value && value->cssText() == property->value()->cssText()) {
-            properties.append(property->id());
+        if (value) {
+            value->ref();
+            if (value->cssText() == property->value()->cssText()) {
+                properties.append(property->id());
+            }
+            value->deref();
         }
     }
     
index bf00532..8b191d4 100644 (file)
@@ -381,7 +381,12 @@ bool StyleChange::currentlyHasStyle(const Position &pos, const CSSProperty *prop
     style->ref();
     CSSValueImpl *value = style->getPropertyCSSValue(property->id(), DoNotUpdateLayout);
     style->deref();
-    return value && strcasecmp(value->cssText(), property->value()->cssText()) == 0;
+    if (!value)
+        return false;
+    value->ref();
+    bool result = strcasecmp(value->cssText(), property->value()->cssText()) == 0;
+    value->deref();
+    return result;
 }
 
 //------------------------------------------------------------------------------------------
@@ -1136,8 +1141,12 @@ void ApplyStyleCommand::removeCSSStyle(CSSStyleDeclarationImpl *style, HTMLEleme
 
     for (QPtrListIterator<CSSProperty> it(*(style->values())); it.current(); ++it) {
         CSSProperty *property = it.current();
-        if (decl->getPropertyCSSValue(property->id()))
+        CSSValueImpl *value = decl->getPropertyCSSValue(property->id());
+        if (value) {
+            value->ref();
             removeCSSProperty(decl, property->id());
+            value->deref();
+        }
     }
 
     if (elem->id() == ID_SPAN && elem->renderer() && elem->renderer()->isInline()) {
index 65bd630..721686e 100644 (file)
@@ -146,14 +146,18 @@ void HTMLBodyElementImpl::parseHTMLAttribute(HTMLAttributeImpl *attr)
                 createLinkDecl();
             m_linkDecl->setProperty(CSS_PROP_COLOR, attr->value(), false, false);
             CSSValueImpl* val = m_linkDecl->getPropertyCSSValue(CSS_PROP_COLOR);
-            if (val && val->isPrimitiveValue()) {
-                QColor col = getDocument()->styleSelector()->getColorFromPrimitiveValue(static_cast<CSSPrimitiveValueImpl*>(val));
-                if (attr->id() == ATTR_LINK)
-                    getDocument()->setLinkColor(col);
-                else if (attr->id() == ATTR_VLINK)
-                    getDocument()->setVisitedLinkColor(col);
-                else
-                    getDocument()->setActiveLinkColor(col);
+            if (val) {
+                val->ref();
+                if (val->isPrimitiveValue()) {
+                    QColor col = getDocument()->styleSelector()->getColorFromPrimitiveValue(static_cast<CSSPrimitiveValueImpl*>(val));
+                    if (attr->id() == ATTR_LINK)
+                        getDocument()->setLinkColor(col);
+                    else if (attr->id() == ATTR_VLINK)
+                        getDocument()->setVisitedLinkColor(col);
+                    else
+                        getDocument()->setActiveLinkColor(col);
+                }
+                val->deref();
             }
         }
         
index 10b1526..af65769 100644 (file)
@@ -406,9 +406,13 @@ void HTMLTableElementImpl::parseHTMLAttribute(HTMLAttributeImpl *attr)
         if (attr->isNull()) break;
         if (attr->decl()) {
             CSSValueImpl* val = attr->decl()->getPropertyCSSValue(CSS_PROP_BORDER_LEFT_WIDTH);
-            if (val && val->isPrimitiveValue()) {
-                CSSPrimitiveValueImpl* primVal = static_cast<CSSPrimitiveValueImpl*>(val);
-                m_noBorder = !primVal->getFloatValue(CSSPrimitiveValue::CSS_NUMBER);
+            if (val) {
+                val->ref();
+                if (val->isPrimitiveValue()) {
+                    CSSPrimitiveValueImpl* primVal = static_cast<CSSPrimitiveValueImpl*>(val);
+                    m_noBorder = !primVal->getFloatValue(CSSPrimitiveValue::CSS_NUMBER);
+                }
+                val->deref();
             }
         }
         else {