Bug 664919: Rewrite Node.normalize such that it's non-recursive and more resilient to mutation even weirdness. r=bz

content/base/public/nsINode.h

@@ -979,6 +979,8 @@ public:
     return NS_ERROR_NOT_IMPLEMENTED;
   }
 
+  nsresult Normalize();
+
   /**
    * Get the base URI for any relative URIs within this piece of
    * content. Generally, this is the document's base URI, but certain

content/base/src/nsDocument.cpp

@@ -5773,12 +5773,7 @@ nsDocument::CloneNode(PRBool aDeep, nsIDOMNode** aReturn)
 NS_IMETHODIMP
 nsDocument::Normalize()
 {
-  for (PRUint32 i = 0; i < mChildren.ChildCount(); ++i) {
-    nsCOMPtr<nsIDOMNode> node(do_QueryInterface(mChildren.ChildAt(i)));
-    node->Normalize();
-  }
-
-  return NS_OK;
+  return nsIDocument::Normalize();
 }
 
 NS_IMETHODIMP

content/base/src/nsGenericDOMDataNode.cpp

@@ -172,12 +172,6 @@ nsGenericDOMDataNode::GetPrefix(nsAString& aPrefix)
   return NS_OK;
 }
 
-nsresult
-nsGenericDOMDataNode::Normalize()
-{
-  return NS_OK;
-}
-
 nsresult
 nsGenericDOMDataNode::IsSupported(const nsAString& aFeature,
                                   const nsAString& aVersion,

content/base/src/nsGenericDOMDataNode.h

@@ -143,7 +143,6 @@ public:
     return NS_OK;
   }
   nsresult GetPrefix(nsAString& aPrefix);
-  nsresult Normalize();
   nsresult IsSupported(const nsAString& aFeature,
                        const nsAString& aVersion,
                        PRBool* aReturn);

content/base/src/nsGenericElement.cpp

@@ -541,6 +541,96 @@ nsINode::RemoveChild(nsIDOMNode* aOldChild, nsIDOMNode** aReturn)
   return rv;
 }
 
+nsresult
+nsINode::Normalize()
+{
+  // First collect list of nodes to be removed
+  nsAutoTArray<nsCOMPtr<nsIContent>, 50> nodes;
+
+  PRBool canMerge = PR_FALSE;
+  for (nsIContent* node = this->GetFirstChild();
+       node;
+       node = node->GetNextNode(this)) {
+    if (node->NodeType() != nsIDOMNode::TEXT_NODE) {
+      canMerge = PR_FALSE;
+      continue;
+    }
+
+    if (canMerge || node->TextLength() == 0) {
+      // No need to touch canMerge. That way we can merge across empty
+      // textnodes if and only if the node before is a textnode
+      nodes.AppendElement(node);
+    }
+    else {
+      canMerge = PR_TRUE;
+    }
+
+    // If there's no following sibling, then we need to ensure that we don't
+    // collect following siblings of our (grand)parent as to-be-removed
+    canMerge = canMerge && !!node->GetNextSibling();
+  }
+
+  if (nodes.IsEmpty()) {
+    return NS_OK;
+  }
+
+  // We're relying on mozAutoSubtreeModified to keep the doc alive here.
+  nsIDocument* doc = GetOwnerDoc();
+
+  // Batch possible DOMSubtreeModified events.
+  mozAutoSubtreeModified subtree(doc, nsnull);
+
+  // Fire all DOMNodeRemoved events. Optimize the common case of there being
+  // no listeners
+  PRBool hasRemoveListeners = nsContentUtils::
+      HasMutationListeners(doc, NS_EVENT_BITS_MUTATION_NODEREMOVED);
+  if (hasRemoveListeners) {
+    for (PRUint32 i = 0; i < nodes.Length(); ++i) {
+      nsContentUtils::MaybeFireNodeRemoved(nodes[i], nodes[i]->GetNodeParent(),
+                                           doc);
+    }
+  }
+
+  mozAutoDocUpdate batch(doc, UPDATE_CONTENT_MODEL, PR_TRUE);
+
+  // Merge and remove all nodes
+  nsAutoString tmpStr;
+  for (PRUint32 i = 0; i < nodes.Length(); ++i) {
+    nsIContent* node = nodes[i];
+    // Merge with previous node unless empty
+    const nsTextFragment* text = node->GetText();
+    if (text->GetLength()) {
+      nsIContent* target = node->GetPreviousSibling();
+      NS_ASSERTION((target && target->NodeType() == nsIDOMNode::TEXT_NODE) ||
+                   hasRemoveListeners,
+                   "Should always have a previous text sibling unless "
+                   "mutation events messed us up");
+      if (!hasRemoveListeners ||
+          (target && target->NodeType() == nsIDOMNode::TEXT_NODE)) {
+        if (text->Is2b()) {
+          target->AppendText(text->Get2b(), text->GetLength(), PR_TRUE);
+        }
+        else {
+          tmpStr.Truncate();
+          text->AppendTo(tmpStr);
+          target->AppendText(tmpStr.get(), tmpStr.Length(), PR_TRUE);
+        }
+      }
+    }
+
+    // Remove node
+    nsINode* parent = node->GetNodeParent();
+    NS_ASSERTION(parent || hasRemoveListeners,
+                 "Should always have a parent unless "
+                 "mutation events messed us up");
+    if (parent) {
+      parent->RemoveChildAt(parent->IndexOf(node), PR_TRUE);
+    }
+  }
+
+  return NS_OK;
+}
+
 nsresult
 nsINode::GetDOMBaseURI(nsAString &aURI) const
 {
@@ -2619,103 +2709,6 @@ nsGenericElement::HasAttributeNS(const nsAString& aNamespaceURI,
   return NS_OK;
 }
 
-nsresult
-nsGenericElement::JoinTextNodes(nsIContent* aFirst,
-                                nsIContent* aSecond)
-{
-  nsresult rv = NS_OK;
-  nsCOMPtr<nsIDOMText> firstText(do_QueryInterface(aFirst, &rv));
-
-  if (NS_SUCCEEDED(rv)) {
-    nsCOMPtr<nsIDOMText> secondText(do_QueryInterface(aSecond, &rv));
-
-    if (NS_SUCCEEDED(rv)) {
-      nsAutoString str;
-
-      rv = secondText->GetData(str);
-      if (NS_SUCCEEDED(rv)) {
-        rv = firstText->AppendData(str);
-      }
-    }
-  }
-
-  return rv;
-}
-
-nsresult
-nsGenericElement::Normalize()
-{
-  // We're relying on mozAutoSubtreeModified to keep the doc alive here.
-  nsIDocument* doc = GetOwnerDoc();
-
-  // Batch possible DOMSubtreeModified events.
-  mozAutoSubtreeModified subtree(doc, nsnull);
-
-  bool hasRemoveListeners = nsContentUtils::
-    HasMutationListeners(doc, NS_EVENT_BITS_MUTATION_NODEREMOVED);
-
-  nsresult result = NS_OK;
-  PRUint32 index, count = GetChildCount();
-
-  for (index = 0; (index < count) && (NS_OK == result); index++) {
-    nsIContent *child = GetChildAt(index);
-
-    switch (child->NodeType()) {
-      case nsIDOMNode::TEXT_NODE:
-
-        // ensure that if the text node is empty, it is removed
-        if (0 == child->TextLength()) {
-          if (hasRemoveListeners) {
-            nsContentUtils::MaybeFireNodeRemoved(child, this, doc);
-          }
-          result = RemoveChildAt(index, PR_TRUE);
-          if (NS_FAILED(result)) {
-            return result;
-          }
-
-          count--;
-          index--;
-          break;
-        }
-
-        if (index+1 < count) {
-          // Get the sibling. If it's also a text node, then
-          // remove it from the tree and join the two text
-          // nodes.
-          nsCOMPtr<nsIContent> sibling = GetChildAt(index + 1);
-
-          if (sibling->NodeType() == nsIDOMNode::TEXT_NODE) {
-            if (hasRemoveListeners) {
-              nsContentUtils::MaybeFireNodeRemoved(sibling, this, doc);
-            }
-            result = RemoveChildAt(index+1, PR_TRUE);
-            if (NS_FAILED(result)) {
-              return result;
-            }
-
-            result = JoinTextNodes(child, sibling);
-            if (NS_FAILED(result)) {
-              return result;
-            }
-            count--;
-            index--;
-          }
-        }
-        break;
-
-      case nsIDOMNode::ELEMENT_NODE:
-        nsCOMPtr<nsIDOMElement> element = do_QueryInterface(child);
-
-        if (element) {
-          result = element->Normalize();
-        }
-        break;
-    }
-  }
-
-  return result;
-}
-
 static nsXBLBinding*
 GetFirstBindingWithContent(nsBindingManager* aBmgr, nsIContent* aBoundElem)
 {

content/base/src/nsGenericElement.h

@@ -400,7 +400,6 @@ public:
   NS_IMETHOD GetAttributes(nsIDOMNamedNodeMap** aAttributes);
   NS_IMETHOD GetNamespaceURI(nsAString& aNamespaceURI);
   NS_IMETHOD GetPrefix(nsAString& aPrefix);
-  NS_IMETHOD Normalize();
   NS_IMETHOD IsSupported(const nsAString& aFeature,
                          const nsAString& aVersion, PRBool* aReturn);
   NS_IMETHOD HasAttributes(PRBool* aHasAttributes);
@@ -479,14 +478,6 @@ public:
    */
   nsresult LeaveLink(nsPresContext* aPresContext);
 
-  /**
-   * Take two text nodes and append the second to the first.
-   * @param aFirst the node which will contain first + second [INOUT]
-   * @param aSecond the node which will be appended
-   */
-  nsresult JoinTextNodes(nsIContent* aFirst,
-                         nsIContent* aSecond);
-
   /**
    * Check whether a spec feature/version is supported.
    * @param aObject the object, which should support the feature,

js/src/xpconnect/src/dom_quickstubs.qsconf

@@ -780,6 +780,10 @@ customMethodCalls = {
                 '    rv = nsGenericElement::doQuerySelectorAll(self, '
                 'arg0, getter_AddRefs(result));'
         },
+    'nsIDOMNode_Normalize': {
+        'thisType': 'nsINode',
+        'canFail': False
+        },
     'nsIDOMNode_GetBaseURI': {
         'thisType': 'nsINode',
         'canFail': False