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 and elements and remove 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: ...FooGoo
//
// Step 1: Remove |blockElem| from its parent, doing a batch detach of all the kids.
+ SharedPtr 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 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 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 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 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 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;