From fae460b8a90328e39d92a48e24f68a2b10ef4e42 Mon Sep 17 00:00:00 2001 From: "Olli.Pettay%helsinki.fi" Date: Wed, 23 Aug 2006 10:00:11 +0000 Subject: [PATCH] Bug 349069, Move more things from ~nsINode to nsNodeUtils::LastRelease, r+sr=sicking git-svn-id: svn://10.0.0.236/trunk@208181 18797224-902f-48f8-a5cc-f745e15eee43 --- mozilla/content/base/src/nsDOMAttribute.cpp | 4 +- mozilla/content/base/src/nsDocument.cpp | 32 ++++------ mozilla/content/base/src/nsDocument.h | 2 + .../content/base/src/nsGenericDOMDataNode.cpp | 4 +- mozilla/content/base/src/nsGenericElement.cpp | 62 +++++-------------- mozilla/content/base/src/nsGenericElement.h | 4 +- mozilla/content/base/src/nsNodeUtils.cpp | 60 +++++++++++++----- mozilla/content/base/src/nsNodeUtils.h | 13 +--- .../xul/document/src/nsXULDocument.cpp | 7 --- 9 files changed, 80 insertions(+), 108 deletions(-) diff --git a/mozilla/content/base/src/nsDOMAttribute.cpp b/mozilla/content/base/src/nsDOMAttribute.cpp index 9f15dbb351b..0507486e08a 100644 --- a/mozilla/content/base/src/nsDOMAttribute.cpp +++ b/mozilla/content/base/src/nsDOMAttribute.cpp @@ -74,8 +74,6 @@ nsDOMAttribute::nsDOMAttribute(nsDOMAttributeMap *aAttrMap, nsDOMAttribute::~nsDOMAttribute() { - nsNodeUtils::NodeWillBeDestroyed(this); - if (mChildList) { mChildList->DropReference(); NS_RELEASE(mChildList); @@ -99,7 +97,7 @@ NS_INTERFACE_MAP_END NS_IMPL_ADDREF(nsDOMAttribute) NS_IMPL_RELEASE_WITH_DESTROY(nsDOMAttribute, - nsNodeUtils::LastRelease(this)) + nsNodeUtils::LastRelease(this, PR_TRUE)) // nsIDOMGCParticipant methods nsIDOMGCParticipant* diff --git a/mozilla/content/base/src/nsDocument.cpp b/mozilla/content/base/src/nsDocument.cpp index 6cd6a765835..dbe1da14f27 100644 --- a/mozilla/content/base/src/nsDocument.cpp +++ b/mozilla/content/base/src/nsDocument.cpp @@ -668,19 +668,6 @@ nsDocument::~nsDocument() mInDestructor = PR_TRUE; - // We can't rely on the nsINode dtor doing this for us since - // by the time it runs GetOwnerDoc will return null. - // This is because we call mNodeInfoManager->DropReference() - // below, which will run before the nsINode dtor. Additionally - // the properties hash and the document will have been destroyed, - // so there would be no way to find the handlers. - if (HasProperties()) { - nsContentUtils::CallUserDataHandler(this, - nsIDOMUserDataHandler::NODE_DELETED, - this, nsnull, nsnull); - } - - nsNodeUtils::NodeWillBeDestroyed(this); // Clear mObservers to keep it in sync with the mutationobserver list mObservers.Clear(); @@ -748,10 +735,6 @@ nsDocument::~nsDocument() NS_RELEASE(mCSSLoader); } - // Delete properties before dropping the document reference from - // NodeInfoManager! - mPropertyTable.DeleteAllProperties(); - // XXX Ideally we'd do this cleanup in the nsIDocument destructor. if (mNodeInfoManager) { mNodeInfoManager->DropDocumentReference(); @@ -771,6 +754,18 @@ nsDocument::~nsDocument() nsLayoutStatics::Release(); } +void +nsDocument::LastRelease() +{ + nsNodeUtils::LastRelease(this, PR_FALSE); + // Delete properties before starting to destruct the document. + // Some of the properties are bound to nsINode objects and the destructor + // functions of the properties may want to use the owner document of the + // nsINode. + mPropertyTable.DeleteAllProperties(); + delete this; +} + NS_INTERFACE_MAP_BEGIN(nsDocument) NS_INTERFACE_MAP_ENTRY(nsIDocument) NS_INTERFACE_MAP_ENTRY(nsIDOMDocument) @@ -813,8 +808,7 @@ NS_INTERFACE_MAP_BEGIN(nsDocument) NS_INTERFACE_MAP_END NS_IMPL_ADDREF(nsDocument) -NS_IMPL_RELEASE_WITH_DESTROY(nsDocument, - nsNodeUtils::LastRelease(this)) +NS_IMPL_RELEASE_WITH_DESTROY(nsDocument, LastRelease()) nsresult nsDocument::Init() diff --git a/mozilla/content/base/src/nsDocument.h b/mozilla/content/base/src/nsDocument.h index d341d7594d0..3edf684bfc7 100644 --- a/mozilla/content/base/src/nsDocument.h +++ b/mozilla/content/base/src/nsDocument.h @@ -683,6 +683,8 @@ protected: nsDocument(const char* aContentType); virtual ~nsDocument(); + void LastRelease(); + nsCString mReferrer; nsString mLastModified; diff --git a/mozilla/content/base/src/nsGenericDOMDataNode.cpp b/mozilla/content/base/src/nsGenericDOMDataNode.cpp index 3a716a17b7d..feefc5940cc 100644 --- a/mozilla/content/base/src/nsGenericDOMDataNode.cpp +++ b/mozilla/content/base/src/nsGenericDOMDataNode.cpp @@ -91,14 +91,12 @@ nsGenericDOMDataNode::~nsGenericDOMDataNode() { NS_PRECONDITION(!IsInDoc(), "Please remove this from the document properly"); - - nsNodeUtils::NodeWillBeDestroyed(this); } NS_IMPL_ADDREF(nsGenericDOMDataNode) NS_IMPL_RELEASE_WITH_DESTROY(nsGenericDOMDataNode, - nsNodeUtils::LastRelease(this)) + nsNodeUtils::LastRelease(this, PR_TRUE)) NS_INTERFACE_MAP_BEGIN(nsGenericDOMDataNode) NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIContent) diff --git a/mozilla/content/base/src/nsGenericElement.cpp b/mozilla/content/base/src/nsGenericElement.cpp index a57ef6c48e0..c9c547af4cb 100644 --- a/mozilla/content/base/src/nsGenericElement.cpp +++ b/mozilla/content/base/src/nsGenericElement.cpp @@ -143,36 +143,7 @@ nsresult NS_NewContentIterator(nsIContentIterator** aInstancePtrResult); nsINode::~nsINode() { - NS_ASSERTION(!HasSlots(), "Don't know how to kill the slots"); - - if (HasFlag(NODE_HAS_RANGELIST)) { -#ifdef DEBUG - if (!nsContentUtils::LookupRangeList(this) && - nsContentUtils::IsInitialized()) { - NS_ERROR("Huh, our bit says we have a range list, but there's nothing " - "in the hash!?!!"); - } -#endif - - nsContentUtils::RemoveRangeList(this); - } - - if (HasFlag(NODE_HAS_LISTENERMANAGER)) { -#ifdef DEBUG - if (nsContentUtils::IsInitialized()) { - nsCOMPtr manager; - PRBool created; - nsContentUtils::GetListenerManager(this, PR_FALSE, - getter_AddRefs(manager), &created); - if (!manager) { - NS_ERROR("Huh, our bit says we have a listener manager list, " - "but there's nothing in the hash!?!!"); - } - } -#endif - - nsContentUtils::RemoveListenerManager(this); - } + delete GetExistingSlots(); } void* @@ -992,21 +963,6 @@ nsGenericElement::nsDOMSlots::nsDOMSlots(PtrBits aFlags) { } -nsGenericElement::nsDOMSlots::~nsDOMSlots() -{ - if (mChildNodes) { - mChildNodes->DropReference(); - } - - if (mStyle) { - mStyle->DropReference(); - } - - if (mAttributeMap) { - mAttributeMap->DropReference(); - } -} - nsGenericElement::nsGenericElement(nsINodeInfo *aNodeInfo) : nsIXMLContent(aNodeInfo) { @@ -1019,8 +975,18 @@ nsGenericElement::~nsGenericElement() { NS_PRECONDITION(!IsInDoc(), "Please remove this from the document properly"); - - nsNodeUtils::NodeWillBeDestroyed(this); + nsDOMSlots* slots = GetExistingDOMSlots(); + if (slots) { + if (slots->mChildNodes) { + slots->mChildNodes->DropReference(); + } + if (slots->mStyle) { + slots->mStyle->DropReference(); + } + if (slots->mAttributeMap) { + slots->mAttributeMap->DropReference(); + } + } } /** @@ -3092,7 +3058,7 @@ NS_INTERFACE_MAP_END NS_IMPL_ADDREF(nsGenericElement) NS_IMPL_RELEASE_WITH_DESTROY(nsGenericElement, - nsNodeUtils::LastRelease(this)) + nsNodeUtils::LastRelease(this, PR_TRUE)) nsresult nsGenericElement::PostQueryInterface(REFNSIID aIID, void** aInstancePtr) diff --git a/mozilla/content/base/src/nsGenericElement.h b/mozilla/content/base/src/nsGenericElement.h index 34a828f77c5..d0decb71e01 100644 --- a/mozilla/content/base/src/nsGenericElement.h +++ b/mozilla/content/base/src/nsGenericElement.h @@ -891,12 +891,14 @@ protected: * objects for each of these instance variables, we put them off * in a side structure that's only allocated when the content is * accessed through the DOM. + * + * @note Any properties in this class has to be cleared in the + * nsGenericElement or nsXULElement destructor. */ class nsDOMSlots : public nsINode::nsSlots { public: nsDOMSlots(PtrBits aFlags); - virtual ~nsDOMSlots(); /** * An object implementing nsIDOMNodeList for this content (childNodes) diff --git a/mozilla/content/base/src/nsNodeUtils.cpp b/mozilla/content/base/src/nsNodeUtils.cpp index 7d3b28de981..117f3609438 100755 --- a/mozilla/content/base/src/nsNodeUtils.cpp +++ b/mozilla/content/base/src/nsNodeUtils.cpp @@ -43,6 +43,7 @@ #include "nsIMutationObserver.h" #include "nsIDocument.h" #include "nsIDOMUserDataHandler.h" +#include "nsIEventListenerManager.h" #define IMPL_MUTATION_NOTIFICATION(func_, content_, params_) \ PR_BEGIN_MACRO \ @@ -147,25 +148,17 @@ nsNodeUtils::ContentRemoved(nsINode* aContainer, } void -nsNodeUtils::NodeWillBeDestroyed(nsINode* aNode) +nsNodeUtils::LastRelease(nsINode* aNode, PRBool aDelete) { nsINode::nsSlots* slots = aNode->GetExistingSlots(); - if (slots) { - if (!slots->mMutationObservers.IsEmpty()) { - NS_OBSERVER_ARRAY_NOTIFY_OBSERVERS(slots->mMutationObservers, - nsIMutationObserver, - NodeWillBeDestroyed, (aNode)); - } - - PtrBits flags = slots->mFlags | NODE_DOESNT_HAVE_SLOTS; - delete slots; - aNode->mFlagsOrSlots = flags; + if (slots && !slots->mMutationObservers.IsEmpty()) { + NS_OBSERVER_ARRAY_NOTIFY_OBSERVERS(slots->mMutationObservers, + nsIMutationObserver, + NodeWillBeDestroyed, (aNode)); } -} -void -nsNodeUtils::LastRelease(nsINode* aNode) -{ + // Kill properties first since that may run external code, so we want to + // be in as complete state as possible at that time. if (aNode->HasProperties()) { nsIDocument* document = aNode->GetOwnerDoc(); if (document) { @@ -174,7 +167,42 @@ nsNodeUtils::LastRelease(nsINode* aNode) aNode, nsnull, nsnull); document->PropertyTable()->DeleteAllPropertiesFor(aNode); } + aNode->UnsetFlags(NODE_HAS_PROPERTIES); + } + + if (aNode->HasFlag(NODE_HAS_RANGELIST)) { +#ifdef DEBUG + if (!nsContentUtils::LookupRangeList(aNode) && + nsContentUtils::IsInitialized()) { + NS_ERROR("Huh, our bit says we have a range list, but there's nothing " + "in the hash!?!!"); + } +#endif + + nsContentUtils::RemoveRangeList(aNode); + aNode->UnsetFlags(NODE_HAS_RANGELIST); + } + + if (aNode->HasFlag(NODE_HAS_LISTENERMANAGER)) { +#ifdef DEBUG + if (nsContentUtils::IsInitialized()) { + nsCOMPtr manager; + PRBool created; + nsContentUtils::GetListenerManager(aNode, PR_FALSE, + getter_AddRefs(manager), &created); + if (!manager) { + NS_ERROR("Huh, our bit says we have a listener manager list, " + "but there's nothing in the hash!?!!"); + } + } +#endif + + nsContentUtils::RemoveListenerManager(aNode); + aNode->UnsetFlags(NODE_HAS_LISTENERMANAGER); + } + + if (aDelete) { + delete aNode; } - delete aNode; } diff --git a/mozilla/content/base/src/nsNodeUtils.h b/mozilla/content/base/src/nsNodeUtils.h index 2459fd3d84e..81afce60a78 100755 --- a/mozilla/content/base/src/nsNodeUtils.h +++ b/mozilla/content/base/src/nsNodeUtils.h @@ -98,22 +98,13 @@ public: static void ContentRemoved(nsINode* aContainer, nsIContent* aChild, PRInt32 aIndexInContainer); - - /** - * Call this before starting to tear down a node. The node should still - * have children and attributes accessible. - * The function will notify nsIMutationObservers as well as delete the - * slots structure. - * @param aNode Node that is being destroyed - * @see nsIMutationObserver::NodeWillBeDestroyed - */ - static void NodeWillBeDestroyed(nsINode* aNode); /** * To be called when reference count of aNode drops to zero. * @param aNode The node which is going to be deleted. + * @param aDelete If PR_TRUE, calling this method also deletes aNode. */ - static void LastRelease(nsINode* aNode); + static void LastRelease(nsINode* aNode, PRBool aDelete); }; #endif // nsNodeUtils_h___ diff --git a/mozilla/content/xul/document/src/nsXULDocument.cpp b/mozilla/content/xul/document/src/nsXULDocument.cpp index 670c1ae36e0..c546cd7a277 100644 --- a/mozilla/content/xul/document/src/nsXULDocument.cpp +++ b/mozilla/content/xul/document/src/nsXULDocument.cpp @@ -215,13 +215,6 @@ nsXULDocument::~nsXULDocument() NS_ASSERTION(mNextSrcLoadWaiter == nsnull, "unreferenced document still waiting for script source to load?"); - // Notify our observers here, we can't let the nsINode - // destructor do that for us since some of the observers are - // deleted by the time we get there. - nsNodeUtils::NodeWillBeDestroyed(this); - // Clear mObservers to keep it in sync with the mutationobserver list - mObservers.Clear(); - // In case we failed somewhere early on and the forward observer // decls never got resolved. DestroyForwardReferences();