Bug 1187782 (part 5) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators. r=khuey.
authorNicholas Nethercote <nnethercote@mozilla.com>
Tue, 27 Oct 2015 15:13:04 -0700
changeset 305267 1e2e66bf01dbbf7d5018185e925d975aa6189786
parent 305266 40e5654d4bdd3ed4939de46a7ad89288285c6039
child 305268 0c4702fbc9268041176571a4ba1decf79aa33621
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey
bugs1187782
milestone44.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 1187782 (part 5) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators. r=khuey.
dom/base/nsDOMAttributeMap.cpp
dom/base/nsDOMAttributeMap.h
dom/base/nsDocument.cpp
--- a/dom/base/nsDOMAttributeMap.cpp
+++ b/dom/base/nsDOMAttributeMap.cpp
@@ -552,23 +552,16 @@ nsDOMAttributeMap::RemoveNamedItemNS(con
 }
 
 uint32_t
 nsDOMAttributeMap::Count() const
 {
   return mAttributeCache.Count();
 }
 
-uint32_t
-nsDOMAttributeMap::Enumerate(AttrCache::EnumReadFunction aFunc,
-                             void *aUserArg) const
-{
-  return mAttributeCache.EnumerateRead(aFunc, aUserArg);
-}
-
 size_t
 nsDOMAttributeMap::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const
 {
   size_t n = aMallocSizeOf(this);
 
   n += mAttributeCache.ShallowSizeOfExcludingThis(aMallocSizeOf);
   for (auto iter = mAttributeCache.ConstIter(); !iter.Done(); iter.Next()) {
     n += aMallocSizeOf(iter.Data().get());
--- a/dom/base/nsDOMAttributeMap.h
+++ b/dom/base/nsDOMAttributeMap.h
@@ -123,23 +123,17 @@ public:
    * attributes in mContent.
    *
    * @return The number of attribute nodes in the map.
    */
   uint32_t Count() const;
 
   typedef nsRefPtrHashtable<nsAttrHashKey, Attr> 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.
-   */
-  uint32_t Enumerate(AttrCache::EnumReadFunction aFunc, void *aUserArg) const;
+  static void BlastSubtreeToPieces(nsINode *aNode);
 
   Element* GetParentObject() const
   {
     return mContent;
   }
   virtual JSObject* WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
 
   // WebIDL
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -7628,52 +7628,46 @@ nsIDocument::GetCompatMode(nsString& aCo
 
   if (mCompatMode == eCompatibility_NavQuirks) {
     aCompatMode.AssignLiteral("BackCompat");
   } else {
     aCompatMode.AssignLiteral("CSS1Compat");
   }
 }
 
-static void BlastSubtreeToPieces(nsINode *aNode);
-
-PLDHashOperator
-BlastFunc(nsAttrHashKey::KeyType aKey, Attr *aData, void* aUserArg)
-{
-  nsCOMPtr<nsIAttribute> *attr =
-    static_cast<nsCOMPtr<nsIAttribute>*>(aUserArg);
-
-  *attr = aData;
-
-  NS_ASSERTION(attr->get(),
-               "non-nsIAttribute somehow made it into the hashmap?!");
-
-  return PL_DHASH_STOP;
-}
-
-static void
-BlastSubtreeToPieces(nsINode *aNode)
+void
+nsDOMAttributeMap::BlastSubtreeToPieces(nsINode *aNode)
 {
   if (aNode->IsElement()) {
     Element *element = aNode->AsElement();
     const nsDOMAttributeMap *map = element->GetAttributeMap();
     if (map) {
       nsCOMPtr<nsIAttribute> attr;
-      while (map->Enumerate(BlastFunc, &attr) > 0) {
+
+      // This non-standard style of iteration is presumably used because some
+      // of the code in the loop body can trigger element removal, which
+      // invalidates the iterator.
+      while (true) {
+        auto iter = map->mAttributeCache.ConstIter();
+        if (iter.Done()) {
+          break;
+        }
+        nsCOMPtr<nsIAttribute> attr = iter.UserData();
+        NS_ASSERTION(attr.get(),
+                     "non-nsIAttribute somehow made it into the hashmap?!");
+
         BlastSubtreeToPieces(attr);
 
-#ifdef DEBUG
-        nsresult rv =
-#endif
+        DebugOnly<nsresult> rv =
           element->UnsetAttr(attr->NodeInfo()->NamespaceID(),
                              attr->NodeInfo()->NameAtom(),
                              false);
 
         // XXX Should we abort here?
-        NS_ASSERTION(NS_SUCCEEDED(rv), "Uhoh, UnsetAttr shouldn't fail!");
+        NS_ASSERTION(NS_SUCCEEDED(rv), "Uh-oh, UnsetAttr shouldn't fail!");
       }
     }
   }
 
   uint32_t count = aNode->GetChildCount();
   for (uint32_t i = 0; i < count; ++i) {
     BlastSubtreeToPieces(aNode->GetFirstChild());
     aNode->RemoveChildAt(0, false);
@@ -7830,17 +7824,17 @@ nsIDocument::AdoptNode(nsINode& aAdopted
   }
 
   nsCOMArray<nsINode> nodesWithProperties;
   rv = nsNodeUtils::Adopt(adoptedNode, sameDocument ? nullptr : mNodeInfoManager,
                           newScope, nodesWithProperties);
   if (rv.Failed()) {
     // Disconnect all nodes from their parents, since some have the old document
     // as their ownerDocument and some have this as their ownerDocument.
-    BlastSubtreeToPieces(adoptedNode);
+    nsDOMAttributeMap::BlastSubtreeToPieces(adoptedNode);
 
     if (!sameDocument && oldDocument) {
       uint32_t count = nodesWithProperties.Count();
       for (uint32_t j = 0; j < oldDocument->GetPropertyTableCount(); ++j) {
         for (uint32_t i = 0; i < count; ++i) {
           // Remove all properties.
           oldDocument->PropertyTable(j)->
             DeleteAllPropertiesFor(nodesWithProperties[i]);
@@ -7859,17 +7853,17 @@ nsIDocument::AdoptNode(nsINode& aAdopted
       for (uint32_t i = 0; i < count; ++i) {
         rv = oldTable->TransferOrDeleteAllPropertiesFor(nodesWithProperties[i],
                                                         newTable);
       }
     }
 
     if (rv.Failed()) {
       // Disconnect all nodes from their parents.
-      BlastSubtreeToPieces(adoptedNode);
+      nsDOMAttributeMap::BlastSubtreeToPieces(adoptedNode);
 
       return nullptr;
     }
   }
 
   NS_ASSERTION(adoptedNode->OwnerDoc() == this,
                "Should still be in the document we just got adopted into");