Don't permit DocumentFragments in the node hierarchy.

Change-Id: Icae3e3e41b4315f975892d5aafeae62d4799036d
http://code.google.com/p/android/issues/detail?id=2735

3 files changed, 39 insertions, 57 deletions

diff --git a/luni/src/main/java/org/apache/harmony/xml/dom/DocumentImpl.java b/luni/src/main/java/org/apache/harmony/xml/dom/DocumentImpl.java
index d1cf342..c5e4054 100644
--- a/luni/src/main/java/org/apache/harmony/xml/dom/DocumentImpl.java
+++ b/luni/src/main/java/org/apache/harmony/xml/dom/DocumentImpl.java
@@ -77,10 +77,9 @@ public final class DocumentImpl extends InnerNodeImpl implements Document {
     public DocumentImpl(DOMImplementationImpl impl, String namespaceURI,
             String qualifiedName, DocumentType doctype, String inputEncoding) {
         super(null);
-
+        this.document = this;
         this.domImplementation = impl;
         this.inputEncoding = inputEncoding;
-        this.document = this;
 
         if (doctype != null) {
             appendChild(doctype);
@@ -402,18 +401,20 @@ public final class DocumentImpl extends InnerNodeImpl implements Document {
         return Node.DOCUMENT_NODE;
     }
 
-    @Override
-    public Node insertChildAt(Node newChild, int index) {
-        // Make sure we have at most one root element and one DTD element.
-        if (newChild instanceof Element && getDocumentElement() != null) {
+    /**
+     * Document elements may have at most one root element and at most one DTD
+     * element.
+     */
+    @Override public Node insertChildAt(Node toInsert, int index) {
+        if (toInsert instanceof Element && getDocumentElement() != null) {
             throw new DOMException(DOMException.HIERARCHY_REQUEST_ERR,
                     "Only one root element allowed");
-        } else if (newChild instanceof DocumentType && getDoctype() != null) {
+        }
+        if (toInsert instanceof DocumentType && getDoctype() != null) {
             throw new DOMException(DOMException.HIERARCHY_REQUEST_ERR,
                     "Only one DOCTYPE element allowed");
         }
-
-        return super.insertChildAt(newChild, index);
+        return super.insertChildAt(toInsert, index);
     }
 
     @Override public String getTextContent() {
@@ -521,10 +522,6 @@ public final class DocumentImpl extends InnerNodeImpl implements Document {
         }
 
         NodeImpl srcImpl = (NodeImpl) source;
-        if (srcImpl.document == null) {
-            return;
-        }
-
         for (Map.Entry<String, UserData> entry
                 : srcImpl.document.getUserDataMapForRead(srcImpl).entrySet()) {
             UserData userData = entry.getValue();
diff --git a/luni/src/main/java/org/apache/harmony/xml/dom/InnerNodeImpl.java b/luni/src/main/java/org/apache/harmony/xml/dom/InnerNodeImpl.java
index 6fcbfe0..911dc0e 100644
--- a/luni/src/main/java/org/apache/harmony/xml/dom/InnerNodeImpl.java
+++ b/luni/src/main/java/org/apache/harmony/xml/dom/InnerNodeImpl.java
@@ -19,6 +19,7 @@ package org.apache.harmony.xml.dom;
 import java.util.ArrayList;
 import java.util.List;
 import org.w3c.dom.DOMException;
+import org.w3c.dom.DocumentFragment;
 import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
 
@@ -95,38 +96,34 @@ public abstract class InnerNodeImpl extends LeafNodeImpl {
     }
 
     /**
-     * Inserts a new child node into this node at a given position. If the new
-     * node is already child of another node, it is first removed from there.
-     * This method is the generalization of the appendChild() and insertBefore()
-     * methods.
-     *
-     * @param newChild The new child node to add.
-     * @param index The index at which to insert the new child node.
-     *
-     * @return The node added.
-     *
-     * @throws DOMException If the attempted operation violates the XML/DOM
-     *         well-formedness rules.
+     * Inserts {@code newChild} at {@code index}. If it is already child of
+     * another node, it is removed from there.
      */
-    public Node insertChildAt(Node newChild, int index) throws DOMException {
-        LeafNodeImpl newChildImpl = (LeafNodeImpl) newChild;
+    Node insertChildAt(Node newChild, int index) throws DOMException {
+        if (newChild instanceof DocumentFragment) {
+            NodeList toAdd = newChild.getChildNodes();
+            for (int i = 0; i < toAdd.getLength(); i++) {
+                insertChildAt(toAdd.item(i), index + i);
+            }
+            return newChild;
+        }
 
-        if (document != null && newChildImpl.document != null && newChildImpl.document != document) {
+        LeafNodeImpl toInsert = (LeafNodeImpl) newChild;
+        if (toInsert.document != document) {
             throw new DOMException(DOMException.WRONG_DOCUMENT_ERR, null);
         }
-
-        if (newChildImpl.isParentOf(this)) {
+        if (toInsert.isParentOf(this)) {
             throw new DOMException(DOMException.HIERARCHY_REQUEST_ERR, null);
         }
 
-        if (newChildImpl.parent != null) {
-            int oldIndex = newChildImpl.index;
-            newChildImpl.parent.children.remove(oldIndex);
-            newChildImpl.parent.refreshIndices(oldIndex);
+        if (toInsert.parent != null) {
+            int oldIndex = toInsert.index;
+            toInsert.parent.children.remove(oldIndex);
+            toInsert.parent.refreshIndices(oldIndex);
         }
 
-        children.add(index, newChildImpl);
-        newChildImpl.parent = this;
+        children.add(index, toInsert);
+        toInsert.parent = this;
         refreshIndices(index);
 
         return newChild;
@@ -175,7 +172,6 @@ public abstract class InnerNodeImpl extends LeafNodeImpl {
         if (oldChildImpl.document != document) {
             throw new DOMException(DOMException.WRONG_DOCUMENT_ERR, null);
         }
-
         if (oldChildImpl.parent != this) {
             throw new DOMException(DOMException.HIERARCHY_REQUEST_ERR, null);
         }
@@ -188,26 +184,15 @@ public abstract class InnerNodeImpl extends LeafNodeImpl {
         return oldChild;
     }
 
+    /**
+     * Removes {@code oldChild} and adds {@code newChild} in its place. This
+     * is not atomic.
+     */
     public Node replaceChild(Node newChild, Node oldChild) throws DOMException {
-        LeafNodeImpl oldChildImpl = (LeafNodeImpl) oldChild;
-        LeafNodeImpl newChildImpl = (LeafNodeImpl) newChild;
-
-        if (oldChildImpl.document != document
-                || newChildImpl.document != document) {
-            throw new DOMException(DOMException.WRONG_DOCUMENT_ERR, null);
-        }
-
-        if (oldChildImpl.parent != this || newChildImpl.isParentOf(this)) {
-            throw new DOMException(DOMException.HIERARCHY_REQUEST_ERR, null);
-        }
-
-        int index = oldChildImpl.index;
-        children.set(index, newChildImpl);
-        oldChildImpl.parent = null;
-        newChildImpl.parent = this;
-        refreshIndices(index);
-
-        return oldChildImpl;
+        int index = ((LeafNodeImpl) oldChild).index;
+        removeChild(oldChild);
+        insertChildAt(newChild, index);
+        return oldChild;
     }
 
     public String getTextContent() throws DOMException {
diff --git a/luni/src/main/java/org/apache/harmony/xml/dom/NodeImpl.java b/luni/src/main/java/org/apache/harmony/xml/dom/NodeImpl.java
index 664d6b5..7c26112 100644
--- a/luni/src/main/java/org/apache/harmony/xml/dom/NodeImpl.java
+++ b/luni/src/main/java/org/apache/harmony/xml/dom/NodeImpl.java
@@ -20,7 +20,6 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import javax.xml.transform.TransformerException;
-import libcore.base.Objects;
 import org.apache.xml.serializer.utils.SystemIDResolver;
 import org.apache.xml.utils.URI;
 import org.w3c.dom.Attr;
@@ -63,6 +62,7 @@ public abstract class NodeImpl implements Node {
         }
     };
 
+    /** The containing document. Non-null. */
     DocumentImpl document;
 
     NodeImpl(DocumentImpl document) {