Part 1 of fix for bug 564266 (DOMCI GetItemAt/GetNamedItem should return nsWrapperCache) - store nsDOMAttribute in nsDOMAttributeMap instead of nsIDOMNode. r=jst.
authorPeter Van der Beken <peterv@propagandism.org>
Mon, 19 Apr 2010 17:41:39 +0200
changeset 47799 bae5b61636fc1a9e2c9eb24ee7584fff274830de
parent 47798 ced41ebe9e7545d14f4138597db806582bf169c6
child 47800 5fdc1c421b2808ee4e6b7ae7742151ca0588cfd5
push id14424
push userpvanderbeken@mozilla.com
push dateFri, 16 Jul 2010 13:09:01 +0000
treeherdermozilla-central@8951841a4cda [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjst
bugs564266
milestone2.0b2pre
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
Part 1 of fix for bug 564266 (DOMCI GetItemAt/GetNamedItem should return nsWrapperCache) - store nsDOMAttribute in nsDOMAttributeMap instead of nsIDOMNode. r=jst.
content/base/src/nsDOMAttributeMap.cpp
content/base/src/nsDOMAttributeMap.h
content/base/src/nsDocument.cpp
content/base/src/nsNodeUtils.h
content/html/content/src/nsHTMLFormElement.h
dom/base/nsDOMClassInfo.cpp
--- a/content/base/src/nsDOMAttributeMap.cpp
+++ b/content/base/src/nsDOMAttributeMap.cpp
@@ -67,21 +67,20 @@ nsDOMAttributeMap::Init()
 {
   return mAttributeCache.Init();
 }
 
 /**
  * Clear map pointer for attributes.
  */
 PLDHashOperator
-RemoveMapRef(nsAttrHashKey::KeyType aKey, nsCOMPtr<nsIDOMNode>& aData, void* aUserArg)
+RemoveMapRef(nsAttrHashKey::KeyType aKey, nsRefPtr<nsDOMAttribute>& aData,
+             void* aUserArg)
 {
-  nsCOMPtr<nsIAttribute> attr(do_QueryInterface(aData));
-  NS_ASSERTION(attr, "non-nsIAttribute somehow made it into the hashmap?!");
-  attr->SetMap(nsnull);
+  aData->SetMap(nsnull);
 
   return PL_DHASH_REMOVE;
 }
 
 nsDOMAttributeMap::~nsDOMAttributeMap()
 {
   mAttributeCache.Enumerate(RemoveMapRef, nsnull);
 }
@@ -96,22 +95,23 @@ nsDOMAttributeMap::DropReference()
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsDOMAttributeMap)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsDOMAttributeMap)
   tmp->DropReference();
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 
 PLDHashOperator
-TraverseMapEntry(nsAttrHashKey::KeyType aKey, nsCOMPtr<nsIDOMNode>& aData, void* aUserArg)
+TraverseMapEntry(nsAttrHashKey::KeyType aKey, nsRefPtr<nsDOMAttribute>& aData,
+                 void* aUserArg)
 {
   nsCycleCollectionTraversalCallback *cb = 
     static_cast<nsCycleCollectionTraversalCallback*>(aUserArg);
 
-  cb->NoteXPCOMChild(aData.get());
+  cb->NoteXPCOMChild(static_cast<nsINode*>(aData.get()));
 
   return PL_DHASH_NEXT;
 }
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsDOMAttributeMap)
   tmp->mAttributeCache.Enumerate(TraverseMapEntry, &cb);
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
@@ -126,22 +126,21 @@ NS_INTERFACE_TABLE_HEAD(nsDOMAttributeMa
   NS_INTERFACE_MAP_ENTRIES_CYCLE_COLLECTION(nsDOMAttributeMap)
   NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(NamedNodeMap)
 NS_INTERFACE_MAP_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(nsDOMAttributeMap)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(nsDOMAttributeMap)
 
 PLDHashOperator
-SetOwnerDocumentFunc(nsAttrHashKey::KeyType aKey, nsCOMPtr<nsIDOMNode>& aData,
+SetOwnerDocumentFunc(nsAttrHashKey::KeyType aKey,
+                     nsRefPtr<nsDOMAttribute>& aData,
                      void* aUserArg)
 {
-  nsCOMPtr<nsIAttribute> attr(do_QueryInterface(aData));
-  NS_ASSERTION(attr, "non-nsIAttribute somehow made it into the hashmap?!");
-  nsresult rv = attr->SetOwnerDocument(static_cast<nsIDocument*>(aUserArg));
+  nsresult rv = aData->SetOwnerDocument(static_cast<nsIDocument*>(aUserArg));
 
   return NS_FAILED(rv) ? PL_DHASH_STOP : PL_DHASH_NEXT;
 }
 
 nsresult
 nsDOMAttributeMap::SetOwnerDocument(nsIDocument* aDocument)
 {
   PRUint32 n = mAttributeCache.Enumerate(SetOwnerDocumentFunc, aDocument);
@@ -149,98 +148,90 @@ nsDOMAttributeMap::SetOwnerDocument(nsID
 
   return NS_OK;
 }
 
 void
 nsDOMAttributeMap::DropAttribute(PRInt32 aNamespaceID, nsIAtom* aLocalName)
 {
   nsAttrKey attr(aNamespaceID, aLocalName);
-  nsIDOMNode *node = mAttributeCache.GetWeak(attr);
+  nsDOMAttribute *node = mAttributeCache.GetWeak(attr);
   if (node) {
-    nsCOMPtr<nsIAttribute> iAttr(do_QueryInterface(node));
-    NS_ASSERTION(iAttr, "non-nsIAttribute somehow made it into the hashmap?!");
-
     // Break link to map
-    iAttr->SetMap(nsnull);
+    node->SetMap(nsnull);
 
     // Remove from cache
     mAttributeCache.Remove(attr);
   }
 }
 
 nsresult
 nsDOMAttributeMap::RemoveAttribute(nsINodeInfo* aNodeInfo, nsIDOMNode** aReturn)
 {
   NS_ASSERTION(aNodeInfo, "RemoveAttribute() called with aNodeInfo == nsnull!");
   NS_ASSERTION(aReturn, "RemoveAttribute() called with aReturn == nsnull");
 
   *aReturn = nsnull;
 
   nsAttrKey attr(aNodeInfo->NamespaceID(), aNodeInfo->NameAtom());
 
-  if (!mAttributeCache.Get(attr, aReturn)) {
+  nsRefPtr<nsDOMAttribute> node;
+  if (!mAttributeCache.Get(attr, getter_AddRefs(node))) {
     nsAutoString value;
     // As we are removing the attribute we need to set the current value in
     // the attribute node.
     mContent->GetAttr(aNodeInfo->NamespaceID(), aNodeInfo->NameAtom(), value);
     nsCOMPtr<nsIDOMNode> newAttr = new nsDOMAttribute(nsnull, aNodeInfo, value);
     if (!newAttr) {
       return NS_ERROR_OUT_OF_MEMORY;
     }
     newAttr.swap(*aReturn);
   }
   else {
-    nsCOMPtr<nsIAttribute> iAttr(do_QueryInterface(*aReturn));
-    NS_ASSERTION(iAttr, "non-nsIAttribute somehow made it into the hashmap?!");
-
     // Break link to map
-    iAttr->SetMap(nsnull);
+    node->SetMap(nsnull);
 
     // Remove from cache
     mAttributeCache.Remove(attr);
+
+    node.forget(aReturn);
   }
 
   return NS_OK;
 }
 
-nsIDOMNode*
+nsDOMAttribute*
 nsDOMAttributeMap::GetAttribute(nsINodeInfo* aNodeInfo)
 {
   NS_ASSERTION(aNodeInfo, "GetAttribute() called with aNodeInfo == nsnull!");
 
   nsAttrKey attr(aNodeInfo->NamespaceID(), aNodeInfo->NameAtom());
 
-  nsIDOMNode* node = mAttributeCache.GetWeak(attr);
+  nsDOMAttribute* node = mAttributeCache.GetWeak(attr);
   if (!node) {
-    nsCOMPtr<nsIDOMNode> newAttr =
+    nsRefPtr<nsDOMAttribute> newAttr =
       new nsDOMAttribute(this, aNodeInfo, EmptyString());
     if (newAttr && mAttributeCache.Put(attr, newAttr)) {
       node = newAttr;
     }
   }
 
   return node;
 }
 
-nsIDOMNode*
+nsDOMAttribute*
 nsDOMAttributeMap::GetNamedItem(const nsAString& aAttrName, nsresult *aResult)
 {
   *aResult = NS_OK;
 
   if (mContent) {
     nsCOMPtr<nsINodeInfo> ni =
       mContent->GetExistingAttrNameFromQName(aAttrName);
     if (ni) {
-      nsIDOMNode* node = GetAttribute(ni);
-      if (node) {
-        return node;
-      }
-
-      *aResult = NS_ERROR_OUT_OF_MEMORY;
+      return GetAttribute(ni);
     }
   }
 
   return nsnull;
 }
 
 NS_IMETHODIMP
 nsDOMAttributeMap::GetNamedItem(const nsAString& aAttrName,
@@ -277,22 +268,23 @@ nsDOMAttributeMap::SetNamedItemInternal(
   nsresult rv = NS_OK;
   *aReturn = nsnull;
   nsCOMPtr<nsIDOMNode> tmpReturn;
 
   if (mContent) {
     // XXX should check same-origin between mContent and aNode however
     // nsContentUtils::CheckSameOrigin can't deal with attributenodes yet
     
-    nsCOMPtr<nsIDOMAttr> attribute(do_QueryInterface(aNode));
     nsCOMPtr<nsIAttribute> iAttribute(do_QueryInterface(aNode));
-    if (!attribute || !iAttribute) {
+    if (!iAttribute) {
       return NS_ERROR_DOM_HIERARCHY_REQUEST_ERR;
     }
 
+    nsDOMAttribute *attribute = static_cast<nsDOMAttribute*>(iAttribute.get());
+
     // Check that attribute is not owned by somebody else
     nsDOMAttributeMap* owner = iAttribute->GetMap();
     if (owner) {
       if (owner != this) {
         return NS_ERROR_DOM_INUSE_ATTRIBUTE_ERR;
       }
 
       // setting a preexisting attribute is a no-op, just return the same
@@ -384,45 +376,44 @@ nsDOMAttributeMap::RemoveNamedItem(const
   nsresult rv = NS_OK;
 
   if (mContent) {
     nsCOMPtr<nsINodeInfo> ni = mContent->GetExistingAttrNameFromQName(aName);
     if (!ni) {
       return NS_ERROR_DOM_NOT_FOUND_ERR;
     }
 
-    rv = GetAttribute(ni, aReturn);
-    NS_ENSURE_SUCCESS(rv, rv);
+    NS_ADDREF(*aReturn = GetAttribute(ni));
 
     // This removes the attribute node from the attribute map.
     rv = mContent->UnsetAttr(ni->NamespaceID(), ni->NameAtom(), PR_TRUE);
   }
 
   return rv;
 }
 
 
-nsIDOMNode*
+nsDOMAttribute*
 nsDOMAttributeMap::GetItemAt(PRUint32 aIndex, nsresult *aResult)
 {
   *aResult = NS_OK;
 
-  nsIDOMNode* node = nsnull;
+  nsDOMAttribute* node = nsnull;
 
   const nsAttrName* name;
   if (mContent && (name = mContent->GetAttrNameAt(aIndex))) {
     // Don't use the nodeinfo even if one exists since it can
     // have the wrong owner document.
     nsCOMPtr<nsINodeInfo> ni;
     ni = mContent->NodeInfo()->NodeInfoManager()->
       GetNodeInfo(name->LocalName(), name->GetPrefix(), name->NamespaceID());
     if (ni) {
       node = GetAttribute(ni);
     }
-    if (!node) {
+    else {
       *aResult = NS_ERROR_OUT_OF_MEMORY;
     }
   }
 
   return node;
 }
 
 NS_IMETHODIMP
@@ -488,17 +479,23 @@ nsDOMAttributeMap::GetNamedItemNSInterna
 
     if (nameSpaceID == attrNS &&
         nameAtom->Equals(aLocalName)) {
       nsCOMPtr<nsINodeInfo> ni;
       ni = mContent->NodeInfo()->NodeInfoManager()->
         GetNodeInfo(nameAtom, name->GetPrefix(), nameSpaceID);
       NS_ENSURE_TRUE(ni, NS_ERROR_OUT_OF_MEMORY);
 
-      return aRemove ? RemoveAttribute(ni, aReturn) : GetAttribute(ni, aReturn);
+      if (aRemove) {
+        return RemoveAttribute(ni, aReturn);
+      }
+
+      NS_ADDREF(*aReturn = GetAttribute(ni));
+
+      return NS_OK;
     }
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDOMAttributeMap::RemoveNamedItemNS(const nsAString& aNamespaceURI,
--- a/content/base/src/nsDOMAttributeMap.h
+++ b/content/base/src/nsDOMAttributeMap.h
@@ -40,17 +40,17 @@
  * Implementation of the |attributes| property of DOM Core's nsIDOMNode object.
  */
 
 #ifndef nsDOMAttributeMap_h___
 #define nsDOMAttributeMap_h___
 
 #include "nsIDOMNamedNodeMap.h"
 #include "nsString.h"
-#include "nsInterfaceHashtable.h"
+#include "nsRefPtrHashtable.h"
 #include "nsCycleCollectionParticipant.h"
 #include "prbit.h"
 #include "nsIDOMNode.h"
 
 class nsIAtom;
 class nsIContent;
 class nsDOMAttribute;
 class nsINodeInfo;
@@ -155,28 +155,28 @@ public:
    * Returns the number of attribute nodes currently in the map.
    * Note: this is just the number of cached attribute nodes, not the number of
    * attributes in mContent.
    *
    * @return The number of attribute nodes in the map.
    */
   PRUint32 Count() const;
 
-  typedef nsInterfaceHashtable<nsAttrHashKey, nsIDOMNode> AttrCache;
+  typedef nsRefPtrHashtable<nsAttrHashKey, nsDOMAttribute> AttrCache;
 
   /**
    * Enumerates over the attribute nodess in the map and calls aFunc for each
    * one. If aFunc returns PL_DHASH_STOP we'll stop enumerating at that point.
    *
    * @return The number of attribute nodes that aFunc was called for.
    */
   PRUint32 Enumerate(AttrCache::EnumReadFunction aFunc, void *aUserArg) const;
 
-  nsIDOMNode* GetItemAt(PRUint32 aIndex, nsresult *rv);
-  nsIDOMNode* GetNamedItem(const nsAString& aAttrName, nsresult *rv);
+  nsDOMAttribute* GetItemAt(PRUint32 aIndex, nsresult *rv);
+  nsDOMAttribute* GetNamedItem(const nsAString& aAttrName, nsresult *rv);
 
   static nsDOMAttributeMap* FromSupports(nsISupports* aSupports)
   {
 #ifdef DEBUG
     {
       nsCOMPtr<nsIDOMNamedNodeMap> map_qi = do_QueryInterface(aSupports);
 
       // If this assertion fires the QI implementation for the object in
@@ -212,34 +212,17 @@ private:
    * GetNamedItemNS() implementation taking |aRemove| for GetAttribute(),
    * which is used by RemoveNamedItemNS().
    */
   nsresult GetNamedItemNSInternal(const nsAString& aNamespaceURI,
                                   const nsAString& aLocalName,
                                   nsIDOMNode** aReturn,
                                   PRBool aRemove = PR_FALSE);
 
-  /**
-   * Returns an attribute, either by retrieving it from the cache or by
-   * creating a new one.
-   */
-  nsresult GetAttribute(nsINodeInfo*     aNodeInfo,
-                        nsIDOMNode**     aReturn)
-  {
-    *aReturn = GetAttribute(aNodeInfo);
-    if (!*aReturn) {
-      return NS_ERROR_OUT_OF_MEMORY;
-    }
-
-    NS_ADDREF(*aReturn);
-
-    return NS_OK;
-  }
-
-  nsIDOMNode* GetAttribute(nsINodeInfo*     aNodeInfo);
+  nsDOMAttribute* GetAttribute(nsINodeInfo* aNodeInfo);
 
   /**
    * Remove an attribute, returns the removed node.
    */
   nsresult RemoveAttribute(nsINodeInfo*     aNodeInfo,
                            nsIDOMNode**     aReturn);
 };
 
--- a/content/base/src/nsDocument.cpp
+++ b/content/base/src/nsDocument.cpp
@@ -5614,22 +5614,22 @@ nsDocument::SetDocumentURI(const nsAStri
   // Not allowing this yet, need to think about security ramifications first.
   // We use mDocumentURI to get principals for this document.
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 static void BlastSubtreeToPieces(nsINode *aNode);
 
 PLDHashOperator
-BlastFunc(nsAttrHashKey::KeyType aKey, nsIDOMNode *aData, void* aUserArg)
+BlastFunc(nsAttrHashKey::KeyType aKey, nsDOMAttribute *aData, void* aUserArg)
 {
   nsCOMPtr<nsIAttribute> *attr =
     static_cast<nsCOMPtr<nsIAttribute>*>(aUserArg);
 
-  *attr = do_QueryInterface(aData);
+  *attr = aData;
 
   NS_ASSERTION(attr->get(),
                "non-nsIAttribute somehow made it into the hashmap?!");
 
   return PL_DHASH_STOP;
 }
 
 static void
--- a/content/base/src/nsNodeUtils.h
+++ b/content/base/src/nsNodeUtils.h
@@ -33,24 +33,23 @@
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #ifndef nsNodeUtils_h___
 #define nsNodeUtils_h___
 
-#include "nsDOMAttributeMap.h"
-#include "nsIDOMNode.h"
 #include "nsINode.h"
 
 struct CharacterDataChangeInfo;
 struct JSContext;
 struct JSObject;
 class nsIVariant;
+class nsIDOMNode;
 class nsIDOMUserDataHandler;
 template<class E> class nsCOMArray;
 class nsCycleCollectionTraversalCallback;
 
 class nsNodeUtils
 {
 public:
   /**
@@ -245,19 +244,16 @@ public:
   /**
    * Release the UserData and UserDataHandlers for aNode.
    *
    * @param aNode the node to release the UserData and UserDataHandlers for
    */
   static void UnlinkUserData(nsINode *aNode);
 
 private:
-  friend PLDHashOperator
-    AdoptFunc(nsAttrHashKey::KeyType aKey, nsIDOMNode *aData, void* aUserArg);
-
   /**
    * Walks aNode, its attributes and, if aDeep is PR_TRUE, its descendant nodes.
    * If aClone is PR_TRUE the nodes will be cloned. If aNewNodeInfoManager is
    * not null, it is used to create new nodeinfos for the nodes. Also reparents
    * the XPConnect wrappers for the nodes in aNewScope if aCx is not null.
    * aNodesWithProperties will be filled with all the nodes that have
    * properties.
    *
--- a/content/html/content/src/nsHTMLFormElement.h
+++ b/content/html/content/src/nsHTMLFormElement.h
@@ -43,16 +43,17 @@
 #include "nsIDOMNSHTMLFormElement.h"
 #include "nsIWebProgressListener.h"
 #include "nsIRadioGroupContainer.h"
 #include "nsIURI.h"
 #include "nsIWeakReferenceUtils.h"
 #include "nsPIDOMWindow.h"
 #include "nsUnicharUtils.h"
 #include "nsThreadUtils.h"
+#include "nsInterfaceHashtable.h"
 
 class nsFormControlList;
 
 /**
  * hashkey wrapper using nsAString KeyType
  *
  * @see nsTHashtable::EntryType for specification
  */
--- a/dom/base/nsDOMClassInfo.cpp
+++ b/dom/base/nsDOMClassInfo.cpp
@@ -209,17 +209,17 @@
 // includes needed for the prototype chain interfaces
 #include "nsIDOMNavigator.h"
 #include "nsIDOMBarProp.h"
 #include "nsIDOMScreen.h"
 #include "nsIDOMDocumentType.h"
 #include "nsIDOMDOMImplementation.h"
 #include "nsIDOMDocumentFragment.h"
 #include "nsIDOMDocumentEvent.h"
-#include "nsIDOMAttr.h"
+#include "nsDOMAttribute.h"
 #include "nsIDOMText.h"
 #include "nsIDOM3Text.h"
 #include "nsIDOMComment.h"
 #include "nsIDOMCDATASection.h"
 #include "nsIDOMProcessingInstruction.h"
 #include "nsIDOMNotation.h"
 #include "nsIDOMNSEvent.h"
 #include "nsIDOMDataContainerEvent.h"
@@ -8356,26 +8356,28 @@ nsNamedArraySH::GetProperty(nsIXPConnect
 // NamedNodeMap helper
 
 nsISupports*
 nsNamedNodeMapSH::GetItemAt(nsISupports *aNative, PRUint32 aIndex,
                             nsresult *aResult)
 {
   nsDOMAttributeMap* map = nsDOMAttributeMap::FromSupports(aNative);
 
-  return map->GetItemAt(aIndex, aResult);
+  nsINode *attr = map->GetItemAt(aIndex, aResult);
+  return attr;
 }
 
 nsISupports*
 nsNamedNodeMapSH::GetNamedItem(nsISupports *aNative, const nsAString& aName,
                                nsresult *aResult)
 {
   nsDOMAttributeMap* map = nsDOMAttributeMap::FromSupports(aNative);
 
-  return map->GetNamedItem(aName, aResult);
+  nsINode *attr = map->GetNamedItem(aName, aResult);
+  return attr;
 }
 
 
 // HTMLCollection helper
 
 nsresult
 nsHTMLCollectionSH::GetLength(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
                               JSObject *obj, PRUint32 *length)