Bug 566416. Make attribute nodes set their mFirstChild correctly. r=sicking
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 27 May 2010 15:10:33 -0400
changeset 42871 19b38b92bda36907cbb4360169a15d9ddddbbb3b
parent 42870 03bd98ae2bf3deff8e1ab5d89a6ab4dad3209270
child 42872 ad277036ccbe362009875dd553e7c522428f38fa
push id13490
push userbzbarsky@mozilla.com
push dateThu, 27 May 2010 19:11:08 +0000
treeherdermozilla-central@b0afde3559f9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking
bugs566416
milestone1.9.3a5pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
Bug 566416. Make attribute nodes set their mFirstChild correctly. r=sicking
content/base/src/nsDOMAttribute.cpp
content/base/src/nsDOMAttribute.h
--- a/content/base/src/nsDOMAttribute.cpp
+++ b/content/base/src/nsDOMAttribute.cpp
@@ -68,23 +68,36 @@ nsDOMAttribute::nsDOMAttribute(nsDOMAttr
                                const nsAString   &aValue)
   : nsIAttribute(aAttrMap, aNodeInfo), mValue(aValue), mChild(nsnull)
 {
   NS_ABORT_IF_FALSE(mNodeInfo, "We must get a nodeinfo here!");
 
 
   // We don't add a reference to our content. It will tell us
   // to drop our reference when it goes away.
+
+  EnsureChildState();
+
+  nsIContent* content = GetContentInternal();
+  if (content) {
+    content->AddMutationObserver(this);
+  }
 }
 
 nsDOMAttribute::~nsDOMAttribute()
 {
   if (mChild) {
     static_cast<nsTextNode*>(mChild)->UnbindFromAttribute();
     NS_RELEASE(mChild);
+    mFirstChild = nsnull;
+  }
+
+  nsIContent* content = GetContentInternal();
+  if (content) {
+    content->RemoveMutationObserver(this);
   }
 }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsDOMAttribute)
 
 NS_IMPL_CYCLE_COLLECTION_ROOT_BEGIN(nsDOMAttribute)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
 NS_IMPL_CYCLE_COLLECTION_ROOT_END
@@ -100,28 +113,29 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(nsDOMAttribute)
   NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsDOMAttribute)
   if (tmp->mChild) {
     static_cast<nsTextNode*>(tmp->mChild)->UnbindFromAttribute();
     NS_RELEASE(tmp->mChild);
+    tmp->mFirstChild = nsnull;
   }
   NS_IMPL_CYCLE_COLLECTION_UNLINK_LISTENERMANAGER
   NS_IMPL_CYCLE_COLLECTION_UNLINK_USERDATA
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 DOMCI_DATA(Attr, nsDOMAttribute)
 
 // QueryInterface implementation for nsDOMAttribute
 NS_INTERFACE_TABLE_HEAD(nsDOMAttribute)
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
-  NS_NODE_INTERFACE_TABLE5(nsDOMAttribute, nsIDOMAttr, nsIAttribute, nsIDOMNode,
-                           nsIDOM3Attr, nsPIDOMEventTarget)
+  NS_NODE_INTERFACE_TABLE6(nsDOMAttribute, nsIDOMAttr, nsIAttribute, nsIDOMNode,
+                           nsIDOM3Attr, nsPIDOMEventTarget, nsIMutationObserver)
   NS_INTERFACE_MAP_ENTRIES_CYCLE_COLLECTION(nsDOMAttribute)
   NS_INTERFACE_MAP_ENTRY_TEAROFF(nsISupportsWeakReference,
                                  new nsNodeSupportsWeakRefTearoff(this))
   NS_INTERFACE_MAP_ENTRY_TEAROFF(nsIDOMEventTarget,
                                  nsDOMEventRTTearoff::Create(this))
   NS_INTERFACE_MAP_ENTRY_TEAROFF(nsIDOM3EventTarget,
                                  nsDOMEventRTTearoff::Create(this))
   NS_INTERFACE_MAP_ENTRY_TEAROFF(nsIDOMNSEventTarget,
@@ -139,16 +153,21 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE_FULL(ns
 void
 nsDOMAttribute::SetMap(nsDOMAttributeMap *aMap)
 {
   if (mAttrMap && !aMap && sInitialized) {
     // We're breaking a relationship with content and not getting a new one,
     // need to locally cache value. GetValue() does that.
     GetValue(mValue);
   }
+
+  nsIContent* content = GetContentInternal();
+  if (content) {
+    content->RemoveMutationObserver(this);
+  }
   
   mAttrMap = aMap;
 }
 
 nsIContent*
 nsDOMAttribute::GetContent() const
 {
   return GetContentInternal();
@@ -301,21 +320,17 @@ NS_IMETHODIMP
 nsDOMAttribute::GetChildNodes(nsIDOMNodeList** aChildNodes)
 {
   return nsINode::GetChildNodes(aChildNodes);
 }
 
 NS_IMETHODIMP
 nsDOMAttribute::HasChildNodes(PRBool* aHasChildNodes)
 {
-  PRBool hasChild;
-  nsresult rv = EnsureChildState(PR_FALSE, hasChild);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  *aHasChildNodes = hasChild;
+  *aHasChildNodes = mFirstChild != nsnull;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDOMAttribute::HasAttributes(PRBool* aHasAttributes)
 {
   NS_ENSURE_ARG_POINTER(aHasAttributes);
@@ -325,22 +340,18 @@ nsDOMAttribute::HasAttributes(PRBool* aH
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDOMAttribute::GetFirstChild(nsIDOMNode** aFirstChild)
 {
   *aFirstChild = nsnull;
 
-  PRBool hasChild;
-  nsresult rv = EnsureChildState(PR_TRUE, hasChild);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (mChild) {
-    CallQueryInterface(mChild, aFirstChild);
+  if (mFirstChild) {
+    CallQueryInterface(mFirstChild, aFirstChild);
   }
   
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDOMAttribute::GetLastChild(nsIDOMNode** aLastChild)
 {
@@ -579,48 +590,40 @@ PRBool
 nsDOMAttribute::IsNodeOfType(PRUint32 aFlags) const
 {
     return !(aFlags & ~eATTRIBUTE);
 }
 
 PRUint32
 nsDOMAttribute::GetChildCount() const
 {
-  return GetChildCount(PR_FALSE);
+  return mFirstChild ? 1 : 0;
 }
 
 nsIContent *
 nsDOMAttribute::GetChildAt(PRUint32 aIndex) const
 {
-  // Don't need to check result of EnsureChildState since mChild will be null.
-  PRBool hasChild;
-  EnsureChildState(PR_TRUE, hasChild);
-
-  return aIndex == 0 && hasChild ? mChild : nsnull;
+  return aIndex == 0 ? mFirstChild : nsnull;
 }
 
 nsIContent * const *
 nsDOMAttribute::GetChildArray(PRUint32* aChildCount) const
 {
-  *aChildCount = GetChildCount(PR_TRUE);
-  return &mChild;
+  *aChildCount = GetChildCount();
+  return &mFirstChild;
 }
 
 PRInt32
 nsDOMAttribute::IndexOf(nsINode* aPossibleChild) const
 {
-  // No need to call EnsureChildState here. If we don't already have a child
-  // then aPossibleChild can't possibly be our child.
-  if (!aPossibleChild || aPossibleChild != mChild) {
+  if (!aPossibleChild || aPossibleChild != mFirstChild) {
     return -1;
   }
 
-  PRBool hasChild;
-  EnsureChildState(PR_FALSE, hasChild);
-  return hasChild ? 0 : -1;
+  return 0;
 }
 
 nsresult
 nsDOMAttribute::InsertChildAt(nsIContent* aKid, PRUint32 aIndex,
                               PRBool aNotify)
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
@@ -654,18 +657,16 @@ nsDOMAttribute::RemoveChildAt(PRUint32 a
     mutation.mRelatedNode =
       do_QueryInterface(static_cast<nsIAttribute*>(this));
     subtree.UpdateTarget(GetOwnerDoc(), this);
     nsEventDispatcher::Dispatch(mChild, nsnull, &mutation);
   }
   if (guard.Mutated(0) && mChild != child) {
     return NS_OK;
   }
-  NS_RELEASE(mChild);
-  static_cast<nsTextNode*>(child.get())->UnbindFromAttribute();
 
   nsString nullString;
   SetDOMStringToNull(nullString);
   SetValue(nullString);
   return NS_OK;
 }
 
 nsresult
@@ -719,44 +720,62 @@ nsDOMAttribute::RemoveEventListenerByIID
 nsresult
 nsDOMAttribute::GetSystemEventGroup(nsIDOMEventGroup** aGroup)
 {
   nsIEventListenerManager* elm = GetListenerManager(PR_TRUE);
   NS_ENSURE_STATE(elm);
   return elm->GetSystemEventGroupLM(aGroup);
 }
 
-nsresult
-nsDOMAttribute::EnsureChildState(PRBool aSetText, PRBool &aHasChild) const
+void
+nsDOMAttribute::EnsureChildState()
 {
-  aHasChild = PR_FALSE;
-
-  nsDOMAttribute* mutableThis = const_cast<nsDOMAttribute*>(this);
+  NS_PRECONDITION(!mChild, "Someone screwed up");
 
   nsAutoString value;
-  mutableThis->GetValue(value);
+  GetValue(value);
+
+  if (!value.IsEmpty()) {
+    NS_NewTextNode(&mChild, mNodeInfo->NodeInfoManager());
+
+    static_cast<nsTextNode*>(mChild)->BindToAttribute(this);
+    mFirstChild = mChild;
 
-  if (!mChild && !value.IsEmpty()) {
-    nsresult rv = NS_NewTextNode(&mutableThis->mChild,
-                                 mNodeInfo->NodeInfoManager());
-    NS_ENSURE_SUCCESS(rv, rv);
+    mChild->SetText(value, PR_FALSE);
+  }
+}
 
-    static_cast<nsTextNode*>(mChild)->BindToAttribute(mutableThis);
+void
+nsDOMAttribute::AttributeChanged(nsIDocument* aDocument,
+                                 nsIContent* aContent,
+                                 PRInt32 aNameSpaceID,
+                                 nsIAtom* aAttribute,
+                                 PRInt32 aModType)
+{
+  nsIContent* content = GetContentInternal();
+  if (aContent != content) {
+    return;
   }
 
-  aHasChild = !value.IsEmpty();
-
-  if (aSetText && aHasChild) {
-    // aNotify should probably be PR_TRUE sometimes, but it's unlikely that
-    // anyone cares. And we aren't updating the node when the attribute changes
-    // anyway so any notifications are way late.
-    mChild->SetText(value, PR_FALSE);
+  if (aNameSpaceID != mNodeInfo->NamespaceID()) {
+    return;
   }
 
-  return NS_OK;
+  nsCOMPtr<nsIAtom> nameAtom = GetNameAtom(content);
+  if (nameAtom != aAttribute) {
+    return;
+  }
+
+  // Just blow away our mChild and recreate it if needed
+  if (mChild) {
+    static_cast<nsTextNode*>(mChild)->UnbindFromAttribute();
+    NS_RELEASE(mChild);
+    mFirstChild = nsnull;
+  }
+  EnsureChildState();
 }
 
 void
 nsDOMAttribute::Initialize()
 {
   sInitialized = PR_TRUE;
 }
 
--- a/content/base/src/nsDOMAttribute.h
+++ b/content/base/src/nsDOMAttribute.h
@@ -48,22 +48,24 @@
 #include "nsIDOMNodeList.h"
 #include "nsString.h"
 #include "nsCOMPtr.h"
 #include "nsINodeInfo.h"
 #include "nsIDOM3Attr.h"
 #include "nsDOMAttributeMap.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsContentUtils.h"
+#include "nsStubMutationObserver.h"
 
 // Attribute helper class used to wrap up an attribute with a dom
 // object that implements nsIDOMAttr, nsIDOM3Attr, nsIDOMNode, nsIDOM3Node
 class nsDOMAttribute : public nsIAttribute,
                        public nsIDOMAttr,
-                       public nsIDOM3Attr
+                       public nsIDOM3Attr,
+                       public nsStubMutationObserver
 {
 public:
   nsDOMAttribute(nsDOMAttributeMap* aAttrMap, nsINodeInfo *aNodeInfo,
                  const nsAString& aValue);
   virtual ~nsDOMAttribute();
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
 
@@ -113,36 +115,30 @@ public:
   virtual nsresult SetTextContent(const nsAString& aTextContent);
 
   static void Initialize();
   static void Shutdown();
 
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_AMBIGUOUS(nsDOMAttribute,
                                                          nsIAttribute)
 
+  NS_DECL_NSIMUTATIONOBSERVER_ATTRIBUTECHANGED
+
 protected:
   virtual mozilla::dom::Element* GetNameSpaceElement()
   {
     return GetContentInternal()->AsElement();
   }
 
   static PRBool sInitialized;
 
 private:
   already_AddRefed<nsIAtom> GetNameAtom(nsIContent* aContent);
 
-  nsresult EnsureChildState(PRBool aSetText, PRBool &aHasChild) const;
-
-  PRUint32 GetChildCount(PRBool aSetText) const
-  {
-    PRBool hasChild;
-    EnsureChildState(aSetText, hasChild);
-
-    return hasChild ? 1 : 0;
-  }
+  void EnsureChildState();
 
   nsString mValue;
   // XXX For now, there's only a single child - a text element
   // representing the value.  This is strong ref, but we use a raw
   // pointer so we can implement GetChildArray().
   nsIContent* mChild;
 
   nsIContent *GetContentInternal() const