Bug 514425. Be a little more careful about our ClearForm calls. r=sicking
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 30 Sep 2009 18:56:50 -0400
changeset 33311 9f3d99c80da21e03c05accc693fa2bc791433913
parent 33310 4e46887f17bcf285ff61c04f1cddeb99fea1dfbd
child 33313 c5ff0838c35f25c14e271c5e0b68c208569174c5
push id9424
push userbzbarsky@mozilla.com
push dateWed, 30 Sep 2009 22:57:13 +0000
treeherdermozilla-central@9f3d99c80da2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking
bugs514425
milestone1.9.3a1pre
Bug 514425. Be a little more careful about our ClearForm calls. r=sicking
content/base/src/nsNodeUtils.cpp
content/html/content/src/nsGenericHTMLElement.cpp
--- a/content/base/src/nsNodeUtils.cpp
+++ b/content/base/src/nsNodeUtils.cpp
@@ -50,16 +50,17 @@
 #include "nsIDOMAttr.h"
 #include "nsCOMArray.h"
 #include "nsPIDOMWindow.h"
 #include "nsDocument.h"
 #ifdef MOZ_XUL
 #include "nsXULElement.h"
 #endif
 #include "nsBindingManager.h"
+#include "nsGenericHTMLElement.h"
 
 // This macro expects the ownerDocument of content_ to be in scope as
 // |nsIDocument* doc|
 #define IMPL_MUTATION_NOTIFICATION(func_, content_, params_)      \
   PR_BEGIN_MACRO                                                  \
   nsINode* node = content_;                                       \
   NS_ASSERTION(node->GetOwnerDoc() == doc, "Bogus document");     \
   if (doc) {                                                      \
@@ -217,22 +218,33 @@ nsNodeUtils::LastRelease(nsINode* aNode)
   // Kill properties first since that may run external code, so we want to
   // be in as complete state as possible at that time.
   if (aNode->IsNodeOfType(nsINode::eDOCUMENT)) {
     // Delete all properties before tearing down the document. Some of the
     // properties are bound to nsINode objects and the destructor functions of
     // the properties may want to use the owner document of the nsINode.
     static_cast<nsIDocument*>(aNode)->PropertyTable()->DeleteAllProperties();
   }
-  else if (aNode->HasProperties()) {
-    // Strong reference to the document so that deleting properties can't
-    // delete the document.
-    nsCOMPtr<nsIDocument> document = aNode->GetOwnerDoc();
-    if (document) {
-      document->PropertyTable()->DeleteAllPropertiesFor(aNode);
+  else {
+    if (aNode->HasProperties()) {
+      // Strong reference to the document so that deleting properties can't
+      // delete the document.
+      nsCOMPtr<nsIDocument> document = aNode->GetOwnerDoc();
+      if (document) {
+        document->PropertyTable()->DeleteAllPropertiesFor(aNode);
+      }
+    }
+
+    // I wonder whether it's faster to do the HasFlag check first....
+    if (aNode->IsNodeOfType(nsINode::eHTML_FORM_CONTROL) &&
+        aNode->HasFlag(ADDED_TO_FORM)) {
+      // Tell the form (if any) this node is going away.  Don't
+      // notify, since we're being destroyed in any case.
+      static_cast<nsGenericHTMLFormElement*>(aNode)->ClearForm(PR_TRUE,
+                                                               PR_FALSE);
     }
   }
   aNode->UnsetFlags(NODE_HAS_PROPERTIES);
 
   if (aNode->HasFlag(NODE_HAS_LISTENERMANAGER)) {
 #ifdef DEBUG
     if (nsContentUtils::IsInitialized()) {
       nsIEventListenerManager* manager =
--- a/content/html/content/src/nsGenericHTMLElement.cpp
+++ b/content/html/content/src/nsGenericHTMLElement.cpp
@@ -2264,24 +2264,18 @@ NS_IMPL_INT_ATTR_DEFAULT_VALUE(nsGeneric
 nsGenericHTMLFormElement::nsGenericHTMLFormElement(nsINodeInfo *aNodeInfo)
   : nsGenericHTMLElement(aNodeInfo)
 {
   mForm = nsnull;
 }
 
 nsGenericHTMLFormElement::~nsGenericHTMLFormElement()
 {
-  // Check that this element is not still the default content
-  // of its parent form.
-  NS_ASSERTION(!mForm || mForm->GetDefaultSubmitElement() != this,
-               "Content being destroyed is the default content");
-
-  // Clean up.  Set the form to nsnull so it knows we went away.
-  // Do not notify as the content is being destroyed.
-  ClearForm(PR_TRUE, PR_FALSE);
+  // Check that this element doesn't know anything about its form at this point.
+  NS_ASSERTION(!mForm, "How did we get here?");
 }
 
 NS_IMPL_QUERY_INTERFACE_INHERITED1(nsGenericHTMLFormElement,
                                    nsGenericHTMLElement,
                                    nsIFormControl)
 
 PRBool
 nsGenericHTMLFormElement::IsNodeOfType(PRUint32 aFlags) const