[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: branches/KDE/3.5/kdelibs/khtml
From: Maks Orlovich <maksim () kde ! org>
Date: 2006-09-12 0:03:29
Message-ID: 1158019409.713028.18036.nullmailer () svn ! kde ! org
[Download RAW message or body]
SVN commit 583314 by orlovich:
Fix a number of leaks involving use of removeChild (and replaceChild). Essentially,
operations like deleteTHead, deleteTableCell etc., could frequently leak nodes
as removeChild would never drop refcount to 0 (just setParent). This changes it to do \
so; and just the users that need node kept alive to do so. Note tha this also means \
these ops no longer return the old node, as it's not guaranteed to be alive...
Prerequisite for making some latter work sane...
M +6 -4 dom/dom_node.cpp
M +4 -6 html/html_elementimpl.cpp
M +4 -6 html/html_formimpl.cpp
M +2 -2 html/html_formimpl.h
M +5 -8 html/html_tableimpl.cpp
M +2 -2 html/html_tableimpl.h
M +2 -0 html/htmlparser.cpp
M +22 -17 xml/dom_nodeimpl.cpp
M +6 -4 xml/dom_nodeimpl.h
--- branches/KDE/3.5/kdelibs/khtml/dom/dom_node.cpp #583313:583314
@@ -270,21 +270,23 @@
{
if (!impl) throw DOMException(DOMException::NOT_FOUND_ERR);
int exceptioncode = 0;
- NodeImpl *r = impl->replaceChild( newChild.impl, oldChild.impl, exceptioncode );
+ impl->replaceChild( newChild.impl, oldChild.impl, exceptioncode );
if (exceptioncode)
throw DOMException(exceptioncode);
if (newChild.impl && !newChild.impl->closed()) newChild.impl->close();
- return r;
+
+ return oldChild;
}
Node Node::removeChild( const Node &oldChild )
{
if (!impl) throw DOMException(DOMException::NOT_FOUND_ERR);
int exceptioncode = 0;
- NodeImpl *r = impl->removeChild( oldChild.impl, exceptioncode );
+ impl->removeChild( oldChild.impl, exceptioncode );
if (exceptioncode)
throw DOMException(exceptioncode);
- return r;
+
+ return oldChild;
}
Node Node::appendChild( const Node &newChild )
--- branches/KDE/3.5/kdelibs/khtml/html/html_elementimpl.cpp #583313:583314
@@ -530,28 +530,26 @@
// we need to pop <html> and <body> elements and remove <head> to
// accomadate folks passing complete HTML documents to make the
// child of an element.
- Node n;
-
for ( NodeImpl* node = fragment->firstChild(); node; ) {
if (node->id() == ID_HTML || node->id() == ID_BODY) {
NodeImpl* firstChild = node->firstChild();
NodeImpl* child = firstChild;
while ( child ) {
NodeImpl *nextChild = child->nextSibling();
- n = fragment->insertBefore(child, node, ignoredExceptionCode);
+ fragment->insertBefore(child, node, ignoredExceptionCode);
child = nextChild;
}
if ( !firstChild ) {
NodeImpl *nextNode = node->nextSibling();
- n = fragment->removeChild(node, ignoredExceptionCode);
+ fragment->removeChild(node, ignoredExceptionCode);
node = nextNode;
} else {
- n = fragment->removeChild(node, ignoredExceptionCode);
+ fragment->removeChild(node, ignoredExceptionCode);
node = firstChild;
}
} else if (node->id() == ID_HEAD) {
NodeImpl *nextNode = node->nextSibling();
- n = fragment->removeChild(node, ignoredExceptionCode);
+ fragment->removeChild(node, ignoredExceptionCode);
node = nextNode;
} else {
node = node->nextSibling();
--- branches/KDE/3.5/kdelibs/khtml/html/html_formimpl.cpp #583313:583314
@@ -2211,20 +2211,18 @@
return result;
}
-NodeImpl *HTMLSelectElementImpl::replaceChild ( NodeImpl *newChild, NodeImpl \
*oldChild, int &exceptioncode ) +void HTMLSelectElementImpl::replaceChild ( NodeImpl \
*newChild, NodeImpl *oldChild, int &exceptioncode ) {
- NodeImpl* const result = \
HTMLGenericFormElementImpl::replaceChild(newChild,oldChild, exceptioncode); + \
HTMLGenericFormElementImpl::replaceChild(newChild,oldChild, exceptioncode); if( \
!exceptioncode ) setRecalcListItems();
- return result;
}
-NodeImpl *HTMLSelectElementImpl::removeChild ( NodeImpl *oldChild, int \
&exceptioncode ) +void HTMLSelectElementImpl::removeChild ( NodeImpl *oldChild, int \
&exceptioncode ) {
- NodeImpl* const result = HTMLGenericFormElementImpl::removeChild(oldChild, \
exceptioncode); + HTMLGenericFormElementImpl::removeChild(oldChild, \
exceptioncode); if( !exceptioncode )
setRecalcListItems();
- return result;
}
NodeImpl *HTMLSelectElementImpl::appendChild ( NodeImpl *newChild, int \
&exceptioncode )
--- branches/KDE/3.5/kdelibs/khtml/html/html_formimpl.h #583313:583314
@@ -413,8 +413,8 @@
virtual void restoreState(const QString &);
virtual NodeImpl *insertBefore ( NodeImpl *newChild, NodeImpl *refChild, int \
&exceptioncode );
- virtual NodeImpl *replaceChild ( NodeImpl *newChild, NodeImpl *oldChild, int \
&exceptioncode );
- virtual NodeImpl *removeChild ( NodeImpl *oldChild, int &exceptioncode );
+ virtual void replaceChild ( NodeImpl *newChild, NodeImpl *oldChild, int \
&exceptioncode ); + virtual void removeChild ( NodeImpl *oldChild, int \
&exceptioncode );
virtual NodeImpl *appendChild ( NodeImpl *newChild, int &exceptioncode );
virtual NodeImpl *addChild( NodeImpl* newChild );
--- branches/KDE/3.5/kdelibs/khtml/html/html_tableimpl.cpp #583313:583314
@@ -389,21 +389,18 @@
return retval;
}
-NodeImpl *HTMLTableElementImpl::replaceChild ( NodeImpl *newChild, NodeImpl \
*oldChild, int &exceptioncode ) +void HTMLTableElementImpl::replaceChild ( NodeImpl \
*newChild, NodeImpl *oldChild, int &exceptioncode ) {
handleChildRemove( oldChild ); //Always safe.
-
- NodeImpl* retval = HTMLElementImpl::replaceChild( newChild, oldChild, \
exceptioncode );
- if (retval)
+ HTMLElementImpl::replaceChild( newChild, oldChild, exceptioncode );
+ if ( !exceptioncode )
handleChildAdd( newChild );
-
- return retval;
}
-NodeImpl *HTMLTableElementImpl::removeChild ( NodeImpl *oldChild, int &exceptioncode \
) +void HTMLTableElementImpl::removeChild ( NodeImpl *oldChild, int &exceptioncode )
{
handleChildRemove( oldChild );
- return HTMLElementImpl::removeChild( oldChild, exceptioncode);
+ HTMLElementImpl::removeChild( oldChild, exceptioncode);
}
void HTMLTableElementImpl::parseAttribute(AttributeImpl *attr)
--- branches/KDE/3.5/kdelibs/khtml/html/html_tableimpl.h #583313:583314
@@ -277,8 +277,8 @@
// overrides
virtual NodeImpl *addChild(NodeImpl *child);
virtual NodeImpl *insertBefore ( NodeImpl *newChild, NodeImpl *refChild, int \
&exceptioncode );
- virtual NodeImpl *replaceChild ( NodeImpl *newChild, NodeImpl *oldChild, int \
&exceptioncode );
- virtual NodeImpl *removeChild ( NodeImpl *oldChild, int &exceptioncode );
+ virtual void replaceChild ( NodeImpl *newChild, NodeImpl *oldChild, int \
&exceptioncode ); + virtual void removeChild ( NodeImpl *oldChild, int \
&exceptioncode );
virtual NodeImpl *appendChild ( NodeImpl *newChild, int &exceptioncode );
virtual void parseAttribute(AttributeImpl *attr);
--- branches/KDE/3.5/kdelibs/khtml/html/htmlparser.cpp #583313:583314
@@ -1408,6 +1408,7 @@
// The end result will be: <b>...</b><p><b>Foo</b>Goo</p>
//
// Step 1: Remove |blockElem| from its parent, doing a batch detach of all the \
kids. + SharedPtr<NodeImpl> guard(blockElem);
blockElem->parentNode()->removeChild(blockElem, exceptionCode);
// Step 2: Clone |residualElem|.
@@ -1419,6 +1420,7 @@
NodeImpl* currNode = blockElem->firstChild();
while (currNode) {
NodeImpl* nextNode = currNode->nextSibling();
+ SharedPtr<NodeImpl> guard(currNode); //Protect from deletion while moving
blockElem->removeChild(currNode, exceptionCode);
newNode->appendChild(currNode, exceptionCode);
currNode = nextNode;
--- branches/KDE/3.5/kdelibs/khtml/xml/dom_nodeimpl.cpp #583313:583314
@@ -137,16 +137,14 @@
return 0;
}
-NodeImpl *NodeImpl::replaceChild( NodeImpl *, NodeImpl *, int &exceptioncode )
+void NodeImpl::replaceChild( NodeImpl *, NodeImpl *, int &exceptioncode )
{
exceptioncode = DOMException::HIERARCHY_REQUEST_ERR;
- return 0;
}
-NodeImpl *NodeImpl::removeChild( NodeImpl *, int &exceptioncode )
+void NodeImpl::removeChild( NodeImpl *, int &exceptioncode )
{
exceptioncode = DOMException::NOT_FOUND_ERR;
- return 0;
}
NodeImpl *NodeImpl::appendChild( NodeImpl *, int &exceptioncode )
@@ -1052,6 +1050,10 @@
// If child is already present in the tree, first remove it
NodeImpl *newParent = child->parentNode();
+
+ //...guard it in case we need to move it..
+ SharedPtr<NodeImpl> guard(child);
+
if(newParent)
newParent->removeChild( child, exceptioncode );
if ( exceptioncode )
@@ -1086,22 +1088,22 @@
return newChild;
}
-NodeImpl *NodeBaseImpl::replaceChild ( NodeImpl *newChild, NodeImpl *oldChild, int \
&exceptioncode ) +void NodeBaseImpl::replaceChild ( NodeImpl *newChild, NodeImpl \
*oldChild, int &exceptioncode ) {
exceptioncode = 0;
if ( oldChild == newChild ) // nothing to do
- return oldChild;
+ return;
// Make sure adding the new child is ok
checkAddChild(newChild, exceptioncode);
if (exceptioncode)
- return 0;
+ return;
// NOT_FOUND_ERR: Raised if oldChild is not a child of this node.
if (!oldChild || oldChild->parentNode() != this) {
exceptioncode = DOMException::NOT_FOUND_ERR;
- return 0;
+ return;
}
bool isFragment = newChild->nodeType() == Node::DOCUMENT_FRAGMENT_NODE;
@@ -1115,7 +1117,7 @@
removeChild(oldChild, exceptioncode);
if (exceptioncode)
- return 0;
+ return;
// Add the new child(ren)
while (child) {
@@ -1127,10 +1129,12 @@
next = child->nextSibling();
if ( child == prev )
prev = child->previousSibling();
+ //...guard it in case we need to move it..
+ SharedPtr<NodeImpl> guard(child);
if(newParent)
newParent->removeChild( child, exceptioncode );
if (exceptioncode)
- return 0;
+ return;
// Add child in the correct position
if (prev) prev->setNextSibling(child);
@@ -1157,29 +1161,31 @@
// ### set style in case it's attached
dispatchSubtreeModifiedEvent();
- return oldChild;
+ return;
}
-NodeImpl *NodeBaseImpl::removeChild ( NodeImpl *oldChild, int &exceptioncode )
+void NodeBaseImpl::removeChild ( NodeImpl *oldChild, int &exceptioncode )
{
exceptioncode = 0;
// NO_MODIFICATION_ALLOWED_ERR: Raised if this node is readonly.
if (isReadOnly()) {
exceptioncode = DOMException::NO_MODIFICATION_ALLOWED_ERR;
- return 0;
+ return;
}
// NOT_FOUND_ERR: Raised if oldChild is not a child of this node.
if (!oldChild || oldChild->parentNode() != this) {
exceptioncode = DOMException::NOT_FOUND_ERR;
- return 0;
+ return;
}
dispatchChildRemovalEvents(oldChild,exceptioncode);
if (exceptioncode)
- return 0;
+ return;
+ SharedPtr<NodeImpl> memManage(oldChild); //Make sure to free if needed
+
// Remove from rendering tree
if (oldChild->attached())
oldChild->detach();
@@ -1210,8 +1216,6 @@
for (NodeImpl *c = oldChild; c; c = c->traverseNextNode(oldChild))
c->removedFromDocument();
}
-
- return oldChild;
}
void NodeBaseImpl::removeChildren()
@@ -1266,6 +1270,7 @@
// If child is already present in the tree, first remove it
NodeImpl *oldParent = child->parentNode();
+ SharedPtr<NodeImpl> guard(child); //Guard in case we move it
if(oldParent) {
oldParent->removeChild( child, exceptioncode );
if (exceptioncode)
--- branches/KDE/3.5/kdelibs/khtml/xml/dom_nodeimpl.h #583313:583314
@@ -131,8 +131,10 @@
// insertBefore, replaceChild and appendChild also close newChild
// unlike the speed optimized addChild (which is used by the parser)
virtual NodeImpl *insertBefore ( NodeImpl *newChild, NodeImpl *refChild, int \
&exceptioncode );
- virtual NodeImpl *replaceChild ( NodeImpl *newChild, NodeImpl *oldChild, int \
&exceptioncode );
- virtual NodeImpl *removeChild ( NodeImpl *oldChild, int &exceptioncode );
+
+ /* These two methods may delete the old node, so make sure to reference it if \
you need it */ + virtual void replaceChild ( NodeImpl *newChild, NodeImpl \
*oldChild, int &exceptioncode ); + virtual void removeChild ( NodeImpl *oldChild, \
int &exceptioncode );
virtual NodeImpl *appendChild ( NodeImpl *newChild, int &exceptioncode );
virtual bool hasChildNodes ( ) const;
virtual NodeImpl *cloneNode ( bool deep ) = 0;
@@ -489,8 +491,8 @@
virtual NodeImpl *firstChild() const;
virtual NodeImpl *lastChild() const;
virtual NodeImpl *insertBefore ( NodeImpl *newChild, NodeImpl *refChild, int \
&exceptioncode );
- virtual NodeImpl *replaceChild ( NodeImpl *newChild, NodeImpl *oldChild, int \
&exceptioncode );
- virtual NodeImpl *removeChild ( NodeImpl *oldChild, int &exceptioncode );
+ virtual void replaceChild ( NodeImpl *newChild, NodeImpl *oldChild, int \
&exceptioncode ); + virtual void removeChild ( NodeImpl *oldChild, int \
&exceptioncode );
virtual NodeImpl *appendChild ( NodeImpl *newChild, int &exceptioncode );
virtual bool hasChildNodes ( ) const;
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic