Don't permit DocumentFragments in the node hierarchy.

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

4 files changed, 107 insertions, 81 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) {
diff --git a/luni/src/test/java/tests/xml/DomTest.java b/luni/src/test/java/tests/xml/DomTest.java
index d7b0ef2..8477676 100644
--- a/luni/src/test/java/tests/xml/DomTest.java
+++ b/luni/src/test/java/tests/xml/DomTest.java
@@ -17,6 +17,26 @@
 package tests.xml;
 
 import dalvik.annotation.KnownFailure;
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.StringReader;
+import java.io.StringWriter;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.transform.OutputKeys;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerException;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.dom.DOMSource;
+import javax.xml.transform.stream.StreamResult;
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 import org.w3c.dom.Attr;
@@ -25,6 +45,7 @@ import org.w3c.dom.Comment;
 import org.w3c.dom.DOMException;
 import org.w3c.dom.DOMImplementation;
 import org.w3c.dom.Document;
+import org.w3c.dom.DocumentFragment;
 import org.w3c.dom.DocumentType;
 import org.w3c.dom.Element;
 import org.w3c.dom.Entity;
@@ -37,34 +58,12 @@ import org.w3c.dom.ProcessingInstruction;
 import org.w3c.dom.Text;
 import org.w3c.dom.TypeInfo;
 import org.w3c.dom.UserDataHandler;
-import org.xml.sax.InputSource;
-import org.xml.sax.SAXException;
-
-import javax.xml.parsers.DocumentBuilder;
-import javax.xml.parsers.DocumentBuilderFactory;
-import javax.xml.transform.OutputKeys;
-import javax.xml.transform.Transformer;
-import javax.xml.transform.TransformerException;
-import javax.xml.transform.TransformerFactory;
-import javax.xml.transform.dom.DOMSource;
-import javax.xml.transform.stream.StreamResult;
-import java.io.File;
-import java.io.FileWriter;
-import java.io.IOException;
-import java.io.StringReader;
-import java.io.StringWriter;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
 import static org.w3c.dom.UserDataHandler.NODE_ADOPTED;
 import static org.w3c.dom.UserDataHandler.NODE_CLONED;
 import static org.w3c.dom.UserDataHandler.NODE_IMPORTED;
 import static org.w3c.dom.UserDataHandler.NODE_RENAMED;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
 
 /**
  * Construct a DOM and then interrogate it.
@@ -1455,6 +1454,51 @@ public class DomTest extends TestCase {
         assertEquals(7, elements.getLength());
     }
 
+    /**
+     * Documents shouldn't contain document fragments.
+     * http://code.google.com/p/android/issues/detail?id=2735
+     */
+    public void testAddingADocumentFragmentAddsItsChildren() {
+        Element a = document.createElement("a");
+        Element b = document.createElement("b");
+        Element c = document.createElement("c");
+        DocumentFragment fragment = document.createDocumentFragment();
+        fragment.appendChild(a);
+        fragment.appendChild(b);
+        fragment.appendChild(c);
+
+        Node returned = menu.appendChild(fragment);
+        assertSame(fragment, returned);
+        NodeList children = menu.getChildNodes();
+        assertEquals(6, children.getLength());
+        assertTrue(children.item(0) instanceof Text); // whitespace
+        assertEquals(item, children.item(1));
+        assertTrue(children.item(2) instanceof Text); // whitespace
+        assertEquals(a, children.item(3));
+        assertEquals(b, children.item(4));
+        assertEquals(c, children.item(5));
+    }
+
+    public void testReplacingWithADocumentFragmentInsertsItsChildren() {
+        Element a = document.createElement("a");
+        Element b = document.createElement("b");
+        Element c = document.createElement("c");
+        DocumentFragment fragment = document.createDocumentFragment();
+        fragment.appendChild(a);
+        fragment.appendChild(b);
+        fragment.appendChild(c);
+
+        Node returned = menu.replaceChild(fragment, item);
+        assertSame(item, returned);
+        NodeList children = menu.getChildNodes();
+        assertEquals(5, children.getLength());
+        assertTrue(children.item(0) instanceof Text); // whitespace
+        assertEquals(a, children.item(1));
+        assertEquals(b, children.item(2));
+        assertEquals(c, children.item(3));
+        assertTrue(children.item(4) instanceof Text); // whitespace
+    }
+
     private class RecordingHandler implements UserDataHandler {
         final Set<String> calls = new HashSet<String>();
         public void handle(short operation, String key, Object data, Node src, Node dst) {