Bug 686941: Make nsIDocument::mCachedRootElement a weak pointer. r=smaug
authorKyle Huey <khuey@kylehuey.com>
Thu, 29 Sep 2011 12:06:35 -0400
changeset 77806 d0dac98118954288c12052e4cc0016273761fbfb
parent 77805 6c18cde2d3b7461c338dd9ccdd6cb5f8941b42f3
child 77807 34b49c378a6baeb5e49fdf5f12a592e241e6280f
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewerssmaug
bugs686941
milestone10.0a1
Bug 686941: Make nsIDocument::mCachedRootElement a weak pointer. r=smaug
content/base/public/nsIDocument.h
content/base/src/nsDocument.cpp
--- a/content/base/public/nsIDocument.h
+++ b/content/base/public/nsIDocument.h
@@ -522,23 +522,18 @@ public:
   /**
    * Find the content node for which aDocument is a sub document.
    */
   virtual nsIContent *FindContentForSubDocument(nsIDocument *aDocument) const = 0;
 
   /**
    * Return the root element for this document.
    */
-  Element *GetRootElement() const
-  {
-    return (mCachedRootElement &&
-            mCachedRootElement->GetNodeParent() == this) ?
-           reinterpret_cast<Element*>(mCachedRootElement.get()) :
-           GetRootElementInternal();
-  }
+  Element *GetRootElement() const;
+
 protected:
   virtual Element *GetRootElementInternal() const = 0;
 
 public:
   // Get the root <html> element, or return null if there isn't one (e.g.
   // if the root isn't <html>)
   Element* GetHtmlElement();
   // Returns the first child of GetHtmlContent which has the given tag,
@@ -1639,19 +1634,17 @@ protected:
 
   nsCString mCharacterSet;
   PRInt32 mCharacterSetSource;
 
   // This is just a weak pointer; the parent document owns its children.
   nsIDocument* mParentDocument;
 
   // A reference to the element last returned from GetRootElement().
-  // This should be an Element, but that would force us to pull in
-  // Element.h and therefore nsIContent.h.
-  nsCOMPtr<nsINode> mCachedRootElement;
+  mozilla::dom::Element* mCachedRootElement;
 
   // We'd like these to be nsRefPtrs, but that'd require us to include
   // additional headers that we don't want to expose.
   // The cleanup is handled by the nsDocument destructor.
   nsNodeInfoManager* mNodeInfoManager; // [STRONG]
   mozilla::css::Loader* mCSSLoader; // [STRONG]
 
   // The set of all object, embed, applet, video and audio elements for
--- a/content/base/src/nsDocument.cpp
+++ b/content/base/src/nsDocument.cpp
@@ -1820,17 +1820,16 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_
 
   // Traverse the mChildren nsAttrAndChildArray.
   for (PRInt32 indx = PRInt32(tmp->mChildren.ChildCount()); indx > 0; --indx) {
     NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mChildren[i]");
     cb.NoteXPCOMChild(tmp->mChildren.ChildAt(indx - 1));
   }
 
   // Traverse all nsIDocument pointer members.
-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mCachedRootElement)
   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)
@@ -1901,17 +1900,17 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ns
   for (PRInt32 indx = PRInt32(tmp->mChildren.ChildCount()) - 1; 
        indx >= 0; --indx) {
     tmp->mChildren.ChildAt(indx)->UnbindFromTree();
     tmp->mChildren.RemoveChildAt(indx);
   }
   tmp->mFirstChild = nsnull;
 
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mXPathEvaluatorTearoff)
-  NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mCachedRootElement)
+  tmp->mCachedRootElement = nsnull; // Avoid a dangling pointer
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mDisplayDocument)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mFirstBaseNodeWithHref)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mDOMImplementation)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mImageMaps)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mOriginalDocument)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mCachedEncoder)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mFullScreenElement)
 
@@ -3392,25 +3391,32 @@ nsDocument::NodeType()
 
 void
 nsDocument::NodeName(nsAString& aNodeName)
 {
   aNodeName.AssignLiteral("#document");
 }
 
 Element*
+nsIDocument::GetRootElement() const
+{
+  return (mCachedRootElement && mCachedRootElement->GetNodeParent() == this) ?
+         mCachedRootElement : GetRootElementInternal();
+}
+
+Element*
 nsDocument::GetRootElementInternal() const
 {
   // Loop backwards because any non-elements, such as doctypes and PIs
   // are likely to appear before the root element.
   PRUint32 i;
   for (i = mChildren.ChildCount(); i > 0; --i) {
     nsIContent* child = mChildren.ChildAt(i - 1);
     if (child->IsElement()) {
-      const_cast<nsDocument*>(this)->mCachedRootElement = child;
+      const_cast<nsDocument*>(this)->mCachedRootElement = child->AsElement();
       return child->AsElement();
     }
   }
   
   const_cast<nsDocument*>(this)->mCachedRootElement = nsnull;
   return nsnull;
 }