Bug 568188: Reduce refcounting by making the id-table hold weak referenes. r=smaug a=jst
authorJonas Sicking <jonas@sicking.cc>
Wed, 01 Sep 2010 15:48:24 -0700
changeset 51858 e1b3964e946cc4bdad612aa108622cb357a882af
parent 51857 bf04d87edcba1b3aa89e8d8cb5b682b25a4246db
child 51859 ed6440d2f0c9cc55e122e1592a21be34405eaceb
push idunknown
push userunknown
push dateunknown
reviewerssmaug, jst
bugs568188
milestone2.0b6pre
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 568188: Reduce refcounting by making the id-table hold weak referenes. r=smaug a=jst
content/base/src/nsDocument.cpp
content/base/src/nsDocument.h
content/base/src/nsGenericElement.cpp
content/base/src/nsGenericElement.h
content/base/src/nsNodeUtils.cpp
content/base/src/nsStyledElement.cpp
content/base/test/test_bug564863.xhtml
content/xml/content/src/nsXMLElement.cpp
content/xml/content/src/nsXMLElement.h
content/xtf/src/nsXTFElementWrapper.h
content/xul/content/src/nsXULElement.cpp
--- a/content/base/src/nsDocument.cpp
+++ b/content/base/src/nsDocument.cpp
@@ -211,41 +211,30 @@ static PRLogModuleInfo* gCspPRLog;
 
 #define NAME_NOT_VALID ((nsBaseContentList*)1)
 
 nsIdentifierMapEntry::~nsIdentifierMapEntry()
 {
   if (mNameContentList && mNameContentList != NAME_NOT_VALID) {
     NS_RELEASE(mNameContentList);
   }
-
-  for (PRInt32 i = 0; i < mIdContentList.Count(); ++i) {
-    nsIContent* content = static_cast<nsIContent*>(mIdContentList[i]);
-    NS_RELEASE(content);
-  }
 }
 
 void
 nsIdentifierMapEntry::Traverse(nsCycleCollectionTraversalCallback* aCallback)
 {
   if (mNameContentList != NAME_NOT_VALID) {
     NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(*aCallback,
                                        "mIdentifierMap mNameContentList");
     aCallback->NoteXPCOMChild(static_cast<nsIDOMNodeList*>(mNameContentList));
   }
 
   NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(*aCallback, "mIdentifierMap mDocAllList");
   aCallback->NoteXPCOMChild(static_cast<nsIDOMNodeList*>(mDocAllList));
 
-  for (PRInt32 i = 0; i < mIdContentList.Count(); ++i) {
-    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(*aCallback,
-                                       "mIdentifierMap mIdContentList element");
-    aCallback->NoteXPCOMChild(static_cast<nsIContent*>(mIdContentList[i]));
-  }
-
   if (mImageElement) {
     NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(*aCallback,
                                        "mIdentifierMap mImageElement element");
     nsIContent* imageElement = mImageElement;
     aCallback->NoteXPCOMChild(imageElement);
   }
 }
 
@@ -368,17 +357,16 @@ nsIdentifierMapEntry::AddIdElement(Eleme
   Element* currentElement =
     static_cast<Element*>(mIdContentList.SafeElementAt(0));
 #endif
 
   // Common case
   if (mIdContentList.Count() == 0) {
     if (!mIdContentList.AppendElement(aElement))
       return PR_FALSE;
-    NS_ADDREF(aElement);
     NS_ASSERTION(currentElement == nsnull, "How did that happen?");
     FireChangeCallbacks(nsnull, aElement);
     return PR_TRUE;
   }
 
   // We seem to have multiple content nodes for the same id, or XUL is messing
   // with us.  Search for the right place to insert the content.
   PRInt32 start = 0;
@@ -401,45 +389,51 @@ nsIdentifierMapEntry::AddIdElement(Eleme
       end = cur;
     } else {
       start = cur + 1;
     }
   } while (start != end);
 
   if (!mIdContentList.InsertElementAt(aElement, start))
     return PR_FALSE;
-  NS_ADDREF(aElement);
+
   if (start == 0) {
     Element* oldElement =
       static_cast<Element*>(mIdContentList.SafeElementAt(1));
     NS_ASSERTION(currentElement == oldElement, "How did that happen?");
     FireChangeCallbacks(oldElement, aElement);
   }
   return PR_TRUE;
 }
 
 void
 nsIdentifierMapEntry::RemoveIdElement(Element* aElement)
 {
+  NS_PRECONDITION(aElement, "Missing element");
+
   // This should only be called while the document is in an update.
   // Assertions near the call to this method guarantee this.
 
+  // This could fire in OOM situations
+  // Only assert this in HTML documents for now as XUL does all sorts of weird
+  // crap.
+  NS_ASSERTION(!aElement->GetOwnerDoc() ||
+               !aElement->GetOwnerDoc()->IsHTML() ||
+               mIdContentList.IndexOf(aElement) >= 0,
+               "Removing id entry that doesn't exist");
+
   // XXXbz should this ever Compact() I guess when all the content is gone
   // we'll just get cleaned up in the natural order of things...
   Element* currentElement =
     static_cast<Element*>(mIdContentList.SafeElementAt(0));
-  if (!mIdContentList.RemoveElement(aElement))
-    return;
+  mIdContentList.RemoveElement(aElement);
   if (currentElement == aElement) {
     FireChangeCallbacks(currentElement,
                         static_cast<Element*>(mIdContentList.SafeElementAt(0)));
   }
-  // Make sure the release happens after the check above, since it'll
-  // null out aContent.
-  NS_RELEASE(aElement);
 }
 
 void
 nsIdentifierMapEntry::SetImageElement(Element* aElement)
 {
   Element* oldElement = GetImageIdElement();
   mImageElement = aElement;
   Element* newElement = GetImageIdElement();
--- a/content/base/src/nsDocument.h
+++ b/content/base/src/nsDocument.h
@@ -252,17 +252,17 @@ public:
     ChangeCallback mKey;
   };
 
 private:
   void FireChangeCallbacks(Element* aOldElement, Element* aNewElement,
                            PRBool aImageOnly = PR_FALSE);
 
   // empty if there are no elementswith this ID.
-  // The elementsnodes are stored addrefed.
+  // The elements are stored as weak pointers.
   nsSmallVoidArray mIdContentList;
   // NAME_NOT_VALID if this id cannot be used as a 'name'.  Otherwise
   // stores Elements.
   nsBaseContentList *mNameContentList;
   nsRefPtr<nsContentList> mDocAllList;
   nsAutoPtr<nsTHashtable<ChangeCallbackEntry> > mChangeCallbacks;
   nsCOMPtr<Element> mImageElement;
 };
--- a/content/base/src/nsGenericElement.cpp
+++ b/content/base/src/nsGenericElement.cpp
@@ -2170,17 +2170,22 @@ nsGenericElement::SetPrefix(const nsAStr
     return NS_ERROR_DOM_NAMESPACE_ERR;
   }
 
   nsCOMPtr<nsINodeInfo> newNodeInfo;
   nsresult rv = nsContentUtils::PrefixChanged(mNodeInfo, prefix,
                                               getter_AddRefs(newNodeInfo));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  mNodeInfo = newNodeInfo;
+  mNodeInfo.swap(newNodeInfo);
+  NodeInfoChanged(newNodeInfo);
+
+  // The id-handling code need to react to unexpected changes to an elements
+  // nodeinfo as that can change the elements id-attribute.
+  nsMutationGuard::DidMutate();
 
   return NS_OK;
 }
 
 static already_AddRefed<nsIDOMNSFeatureFactory>
 GetDOMFeatureFactory(const nsAString& aFeature, const nsAString& aVersion)
 {
   nsIDOMNSFeatureFactory *factory = nsnull;
@@ -3565,16 +3570,18 @@ nsINode::doInsertChildAt(nsIContent* aKi
 
       NS_ASSERTION(adoptedKid == kid, "Uh, adopt node changed nodes?");
     }
   }
 
   PRUint32 childCount = aChildArray.ChildCount();
   NS_ENSURE_TRUE(aIndex <= childCount, NS_ERROR_ILLEGAL_VALUE);
 
+  // The id-handling code, and in the future possibly other code, need to
+  // react to unexpected attribute changes.
   nsMutationGuard::DidMutate();
 
   PRBool isAppend = (aIndex == childCount);
 
   nsIDocument* doc = GetCurrentDoc();
   mozAutoDocUpdate updateBatch(doc, UPDATE_CONTENT_MODEL, aNotify);
 
   rv = aChildArray.InsertChildAt(aKid, aIndex);
@@ -4633,16 +4640,18 @@ nsGenericElement::SetAttrAndNotify(PRInt
 
   // When notifying, make sure to keep track of states whose value
   // depends solely on the value of an attribute.
   PRUint32 stateMask;
   if (aNotify) {
     stateMask = PRUint32(IntrinsicState());
   }
 
+  nsMutationGuard::DidMutate();
+
   if (aNamespaceID == kNameSpaceID_None) {
     // XXXbz Perhaps we should push up the attribute mapping function
     // stuff to nsGenericElement?
     if (!IsAttributeMapped(aName) ||
         !SetMappedAttribute(document, aName, aParsedValue, &rv)) {
       rv = mAttrsAndChildren.SetAndTakeAttr(aName, aParsedValue);
     }
   }
@@ -4890,16 +4899,20 @@ nsGenericElement::UnsetAttr(PRInt32 aNam
   }
 
   // Clear binding to nsIDOMNamedNodeMap
   nsDOMSlots *slots = GetExistingDOMSlots();
   if (slots && slots->mAttributeMap) {
     slots->mAttributeMap->DropAttribute(aNameSpaceID, aName);
   }
 
+  // The id-handling code, and in the future possibly other code, need to
+  // react to unexpected attribute changes.
+  nsMutationGuard::DidMutate();
+
   nsAttrValue oldValue;
   rv = mAttrsAndChildren.RemoveAttrAt(index, oldValue);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (document || HasFlag(NODE_FORCE_XBL_BINDINGS)) {
     nsIDocument* ownerDoc = GetOwnerDoc();
     if (ownerDoc) {
       nsRefPtr<nsXBLBinding> binding =
--- a/content/base/src/nsGenericElement.h
+++ b/content/base/src/nsGenericElement.h
@@ -757,16 +757,20 @@ public:
    * live on this element (and is only virtual to handle XUL prototypes).  That
    * is, this should only be called from methods that only care about attrs
    * that effectively live in mAttrsAndChildren.
    */
   virtual nsAttrInfo GetAttrInfo(PRInt32 aNamespaceID, nsIAtom* aName) const;
 
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(nsGenericElement)
 
+  virtual void NodeInfoChanged(nsINodeInfo* aOldNodeInfo)
+  {
+  }
+
 protected:
   /**
    * Set attribute and (if needed) notify documentobservers and fire off
    * mutation events.  This will send the AttributeChanged notification.
    * Callers of this method are responsible for calling AttributeWillChange,
    * since that needs to happen before the new attr value has been set, and
    * in particular before it has been parsed.
    *
--- a/content/base/src/nsNodeUtils.cpp
+++ b/content/base/src/nsNodeUtils.cpp
@@ -490,16 +490,19 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNod
     PRBool wasRegistered = PR_FALSE;
     if (oldDoc && aNode->IsElement()) {
       Element* element = aNode->AsElement();
       oldDoc->ClearBoxObjectFor(element);
       wasRegistered = oldDoc->UnregisterFreezableElement(element);
     }
 
     aNode->mNodeInfo.swap(newNodeInfo);
+    if (elem) {
+      elem->NodeInfoChanged(newNodeInfo);
+    }
 
     nsIDocument* newDoc = aNode->GetOwnerDoc();
     if (newDoc) {
       // XXX what if oldDoc is null, we don't know if this should be
       // registered or not! Can that really happen?
       if (wasRegistered) {
         newDoc->RegisterFreezableElement(aNode->AsElement());
       }
--- a/content/base/src/nsStyledElement.cpp
+++ b/content/base/src/nsStyledElement.cpp
@@ -132,20 +132,23 @@ nsStyledElement::UnsetAttr(PRInt32 aName
                            PRBool aNotify)
 {
   PRBool isId = PR_FALSE;
   if (aAttribute == nsGkAtoms::id && aNameSpaceID == kNameSpaceID_None) {
     // Have to do this before clearing flag. See RemoveFromIdTable
     RemoveFromIdTable();
     isId = PR_TRUE;
   }
+
+  nsMutationGuard guard;
   
   nsresult rv = nsGenericElement::UnsetAttr(aNameSpaceID, aAttribute, aNotify);
 
-  if (isId) {
+  if (isId &&
+      (!guard.Mutated(0) || !HasAttr(kNameSpaceID_None, nsGkAtoms::id))) {
     UnsetFlags(NODE_HAS_ID);
   }
 
   return rv;
 }
 
 NS_IMETHODIMP
 nsStyledElement::SetInlineStyleRule(nsICSSStyleRule* aStyleRule, PRBool aNotify)
--- a/content/base/test/test_bug564863.xhtml
+++ b/content/base/test/test_bug564863.xhtml
@@ -254,59 +254,59 @@ removeNode(svg);
 removeNode(nsx);
 
 checkHasIdNoGEBI("removed node");
 
 // Check that removing an element during UnsetAttr works
 is(div.id, "div_id", "div still has id set");
 var mutateFired = false;
 root.appendChild(div);
-var f = function(e) {
-  div.removeEventListener("DOMAttrModified", f, false);
+div.addEventListener("DOMAttrModified", function(e) {
+  div.removeEventListener("DOMAttrModified", arguments.callee, false);
   is(e.target, div, "target is div");
   is(div.id, "", "div no longer has id");
   is(div.getAttribute("id"), null, "div no longer has id attr");
   removeNode(div);
   is(div.parentNode, null, "div was removed");
   mutateFired = true;
-}
-div.addEventListener("DOMAttrModified", f, false);
+}, false);
 div.removeAttribute("id");
 ok(mutateFired, "mutation event fired");
 
 // Check same for XML elements
 is(nsx.getAttribute("id"), "ns_id", "nsx still has id set");
 mutateFired = false;
 root.appendChild(nsx);
-var f2 = function(e) {
-  nsx.removeEventListener("DOMAttrModified", f2, false);
+nsx.addEventListener("DOMAttrModified", function(e) {
+  nsx.removeEventListener("DOMAttrModified", arguments.callee, false);
   is(e.target, nsx, "target is nsx");
   is(nsx.getAttribute("id"), null, "nsx no longer has id attr");
   removeNode(nsx);
   is(nsx.parentNode, null, "nsx was removed");
   mutateFired = true;
-}
-nsx.addEventListener("DOMAttrModified", f2, false);
+}, false);
 nsx.removeAttribute("id");
 ok(mutateFired, "mutation event fired");
 
 
 // Check that modifying the id-name of an XML element works
 ok($("ns-holder"), "ns-holder exists");
 ok($("ns2-holder"), "ns2-holder exists");
 root.appendChild(nsx);
 nsx.setAttribute("id", "ns_id");
 is(nsx_cs.color, "rgb(50, 50, 50)", "nsx has initial id");
 is($("ns_id"), nsx, "gEBI works on old id");
 nsx.setAttribute("id_2", "ns2_id");
 nsx.prefix = "ns2";
+is($("ns2_id"), nsx, "gEBI works on new id");
+is($("ns_id"), null, "gEBI does't work on old id");
 removeNode(nsx);
 root.appendChild(nsx);
 is(nsx_cs.color, "rgb(60, 60, 60)", "nsx has new id");
-is($("ns2_id"), nsx, "gEBI works on new id");
+is($("ns2_id"), nsx, "gEBI still works on new id");
 
 // Need to change id and then CC to ensure that we don't end up with dangling pointers
 function gc() {
   netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
   window.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
         .getInterface(Components.interfaces.nsIDOMWindowUtils)
         .garbageCollect();
 }
@@ -319,13 +319,106 @@ is($("tempNode_id"), tempNode, "tempNode
 tempNode.prefix = "ns2";
 removeNode(tempNode);
 tempNode = null;
 gc();
 $("tempNode_id");
 ok(true, "Didn't crash when accessing old id");
 
 
+// Update the id-name inside a mutation event
+removeNode(nsx);
+nsx = document.createElementNS("urn:namespace", "ns:x");
+nsx.setAttribute("id", "ns_id");
+nsx.setAttribute("id_2", "ns2_id");
+root.appendChild(nsx);
+is($("ns_id"), nsx, "new nsx is set up");
+is($("ns2_id"), null, "ns2_id doesn't exist");
+mutateFired = false;
+nsx.addEventListener("DOMAttrModified", function(e) {
+  nsx.removeEventListener("DOMAttrModified", arguments.callee, false);
+  is(e.target, nsx, "target is nsx");
+  is(nsx.getAttribute("id"), null, "nsx no longer has id attr");
+  nsx.prefix = "ns2";
+  mutateFired = true;
+}, false);
+nsx.removeAttribute("id");
+ok(mutateFired, "mutation event fired");
+is($("ns_id"), null, "ns_id was removed from table");
+is($("ns2_id"), nsx, "ns2_id was added");
+removeNode(nsx);
+is($("ns2_id"), null, "ns2_id was removed");
+nsx = null;
+gc();
+$("ns2_id");
+ok(true, "we didn't crash");
+
+// Re-add the id inside a mutation event on a XML element
+is($("ns_id"), null, "no nsx");
+is($("ns2_id"), null, "no nsx");
+nsx = document.createElementNS("urn:namespace", "ns:x");
+nsx.setAttribute("id", "ns_id");
+root.appendChild(nsx);
+is($("ns_id"), nsx, "new nsx is set up");
+mutateFired = false;
+nsx.addEventListener("DOMAttrModified", function(e) {
+  nsx.removeEventListener("DOMAttrModified", arguments.callee, false);
+  is(e.target, nsx, "target is nsx");
+  is(nsx.getAttribute("id"), null, "nsx no longer has id attr");
+  nsx.setAttribute("id", "other_id");
+  mutateFired = true;
+}, false);
+nsx.removeAttribute("id");
+ok(mutateFired, "mutation event fired");
+is($("ns_id"), null, "ns_id was removed from table");
+is($("other_id"), nsx, "other_id was added");
+removeNode(nsx);
+is($("other_id"), null, "other_id was removed");
+
+// Re-add the id inside a mutation event on a HTML element
+is($("div_id"), null, "no div");
+div = document.createElement("div");
+div.id = "div_id";
+root.appendChild(div);
+is($("div_id"), div, "new div is set up");
+mutateFired = false;
+div.addEventListener("DOMAttrModified", function(e) {
+  div.removeEventListener("DOMAttrModified", arguments.callee, false);
+  is(e.target, div, "target is div");
+  is(div.getAttribute("id"), null, "div no longer has id attr");
+  is(div.id, "", "div no longer has id");
+  div.id = "other_div_id";
+  mutateFired = true;
+}, false);
+div.removeAttribute("id");
+ok(mutateFired, "mutation event fired");
+is($("div_id"), null, "div_id was removed from table");
+is($("other_div_id"), div, "other_div_id was added");
+removeNode(div);
+is($("other_div_id"), null, "other_div_id was removed");
+
+// Re-add the id inside a mutation event on a XUL element
+is($("xul_id"), null, "no xul");
+xul = document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul", "button");
+xul.id = "xul_id";
+root.appendChild(xul);
+is($("xul_id"), xul, "new xul is set up");
+mutateFired = false;
+xul.addEventListener("DOMAttrModified", function(e) {
+  xul.removeEventListener("DOMAttrModified", arguments.callee, false);
+  is(e.target, xul, "target is xul");
+  is(xul.getAttribute("id"), "", "xul no longer has id attr");
+  is(xul.id, "", "xul no longer has id");
+  xul.id = "other_xul_id";
+  mutateFired = true;
+}, false);
+xul.removeAttribute("id");
+ok(mutateFired, "mutation event fired");
+is($("xul_id"), null, "xul_id was removed from table");
+is($("other_xul_id"), xul, "other_xul_id was added");
+removeNode(xul);
+is($("other_xul_id"), null, "other_xul_id was removed");
+
 ]]>
 </script>
 </pre>
 </body>
 </html>
--- a/content/xml/content/src/nsXMLElement.cpp
+++ b/content/xml/content/src/nsXMLElement.cpp
@@ -74,20 +74,25 @@ nsXMLElement::UnsetAttr(PRInt32 aNameSpa
 {
   PRBool isId = PR_FALSE;
   if (aAttribute == GetIDAttributeName() &&
       aNameSpaceID == kNameSpaceID_None) {
     // Have to do this before clearing flag. See RemoveFromIdTable
     RemoveFromIdTable();
     isId = PR_TRUE;
   }
-  
+
+  nsMutationGuard guard;
+
   nsresult rv = nsGenericElement::UnsetAttr(aNameSpaceID, aAttribute, aNotify);
-  
-  if (isId) {
+
+  if (isId &&
+      (!guard.Mutated(0) ||
+       !mNodeInfo->GetIDAttributeAtom() ||
+       !HasAttr(kNameSpaceID_None, GetIDAttributeName()))) {
     UnsetFlags(NODE_HAS_ID);
   }
   
   return rv;
 }
 
 nsIAtom *
 nsXMLElement::GetIDAttributeName() const
@@ -95,39 +100,56 @@ nsXMLElement::GetIDAttributeName() const
   return mNodeInfo->GetIDAttributeAtom();
 }
 
 nsIAtom*
 nsXMLElement::DoGetID() const
 {
   NS_ASSERTION(HasFlag(NODE_HAS_ID), "Unexpected call");
 
+  const nsAttrValue* attrVal = mAttrsAndChildren.GetAttr(GetIDAttributeName());
+  return attrVal ? attrVal->GetAtomValue() : nsnull;
+}
+
+void
+nsXMLElement::NodeInfoChanged(nsINodeInfo* aOldNodeInfo)
+{
+  NS_ASSERTION(!IsInDoc() ||
+               aOldNodeInfo->GetDocument() == mNodeInfo->GetDocument(),
+               "Can only change document if we're not inside one");
+  nsIDocument* doc = GetCurrentDoc();
+
+  if (HasFlag(NODE_HAS_ID) && doc) {
+    const nsAttrValue* attrVal =
+      mAttrsAndChildren.GetAttr(aOldNodeInfo->GetIDAttributeAtom());
+    if (attrVal) {
+      doc->RemoveFromIdTable(this, attrVal->GetAtomValue());
+    }
+  }
+  
+  UnsetFlags(NODE_HAS_ID);
+
   nsIAtom* IDName = GetIDAttributeName();
   if (IDName) {
     const nsAttrValue* attrVal = mAttrsAndChildren.GetAttr(IDName);
     if (attrVal) {
-      if (attrVal->Type() == nsAttrValue::eAtom) {
-        return attrVal->GetAtomValue();
-      }
-      if (attrVal->IsEmptyString()) {
-        return nsnull;
-      }
-      // Check if the ID has been stored as a string.
-      // This would occur if the ID attribute name changed after 
-      // the ID was parsed. 
+      SetFlags(NODE_HAS_ID);
       if (attrVal->Type() == nsAttrValue::eString) {
-        nsAutoString idVal(attrVal->GetStringValue());
+        nsString idVal(attrVal->GetStringValue());
 
         // Create an atom from the value and set it into the attribute list. 
         const_cast<nsAttrValue*>(attrVal)->ParseAtom(idVal);
-        return attrVal->GetAtomValue();
+      }
+      NS_ASSERTION(attrVal->Type() == nsAttrValue::eAtom,
+                   "Should be atom by now");
+      if (doc) {
+        doc->AddToIdTable(this, attrVal->GetAtomValue());
       }
     }
   }
-  return nsnull;
 }
 
 PRBool
 nsXMLElement::ParseAttribute(PRInt32 aNamespaceID,
                              nsIAtom* aAttribute,
                              const nsAString& aValue,
                              nsAttrValue& aResult)
 {
--- a/content/xml/content/src/nsXMLElement.h
+++ b/content/xml/content/src/nsXMLElement.h
@@ -73,11 +73,16 @@ public:
                               PRBool aCompileEventHandlers);
   virtual void UnbindFromTree(PRBool aDeep, PRBool aNullParent);
   virtual nsresult UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aAttribute,
                              PRBool aNotify);
   virtual PRBool ParseAttribute(PRInt32 aNamespaceID,
                                 nsIAtom* aAttribute,
                                 const nsAString& aValue,
                                 nsAttrValue& aResult);
+
+  // nsGenericElement overrides
+  virtual void NodeInfoChanged(nsINodeInfo* aOldNodeInfo);
+
+
 };
 
 #endif // nsXMLElement_h___
--- a/content/xtf/src/nsXTFElementWrapper.h
+++ b/content/xtf/src/nsXTFElementWrapper.h
@@ -150,16 +150,20 @@ public:
   nsresult CloneState(nsIDOMElement *aElement)
   {
     return GetXTFElement()->CloneState(aElement);
   }
   nsresult Clone(nsINodeInfo *aNodeInfo, nsINode **aResult) const;
 
   virtual nsXPCClassInfo* GetClassInfo() { return this; }
 
+  virtual void NodeInfoChanged(nsINodeInfo* aOldNodeInfo)
+  {
+  }
+
 protected:
   virtual nsIXTFElement* GetXTFElement() const
   {
     return mXTFElement;
   }
 
   static nsXPCClassInfo* GetBaseXPCClassInfo()
   {
--- a/content/xul/content/src/nsXULElement.cpp
+++ b/content/xul/content/src/nsXULElement.cpp
@@ -1374,16 +1374,20 @@ nsXULElement::UnsetAttr(PRInt32 aNameSpa
         GetAttributeNodeNS(ns, nsDependentAtomString(aName), getter_AddRefs(attrNode));
     }
 
     nsDOMSlots *slots = GetExistingDOMSlots();
     if (slots && slots->mAttributeMap) {
       slots->mAttributeMap->DropAttribute(aNameSpaceID, aName);
     }
 
+    // The id-handling code, and in the future possibly other code, need to
+    // react to unexpected attribute changes.
+    nsMutationGuard::DidMutate();
+
     nsAttrValue ignored;
     rv = mAttrsAndChildren.RemoveAttrAt(index, ignored);
     NS_ENSURE_SUCCESS(rv, rv);
 
     // XXX if the RemoveAttrAt() call fails, we might end up having removed
     // the attribute from the attribute map even though the attribute is still
     // on the element
     // https://bugzilla.mozilla.org/show_bug.cgi?id=296205