bug 335998, strong parentNode, r=peterv
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Tue, 26 Jul 2011 14:11:14 +0300
changeset 73326 c53ee19aeffa1ed8c0526973272e25e267494af7
parent 73325 982a5835fba1bddf72e64775a5601132bef77181
child 73327 029dd90bd20e0ec47f5a4e83129d1ac1dbc6b5c4
push id20847
push useropettay@mozilla.com
push dateTue, 26 Jul 2011 12:00:27 +0000
treeherdermozilla-central@c53ee19aeffa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs335998
milestone8.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 335998, strong parentNode, r=peterv
content/base/src/nsDocument.cpp
content/base/src/nsGenericDOMDataNode.cpp
content/base/src/nsGenericElement.cpp
content/base/src/nsNodeInfo.cpp
content/base/src/nsNodeInfoManager.cpp
content/base/src/nsNodeInfoManager.h
--- a/content/base/src/nsDocument.cpp
+++ b/content/base/src/nsDocument.cpp
@@ -1645,17 +1645,16 @@ nsDocument::~nsDocument()
     // Could be null here if Init() failed
     mCSSLoader->DropDocumentReference();
     NS_RELEASE(mCSSLoader);
   }
 
   // XXX Ideally we'd do this cleanup in the nsIDocument destructor.
   if (mNodeInfoManager) {
     mNodeInfoManager->DropDocumentReference();
-    NS_RELEASE(mNodeInfoManager);
   }
 
   if (mAttrStyleSheet) {
     mAttrStyleSheet->SetOwningDocument(nsnull);
   }
   
   if (mStyleAttrStyleSheet) {
     mStyleAttrStyleSheet->SetOwningDocument(nsnull);
@@ -1831,18 +1830,16 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_
     NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mChildren[i]");
     cb.NoteXPCOMChild(tmp->mChildren.ChildAt(indx - 1));
   }
 
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_USERDATA
 
   // Traverse all nsIDocument pointer members.
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mCachedRootElement)
-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NATIVE_MEMBER(mNodeInfoManager,
-                                                  nsNodeInfoManager)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mSecurityInfo)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mDisplayDocument)
 
   // Traverse all nsDocument nsCOMPtrs.
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mParser)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mScriptGlobalObject)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NATIVE_MEMBER(mListenerManager,
                                                   nsEventListenerManager)
@@ -1997,21 +1994,20 @@ nsDocument::Init()
   NS_ENSURE_TRUE(mCSSLoader, NS_ERROR_OUT_OF_MEMORY);
   NS_ADDREF(mCSSLoader);
   // Assume we're not quirky, until we know otherwise
   mCSSLoader->SetCompatibilityMode(eCompatibility_FullStandards);
 
   mNodeInfoManager = new nsNodeInfoManager();
   NS_ENSURE_TRUE(mNodeInfoManager, NS_ERROR_OUT_OF_MEMORY);
 
-  NS_ADDREF(mNodeInfoManager);
-
   nsresult  rv = mNodeInfoManager->Init(this);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  // mNodeInfo keeps NodeInfoManager alive!
   mNodeInfo = mNodeInfoManager->GetDocumentNodeInfo();
   NS_ENSURE_TRUE(mNodeInfo, NS_ERROR_OUT_OF_MEMORY);
   NS_ABORT_IF_FALSE(mNodeInfo->NodeType() == nsIDOMNode::DOCUMENT_NODE,
                     "Bad NodeType in aNodeInfo");
 
   NS_ASSERTION(GetOwnerDoc() == this, "Our nodeinfo is busted!");
 
   mScriptLoader = new nsScriptLoader(this);
--- a/content/base/src/nsGenericDOMDataNode.cpp
+++ b/content/base/src/nsGenericDOMDataNode.cpp
@@ -80,16 +80,19 @@ nsGenericDOMDataNode::nsGenericDOMDataNo
                     mNodeInfo->NodeType() == nsIDOMNode::DOCUMENT_TYPE_NODE,
                     "Bad NodeType in aNodeInfo");
 }
 
 nsGenericDOMDataNode::~nsGenericDOMDataNode()
 {
   NS_PRECONDITION(!IsInDoc(),
                   "Please remove this from the document properly");
+  if (GetParent()) {
+    NS_RELEASE(mParent);
+  }
 }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsGenericDOMDataNode)
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(nsGenericDOMDataNode)
   NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
@@ -108,16 +111,17 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(
 
   nsIDocument* ownerDoc = tmp->GetOwnerDoc();
   if (ownerDoc) {
     ownerDoc->BindingManager()->Traverse(tmp, cb);
   }
 
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_LISTENERMANAGER
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_USERDATA
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_RAWPTR(GetParent())
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsGenericDOMDataNode)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
   NS_IMPL_CYCLE_COLLECTION_UNLINK_LISTENERMANAGER
   NS_IMPL_CYCLE_COLLECTION_UNLINK_USERDATA
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
@@ -493,16 +497,19 @@ nsGenericDOMDataNode::BindToTree(nsIDocu
     slots->mBindingParent = aBindingParent; // Weak, so no addref happens.
     if (aParent->IsInNativeAnonymousSubtree()) {
       SetFlags(NODE_IS_IN_ANONYMOUS_SUBTREE);
     }
   }
 
   // Set parent
   if (aParent) {
+    if (!GetParent()) {
+      NS_ADDREF(aParent);
+    }
     mParent = aParent;
   }
   else {
     mParent = aDocument;
   }
   SetParentIsContent(aParent);
 
   // XXXbz sXBL/XBL2 issue!
@@ -541,17 +548,21 @@ nsGenericDOMDataNode::UnbindFromTree(PRB
   if (document) {
     // Notify XBL- & nsIAnonymousContentCreator-generated
     // anonymous content that the document is changing.
     // This is needed to update the insertion point.
     document->BindingManager()->RemovedFromDocument(this, document);
   }
 
   if (aNullParent) {
-    mParent = nsnull;
+    if (GetParent()) {
+      NS_RELEASE(mParent);
+    } else {
+      mParent = nsnull;
+    }
     SetParentIsContent(false);
   }
   ClearInDocument();
 
   nsDataSlots *slots = GetExistingDataSlots();
   if (slots) {
     slots->mBindingParent = nsnull;
   }
--- a/content/base/src/nsGenericElement.cpp
+++ b/content/base/src/nsGenericElement.cpp
@@ -2195,16 +2195,19 @@ nsGenericElement::nsGenericElement(alrea
   SetFlags((nsIProgrammingLanguage::JAVASCRIPT << NODE_SCRIPT_TYPE_OFFSET));
   SetIsElement();
 }
 
 nsGenericElement::~nsGenericElement()
 {
   NS_PRECONDITION(!IsInDoc(),
                   "Please remove this from the document properly");
+  if (GetParent()) {
+    NS_RELEASE(mParent);
+  }
 }
 
 NS_IMETHODIMP
 nsGenericElement::GetNodeName(nsAString& aNodeName)
 {
   aNodeName = NodeName();
   return NS_OK;
 }
@@ -2817,16 +2820,19 @@ nsGenericElement::BindToTree(nsIDocument
   if (aParent && aParent->IsInNativeAnonymousSubtree()) {
     SetFlags(NODE_IS_IN_ANONYMOUS_SUBTREE);
   }
 
   PRBool hadForceXBL = HasFlag(NODE_FORCE_XBL_BINDINGS);
 
   // Now set the parent and set the "Force attach xbl" flag if needed.
   if (aParent) {
+    if (!GetParent()) {
+      NS_ADDREF(aParent);
+    }
     mParent = aParent;
 
     if (aParent->HasFlag(NODE_FORCE_XBL_BINDINGS)) {
       SetFlags(NODE_FORCE_XBL_BINDINGS);
     }
   }
   else {
     mParent = aDocument;
@@ -2925,17 +2931,21 @@ nsGenericElement::UnbindFromTree(PRBool 
   NS_PRECONDITION(aDeep || (!GetCurrentDoc() && !GetBindingParent()),
                   "Shallow unbind won't clear document and binding parent on "
                   "kids!");
   // Make sure to unbind this node before doing the kids
   nsIDocument *document =
     HasFlag(NODE_FORCE_XBL_BINDINGS) ? GetOwnerDoc() : GetCurrentDoc();
 
   if (aNullParent) {
-    mParent = nsnull;
+    if (GetParent()) {
+      NS_RELEASE(mParent);
+    } else {
+      mParent = nsnull;
+    }
     SetParentIsContent(false);
   }
   ClearInDocument();
 
   if (document) {
     // Notify XBL- & nsIAnonymousContentCreator-generated
     // anonymous content that the document is changing.
     document->BindingManager()->RemovedFromDocument(this, document);
@@ -4230,17 +4240,19 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_
       NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "slots mAttributeMap");
       cb.NoteXPCOMChild(slots->mAttributeMap.get());
 
       if (tmp->IsXUL())
         cb.NoteXPCOMChild(slots->mControllers);
       cb.NoteXPCOMChild(
         static_cast<nsIDOMNodeList*>(slots->mChildrenList.get()));
     }
-  }
+    NS_IMPL_CYCLE_COLLECTION_TRAVERSE_RAWPTR(GetParent())
+  }
+  
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 
 NS_INTERFACE_MAP_BEGIN(nsGenericElement)
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
   NS_INTERFACE_MAP_ENTRIES_CYCLE_COLLECTION(nsGenericElement)
   NS_INTERFACE_MAP_ENTRY(nsIContent)
   NS_INTERFACE_MAP_ENTRY(nsINode)
--- a/content/base/src/nsNodeInfo.cpp
+++ b/content/base/src/nsNodeInfo.cpp
@@ -94,21 +94,21 @@ nsNodeInfo::Create(nsIAtom *aName, nsIAt
     new (place) nsNodeInfo(aName, aPrefix, aNamespaceID, aNodeType, aExtraName,
                            aOwnerManager) :
     nsnull;
 }
 
 nsNodeInfo::~nsNodeInfo()
 {
   mOwnerManager->RemoveNodeInfo(this);
-  NS_RELEASE(mOwnerManager);
 
   NS_RELEASE(mInner.mName);
   NS_IF_RELEASE(mInner.mPrefix);
   NS_IF_RELEASE(mInner.mExtraName);
+  NS_RELEASE(mOwnerManager);
 }
 
 
 nsNodeInfo::nsNodeInfo(nsIAtom *aName, nsIAtom *aPrefix, PRInt32 aNamespaceID,
                        PRUint16 aNodeType, nsIAtom* aExtraName,
                        nsNodeInfoManager *aOwnerManager)
 {
   CHECK_VALID_NODEINFO(aNodeType, aName, aNamespaceID, aExtraName);
--- a/content/base/src/nsNodeInfoManager.cpp
+++ b/content/base/src/nsNodeInfoManager.cpp
@@ -107,16 +107,17 @@ nsNodeInfoManager::NodeInfoInnerKeyCompa
     return (node2->mName->Equals(*(node1->mNameString)));
   }
   return (node1->mNameString->Equals(*(node2->mNameString)));
 }
 
 
 nsNodeInfoManager::nsNodeInfoManager()
   : mDocument(nsnull),
+    mNonDocumentNodeInfos(0),
     mPrincipal(nsnull),
     mTextNodeInfo(nsnull),
     mCommentNodeInfo(nsnull),
     mDocumentNodeInfo(nsnull),
     mBindingManager(nsnull)
 {
   nsLayoutStatics::AddRef();
 
@@ -155,16 +156,19 @@ nsNodeInfoManager::~nsNodeInfoManager()
 }
 
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsNodeInfoManager)
 NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(nsNodeInfoManager, AddRef)
 NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(nsNodeInfoManager, Release)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_NATIVE_0(nsNodeInfoManager)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NATIVE_BEGIN(nsNodeInfoManager)
+  if (tmp->mNonDocumentNodeInfos) {
+    NS_IMPL_CYCLE_COLLECTION_TRAVERSE_RAWPTR(mDocument)
+  }
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_RAWPTR(mBindingManager)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 nsresult
 nsNodeInfoManager::Init(nsIDocument *aDocument)
 {
   NS_ENSURE_TRUE(mNodeInfoHash, NS_ERROR_OUT_OF_MEMORY);
 
@@ -204,18 +208,20 @@ nsNodeInfoManager::DropNodeInfoDocument(
 
 void
 nsNodeInfoManager::DropDocumentReference()
 {
   if (mBindingManager) {
     mBindingManager->DropDocumentReference();
   }
 
+  // This is probably not needed anymore.
   PL_HashTableEnumerateEntries(mNodeInfoHash, DropNodeInfoDocument, nsnull);
 
+  NS_ASSERTION(!mNonDocumentNodeInfos, "Shouldn't have non-document nodeinfos!");
   mDocument = nsnull;
 }
 
 
 already_AddRefed<nsINodeInfo>
 nsNodeInfoManager::GetNodeInfo(nsIAtom *aName, nsIAtom *aPrefix,
                                PRInt32 aNamespaceID, PRUint16 aNodeType,
                                nsIAtom* aExtraName /* = nsnull */)
@@ -241,16 +247,21 @@ nsNodeInfoManager::GetNodeInfo(nsIAtom *
   NS_ENSURE_TRUE(newNodeInfo, nsnull);
   
   PLHashEntry *he;
   he = PL_HashTableAdd(mNodeInfoHash, &newNodeInfo->mInner, newNodeInfo);
   NS_ENSURE_TRUE(he, nsnull);
 
   // Have to do the swap thing, because already_AddRefed<nsNodeInfo>
   // doesn't cast to already_AddRefed<nsINodeInfo>
+  ++mNonDocumentNodeInfos;
+  if (mNonDocumentNodeInfos == 1) {
+    NS_IF_ADDREF(mDocument);
+  }
+
   nsNodeInfo *nodeInfo = nsnull;
   newNodeInfo.swap(nodeInfo);
 
   return nodeInfo;
 }
 
 
 nsresult
@@ -285,16 +296,21 @@ nsNodeInfoManager::GetNodeInfo(const nsA
       nsNodeInfo::Create(nameAtom, aPrefix, aNamespaceID, aNodeType, nsnull,
                          this);
   NS_ENSURE_TRUE(newNodeInfo, NS_ERROR_OUT_OF_MEMORY);
 
   PLHashEntry *he;
   he = PL_HashTableAdd(mNodeInfoHash, &newNodeInfo->mInner, newNodeInfo);
   NS_ENSURE_TRUE(he, NS_ERROR_FAILURE);
 
+  ++mNonDocumentNodeInfos;
+  if (mNonDocumentNodeInfos == 1) {
+    NS_IF_ADDREF(mDocument);
+  }
+
   newNodeInfo.forget(aNodeInfo);
 
   return NS_OK;
 }
 
 
 nsresult
 nsNodeInfoManager::GetNodeInfo(const nsAString& aName, nsIAtom *aPrefix,
@@ -342,19 +358,24 @@ nsNodeInfoManager::GetCommentNodeInfo()
 
   return mCommentNodeInfo;
 }
 
 already_AddRefed<nsINodeInfo>
 nsNodeInfoManager::GetDocumentNodeInfo()
 {
   if (!mDocumentNodeInfo) {
+    NS_ASSERTION(mDocument, "Should have mDocument!");
     mDocumentNodeInfo = GetNodeInfo(nsGkAtoms::documentNodeName, nsnull,
                                     kNameSpaceID_None,
                                     nsIDOMNode::DOCUMENT_NODE, nsnull).get();
+    --mNonDocumentNodeInfos;
+    if (!mNonDocumentNodeInfos) {
+      mDocument->Release(); // Don't set mDocument to null!
+    }
   }
   else {
     NS_ADDREF(mDocumentNodeInfo);
   }
 
   return mDocumentNodeInfo;
 }
 
@@ -371,25 +392,34 @@ nsNodeInfoManager::SetDocumentPrincipal(
   NS_ADDREF(mPrincipal = aPrincipal);
 }
 
 void
 nsNodeInfoManager::RemoveNodeInfo(nsNodeInfo *aNodeInfo)
 {
   NS_PRECONDITION(aNodeInfo, "Trying to remove null nodeinfo from manager!");
 
-  // Drop weak reference if needed
-  if (aNodeInfo == mTextNodeInfo) {
-    mTextNodeInfo = nsnull;
-  }
-  else if (aNodeInfo == mCommentNodeInfo) {
-    mCommentNodeInfo = nsnull;
-  }
-  else if (aNodeInfo == mDocumentNodeInfo) {
+  if (aNodeInfo == mDocumentNodeInfo) {
     mDocumentNodeInfo = nsnull;
+    mDocument = nsnull;
+  } else {
+    if (--mNonDocumentNodeInfos == 0) {
+      if (mDocument) {
+        // Note, whoever calls this method should keep NodeInfoManager alive,
+        // even if mDocument gets deleted.
+        mDocument->Release();
+      }
+    }
+    // Drop weak reference if needed
+    if (aNodeInfo == mTextNodeInfo) {
+      mTextNodeInfo = nsnull;
+    }
+    else if (aNodeInfo == mCommentNodeInfo) {
+      mCommentNodeInfo = nsnull;
+    }
   }
 
 #ifdef DEBUG
   PRBool ret =
 #endif
   PL_HashTableRemove(mNodeInfoHash, &aNodeInfo->mInner);
 
   NS_POSTCONDITION(ret, "Can't find nsINodeInfo to remove!!!");
--- a/content/base/src/nsNodeInfoManager.h
+++ b/content/base/src/nsNodeInfoManager.h
@@ -153,16 +153,17 @@ protected:
 private:
   static PRIntn NodeInfoInnerKeyCompare(const void *key1, const void *key2);
   static PLHashNumber GetNodeInfoInnerHashValue(const void *key);
   static PRIntn DropNodeInfoDocument(PLHashEntry *he, PRIntn hashIndex,
                                      void *arg);
 
   PLHashTable *mNodeInfoHash;
   nsIDocument *mDocument; // WEAK
+  PRUint32 mNonDocumentNodeInfos;
   nsIPrincipal *mPrincipal; // STRONG, but not nsCOMPtr to avoid include hell
                             // while inlining DocumentPrincipal().  Never null
                             // after Init() succeeds.
   nsCOMPtr<nsIPrincipal> mDefaultPrincipal; // Never null after Init() succeeds
   nsINodeInfo *mTextNodeInfo; // WEAK to avoid circular ownership
   nsINodeInfo *mCommentNodeInfo; // WEAK to avoid circular ownership
   nsINodeInfo *mDocumentNodeInfo; // WEAK to avoid circular ownership
   nsBindingManager* mBindingManager; // STRONG, but not nsCOMPtr to avoid