[webkit-changes] cvs commit: WebCore/khtml/editing apply_style_command.cpp apply_style_command.h

Darin darin at opensource.apple.com
Fri Sep 2 09:45:39 PDT 2005


darin       05/09/02 09:45:38

  Modified:    .        ChangeLog
               khtml/editing apply_style_command.cpp apply_style_command.h
  Log:
          Reviewed by Maciej.
  
          - fixed http://bugzilla.opendarwin.org/show_bug.cgi?id=4757
            leaks found by code inspection in apply-style command
  
          * khtml/editing/apply_style_command.h: Change m_style to be a SharedPtr.
          * khtml/editing/apply_style_command.cpp:
          (khtml::StyleChange::init): Added use of SharedPtr.
          (khtml::StyleChange::currentlyHasStyle): Ditto.
          (khtml::ApplyStyleCommand::ApplyStyleCommand): Ditto.
          (khtml::ApplyStyleCommand::~ApplyStyleCommand): Ditto.
          (khtml::ApplyStyleCommand::doApply): Ditto.
          (khtml::ApplyStyleCommand::applyRelativeFontStyleChange): Ditto.
          (khtml::ApplyStyleCommand::removeCSSStyle): Ditto.
          (khtml::hasTextDecorationProperty): Ditto.
          (khtml::ApplyStyleCommand::extractTextDecorationStyle): Ditto.
          (khtml::ApplyStyleCommand::extractAndNegateTextDecorationStyle): Ditto.
          (khtml::ApplyStyleCommand::pushDownTextDecorationStyleAroundNode): Ditto.
          (khtml::ApplyStyleCommand::removeInlineStyle): Ditto.
          (khtml::ApplyStyleCommand::addInlineStyleIfNeeded): Ditto.
          (khtml::ApplyStyleCommand::computedFontSize): Ditto.
  
  Revision  Changes    Path
  1.77      +24 -0     WebCore/ChangeLog
  
  Index: ChangeLog
  ===================================================================
  RCS file: /cvs/root/WebCore/ChangeLog,v
  retrieving revision 1.76
  retrieving revision 1.77
  diff -u -r1.76 -r1.77
  --- ChangeLog	2 Sep 2005 11:15:41 -0000	1.76
  +++ ChangeLog	2 Sep 2005 16:45:37 -0000	1.77
  @@ -1,3 +1,27 @@
  +2005-09-02  Darin Adler  <darin at apple.com>
  +
  +        Reviewed by Maciej.
  +
  +        - fixed http://bugzilla.opendarwin.org/show_bug.cgi?id=4757
  +          leaks found by code inspection in apply-style command
  +
  +        * khtml/editing/apply_style_command.h: Change m_style to be a SharedPtr.
  +        * khtml/editing/apply_style_command.cpp:
  +        (khtml::StyleChange::init): Added use of SharedPtr.
  +        (khtml::StyleChange::currentlyHasStyle): Ditto.
  +        (khtml::ApplyStyleCommand::ApplyStyleCommand): Ditto.
  +        (khtml::ApplyStyleCommand::~ApplyStyleCommand): Ditto.
  +        (khtml::ApplyStyleCommand::doApply): Ditto.
  +        (khtml::ApplyStyleCommand::applyRelativeFontStyleChange): Ditto.
  +        (khtml::ApplyStyleCommand::removeCSSStyle): Ditto.
  +        (khtml::hasTextDecorationProperty): Ditto.
  +        (khtml::ApplyStyleCommand::extractTextDecorationStyle): Ditto.
  +        (khtml::ApplyStyleCommand::extractAndNegateTextDecorationStyle): Ditto.
  +        (khtml::ApplyStyleCommand::pushDownTextDecorationStyleAroundNode): Ditto.
  +        (khtml::ApplyStyleCommand::removeInlineStyle): Ditto.
  +        (khtml::ApplyStyleCommand::addInlineStyleIfNeeded): Ditto.
  +        (khtml::ApplyStyleCommand::computedFontSize): Ditto.
  +
   2005-09-02  Eric Seidel  <eseidel at apple.com>
   
           Reviewed by mjs (continuation of previous commit).
  
  
  
  1.12      +44 -104   WebCore/khtml/editing/apply_style_command.cpp
  
  Index: apply_style_command.cpp
  ===================================================================
  RCS file: /cvs/root/WebCore/khtml/editing/apply_style_command.cpp,v
  retrieving revision 1.11
  retrieving revision 1.12
  diff -u -r1.11 -r1.12
  --- apply_style_command.cpp	25 Aug 2005 23:13:43 -0000	1.11
  +++ apply_style_command.cpp	2 Sep 2005 16:45:38 -0000	1.12
  @@ -123,8 +123,7 @@
   void StyleChange::init(CSSStyleDeclarationImpl *style, const Position &position)
   {
       style->ref();
  -    CSSMutableStyleDeclarationImpl *mutableStyle = style->makeMutable();
  -    mutableStyle->ref();
  +    SharedPtr<CSSMutableStyleDeclarationImpl> mutableStyle = style->makeMutable();
       style->deref();
       
       QString styleText("");
  @@ -153,8 +152,6 @@
           }
       }
   
  -    mutableStyle->deref();
  -
       // Save the result for later
       m_cssStyle = styleText.stripWhiteSpace();
   }
  @@ -226,17 +223,11 @@
   bool StyleChange::currentlyHasStyle(const Position &pos, const CSSProperty *property)
   {
       ASSERT(pos.isNotNull());
  -    CSSComputedStyleDeclarationImpl *style = pos.computedStyle();
  -    ASSERT(style);
  -    style->ref();
  -    CSSValueImpl *value = style->getPropertyCSSValue(property->id(), DoNotUpdateLayout);
  -    style->deref();
  +    SharedPtr<CSSComputedStyleDeclarationImpl> style = pos.computedStyle();
  +    SharedPtr<CSSValueImpl> value = style->getPropertyCSSValue(property->id(), DoNotUpdateLayout);
       if (!value)
           return false;
  -    value->ref();
  -    bool result = strcasecmp(value->cssText(), property->value()->cssText()) == 0;
  -    value->deref();
  -    return result;
  +    return strcasecmp(value->cssText(), property->value()->cssText()) == 0;
   }
   
   static DOMString &styleSpanClassString()
  @@ -295,14 +286,11 @@
   ApplyStyleCommand::ApplyStyleCommand(DocumentImpl *document, CSSStyleDeclarationImpl *style, EditAction editingAction, EPropertyLevel propertyLevel)
       : CompositeEditCommand(document), m_style(style->makeMutable()), m_editingAction(editingAction), m_propertyLevel(propertyLevel)
   {   
  -    ASSERT(m_style);
  -    m_style->ref();
  +    ASSERT(style);
   }
   
   ApplyStyleCommand::~ApplyStyleCommand()
   {
  -    ASSERT(m_style);
  -    m_style->deref();
   }
   
   void ApplyStyleCommand::doApply()
  @@ -310,25 +298,21 @@
       switch (m_propertyLevel) {
           case PropertyDefault: {
               // apply the block-centric properties of the style
  -            CSSMutableStyleDeclarationImpl *blockStyle = m_style->copyBlockProperties();
  -            blockStyle->ref();
  -            applyBlockStyle(blockStyle);
  +            SharedPtr<CSSMutableStyleDeclarationImpl> blockStyle = m_style->copyBlockProperties();
  +            applyBlockStyle(blockStyle.get());
               // apply any remaining styles to the inline elements
               // NOTE: hopefully, this string comparison is the same as checking for a non-null diff
               if (blockStyle->length() < m_style->length()) {
  -                CSSMutableStyleDeclarationImpl *inlineStyle = m_style->copy();
  -                inlineStyle->ref();
  -                applyRelativeFontStyleChange(inlineStyle);
  -                blockStyle->diff(inlineStyle);
  -                applyInlineStyle(inlineStyle);
  -                inlineStyle->deref();
  +                SharedPtr<CSSMutableStyleDeclarationImpl> inlineStyle = m_style->copy();
  +                applyRelativeFontStyleChange(inlineStyle.get());
  +                blockStyle->diff(inlineStyle.get());
  +                applyInlineStyle(inlineStyle.get());
               }
  -            blockStyle->deref();
               break;
           }
           case ForceBlockProperties:
               // Force all properties to be applied as block styles.
  -            applyBlockStyle(m_style);
  +            applyBlockStyle(m_style.get());
               break;
       }
      
  @@ -389,20 +373,20 @@
   
   void ApplyStyleCommand::applyRelativeFontStyleChange(CSSMutableStyleDeclarationImpl *style)
   {
  -    if (style->getPropertyCSSValue(CSS_PROP_FONT_SIZE)) {
  +    SharedPtr<CSSValueImpl> value = style->getPropertyCSSValue(CSS_PROP_FONT_SIZE);
  +    if (value) {
           // Explicit font size overrides any delta.
           style->removeProperty(CSS_PROP__KHTML_FONT_SIZE_DELTA);
           return;
       }
   
       // Get the adjustment amount out of the style.
  -    CSSValueImpl *value = style->getPropertyCSSValue(CSS_PROP__KHTML_FONT_SIZE_DELTA);
  +    value = style->getPropertyCSSValue(CSS_PROP__KHTML_FONT_SIZE_DELTA);
       if (!value)
           return;
  -    value->ref();
       float adjustment = NoFontDelta;
       if (value->cssValueType() == CSSValue::CSS_PRIMITIVE_VALUE) {
  -        CSSPrimitiveValueImpl *primitiveValue = static_cast<CSSPrimitiveValueImpl *>(value);
  +        CSSPrimitiveValueImpl *primitiveValue = static_cast<CSSPrimitiveValueImpl *>(value.get());
           if (primitiveValue->primitiveType() == CSSPrimitiveValue::CSS_PX) {
               // Only PX handled now. If we handle more types in the future, perhaps
               // a switch statement here would be more appropriate.
  @@ -410,7 +394,6 @@
           }
       }
       style->removeProperty(CSS_PROP__KHTML_FONT_SIZE_DELTA);
  -    value->deref();
       if (adjustment == NoFontDelta)
           return;
       
  @@ -490,7 +473,8 @@
           CSSMutableStyleDeclarationImpl *inlineStyleDecl = elem->getInlineStyleDecl();
           float currentFontSize = computedFontSize(node);
           float desiredFontSize = kMax(MinimumFontSize, startingFontSizes[node] + adjustment);
  -        if (inlineStyleDecl->getPropertyCSSValue(CSS_PROP_FONT_SIZE)) {
  +        SharedPtr<CSSValueImpl> value = inlineStyleDecl->getPropertyCSSValue(CSS_PROP_FONT_SIZE);
  +        if (value) {
               inlineStyleDecl->removeProperty(CSS_PROP_FONT_SIZE, true);
               currentFontSize = computedFontSize(node);
           }
  @@ -678,14 +662,9 @@
       QValueListConstIterator<CSSProperty> end;
       for (QValueListConstIterator<CSSProperty> it = style->valuesIterator(); it != end; ++it) {
           int propertyID = (*it).id();
  -        CSSValueImpl *value = decl->getPropertyCSSValue(propertyID);
  -        if (value) {
  -            if (propertyID == CSS_PROP_WHITE_SPACE && isTabSpanNode(elem))
  -                continue;
  -            value->ref();
  +        SharedPtr<CSSValueImpl> value = decl->getPropertyCSSValue(propertyID);
  +        if (value && (propertyID != CSS_PROP_WHITE_SPACE || !isTabSpanNode(elem)))
               removeCSSProperty(decl, propertyID);
  -            value->deref();
  -        }
       }
   
       if (isEmptyStyleSpan(elem))
  @@ -709,18 +688,8 @@
   
       ElementImpl *element = static_cast<ElementImpl *>(node);
       CSSComputedStyleDeclarationImpl style(element);
  -
  -    CSSValueImpl *value = style.getPropertyCSSValue(CSS_PROP_TEXT_DECORATION, DoNotUpdateLayout);
  -
  -    if (value) {
  -        value->ref();
  -        DOMString valueText(value->cssText());
  -        value->deref();
  -        if (strcasecmp(valueText,"none") != 0)
  -            return true;
  -    }
  -
  -    return false;
  +    SharedPtr<CSSValueImpl> value = style.getPropertyCSSValue(CSS_PROP_TEXT_DECORATION, DoNotUpdateLayout);
  +    return value && strcasecmp(value->cssText(), "none") != 0;
   }
   
   static NodeImpl* highestAncestorWithTextDecoration(NodeImpl *node)
  @@ -745,20 +714,16 @@
           return 0;
   
       HTMLElementImpl *element = static_cast<HTMLElementImpl *>(node);
  -    CSSMutableStyleDeclarationImpl *style = element->inlineStyleDecl();
  +    SharedPtr<CSSMutableStyleDeclarationImpl> style = element->inlineStyleDecl();
       if (!style)
           return 0;
   
  -    style->ref();
       int properties[1] = { CSS_PROP_TEXT_DECORATION };
       CSSMutableStyleDeclarationImpl *textDecorationStyle = style->copyPropertiesInSet(properties, 1);
   
  -    CSSValueImpl *property = style->getPropertyCSSValue(CSS_PROP_TEXT_DECORATION);
  -    if (property && strcasecmp(property->cssText(), "none") != 0) {
  -        removeCSSProperty(style, CSS_PROP_TEXT_DECORATION);
  -    }
  -
  -    style->deref();
  +    SharedPtr<CSSValueImpl> property = style->getPropertyCSSValue(CSS_PROP_TEXT_DECORATION);
  +    if (property && strcasecmp(property->cssText(), "none") != 0)
  +        removeCSSProperty(style.get(), CSS_PROP_TEXT_DECORATION);
   
       return textDecorationStyle;
   }
  @@ -773,30 +738,19 @@
           return 0;
   
       HTMLElementImpl *element = static_cast<HTMLElementImpl *>(node);
  -    CSSComputedStyleDeclarationImpl *computedStyle = new CSSComputedStyleDeclarationImpl(element);
  +    SharedPtr<CSSComputedStyleDeclarationImpl> computedStyle = new CSSComputedStyleDeclarationImpl(element);
       ASSERT(computedStyle);
   
  -    computedStyle->ref();
  -
       int properties[1] = { CSS_PROP_TEXT_DECORATION };
       CSSMutableStyleDeclarationImpl *textDecorationStyle = computedStyle->copyPropertiesInSet(properties, 1);
  -    
   
  -    CSSValueImpl *property = computedStyle->getPropertyCSSValue(CSS_PROP_TEXT_DECORATION);
  +    SharedPtr<CSSValueImpl> property = computedStyle->getPropertyCSSValue(CSS_PROP_TEXT_DECORATION);
       if (property && strcasecmp(property->cssText(), "none") != 0) {
  -        property->ref();
  -        CSSMutableStyleDeclarationImpl *newStyle = textDecorationStyle->copy();
  -
  -        newStyle->ref();
  +        SharedPtr<CSSMutableStyleDeclarationImpl> newStyle = textDecorationStyle->copy();
           newStyle->setProperty(CSS_PROP_TEXT_DECORATION, "none");
  -        applyTextDecorationStyle(node, newStyle);
  -        newStyle->deref();
  -
  -        property->deref();
  +        applyTextDecorationStyle(node, newStyle.get());
       }
   
  -    computedStyle->deref();
  -
       return textDecorationStyle;
   }
   
  @@ -841,9 +795,7 @@
               
               nextCurrent = NULL;
               
  -            CSSMutableStyleDeclarationImpl *decoration = force ? extractAndNegateTextDecorationStyle(current) : extractTextDecorationStyle(current);
  -            if (decoration)
  -                decoration->ref();
  +            SharedPtr<CSSMutableStyleDeclarationImpl> decoration = force ? extractAndNegateTextDecorationStyle(current) : extractTextDecorationStyle(current);
   
               for (NodeImpl *child = current->firstChild(); child; child = nextChild) {
                   nextChild = child->nextSibling();
  @@ -851,15 +803,12 @@
                   if (node == child) {
                       nextCurrent = child;
                   } else if (node->isAncestor(child)) {
  -                    applyTextDecorationStyle(child, decoration);
  +                    applyTextDecorationStyle(child, decoration.get());
                       nextCurrent = child;
                   } else {
  -                    applyTextDecorationStyle(child, decoration);
  +                    applyTextDecorationStyle(child, decoration.get());
                   }
               }
  -
  -            if (decoration)
  -                decoration->deref();
           }
       }
   }
  @@ -899,7 +848,7 @@
       ASSERT(end.node()->inDocument());
       ASSERT(RangeImpl::compareBoundaryPoints(start, end) < 0);
       
  -    CSSValueImpl *textDecorationSpecialProperty = style->getPropertyCSSValue(CSS_PROP__KHTML_TEXT_DECORATIONS_IN_EFFECT);
  +    SharedPtr<CSSValueImpl> textDecorationSpecialProperty = style->getPropertyCSSValue(CSS_PROP__KHTML_TEXT_DECORATIONS_IN_EFFECT);
   
       if (textDecorationSpecialProperty) {
           pushDownTextDecorationStyleAtBoundaries(start.downstream(), end.upstream());
  @@ -1273,12 +1222,10 @@
       }
   
       if (styleChange.cssStyle().length() > 0) {
  -        ElementImpl *styleElement = createStyleSpanElement(document());
  -        styleElement->ref();
  +        SharedPtr<ElementImpl> styleElement = createStyleSpanElement(document());
           styleElement->setAttribute(styleAttr, styleChange.cssStyle());
  -        insertNodeBefore(styleElement, startNode);
  -        styleElement->deref();
  -        surroundNodeRangeWithElement(startNode, endNode, styleElement);
  +        insertNodeBefore(styleElement.get(), startNode);
  +        surroundNodeRangeWithElement(startNode, endNode, styleElement.get());
       }
   
       if (styleChange.applyBold()) {
  @@ -1298,26 +1245,19 @@
   
   float ApplyStyleCommand::computedFontSize(const NodeImpl *node)
   {
  -    float size = 0.0f;
  -    
       if (!node)
  -        return size;
  +        return 0;
       
       Position pos(const_cast<NodeImpl *>(node), 0);
  -    CSSComputedStyleDeclarationImpl *computedStyle = pos.computedStyle();
  +    SharedPtr<CSSComputedStyleDeclarationImpl> computedStyle = pos.computedStyle();
       if (!computedStyle)
  -        return size;
  -    computedStyle->ref();
  +        return 0;
   
  -    CSSPrimitiveValueImpl *value = static_cast<CSSPrimitiveValueImpl *>(computedStyle->getPropertyCSSValue(CSS_PROP_FONT_SIZE));
  -    if (value) {
  -        value->ref();
  -        size = value->getFloatValue(CSSPrimitiveValue::CSS_PX);
  -        value->deref();
  -    }
  +    SharedPtr<CSSPrimitiveValueImpl> value = static_cast<CSSPrimitiveValueImpl *>(computedStyle->getPropertyCSSValue(CSS_PROP_FONT_SIZE));
  +    if (!value)
  +        return 0;
   
  -    computedStyle->deref();
  -    return size;
  +    return value->getFloatValue(CSSPrimitiveValue::CSS_PX);
   }
   
   void ApplyStyleCommand::joinChildTextNodes(NodeImpl *node, const Position &start, const Position &end)
  
  
  
  1.2       +2 -2      WebCore/khtml/editing/apply_style_command.h
  
  Index: apply_style_command.h
  ===================================================================
  RCS file: /cvs/root/WebCore/khtml/editing/apply_style_command.h,v
  retrieving revision 1.1
  retrieving revision 1.2
  diff -u -r1.1 -r1.2
  --- apply_style_command.h	12 May 2005 04:00:19 -0000	1.1
  +++ apply_style_command.h	2 Sep 2005 16:45:38 -0000	1.2
  @@ -45,7 +45,7 @@
       virtual void doApply();
       virtual EditAction editingAction() const;
   
  -    DOM::CSSMutableStyleDeclarationImpl *style() const { return m_style; }
  +    DOM::CSSMutableStyleDeclarationImpl *style() const { return m_style.get(); }
   
   private:
       // style-removal helpers
  @@ -81,7 +81,7 @@
       float computedFontSize(const DOM::NodeImpl *);
       void joinChildTextNodes(DOM::NodeImpl *, const DOM::Position &start, const DOM::Position &end);
       
  -    DOM::CSSMutableStyleDeclarationImpl *m_style;
  +    SharedPtr<DOM::CSSMutableStyleDeclarationImpl> m_style;
       EditAction m_editingAction;
       EPropertyLevel m_propertyLevel;
   };
  
  
  



More information about the webkit-changes mailing list